Clarify node drain requirement for kubelet minor version upgrades #46359
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
|
Welcome @avestuk! |
|
[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
the
area/release-eng
k8s-ci-robot
added
language/en
✅ Pull request preview available for checkingBuilt without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
This change clarifies the requirement that nodes are drained before a minor version upgrade takes place. The old wording implied that a minor version upgrade of the kubelet was not supported at all. Co-authored-by: Tim Bannister <tim+github@scalefactory.com> Co-authored-by: Xander Grzywinski <xandergrzyw@gmail.com>
avestuk
force-pushed
the
kubelet-in-place-reword
branch
from
0f3eba4
to
3f622d2
Compare
|
@sftim @salaxander you both happy with the latest version? Also thank you for the reviews! |
| @@ -217,7 +217,7 @@ Optionally upgrade `kubelet` instances to **{{< skew currentVersion >}}** (or th | |||
|
|
|||
| {{< note >}} | |||
| Before performing a minor version `kubelet` upgrade, [drain](/docs/tasks/administer-cluster/safely-drain-node/) pods from that node. | |||
| In-place minor version `kubelet` upgrades are not supported. | |||
| In-place minor version `kubelet` upgrades, to a newer minor version of Kubernetes, without a node drain, are not supported. | |||
There was a problem hiding this comment.
I think the "to a newer minor version of Kubernetes" bit is redundant.
I also think the content of kubelet-managed directories is not necessarily cross-minor-version compatible. I'd like sig-node's input on highlighting the need to clear those directories before an in-place upgrade, something like this:
| In-place minor version `kubelet` upgrades, to a newer minor version of Kubernetes, without a node drain, are not supported. | |
| In-place minor version `kubelet` upgrades without draining pods and clearing the content of the kubelet-managed directories are not supported. |
There was a problem hiding this comment.
OK, so combining the thinking so far, I suggest:
| In-place minor version `kubelet` upgrades, to a newer minor version of Kubernetes, without a node drain, are not supported. | |
| In-place `kubelet` upgrades, without first draining pods from the node **and** clearing the content | |
| of the kubelet-managed directories, are only supported for upgrades to a newer patch release within | |
| the same minor version of Kubernetes. |
This makes for a problem, though: can we hyperlink “clearing the content of the kubelet-managed directories” to the specific text about how to do that? If not, we're almost certainly going to get issues filed about adding more detail.
There was a problem hiding this comment.
More information about this clearing of directories would definitely be good because the current documentation doesn't state anything about needing to do this - see https://kubernetes.io/docs/tasks/administer-cluster/kubeadm/upgrading-linux-nodes/
There was a problem hiding this comment.
@liggitt Is there somebody from sig-node you'd recommend asking for a consult on this?
There was a problem hiding this comment.
probably folks in https://github.com/kubernetes/community/tree/master/sig-node#technical-leads or delegates
cc @kubernetes/sig-node-pr-reviews
/cc @dchen1107 @derekwaynecarr @mrunalp
k8s-ci-robot
added
the
sig/node
Addresses the issue raised in #46356 where I suggested that the language here could be made more clear.
This change clarifies the requirement that nodes are drained before a minor version upgrade takes place. The old wording implied that a minor version upgrade of the kubelet was not supported at all.