Apply uninitialized taint to nodes at creation

and delete after labels are synced

Author: Yuvaraj Kakaraparthi

api/v1beta1/common_types.go

@@ -146,6 +146,16 @@ const (
 	VariableDefinitionFromInline = "inline"
 )
 
+// NodeUninitializedTaint can be added to Nodes at creation by the bootstrap provider, e.g. the
+// KubeadmBootstrap provider will add the taint.
+// This taint is used to prevent workloads to be scheduled on Nodes before the node is initialized by Cluster API.
+// As of today the Node initialization consists of syncing labels from Machines to Nodes. Once the labels
+// have been initially synced the taint is removed form the Node.
+var NodeUninitializedTaint = corev1.Taint{
+	Key:    "node.cluster.x-k8s.io/uninitialized",
+	Effect: corev1.TaintEffectNoSchedule,
+}
+
 const (
 	// TemplateSuffix is the object kind suffix used by template types.
 	TemplateSuffix = "Template"

bootstrap/kubeadm/internal/controllers/kubeadmconfig_controller.go

@@ -63,6 +63,23 @@ const (
 	KubeadmConfigControllerName = "kubeadmconfig-controller"
 )
 
+var (
+	// controlPlaneTaint is the taint that kubeadm applies to the control plane nodes.
+	// The values are copied from kubeadm codebase.
+	controlPlaneTaint = corev1.Taint{
+		Key:    "node-role.kubernetes.io/control-plane",
+		Effect: corev1.TaintEffectNoSchedule,
+	}
+
+	// oldControlPlaneTaint is the taint that kubeadm applies to the control plane nodes
+	// for Kubernetes version < v1.25.0.
+	// The values are copied from kubeadm codebase.
+	oldControlPlaneTaint = corev1.Taint{
+		Key:    "node-role.kubernetes.io/master",
+		Effect: corev1.TaintEffectNoSchedule,
+	}
+)
+
 const (
 	// DefaultTokenTTL is the default TTL used for tokens.
 	DefaultTokenTTL = 15 * time.Minute
@@ -415,7 +432,15 @@ func (r *KubeadmConfigReconciler) handleClusterNotInitialized(ctx context.Contex
 			},
 		}
 	}
-	initdata, err := kubeadmtypes.MarshalInitConfigurationForVersion(scope.Config.Spec.InitConfiguration, parsedVersion)
+
+	// Add the node uninitialized taint to the list of taints.
+	// DeepCopy the InitConfiguration to prevent updating the actual KubeadmConfig.
+	// Do not modify the KubeadmConfig in etcd as this is a temporary taint that will be dropped after the node
+	// is initialized by ClusterAPI.
+	initConfiguration := scope.Config.Spec.InitConfiguration.DeepCopy()
+	addNodeUninitializedTaint(&initConfiguration.NodeRegistration, true, parsedVersion)
+
+	initdata, err := kubeadmtypes.MarshalInitConfigurationForVersion(initConfiguration, parsedVersion)
 	if err != nil {
 		scope.Error(err, "Failed to marshal init configuration")
 		return ctrl.Result{}, err
@@ -551,7 +576,14 @@ func (r *KubeadmConfigReconciler) joinWorker(ctx context.Context, scope *Scope)
 		return ctrl.Result{}, errors.Wrapf(err, "failed to parse kubernetes version %q", kubernetesVersion)
 	}
 
-	joinData, err := kubeadmtypes.MarshalJoinConfigurationForVersion(scope.Config.Spec.JoinConfiguration, parsedVersion)
+	// Add the node uninitialized taint to the list of taints.
+	// DeepCopy the JoinConfiguration to prevent updating the actual KubeadmConfig.
+	// Do not modify the KubeadmConfig in etcd as this is a temporary taint that will be dropped after the node
+	// is initialized by ClusterAPI.
+	joinConfiguration := scope.Config.Spec.JoinConfiguration.DeepCopy()
+	addNodeUninitializedTaint(&joinConfiguration.NodeRegistration, false, parsedVersion)
+
+	joinData, err := kubeadmtypes.MarshalJoinConfigurationForVersion(joinConfiguration, parsedVersion)
 	if err != nil {
 		scope.Error(err, "Failed to marshal join configuration")
 		return ctrl.Result{}, err
@@ -657,7 +689,14 @@ func (r *KubeadmConfigReconciler) joinControlplane(ctx context.Context, scope *S
 		return ctrl.Result{}, errors.Wrapf(err, "failed to parse kubernetes version %q", kubernetesVersion)
 	}
 
-	joinData, err := kubeadmtypes.MarshalJoinConfigurationForVersion(scope.Config.Spec.JoinConfiguration, parsedVersion)
+	// Add the node uninitialized taint to the list of taints.
+	// DeepCopy the JoinConfiguration to prevent updating the actual KubeadmConfig.
+	// Do not modify the KubeadmConfig in etcd as this is a temporary taint that will be dropped after the node
+	// is initialized by ClusterAPI.
+	joinConfiguration := scope.Config.Spec.JoinConfiguration.DeepCopy()
+	addNodeUninitializedTaint(&joinConfiguration.NodeRegistration, true, parsedVersion)
+
+	joinData, err := kubeadmtypes.MarshalJoinConfigurationForVersion(joinConfiguration, parsedVersion)
 	if err != nil {
 		scope.Error(err, "Failed to marshal join configuration")
 		return ctrl.Result{}, err
@@ -1066,3 +1105,43 @@ func (r *KubeadmConfigReconciler) ensureBootstrapSecretOwnersRef(ctx context.Con
 	}
 	return nil
 }
+
+// addNodeUninitializedTaint adds the NodeUninitializedTaint to the nodeRegistration.
+// Note: If isControlPlane is true then it also adds the control plane taint if the initial set of taints is nil.
+// This is to ensure consistency with kubeadm's defaulting behavior.
+func addNodeUninitializedTaint(nodeRegistration *bootstrapv1.NodeRegistrationOptions, isControlPlane bool, kubernetesVersion semver.Version) {
+	var taints []corev1.Taint
+	taints = nodeRegistration.Taints
+	if hasTaint(taints, clusterv1.NodeUninitializedTaint) {
+		return
+	}
+
+	// For a control plane, kubeadm adds the default control plane taint if the provided taints are nil.
+	// Since we are adding the uninitialized taint we also have to add the default taints kubeadm would have added if
+	// the taints were nil.
+	if isControlPlane && taints == nil {
+		// Note: Kubeadm uses a different default control plane taint depending on the kubernetes version.
+		// Ref: https://github.com/kubernetes/kubeadm/issues/2200
+		if kubernetesVersion.LT(semver.MustParse("1.24.0")) {
+			taints = []corev1.Taint{oldControlPlaneTaint}
+		} else if kubernetesVersion.GTE(semver.MustParse("1.24.0")) && kubernetesVersion.LT(semver.MustParse("1.25.0")) {
+			taints = []corev1.Taint{
+				oldControlPlaneTaint,
+				controlPlaneTaint,
+			}
+		} else {
+			taints = []corev1.Taint{controlPlaneTaint}
+		}
+	}
+	taints = append(taints, clusterv1.NodeUninitializedTaint)
+	nodeRegistration.Taints = taints
+}
+
+func hasTaint(taints []corev1.Taint, targetTaint corev1.Taint) bool {
+	for _, taint := range taints {
+		if taint.MatchTaint(&targetTaint) {
+			return true
+		}
+	}
+	return false
+}

bootstrap/kubeadm/internal/controllers/kubeadmconfig_controller_test.go

@@ -24,6 +24,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/blang/semver"
 	ignition "github.com/flatcar/ignition/config/v2_3"
 	. "github.com/onsi/gomega"
 	corev1 "k8s.io/api/core/v1"
@@ -2193,6 +2194,115 @@ func TestKubeadmConfigReconciler_ResolveUsers(t *testing.T) {
 	}
 }
 
+func TestAddNodeUninitializedTaint(t *testing.T) {
+	dummyTaint := corev1.Taint{
+		Key:    "dummy-taint",
+		Value:  "",
+		Effect: corev1.TaintEffectNoSchedule,
+	}
+
+	tests := []struct {
+		name              string
+		nodeRegistration  *bootstrapv1.NodeRegistrationOptions
+		kubernetesVersion semver.Version
+		isControlPlane    bool
+		wantTaints        []corev1.Taint
+	}{
+		{
+			name: "for control plane with version < v1.24.0, if taints is nil it should add the uninitialized and the master taints",
+			nodeRegistration: &bootstrapv1.NodeRegistrationOptions{
+				Taints: nil,
+			},
+			kubernetesVersion: semver.MustParse("1.23.0"),
+			isControlPlane:    true,
+			wantTaints: []corev1.Taint{
+				oldControlPlaneTaint,
+				clusterv1.NodeUninitializedTaint,
+			},
+		},
+		{
+			name: "for control plane with version >= v1.24.0 and < v1.25.0, if taints is nil it should add the uninitialized, control-plane and the master taints",
+			nodeRegistration: &bootstrapv1.NodeRegistrationOptions{
+				Taints: nil,
+			},
+			kubernetesVersion: semver.MustParse("1.24.5"),
+			isControlPlane:    true,
+			wantTaints: []corev1.Taint{
+				oldControlPlaneTaint,
+				controlPlaneTaint,
+				clusterv1.NodeUninitializedTaint,
+			},
+		},
+		{
+			name: "for control plane with version >= v1.25.0, if taints is nil it should add the uninitialized and the control-plane taints",
+			nodeRegistration: &bootstrapv1.NodeRegistrationOptions{
+				Taints: nil,
+			},
+			kubernetesVersion: semver.MustParse("1.25.0"),
+			isControlPlane:    true,
+			wantTaints: []corev1.Taint{
+				controlPlaneTaint,
+				clusterv1.NodeUninitializedTaint,
+			},
+		},
+		{
+			name: "for control plane, if taints is not nil it should only add the uninitialized taint",
+			nodeRegistration: &bootstrapv1.NodeRegistrationOptions{
+				Taints: []corev1.Taint{},
+			},
+			kubernetesVersion: semver.MustParse("1.26.0"),
+			isControlPlane:    true,
+			wantTaints: []corev1.Taint{
+				clusterv1.NodeUninitializedTaint,
+			},
+		},
+		{
+			name: "for control plane, if it already has some taints it should add the uninitialized taint",
+			nodeRegistration: &bootstrapv1.NodeRegistrationOptions{
+				Taints: []corev1.Taint{dummyTaint},
+			},
+			kubernetesVersion: semver.MustParse("1.26.0"),
+			isControlPlane:    true,
+			wantTaints: []corev1.Taint{
+				clusterv1.NodeUninitializedTaint,
+				dummyTaint,
+			},
+		},
+		{
+			name:              "for worker, it should add the uninitialized taint",
+			nodeRegistration:  &bootstrapv1.NodeRegistrationOptions{},
+			kubernetesVersion: semver.MustParse("1.26.0"),
+			isControlPlane:    false,
+			wantTaints: []corev1.Taint{
+				clusterv1.NodeUninitializedTaint,
+			},
+		},
+		{
+			name: "for worker, if it already has some taints it should add the uninitialized taint",
+			nodeRegistration: &bootstrapv1.NodeRegistrationOptions{
+				Taints: []corev1.Taint{dummyTaint},
+			},
+			kubernetesVersion: semver.MustParse("1.26.0"),
+			isControlPlane:    false,
+			wantTaints: []corev1.Taint{
+				dummyTaint,
+				clusterv1.NodeUninitializedTaint,
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+			addNodeUninitializedTaint(tt.nodeRegistration, tt.isControlPlane, tt.kubernetesVersion)
+			g.Expect(tt.nodeRegistration.Taints).To(HaveLen(len(tt.wantTaints)))
+			for _, taint := range tt.wantTaints {
+				g.Expect(hasTaint(tt.nodeRegistration.Taints, taint)).To(BeTrue())
+			}
+		})
+	}
+}
+
 // test utils.
 
 // newWorkerMachineForCluster returns a Machine with the passed Cluster's information and a pre-configured name.

docs/book/src/developer/providers/bootstrap.md

@@ -123,6 +123,13 @@ The following diagram shows the typical logic for a bootstrap provider:
 
 A bootstrap provider's bootstrap data must create `/run/cluster-api/bootstrap-success.complete` (or `C:\run\cluster-api\bootstrap-success.complete` for Windows machines) upon successful bootstrapping of a Kubernetes node. This allows infrastructure providers to detect and act on bootstrap failures.
 
+## Taint Nodes at creation
+
+A bootstrap provider can optionally taint nodes at creation with `node.cluster.x-k8s.io/uninitialized:NoSchedule`.
+This taint is used to prevent workloads to be scheduled on Nodes before the node is initialized by Cluster API.
+As of today the Node initialization consists of syncing labels from Machines to Nodes. Once the labels have been 
+initially synced the taint is removed form the Node.
+
 ## RBAC
 
 ### Provider controller

internal/controllers/machine/machine_controller_noderef.go

@@ -133,6 +133,11 @@ func (r *Reconciler) reconcileNode(ctx context.Context, cluster *clusterv1.Clust
 		return ctrl.Result{}, errors.Wrap(err, "failed to apply patch label to the node")
 	}
 
+	// Reconcile node taints
+	if err := r.reconcileNodeTaints(ctx, remoteClient, node); err != nil {
+		return ctrl.Result{}, errors.Wrapf(err, "failed to reconcile taints on Node %s", klog.KObj(node))
+	}
+
 	// Do the remaining node health checks, then set the node health to true if all checks pass.
 	status, message := summarizeNodeConditions(node)
 	if status == corev1.ConditionFalse {
@@ -260,3 +265,23 @@ func (r *Reconciler) getNode(ctx context.Context, c client.Reader, providerID *n
 
 	return &nodeList.Items[0], nil
 }
+
+func (r *Reconciler) reconcileNodeTaints(ctx context.Context, remoteClient client.Client, node *corev1.Node) error {
+	taints := []corev1.Taint{}
+	patchHelper, err := patch.NewHelper(node, remoteClient)
+	if err != nil {
+		return errors.Wrapf(err, "failed to create patch helper for Node %s", klog.KObj(node))
+	}
+	// Drop the NodeUninitializedTaint taint on the node.
+	for _, taint := range node.Spec.Taints {
+		if taint.MatchTaint(&clusterv1.NodeUninitializedTaint) {
+			continue
+		}
+		taints = append(taints, taint)
+	}
+	node.Spec.Taints = taints
+	if err := patchHelper.Patch(ctx, node); err != nil {
+		return errors.Wrapf(err, "failed patch Node %s to set taints", klog.KObj(node))
+	}
+	return nil
+}

internal/controllers/machine/machine_controller_noderef_test.go

@@ -27,6 +27,7 @@ import (
 	"k8s.io/utils/pointer"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
@@ -459,3 +460,70 @@ func TestGetManagedLabels(t *testing.T) {
 	got := getManagedLabels(allLabels)
 	g.Expect(got).To(BeEquivalentTo(managedLabels))
 }
+
+func TestReconcileNodeTaints(t *testing.T) {
+	tests := []struct {
+		name string
+		node *corev1.Node
+	}{
+		{
+			name: "if the node has the uninitialized taint it should be dropped from the node",
+			node: &corev1.Node{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-node-1",
+				},
+				Spec: corev1.NodeSpec{
+					Taints: []corev1.Taint{
+						clusterv1.NodeUninitializedTaint,
+					},
+				},
+			},
+		},
+		{
+			name: "if the node is a control plane and has the uninitialized taint it should be dropped from the node",
+			node: &corev1.Node{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-node-1",
+				},
+				Spec: corev1.NodeSpec{
+					Taints: []corev1.Taint{
+						{
+							Key:    "node-role.kubernetes.io/control-plane",
+							Effect: corev1.TaintEffectNoSchedule,
+						},
+						clusterv1.NodeUninitializedTaint,
+					},
+				},
+			},
+		},
+		{
+			name: "if the node does not have the uninitialized taint it should remain absent from the node",
+			node: &corev1.Node{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "test-node-1",
+				},
+				Spec: corev1.NodeSpec{
+					Taints: []corev1.Taint{},
+				},
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+			fakeClient := fake.NewClientBuilder().WithObjects(tt.node).WithScheme(fakeScheme).Build()
+			nodeBefore := tt.node.DeepCopy()
+			r := &Reconciler{}
+			g.Expect(r.reconcileNodeTaints(ctx, fakeClient, tt.node)).Should(Succeed())
+			// Verify the NodeUninitializedTaint is dropped.
+			g.Expect(tt.node.Spec.Taints).ShouldNot(ContainElement(clusterv1.NodeUninitializedTaint))
+			// Verify all other taints are same.
+			for _, taint := range nodeBefore.Spec.Taints {
+				if !taint.MatchTaint(&clusterv1.NodeUninitializedTaint) {
+					g.Expect(tt.node.Spec.Taints).Should(ContainElement(taint))
+				}
+			}
+		})
+	}
+}