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.

Sign up for GitHub

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

Jump to bottom

Add E2E test for NFD tainting feature #932

Merged
merged 1 commit into from
Dec 19, 2022
Merged

Add E2E test for NFD tainting feature #932

merged 1 commit into from
Dec 19, 2022

Conversation

fmuyassarov
Copy link
Member

@fmuyassarov fmuyassarov commented Oct 21, 2022 •
edited
Loading

This PR extends E2E to test NodeFeatureRule tainting functionality implemented in #910.

/hold
until #910 is not merged.

@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Oct 21, 2022
@netlify
Copy link

netlify bot commented Oct 21, 2022 •
edited
Loading

✅ Deploy Preview for kubernetes-sigs-nfd ready!

Name Link
🔨 Latest commit 3e29f5c
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-sigs-nfd/deploys/63a04dc3188592000825f2c4
😎 Deploy Preview https://deploy-preview-932--kubernetes-sigs-nfd.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 21, 2022
@fmuyassarov
Copy link
Member Author

/cc @marquiz
No rush to review it this week, can be after the KubeCon 😊

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 22, 2022
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 2, 2022
@fmuyassarov
Copy link
Member Author

I will still keep the hold until #910 is not merged. But I think it is ready to review.
/cc @marquiz @ArangoGutierrez

@fmuyassarov
Copy link
Member Author

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 7, 2022
@fmuyassarov
Copy link
Member Author

#910 has landed and this PR is rebased on top latest master.

@fmuyassarov
Copy link
Member Author

@marquiz I think we need to take this in also before the release.

@marquiz marquiz added this to the v.0.12.0 milestone Dec 8, 2022
@marquiz
Copy link
Contributor

marquiz commented Dec 8, 2022

I think we need to take this in also before the release.

Yes, definitely!

marquiz
marquiz reviewed Dec 9, 2022
View reviewed changes
Copy link
Contributor

@marquiz marquiz left a comment

Choose a reason for hiding this comment

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

Great work @fmuyassarov. Just two comments. Could you also rebase on the latest master (easier to test locally)?

test/e2e/utils/pod/pod.go Outdated Show resolved Hide resolved
test/e2e/node_feature_discovery.go Outdated Show resolved Hide resolved
@fmuyassarov fmuyassarov requested review from marquiz and removed request for jjacobelli and ArangoGutierrez December 12, 2022 12:06
@fmuyassarov
Copy link
Member Author

/test pull-node-feature-discovery-build-image-cross-generic

@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 15, 2022
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 16, 2022
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 16, 2022
@fmuyassarov
Copy link
Member Author

/hold
until I get successful results from local tests.

@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Dec 16, 2022
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 17, 2022
@fmuyassarov
Copy link
Member Author

/hold cancel
Tested locally. @marquiz PTAL.

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 17, 2022
marquiz
marquiz reviewed Dec 19, 2022
View reviewed changes
Copy link
Contributor

@marquiz marquiz left a comment

Choose a reason for hiding this comment

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

Thanks @fmuyassarov, e2e-tests now pass 🎉 Just two comments

test/e2e/node_feature_discovery.go Outdated Show resolved Hide resolved
test/e2e/node_feature_discovery.go Outdated Show resolved Hide resolved
@fmuyassarov fmuyassarov force-pushed the e2e-taints branch 2 times, most recently from 0e31782 to b7f77fa Compare December 19, 2022 11:22
@fmuyassarov
Add nfd E2E tests for tainting feature
3e29f5c 
Extend current E2E tests to check tainting feature of nfd implemented
in kubernetes-sigs#910

Signed-off-by: Feruzjon Muyassarov <feruzjon.muyassarov@intel.com>
marquiz
marquiz approved these changes Dec 19, 2022
View reviewed changes
Copy link
Contributor

@marquiz marquiz left a comment

Choose a reason for hiding this comment

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

Great work here @fmuyassarov! Good to get tainting covered by e2e 🚀
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 19, 2022
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: b5f43b380c90a7dfe404bc1f869518e364c45a77

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fmuyassarov, marquiz

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 19, 2022
@k8s-ci-robot k8s-ci-robot merged commit e684197 into kubernetes-sigs:master Dec 19, 2022
@marquiz marquiz mentioned this pull request Dec 20, 2022
Closed
22 tasks
@fmuyassarov fmuyassarov deleted the e2e-taints branch January 2, 2023 21:27
@fmuyassarov fmuyassarov restored the e2e-taints branch January 2, 2023 21:27
@fmuyassarov fmuyassarov deleted the e2e-taints branch January 2, 2023 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marquiz marquiz marquiz approved these changes

Assignees

@marquiz marquiz

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
v.0.12.0
Development

Successfully merging this pull request may close these issues.
3 participants
@fmuyassarov @marquiz @k8s-ci-robot