MULTIARCH-4556: Introduce ImageStreamImportMode field in the image config #1928

Prashanth684 · 2024-06-11T20:42:53Z

This is the api implementation for openshift/enhancements#1605

Introduces ImageStreamImportMode field in the image config spec and status - the status will be consumed by apiserver to set importMode for all imagestreams
Introduce an ImageStreamImportMode tech preview feature gate
Add feature gate test

openshift-ci-robot · 2024-06-11T20:42:57Z

@Prashanth684: This pull request references MULTIARCH-4556 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.17.0" version, but no target version was set.

Details

In response to this:

This is the api implementation for openshift/enhancements#1605

Introduces ImageStreamImportMode field in the image config spec and status - the status will be consumed by apiserver to set importMode for all imagestreams

Introduce an ImageStreamImportMode tech preview feature gate

Add feature gate test

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 · 2024-06-11T20:42:59Z

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.

Prashanth684 · 2024-06-12T16:46:16Z

/test e2e-aws-ovn-techpreview

Prashanth684 · 2024-06-12T16:46:23Z

/test e2e-upgrade

Prashanth684 · 2024-06-12T21:02:47Z

verify and verify-crd-schema are failing as these are changes to a crd which has not been touched in a while.

config/v1/types_image.go

features/features.go

image/v1/tests/ImageStreamImportMode.yaml

deads2k · 2024-06-13T12:52:28Z

config/v1/types_image.go

 package v1

 import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+import imagev1 "github.com/openshift/api/image/v1"


this import shouldn't happen. It couples our configuration to an image API that may evolve independently.

It does in a way depend on how the image API evolves though..if there is a change in the image API, I would assume we would want that to reflect in this field. Also, if we don't base this on the image API, then what about the field in the openshiftapiserver imagepolicyconfig? Would it be ok if that depends on the image API ? It is set based on the import mode in the image config status.

config/v1/types_image.go

hack/update-protobuf.sh

config/v1/types_image.go

image/v1/tests/ImageStreamImportMode.yaml

openshiftcontrolplane/v1/types.go

When working on openshift#1928, noticed that the paylaod crds were not being generated after running a `make update`. This is because codegen is run after update-scripts. This PR removes update-payload-crds from update-scripts and runs it after codegen.

Prashanth684 · 2024-08-13T14:56:11Z

could not load schema check generation contexts from dir "/go/src/github.com/openshift/api/image/v1/tests": could not unmarshal YAML for type meta inspection: error converting YAML to JSON: yaml: line 7: mapping values are not allowed in this context

Still something wrong in the test files, from verify-crd-schema

ah..thanks for the catch..there was an indentation error in the ungated file. fixed it. The verify-crd-schema still fails, but think it is expected.

Prashanth684 · 2024-08-13T15:16:28Z

Please can you resolve the following

error in config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_images-CustomNoUpgrade.crd.yaml: ListsMustHaveSSATags: crd/images.config.openshift.io version/v1 field/^.spec.externalRegistryHostnames must set x-kubernetes-list-type
		error in config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_images-CustomNoUpgrade.crd.yaml: ListsMustHaveSSATags: crd/images.config.openshift.io version/v1 field/^.spec.registrySources.allowedRegistries must set x-kubernetes-list-type
		error in config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_images-CustomNoUpgrade.crd.yaml: ListsMustHaveSSATags: crd/images.config.openshift.io version/v1 field/^.spec.registrySources.blockedRegistries must set x-kubernetes-list-type
		error in config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_images-CustomNoUpgrade.crd.yaml: ListsMustHaveSSATags: crd/images.config.openshift.io version/v1 field/^.spec.registrySources.insecureRegistries must set x-kubernetes-list-type
		error in config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_images-CustomNoUpgrade.crd.yaml: ListsMustHaveSSATags: crd/images.config.openshift.io version/v1 field/^.spec.allowedRegistriesForImport must set x-kubernetes-list-type
		error in config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_images-CustomNoUpgrade.crd.yaml: ListsMustHaveSSATags: crd/images.config.openshift.io version/v1 field/^.status.externalRegistryHostnames must set x-kubernetes-list-type

SSA tags should be fixed as they come up

These are already existing fields in an already existing CRD. is the change simply to add // +listType=atomic ? Hopefully it won't impact the existing crd?

This is the api implementation for openshift/enhancements#1605 - Introduces ImageStreamImportMode field in the image config spec and status - the status will be consumed by apiserver to set importMode for all imagestreams - Introduce an `ImageStreamImportMode` tech preview feature gate - Add feature gate test

JoelSpeed · 2024-08-13T16:45:32Z

/lgtm
/override ci/prow/verify-crd-schema

openshift-ci · 2024-08-13T16:46:17Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JoelSpeed, Prashanth684

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:

~~OWNERS~~ [JoelSpeed]

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

openshift-ci · 2024-08-13T16:46:21Z

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

Details

In response to this:

/lgtm
/override ci/prow/verify-crd-schema

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.

openshift-ci-robot · 2024-08-13T17:31:44Z

/retest-required

Remaining retests: 0 against base HEAD e48b223 and 2 for PR HEAD fa3fbfe in total

Prashanth684 · 2024-08-13T21:42:15Z

/retest-required

Prashanth684 · 2024-08-14T02:56:41Z

/test e2e-upgrade-minor

Prashanth684 · 2024-08-14T03:44:40Z

/test e2e-upgrade-minor

JoelSpeed · 2024-08-14T08:01:19Z

/override ci/prow/e2e-upgrade-minor

The minor upgrade has been broken since branching, I'm working on fixing it. Looking at the changes here the only upgrade potential issue would be the new SSA tags, but they are all atomic which is the default anyway, so there's no actual functional change

openshift-ci · 2024-08-14T08:04:22Z

@JoelSpeed: Overrode contexts on behalf of JoelSpeed: ci/prow/e2e-upgrade-minor

Details

In response to this:

/override ci/prow/e2e-upgrade-minor

The minor upgrade has been broken since branching, I'm working on fixing it. Looking at the changes here the only upgrade potential issue would be the new SSA tags, but they are all atomic which is the default anyway, so there's no actual functional change

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.

openshift-ci · 2024-08-14T08:04:26Z

@Prashanth684: all tests passed!

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.

This PR openshift/api#1928 introduced a string field which has no omitempty tag. This result in our mashalling transparently changing. This produces a different configuration Hash. This PR drops the field when empty from the mashalled string to keep backward compatibility. Implementing this at the marshal operation level might result in undesired impact as we might pontetially modify other fields and ordering is not deterministic. The PR also increases test coverage using raw strings to make sure they fail if any input for our Hash generation ever changes transparently

This PR openshift/api#1928 introduced a string field which has no omitempty tag. This result in our mashalling transparently changing. This produces a different configuration Hash. This PR drops the field when empty from the mashalled string to keep backward compatibility. Implementing this at the marshal operation level might result in undesired impact as we might pontetially modify other fields and ordering is not deterministic. The PR also increases test coverage using raw strings to make sure they fail if any input for our Hash generation ever changes transparently (cherry picked from commit 045ba5c)

This PR openshift/api#1928 introduced a string field which has no omitempty tag. This result in our mashalling transparently changing. This produces a different configuration Hash. This PR drops the field when empty from the mashalled string to keep backward compatibility. Implementing this at the marshal operation level might result in undesired impact as we might pontetially modify other fields and ordering is not deterministic. The PR also increases test coverage using raw strings to make sure they fail if any input for our Hash generation ever changes transparently

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jun 11, 2024

openshift-ci bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jun 11, 2024

openshift-ci bot requested review from bparees and soltysh June 11, 2024 20:43

Prashanth684 force-pushed the api-importmode branch from e498a31 to 3275b7a Compare June 11, 2024 20:49

Prashanth684 requested review from deads2k and wking June 11, 2024 21:24

Prashanth684 self-assigned this Jun 11, 2024

Prashanth684 requested review from dmage and flavianmissi June 12, 2024 21:10

JoelSpeed reviewed Jun 13, 2024

View reviewed changes

deads2k reviewed Jun 13, 2024

View reviewed changes

JoelSpeed reviewed Jun 18, 2024

View reviewed changes

config/v1/types_image.go Show resolved Hide resolved

config/v1/types_image.go Outdated Show resolved Hide resolved

config/v1/types_image.go Outdated Show resolved Hide resolved

config/v1/types_image.go Show resolved Hide resolved

hack/update-protobuf.sh Outdated Show resolved Hide resolved

JoelSpeed reviewed Jun 25, 2024

View reviewed changes

config/v1/types_image.go Outdated Show resolved Hide resolved

image/v1/tests/ImageStreamImportMode.yaml Show resolved Hide resolved

image/v1/tests/ImageStreamImportMode.yaml Outdated Show resolved Hide resolved

openshiftcontrolplane/v1/types.go Outdated Show resolved Hide resolved

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 9, 2024

Prashanth684 force-pushed the api-importmode branch from 5fb1723 to 93e83ff Compare July 9, 2024 16:25

openshift-merge-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jul 9, 2024

Prashanth684 force-pushed the api-importmode branch from 93e83ff to d993f5b Compare July 11, 2024 16:55

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 11, 2024

JoelSpeed reviewed Jul 19, 2024

View reviewed changes

openshiftcontrolplane/v1/types.go Show resolved Hide resolved

openshiftcontrolplane/v1/types.go Show resolved Hide resolved

Prashanth684 force-pushed the api-importmode branch from a83b1bf to 1182481 Compare July 19, 2024 19:19

Prashanth684 mentioned this pull request Jul 26, 2024

Run update-payload-crds after update-codegen-crds #1974

Merged

Prashanth684 force-pushed the api-importmode branch from 1182481 to b921863 Compare August 2, 2024 17:27

Prashanth684 added 2 commits August 13, 2024 08:40

set x-kubernetes-list-type for image crd list fields

fa3fbfe

Prashanth684 force-pushed the api-importmode branch from 11edb6c to fa3fbfe Compare August 13, 2024 16:13

openshift-ci bot assigned JoelSpeed Aug 13, 2024

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 13, 2024

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 13, 2024

openshift-merge-bot bot merged commit b3ee44d into openshift:master Aug 14, 2024

Prashanth684 mentioned this pull request Sep 16, 2024

MULTIARCH-4559: config/v1/types_cluster_version.go: Add 'architecture' to the release structure #2024

Merged

openshift-ci-robot mentioned this pull request Nov 7, 2024

NO-JIRA: Change globalConfigString to drop empty imageStreamImportMode openshift/hypershift#4942

Merged

4 tasks

MULTIARCH-4556: Introduce ImageStreamImportMode field in the image config #1928

MULTIARCH-4556: Introduce ImageStreamImportMode field in the image config #1928

Uh oh!

Conversation

Prashanth684 commented Jun 11, 2024

Uh oh!

openshift-ci-robot commented Jun 11, 2024 • edited by openshift-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci bot commented Jun 11, 2024

Uh oh!

Prashanth684 commented Jun 12, 2024

Uh oh!

Prashanth684 commented Jun 12, 2024

Uh oh!

Prashanth684 commented Jun 12, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

deads2k Jun 13, 2024

Choose a reason for hiding this comment

Uh oh!

Prashanth684 Jun 14, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Prashanth684 commented Aug 13, 2024

Uh oh!

Prashanth684 commented Aug 13, 2024

Uh oh!

JoelSpeed commented Aug 13, 2024

Uh oh!

openshift-ci bot commented Aug 13, 2024

Uh oh!

openshift-ci bot commented Aug 13, 2024

Uh oh!

openshift-ci-robot commented Aug 13, 2024

Uh oh!

Prashanth684 commented Aug 13, 2024

Uh oh!

Prashanth684 commented Aug 14, 2024

Uh oh!

Prashanth684 commented Aug 14, 2024

Uh oh!

JoelSpeed commented Aug 14, 2024

Uh oh!

openshift-ci bot commented Aug 14, 2024

Uh oh!

openshift-ci bot commented Aug 14, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

openshift-ci-robot commented Jun 11, 2024 •

edited by openshift-ci bot

Loading