Skip to content

Commit b24eba0

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#90563 from cici37/traint
Copy RemoveTaintOffNode logic to k8s.io/cloud-provider
2 parents af67408 + 3241101 commit b24eba0

File tree

5 files changed

+330
-5
lines changed

5 files changed

+330
-5
lines changed

‎pkg/controller/cloud/BUILD‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ go_library(
99
importpath = "k8s.io/kubernetes/pkg/controller/cloud",
1010
visibility = ["//visibility:public"],
1111
deps = [
12-
"//pkg/controller:go_default_library",
1312
"//staging/src/k8s.io/api/core/v1:go_default_library",
1413
"//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",
1514
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",

‎pkg/controller/cloud/node_lifecycle_controller.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ import (
3838
cloudproviderapi "k8s.io/cloud-provider/api"
3939
cloudnodeutil "k8s.io/cloud-provider/node/helpers"
4040
"k8s.io/klog"
41-
"k8s.io/kubernetes/pkg/controller"
4241
)
4342

4443
const (
@@ -140,7 +139,7 @@ func (c *CloudNodeLifecycleController) MonitorNodes() {
140139

141140
if status == v1.ConditionTrue {
142141
// if taint exist remove taint
143-
err = controller.RemoveTaintOffNode(c.kubeClient, node.Name, node, ShutdownTaint)
142+
err = cloudnodeutil.RemoveTaintOffNode(c.kubeClient, node.Name, node, ShutdownTaint)
144143
if err != nil {
145144
klog.Errorf("error patching node taints: %v", err)
146145
}
@@ -185,7 +184,7 @@ func (c *CloudNodeLifecycleController) MonitorNodes() {
185184

186185
if shutdown && err == nil {
187186
// if node is shutdown add shutdown taint
188-
err = controller.AddOrUpdateTaintOnNode(c.kubeClient, node.Name, ShutdownTaint)
187+
err = cloudnodeutil.AddOrUpdateTaintOnNode(c.kubeClient, node.Name, ShutdownTaint)
189188
if err != nil {
190189
klog.Errorf("failed to apply shutdown taint to node %s, it may have been deleted.", node.Name)
191190
}

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,10 @@ filegroup(
4343

4444
go_test(
4545
name = "go_default_test",
46-
srcs = ["address_test.go"],
46+
srcs = [
47+
"address_test.go",
48+
"taints_test.go",
49+
],
4750
embed = [":go_default_library"],
4851
deps = [
4952
"//staging/src/k8s.io/api/core/v1:go_default_library",

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

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,3 +139,102 @@ func addOrUpdateTaint(node *v1.Node, taint *v1.Taint) (*v1.Node, bool, error) {
139139
newNode.Spec.Taints = newTaints
140140
return newNode, true, nil
141141
}
142+
143+
// RemoveTaintOffNode is for cleaning up taints temporarily added to node,
144+
// won't fail if target taint doesn't exist or has been removed.
145+
// If passed a node it'll check if there's anything to be done, if taint is not present it won't issue
146+
// any API calls.
147+
func RemoveTaintOffNode(c clientset.Interface, nodeName string, node *v1.Node, taints ...*v1.Taint) error {
148+
if len(taints) == 0 {
149+
return nil
150+
}
151+
// Short circuit for limiting amount of API calls.
152+
if node != nil {
153+
match := false
154+
for _, taint := range taints {
155+
if taintExists(node.Spec.Taints, taint) {
156+
match = true
157+
break
158+
}
159+
}
160+
if !match {
161+
return nil
162+
}
163+
}
164+
165+
firstTry := true
166+
return clientretry.RetryOnConflict(updateTaintBackoff, func() error {
167+
var err error
168+
var oldNode *v1.Node
169+
// First we try getting node from the API server cache, as it's cheaper. If it fails
170+
// we get it from etcd to be sure to have fresh data.
171+
if firstTry {
172+
oldNode, err = c.CoreV1().Nodes().Get(context.TODO(), nodeName, metav1.GetOptions{ResourceVersion: "0"})
173+
firstTry = false
174+
} else {
175+
oldNode, err = c.CoreV1().Nodes().Get(context.TODO(), nodeName, metav1.GetOptions{})
176+
}
177+
if err != nil {
178+
return err
179+
}
180+
181+
var newNode *v1.Node
182+
oldNodeCopy := oldNode
183+
updated := false
184+
for _, taint := range taints {
185+
curNewNode, ok, err := removeTaint(oldNodeCopy, taint)
186+
if err != nil {
187+
return fmt.Errorf("failed to remove taint of node")
188+
}
189+
updated = updated || ok
190+
newNode = curNewNode
191+
oldNodeCopy = curNewNode
192+
}
193+
if !updated {
194+
return nil
195+
}
196+
return PatchNodeTaints(c, nodeName, oldNode, newNode)
197+
})
198+
}
199+
200+
// taintExists checks if the given taint exists in list of taints. Returns true if exists false otherwise.
201+
func taintExists(taints []v1.Taint, taintToFind *v1.Taint) bool {
202+
for _, taint := range taints {
203+
if taint.MatchTaint(taintToFind) {
204+
return true
205+
}
206+
}
207+
return false
208+
}
209+
210+
// removeTaint tries to remove a taint from annotations list. Returns a new copy of updated Node and true if something was updated
211+
// false otherwise.
212+
func removeTaint(node *v1.Node, taint *v1.Taint) (*v1.Node, bool, error) {
213+
newNode := node.DeepCopy()
214+
nodeTaints := newNode.Spec.Taints
215+
if len(nodeTaints) == 0 {
216+
return newNode, false, nil
217+
}
218+
219+
if !taintExists(nodeTaints, taint) {
220+
return newNode, false, nil
221+
}
222+
223+
newTaints, _ := deleteTaint(nodeTaints, taint)
224+
newNode.Spec.Taints = newTaints
225+
return newNode, true, nil
226+
}
227+
228+
// deleteTaint removes all the taints that have the same key and effect to given taintToDelete.
229+
func deleteTaint(taints []v1.Taint, taintToDelete *v1.Taint) ([]v1.Taint, bool) {
230+
newTaints := []v1.Taint{}
231+
deleted := false
232+
for i := range taints {
233+
if taintToDelete.MatchTaint(&taints[i]) {
234+
deleted = true
235+
continue
236+
}
237+
newTaints = append(newTaints, taints[i])
238+
}
239+
return newTaints, deleted
240+
}

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

Lines changed: 225 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,225 @@
1+
/*
2+
Copyright 2016 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package helpers
18+
19+
import (
20+
"reflect"
21+
"testing"
22+
23+
"k8s.io/api/core/v1"
24+
)
25+
26+
func TestTaintExists(t *testing.T) {
27+
testingTaints := []v1.Taint{
28+
{
29+
Key: "foo_1",
30+
Value: "bar_1",
31+
Effect: v1.TaintEffectNoExecute,
32+
},
33+
{
34+
Key: "foo_2",
35+
Value: "bar_2",
36+
Effect: v1.TaintEffectNoSchedule,
37+
},
38+
}
39+
40+
cases := []struct {
41+
name string
42+
taintToFind *v1.Taint
43+
expectedResult bool
44+
}{
45+
{
46+
name: "taint exists",
47+
taintToFind: &v1.Taint{Key: "foo_1", Value: "bar_1", Effect: v1.TaintEffectNoExecute},
48+
expectedResult: true,
49+
},
50+
{
51+
name: "different key",
52+
taintToFind: &v1.Taint{Key: "no_such_key", Value: "bar_1", Effect: v1.TaintEffectNoExecute},
53+
expectedResult: false,
54+
},
55+
{
56+
name: "different effect",
57+
taintToFind: &v1.Taint{Key: "foo_1", Value: "bar_1", Effect: v1.TaintEffectNoSchedule},
58+
expectedResult: false,
59+
},
60+
}
61+
62+
for _, c := range cases {
63+
result := taintExists(testingTaints, c.taintToFind)
64+
65+
if result != c.expectedResult {
66+
t.Errorf("[%s] unexpected results: %v", c.name, result)
67+
continue
68+
}
69+
}
70+
}
71+
72+
func TestRemoveTaint(t *testing.T) {
73+
cases := []struct {
74+
name string
75+
node *v1.Node
76+
taintToRemove *v1.Taint
77+
expectedTaints []v1.Taint
78+
expectedResult bool
79+
}{
80+
{
81+
name: "remove taint unsuccessfully",
82+
node: &v1.Node{
83+
Spec: v1.NodeSpec{
84+
Taints: []v1.Taint{
85+
{
86+
Key: "foo",
87+
Effect: v1.TaintEffectNoSchedule,
88+
},
89+
},
90+
},
91+
},
92+
taintToRemove: &v1.Taint{
93+
Key: "foo_1",
94+
Effect: v1.TaintEffectNoSchedule,
95+
},
96+
expectedTaints: []v1.Taint{
97+
{
98+
Key: "foo",
99+
Effect: v1.TaintEffectNoSchedule,
100+
},
101+
},
102+
expectedResult: false,
103+
},
104+
{
105+
name: "remove taint successfully",
106+
node: &v1.Node{
107+
Spec: v1.NodeSpec{
108+
Taints: []v1.Taint{
109+
{
110+
Key: "foo",
111+
Effect: v1.TaintEffectNoSchedule,
112+
},
113+
},
114+
},
115+
},
116+
taintToRemove: &v1.Taint{
117+
Key: "foo",
118+
Effect: v1.TaintEffectNoSchedule,
119+
},
120+
expectedTaints: []v1.Taint{},
121+
expectedResult: true,
122+
},
123+
{
124+
name: "remove taint from node with no taint",
125+
node: &v1.Node{
126+
Spec: v1.NodeSpec{
127+
Taints: []v1.Taint{},
128+
},
129+
},
130+
taintToRemove: &v1.Taint{
131+
Key: "foo",
132+
Effect: v1.TaintEffectNoSchedule,
133+
},
134+
expectedTaints: []v1.Taint{},
135+
expectedResult: false,
136+
},
137+
}
138+
139+
for _, c := range cases {
140+
newNode, result, err := removeTaint(c.node, c.taintToRemove)
141+
if err != nil {
142+
t.Errorf("[%s] should not raise error but got: %v", c.name, err)
143+
}
144+
if result != c.expectedResult {
145+
t.Errorf("[%s] should return %t, but got: %t", c.name, c.expectedResult, result)
146+
}
147+
if !reflect.DeepEqual(newNode.Spec.Taints, c.expectedTaints) {
148+
t.Errorf("[%s] the new node object should have taints %v, but got: %v", c.name, c.expectedTaints, newNode.Spec.Taints)
149+
}
150+
}
151+
}
152+
153+
func TestDeleteTaint(t *testing.T) {
154+
cases := []struct {
155+
name string
156+
taints []v1.Taint
157+
taintToDelete *v1.Taint
158+
expectedTaints []v1.Taint
159+
expectedResult bool
160+
}{
161+
{
162+
name: "delete taint with different name",
163+
taints: []v1.Taint{
164+
{
165+
Key: "foo",
166+
Effect: v1.TaintEffectNoSchedule,
167+
},
168+
},
169+
taintToDelete: &v1.Taint{Key: "foo_1", Effect: v1.TaintEffectNoSchedule},
170+
expectedTaints: []v1.Taint{
171+
{
172+
Key: "foo",
173+
Effect: v1.TaintEffectNoSchedule,
174+
},
175+
},
176+
expectedResult: false,
177+
},
178+
{
179+
name: "delete taint with different effect",
180+
taints: []v1.Taint{
181+
{
182+
Key: "foo",
183+
Effect: v1.TaintEffectNoSchedule,
184+
},
185+
},
186+
taintToDelete: &v1.Taint{Key: "foo", Effect: v1.TaintEffectNoExecute},
187+
expectedTaints: []v1.Taint{
188+
{
189+
Key: "foo",
190+
Effect: v1.TaintEffectNoSchedule,
191+
},
192+
},
193+
expectedResult: false,
194+
},
195+
{
196+
name: "delete taint successfully",
197+
taints: []v1.Taint{
198+
{
199+
Key: "foo",
200+
Effect: v1.TaintEffectNoSchedule,
201+
},
202+
},
203+
taintToDelete: &v1.Taint{Key: "foo", Effect: v1.TaintEffectNoSchedule},
204+
expectedTaints: []v1.Taint{},
205+
expectedResult: true,
206+
},
207+
{
208+
name: "delete taint from empty taint array",
209+
taints: []v1.Taint{},
210+
taintToDelete: &v1.Taint{Key: "foo", Effect: v1.TaintEffectNoSchedule},
211+
expectedTaints: []v1.Taint{},
212+
expectedResult: false,
213+
},
214+
}
215+
216+
for _, c := range cases {
217+
taints, result := deleteTaint(c.taints, c.taintToDelete)
218+
if result != c.expectedResult {
219+
t.Errorf("[%s] should return %t, but got: %t", c.name, c.expectedResult, result)
220+
}
221+
if !reflect.DeepEqual(taints, c.expectedTaints) {
222+
t.Errorf("[%s] the result taints should be %v, but got: %v", c.name, c.expectedTaints, taints)
223+
}
224+
}
225+
}

0 commit comments

Comments
 (0)