Set up CustomNoUpgrade copies of CRDs #1490
Conversation
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
Hello @JoelSpeed! Some important instructions when contributing to openshift/api: |
openshift-ci
bot
added
the
size/XXL
openshift-ci
bot
added
the
approved
JoelSpeed
changed the title
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
CustomNoUpgrade is even-more-cutting-edge than TechPreviewNoUpgrade [1]. Before this commit, custom resources with properties with: +openshift:enable:FeatureSets=TechPreviewNoUpgrade were being sharded into Default and TechPreviewNoUpgrade flavors, which left CustomNoUpgrade clusters with neither shard. With this commit, the CustomNoUpgrade cluster will get the same CRD as the TechPreviewNoUpgrade cluster. It's not entirely clear to me how folks would annotate a CustomNoUpgrade-only resource, because: +openshift:enable:FeatureSets=CustomNoUpgrade might not generate a: release.openshift.io/feature-set: TechPreviewNoUpgrade,Default manifest for the less-cutting-edge clusters. But we can work that out in future work, and get this commit in now to get CustomNoUpgrade back off the ground. Generated with: $ sed -i 's/openshift:enable:FeatureSets=TechPreviewNoUpgrade/openshift:enable:FeatureSets=CustomNoUpgrade,TechPreviewNoUpgrade/' $(git grep -l openshift:enable:FeatureSets=) [1]: openshift#370 (comment)
JoelSpeed
force-pushed
the
customnoupgrade-crds
branch
from
f5a463c to
744c871
Compare
Looks like a string that needs to be updated to the new |
Just pushed that as you commented, should be better now |
| @@ -57,7 +57,7 @@ type DNSSpec struct { | |||
| // infrastructure provider for DNS. | |||
| // When omitted, this means the user has no opinion and the platform is left | |||
| // to choose reasonable defaults. These defaults are subject to change over time. | |||
| // +openshift:enable:FeatureSets=TechPreviewNoUpgrade | |||
| // +openshift:enable:FeatureSets=CustomNoUpgrade;TechPreviewNoUpgrade | |||
There was a problem hiding this comment.
97802d4 doesn't describe the delimiter pivot from , to ;. Is that a restriction of the kubebuilder syntax?
There was a problem hiding this comment.
Yeah that's parts of the syntax, different values within the same set should be ; separated. If a validation rule has multiple different parameters, these are , separated, so you could have:
a:b:c=foo=1;2,bar=3;4
Then the code has the input foo: [1, 2], bar: [3, 4]
JoelSpeed
force-pushed
the
customnoupgrade-crds
branch
from
744c871 to
300d3b6
Compare
|
/retest These pass locally, wonder if there's some weird merging happening |
JoelSpeed
force-pushed
the
customnoupgrade-crds
branch
from
300d3b6 to
1fa9175
Compare
|
@JoelSpeed: 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/test-infra repository. I understand the commands that are listed here. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: JoelSpeed, wking 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:
Approvers can indicate their approval by writing |
This is a continuation of #1481
We need to have a copy of the CRD for CRDs that are non-default for all available FeatureSets (that is
Default,TechPreviewNoUpgradeandCustomNoUpgrade.Without these,
CustomNoUpgradeclusters cannot bootstrap as the CVO does not know which CRD to install.For now, this updates the
CustomNoUpgradefeatures to be identical toTechPreviewNoUpgrade, in the future, users should be able to make a distinction between the two. However, I suspect everything in TechPreviewNoUpgrade should be inCustomNoUpgradeas well.Note, we need openshift/kubernetes-sigs-controller-tools#15 to make this work, changes are pulled in manually now but will need to update the go.mod before merge
CC @wking