Merge pull request #90563 from cici37/traint

Copy RemoveTaintOffNode logic to k8s.io/cloud-provider
This commit is contained in:
Kubernetes Prow Robot 2020-05-07 14:01:42 -07:00 committed by GitHub
commit b24eba019d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 330 additions and 5 deletions

View File

@ -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",

View File

@ -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)
}

View File

@ -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",

View File

@ -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
}

View File

@ -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)
}
}
}