diff --git a/pkg/controller/cloud/BUILD b/pkg/controller/cloud/BUILD index 3ed301fa5cb..912860daa6f 100644 --- a/pkg/controller/cloud/BUILD +++ b/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", diff --git a/pkg/controller/cloud/node_lifecycle_controller.go b/pkg/controller/cloud/node_lifecycle_controller.go index 8509a712e55..0f3f298c1df 100644 --- a/pkg/controller/cloud/node_lifecycle_controller.go +++ b/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) } diff --git a/staging/src/k8s.io/cloud-provider/node/helpers/BUILD b/staging/src/k8s.io/cloud-provider/node/helpers/BUILD index 2f944e330ad..6cd983982ce 100644 --- a/staging/src/k8s.io/cloud-provider/node/helpers/BUILD +++ b/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", diff --git a/staging/src/k8s.io/cloud-provider/node/helpers/taints.go b/staging/src/k8s.io/cloud-provider/node/helpers/taints.go index fa8009dd491..ca6d27336a3 100644 --- a/staging/src/k8s.io/cloud-provider/node/helpers/taints.go +++ b/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 +} diff --git a/staging/src/k8s.io/cloud-provider/node/helpers/taints_test.go b/staging/src/k8s.io/cloud-provider/node/helpers/taints_test.go new file mode 100644 index 00000000000..859b517e175 --- /dev/null +++ b/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) + } + } +}