diff --git a/pkg/util/taints/taints.go b/pkg/util/taints/taints.go index 5957adfb27..5b7ff0d81e 100644 --- a/pkg/util/taints/taints.go +++ b/pkg/util/taints/taints.go @@ -45,15 +45,14 @@ func parseTaint(st string) (v1.Taint, error) { parts2 := strings.Split(parts[1], ":") - effect := v1.TaintEffect(parts2[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, "; ")) } - if effect != v1.TaintEffectNoSchedule && effect != v1.TaintEffectPreferNoSchedule && effect != v1.TaintEffectNoExecute { - return taint, fmt.Errorf("invalid taint spec: %v, unsupported taint effect", st) + effect := v1.TaintEffect(parts2[1]) + if err := validateTaintEffect(effect); err != nil { + return taint, err } taint.Key = parts[0] @@ -63,6 +62,14 @@ func parseTaint(st string) (v1.Taint, error) { return taint, nil } +func validateTaintEffect(effect v1.TaintEffect) error { + if effect != v1.TaintEffectNoSchedule && effect != v1.TaintEffectPreferNoSchedule && effect != v1.TaintEffectNoExecute { + return fmt.Errorf("invalid taint effect: %v, unsupported taint effect", effect) + } + + return nil +} + // NewTaintsVar wraps []api.Taint in a struct that implements flag.Value to allow taints to be // bound to command line flags. func NewTaintsVar(ptr *[]api.Taint) taintsVar { @@ -134,6 +141,14 @@ func ParseTaints(spec []string) ([]v1.Taint, []v1.Taint, error) { taintKey = parts[0] effect = v1.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, v1.Taint{Key: taintKey, Effect: effect}) } else { return nil, nil, fmt.Errorf("unknown taint spec: %v", taintSpec) diff --git a/pkg/util/taints/taints_test.go b/pkg/util/taints/taints_test.go index e27e6ad37a..280cb616d0 100644 --- a/pkg/util/taints/taints_test.go +++ b/pkg/util/taints/taints_test.go @@ -111,3 +111,104 @@ func TestAddOrUpdateTaint(t *testing.T) { newNode, result, err = AddOrUpdateTaint(node, taint) checkResult("Add Duplicate Taint", newNode, taint, result, false, err) } + +func TestParseTaints(t *testing.T) { + cases := []struct { + name string + spec []string + expectedTaints []v1.Taint + expectedTaintsToRemove []v1.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: []v1.Taint{ + { + Key: "foo", + Value: "abc", + Effect: v1.TaintEffectNoSchedule, + }, + { + Key: "bar", + Value: "abc", + Effect: v1.TaintEffectNoSchedule, + }, + }, + expectedErr: false, + }, + { + name: "delete taints", + spec: []string{"foo:NoSchedule-", "bar:NoSchedule-"}, + expectedTaintsToRemove: []v1.Taint{ + { + Key: "foo", + Effect: v1.TaintEffectNoSchedule, + }, + { + Key: "bar", + Effect: v1.TaintEffectNoSchedule, + }, + }, + expectedErr: false, + }, + { + name: "add taints and delete taints", + spec: []string{"foo=abc:NoSchedule", "bar=abc:NoSchedule", "foo:NoSchedule-", "bar:NoSchedule-"}, + expectedTaints: []v1.Taint{ + { + Key: "foo", + Value: "abc", + Effect: v1.TaintEffectNoSchedule, + }, + { + Key: "bar", + Value: "abc", + Effect: v1.TaintEffectNoSchedule, + }, + }, + expectedTaintsToRemove: []v1.Taint{ + { + Key: "foo", + Effect: v1.TaintEffectNoSchedule, + }, + { + Key: "bar", + Effect: v1.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) + } + } +}