topology updater: add e2e tests #528

ffromani · 2021-05-18T09:05:31Z

Builds on top of #525 and add e2e tests for the new component.
Some refactoring was performed along the way to make room for the new tests; no intended changes in the behaviour of the existing tests.

Design Document: here
Issue: #333
Documentation: #526

k8s-ci-robot · 2021-05-18T09:05:39Z

Welcome @fromanirh!

It looks like this is your first PR to kubernetes-sigs/node-feature-discovery 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/node-feature-discovery has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2021-05-18T09:05:39Z

Hi @fromanirh. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

ffromani · 2021-05-18T09:32:24Z

reviewers please note the first 4 commits (up to Expose Topology manager scope, included) are part of #525 , and are included here for PR consistency sake and to make CI pass.

swatisehgal · 2021-05-18T09:43:30Z

/cc @marquiz @ArangoGutierrez

In case you would like to review the E2E tests corresponding to #525.

marquiz · 2021-09-16T14:48:13Z

Rebase and drop the vendor directory

/ok-to-test

marquiz

Drop the vendor/ dir

ffromani · 2022-08-09T10:09:10Z

the PR is now reivewable again, but depends on #854

marquiz

Good job @fromanirh, I was now actually able to run the e2e-tests successfully in our test cluster 🥳

I left just a few comments but after those I think we're good to go with this

test/e2e/utils/config.go

test/e2e/topology_updater.go

test/e2e/utils/pod.go

test/e2e/utils/noderesourcetopology.go

marquiz · 2022-08-10T12:04:48Z

We actually need to update our e2e-test-config in k8s test-infra before merging this (I'm on it)
/hold

In some cases (CI) it is useful to run NFD e2e tests using ephemeral clusters. To save time and bandwidth, it is also useful to prime the ephemeral cluster with the images under test. In these circumstances there is no risk of running a stale image, and having a `Always` PullPolicy hardcoded actually makes the whole exercise null. So we add a new option, disabled by default, to make the e2e manifest use the `IfNotPresent` pull policy, to effectively cover this use case. Signed-off-by: Francesco Romani <[email protected]>

marquiz

Getting there. Just a few more notes.

test/e2e/utils/pod.go

test/e2e/utils/config.go

marquiz · 2022-08-10T18:54:35Z

We also need kubernetes/test-infra#27106 to get merged before merging this one.

Co-authored-by: Swati Sehgal <[email protected]> Co-authored-by: Francesco Romani <[email protected]> Signed-off-by: Artyom Lukianov <[email protected]>

We need this fix kubernetes/kubernetes#110875 to have reliable tests, but up until we can bump the k/k deps to 1.25+, we can't consume it. So borrow it from k/k repo for the time being. Signed-off-by: Francesco Romani <[email protected]>

marquiz

Great persistence @fromanirh on this one 😊 I think this is good to go now 🎉
/lgtm

k8s-ci-robot · 2022-08-11T08:14:45Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fromanirh, 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:

~~OWNERS~~ [marquiz]

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

marquiz · 2022-08-11T08:16:43Z

...just need to get that test-infra pr merged in order to unhold this

ffromani · 2022-08-11T08:30:01Z

Thanks for the reviews @marquiz !

marquiz · 2022-08-11T12:26:56Z

/unhold

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label May 18, 2021

k8s-ci-robot requested review from adrianchiris and zvonkok May 18, 2021 09:05

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels May 18, 2021

ffromani mentioned this pull request May 18, 2021

Exposing Hardware Topology through CRDs in Node feature discovery #333

Closed

ffromani force-pushed the topology-updater-e2e-tests branch from 547a029 to 0be8192 Compare May 18, 2021 09:30

Tal-or mentioned this pull request May 18, 2021

WIP: Topology updater: TM policy from configz endpoint #530

Closed

Tal-or mentioned this pull request Jun 16, 2021

Allow excluding of specific resource accounting from NRT's objects #545

Closed

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 7, 2021

swatisehgal mentioned this pull request Jul 13, 2021

Introducing NFD Topology Updater exposing Resource hardware Topology info through CRs #525

Merged

ffromani changed the title ~~Topology updater: add e2e tests~~ WIP: Topology updater: add e2e tests Aug 4, 2021

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 4, 2021

ffromani force-pushed the topology-updater-e2e-tests branch from 0be8192 to 6d723f4 Compare August 4, 2021 14:07

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 4, 2021

ffromani force-pushed the topology-updater-e2e-tests branch 2 times, most recently from 602565e to b3d2819 Compare August 4, 2021 16:40

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 4, 2021

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Sep 16, 2021

marquiz requested changes Sep 16, 2021

View reviewed changes

ffromani force-pushed the topology-updater-e2e-tests branch from b3d2819 to 7fee801 Compare September 21, 2021 11:36

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 21, 2021

ffromani changed the title ~~WIP: Topology updater: add e2e tests~~ topology updater: add e2e tests Sep 21, 2021

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 21, 2021

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 8, 2022

ffromani force-pushed the topology-updater-e2e-tests branch 2 times, most recently from d9fa944 to a4252aa Compare August 9, 2022 10:08

ffromani changed the title ~~WIP: topology updater: add e2e tests~~ topology updater: add e2e tests Aug 9, 2022

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 9, 2022

ffromani force-pushed the topology-updater-e2e-tests branch from a4252aa to 90c138b Compare August 9, 2022 13:25

marquiz reviewed Aug 10, 2022

View reviewed changes

test/e2e/utils/config.go Show resolved Hide resolved

test/e2e/topology_updater.go Outdated Show resolved Hide resolved

test/e2e/utils/pod.go Show resolved Hide resolved

test/e2e/utils/noderesourcetopology.go Show resolved Hide resolved

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 10, 2022

ffromani force-pushed the topology-updater-e2e-tests branch from 90c138b to c67b918 Compare August 10, 2022 14:20

marquiz reviewed Aug 10, 2022

View reviewed changes

test/e2e/utils/pod.go Outdated Show resolved Hide resolved

test/e2e/utils/pod.go Outdated Show resolved Hide resolved

test/e2e/utils/config.go Show resolved Hide resolved

marquiz mentioned this pull request Aug 10, 2022

kubernetes-sigs/nfd: update e2e test config kubernetes/test-infra#27106

Merged

ffromani and others added 2 commits August 11, 2022 08:53

e2e: add basic topology updater test

7d37f72

Co-authored-by: Swati Sehgal <[email protected]> Co-authored-by: Francesco Romani <[email protected]> Signed-off-by: Artyom Lukianov <[email protected]>

ffromani force-pushed the topology-updater-e2e-tests branch from c67b918 to d70f8c6 Compare August 11, 2022 06:53

marquiz approved these changes Aug 11, 2022

View reviewed changes

k8s-ci-robot assigned marquiz Aug 11, 2022

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 11, 2022

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 11, 2022

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 11, 2022

k8s-ci-robot merged commit 65bdcaa into kubernetes-sigs:master Aug 11, 2022

ffromani deleted the topology-updater-e2e-tests branch August 11, 2022 12:30

marquiz mentioned this pull request Dec 20, 2022

Release v0.12.0 #985

Closed

22 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

topology updater: add e2e tests #528

topology updater: add e2e tests #528

ffromani commented May 18, 2021 •

edited

Loading

k8s-ci-robot commented May 18, 2021

k8s-ci-robot commented May 18, 2021

ffromani commented May 18, 2021

swatisehgal commented May 18, 2021 •

edited

Loading

marquiz commented Sep 16, 2021

marquiz left a comment

ffromani commented Aug 9, 2022

marquiz left a comment

marquiz commented Aug 10, 2022

marquiz left a comment

marquiz commented Aug 10, 2022

marquiz left a comment

k8s-ci-robot commented Aug 11, 2022

marquiz commented Aug 11, 2022

ffromani commented Aug 11, 2022

marquiz commented Aug 11, 2022

topology updater: add e2e tests #528

topology updater: add e2e tests #528

Conversation

ffromani commented May 18, 2021 • edited Loading

k8s-ci-robot commented May 18, 2021

k8s-ci-robot commented May 18, 2021

ffromani commented May 18, 2021

swatisehgal commented May 18, 2021 • edited Loading

marquiz commented Sep 16, 2021

marquiz left a comment

Choose a reason for hiding this comment

ffromani commented Aug 9, 2022

marquiz left a comment

Choose a reason for hiding this comment

marquiz commented Aug 10, 2022

marquiz left a comment

Choose a reason for hiding this comment

marquiz commented Aug 10, 2022

marquiz left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Aug 11, 2022

marquiz commented Aug 11, 2022

ffromani commented Aug 11, 2022

marquiz commented Aug 11, 2022

ffromani commented May 18, 2021 •

edited

Loading

swatisehgal commented May 18, 2021 •

edited

Loading