kubeadm: cleanup the "master" taint on CP nodes during upgrade #110095
Conversation
- iniconfiguration.go: stop applying the "master" taint for new clusters; update related unit tests in _test.go - apply.go: Remove logic related to cleanup of the "master" label during upgrade - apply.go: Add cleanup of the "master" taint on CP nodes during upgrade - controlplane_nodes_test.go: remove test for old "master" taint on nodes (this needs backport to 1.24, because we have a kubeadm 1.25 vs kubernetes test suite 1.24 e2e test)
k8s-ci-robot
added
release-note
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: neolit123 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 |
k8s-ci-robot
added
the
approved
|
/milestone v1.25 |
k8s-ci-robot
added
area/kubeadm
area/test
sig/cluster-lifecycle
k8s-ci-robot
removed
do-not-merge/needs-sig
| // TODO: remove the legacy taint check in 1.25: | ||
| // https://github.com/kubernetes/kubeadm/issues/2200 | ||
| for _, cp := range controlPlanes.Items { | ||
| framework.ExpectNodeHasTaint(f.ClientSet, cp.GetName(), &corev1.Taint{Key: controlPlaneLabel, Effect: corev1.TaintEffectNoSchedule}) | ||
| framework.ExpectNodeHasTaint(f.ClientSet, cp.GetName(), &corev1.Taint{Key: controlPlaneLabelLegacy, Effect: corev1.TaintEffectNoSchedule}) |
There was a problem hiding this comment.
controlplane_nodes_test.go: remove test for old "master" taint
on nodes (this needs backport to 1.24, because we have a kubeadm
1.25 vs kubernetes test suite 1.24 e2e test)
due to our skew tests we also have to make this change in release-1.24.
otherwise this test job will fail:
https://k8s-testgrid.appspot.com/sig-cluster-lifecycle-kubeadm#kubeadm-kinder-latest-on-1-24
the 1.24 test suite will try to find both old + new taint on the CP nodes, but kubeadm 1.25 will only add the new taint.
| @@ -238,10 +238,10 @@ func RemoveOldControlPlaneLabel(client clientset.Interface) error { | |||
| return nil | |||
There was a problem hiding this comment.
RemoveOldControlPlaneLabel is not used anymore and can be removed as https://github.com/kubernetes/kubernetes/pull/110095/files#diff-fd7aac3195ba96f8f6beb0a559e48ebbc93ef87df4c197de70cacf9a97117c56L162.
There was a problem hiding this comment.
I missed that. It has a TODO linking to the issue, at least.
k8s-ci-robot
added
the
lgtm
| // If the old taint is present and the new taint is missing, patch the node with the new taint. | ||
| // When the old taint is missing, assume the user has manually untainted the node and take no action. | ||
| if !hasNewTaint && hasOldTaint { | ||
| // If the old taint is present remove it |
There was a problem hiding this comment.
If someone manually removes the new taint before they upgrade to v1.25, are we assured that the new taint is put back before we remove the old taint?
(I'd expected to see at least a warning that the old taint is being removed but the new taint isn't present).
There was a problem hiding this comment.
If someone manually removes the new taint before they upgrade to v1.25, are we assured that the new taint is put back before we remove the old taint?
I personally don't expect this use case. If someone manually removed the new taint we can assume they wanted it removed and we should not put it back in. There was a related bug report that kubeadm should not reintroduce removed taints - e.g. in single control plane node clusters.
(I'd expected to see at least a warning that the old taint is being removed but the new taint isn't present).
The output of "upgrade" will print a message that the old taint is being removed. Combined with the release note it should give sufficient awareness to users.
What type of PR is this?
/kind cleanup
/kind deprecation
What this PR does / why we need it:
for new clusters; update related unit tests in _test.go
during upgrade
during upgrade
on nodes (this needs backport to 1.24, because we have a kubeadm
1.25 vs kubernetes test suite 1.24 e2e test)
Which issue(s) this PR fixes:
xref kubernetes/kubeadm#2200
Special notes for your reviewer:
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: