specify Featuregates by cluster profile #1788
Conversation
|
Hello @deads2k! Some important instructions when contributing to openshift/api: |
openshift-ci
bot
added
the
size/XXL
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: deads2k 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 |
openshift-ci
bot
added
the
approved
|
well, clearly I broke soemthing. |
deads2k
force-pushed
the
featuregate-by-cluster-profile
branch
2 times, most recently
from
d1adc84
to
bcca5ad
Compare
|
/retest |
| ResponsiblePerson: "jhernand", | ||
| OwningProduct: ocpSpecific, | ||
| } | ||
| for clusterProfile, byFeatureSet := range b.statusByClusterProfileByFeatureSet { |
There was a problem hiding this comment.
Might be worth a comment to explain that this is initialised in the constructor so will never be an empty list
| reportProblemsToJiraComponent("cloud-provider"). | ||
| contactPerson("jspeed"). | ||
| productScope(ocpSpecific). | ||
| enableIn(Default, TechPreviewNoUpgrade). |
There was a problem hiding this comment.
Previously, anything that was in default automatically was in TPNU, is this something we have to worry about separately now?
There was a problem hiding this comment.
Previously, anything that was in default automatically was in TPNU, is this something we have to worry about separately now?
yes, in thinking about how to enable across many criteria (featuresets + clusterprofiles), disabled everywhere by default was easier to understand.
There was a problem hiding this comment.
Are there any feature gates in here today that aren't enabling a base level, ie only available in custom?
There was a problem hiding this comment.
ClusterAPIInstall is one example, assuming nothing panicking on that gate being not registered I think we are good to go
There was a problem hiding this comment.
ClusterAPIInstallis one example, assuming nothing panicking on that gate being not registered I think we are good to go
Seems to be.
There was a problem hiding this comment.
Is there not an equivalent file that needs to be cleaned up now?
There was a problem hiding this comment.
Is there not an equivalent file that needs to be cleaned up now?
Not yet, I think we need to keep the existing files so that hypershift doesn't insta-break
There was a problem hiding this comment.
Are the old files still being updated in tandem with the new ones?
There was a problem hiding this comment.
|
/test all |
|
/retest |
deads2k
force-pushed
the
featuregate-by-cluster-profile
branch
from
e162141
to
a65f80f
Compare
deads2k
force-pushed
the
featuregate-by-cluster-profile
branch
from
a65f80f
to
68c8c83
Compare
|
/override ci/prow/e2e-gcp This is systemically broken it seems. Given centrality, I'll take the chance. |
|
@deads2k: Overrode contexts on behalf of deads2k: ci/prow/e2e-gcp 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 kubernetes/test-infra repository. |
|
past the previous bootstrapping failures! |
Joel looks largely ok with it. Passed CI, I'm labelling to move forward and will fix IOUs as we go along. |
|
all green, no merges to head, merging. |
|
[ART PR BUILD NOTIFIER] This PR has been included in build ose-cluster-config-api-container-v4.16.0-202403120045.p0.ge3b1aac.assembly.stream.el9 for distgit ose-cluster-config-api. |
|
@deads2k: 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. |
This new construction allows us to specify different featuregate enablement for different clusterprofiles. We need to do this for ExternalOIDC.
It's possible that we need additional decision planes in the future (managed versus unmanaged has already come up for NetworkingLiveMigration), but this starts that separation.
I think this can merge independently, but will become far more useful once openshift/cluster-config-operator#411 merges after this.
/assign @JoelSpeed