Add support for specifying custom test container images. #855

aznashwan · 2021-12-10T07:41:16Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

This patch adds an optional --test-images-file argument for specifying the path to a YAML file containing custom container image references to be used in tests.

This will allow for user selection of more recent or even custom test images, as well as prevent the need to update/re-ship cri-tools in case of issues related to the currently-hardcoded default images. (e.g. updating their manifests for new Window releases, see #853 )

Which issue(s) this PR fixes:

None.

Special notes for your reviewer:

None.

Does this PR introduce a user-facing change?

Added optional `--test-images-file` CLI flag for providing the path to a YAML file defining custom container image references to be used in tests, with the current user-overridable image keys being `defaultTestContainerImage` (the image used in the majority of tests), and `webServerTestImage`. (which is used in Networking-related tests)

linux-foundation-easycla · 2021-12-10T07:41:18Z

The committers are authorized under a signed CLA.

✅ Nashwan Azhari (fccfe65)

k8s-ci-robot · 2021-12-10T07:41:19Z

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
If you have done the above and are still having issues with the CLA being reported as unsigned, please log a ticket with the Linux Foundation Helpdesk: https://support.linuxfoundation.org/
Should you encounter any issues with the Linux Foundation Helpdesk, send a message to the backup e-mail support address at: [email protected]

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. I understand the commands that are listed here.

k8s-ci-robot · 2021-12-10T07:41:23Z

Welcome @aznashwan!

It looks like this is your first PR to kubernetes-sigs/cri-tools 🎉. 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/cri-tools 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. 😃

aznashwan · 2021-12-10T11:20:27Z

@k8s-ci-robot CLA recheck please

This patch makes the Windows Periodic workflow pass specific test images to CRITest. This will allow full control over the container images used in workflow runs as opposed to relying on the hardcoded defaults in `cri-tools` to be compatible with all the tested Windows releases. Depends-On: kubernetes-sigs/cri-tools#855 Signed-off-by: Nashwan Azhari <[email protected]>

aznashwan · 2021-12-14T15:40:20Z

Updated PR by:

enforcing TestImageList struct field YAML tags
moving YAML loading logic into method on the TestingContextType

This patch makes the Windows Periodic workflow pass specific test images to CRITest. This will allow full control over the container images used in workflow runs as opposed to relying on the hardcoded defaults in `cri-tools` to be compatible with all the tested Windows releases. Depends-On: kubernetes-sigs/cri-tools#855 Signed-off-by: Nashwan Azhari <[email protected]>

aznashwan · 2021-12-14T16:48:15Z

@k8s-ci-robot CLA recheck please?

aznashwan · 2021-12-14T17:46:06Z

@k8s-ci-robot another recheck please?

dealako · 2021-12-14T18:45:31Z

@aznashwan checking the EasyCLA logs and database, I don't see that you are authorized by EasyCLA. You can do one of two things:

Individual CLA: Execute the workflow and sign the DocuSign document, individual docs info, or
Employee CLA: Execute the corporate contributor workflow, request that your CLA manager add you to the approval list, then (after they have added you), perform the corporate workflow to acknowledge your affiliation, corporate docs info.

Either or both will allow you to pass the EasyCLA authorization.

aznashwan · 2021-12-14T19:02:36Z

@dealako many thanks for the quick follow-up, we'll get to looking into it on our end!

This patch makes the Windows Periodic workflow pass specific test images to CRITest. This will allow full control over the container images used in workflow runs as opposed to relying on the hardcoded defaults in `cri-tools` to be compatible with all the tested Windows releases. Depends-On: kubernetes-sigs/cri-tools#855 Signed-off-by: Nashwan Azhari <[email protected]>

aznashwan · 2021-12-17T14:04:37Z

@k8s-ci-robot /check-cla

aznashwan · 2021-12-20T12:46:39Z

/check-cla

WillsonHG · 2021-12-20T13:54:40Z

/check-cla

This patch makes the Windows Periodic workflow pass specific test images to CRITest. This will allow full control over the container images used in workflow runs as opposed to relying on the hardcoded defaults in `cri-tools` to be compatible with all the tested Windows releases. Depends-On: kubernetes-sigs/cri-tools#855 Signed-off-by: Nashwan Azhari <[email protected]>

aznashwan · 2021-12-27T13:39:50Z

Updated the PR to make the YAML tags conform to the linter's requirements. (namely had to un-capitalize the tags)

I've also retested this PR as part of the Windows Periodic Workflow here.

@feiskyer @Random-Liu considering that my CLA issues have been resolved and the PR is now hypothetically merge-able, could I please get some reviews on it?

feiskyer · 2021-12-29T02:30:12Z

Triggering github actions now

feiskyer · 2021-12-29T02:31:46Z

pkg/framework/test_context.go

@@ -69,6 +81,7 @@ func RegisterFlags() {
 	flag.StringVar(&TestContext.ReportPrefix, "report-prefix", "", "Optional prefix for JUnit XML reports. Default is empty, which doesn't prepend anything to the default name.")
 	flag.StringVar(&TestContext.ReportDir, "report-dir", "", "Path to the directory where the JUnit XML reports should be saved. Default is empty, which doesn't generate these reports.")
 	flag.StringVar(&TestContext.ImageServiceAddr, "image-endpoint", "", "Image service socket for client to connect.")
+	flag.StringVar(&TestContext.TestImagesFilePath, "test-images-file", "", "Optional path to a YAML file containing references to custom container images to be used in tests.")


could you also update the docs for this new flag?

Good catch, added here, thank you!

dcantah · 2022-01-03T22:53:42Z

@aznashwan I think you can re-push or close + re-open to trigger the CI. This should have fixed the critest dockershim failures. #867

This patch adds an optional `--test-images-file` argument for specifying the path to a YAML file containing custom container image references to be used in tests. Initial user-overridable image keys being `defaultTestContainerImage` (the image used in the majority of tests), and `webServerTestImage` (which is used in Networking-related tests) Signed-off-by: Nashwan Azhari <[email protected]>

feiskyer · 2022-01-04T03:16:28Z

reruning the tests now

aznashwan · 2022-01-06T16:15:42Z

Could I get a recheck on this please?

feiskyer

sorry for the delay

/lgtm

k8s-ci-robot · 2022-01-07T12:59:14Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aznashwan, feiskyer

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~~ [feiskyer]

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

This patch makes the Windows Periodic workflow pass specific test images to CRITest. This will allow full control over the container images used in workflow runs as opposed to relying on the hardcoded defaults in `cri-tools` to be compatible with all the tested Windows releases. Depends-On: kubernetes-sigs/cri-tools#855 Signed-off-by: Nashwan Azhari <[email protected]>

k8s-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Dec 10, 2021

k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Dec 10, 2021

k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Dec 10, 2021

k8s-ci-robot requested review from feiskyer and Random-Liu December 10, 2021 07:41

aznashwan force-pushed the parametrized-test-images branch from fcabcac to fccfe65 Compare December 14, 2021 15:37

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 14, 2021

aznashwan mentioned this pull request Dec 14, 2021

Integration: Enable CRITest on Windows 2022. containerd/containerd#6378

Merged

aznashwan mentioned this pull request Dec 14, 2021

EasyCLA Feedback & Issues kubernetes/org#3068

Closed

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Dec 20, 2021

aznashwan force-pushed the parametrized-test-images branch from fccfe65 to 0472da6 Compare December 27, 2021 13:35

feiskyer reviewed Dec 29, 2021

View reviewed changes

aznashwan force-pushed the parametrized-test-images branch from 0472da6 to 4026dfb Compare December 29, 2021 13:12

aznashwan force-pushed the parametrized-test-images branch from 4026dfb to 7bbcf89 Compare January 3, 2022 23:06

aznashwan closed this Jan 3, 2022

aznashwan reopened this Jan 3, 2022

feiskyer approved these changes Jan 7, 2022

View reviewed changes

k8s-ci-robot assigned feiskyer Jan 7, 2022

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

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

k8s-ci-robot merged commit cf75c45 into kubernetes-sigs:master Jan 7, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for specifying custom test container images. #855

Add support for specifying custom test container images. #855

aznashwan commented Dec 10, 2021 •

edited

Loading

linux-foundation-easycla bot commented Dec 10, 2021 •

edited

Loading

k8s-ci-robot commented Dec 10, 2021

k8s-ci-robot commented Dec 10, 2021

aznashwan commented Dec 10, 2021

aznashwan commented Dec 14, 2021

aznashwan commented Dec 14, 2021

aznashwan commented Dec 14, 2021

dealako commented Dec 14, 2021 •

edited

Loading

aznashwan commented Dec 14, 2021

aznashwan commented Dec 17, 2021

aznashwan commented Dec 20, 2021

WillsonHG commented Dec 20, 2021

aznashwan commented Dec 27, 2021

feiskyer commented Dec 29, 2021

feiskyer Dec 29, 2021

aznashwan Dec 29, 2021

dcantah commented Jan 3, 2022

feiskyer commented Jan 4, 2022

aznashwan commented Jan 6, 2022

feiskyer left a comment

k8s-ci-robot commented Jan 7, 2022

Add support for specifying custom test container images. #855

Add support for specifying custom test container images. #855

Conversation

aznashwan commented Dec 10, 2021 • edited Loading

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

linux-foundation-easycla bot commented Dec 10, 2021 • edited Loading

k8s-ci-robot commented Dec 10, 2021

k8s-ci-robot commented Dec 10, 2021

aznashwan commented Dec 10, 2021

aznashwan commented Dec 14, 2021

aznashwan commented Dec 14, 2021

aznashwan commented Dec 14, 2021

dealako commented Dec 14, 2021 • edited Loading

aznashwan commented Dec 14, 2021

aznashwan commented Dec 17, 2021

aznashwan commented Dec 20, 2021

WillsonHG commented Dec 20, 2021

aznashwan commented Dec 27, 2021

feiskyer commented Dec 29, 2021

feiskyer Dec 29, 2021

Choose a reason for hiding this comment

aznashwan Dec 29, 2021

Choose a reason for hiding this comment

dcantah commented Jan 3, 2022

feiskyer commented Jan 4, 2022

aznashwan commented Jan 6, 2022

feiskyer left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Jan 7, 2022

aznashwan commented Dec 10, 2021 •

edited

Loading

linux-foundation-easycla bot commented Dec 10, 2021 •

edited

Loading

dealako commented Dec 14, 2021 •

edited

Loading