-
Notifications
You must be signed in to change notification settings - Fork 510
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MULTIARCH-4559: config/v1/types_cluster_version.go: Add 'architecture' to the release structure #2024
base: master
Are you sure you want to change the base?
Conversation
…ructure As requested in the enhancement [1]. Motivation for the new property from the enhancement [2]: There were a few options discussed in lieu of introducing a new ClusterVersion status field and the potential risks for doing so. The alternatives are highlighted with reasoning given for why they were not pursued: * Default ImportMode to PreserveOriginal everywhere: single-arch-release users maybe concerned about import size and the lack of metadata like dockerImageLayers and dockerImageMetadata for manifestlisted imagestream tags. * Clusters with homogeneous nodes running the multi payload who do not want to import manifestlists: The clusters can either migrate to single arch payloads or manually toggle the importMode through the image config CRD. * CVO provides architecural knowledge to the cluster-image-registry-operator through a configmap or the image config CRD: To limit the risk of many external consumers using CVO's status field to determine that their cluster is multi-arch ready, the idea was to expose this information to the specific controller. This solution is not necessary as we let other controller implementers decide if the CVO's new status field is the best fit for their use case. [1]: https://github.com/openshift/enhancements/blob/36ef3f51e5b7bfb2d4af18d415ba762e0a0431ac/enhancements/multi-arch/dynamic-imagestream-importmode-setting.md#api-extensions [2]: https://github.com/openshift/enhancements/blob/36ef3f51e5b7bfb2d4af18d415ba762e0a0431ac/enhancements/multi-arch/dynamic-imagestream-importmode-setting.md#motivations-for-a-new-clusterversion-status-property
@wking: This pull request references MULTIARCH-4559 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.18.0" version, but no target version was set. In response to this:
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. |
Hello @wking! Some important instructions when contributing to openshift/api: |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: wking The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Generated using: $ make update
@wking: all tests passed! 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. |
// Valid values are 'Multi' and empty. | ||
// | ||
// +optional | ||
Architecture ClusterVersionArchitecture `json:"architecture,omitempty"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs featuregate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which feature gate would you like? The enhancement is pretty thin in that section.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the ImageStreamImportMode
featuregate implemented here: #1928
// value of the cluster architecture. In this context cluster | ||
// architecture means either a single architecture or a multi | ||
// architecture. | ||
// Valid values are 'Multi' and empty. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may very well end up extended with more detail in the near future, but starting here is acceptable.
As requested in the enhancement. Motivation for the new property from the enhancement: