diff --git a/pkg/kubectl/cmd/taint/BUILD b/pkg/kubectl/cmd/taint/BUILD index a4abc94e270..19767bf0ed7 100644 --- a/pkg/kubectl/cmd/taint/BUILD +++ b/pkg/kubectl/cmd/taint/BUILD @@ -2,7 +2,10 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "go_default_library", - srcs = ["taint.go"], + srcs = [ + "taint.go", + "utils.go", + ], importpath = "k8s.io/kubernetes/pkg/kubectl/cmd/taint", visibility = ["//visibility:public"], deps = [ @@ -10,11 +13,12 @@ go_library( "//pkg/kubectl/scheme:go_default_library", "//pkg/kubectl/util/i18n:go_default_library", "//pkg/kubectl/util/templates:go_default_library", - "//pkg/util/taints:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/meta:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/util/errors:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/strategicpatch:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/validation:go_default_library", "//staging/src/k8s.io/cli-runtime/pkg/genericclioptions:go_default_library", @@ -27,7 +31,10 @@ go_library( go_test( name = "go_default_test", - srcs = ["taint_test.go"], + srcs = [ + "taint_test.go", + "utils_test.go", + ], embed = [":go_default_library"], deps = [ "//pkg/kubectl/cmd/testing:go_default_library", diff --git a/pkg/kubectl/cmd/taint/taint.go b/pkg/kubectl/cmd/taint/taint.go index a094f1db31e..96ba761dcce 100644 --- a/pkg/kubectl/cmd/taint/taint.go +++ b/pkg/kubectl/cmd/taint/taint.go @@ -37,7 +37,6 @@ import ( "k8s.io/kubernetes/pkg/kubectl/scheme" "k8s.io/kubernetes/pkg/kubectl/util/i18n" "k8s.io/kubernetes/pkg/kubectl/util/templates" - taintutils "k8s.io/kubernetes/pkg/util/taints" ) // TaintOptions have the data required to perform the taint operation @@ -159,7 +158,7 @@ func (o *TaintOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []st return fmt.Errorf("at least one taint update is required") } - if o.taintsToAdd, o.taintsToRemove, err = taintutils.ParseTaints(taintArgs); err != nil { + if o.taintsToAdd, o.taintsToRemove, err = parseTaints(taintArgs); err != nil { return cmdutil.UsageErrorf(cmd, err.Error()) } o.builder = f.NewBuilder(). @@ -298,11 +297,11 @@ func (o TaintOptions) updateTaints(obj runtime.Object) (string, error) { return "", fmt.Errorf("unexpected type %T, expected Node", obj) } if !o.overwrite { - if exists := taintutils.CheckIfTaintsAlreadyExists(node.Spec.Taints, o.taintsToAdd); len(exists) != 0 { + if exists := checkIfTaintsAlreadyExists(node.Spec.Taints, o.taintsToAdd); len(exists) != 0 { return "", fmt.Errorf("Node %s already has %v taint(s) with same effect(s) and --overwrite is false", node.Name, exists) } } - operation, newTaints, err := taintutils.ReorganizeTaints(node, o.overwrite, o.taintsToAdd, o.taintsToRemove) + operation, newTaints, err := reorganizeTaints(node, o.overwrite, o.taintsToAdd, o.taintsToRemove) if err != nil { return "", err } diff --git a/pkg/kubectl/cmd/taint/utils.go b/pkg/kubectl/cmd/taint/utils.go new file mode 100644 index 00000000000..46245fb24ec --- /dev/null +++ b/pkg/kubectl/cmd/taint/utils.go @@ -0,0 +1,209 @@ +/* +Copyright 2018 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 taints implements utilites for working with taints +package taint + +import ( + "fmt" + "strings" + + corev1 "k8s.io/api/core/v1" + utilerrors "k8s.io/apimachinery/pkg/util/errors" + "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/apimachinery/pkg/util/validation" +) + +// Exported taint constant strings +const ( + MODIFIED = "modified" + TAINTED = "tainted" + UNTAINTED = "untainted" +) + +// parseTaints takes a spec which is an array and creates slices for new taints to be added, taints to be deleted. +func parseTaints(spec []string) ([]corev1.Taint, []corev1.Taint, error) { + var taints, taintsToRemove []corev1.Taint + uniqueTaints := map[corev1.TaintEffect]sets.String{} + + for _, taintSpec := range spec { + if strings.Index(taintSpec, "=") != -1 && strings.Index(taintSpec, ":") != -1 { + newTaint, err := parseTaint(taintSpec) + if err != nil { + return nil, nil, err + } + // validate if taint is unique by + if len(uniqueTaints[newTaint.Effect]) > 0 && uniqueTaints[newTaint.Effect].Has(newTaint.Key) { + return nil, nil, fmt.Errorf("duplicated taints with the same key and effect: %v", newTaint) + } + // add taint to existingTaints for uniqueness check + if len(uniqueTaints[newTaint.Effect]) == 0 { + uniqueTaints[newTaint.Effect] = sets.String{} + } + uniqueTaints[newTaint.Effect].Insert(newTaint.Key) + + taints = append(taints, newTaint) + } else if strings.HasSuffix(taintSpec, "-") { + taintKey := taintSpec[:len(taintSpec)-1] + var effect corev1.TaintEffect + if strings.Index(taintKey, ":") != -1 { + parts := strings.Split(taintKey, ":") + taintKey = parts[0] + effect = corev1.TaintEffect(parts[1]) + } + + // If effect is specified, need to validate it. + if len(effect) > 0 { + err := validateTaintEffect(effect) + if err != nil { + return nil, nil, err + } + } + taintsToRemove = append(taintsToRemove, corev1.Taint{Key: taintKey, Effect: effect}) + } else { + return nil, nil, fmt.Errorf("unknown taint spec: %v", taintSpec) + } + } + return taints, taintsToRemove, nil +} + +// parseTaint parses a taint from a string. Taint must be of the format '=:'. +func parseTaint(st string) (corev1.Taint, error) { + var taint corev1.Taint + parts := strings.Split(st, "=") + if len(parts) != 2 || len(parts[1]) == 0 || len(validation.IsQualifiedName(parts[0])) > 0 { + return taint, fmt.Errorf("invalid taint spec: %v", st) + } + + parts2 := strings.Split(parts[1], ":") + + errs := validation.IsValidLabelValue(parts2[0]) + if len(parts2) != 2 || len(errs) != 0 { + return taint, fmt.Errorf("invalid taint spec: %v, %s", st, strings.Join(errs, "; ")) + } + + effect := corev1.TaintEffect(parts2[1]) + if err := validateTaintEffect(effect); err != nil { + return taint, err + } + + taint.Key = parts[0] + taint.Value = parts2[0] + taint.Effect = effect + + return taint, nil +} + +func validateTaintEffect(effect corev1.TaintEffect) error { + if effect != corev1.TaintEffectNoSchedule && effect != corev1.TaintEffectPreferNoSchedule && effect != corev1.TaintEffectNoExecute { + return fmt.Errorf("invalid taint effect: %v, unsupported taint effect", effect) + } + + return nil +} + +// ReorganizeTaints returns the updated set of taints, taking into account old taints that were not updated, +// old taints that were updated, old taints that were deleted, and new taints. +func reorganizeTaints(node *corev1.Node, overwrite bool, taintsToAdd []corev1.Taint, taintsToRemove []corev1.Taint) (string, []corev1.Taint, error) { + newTaints := append([]corev1.Taint{}, taintsToAdd...) + oldTaints := node.Spec.Taints + // add taints that already existing but not updated to newTaints + added := addTaints(oldTaints, &newTaints) + allErrs, deleted := deleteTaints(taintsToRemove, &newTaints) + if (added && deleted) || overwrite { + return MODIFIED, newTaints, utilerrors.NewAggregate(allErrs) + } else if added { + return TAINTED, newTaints, utilerrors.NewAggregate(allErrs) + } + return UNTAINTED, newTaints, utilerrors.NewAggregate(allErrs) +} + +// deleteTaints deletes the given taints from the node's taintlist. +func deleteTaints(taintsToRemove []corev1.Taint, newTaints *[]corev1.Taint) ([]error, bool) { + allErrs := []error{} + var removed bool + for _, taintToRemove := range taintsToRemove { + removed = false + if len(taintToRemove.Effect) > 0 { + *newTaints, removed = deleteTaint(*newTaints, &taintToRemove) + } else { + *newTaints, removed = deleteTaintsByKey(*newTaints, taintToRemove.Key) + } + if !removed { + allErrs = append(allErrs, fmt.Errorf("taint %q not found", taintToRemove.ToString())) + } + } + return allErrs, removed +} + +// addTaints adds the newTaints list to existing ones and updates the newTaints List. +// TODO: This needs a rewrite to take only the new values instead of appended newTaints list to be consistent. +func addTaints(oldTaints []corev1.Taint, newTaints *[]corev1.Taint) bool { + for _, oldTaint := range oldTaints { + existsInNew := false + for _, taint := range *newTaints { + if taint.MatchTaint(&oldTaint) { + existsInNew = true + break + } + } + if !existsInNew { + *newTaints = append(*newTaints, oldTaint) + } + } + return len(oldTaints) != len(*newTaints) +} + +// CheckIfTaintsAlreadyExists checks if the node already has taints that we want to add and returns a string with taint keys. +func checkIfTaintsAlreadyExists(oldTaints []corev1.Taint, taints []corev1.Taint) string { + var existingTaintList = make([]string, 0) + for _, taint := range taints { + for _, oldTaint := range oldTaints { + if taint.Key == oldTaint.Key && taint.Effect == oldTaint.Effect { + existingTaintList = append(existingTaintList, taint.Key) + } + } + } + return strings.Join(existingTaintList, ",") +} + +// DeleteTaintsByKey removes all the taints that have the same key to given taintKey +func deleteTaintsByKey(taints []corev1.Taint, taintKey string) ([]corev1.Taint, bool) { + newTaints := []corev1.Taint{} + deleted := false + for i := range taints { + if taintKey == taints[i].Key { + deleted = true + continue + } + newTaints = append(newTaints, taints[i]) + } + return newTaints, deleted +} + +// DeleteTaint removes all the taints that have the same key and effect to given taintToDelete. +func deleteTaint(taints []corev1.Taint, taintToDelete *corev1.Taint) ([]corev1.Taint, bool) { + newTaints := []corev1.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/pkg/kubectl/cmd/taint/utils_test.go b/pkg/kubectl/cmd/taint/utils_test.go new file mode 100644 index 00000000000..1004193dc04 --- /dev/null +++ b/pkg/kubectl/cmd/taint/utils_test.go @@ -0,0 +1,471 @@ +/* +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 taint + +import ( + "reflect" + "testing" + + corev1 "k8s.io/api/core/v1" +) + +func TestDeleteTaint(t *testing.T) { + cases := []struct { + name string + taints []corev1.Taint + taintToDelete *corev1.Taint + expectedTaints []corev1.Taint + expectedResult bool + }{ + { + name: "delete taint with different name", + taints: []corev1.Taint{ + { + Key: "foo", + Effect: corev1.TaintEffectNoSchedule, + }, + }, + taintToDelete: &corev1.Taint{Key: "foo_1", Effect: corev1.TaintEffectNoSchedule}, + expectedTaints: []corev1.Taint{ + { + Key: "foo", + Effect: corev1.TaintEffectNoSchedule, + }, + }, + expectedResult: false, + }, + { + name: "delete taint with different effect", + taints: []corev1.Taint{ + { + Key: "foo", + Effect: corev1.TaintEffectNoSchedule, + }, + }, + taintToDelete: &corev1.Taint{Key: "foo", Effect: corev1.TaintEffectNoExecute}, + expectedTaints: []corev1.Taint{ + { + Key: "foo", + Effect: corev1.TaintEffectNoSchedule, + }, + }, + expectedResult: false, + }, + { + name: "delete taint successfully", + taints: []corev1.Taint{ + { + Key: "foo", + Effect: corev1.TaintEffectNoSchedule, + }, + }, + taintToDelete: &corev1.Taint{Key: "foo", Effect: corev1.TaintEffectNoSchedule}, + expectedTaints: []corev1.Taint{}, + expectedResult: true, + }, + { + name: "delete taint from empty taint array", + taints: []corev1.Taint{}, + taintToDelete: &corev1.Taint{Key: "foo", Effect: corev1.TaintEffectNoSchedule}, + expectedTaints: []corev1.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) + } + } +} + +func TestDeleteTaintByKey(t *testing.T) { + cases := []struct { + name string + taints []corev1.Taint + taintKey string + expectedTaints []corev1.Taint + expectedResult bool + }{ + { + name: "delete taint unsuccessfully", + taints: []corev1.Taint{ + { + Key: "foo", + Value: "bar", + Effect: corev1.TaintEffectNoSchedule, + }, + }, + taintKey: "foo_1", + expectedTaints: []corev1.Taint{ + { + Key: "foo", + Value: "bar", + Effect: corev1.TaintEffectNoSchedule, + }, + }, + expectedResult: false, + }, + { + name: "delete taint successfully", + taints: []corev1.Taint{ + { + Key: "foo", + Value: "bar", + Effect: corev1.TaintEffectNoSchedule, + }, + }, + taintKey: "foo", + expectedTaints: []corev1.Taint{}, + expectedResult: true, + }, + { + name: "delete taint from empty taint array", + taints: []corev1.Taint{}, + taintKey: "foo", + expectedTaints: []corev1.Taint{}, + expectedResult: false, + }, + } + + for _, c := range cases { + taints, result := deleteTaintsByKey(c.taints, c.taintKey) + if result != c.expectedResult { + t.Errorf("[%s] should return %t, but got: %t", c.name, c.expectedResult, result) + } + if !reflect.DeepEqual(c.expectedTaints, taints) { + t.Errorf("[%s] the result taints should be %v, but got: %v", c.name, c.expectedTaints, taints) + } + } +} + +func TestCheckIfTaintsAlreadyExists(t *testing.T) { + oldTaints := []corev1.Taint{ + { + Key: "foo_1", + Value: "bar", + Effect: corev1.TaintEffectNoSchedule, + }, + { + Key: "foo_2", + Value: "bar", + Effect: corev1.TaintEffectNoSchedule, + }, + { + Key: "foo_3", + Value: "bar", + Effect: corev1.TaintEffectNoSchedule, + }, + } + + cases := []struct { + name string + taintsToCheck []corev1.Taint + expectedResult string + }{ + { + name: "empty array", + taintsToCheck: []corev1.Taint{}, + expectedResult: "", + }, + { + name: "no match", + taintsToCheck: []corev1.Taint{ + { + Key: "foo_1", + Effect: corev1.TaintEffectNoExecute, + }, + }, + expectedResult: "", + }, + { + name: "match one taint", + taintsToCheck: []corev1.Taint{ + { + Key: "foo_2", + Effect: corev1.TaintEffectNoSchedule, + }, + }, + expectedResult: "foo_2", + }, + { + name: "match two taints", + taintsToCheck: []corev1.Taint{ + { + Key: "foo_2", + Effect: corev1.TaintEffectNoSchedule, + }, + { + Key: "foo_3", + Effect: corev1.TaintEffectNoSchedule, + }, + }, + expectedResult: "foo_2,foo_3", + }, + } + + for _, c := range cases { + result := checkIfTaintsAlreadyExists(oldTaints, c.taintsToCheck) + if result != c.expectedResult { + t.Errorf("[%s] should return '%s', but got: '%s'", c.name, c.expectedResult, result) + } + } +} + +func TestReorganizeTaints(t *testing.T) { + node := &corev1.Node{ + Spec: corev1.NodeSpec{ + Taints: []corev1.Taint{ + { + Key: "foo", + Value: "bar", + Effect: corev1.TaintEffectNoSchedule, + }, + }, + }, + } + + cases := []struct { + name string + overwrite bool + taintsToAdd []corev1.Taint + taintsToDelete []corev1.Taint + expectedTaints []corev1.Taint + expectedOperation string + expectedErr bool + }{ + { + name: "no changes with overwrite is true", + overwrite: true, + taintsToAdd: []corev1.Taint{}, + taintsToDelete: []corev1.Taint{}, + expectedTaints: node.Spec.Taints, + expectedOperation: MODIFIED, + expectedErr: false, + }, + { + name: "no changes with overwrite is false", + overwrite: false, + taintsToAdd: []corev1.Taint{}, + taintsToDelete: []corev1.Taint{}, + expectedTaints: node.Spec.Taints, + expectedOperation: UNTAINTED, + expectedErr: false, + }, + { + name: "add new taint", + overwrite: false, + taintsToAdd: []corev1.Taint{ + { + Key: "foo_1", + Effect: corev1.TaintEffectNoExecute, + }, + }, + taintsToDelete: []corev1.Taint{}, + expectedTaints: append([]corev1.Taint{{Key: "foo_1", Effect: corev1.TaintEffectNoExecute}}, node.Spec.Taints...), + expectedOperation: TAINTED, + expectedErr: false, + }, + { + name: "delete taint with effect", + overwrite: false, + taintsToAdd: []corev1.Taint{}, + taintsToDelete: []corev1.Taint{ + { + Key: "foo", + Effect: corev1.TaintEffectNoSchedule, + }, + }, + expectedTaints: []corev1.Taint{}, + expectedOperation: UNTAINTED, + expectedErr: false, + }, + { + name: "delete taint with no effect", + overwrite: false, + taintsToAdd: []corev1.Taint{}, + taintsToDelete: []corev1.Taint{ + { + Key: "foo", + }, + }, + expectedTaints: []corev1.Taint{}, + expectedOperation: UNTAINTED, + expectedErr: false, + }, + { + name: "delete non-exist taint", + overwrite: false, + taintsToAdd: []corev1.Taint{}, + taintsToDelete: []corev1.Taint{ + { + Key: "foo_1", + Effect: corev1.TaintEffectNoSchedule, + }, + }, + expectedTaints: node.Spec.Taints, + expectedOperation: UNTAINTED, + expectedErr: true, + }, + { + name: "add new taint and delete old one", + overwrite: false, + taintsToAdd: []corev1.Taint{ + { + Key: "foo_1", + Effect: corev1.TaintEffectNoSchedule, + }, + }, + taintsToDelete: []corev1.Taint{ + { + Key: "foo", + Effect: corev1.TaintEffectNoSchedule, + }, + }, + expectedTaints: []corev1.Taint{ + { + Key: "foo_1", + Effect: corev1.TaintEffectNoSchedule, + }, + }, + expectedOperation: MODIFIED, + expectedErr: false, + }, + } + + for _, c := range cases { + operation, taints, err := reorganizeTaints(node, c.overwrite, c.taintsToAdd, c.taintsToDelete) + if c.expectedErr && err == nil { + t.Errorf("[%s] expect to see an error, but did not get one", c.name) + } else if !c.expectedErr && err != nil { + t.Errorf("[%s] expect not to see an error, but got one: %v", c.name, err) + } + + if !reflect.DeepEqual(c.expectedTaints, taints) { + t.Errorf("[%s] expect to see taint list %#v, but got: %#v", c.name, c.expectedTaints, taints) + } + + if c.expectedOperation != operation { + t.Errorf("[%s] expect to see operation %s, but got: %s", c.name, c.expectedOperation, operation) + } + } +} + +func TestParseTaints(t *testing.T) { + cases := []struct { + name string + spec []string + expectedTaints []corev1.Taint + expectedTaintsToRemove []corev1.Taint + expectedErr bool + }{ + { + name: "invalid spec format", + spec: []string{"foo=abc"}, + expectedErr: true, + }, + { + name: "invalid spec effect for adding taint", + spec: []string{"foo=abc:invalid_effect"}, + expectedErr: true, + }, + { + name: "invalid spec effect for deleting taint", + spec: []string{"foo:invalid_effect-"}, + expectedErr: true, + }, + { + name: "add new taints", + spec: []string{"foo=abc:NoSchedule", "bar=abc:NoSchedule"}, + expectedTaints: []corev1.Taint{ + { + Key: "foo", + Value: "abc", + Effect: corev1.TaintEffectNoSchedule, + }, + { + Key: "bar", + Value: "abc", + Effect: corev1.TaintEffectNoSchedule, + }, + }, + expectedErr: false, + }, + { + name: "delete taints", + spec: []string{"foo:NoSchedule-", "bar:NoSchedule-"}, + expectedTaintsToRemove: []corev1.Taint{ + { + Key: "foo", + Effect: corev1.TaintEffectNoSchedule, + }, + { + Key: "bar", + Effect: corev1.TaintEffectNoSchedule, + }, + }, + expectedErr: false, + }, + { + name: "add taints and delete taints", + spec: []string{"foo=abc:NoSchedule", "bar=abc:NoSchedule", "foo:NoSchedule-", "bar:NoSchedule-"}, + expectedTaints: []corev1.Taint{ + { + Key: "foo", + Value: "abc", + Effect: corev1.TaintEffectNoSchedule, + }, + { + Key: "bar", + Value: "abc", + Effect: corev1.TaintEffectNoSchedule, + }, + }, + expectedTaintsToRemove: []corev1.Taint{ + { + Key: "foo", + Effect: corev1.TaintEffectNoSchedule, + }, + { + Key: "bar", + Effect: corev1.TaintEffectNoSchedule, + }, + }, + expectedErr: false, + }, + } + + for _, c := range cases { + taints, taintsToRemove, err := parseTaints(c.spec) + if c.expectedErr && err == nil { + t.Errorf("[%s] expected error, but got nothing", c.name) + } + if !c.expectedErr && err != nil { + t.Errorf("[%s] expected no error, but got: %v", c.name, err) + } + if !reflect.DeepEqual(c.expectedTaints, taints) { + t.Errorf("[%s] expected returen taints as %v, but got: %v", c.name, c.expectedTaints, taints) + } + if !reflect.DeepEqual(c.expectedTaintsToRemove, taintsToRemove) { + t.Errorf("[%s] expected return taints to be removed as %v, but got: %v", c.name, c.expectedTaintsToRemove, taintsToRemove) + } + } +}