Copy RemoveTaintOffNode logic to k8s.io/cloud-provider

pkg/controller/cloud/BUILD

@@ -9,7 +9,6 @@ go_library(
     importpath = "k8s.io/kubernetes/pkg/controller/cloud",
     visibility = ["//visibility:public"],
     deps = [
-        "//pkg/controller:go_default_library",
         "//staging/src/k8s.io/api/core/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",

pkg/controller/cloud/node_lifecycle_controller.go

@@ -38,7 +38,6 @@ import (
 	cloudproviderapi "k8s.io/cloud-provider/api"
 	cloudnodeutil "k8s.io/cloud-provider/node/helpers"
 	"k8s.io/klog"
-	"k8s.io/kubernetes/pkg/controller"
 )
 
 const (
@@ -140,7 +139,7 @@ func (c *CloudNodeLifecycleController) MonitorNodes() {
 
 		if status == v1.ConditionTrue {
 			// if taint exist remove taint
-			err = controller.RemoveTaintOffNode(c.kubeClient, node.Name, node, ShutdownTaint)
+			err = cloudnodeutil.RemoveTaintOffNode(c.kubeClient, node.Name, node, ShutdownTaint)
 			if err != nil {
 				klog.Errorf("error patching node taints: %v", err)
 			}
@@ -185,7 +184,7 @@ func (c *CloudNodeLifecycleController) MonitorNodes() {
 
 			if shutdown && err == nil {
 				// if node is shutdown add shutdown taint
-				err = controller.AddOrUpdateTaintOnNode(c.kubeClient, node.Name, ShutdownTaint)
+				err = cloudnodeutil.AddOrUpdateTaintOnNode(c.kubeClient, node.Name, ShutdownTaint)
 				if err != nil {
 					klog.Errorf("failed to apply shutdown taint to node %s, it may have been deleted.", node.Name)
 				}

staging/src/k8s.io/cloud-provider/node/helpers/BUILD

@@ -43,7 +43,10 @@ filegroup(
 
 go_test(
     name = "go_default_test",
-    srcs = ["address_test.go"],
+    srcs = [
+        "address_test.go",
+        "taints_test.go",
+    ],
     embed = [":go_default_library"],
     deps = [
         "//staging/src/k8s.io/api/core/v1:go_default_library",

staging/src/k8s.io/cloud-provider/node/helpers/taints.go

@@ -139,3 +139,102 @@ func addOrUpdateTaint(node *v1.Node, taint *v1.Taint) (*v1.Node, bool, error) {
 	newNode.Spec.Taints = newTaints
 	return newNode, true, nil
 }
+
+// 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 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.CoreV1().Nodes().Get(context.TODO(), nodeName, metav1.GetOptions{ResourceVersion: "0"})
+			firstTry = false
+		} else {
+			oldNode, err = c.CoreV1().Nodes().Get(context.TODO(), nodeName, metav1.GetOptions{})
+		}
+		if err != nil {
+			return err
+		}
+
+		var newNode *v1.Node
+		oldNodeCopy := oldNode
+		updated := false
+		for _, taint := range taints {
+			curNewNode, ok, err := 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)
+	})
+}
+
+// taintExists checks if the given taint exists in list of taints. Returns true if exists false otherwise.
+func taintExists(taints []v1.Taint, taintToFind *v1.Taint) bool {
+	for _, taint := range taints {
+		if taint.MatchTaint(taintToFind) {
+			return true
+		}
+	}
+	return false
+}
+
+// removeTaint tries to remove a taint from annotations list. Returns a new copy of updated Node and true if something was updated
+// false otherwise.
+func removeTaint(node *v1.Node, taint *v1.Taint) (*v1.Node, bool, error) {
+	newNode := node.DeepCopy()
+	nodeTaints := newNode.Spec.Taints
+	if len(nodeTaints) == 0 {
+		return newNode, false, nil
+	}
+
+	if !taintExists(nodeTaints, taint) {
+		return newNode, false, nil
+	}
+
+	newTaints, _ := deleteTaint(nodeTaints, taint)
+	newNode.Spec.Taints = newTaints
+	return newNode, true, nil
+}
+
+// deleteTaint removes all the taints that have the same key and effect to given taintToDelete.
+func deleteTaint(taints []v1.Taint, taintToDelete *v1.Taint) ([]v1.Taint, bool) {
+	newTaints := []v1.Taint{}
+	deleted := false
+	for i := range taints {
+		if taintToDelete.MatchTaint(&taints[i]) {
+			deleted = true
+			continue
+		}
+		newTaints = append(newTaints, taints[i])
+	}
+	return newTaints, deleted
+}

staging/src/k8s.io/cloud-provider/node/helpers/taints_test.go

@@ -0,0 +1,225 @@
+/*
+Copyright 2016 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package helpers
+
+import (
+	"reflect"
+	"testing"
+
+	"k8s.io/api/core/v1"
+)
+
+func TestTaintExists(t *testing.T) {
+	testingTaints := []v1.Taint{
+		{
+			Key:    "foo_1",
+			Value:  "bar_1",
+			Effect: v1.TaintEffectNoExecute,
+		},
+		{
+			Key:    "foo_2",
+			Value:  "bar_2",
+			Effect: v1.TaintEffectNoSchedule,
+		},
+	}
+
+	cases := []struct {
+		name           string
+		taintToFind    *v1.Taint
+		expectedResult bool
+	}{
+		{
+			name:           "taint exists",
+			taintToFind:    &v1.Taint{Key: "foo_1", Value: "bar_1", Effect: v1.TaintEffectNoExecute},
+			expectedResult: true,
+		},
+		{
+			name:           "different key",
+			taintToFind:    &v1.Taint{Key: "no_such_key", Value: "bar_1", Effect: v1.TaintEffectNoExecute},
+			expectedResult: false,
+		},
+		{
+			name:           "different effect",
+			taintToFind:    &v1.Taint{Key: "foo_1", Value: "bar_1", Effect: v1.TaintEffectNoSchedule},
+			expectedResult: false,
+		},
+	}
+
+	for _, c := range cases {
+		result := taintExists(testingTaints, c.taintToFind)
+
+		if result != c.expectedResult {
+			t.Errorf("[%s] unexpected results: %v", c.name, result)
+			continue
+		}
+	}
+}
+
+func TestRemoveTaint(t *testing.T) {
+	cases := []struct {
+		name           string
+		node           *v1.Node
+		taintToRemove  *v1.Taint
+		expectedTaints []v1.Taint
+		expectedResult bool
+	}{
+		{
+			name: "remove taint unsuccessfully",
+			node: &v1.Node{
+				Spec: v1.NodeSpec{
+					Taints: []v1.Taint{
+						{
+							Key:    "foo",
+							Effect: v1.TaintEffectNoSchedule,
+						},
+					},
+				},
+			},
+			taintToRemove: &v1.Taint{
+				Key:    "foo_1",
+				Effect: v1.TaintEffectNoSchedule,
+			},
+			expectedTaints: []v1.Taint{
+				{
+					Key:    "foo",
+					Effect: v1.TaintEffectNoSchedule,
+				},
+			},
+			expectedResult: false,
+		},
+		{
+			name: "remove taint successfully",
+			node: &v1.Node{
+				Spec: v1.NodeSpec{
+					Taints: []v1.Taint{
+						{
+							Key:    "foo",
+							Effect: v1.TaintEffectNoSchedule,
+						},
+					},
+				},
+			},
+			taintToRemove: &v1.Taint{
+				Key:    "foo",
+				Effect: v1.TaintEffectNoSchedule,
+			},
+			expectedTaints: []v1.Taint{},
+			expectedResult: true,
+		},
+		{
+			name: "remove taint from node with no taint",
+			node: &v1.Node{
+				Spec: v1.NodeSpec{
+					Taints: []v1.Taint{},
+				},
+			},
+			taintToRemove: &v1.Taint{
+				Key:    "foo",
+				Effect: v1.TaintEffectNoSchedule,
+			},
+			expectedTaints: []v1.Taint{},
+			expectedResult: false,
+		},
+	}
+
+	for _, c := range cases {
+		newNode, result, err := removeTaint(c.node, c.taintToRemove)
+		if err != nil {
+			t.Errorf("[%s] should not raise error but got: %v", c.name, err)
+		}
+		if result != c.expectedResult {
+			t.Errorf("[%s] should return %t, but got: %t", c.name, c.expectedResult, result)
+		}
+		if !reflect.DeepEqual(newNode.Spec.Taints, c.expectedTaints) {
+			t.Errorf("[%s] the new node object should have taints %v, but got: %v", c.name, c.expectedTaints, newNode.Spec.Taints)
+		}
+	}
+}
+
+func TestDeleteTaint(t *testing.T) {
+	cases := []struct {
+		name           string
+		taints         []v1.Taint
+		taintToDelete  *v1.Taint
+		expectedTaints []v1.Taint
+		expectedResult bool
+	}{
+		{
+			name: "delete taint with different name",
+			taints: []v1.Taint{
+				{
+					Key:    "foo",
+					Effect: v1.TaintEffectNoSchedule,
+				},
+			},
+			taintToDelete: &v1.Taint{Key: "foo_1", Effect: v1.TaintEffectNoSchedule},
+			expectedTaints: []v1.Taint{
+				{
+					Key:    "foo",
+					Effect: v1.TaintEffectNoSchedule,
+				},
+			},
+			expectedResult: false,
+		},
+		{
+			name: "delete taint with different effect",
+			taints: []v1.Taint{
+				{
+					Key:    "foo",
+					Effect: v1.TaintEffectNoSchedule,
+				},
+			},
+			taintToDelete: &v1.Taint{Key: "foo", Effect: v1.TaintEffectNoExecute},
+			expectedTaints: []v1.Taint{
+				{
+					Key:    "foo",
+					Effect: v1.TaintEffectNoSchedule,
+				},
+			},
+			expectedResult: false,
+		},
+		{
+			name: "delete taint successfully",
+			taints: []v1.Taint{
+				{
+					Key:    "foo",
+					Effect: v1.TaintEffectNoSchedule,
+				},
+			},
+			taintToDelete:  &v1.Taint{Key: "foo", Effect: v1.TaintEffectNoSchedule},
+			expectedTaints: []v1.Taint{},
+			expectedResult: true,
+		},
+		{
+			name:           "delete taint from empty taint array",
+			taints:         []v1.Taint{},
+			taintToDelete:  &v1.Taint{Key: "foo", Effect: v1.TaintEffectNoSchedule},
+			expectedTaints: []v1.Taint{},
+			expectedResult: false,
+		},
+	}
+
+	for _, c := range cases {
+		taints, result := deleteTaint(c.taints, c.taintToDelete)
+		if result != c.expectedResult {
+			t.Errorf("[%s] should return %t, but got: %t", c.name, c.expectedResult, result)
+		}
+		if !reflect.DeepEqual(taints, c.expectedTaints) {
+			t.Errorf("[%s] the result taints should be %v, but got: %v", c.name, c.expectedTaints, taints)
+		}
+	}
+}