MULTIARCH-5190: Promote ImageStreamImportMode to default

[Prashanth684]

The ImageStreamImportMode feature gate was introduced in 4.18 and tests were added around the 4.19 timeframe. The feature has been tested for a while now and the sippy dashboard looks healthy. There are 3 tests that were added to exercise the testing of this feature.

[openshift-ci-robot]

@Prashanth684: This pull request references MULTIARCH-5190 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.

In response to this:

The ImageStreamImportMode feature gate was introduced in 4.18 and tests were added around the 4.19 timeframe. The feature has been tested for a while now and the sippy dashboard looks healthy. There are 3 tests that were added to exercise the testing of this feature.

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]

Hello @Prashanth684! Some important instructions when contributing to openshift/api:
API design plays an important part in the user experience of OpenShift and as such API PRs are subject to a high level of scrutiny to ensure they follow our best practices. If you haven't already done so, please review the OpenShift API Conventions and ensure that your proposed changes are compliant. Following these conventions will help expedite the api review process for your PR.

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Prashanth684
Once this PR has been reviewed and has the lgtm label, please assign joelspeed for approval. For more information see the Code Review Process.

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

Needs approval from an approver in each of these files:

• OWNERS

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

[Prashanth684]

verify-feature-promotion is failing with:

F0409 12:32:19.597966   10476 root.go:64] Error running codegen: error: only 0 tests found, need at least 5 for "ImageStreamImportMode" on {metal amd64 ha dual}
error: only 2 tests found, need at least 5 for "ImageStreamImportMode" on {aws amd64 ha }
error: only 2 tests found, need at least 5 for "ImageStreamImportMode" on {azure amd64 ha }
error: only 2 tests found, need at least 5 for "ImageStreamImportMode" on {gcp amd64 ha }
error: only 2 tests found, need at least 5 for "ImageStreamImportMode" on {vsphere amd64 ha }
error: only 0 tests found, need at least 5 for "ImageStreamImportMode" on {metal amd64 ha ipv4}
error: only 0 tests found, need at least 5 for "ImageStreamImportMode" on {metal amd64 ha ipv6}

The tests that were added seem to not be running for baremetal - not sure why that's the case. This feature is platform agnostic though so it shouldn't matter. There are 3 tests defined, but only 2 run because the importmode field already has the default Legacy set for all clusters today, so for a single arch cluster, the test that toggles the field to legacy will be skipped.

[Prashanth684]

/cc @JoelSpeed

is it a hard and fast requirement to have "5" tests ?

[JoelSpeed]

Could we see if we can work out why these aren't running on metal?

[JoelSpeed]

is it a hard and fast requirement to have "5" tests ?

Generally the expectation is that it shouldn't be too hard to write 5 tests for a feature. In this case, can you explain the test cases you have, and why they completely cover the feature and additional tests should not be needed?

[Prashanth684]

is it a hard and fast requirement to have "5" tests ?

Generally the expectation is that it shouldn't be too hard to write 5 tests for a feature. In this case, can you explain the test cases you have, and why they completely cover the feature and additional tests should not be needed?

The ImportMode default used to be Legacy. With this feature:

• The default for ImportMode depends on the CV's status.Desired.Architecture
• The default can also be changed through a knob in the images.config.openshift.io, ImageStreamImportMode which can be set to Legacy or PreserveOriginal.

There are three test cases. All the test cases check the value of ImportMode in a created imagestream tag's ImportPolicy.

• The first test checks that for an imagestream that is created, the ImportMode in the tag's ImportPolicy is set based on the ClusterVersion's desired.Architecture field in its status (if arch == Multi, the importMode should be PreserveOriginal, if not it should be Legacy)
• The second test checks that when the image config's ImageStreamImportMode is changed to Legacy (when the default is not Legacy), a newly created imagestream's ImportMode is also set to Legacy.
• The third test checks that when the image config's ImageStreamImportMode is changed to PreserveOriginal (when the default is not PreserveOriginal), a newly created imagestream's ImportMode is also set to PreserveOriginal.

Only two tests run due to the first part of the feature where the default import mode is set based on the CV's architecture in the desired status.

[Prashanth684]

Could we see if we can work out why these aren't running on metal?

ah.. i think it's because they are part of the serial test suite and metal only runs the parallel test suite:

[sig-imageregistry][OCPFeatureGate:ImageStreamImportMode][Serial] ImageStream API import mode should be PreserveOriginal or Legacy depending on desired.architecture field in the CV [apigroup:image.openshift.io] [Suite:openshift/conformance/serial]

[JoelSpeed]

/retest

[JoelSpeed]

/retest

[openshift-ci]

@Prashanth684: The following tests 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/verify	49d3888	link	true	/test verify
ci/prow/verify-client-go	49d3888	link	true	/test verify-client-go
ci/prow/e2e-aws-ovn-techpreview	49d3888	link	true	/test e2e-aws-ovn-techpreview
ci/prow/verify-feature-promotion	49d3888	link	true	/test verify-feature-promotion
ci/prow/e2e-aws-serial-techpreview	49d3888	link	true	/test e2e-aws-serial-techpreview

Full PR test history. Your PR dashboard.

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.

[JoelSpeed]

/override ci/prow/verify-crd-schema

Existing failure that cannot be resolved now

[JoelSpeed]

i think it's because they are part of the serial test suite and metal only runs the parallel test suite:

Metal runs the serial suite for the stable featureset, but doesn't currently for tech preview, this should be fixed so that we can get signal

[openshift-ci]

@JoelSpeed: Overrode contexts on behalf of JoelSpeed: ci/prow/verify-crd-schema

In response to this:

/override ci/prow/verify-crd-schema

Existing failure that cannot be resolved now

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.