-
Notifications
You must be signed in to change notification settings - Fork 14.6k
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
kubeadm: update docs for removed UpgradeAddonsBeforeControlPlane feature gate #46274
kubeadm: update docs for removed UpgradeAddonsBeforeControlPlane feature gate #46274
Conversation
👷 Deploy Preview for kubernetes-io-vnext-staging processing.
|
/cc @neolit123 |
✅ Pull request preview available for checkingBuilt without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
: This is as a **disabled** feature gate that was introduced for Kubernetes v1.28, in order to allow reactivating a legacy | ||
and deprecated behavior during cluster upgrade. For kubeadm versions prior to v1.28, kubeadm upgrades cluster addons (including | ||
CoreDNS and kube-proxy) immediately during `kubeadm upgrade apply`, regardless of whether there are other control plane | ||
instances that have not been upgraded. This may cause compatibility problems. Since v1.28, kubeadm defaults to a mode that | ||
always checks whether all the control plane instances have been upgraded before starting to upgrade the addons. This behavior | ||
is applied to both `kubeadm upgrade apply` and `kubeadm upgrade node`. kubeadm determines whether a control plane instance | ||
has been upgraded by checking whether the image of the kube-apiserver Pod has been upgraded. You must perform control plane | ||
instances upgrade sequentially or at least ensure that the last control plane instance upgrade is not started until all the | ||
other control plane instances have been upgraded completely, and the addons upgrade will be performed after the last control plane | ||
instance is upgraded. The deprecated `UpgradeAddonsBeforeControlPlane` feature gate gives you a chance to keep the old upgrade | ||
behavior. You should not need this old behavior; if you do, you should consider changing your cluster or upgrade processes. |
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.
Ideally, add all this text in to the main branch (which at time of writing documents v1.30), and then once it has merged, update a couple of lines to make it correct for v1.31
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.
thanks @sftim
Does that mean we can commit this to the main branch first? Will that affect the documentation content of v1.30? We expect this change to only be seen in v1.31 and later.
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.
On the contrary, my take of this change is that the gate can be used on v1.28-v1.30.
It is something that we should have documented since v1.28 and then removed in v1.31.
How about we document this into v1.30 (main) branch and then backport to v1.28-v1.29. The update we then need for v1.31 would be deleting or drastically simplify this paragraph because the gate has been removed.
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.
the gate existed as "deprecated" since 1.28 up until 1.31 under "List of deprecated feature gates:". it started as a deprecated FG. see https://v1-28.docs.kubernetes.io/docs/reference/setup-tools/kubeadm/kubeadm-init/#feature-gates
i don't understand why this change should be backported or targeting anything other than dev-1.31.
for v1.31 would be deleting or drastically simplify this paragraph because the gate has been removed.
this can be done, but we haven't done such simplifications for prior removals of FGs.
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.
A gate, even for one that is deprecated when first introduced, is still something a user can use. I misunderstood the whole changes proposed in this PR. Actually this PR is about moving rather than removing the description about the gate. That should be fine. I mean, we don't need to backport anything.
For v1.31, the "drastically simplified version" could be something like:
This gate has been removed. It was introduced in v1.28 as a deprecated feature and then removed in v1.31. For documentation on older versions, please switch to the corresponding website version.
I mean, we don't need to add more details for such a "born to be deprecated" feature in v1.31.
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.
This gate has been removed. It was introduced in v1.28 as a deprecated feature and then removed in v1.31. For documentation on older versions, please switch to the corresponding website version.
I mean, we don't need to add more details for such a "born to be deprecated" feature in v1.31.
SGTM
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.
thanks, updated. @neolit123 @tengqm
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 minus one nit
@sftim i also think dev-1.31 is the right branch
/sig cluster-lifecycle |
bca8c8c
to
5c11a74
Compare
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
LGTM label has been added. Git tree hash: c46f82aae88f0276d49af3318a623008e43aa312
|
…ure gate Co-authored-by: Lubomir I. Ivanov <[email protected]>
5c11a74
to
82348a6
Compare
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: tengqm 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 |
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
LGTM label has been added. Git tree hash: 3620fd4fa161f99e60422e39104ab129e7083d93
|
kubeadm: update docs for removed UpgradeAddonsBeforeControlPlane feature gate
Ref: kubernetes/kubernetes#124715