Merge pull request #49699 from xingzhou/kube-49384-bug

Automatic merge from submit-queue (batch tested with PRs 50537, 49699, 50160, 49025, 50205)

AddOrUpdateTaint should ignore duplicate Taint.

The parameter of AddOrUpdateTaint is Taint pointer, so should use
Taint object itself to compare with the node's taint list to ignore
duplicate taint.

While doing #49384, found this issue and fixed.

Fixed part of #49384, other test cases will be added in the following patch

**Release note**:
```
None
```

pkg/controller/controller_utils_test.go

@@ -753,7 +753,7 @@ func TestAddOrUpdateTaintOnNode(t *testing.T) {
 				{Key: "key1", Value: "value1", Effect: "NoSchedule"},
 				{Key: "key2", Value: "value2", Effect: "NoExecute"},
 			},
-			requestCount: 3,
+			requestCount: 2,
 		},
 		{
 			name: "add taint to node without taints",

pkg/util/taints/BUILD

@@ -26,6 +26,7 @@ go_test(
     deps = [
         "//pkg/api:go_default_library",
         "//vendor/github.com/spf13/pflag:go_default_library",
+        "//vendor/k8s.io/api/core/v1:go_default_library",
     ],
 )
 

pkg/util/taints/taints.go

@@ -271,7 +271,7 @@ func AddOrUpdateTaint(node *v1.Node, taint *v1.Taint) (*v1.Node, bool, error) {
 	updated := false
 	for i := range nodeTaints {
 		if taint.MatchTaint(&nodeTaints[i]) {
-			if helper.Semantic.DeepEqual(taint, nodeTaints[i]) {
+			if helper.Semantic.DeepEqual(*taint, nodeTaints[i]) {
 				return newNode, false, nil
 			}
 			newTaints = append(newTaints, *taint)

pkg/util/taints/taints_test.go

@@ -21,6 +21,7 @@ import (
 	"strings"
 	"testing"
 
+	"k8s.io/api/core/v1"
 	"k8s.io/kubernetes/pkg/api"
 
 	"github.com/spf13/pflag"
@@ -74,3 +75,39 @@ func TestTaintsVar(t *testing.T) {
 	}
 
 }
+
+func TestAddOrUpdateTaint(t *testing.T) {
+	node := &v1.Node{}
+
+	taint := &v1.Taint{
+		Key:    "foo",
+		Value:  "bar",
+		Effect: v1.TaintEffectNoSchedule,
+	}
+
+	checkResult := func(testCaseName string, newNode *v1.Node, expectedTaint *v1.Taint, result, expectedResult bool, err error) {
+		if err != nil {
+			t.Errorf("[%s] should not raise error but got %v", testCaseName, err)
+		}
+		if result != expectedResult {
+			t.Errorf("[%s] should return %t, but got: %t", testCaseName, expectedResult, result)
+		}
+		if len(newNode.Spec.Taints) != 1 || !reflect.DeepEqual(newNode.Spec.Taints[0], *expectedTaint) {
+			t.Errorf("[%s] node should only have one taint: %v, but got: %v", testCaseName, *expectedTaint, newNode.Spec.Taints)
+		}
+	}
+
+	// Add a new Taint.
+	newNode, result, err := AddOrUpdateTaint(node, taint)
+	checkResult("Add New Taint", newNode, taint, result, true, err)
+
+	// Update a Taint.
+	taint.Value = "bar_1"
+	newNode, result, err = AddOrUpdateTaint(node, taint)
+	checkResult("Update Taint", newNode, taint, result, true, err)
+
+	// Add a duplicate Taint.
+	node = newNode
+	newNode, result, err = AddOrUpdateTaint(node, taint)
+	checkResult("Add Duplicate Taint", newNode, taint, result, false, err)
+}