Merge pull request #107533 from neolit123/1.24-update-master-label-taint

kubeadm: apply "master" label/taint migration for 1.24

cmd/kubeadm/app/apis/kubeadm/types.go

@@ -212,9 +212,9 @@ type NodeRegistrationOptions struct {
 	// CRISocket is used to retrieve container runtime info. This information will be annotated to the Node API object, for later re-use
 	CRISocket string
 
-	// Taints specifies the taints the Node API object should be registered with. If this field is unset, i.e. nil, in the `kubeadm init` process
+	// Taints specifies the taints the Node API object should be registered with. If this field is unset, i.e. nil,
-	// it will be defaulted to []v1.Taint{'node-role.kubernetes.io/master=""'}. If you don't want to taint your control-plane node, set this field to an
+	// it will be defaulted with a control-plane taint for control-plane nodes. If you don't want to taint your control-plane
-	// empty slice, i.e. `taints: []` in the YAML file. This field is solely used for Node registration.
+	// node, set this field to an empty slice, i.e. `taints: []` in the YAML file. This field is solely used for Node registration.
 	Taints []v1.Taint
 
 	// KubeletExtraArgs passes through extra arguments to the kubelet. The arguments here are passed to the kubelet command line via the environment file

cmd/kubeadm/app/apis/kubeadm/v1beta2/doc.go

@@ -172,7 +172,7 @@ limitations under the License.
 // 	  criSocket: "unix:///var/run/containerd/containerd.sock"
 // 	  taints:
 // 	  - key: "kubeadmNode"
-// 	    value: "master"
+// 	    value: "someValue"
 // 	    effect: "NoSchedule"
 // 	  kubeletExtraArgs:
 // 	    v: 4

cmd/kubeadm/app/apis/kubeadm/v1beta2/types.go

@@ -201,9 +201,9 @@ type NodeRegistrationOptions struct {
 	// CRISocket is used to retrieve container runtime info. This information will be annotated to the Node API object, for later re-use
 	CRISocket string `json:"criSocket,omitempty"`
 
-	// Taints specifies the taints the Node API object should be registered with. If this field is unset, i.e. nil, in the `kubeadm init` process
+	// Taints specifies the taints the Node API object should be registered with. If this field is unset, i.e. nil,
-	// it will be defaulted to []v1.Taint{'node-role.kubernetes.io/master=""'}. If you don't want to taint your control-plane node, set this field to an
+	// it will be defaulted with a control-plane taint for control-plane nodes. If you don't want to taint your control-plane
-	// empty slice, i.e. `taints: []` in the YAML file. This field is solely used for Node registration.
+	// node, set this field to an empty slice, i.e. `taints: []` in the YAML file. This field is solely used for Node registration.
 	Taints []v1.Taint `json:"taints"`
 
 	// KubeletExtraArgs passes through extra arguments to the kubelet. The arguments here are passed to the kubelet command line via the environment file

cmd/kubeadm/app/apis/kubeadm/v1beta3/doc.go

@@ -176,7 +176,7 @@ limitations under the License.
 // 	  criSocket: "unix:///var/run/containerd/containerd.sock"
 // 	  taints:
 // 	  - key: "kubeadmNode"
-// 	    value: "master"
+// 	    value: "someValue"
 // 	    effect: "NoSchedule"
 // 	  kubeletExtraArgs:
 // 	    v: 4

cmd/kubeadm/app/apis/kubeadm/v1beta3/types.go

@@ -215,9 +215,9 @@ type NodeRegistrationOptions struct {
 	// +optional
 	CRISocket string `json:"criSocket,omitempty"`
 
-	// Taints specifies the taints the Node API object should be registered with. If this field is unset, i.e. nil, in the `kubeadm init` process
+	// Taints specifies the taints the Node API object should be registered with. If this field is unset, i.e. nil,
-	// it will be defaulted to []v1.Taint{'node-role.kubernetes.io/master=""'}. If you don't want to taint your control-plane node, set this field to an
+	// it will be defaulted with a control-plane taint for control-plane nodes. If you don't want to taint your control-plane
-	// empty slice, i.e. `taints: []` in the YAML file. This field is solely used for Node registration.
+	// node, set this field to an empty slice, i.e. `taints: []` in the YAML file. This field is solely used for Node registration.
 	Taints []corev1.Taint `json:"taints"`
 
 	// KubeletExtraArgs passes through extra arguments to the kubelet. The arguments here are passed to the kubelet command line via the environment file

cmd/kubeadm/app/cmd/join.go

@@ -64,7 +64,7 @@ var (
 
 		* Certificate signing request was sent to apiserver and approval was received.
 		* The Kubelet was informed of the new secure connection details.
-		* Control plane (master) label and taint were applied to the new node.
+		* Control plane label and taint were applied to the new node.
 		* The Kubernetes control plane instances scaled up.
 		{{.etcdMessage}}
 

cmd/kubeadm/app/cmd/upgrade/apply.go

@@ -157,9 +157,19 @@ func runApply(flags *applyFlags, args []string) error {
 	}
 
 	// TODO: https://github.com/kubernetes/kubeadm/issues/2200
-	fmt.Printf("[upgrade/postupgrade] Applying label %s='' to Nodes with label %s='' (deprecated)\n",
+	fmt.Printf("[upgrade/postupgrade] Removing the deprecated label %s='' from all control plane Nodes. "+
-		kubeadmconstants.LabelNodeRoleControlPlane, kubeadmconstants.LabelNodeRoleOldControlPlane)
+		"After this step only the label %s='' will be present on control plane Nodes.\n",
-	if err := upgrade.LabelOldControlPlaneNodes(client); err != nil {
+		kubeadmconstants.LabelNodeRoleOldControlPlane, kubeadmconstants.LabelNodeRoleControlPlane)
+	if err := upgrade.RemoveOldControlPlaneLabel(client); err != nil {
+		return err
+	}
+
+	// TODO: https://github.com/kubernetes/kubeadm/issues/2200
+	fmt.Printf("[upgrade/postupgrade] Adding the new taint %s to all control plane Nodes. "+
+		"After this step both taints %s and %s should be present on control plane Nodes.\n",
+		kubeadmconstants.ControlPlaneTaint.String(), kubeadmconstants.ControlPlaneTaint.String(),
+		kubeadmconstants.OldControlPlaneTaint.String())
+	if err := upgrade.AddNewControlPlaneTaint(client); err != nil {
 		return err
 	}
 

cmd/kubeadm/app/phases/markcontrolplane/markcontrolplane.go

@@ -19,31 +19,23 @@ package markcontrolplane
 import (
 	"fmt"
 
-	"k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 	clientset "k8s.io/client-go/kubernetes"
 
 	"k8s.io/kubernetes/cmd/kubeadm/app/constants"
 	"k8s.io/kubernetes/cmd/kubeadm/app/util/apiclient"
 )
 
+// labelsToAdd holds a list of labels that are applied on kubeadm managed control plane nodes
 var labelsToAdd = []string{
-	// TODO: remove this label:
-	// https://github.com/kubernetes/kubeadm/issues/2200
-	constants.LabelNodeRoleOldControlPlane,
 	constants.LabelNodeRoleControlPlane,
 	constants.LabelExcludeFromExternalLB,
 }
 
 // MarkControlPlane taints the control-plane and sets the control-plane label
 func MarkControlPlane(client clientset.Interface, controlPlaneName string, taints []v1.Taint) error {
-	// TODO: remove this "deprecated" amend and pass "labelsToAdd" directly:
-	// https://github.com/kubernetes/kubeadm/issues/2200
-	labels := make([]string, len(labelsToAdd))
-	copy(labels, labelsToAdd)
-	labels[0] = constants.LabelNodeRoleOldControlPlane + "(deprecated)"
-
 	fmt.Printf("[mark-control-plane] Marking the node %s as control-plane by adding the labels: %v\n",
-		controlPlaneName, labels)
+		controlPlaneName, labelsToAdd)
 
 	if len(taints) > 0 {
 		taintStrs := []string{}

cmd/kubeadm/app/phases/markcontrolplane/markcontrolplane_test.go

@@ -49,26 +49,25 @@ func TestMarkControlPlane(t *testing.T) {
 			existingLabels: []string{""},
 			existingTaints: nil,
 			newTaints:      []v1.Taint{kubeadmconstants.OldControlPlaneTaint},
-			expectedPatch:  `{"metadata":{"labels":{"node-role.kubernetes.io/control-plane":"","node-role.kubernetes.io/master":"","node.kubernetes.io/exclude-from-external-load-balancers":""}},"spec":{"taints":[{"effect":"NoSchedule","key":"node-role.kubernetes.io/master"}]}}`,
+			expectedPatch:  `{"metadata":{"labels":{"node-role.kubernetes.io/control-plane":"","node.kubernetes.io/exclude-from-external-load-balancers":""}},"spec":{"taints":[{"effect":"NoSchedule","key":"node-role.kubernetes.io/master"}]}}`,
 		},
 		{
 			name:           "control-plane label and taint missing but taint not wanted",
 			existingLabels: []string{""},
 			existingTaints: nil,
 			newTaints:      nil,
-			expectedPatch:  `{"metadata":{"labels":{"node-role.kubernetes.io/control-plane":"","node-role.kubernetes.io/master":"","node.kubernetes.io/exclude-from-external-load-balancers":""}}}`,
+			expectedPatch:  `{"metadata":{"labels":{"node-role.kubernetes.io/control-plane":"","node.kubernetes.io/exclude-from-external-load-balancers":""}}}`,
 		},
 		{
 			name:           "control-plane label missing",
 			existingLabels: []string{""},
 			existingTaints: []v1.Taint{kubeadmconstants.OldControlPlaneTaint},
 			newTaints:      []v1.Taint{kubeadmconstants.OldControlPlaneTaint},
-			expectedPatch:  `{"metadata":{"labels":{"node-role.kubernetes.io/control-plane":"","node-role.kubernetes.io/master":"","node.kubernetes.io/exclude-from-external-load-balancers":""}}}`,
+			expectedPatch:  `{"metadata":{"labels":{"node-role.kubernetes.io/control-plane":"","node.kubernetes.io/exclude-from-external-load-balancers":""}}}`,
 		},
 		{
 			name: "control-plane taint missing",
 			existingLabels: []string{
-				kubeadmconstants.LabelNodeRoleOldControlPlane,
 				kubeadmconstants.LabelNodeRoleControlPlane,
 				kubeadmconstants.LabelExcludeFromExternalLB,
 			},
@@ -79,7 +78,6 @@ func TestMarkControlPlane(t *testing.T) {
 		{
 			name: "nothing missing",
 			existingLabels: []string{
-				kubeadmconstants.LabelNodeRoleOldControlPlane,
 				kubeadmconstants.LabelNodeRoleControlPlane,
 				kubeadmconstants.LabelExcludeFromExternalLB,
 			},
@@ -90,7 +88,6 @@ func TestMarkControlPlane(t *testing.T) {
 		{
 			name: "has taint and no new taints wanted",
 			existingLabels: []string{
-				kubeadmconstants.LabelNodeRoleOldControlPlane,
 				kubeadmconstants.LabelNodeRoleControlPlane,
 				kubeadmconstants.LabelExcludeFromExternalLB,
 			},

cmd/kubeadm/app/phases/upgrade/health.go

@@ -212,34 +212,17 @@ func deleteHealthCheckJob(client clientset.Interface, ns, jobName string) error
 
 // controlPlaneNodesReady checks whether all control-plane Nodes in the cluster are in the Running state
 func controlPlaneNodesReady(client clientset.Interface, _ *kubeadmapi.ClusterConfiguration) error {
-	// list nodes labeled with a "master" node-role
-	selectorOldControlPlane := labels.SelectorFromSet(labels.Set(map[string]string{
-		constants.LabelNodeRoleOldControlPlane: "",
-	}))
-	nodesWithOldLabel, err := client.CoreV1().Nodes().List(context.TODO(), metav1.ListOptions{
-		LabelSelector: selectorOldControlPlane.String(),
-	})
-	if err != nil {
-		return errors.Wrapf(err, "could not list nodes labeled with %q", constants.LabelNodeRoleOldControlPlane)
-	}
-
-	// list nodes labeled with a "control-plane" node-role
 	selectorControlPlane := labels.SelectorFromSet(labels.Set(map[string]string{
 		constants.LabelNodeRoleControlPlane: "",
 	}))
-	nodesControlPlane, err := client.CoreV1().Nodes().List(context.TODO(), metav1.ListOptions{
+	nodes, err := client.CoreV1().Nodes().List(context.TODO(), metav1.ListOptions{
 		LabelSelector: selectorControlPlane.String(),
 	})
 	if err != nil {
 		return errors.Wrapf(err, "could not list nodes labeled with %q", constants.LabelNodeRoleControlPlane)
 	}
 
-	nodes := append(nodesWithOldLabel.Items, nodesControlPlane.Items...)
+	notReadyControlPlanes := getNotReadyNodes(nodes.Items)
-	if len(nodes) == 0 {
-		return errors.New("failed to find any nodes with a control-plane role")
-	}
-
-	notReadyControlPlanes := getNotReadyNodes(nodes)
 	if len(notReadyControlPlanes) != 0 {
 		return errors.Errorf("there are NotReady control-planes in the cluster: %v", notReadyControlPlanes)
 	}

cmd/kubeadm/app/phases/upgrade/postupgrade.go

@@ -214,10 +214,9 @@ func rollbackFiles(files map[string]string, originalErr error) error {
 	return errors.Errorf("couldn't move these files: %v. Got errors: %v", files, errorsutil.NewAggregate(errs))
 }
 
-// LabelOldControlPlaneNodes finds all nodes with the legacy node-role label and also applies
+// RemoveOldControlPlaneLabel finds all nodes with the legacy node-role label and removes it
-// the "control-plane" node-role label to them.
 // TODO: https://github.com/kubernetes/kubeadm/issues/2200
-func LabelOldControlPlaneNodes(client clientset.Interface) error {
+func RemoveOldControlPlaneLabel(client clientset.Interface) error {
 	selectorOldControlPlane := labels.SelectorFromSet(labels.Set(map[string]string{
 		kubeadmconstants.LabelNodeRoleOldControlPlane: "",
 	}))
@@ -229,11 +228,8 @@ func LabelOldControlPlaneNodes(client clientset.Interface) error {
 	}
 
 	for _, n := range nodesWithOldLabel.Items {
-		if _, hasNewLabel := n.ObjectMeta.Labels[kubeadmconstants.LabelNodeRoleControlPlane]; hasNewLabel {
-			continue
-		}
 		err = apiclient.PatchNode(client, n.Name, func(n *v1.Node) {
-			n.ObjectMeta.Labels[kubeadmconstants.LabelNodeRoleControlPlane] = ""
+			delete(n.ObjectMeta.Labels, kubeadmconstants.LabelNodeRoleOldControlPlane)
 		})
 		if err != nil {
 			return err
@@ -242,6 +238,42 @@ func LabelOldControlPlaneNodes(client clientset.Interface) error {
 	return nil
 }
 
+// AddNewControlPlaneTaint finds all nodes with the new "control-plane" node-role label
+// and adds the new "control-plane" taint to them.
+// TODO: https://github.com/kubernetes/kubeadm/issues/2200
+func AddNewControlPlaneTaint(client clientset.Interface) error {
+	selectorControlPlane := labels.SelectorFromSet(labels.Set(map[string]string{
+		kubeadmconstants.LabelNodeRoleControlPlane: "",
+	}))
+	nodes, err := client.CoreV1().Nodes().List(context.TODO(), metav1.ListOptions{
+		LabelSelector: selectorControlPlane.String(),
+	})
+	if err != nil {
+		return errors.Wrapf(err, "could not list nodes labeled with %q", kubeadmconstants.LabelNodeRoleControlPlane)
+	}
+
+	for _, n := range nodes.Items {
+		// Check if the node has the taint already and skip it if so
+		hasTaint := false
+		for _, t := range n.Spec.Taints {
+			if t.String() == kubeadmconstants.ControlPlaneTaint.String() {
+				hasTaint = true
+				break
+			}
+		}
+		// If the node does not have the taint, patch it
+		if !hasTaint {
+			err = apiclient.PatchNode(client, n.Name, func(n *v1.Node) {
+				n.Spec.Taints = append(n.Spec.Taints, kubeadmconstants.ControlPlaneTaint)
+			})
+			if err != nil {
+				return err
+			}
+		}
+	}
+	return nil
+}
+
 // UpdateKubeletDynamicEnvFileWithURLScheme reads the kubelet dynamic environment file
 // from disk, ensure that the CRI endpoint flag has a scheme prefix and writes it
 // back to disk.

cmd/kubeadm/app/util/config/initconfiguration.go

@@ -106,7 +106,7 @@ func SetNodeRegistrationDynamicDefaults(cfg *kubeadmapi.NodeRegistrationOptions,
 	// Only if the slice is nil, we should append the control-plane taint. This allows the user to specify an empty slice for no default control-plane taint
 	if controlPlaneTaint && cfg.Taints == nil {
 		// TODO: https://github.com/kubernetes/kubeadm/issues/2200
-		cfg.Taints = []v1.Taint{kubeadmconstants.OldControlPlaneTaint}
+		cfg.Taints = []v1.Taint{kubeadmconstants.OldControlPlaneTaint, kubeadmconstants.ControlPlaneTaint}
 	}
 
 	if cfg.CRISocket == "" {

cmd/kubeadm/app/util/config/initconfiguration_test.go

@@ -115,17 +115,17 @@ func TestDefaultTaintsMarshaling(t *testing.T) {
 		expectedTaintCnt int
 	}{
 		{
-			desc: "Uninitialized nodeRegistration field produces a single taint (the master one)",
+			desc: "Uninitialized nodeRegistration field produces expected taints",
 			cfg: kubeadmapiv1.InitConfiguration{
 				TypeMeta: metav1.TypeMeta{
 					APIVersion: kubeadmapiv1.SchemeGroupVersion.String(),
 					Kind:       constants.InitConfigurationKind,
 				},
 			},
-			expectedTaintCnt: 1,
+			expectedTaintCnt: 2,
 		},
 		{
-			desc: "Uninitialized taints field produces a single taint (the master one)",
+			desc: "Uninitialized taints field produces expected taints",
 			cfg: kubeadmapiv1.InitConfiguration{
 				TypeMeta: metav1.TypeMeta{
 					APIVersion: kubeadmapiv1.SchemeGroupVersion.String(),
@@ -133,7 +133,7 @@ func TestDefaultTaintsMarshaling(t *testing.T) {
 				},
 				NodeRegistration: kubeadmapiv1.NodeRegistrationOptions{},
 			},
-			expectedTaintCnt: 1,
+			expectedTaintCnt: 2,
 		},
 		{
 			desc: "Forsing taints to an empty slice produces no taints",

cmd/kubeadm/app/util/staticpod/utils.go

@@ -287,13 +287,6 @@ func createHTTPProbe(host, path string, port int, scheme v1.URIScheme, initialDe
 
 // GetAPIServerProbeAddress returns the probe address for the API server
 func GetAPIServerProbeAddress(endpoint *kubeadmapi.APIEndpoint) string {
-	// In the case of a self-hosted deployment, the initial host on which kubeadm --init is run,
-	// will generate a DaemonSet with a nodeSelector such that all nodes with the label
-	// node-role.kubernetes.io/master='' will have the API server deployed to it. Since the init
-	// is run only once on an initial host, the API advertise address will be invalid for any
-	// future hosts that do not have the same address. Furthermore, since liveness and readiness
-	// probes do not support the Downward API we cannot dynamically set the advertise address to
-	// the node's IP. The only option then is to use localhost.
 	if endpoint != nil && endpoint.AdvertiseAddress != "" {
 		return getProbeAddress(endpoint.AdvertiseAddress)
 	}

test/e2e/framework/test_context.go

@@ -313,7 +313,12 @@ func RegisterCommonFlags(flags *flag.FlagSet) {
 	flags.BoolVar(&TestContext.DumpSystemdJournal, "dump-systemd-journal", false, "Whether to dump the full systemd journal.")
 	flags.StringVar(&TestContext.ImageServiceEndpoint, "image-service-endpoint", "", "The image service endpoint of cluster VM instances.")
 	flags.StringVar(&TestContext.DockershimCheckpointDir, "dockershim-checkpoint-dir", "/var/lib/dockershim/sandbox", "The directory for dockershim to store sandbox checkpoints.")
-	flags.StringVar(&TestContext.NonblockingTaints, "non-blocking-taints", `node-role.kubernetes.io/master`, "Nodes with taints in this comma-delimited list will not block the test framework from starting tests.")
+	// TODO: remove the node-role.kubernetes.io/master taint in 1.25 or later.
+	// The change will likely require an action for some users that do not
+	// use k8s originated tools like kubeadm or kOps for creating clusters
+	// and taint their control plane nodes with "master", expecting the test
+	// suite to work with this legacy non-blocking taint.
+	flags.StringVar(&TestContext.NonblockingTaints, "non-blocking-taints", `node-role.kubernetes.io/control-plane,node-role.kubernetes.io/master`, "Nodes with taints in this comma-delimited list will not block the test framework from starting tests. The default taint 'node-role.kubernetes.io/master' is DEPRECATED and will be removed from the list in a future release.")
 
 	flags.BoolVar(&TestContext.ListImages, "list-images", false, "If true, will show list of images used for runnning tests.")
 	flags.BoolVar(&TestContext.ListConformanceTests, "list-conformance-tests", false, "If true, will show list of conformance tests.")

test/e2e_kubeadm/controlplane_nodes_test.go

@@ -18,6 +18,7 @@ package kubeadm
 
 import (
 	"context"
+
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
@@ -29,7 +30,10 @@ import (
 )
 
 const (
-	controlPlaneTaint = "node-role.kubernetes.io/master"
+	controlPlaneLabel = "node-role.kubernetes.io/control-plane"
+	// TODO: remove the legacy label in 1.25:
+	// https://github.com/kubernetes/kubeadm/issues/2200
+	controlPlaneLabelLegacy = "node-role.kubernetes.io/master"
 )
 
 // Define container for all the test specification aimed at verifying
@@ -50,20 +54,22 @@ var _ = Describe("control-plane node", func() {
 		controlPlanes := getControlPlaneNodes(f.ClientSet)
 
 		// checks if there is at least one control-plane node
-		gomega.Expect(controlPlanes.Items).NotTo(gomega.BeEmpty(), "at least one node with label %s should exist. if you are running test on a single-node cluster, you can skip this test with SKIP=multi-node", controlPlaneTaint)
+		gomega.Expect(controlPlanes.Items).NotTo(gomega.BeEmpty(), "at least one node with label %s should exist. if you are running test on a single-node cluster, you can skip this test with SKIP=multi-node", controlPlaneLabel)
 
-		// checks that the control-plane nodes have the expected taint
+		// checks that the control-plane nodes have the expected taints
+		// 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: controlPlaneTaint, Effect: corev1.TaintEffectNoSchedule})
+			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})
 		}
 	})
 })
 
 func getControlPlaneNodes(c clientset.Interface) *corev1.NodeList {
-	selector := labels.Set{controlPlaneTaint: ""}.AsSelector()
+	selector := labels.Set{controlPlaneLabel: ""}.AsSelector()
-	masters, err := c.CoreV1().Nodes().
+	cpNodes, err := c.CoreV1().Nodes().
 		List(context.TODO(), metav1.ListOptions{LabelSelector: selector.String()})
 	framework.ExpectNoError(err, "error reading control-plane nodes")
-
+	return cpNodes
-	return masters
 }