Ensure added taints are present on the node in the snapshot

cluster-autoscaler/utils/taints/taints.go

@@ -31,6 +31,7 @@ import (
 	kube_client "k8s.io/client-go/kubernetes"
 	kube_record "k8s.io/client-go/tools/record"
 	cloudproviderapi "k8s.io/cloud-provider/api"
+	"k8s.io/kubernetes/pkg/controller"
 
 	klog "k8s.io/klog/v2"
 )
@@ -181,42 +182,20 @@ func MarkDeletionCandidate(node *apiv1.Node, client kube_client.Interface) error
 
 // AddTaints sets the specified taints on the node.
 func AddTaints(node *apiv1.Node, client kube_client.Interface, taints []apiv1.Taint, cordonNode bool) error {
-	retryDeadline := time.Now().Add(maxRetryDeadline)
-	freshNode := node.DeepCopy()
-	var err error
-	refresh := false
-	for {
-		if refresh {
-			// Get the newest version of the node.
-			freshNode, err = client.CoreV1().Nodes().Get(context.TODO(), node.Name, metav1.GetOptions{})
-			if err != nil || freshNode == nil {
-				klog.Warningf("Error while adding %v taints on node %v: %v", strings.Join(taintKeys(taints), ","), node.Name, err)
-				return fmt.Errorf("failed to get node %v: %v", node.Name, err)
-			}
-		}
-
-		if !addTaintsToSpec(freshNode, taints, cordonNode) {
-			if !refresh {
-				// Make sure we have the latest version before skipping update.
-				refresh = true
-				continue
-			}
-			return nil
-		}
-		_, err = client.CoreV1().Nodes().Update(context.TODO(), freshNode, metav1.UpdateOptions{})
-		if err != nil && errors.IsConflict(err) && time.Now().Before(retryDeadline) {
-			refresh = true
-			time.Sleep(conflictRetryInterval)
-			continue
-		}
-
-		if err != nil {
-			klog.Warningf("Error while adding %v taints on node %v: %v", strings.Join(taintKeys(taints), ","), node.Name, err)
-			return err
-		}
-		klog.V(1).Infof("Successfully added %v on node %v", strings.Join(taintKeys(taints), ","), node.Name)
+	if !addTaintsToSpec(node, taints, cordonNode) {
 		return nil
 	}
+	taintPtrs := make([]*apiv1.Taint, len(taints))
+	for i, t := range taints {
+		taintPtrs[i] = &t
+	}
+	err := controller.AddOrUpdateTaintOnNode(context.TODO(), client, node.Name, taintPtrs...)
+	if err != nil {
+		klog.Warningf("Error while adding %v taints on node %v: %v", strings.Join(taintKeys(taints), ","), node.Name, err)
+		return err
+	}
+	klog.V(1).Infof("Successfully added %v on node %v", strings.Join(taintKeys(taints), ","), node.Name)
+	return nil
 }
 
 func addTaintsToSpec(node *apiv1.Node, taints []apiv1.Taint, cordonNode bool) bool {

cluster-autoscaler/utils/taints/taints_test.go

@@ -723,3 +723,58 @@ func TestCountNodeTaints(t *testing.T) {
 	got := CountNodeTaints([]*apiv1.Node{node, node2}, taintConfig)
 	assert.Equal(t, want, got)
 }
+
+func TestAddTaints(t *testing.T) {
+	testCases := []struct {
+		name           string
+		existingTaints []string
+		newTaints      []string
+		wantTaints     []string
+	}{
+		{
+			name:       "no existing taints",
+			newTaints:  []string{"t1", "t2"},
+			wantTaints: []string{"t1", "t2"},
+		},
+		{
+			name:           "existing taints - no overlap",
+			existingTaints: []string{"t1"},
+			newTaints:      []string{"t2", "t3"},
+			wantTaints:     []string{"t1", "t2", "t3"},
+		},
+		{
+			name:           "existing taints - duplicates",
+			existingTaints: []string{"t1"},
+			newTaints:      []string{"t1", "t2"},
+			wantTaints:     []string{"t1", "t2"},
+		},
+	}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			n := BuildTestNode("node", 1000, 1000)
+			existingTaints := make([]apiv1.Taint, len(tc.existingTaints))
+			for i, t := range tc.existingTaints {
+				existingTaints[i] = apiv1.Taint{
+					Key:    t,
+					Effect: apiv1.TaintEffectNoSchedule,
+				}
+			}
+			n.Spec.Taints = append([]apiv1.Taint{}, existingTaints...)
+			fakeClient := buildFakeClient(t, n)
+			newTaints := make([]apiv1.Taint, len(tc.newTaints))
+			for i, t := range tc.newTaints {
+				newTaints[i] = apiv1.Taint{
+					Key:    t,
+					Effect: apiv1.TaintEffectNoSchedule,
+				}
+			}
+			err := AddTaints(n, fakeClient, newTaints, false)
+			assert.NoError(t, err)
+			apiNode := getNode(t, fakeClient, "node")
+			for _, want := range tc.wantTaints {
+				assert.True(t, HasTaint(n, want))
+				assert.True(t, HasTaint(apiNode, want))
+			}
+		})
+	}
+}