Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

e2e: add multiple labels to node #4224

Merged
merged 2 commits into from
Nov 3, 2023
Merged

e2e: add multiple labels to node #4224

merged 2 commits into from
Nov 3, 2023

Conversation

Madhu-1
Copy link
Collaborator

@Madhu-1 Madhu-1 commented Nov 2, 2023

update the e2e code to add multiple labels to the node at a time.

fixes: #4146

@Madhu-1 Madhu-1 added the ci/skip/multi-arch-build skip building on multiple architectures label Nov 2, 2023
@Madhu-1 Madhu-1 requested review from iPraveenParihar and a team November 2, 2023 11:15
@mergify mergify bot added the component/testing Additional test cases or CI work label Nov 2, 2023
@iPraveenParihar
Copy link
Contributor

iPraveenParihar commented Nov 2, 2023

Can we do the same for deleteNodeLabel() ?

@Madhu-1
Copy link
Collaborator Author

Madhu-1 commented Nov 2, 2023

Can we do the same for deleteNodeLabel() ?

i didnt noticed that earlier, Done same, PTAL

Copy link
Contributor

@iPraveenParihar iPraveenParihar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small nits

)

func createNodeLabel(f *framework.Framework, labelKey, labelValue string) error {
func addLabelsToNodes(f *framework.Framework, labels map[string]string) error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
func addLabelsToNodes(f *framework.Framework, labels map[string]string) error {
func addNodeLabels(f *framework.Framework, labels map[string]string) error {

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

addLabelsToNodes seems more opt to me, same is used in Kuberentes util as well.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought using deleteNodeLabels and addNodeLabels would be a good idea.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

followed the format from the kubernetes package as an example, lets see what others think, based on the will change this if required.

@Madhu-1 Madhu-1 requested a review from a team November 2, 2023 12:41
@nixpanic
Copy link
Member

nixpanic commented Nov 3, 2023

@Mergifyio rebase

update the e2e code to add multiple
labels to the node at a time.

fixes: ceph#4146

Signed-off-by: Madhu Rajanna <[email protected]>
update the e2e code to remove multiple
labels to the node at a time.

Signed-off-by: Madhu Rajanna <[email protected]>
Copy link
Contributor

mergify bot commented Nov 3, 2023

rebase

✅ Branch has been successfully rebased

@nixpanic
Copy link
Member

nixpanic commented Nov 3, 2023

@Mergifyio queue

Copy link
Contributor

mergify bot commented Nov 3, 2023

queue

🛑 The pull request has been removed from the queue default

The queue conditions cannot be satisfied due to failing checks.

You can take a look at Queue: Embarked in merge queue check runs for more details.

In case of a failure due to a flaky test, you should first retrigger the CI.
Then, re-embark the pull request into the merge queue by posting the comment
@mergifyio refresh on the pull request.

@mergify mergify bot added the ok-to-test Label to trigger E2E tests label Nov 3, 2023
@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.27

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.26

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.28

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.27

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-cephfs

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.26

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.27

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.28

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-rbd

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.26

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.28

@ceph-csi-bot ceph-csi-bot removed the ok-to-test Label to trigger E2E tests label Nov 3, 2023
@nixpanic
Copy link
Member

nixpanic commented Nov 3, 2023

/retest ci/centos/mini-e2e-helm/k8s-1.27

@nixpanic
Copy link
Member

nixpanic commented Nov 3, 2023

/retest ci/centos/mini-e2e-helm/k8s-1.27

not sure why it failed, e2e seems to report a panic logs

@nixpanic
Copy link
Member

nixpanic commented Nov 3, 2023

@Mergifyio requeue

Copy link
Contributor

mergify bot commented Nov 3, 2023

requeue

✅ The queue state of this pull request has been cleaned. It can be re-embarked automatically

@mergify mergify bot merged commit 5576800 into ceph:devel Nov 3, 2023
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci/skip/multi-arch-build skip building on multiple architectures cleanup component/testing Additional test cases or CI work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

e2e: enhance CreateNodeLabel() to take multiple labels
5 participants