-
Notifications
You must be signed in to change notification settings - Fork 1.5k
KEP-961: Bump Statefulset MaxUnavailable to beta (built on top of #4474) #5228
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
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: kerthcet <[email protected]>
Signed-off-by: kerthcet <[email protected]>
Signed-off-by: kerthcet <[email protected]>
Signed-off-by: kerthcet <[email protected]>
Signed-off-by: kerthcet <[email protected]>
Signed-off-by: Lucas Severo Alves <[email protected]>
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Edwinhr716 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 |
/assign @janetkuo |
cluster required to make on upgrade, in order to maintain previous behavior? | ||
- This is a new field, it will only be enabled after an upgrade. To maintain the | ||
previous behavior, you can disable the feature gate manually in Beta or leave | ||
the field unconfigured. |
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.
Perhaps we can discuss the case of when existing clusters already use this feature in alpha before upgrade.
Also, discuss what happens if the feature gate was disabled, and the field was set, before upgrading
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.
Added both cases, PTAL
If their StatefulSet rollingUpdate section has the field maxUnavailable specified with | ||
a value different than 1. | ||
The below command should show maxUnavailable value: | ||
a value different than 1. While in alpha and beta, the feature-gate needs to be enabled. |
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.
You can still set it to 1 when this feature is enabled, right? Perhaps they just need to check the existence of the maxUnavailable field, and that the feature gate is enabled.
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.
true, it can be both. During rollout though you won't see a behavioral change if it is set to 1, but you will see it if it set to anything else. As an example, LWS sets the value even if the feature flag was disabled. The only way we could tell it wasn't enabled by default was by testing it with a value larger than one.
Co-authored-by: Janet Kuo <[email protected]>
Co-authored-by: Janet Kuo <[email protected]>
Co-authored-by: Janet Kuo <[email protected]>
Co-authored-by: Janet Kuo <[email protected]>
Supersedes #4474
Metrics PR: kubernetes/kubernetes#130951