Merge pull request #49524 from k82cn/k8s_49522

Automatic merge from submit-queue (batch tested with PRs 49524, 46760, 50206, 50166, 49603)

Handled taints on node in batch.

**What this PR does / why we need it**:
Enhanced helpers to handled taints on node in batch.

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #49522 

**Release note**:

```release-note
None
```

pkg/controller/BUILD

@@ -18,7 +18,9 @@ go_test(
     tags = ["automanaged"],
     deps = [
         "//pkg/api:go_default_library",
+        "//pkg/api/install:go_default_library",
         "//pkg/api/testapi:go_default_library",
+        "//pkg/controller/node/testutil:go_default_library",
         "//pkg/securitycontext:go_default_library",
         "//vendor/k8s.io/api/core/v1:go_default_library",
         "//vendor/k8s.io/api/extensions/v1beta1:go_default_library",
@@ -32,6 +34,7 @@ go_test(
         "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/uuid:go_default_library",
         "//vendor/k8s.io/client-go/kubernetes:go_default_library",
+        "//vendor/k8s.io/client-go/kubernetes/fake:go_default_library",
         "//vendor/k8s.io/client-go/rest:go_default_library",
         "//vendor/k8s.io/client-go/tools/cache:go_default_library",
         "//vendor/k8s.io/client-go/tools/record:go_default_library",

pkg/controller/controller_utils.go

@@ -885,50 +885,11 @@ func (o ReplicaSetsBySizeNewer) Less(i, j int) bool {
 	return *(o[i].Spec.Replicas) > *(o[j].Spec.Replicas)
 }
 
-func AddOrUpdateTaintOnNode(c clientset.Interface, nodeName string, taint *v1.Taint) error {
+// AddOrUpdateTaintOnNode add taints to the node. If taint was added into node, it'll issue API calls
-	firstTry := true
+// to update nodes; otherwise, no API calls. Return error if any.
-	return clientretry.RetryOnConflict(UpdateTaintBackoff, func() error {
+func AddOrUpdateTaintOnNode(c clientset.Interface, nodeName string, taints ...*v1.Taint) error {
-		var err error
+	if len(taints) == 0 {
-		var oldNode *v1.Node
+		return nil
-		// First we try getting node from the API server cache, as it's cheaper. If it fails
-		// we get it from etcd to be sure to have fresh data.
-		if firstTry {
-			oldNode, err = c.Core().Nodes().Get(nodeName, metav1.GetOptions{ResourceVersion: "0"})
-			firstTry = false
-		} else {
-			oldNode, err = c.Core().Nodes().Get(nodeName, metav1.GetOptions{})
-		}
-		if err != nil {
-			return err
-		}
-		newNode, ok, err := taintutils.AddOrUpdateTaint(oldNode, taint)
-		if err != nil {
-			return fmt.Errorf("Failed to update taint annotation!")
-		}
-		if !ok {
-			return nil
-		}
-		return PatchNodeTaints(c, nodeName, oldNode, newNode)
-	})
-}
-
-// RemoveTaintOffNode is for cleaning up taints temporarily added to node,
-// won't fail if target taint doesn't exist or has been removed.
-// If passed a node it'll check if there's anything to be done, if taint is not present it won't issue
-// any API calls.
-func RemoveTaintOffNode(c clientset.Interface, nodeName string, taint *v1.Taint, node *v1.Node) error {
-	// Short circuit for limiting amount of API calls.
-	if node != nil {
-		match := false
-		for i := range node.Spec.Taints {
-			if node.Spec.Taints[i].MatchTaint(taint) {
-				match = true
-				break
-			}
-		}
-		if !match {
-			return nil
-		}
 	}
 	firstTry := true
 	return clientretry.RetryOnConflict(UpdateTaintBackoff, func() error {
@@ -945,11 +906,77 @@ func RemoveTaintOffNode(c clientset.Interface, nodeName string, taint *v1.Taint,
 		if err != nil {
 			return err
 		}
-		newNode, ok, err := taintutils.RemoveTaint(oldNode, taint)
+
-		if err != nil {
+		var newNode *v1.Node
-			return fmt.Errorf("Failed to update taint annotation!")
+		oldNodeCopy := oldNode
+		updated := false
+		for _, taint := range taints {
+			curNewNode, ok, err := taintutils.AddOrUpdateTaint(oldNodeCopy, taint)
+			if err != nil {
+				return fmt.Errorf("Failed to update taint of node!")
+			}
+			updated = updated || ok
+			newNode = curNewNode
+			oldNodeCopy = curNewNode
 		}
-		if !ok {
+		if !updated {
+			return nil
+		}
+		return PatchNodeTaints(c, nodeName, oldNode, newNode)
+	})
+}
+
+// RemoveTaintOffNode is for cleaning up taints temporarily added to node,
+// won't fail if target taint doesn't exist or has been removed.
+// If passed a node it'll check if there's anything to be done, if taint is not present it won't issue
+// any API calls.
+func RemoveTaintOffNode(c clientset.Interface, nodeName string, node *v1.Node, taints ...*v1.Taint) error {
+	if len(taints) == 0 {
+		return nil
+	}
+	// Short circuit for limiting amount of API calls.
+	if node != nil {
+		match := false
+		for _, taint := range taints {
+			if taintutils.TaintExists(node.Spec.Taints, taint) {
+				match = true
+				break
+			}
+		}
+		if !match {
+			return nil
+		}
+	}
+
+	firstTry := true
+	return clientretry.RetryOnConflict(UpdateTaintBackoff, func() error {
+		var err error
+		var oldNode *v1.Node
+		// First we try getting node from the API server cache, as it's cheaper. If it fails
+		// we get it from etcd to be sure to have fresh data.
+		if firstTry {
+			oldNode, err = c.Core().Nodes().Get(nodeName, metav1.GetOptions{ResourceVersion: "0"})
+			firstTry = false
+		} else {
+			oldNode, err = c.Core().Nodes().Get(nodeName, metav1.GetOptions{})
+		}
+		if err != nil {
+			return err
+		}
+
+		var newNode *v1.Node
+		oldNodeCopy := oldNode
+		updated := false
+		for _, taint := range taints {
+			curNewNode, ok, err := taintutils.RemoveTaint(oldNodeCopy, taint)
+			if err != nil {
+				return fmt.Errorf("Failed to remove taint of node!")
+			}
+			updated = updated || ok
+			newNode = curNewNode
+			oldNodeCopy = curNewNode
+		}
+		if !updated {
 			return nil
 		}
 		return PatchNodeTaints(c, nodeName, oldNode, newNode)

pkg/controller/controller_utils_test.go

@@ -37,12 +37,15 @@ import (
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/uuid"
 	clientset "k8s.io/client-go/kubernetes"
+	"k8s.io/client-go/kubernetes/fake"
 	restclient "k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/cache"
 	"k8s.io/client-go/tools/record"
 	utiltesting "k8s.io/client-go/util/testing"
 	"k8s.io/kubernetes/pkg/api"
+	_ "k8s.io/kubernetes/pkg/api/install"
 	"k8s.io/kubernetes/pkg/api/testapi"
+	"k8s.io/kubernetes/pkg/controller/node/testutil"
 	"k8s.io/kubernetes/pkg/securitycontext"
 )
 
@@ -479,3 +482,358 @@ func TestComputeHash(t *testing.T) {
 		}
 	}
 }
+
+func TestRemoveTaintOffNode(t *testing.T) {
+	tests := []struct {
+		name           string
+		nodeHandler    *testutil.FakeNodeHandler
+		nodeName       string
+		taintsToRemove []*v1.Taint
+		expectedTaints []v1.Taint
+		requestCount   int
+	}{
+		{
+			name: "remove one taint from node",
+			nodeHandler: &testutil.FakeNodeHandler{
+				Existing: []*v1.Node{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name: "node1",
+						},
+						Spec: v1.NodeSpec{
+							Taints: []v1.Taint{
+								{Key: "key1", Value: "value1", Effect: "NoSchedule"},
+								{Key: "key2", Value: "value2", Effect: "NoExecute"},
+							},
+						},
+					},
+				},
+				Clientset: fake.NewSimpleClientset(&v1.PodList{Items: []v1.Pod{*testutil.NewPod("pod0", "node0")}}),
+			},
+			nodeName: "node1",
+			taintsToRemove: []*v1.Taint{
+				{Key: "key2", Value: "value2", Effect: "NoExecute"},
+			},
+			expectedTaints: []v1.Taint{
+				{Key: "key1", Value: "value1", Effect: "NoSchedule"},
+			},
+			requestCount: 4,
+		},
+		{
+			name: "remove multiple taints from node",
+			nodeHandler: &testutil.FakeNodeHandler{
+				Existing: []*v1.Node{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name: "node1",
+						},
+						Spec: v1.NodeSpec{
+							Taints: []v1.Taint{
+								{Key: "key1", Value: "value1", Effect: "NoSchedule"},
+								{Key: "key2", Value: "value2", Effect: "NoExecute"},
+								{Key: "key3", Value: "value3", Effect: "NoSchedule"},
+								{Key: "key4", Value: "value4", Effect: "NoExecute"},
+							},
+						},
+					},
+				},
+				Clientset: fake.NewSimpleClientset(&v1.PodList{Items: []v1.Pod{*testutil.NewPod("pod0", "node0")}}),
+			},
+			nodeName: "node1",
+			taintsToRemove: []*v1.Taint{
+				{Key: "key2", Value: "value2", Effect: "NoExecute"},
+				{Key: "key3", Value: "value3", Effect: "NoSchedule"},
+			},
+			expectedTaints: []v1.Taint{
+				{Key: "key1", Value: "value1", Effect: "NoSchedule"},
+				{Key: "key4", Value: "value4", Effect: "NoExecute"},
+			},
+			requestCount: 4,
+		},
+		{
+			name: "remove no-exist taints from node",
+			nodeHandler: &testutil.FakeNodeHandler{
+				Existing: []*v1.Node{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name: "node1",
+						},
+						Spec: v1.NodeSpec{
+							Taints: []v1.Taint{
+								{Key: "key1", Value: "value1", Effect: "NoSchedule"},
+								{Key: "key2", Value: "value2", Effect: "NoExecute"},
+							},
+						},
+					},
+				},
+				Clientset: fake.NewSimpleClientset(&v1.PodList{Items: []v1.Pod{*testutil.NewPod("pod0", "node0")}}),
+			},
+			nodeName: "node1",
+			taintsToRemove: []*v1.Taint{
+				{Key: "key3", Value: "value3", Effect: "NoSchedule"},
+			},
+			expectedTaints: []v1.Taint{
+				{Key: "key1", Value: "value1", Effect: "NoSchedule"},
+				{Key: "key2", Value: "value2", Effect: "NoExecute"},
+			},
+			requestCount: 2,
+		},
+		{
+			name: "remove taint from node without taints",
+			nodeHandler: &testutil.FakeNodeHandler{
+				Existing: []*v1.Node{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name: "node1",
+						},
+					},
+				},
+				Clientset: fake.NewSimpleClientset(&v1.PodList{Items: []v1.Pod{*testutil.NewPod("pod0", "node0")}}),
+			},
+			nodeName: "node1",
+			taintsToRemove: []*v1.Taint{
+				{Key: "key3", Value: "value3", Effect: "NoSchedule"},
+			},
+			expectedTaints: nil,
+			requestCount:   2,
+		},
+		{
+			name: "remove empty taint list from node without taints",
+			nodeHandler: &testutil.FakeNodeHandler{
+				Existing: []*v1.Node{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name: "node1",
+						},
+					},
+				},
+				Clientset: fake.NewSimpleClientset(&v1.PodList{Items: []v1.Pod{*testutil.NewPod("pod0", "node0")}}),
+			},
+			nodeName:       "node1",
+			taintsToRemove: []*v1.Taint{},
+			expectedTaints: nil,
+			requestCount:   2,
+		},
+		{
+			name: "remove empty taint list from node",
+			nodeHandler: &testutil.FakeNodeHandler{
+				Existing: []*v1.Node{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name: "node1",
+						},
+						Spec: v1.NodeSpec{
+							Taints: []v1.Taint{
+								{Key: "key1", Value: "value1", Effect: "NoSchedule"},
+								{Key: "key2", Value: "value2", Effect: "NoExecute"},
+							},
+						},
+					},
+				},
+				Clientset: fake.NewSimpleClientset(&v1.PodList{Items: []v1.Pod{*testutil.NewPod("pod0", "node0")}}),
+			},
+			nodeName:       "node1",
+			taintsToRemove: []*v1.Taint{},
+			expectedTaints: []v1.Taint{
+				{Key: "key1", Value: "value1", Effect: "NoSchedule"},
+				{Key: "key2", Value: "value2", Effect: "NoExecute"},
+			},
+			requestCount: 2,
+		},
+	}
+	for _, test := range tests {
+		node, _ := test.nodeHandler.Get(test.nodeName, metav1.GetOptions{})
+		if err := RemoveTaintOffNode(test.nodeHandler, test.nodeName, node, test.taintsToRemove...); err != nil {
+			t.Errorf("%s: RemoveTaintOffNode() error = %v", test.name, err)
+		}
+
+		node, _ = test.nodeHandler.Get(test.nodeName, metav1.GetOptions{})
+		if !reflect.DeepEqual(node.Spec.Taints, test.expectedTaints) {
+			t.Errorf("%s: failed to remove taint off node: expected %+v, got %+v",
+				test.name, test.expectedTaints, node.Spec.Taints)
+		}
+
+		if test.nodeHandler.RequestCount != test.requestCount {
+			t.Errorf("%s: unexpected request count: expected %+v, got %+v",
+				test.name, test.requestCount, test.nodeHandler.RequestCount)
+		}
+	}
+}
+
+func TestAddOrUpdateTaintOnNode(t *testing.T) {
+	tests := []struct {
+		name           string
+		nodeHandler    *testutil.FakeNodeHandler
+		nodeName       string
+		taintsToAdd    []*v1.Taint
+		expectedTaints []v1.Taint
+		requestCount   int
+	}{
+		{
+			name: "add one taint on node",
+			nodeHandler: &testutil.FakeNodeHandler{
+				Existing: []*v1.Node{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name: "node1",
+						},
+						Spec: v1.NodeSpec{
+							Taints: []v1.Taint{
+								{Key: "key1", Value: "value1", Effect: "NoSchedule"},
+							},
+						},
+					},
+				},
+				Clientset: fake.NewSimpleClientset(&v1.PodList{Items: []v1.Pod{*testutil.NewPod("pod0", "node0")}}),
+			},
+			nodeName: "node1",
+			taintsToAdd: []*v1.Taint{
+				{Key: "key2", Value: "value2", Effect: "NoExecute"},
+			},
+			expectedTaints: []v1.Taint{
+				{Key: "key1", Value: "value1", Effect: "NoSchedule"},
+				{Key: "key2", Value: "value2", Effect: "NoExecute"},
+			},
+			requestCount: 3,
+		},
+		{
+			name: "add multiple taints to node",
+			nodeHandler: &testutil.FakeNodeHandler{
+				Existing: []*v1.Node{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name: "node1",
+						},
+						Spec: v1.NodeSpec{
+							Taints: []v1.Taint{
+								{Key: "key1", Value: "value1", Effect: "NoSchedule"},
+								{Key: "key2", Value: "value2", Effect: "NoExecute"},
+							},
+						},
+					},
+				},
+				Clientset: fake.NewSimpleClientset(&v1.PodList{Items: []v1.Pod{*testutil.NewPod("pod0", "node0")}}),
+			},
+			nodeName: "node1",
+			taintsToAdd: []*v1.Taint{
+				{Key: "key3", Value: "value3", Effect: "NoSchedule"},
+				{Key: "key4", Value: "value4", Effect: "NoExecute"},
+			},
+			expectedTaints: []v1.Taint{
+				{Key: "key1", Value: "value1", Effect: "NoSchedule"},
+				{Key: "key2", Value: "value2", Effect: "NoExecute"},
+				{Key: "key3", Value: "value3", Effect: "NoSchedule"},
+				{Key: "key4", Value: "value4", Effect: "NoExecute"},
+			},
+			requestCount: 3,
+		},
+		{
+			name: "add exist taints to node",
+			nodeHandler: &testutil.FakeNodeHandler{
+				Existing: []*v1.Node{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name: "node1",
+						},
+						Spec: v1.NodeSpec{
+							Taints: []v1.Taint{
+								{Key: "key1", Value: "value1", Effect: "NoSchedule"},
+								{Key: "key2", Value: "value2", Effect: "NoExecute"},
+							},
+						},
+					},
+				},
+				Clientset: fake.NewSimpleClientset(&v1.PodList{Items: []v1.Pod{*testutil.NewPod("pod0", "node0")}}),
+			},
+			nodeName: "node1",
+			taintsToAdd: []*v1.Taint{
+				{Key: "key2", Value: "value2", Effect: "NoExecute"},
+			},
+			expectedTaints: []v1.Taint{
+				{Key: "key1", Value: "value1", Effect: "NoSchedule"},
+				{Key: "key2", Value: "value2", Effect: "NoExecute"},
+			},
+			requestCount: 3,
+		},
+		{
+			name: "add taint to node without taints",
+			nodeHandler: &testutil.FakeNodeHandler{
+				Existing: []*v1.Node{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name: "node1",
+						},
+					},
+				},
+				Clientset: fake.NewSimpleClientset(&v1.PodList{Items: []v1.Pod{*testutil.NewPod("pod0", "node0")}}),
+			},
+			nodeName: "node1",
+			taintsToAdd: []*v1.Taint{
+				{Key: "key3", Value: "value3", Effect: "NoSchedule"},
+			},
+			expectedTaints: []v1.Taint{
+				{Key: "key3", Value: "value3", Effect: "NoSchedule"},
+			},
+			requestCount: 3,
+		},
+		{
+			name: "add empty taint list to node without taints",
+			nodeHandler: &testutil.FakeNodeHandler{
+				Existing: []*v1.Node{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name: "node1",
+						},
+					},
+				},
+				Clientset: fake.NewSimpleClientset(&v1.PodList{Items: []v1.Pod{*testutil.NewPod("pod0", "node0")}}),
+			},
+			nodeName:       "node1",
+			taintsToAdd:    []*v1.Taint{},
+			expectedTaints: nil,
+			requestCount:   1,
+		},
+		{
+			name: "add empty taint list to node",
+			nodeHandler: &testutil.FakeNodeHandler{
+				Existing: []*v1.Node{
+					{
+						ObjectMeta: metav1.ObjectMeta{
+							Name: "node1",
+						},
+						Spec: v1.NodeSpec{
+							Taints: []v1.Taint{
+								{Key: "key1", Value: "value1", Effect: "NoSchedule"},
+								{Key: "key2", Value: "value2", Effect: "NoExecute"},
+							},
+						},
+					},
+				},
+				Clientset: fake.NewSimpleClientset(&v1.PodList{Items: []v1.Pod{*testutil.NewPod("pod0", "node0")}}),
+			},
+			nodeName:    "node1",
+			taintsToAdd: []*v1.Taint{},
+			expectedTaints: []v1.Taint{
+				{Key: "key1", Value: "value1", Effect: "NoSchedule"},
+				{Key: "key2", Value: "value2", Effect: "NoExecute"},
+			},
+			requestCount: 1,
+		},
+	}
+	for _, test := range tests {
+		if err := AddOrUpdateTaintOnNode(test.nodeHandler, test.nodeName, test.taintsToAdd...); err != nil {
+			t.Errorf("%s: AddOrUpdateTaintOnNode() error = %v", test.name, err)
+		}
+
+		node, _ := test.nodeHandler.Get(test.nodeName, metav1.GetOptions{})
+		if !reflect.DeepEqual(node.Spec.Taints, test.expectedTaints) {
+			t.Errorf("%s: failed to add taint to node: expected %+v, got %+v",
+				test.name, test.expectedTaints, node.Spec.Taints)
+		}
+
+		if test.nodeHandler.RequestCount != test.requestCount {
+			t.Errorf("%s: unexpected request count: expected %+v, got %+v",
+				test.name, test.requestCount, test.nodeHandler.RequestCount)
+		}
+	}
+}

pkg/controller/node/controller_utils.go

@@ -303,7 +303,7 @@ func swapNodeControllerTaint(kubeClient clientset.Interface, taintToAdd, taintTo
 	}
 	glog.V(4).Infof("Added %v Taint to Node %v", taintToAdd, node.Name)
 
-	err = controller.RemoveTaintOffNode(kubeClient, node.Name, taintToRemove, node)
+	err = controller.RemoveTaintOffNode(kubeClient, node.Name, node, taintToRemove)
 	if err != nil {
 		utilruntime.HandleError(
 			fmt.Errorf(

pkg/controller/node/node_controller.go

@@ -1072,12 +1072,12 @@ func (nc *NodeController) markNodeForTainting(node *v1.Node) bool {
 func (nc *NodeController) markNodeAsHealthy(node *v1.Node) (bool, error) {
 	nc.evictorLock.Lock()
 	defer nc.evictorLock.Unlock()
-	err := controller.RemoveTaintOffNode(nc.kubeClient, node.Name, UnreachableTaintTemplate, node)
+	err := controller.RemoveTaintOffNode(nc.kubeClient, node.Name, node, UnreachableTaintTemplate)
 	if err != nil {
 		glog.Errorf("Failed to remove taint from node %v: %v", node.Name, err)
 		return false, err
 	}
-	err = controller.RemoveTaintOffNode(nc.kubeClient, node.Name, NotReadyTaintTemplate, node)
+	err = controller.RemoveTaintOffNode(nc.kubeClient, node.Name, node, NotReadyTaintTemplate)
 	if err != nil {
 		glog.Errorf("Failed to remove taint from node %v: %v", node.Name, err)
 		return false, err

test/e2e/framework/util.go

@@ -2551,7 +2551,7 @@ func ExpectNodeHasLabel(c clientset.Interface, nodeName string, labelKey string,
 }
 
 func RemoveTaintOffNode(c clientset.Interface, nodeName string, taint v1.Taint) {
-	ExpectNoError(controller.RemoveTaintOffNode(c, nodeName, &taint, nil))
+	ExpectNoError(controller.RemoveTaintOffNode(c, nodeName, nil, &taint))
 	VerifyThatTaintIsGone(c, nodeName, &taint)
 }