diff --git a/pkg/api/helpers.go b/pkg/api/helpers.go index 50e811b352f..c78ef0e89af 100644 --- a/pkg/api/helpers.go +++ b/pkg/api/helpers.go @@ -526,6 +526,20 @@ func TaintToleratedByTolerations(taint *Taint, tolerations []Toleration) bool { return tolerated } +// MatchTaint checks if the taint matches taintToMatch. Taints are unique by key:effect, +// if the two taints have same key:effect, regard as they match. +func (t *Taint) MatchTaint(taintToMatch Taint) bool { + return t.Key == taintToMatch.Key && t.Effect == taintToMatch.Effect +} + +// taint.ToString() converts taint struct to string in format key=value:effect or key:effect. +func (t *Taint) ToString() string { + if len(t.Value) == 0 { + return fmt.Sprintf("%v:%v", t.Key, t.Effect) + } + return fmt.Sprintf("%v=%v:%v", t.Key, t.Value, t.Effect) +} + func GetAvoidPodsFromNodeAnnotations(annotations map[string]string) (AvoidPods, error) { var avoidPods AvoidPods if len(annotations) > 0 && annotations[PreferAvoidPodsAnnotationKey] != "" { diff --git a/pkg/api/helpers_test.go b/pkg/api/helpers_test.go index b40f6fdca3e..70abca2a1b3 100644 --- a/pkg/api/helpers_test.go +++ b/pkg/api/helpers_test.go @@ -296,6 +296,107 @@ func TestGetAffinityFromPod(t *testing.T) { } } +func TestTaintToString(t *testing.T) { + testCases := []struct { + taint *Taint + expectedString string + }{ + { + taint: &Taint{ + Key: "foo", + Value: "bar", + Effect: TaintEffectNoSchedule, + }, + expectedString: "foo=bar:NoSchedule", + }, + { + taint: &Taint{ + Key: "foo", + Effect: TaintEffectNoSchedule, + }, + expectedString: "foo:NoSchedule", + }, + } + + for i, tc := range testCases { + if tc.expectedString != tc.taint.ToString() { + t.Errorf("[%v] expected taint %v converted to %s, got %s", i, tc.taint, tc.expectedString, tc.taint.ToString()) + } + } +} + +func TestMatchTaint(t *testing.T) { + testCases := []struct { + description string + taint *Taint + taintToMatch Taint + expectMatch bool + }{ + { + description: "two taints with the same key,value,effect should match", + taint: &Taint{ + Key: "foo", + Value: "bar", + Effect: TaintEffectNoSchedule, + }, + taintToMatch: Taint{ + Key: "foo", + Value: "bar", + Effect: TaintEffectNoSchedule, + }, + expectMatch: true, + }, + { + description: "two taints with the same key,effect but different value should match", + taint: &Taint{ + Key: "foo", + Value: "bar", + Effect: TaintEffectNoSchedule, + }, + taintToMatch: Taint{ + Key: "foo", + Value: "different-value", + Effect: TaintEffectNoSchedule, + }, + expectMatch: true, + }, + { + description: "two taints with the different key cannot match", + taint: &Taint{ + Key: "foo", + Value: "bar", + Effect: TaintEffectNoSchedule, + }, + taintToMatch: Taint{ + Key: "different-key", + Value: "bar", + Effect: TaintEffectNoSchedule, + }, + expectMatch: false, + }, + { + description: "two taints with the different effect cannot match", + taint: &Taint{ + Key: "foo", + Value: "bar", + Effect: TaintEffectNoSchedule, + }, + taintToMatch: Taint{ + Key: "foo", + Value: "bar", + Effect: TaintEffectPreferNoSchedule, + }, + expectMatch: false, + }, + } + + for _, tc := range testCases { + if tc.expectMatch != tc.taint.MatchTaint(tc.taintToMatch) { + t.Errorf("[%s] expect taint %s match taint %s", tc.description, tc.taint.ToString(), tc.taintToMatch.ToString()) + } + } +} + func TestGetAvoidPodsFromNode(t *testing.T) { controllerFlag := true testCases := []struct { diff --git a/pkg/kubectl/cmd/taint.go b/pkg/kubectl/cmd/taint.go index a794a7184b7..25b7dcb8d9e 100644 --- a/pkg/kubectl/cmd/taint.go +++ b/pkg/kubectl/cmd/taint.go @@ -144,7 +144,7 @@ func reorganizeTaints(accessor meta.Object, overwrite bool, taintsToAdd []api.Ta for _, oldTaint := range oldTaints { existsInNew := false for _, taint := range newTaints { - if taint.Key == oldTaint.Key && taint.Effect == oldTaint.Effect { + if taint.MatchTaint(oldTaint) { existsInNew = true break } diff --git a/pkg/kubectl/describe.go b/pkg/kubectl/describe.go index d3aaefcdabf..b6a600ced11 100644 --- a/pkg/kubectl/describe.go +++ b/pkg/kubectl/describe.go @@ -2608,23 +2608,19 @@ func printTaintsMultilineWithIndent(out io.Writer, initialIndent, title, innerIn // to print taints in the sorted order keys := make([]string, 0, len(taints)) for _, taint := range taints { - keys = append(keys, taint.Key) + keys = append(keys, string(taint.Effect)+","+taint.Key) } sort.Strings(keys) - effects := []api.TaintEffect{api.TaintEffectNoSchedule, api.TaintEffectPreferNoSchedule} - for i, key := range keys { - for _, effect := range effects { - for _, taint := range taints { - if taint.Key == key && taint.Effect == effect { - if i != 0 { - fmt.Fprint(out, initialIndent) - fmt.Fprint(out, innerIndent) - } - fmt.Fprintf(out, "%s=%s:%s\n", taint.Key, taint.Value, taint.Effect) - i++ + for _, taint := range taints { + if string(taint.Effect)+","+taint.Key == key { + if i != 0 { + fmt.Fprint(out, initialIndent) + fmt.Fprint(out, innerIndent) } + fmt.Fprintf(out, "%s\n", taint.ToString()) + i++ } } } diff --git a/test/e2e/framework/util.go b/test/e2e/framework/util.go index 6147f94f747..6240d351586 100644 --- a/test/e2e/framework/util.go +++ b/test/e2e/framework/util.go @@ -3059,7 +3059,7 @@ func AddOrUpdateTaintOnNode(c *client.Client, nodeName string, taint api.Taint) var newTaints []api.Taint updated := false for _, existingTaint := range nodeTaints { - if existingTaint.Key == taint.Key { + if taint.MatchTaint(existingTaint) { newTaints = append(newTaints, taint) updated = true continue @@ -3093,49 +3093,49 @@ func AddOrUpdateTaintOnNode(c *client.Client, nodeName string, taint api.Taint) } } -func taintExists(taints []api.Taint, taintKey string) bool { +func taintExists(taints []api.Taint, taintToFind api.Taint) bool { for _, taint := range taints { - if taint.Key == taintKey { + if taint.MatchTaint(taintToFind) { return true } } return false } -func ExpectNodeHasTaint(c *client.Client, nodeName string, taintKey string) { - By("verifying the node has the taint " + taintKey) +func ExpectNodeHasTaint(c *client.Client, nodeName string, taint api.Taint) { + By("verifying the node has the taint " + taint.ToString()) node, err := c.Nodes().Get(nodeName) ExpectNoError(err) nodeTaints, err := api.GetTaintsFromNodeAnnotations(node.Annotations) ExpectNoError(err) - if len(nodeTaints) == 0 || !taintExists(nodeTaints, taintKey) { - Failf("Failed to find taint %s on node %s", taintKey, nodeName) + if len(nodeTaints) == 0 || !taintExists(nodeTaints, taint) { + Failf("Failed to find taint %s on node %s", taint.ToString(), nodeName) } } -func deleteTaintByKey(taints []api.Taint, taintKey string) ([]api.Taint, error) { +func deleteTaint(oldTaints []api.Taint, taintToDelete api.Taint) ([]api.Taint, error) { newTaints := []api.Taint{} found := false - for _, taint := range taints { - if taint.Key == taintKey { + for _, oldTaint := range oldTaints { + if oldTaint.MatchTaint(taintToDelete) { found = true continue } - newTaints = append(newTaints, taint) + newTaints = append(newTaints, taintToDelete) } if !found { - return nil, fmt.Errorf("taint key=\"%s\" not found.", taintKey) + return nil, fmt.Errorf("taint %s not found.", taintToDelete.ToString()) } return newTaints, nil } // RemoveTaintOffNode is for cleaning up taints temporarily added to node, // won't fail if target taint doesn't exist or has been removed. -func RemoveTaintOffNode(c *client.Client, nodeName string, taintKey string) { - By("removing the taint " + taintKey + " off the node " + nodeName) +func RemoveTaintOffNode(c *client.Client, nodeName string, taint api.Taint) { + By("removing the taint " + taint.ToString() + " off the node " + nodeName) for attempt := 0; attempt < UpdateRetries; attempt++ { node, err := c.Nodes().Get(nodeName) ExpectNoError(err) @@ -3146,11 +3146,11 @@ func RemoveTaintOffNode(c *client.Client, nodeName string, taintKey string) { return } - if !taintExists(nodeTaints, taintKey) { + if !taintExists(nodeTaints, taint) { return } - newTaints, err := deleteTaintByKey(nodeTaints, taintKey) + newTaints, err := deleteTaint(nodeTaints, taint) ExpectNoError(err) taintsData, err := json.Marshal(newTaints) @@ -3161,7 +3161,7 @@ func RemoveTaintOffNode(c *client.Client, nodeName string, taintKey string) { if !apierrs.IsConflict(err) { ExpectNoError(err) } else { - Logf("Conflict when trying to add/update taint %v to %v", taintKey, nodeName) + Logf("Conflict when trying to add/update taint %s to node %v", taint.ToString(), nodeName) } } else { break @@ -3171,11 +3171,11 @@ func RemoveTaintOffNode(c *client.Client, nodeName string, taintKey string) { nodeUpdated, err := c.Nodes().Get(nodeName) ExpectNoError(err) - By("verifying the node doesn't have the taint " + taintKey) + By("verifying the node doesn't have the taint " + taint.ToString()) taintsGot, err := api.GetTaintsFromNodeAnnotations(nodeUpdated.Annotations) ExpectNoError(err) - if taintExists(taintsGot, taintKey) { - Failf("Failed removing taint " + taintKey + " of the node " + nodeName) + if taintExists(taintsGot, taint) { + Failf("Failed removing taint " + taint.ToString() + " of the node " + nodeName) } } diff --git a/test/e2e/kubectl.go b/test/e2e/kubectl.go index 9555a36f729..c62cb702587 100644 --- a/test/e2e/kubectl.go +++ b/test/e2e/kubectl.go @@ -1232,76 +1232,83 @@ var _ = framework.KubeDescribe("Kubectl client", func() { framework.KubeDescribe("Kubectl taint", func() { It("should update the taint on a node", func() { - taintName := fmt.Sprintf("kubernetes.io/e2e-taint-key-%s", string(uuid.NewUUID())) - taintValue := "testing-taint-value" - taintEffect := fmt.Sprintf("%s", api.TaintEffectNoSchedule) + testTaint := api.Taint{ + Key: fmt.Sprintf("kubernetes.io/e2e-taint-key-%s", string(uuid.NewUUID())), + Value: "testing-taint-value", + Effect: api.TaintEffectNoSchedule, + } nodes, err := c.Nodes().List(api.ListOptions{}) Expect(err).NotTo(HaveOccurred()) node := nodes.Items[0] nodeName := node.Name - By("adding the taint " + taintName + " with value " + taintValue + " and taint effect " + taintEffect + " to a node") - framework.RunKubectlOrDie("taint", "nodes", nodeName, taintName+"="+taintValue+":"+taintEffect) - By("verifying the node has the taint " + taintName + " with the value " + taintValue) + By("adding the taint " + testTaint.ToString() + " to a node") + framework.RunKubectlOrDie("taint", "nodes", nodeName, testTaint.ToString()) + By("verifying the node has the taint " + testTaint.ToString()) output := framework.RunKubectlOrDie("describe", "node", nodeName) requiredStrings := [][]string{ {"Name:", nodeName}, {"Taints:"}, - {taintName + "=" + taintValue + ":" + taintEffect}, + {testTaint.ToString()}, } checkOutput(output, requiredStrings) - By("removing the taint " + taintName + " of a node") - framework.RunKubectlOrDie("taint", "nodes", nodeName, taintName+":"+taintEffect+"-") - By("verifying the node doesn't have the taint " + taintName) + By("removing the taint " + testTaint.ToString() + " of a node") + framework.RunKubectlOrDie("taint", "nodes", nodeName, testTaint.Key+":"+string(testTaint.Effect)+"-") + By("verifying the node doesn't have the taint " + testTaint.Key) output = framework.RunKubectlOrDie("describe", "node", nodeName) - if strings.Contains(output, taintName) { - framework.Failf("Failed removing taint " + taintName + " of the node " + nodeName) + if strings.Contains(output, testTaint.Key) { + framework.Failf("Failed removing taint " + testTaint.Key + " of the node " + nodeName) } }) It("should remove all the taints with the same key off a node", func() { - taintName := fmt.Sprintf("kubernetes.io/e2e-taint-key-%s", string(uuid.NewUUID())) - taintValue := "testing-taint-value" - taintEffect := fmt.Sprintf("%s", api.TaintEffectNoSchedule) + testTaint := api.Taint{ + Key: fmt.Sprintf("kubernetes.io/e2e-taint-key-%s", string(uuid.NewUUID())), + Value: "testing-taint-value", + Effect: api.TaintEffectNoSchedule, + } nodes, err := c.Nodes().List(api.ListOptions{}) Expect(err).NotTo(HaveOccurred()) node := nodes.Items[0] nodeName := node.Name - By("adding the taint " + taintName + " with value " + taintValue + " and taint effect " + taintEffect + " to a node") - framework.RunKubectlOrDie("taint", "nodes", nodeName, taintName+"="+taintValue+":"+taintEffect) - By("verifying the node has the taint " + taintName + " with the value " + taintValue) + By("adding the taint " + testTaint.ToString() + " to a node") + framework.RunKubectlOrDie("taint", "nodes", nodeName, testTaint.ToString()) + By("verifying the node has the taint " + testTaint.ToString()) output := framework.RunKubectlOrDie("describe", "node", nodeName) requiredStrings := [][]string{ {"Name:", nodeName}, {"Taints:"}, - {taintName + "=" + taintValue + ":" + taintEffect}, + {testTaint.ToString()}, } checkOutput(output, requiredStrings) - newTaintValue := "another-testing-taint-value" - newTaintEffect := fmt.Sprintf("%s", api.TaintEffectPreferNoSchedule) - By("adding another taint " + taintName + " with value " + newTaintValue + " and taint effect " + newTaintEffect + " to the node") - framework.RunKubectlOrDie("taint", "nodes", nodeName, taintName+"="+newTaintValue+":"+newTaintEffect) - By("verifying the node has the taint " + taintName + " with the value " + newTaintValue) + newTestTaint := api.Taint{ + Key: testTaint.Key, + Value: "another-testing-taint-value", + Effect: api.TaintEffectPreferNoSchedule, + } + By("adding another taint " + newTestTaint.ToString() + " to the node") + framework.RunKubectlOrDie("taint", "nodes", nodeName, newTestTaint.ToString()) + By("verifying the node has the taint " + newTestTaint.ToString()) output = framework.RunKubectlOrDie("describe", "node", nodeName) requiredStrings = [][]string{ {"Name:", nodeName}, {"Taints:"}, - {taintName + "=" + newTaintValue + ":" + newTaintEffect}, + {newTestTaint.ToString()}, } checkOutput(output, requiredStrings) - By("removing the taint " + taintName + " of a node") - framework.RunKubectlOrDie("taint", "nodes", nodeName, taintName+"-") - By("verifying the node doesn't have the taints " + taintName) + By("removing all taints that have the same key " + testTaint.Key + " of the node") + framework.RunKubectlOrDie("taint", "nodes", nodeName, testTaint.Key+"-") + By("verifying the node doesn't have the taints that have the same key " + testTaint.Key) output = framework.RunKubectlOrDie("describe", "node", nodeName) - if strings.Contains(output, taintName) { - framework.Failf("Failed removing taint " + taintName + " of the node " + nodeName) + if strings.Contains(output, testTaint.Key) { + framework.Failf("Failed removing taints " + testTaint.Key + " of the node " + nodeName) } }) }) diff --git a/test/e2e/scheduler_predicates.go b/test/e2e/scheduler_predicates.go index 1a9904f492c..608ef691c18 100644 --- a/test/e2e/scheduler_predicates.go +++ b/test/e2e/scheduler_predicates.go @@ -1328,12 +1328,14 @@ var _ = framework.KubeDescribe("SchedulerPredicates [Serial]", func() { framework.ExpectNoError(err) By("Trying to apply a random taint on the found node.") - taintName := fmt.Sprintf("kubernetes.io/e2e-taint-key-%s", string(uuid.NewUUID())) - taintValue := "testing-taint-value" - taintEffect := api.TaintEffectNoSchedule - framework.AddOrUpdateTaintOnNode(c, nodeName, api.Taint{Key: taintName, Value: taintValue, Effect: taintEffect}) - framework.ExpectNodeHasTaint(c, nodeName, taintName) - defer framework.RemoveTaintOffNode(c, nodeName, taintName) + testTaint := api.Taint{ + Key: fmt.Sprintf("kubernetes.io/e2e-taint-key-%s", string(uuid.NewUUID())), + Value: "testing-taint-value", + Effect: api.TaintEffectNoSchedule, + } + framework.AddOrUpdateTaintOnNode(c, nodeName, testTaint) + framework.ExpectNodeHasTaint(c, nodeName, testTaint) + defer framework.RemoveTaintOffNode(c, nodeName, testTaint) By("Trying to apply a random label on the found node.") labelKey := fmt.Sprintf("kubernetes.io/e2e-label-key-%s", string(uuid.NewUUID())) @@ -1354,9 +1356,9 @@ var _ = framework.KubeDescribe("SchedulerPredicates [Serial]", func() { "scheduler.alpha.kubernetes.io/tolerations": ` [ { - "key": "` + taintName + `", - "value": "` + taintValue + `", - "effect": "` + string(taintEffect) + `" + "key": "` + testTaint.Key + `", + "value": "` + testTaint.Value + `", + "effect": "` + string(testTaint.Effect) + `" } ]`, }, @@ -1423,12 +1425,14 @@ var _ = framework.KubeDescribe("SchedulerPredicates [Serial]", func() { framework.ExpectNoError(err) By("Trying to apply a random taint on the found node.") - taintName := fmt.Sprintf("kubernetes.io/e2e-taint-key-%s", string(uuid.NewUUID())) - taintValue := "testing-taint-value" - taintEffect := api.TaintEffectNoSchedule - framework.AddOrUpdateTaintOnNode(c, nodeName, api.Taint{Key: taintName, Value: taintValue, Effect: taintEffect}) - framework.ExpectNodeHasTaint(c, nodeName, taintName) - defer framework.RemoveTaintOffNode(c, nodeName, taintName) + testTaint := api.Taint{ + Key: fmt.Sprintf("kubernetes.io/e2e-taint-key-%s", string(uuid.NewUUID())), + Value: "testing-taint-value", + Effect: api.TaintEffectNoSchedule, + } + framework.AddOrUpdateTaintOnNode(c, nodeName, testTaint) + framework.ExpectNodeHasTaint(c, nodeName, testTaint) + defer framework.RemoveTaintOffNode(c, nodeName, testTaint) By("Trying to apply a random label on the found node.") labelKey := fmt.Sprintf("kubernetes.io/e2e-label-key-%s", string(uuid.NewUUID())) @@ -1466,7 +1470,7 @@ var _ = framework.KubeDescribe("SchedulerPredicates [Serial]", func() { verifyResult(c, podNameNoTolerations, 0, 1, ns) By("Removing taint off the node") - framework.RemoveTaintOffNode(c, nodeName, taintName) + framework.RemoveTaintOffNode(c, nodeName, testTaint) // Wait a bit to allow scheduler to do its thing // TODO: this is brittle; there's no guarantee the scheduler will have run in 10 seconds. framework.Logf("Sleeping 10 seconds and crossing our fingers that scheduler will run in that time.")