Skip to content

MULTIARCH-5318: Add tests for ImageStreamImportMode feature gate#29524

Merged
openshift-merge-bot[bot] merged 2 commits intoopenshift:masterfrom
Prashanth684:imagestreamimportmode
Feb 11, 2025
Merged

MULTIARCH-5318: Add tests for ImageStreamImportMode feature gate#29524
openshift-merge-bot[bot] merged 2 commits intoopenshift:masterfrom
Prashanth684:imagestreamimportmode

Conversation

@Prashanth684
Copy link
Copy Markdown
Contributor

@Prashanth684 Prashanth684 commented Feb 3, 2025

Add tests to exercise these scenarios under a featuregate:

  • verify that the imagestream tag's import mode corresponds to the desired.Architecture field in the status reported by CV. multi => PreserveOriginal, else Legacy.
  • verify that setting the ImageStreamImportMode field in the image.config.io spec to Legacy, will be the import mode for any newly created imagestream tag henceforth
  • verify that setting the ImageStreamImportMode field in the image.config.io spec to PreserveOriginal, will be the import mode for any newly created imagestream tag henceforth

@Prashanth684
update openshift/api
2158f4d
@Prashanth684
Add tests for ImageStreamImportMode feature gate
b8d6fd7 
Add tests to exercise these scenarios under a featuregate:
- verify that the imagestream tag's import mode corresponds to the desired.Architecture field in the status reported by CV. multi => PreserveOriginal, else Legacy.
- verify that setting the ImageStreamImportMode field in the image.config.io spec to Legacy, will be the import mode for any newly created imagestream tag henceforth
- verify that setting the ImageStreamImportMode field in the image.config.io spec to PreserveOriginal, will be the import mode for any newly created imagestream tag henceforth
@openshift-ci openshift-ci Bot added the vendor-update Touching vendor dir or related files label Feb 3, 2025
@Prashanth684
Copy link
Copy Markdown
Contributor Author

Prashanth684 commented Feb 3, 2025

/retest

@Prashanth684 Prashanth684 changed the title Add tests for ImageStreamImportMode feature gate MULTIARCH-5318: Add tests for ImageStreamImportMode feature gate Feb 3, 2025
@openshift-ci-robot
Copy link
Copy Markdown

openshift-ci-robot commented Feb 3, 2025 •
edited by openshift-ci Bot
Loading

@Prashanth684: This pull request references MULTIARCH-5318 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.19.0" version, but no target version was set.

Details

In response to this:

Add tests to exercise these scenarios under a featuregate:

  • verify that the imagestream tag's import mode corresponds to the desired.Architecture field in the status reported by CV. multi => PreserveOriginal, else Legacy.
  • verify that setting the ImageStreamImportMode field in the image.config.io spec to Legacy, will be the import mode for any newly created imagestream tag henceforth
  • verify that setting the ImageStreamImportMode field in the image.config.io spec to PreserveOriginal, will be the import mode for any newly created imagestream tag henceforth

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 openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Feb 3, 2025
flavianmissi
flavianmissi reviewed Feb 4, 2025
View reviewed changes
Comment thread test/extended/images/tagimportmode.go
oc := exutil.NewCLI("imagestream-api")

g.It("import mode should be PreserveOriginal or Legacy depending on desired.architecture field in the CV [apigroup:image.openshift.io]", func() {
TestImageStreamImportMode(g.GinkgoT(), oc)
Copy link
Copy Markdown
Member

@flavianmissi flavianmissi Feb 4, 2025

Choose a reason for hiding this comment

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

Given that the two test cases below cover both Legacy and PreserveOriginal, do we really need this test case? I can't really see what value it's adding, though I may be missing something.

Copy link
Copy Markdown
Contributor Author

@Prashanth684 Prashanth684 Feb 4, 2025

Choose a reason for hiding this comment

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

this test case is actually the only "necessary" one in my opinion because it changes the default import mode based on the CV's architecture. The other two test cases are specifically for testing the knob to change the import mode which I don't see being used frequently

Copy link
Copy Markdown
Contributor Author

@Prashanth684 Prashanth684 Feb 4, 2025

Choose a reason for hiding this comment

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

the main idea here is that we have these tests which are exercised behind the featuregate and once we let them run for a release (or maybe a few sprint cycles), we use that signal to promote the featuregate to default and then I was thinking we remove the two tests which change the import mode knob.

Copy link
Copy Markdown
Member

@flavianmissi flavianmissi Feb 5, 2025

Choose a reason for hiding this comment

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

ack, thanks for expanding!

@flavianmissi
Copy link
Copy Markdown
Member

flavianmissi commented Feb 4, 2025 •
edited
Loading

left a question but lgtm either way
/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Feb 4, 2025
@Prashanth684 Prashanth684 requested review from DennisPeriquet and removed request for knobunc February 4, 2025 18:52
@Prashanth684
Copy link
Copy Markdown
Contributor Author

Prashanth684 commented Feb 4, 2025

/retest

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Feb 4, 2025

@Prashanth684: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-aws-ovn-upgrade-rollback b8d6fd7 link false /test e2e-aws-ovn-upgrade-rollback

Full PR test history. Your PR dashboard.

Details

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

@sosiouxme
Copy link
Copy Markdown
Member

sosiouxme commented Feb 10, 2025

/approve

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Feb 10, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: flavianmissi, Prashanth684, sosiouxme

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

The pull request process is described here

Details 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

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 10, 2025
@Prashanth684 Prashanth684 added the acknowledge-critical-fixes-only Indicates if the issuer of the label is OK with the policy. label Feb 10, 2025
@openshift-merge-bot openshift-merge-bot Bot merged commit 7251cee into openshift:master Feb 11, 2025
@openshift-bot
Copy link
Copy Markdown
Contributor

openshift-bot commented Feb 11, 2025

[ART PR BUILD NOTIFIER]

Distgit: openshift-enterprise-tests
This PR has been included in build openshift-enterprise-tests-container-v4.19.0-202502110410.p0.g7251cee.assembly.stream.el9.
All builds following this will include this PR.

@Prashanth684 Prashanth684 mentioned this pull request Apr 10, 2025
Merged
@jatinsu jatinsu mentioned this pull request Sep 11, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@DennisPeriquet DennisPeriquet Awaiting requested review from DennisPeriquet

1 more reviewer

@flavianmissi flavianmissi flavianmissi left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@flavianmissi flavianmissi

Labels

acknowledge-critical-fixes-only Indicates if the issuer of the label is OK with the policy. approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. vendor-update Touching vendor dir or related files

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Prashanth684 @openshift-ci-robot @flavianmissi @sosiouxme @openshift-bot