-
Notifications
You must be signed in to change notification settings - Fork 582
Remove upstream on-by-default cloud feature gates (CloudDualStackNodeIPs, DisableKubeletCloudCredentialProviders) #2249
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
Remove upstream on-by-default cloud feature gates (CloudDualStackNodeIPs, DisableKubeletCloudCredentialProviders) #2249
Conversation
|
Hello @JoelSpeed! Some important instructions when contributing to openshift/api: |
|
/approve cancel Just for now until I've cleared this with CI and the architects |
|
E2E failing because KASO is panicking, due to https://github.com/openshift/cluster-kube-apiserver-operator/blob/e41854880dcb1a9df3322c1494c93e9a9ac0babe/pkg/operator/configobservation/apienablement/observe_runtime_config.go#L19 @benluddy Do you know what would happen if we remove this particular line? |
We'll stop serving that GV... which we never wanted to serve to begin with outside of tech preview. I remember there was some pain during the rebase because both the feature and its related APIs went directly from beta/off to GA/on. The intention must have been to follow up the rebase by removing the --runtime-config behavior from KASO. I don't know why openshift/cluster-kube-apiserver-operator#1687 was closed. |
|
@benluddy I assume we don't want to forever gate this serving on the presence of the feature gate being passed, so, there are 2 paths forward I guess
I assume 2 is the actual only real path, is that a lot of maintenance burden do you think? Or is there a viable "we think we can reason why 1 is ok" path? |
54ba57c to
dec0a2e
Compare
|
/approve |
|
Having dropped ValidatingAdmissionPolicy from this PR, this is now ready to go CC @elmiko as sig-cloud representative |
elmiko
left a comment
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.
/lgtm
|
/label acknowledge-critical-fixes-only The CCMs dropped these last release, so these aren't leveraged anymore |
2 similar comments
3 similar comments
…IPs, DisableKubeletCloudCredentialProviders)
dec0a2e to
46fd228
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: elmiko, JoelSpeed 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 |
1 similar comment
|
/override ci/prow/e2e-aws-serial Suite succeeded, but timed out on deprovision |
|
@JoelSpeed: Overrode contexts on behalf of JoelSpeed: ci/prow/e2e-aws-serial 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-sigs/prow repository. |
1 similar comment
|
/retest-required |
|
@JoelSpeed: The following tests failed, say
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. |
b7d0ca2
into
openshift:master
|
[ART PR BUILD NOTIFIER] Distgit: ose-cluster-config-api |
These have been enabled since before March 2024.
Upstream:
We can therefore safely remove these now that we are running 1.32 everywhere.