WIP: Add FeatureGate toggling implementation change since v1.33 #50122
Conversation
k8s-ci-robot
added
do-not-merge/work-in-progress
k8s-ci-robot
added
the
size/S
✅ Pull request preview available for checkingBuilt without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
| render: false | ||
|
|
||
| stages: | ||
| - stage: beta |
There was a problem hiding this comment.
For this kind of shit as gate features, can we start from "Alpha"?
We as developers should avoid changing user land whenever possible.
If we have to do it, to fix a serious bug, we just fix it.
Why are we introducing a feature gate for such a change?
Please think from users' perspective. I, as a user, have no idea why this change is supposed to be disabled, or enabled. Even I do understand the corner case and its impact on my workloads, I have no idea which components this feature affects. Then I have to restart all my control-plane pods and all kubelets on all nodes. Is this a scenario we the development community is happy with?
There was a problem hiding this comment.
@tengqm
Thank you for your comment.
This feature is an implementation design change of KEP-3243.
We consider the risk to be low and the FeatureGate of KEP-3243 MatchLabelKeysInPodTopologySpread has been true since v1.27.
Therefore in our discussion, we decided to add the new FeatureGate MatchLabelKeysInPodTopologySpreadSelectorMerge to control whether this change enable or disable, and start at the same level as MatchLabelKeysInPodTopologySpread(beta).
There was a problem hiding this comment.
Thanks for the clarification, @mochizuki875.
I may sound too harsh for a change that has been thoroughly discussed, but please don't take it personal. I've been watching feature gate changes for many years. One thing that concerns me is that some developers are abusing feature gates for things that they are not sure about. They are doing this without thinking from user's perspective. For example, when we are introducing a new behavior change to a particular component, say kube-scheduler, we may add a configuration option to the scheduler config file, or a command line flag. Feature gates are heavy. Given that we cannot tell which gate is relevant to which component, the safest way to make a change is to restart all control-plane components, and all node agents including kubelet and kube-proxy. For a large scale cluster, this process is not trivial. We develop features or propose bug fixes to make people's lives easier, not harder. We provide clear documentation for them to know why there is such a gate and what are the pros and cons for enabling/disabling it. Kubernetes are used in many production environments, we are not supposed to treat changes like this as developer-facing experiments. By saying this, I'm talking about a lot of shit gates we encountered lately.
There was a problem hiding this comment.
@tengqm
OK, I've added additional description about the FeatureGate in latest commit.
Do you think there is anything else we should include?
In addition, the code change(#129874) may not meet the v1.33 code freeze.
Therefore I'll revert docs change(#49607) and close this PR.
After that, I'll submit new PR merging them.
There was a problem hiding this comment.
No worry. Let's put this on hold. We'll only lift the hold when the upstream change is merged.
/hold
There was a problem hiding this comment.
@tengqm
Thank you.
Since the code change(#129874) has not meet the v1.33 code freeze, I've submitted revert PR(#50164) which remove the description of this feature from dev-1.33 branch.
For the next step, which option do you think is better? Alternatively, do you have any other suggestions?
- Keep this featuregate PR and resubmit new PR including the description of this feature(#49607).
- Merge the description of this feature(#49607) into this PR and retitle
- Close this featuregate PR and resubmit new PR merging the description of this feature(#49607)
Anyway, I think I need to change the target branch dev-v1.33 to dev-1.34(when created).
There was a problem hiding this comment.
In that case, I'd suggest we close this one and resubmit a new one when the upstream code change is ready.
k8s-ci-robot
added
size/XL
mochizuki875
force-pushed
the
fg_matchlabelkeys_podtopologyspread_design_change
branch
from
0837480 to
44b77c2
Compare
k8s-ci-robot
added
area/blog
👷 Deploy Preview for kubernetes-io-vnext-staging processing.
|
mochizuki875
force-pushed
the
fg_matchlabelkeys_podtopologyspread_design_change
branch
from
44b77c2 to
99a018c
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
k8s-ci-robot
added
size/S
| Before v1.33, kube-scheduler just internally handled `matchLabelKeys` before the calculation of scheduling results. | ||
| Since v1.33, merging selectors built from `matchLabelKeys` into `labelSelector` is enabled by default. | ||
| You can disable it and revert to the previous behavior by disabling the `MatchLabelKeysInPodTopologySpreadSelectorMerge` | ||
| [feature gate](/docs/reference/command-line-tools-reference/feature-gates/) of kube-apiserver. |
There was a problem hiding this comment.
This clarification means we only need to toggle the gate on the API server, right?
That is nice.
There was a problem hiding this comment.
This clarification means we only need to toggle the gate on the API server, right?
Yes, that's right.
| render: false | ||
|
|
||
| stages: | ||
| - stage: beta |
There was a problem hiding this comment.
No worry. Let's put this on hold. We'll only lift the hold when the upstream change is merged.
/hold
k8s-ci-robot
added
the
do-not-merge/hold
|
Since kubernetes/kubernetes#129874 have not meet the v1.33 code freeze, I've closed this PR. /close |
|
@mochizuki875: Closed this PR. 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. |
Description
This PR add the description about new FeatureGate toggling implementation change since v1.33.
related: