diff --git a/cmd/kube-scheduler/app/options/deprecated.go b/cmd/kube-scheduler/app/options/deprecated.go index 7f9d03c6a52..bae856bbde7 100644 --- a/cmd/kube-scheduler/app/options/deprecated.go +++ b/cmd/kube-scheduler/app/options/deprecated.go @@ -23,6 +23,7 @@ import ( "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/kubernetes/pkg/scheduler/algorithmprovider" kubeschedulerconfig "k8s.io/kubernetes/pkg/scheduler/apis/config" + "k8s.io/kubernetes/pkg/scheduler/apis/config/validation" "k8s.io/kubernetes/pkg/scheduler/framework/plugins/interpodaffinity" ) @@ -79,7 +80,7 @@ func (o *DeprecatedOptions) Validate() []error { errs = append(errs, field.Required(field.NewPath("policyConfigFile"), "required when --use-legacy-policy-config is true")) } - if err := interpodaffinity.ValidateHardPodAffinityWeight(field.NewPath("hardPodAffinitySymmetricWeight"), o.HardPodAffinitySymmetricWeight); err != nil { + if err := validation.ValidateHardPodAffinityWeight(field.NewPath("hardPodAffinitySymmetricWeight"), o.HardPodAffinitySymmetricWeight); err != nil { errs = append(errs, err) } diff --git a/pkg/scheduler/apis/config/validation/BUILD b/pkg/scheduler/apis/config/validation/BUILD index edea5e2bba6..a259578e3ac 100644 --- a/pkg/scheduler/apis/config/validation/BUILD +++ b/pkg/scheduler/apis/config/validation/BUILD @@ -2,13 +2,17 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "go_default_library", - srcs = ["validation.go"], + srcs = [ + "validation.go", + "validation_pluginargs.go", + ], importpath = "k8s.io/kubernetes/pkg/scheduler/apis/config/validation", visibility = ["//visibility:public"], deps = [ "//pkg/apis/core/v1/helper:go_default_library", "//pkg/scheduler/apis/config:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation: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/validation:go_default_library", @@ -20,10 +24,14 @@ go_library( go_test( name = "go_default_test", - srcs = ["validation_test.go"], + srcs = [ + "validation_pluginargs_test.go", + "validation_test.go", + ], embed = [":go_default_library"], deps = [ "//pkg/scheduler/apis/config:go_default_library", + "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/component-base/config:go_default_library", ], diff --git a/pkg/scheduler/apis/config/validation/validation_pluginargs.go b/pkg/scheduler/apis/config/validation/validation_pluginargs.go new file mode 100644 index 00000000000..19236aafe5b --- /dev/null +++ b/pkg/scheduler/apis/config/validation/validation_pluginargs.go @@ -0,0 +1,135 @@ +/* +Copyright 2020 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 validation + +import ( + "fmt" + + v1 "k8s.io/api/core/v1" + metav1validation "k8s.io/apimachinery/pkg/apis/meta/v1/validation" + "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/kubernetes/pkg/scheduler/apis/config" +) + +// ValidateInterPodAffinityArgs validates that InterPodAffinityArgs are correct. +func ValidateInterPodAffinityArgs(args config.InterPodAffinityArgs) error { + return ValidateHardPodAffinityWeight(field.NewPath("hardPodAffinityWeight"), args.HardPodAffinityWeight) +} + +// ValidateHardPodAffinityWeight validates that weight is within allowed range. +func ValidateHardPodAffinityWeight(path *field.Path, w int32) error { + const ( + minHardPodAffinityWeight = 0 + maxHardPodAffinityWeight = 100 + ) + + if w < minHardPodAffinityWeight || w > maxHardPodAffinityWeight { + msg := fmt.Sprintf("not in valid range [%d-%d]", minHardPodAffinityWeight, maxHardPodAffinityWeight) + return field.Invalid(path, w, msg) + } + return nil +} + +// ValidateNodeLabelArgs validates that NodeLabelArgs are correct. +func ValidateNodeLabelArgs(args config.NodeLabelArgs) error { + if err := validateNoConflict(args.PresentLabels, args.AbsentLabels); err != nil { + return err + } + if err := validateNoConflict(args.PresentLabelsPreference, args.AbsentLabelsPreference); err != nil { + return err + } + return nil +} + +// validateNoConflict validates that presentLabels and absentLabels do not conflict. +func validateNoConflict(presentLabels []string, absentLabels []string) error { + m := make(map[string]struct{}, len(presentLabels)) + for _, l := range presentLabels { + m[l] = struct{}{} + } + for _, l := range absentLabels { + if _, ok := m[l]; ok { + return fmt.Errorf("detecting at least one label (e.g., %q) that exist in both the present(%+v) and absent(%+v) label list", l, presentLabels, absentLabels) + } + } + return nil +} + +// ValidatePodTopologySpreadArgs validates that PodTopologySpreadArgs are correct. +// It replicates the validation from pkg/apis/core/validation.validateTopologySpreadConstraints +// with an additional check for .labelSelector to be nil. +func ValidatePodTopologySpreadArgs(args *config.PodTopologySpreadArgs) error { + var allErrs field.ErrorList + path := field.NewPath("defaultConstraints") + + for i, c := range args.DefaultConstraints { + p := path.Index(i) + if c.MaxSkew <= 0 { + f := p.Child("maxSkew") + allErrs = append(allErrs, field.Invalid(f, c.MaxSkew, "must be greater than zero")) + } + allErrs = append(allErrs, validateTopologyKey(p.Child("topologyKey"), c.TopologyKey)...) + if err := validateWhenUnsatisfiable(p.Child("whenUnsatisfiable"), c.WhenUnsatisfiable); err != nil { + allErrs = append(allErrs, err) + } + if c.LabelSelector != nil { + f := field.Forbidden(p.Child("labelSelector"), "constraint must not define a selector, as they deduced for each pod") + allErrs = append(allErrs, f) + } + if err := validateConstraintNotRepeat(path, args.DefaultConstraints, i); err != nil { + allErrs = append(allErrs, err) + } + } + if len(allErrs) == 0 { + return nil + } + return allErrs.ToAggregate() +} + +func validateTopologyKey(p *field.Path, v string) field.ErrorList { + var allErrs field.ErrorList + if len(v) == 0 { + allErrs = append(allErrs, field.Required(p, "can not be empty")) + } else { + allErrs = append(allErrs, metav1validation.ValidateLabelName(v, p)...) + } + return allErrs +} + +func validateWhenUnsatisfiable(p *field.Path, v v1.UnsatisfiableConstraintAction) *field.Error { + supportedScheduleActions := sets.NewString(string(v1.DoNotSchedule), string(v1.ScheduleAnyway)) + + if len(v) == 0 { + return field.Required(p, "can not be empty") + } + if !supportedScheduleActions.Has(string(v)) { + return field.NotSupported(p, v, supportedScheduleActions.List()) + } + return nil +} + +func validateConstraintNotRepeat(path *field.Path, constraints []v1.TopologySpreadConstraint, idx int) *field.Error { + c := &constraints[idx] + for i := range constraints[:idx] { + other := &constraints[i] + if c.TopologyKey == other.TopologyKey && c.WhenUnsatisfiable == other.WhenUnsatisfiable { + return field.Duplicate(path.Index(idx), fmt.Sprintf("{%v, %v}", c.TopologyKey, c.WhenUnsatisfiable)) + } + } + return nil +} diff --git a/pkg/scheduler/apis/config/validation/validation_pluginargs_test.go b/pkg/scheduler/apis/config/validation/validation_pluginargs_test.go new file mode 100644 index 00000000000..e1ceaa7745a --- /dev/null +++ b/pkg/scheduler/apis/config/validation/validation_pluginargs_test.go @@ -0,0 +1,222 @@ +/* +Copyright 2020 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 validation + +import ( + "testing" + + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/kubernetes/pkg/scheduler/apis/config" +) + +func TestValidateInterPodAffinityArgs(t *testing.T) { + cases := map[string]struct { + args config.InterPodAffinityArgs + wantErr string + }{ + "valid args": { + args: config.InterPodAffinityArgs{ + HardPodAffinityWeight: 10, + }, + }, + "hardPodAffinityWeight less than min": { + args: config.InterPodAffinityArgs{ + HardPodAffinityWeight: -1, + }, + wantErr: `hardPodAffinityWeight: Invalid value: -1: not in valid range [0-100]`, + }, + "hardPodAffinityWeight more than max": { + args: config.InterPodAffinityArgs{ + HardPodAffinityWeight: 101, + }, + wantErr: `hardPodAffinityWeight: Invalid value: 101: not in valid range [0-100]`, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + err := ValidateInterPodAffinityArgs(tc.args) + assertErr(t, tc.wantErr, err) + }) + } +} + +func TestValidateNodeLabelArgs(t *testing.T) { + cases := map[string]struct { + args config.NodeLabelArgs + wantErr string + }{ + "valid config": { + args: config.NodeLabelArgs{ + PresentLabels: []string{"present"}, + AbsentLabels: []string{"absent"}, + PresentLabelsPreference: []string{"present-preference"}, + AbsentLabelsPreference: []string{"absent-preference"}, + }, + }, + "labels conflict": { + args: config.NodeLabelArgs{ + PresentLabels: []string{"label"}, + AbsentLabels: []string{"label"}, + }, + wantErr: `detecting at least one label (e.g., "label") that exist in both the present([label]) and absent([label]) label list`, + }, + "labels preference conflict": { + args: config.NodeLabelArgs{ + PresentLabelsPreference: []string{"label"}, + AbsentLabelsPreference: []string{"label"}, + }, + wantErr: `detecting at least one label (e.g., "label") that exist in both the present([label]) and absent([label]) label list`, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + err := ValidateNodeLabelArgs(tc.args) + assertErr(t, tc.wantErr, err) + }) + } +} + +func TestValidatePodTopologySpreadArgs(t *testing.T) { + cases := map[string]struct { + args *config.PodTopologySpreadArgs + wantErr string + }{ + "valid config": { + args: &config.PodTopologySpreadArgs{ + DefaultConstraints: []v1.TopologySpreadConstraint{ + { + MaxSkew: 1, + TopologyKey: "node", + WhenUnsatisfiable: v1.DoNotSchedule, + }, + { + MaxSkew: 2, + TopologyKey: "zone", + WhenUnsatisfiable: v1.ScheduleAnyway, + }, + }, + }, + }, + "max skew less than zero": { + args: &config.PodTopologySpreadArgs{ + DefaultConstraints: []v1.TopologySpreadConstraint{ + { + MaxSkew: -1, + TopologyKey: "node", + WhenUnsatisfiable: v1.DoNotSchedule, + }, + }, + }, + wantErr: `defaultConstraints[0].maxSkew: Invalid value: -1: must be greater than zero`, + }, + "empty topology key": { + args: &config.PodTopologySpreadArgs{ + DefaultConstraints: []v1.TopologySpreadConstraint{ + { + MaxSkew: 1, + TopologyKey: "", + WhenUnsatisfiable: v1.DoNotSchedule, + }, + }, + }, + wantErr: `defaultConstraints[0].topologyKey: Required value: can not be empty`, + }, + "WhenUnsatisfiable is empty": { + args: &config.PodTopologySpreadArgs{ + DefaultConstraints: []v1.TopologySpreadConstraint{ + { + MaxSkew: 1, + TopologyKey: "node", + WhenUnsatisfiable: "", + }, + }, + }, + wantErr: `defaultConstraints[0].whenUnsatisfiable: Required value: can not be empty`, + }, + "WhenUnsatisfiable contains unsupported action": { + args: &config.PodTopologySpreadArgs{ + DefaultConstraints: []v1.TopologySpreadConstraint{ + { + MaxSkew: 1, + TopologyKey: "node", + WhenUnsatisfiable: "unknown action", + }, + }, + }, + wantErr: `defaultConstraints[0].whenUnsatisfiable: Unsupported value: "unknown action": supported values: "DoNotSchedule", "ScheduleAnyway"`, + }, + "duplicated constraints": { + args: &config.PodTopologySpreadArgs{ + DefaultConstraints: []v1.TopologySpreadConstraint{ + { + MaxSkew: 1, + TopologyKey: "node", + WhenUnsatisfiable: v1.DoNotSchedule, + }, + { + MaxSkew: 2, + TopologyKey: "node", + WhenUnsatisfiable: v1.DoNotSchedule, + }, + }, + }, + wantErr: `defaultConstraints[1]: Duplicate value: "{node, DoNotSchedule}"`, + }, + "label selector present": { + args: &config.PodTopologySpreadArgs{ + DefaultConstraints: []v1.TopologySpreadConstraint{ + { + MaxSkew: 1, + TopologyKey: "key", + WhenUnsatisfiable: v1.DoNotSchedule, + LabelSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "a": "b", + }, + }, + }, + }, + }, + wantErr: `defaultConstraints[0].labelSelector: Forbidden: constraint must not define a selector, as they deduced for each pod`, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + err := ValidatePodTopologySpreadArgs(tc.args) + assertErr(t, tc.wantErr, err) + }) + } +} + +func assertErr(t *testing.T, wantErr string, gotErr error) { + if wantErr == "" { + if gotErr != nil { + t.Fatalf("wanted err to be: 'nil', got: '%s'", gotErr.Error()) + } + } else { + if gotErr == nil { + t.Fatalf("wanted err to be: '%s', got: nil", wantErr) + } + if gotErr.Error() != wantErr { + t.Errorf("wanted err to be: '%s', got '%s'", wantErr, gotErr.Error()) + } + } +} diff --git a/pkg/scheduler/framework/plugins/interpodaffinity/BUILD b/pkg/scheduler/framework/plugins/interpodaffinity/BUILD index 3aebd928c89..3c4ac73435b 100644 --- a/pkg/scheduler/framework/plugins/interpodaffinity/BUILD +++ b/pkg/scheduler/framework/plugins/interpodaffinity/BUILD @@ -11,12 +11,12 @@ go_library( visibility = ["//visibility:public"], deps = [ "//pkg/scheduler/apis/config:go_default_library", + "//pkg/scheduler/apis/config/validation:go_default_library", "//pkg/scheduler/framework/v1alpha1:go_default_library", "//pkg/scheduler/internal/parallelize:go_default_library", "//pkg/scheduler/util:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", - "//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library", "//vendor/k8s.io/klog/v2:go_default_library", ], ) diff --git a/pkg/scheduler/framework/plugins/interpodaffinity/plugin.go b/pkg/scheduler/framework/plugins/interpodaffinity/plugin.go index 69e833e2cef..b4953ad4397 100644 --- a/pkg/scheduler/framework/plugins/interpodaffinity/plugin.go +++ b/pkg/scheduler/framework/plugins/interpodaffinity/plugin.go @@ -20,19 +20,14 @@ import ( "fmt" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/kubernetes/pkg/scheduler/apis/config" + "k8s.io/kubernetes/pkg/scheduler/apis/config/validation" framework "k8s.io/kubernetes/pkg/scheduler/framework/v1alpha1" ) const ( // Name is the name of the plugin used in the plugin registry and configurations. Name = "InterPodAffinity" - - // MinHardPodAffinityWeight is the minimum HardPodAffinityWeight. - MinHardPodAffinityWeight int32 = 0 - // MaxHardPodAffinityWeight is the maximum HardPodAffinityWeight. - MaxHardPodAffinityWeight int32 = 100 ) var _ framework.PreFilterPlugin = &InterPodAffinity{} @@ -65,7 +60,7 @@ func New(plArgs runtime.Object, h framework.FrameworkHandle) (framework.Plugin, if err != nil { return nil, err } - if err := ValidateHardPodAffinityWeight(field.NewPath("hardPodAffinityWeight"), args.HardPodAffinityWeight); err != nil { + if err := validation.ValidateInterPodAffinityArgs(args); err != nil { return nil, err } return &InterPodAffinity{ @@ -81,12 +76,3 @@ func getArgs(obj runtime.Object) (config.InterPodAffinityArgs, error) { } return *ptr, nil } - -// ValidateHardPodAffinityWeight validates that weight is within allowed range. -func ValidateHardPodAffinityWeight(path *field.Path, w int32) error { - if w < MinHardPodAffinityWeight || w > MaxHardPodAffinityWeight { - msg := fmt.Sprintf("not in valid range [%d-%d]", MinHardPodAffinityWeight, MaxHardPodAffinityWeight) - return field.Invalid(path, w, msg) - } - return nil -} diff --git a/pkg/scheduler/framework/plugins/nodelabel/BUILD b/pkg/scheduler/framework/plugins/nodelabel/BUILD index cdc9d062755..9e1b636d1a3 100644 --- a/pkg/scheduler/framework/plugins/nodelabel/BUILD +++ b/pkg/scheduler/framework/plugins/nodelabel/BUILD @@ -7,6 +7,7 @@ go_library( visibility = ["//visibility:public"], deps = [ "//pkg/scheduler/apis/config:go_default_library", + "//pkg/scheduler/apis/config/validation:go_default_library", "//pkg/scheduler/framework/v1alpha1:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library", diff --git a/pkg/scheduler/framework/plugins/nodelabel/node_label.go b/pkg/scheduler/framework/plugins/nodelabel/node_label.go index 350c7e4728c..34e73bb56e4 100644 --- a/pkg/scheduler/framework/plugins/nodelabel/node_label.go +++ b/pkg/scheduler/framework/plugins/nodelabel/node_label.go @@ -24,6 +24,7 @@ import ( "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/kubernetes/pkg/scheduler/apis/config" + "k8s.io/kubernetes/pkg/scheduler/apis/config/validation" framework "k8s.io/kubernetes/pkg/scheduler/framework/v1alpha1" ) @@ -35,32 +36,17 @@ const ( ErrReasonPresenceViolated = "node(s) didn't have the requested labels" ) -// validateArgs validates that presentLabels and absentLabels do not conflict. -func validateNoConflict(presentLabels []string, absentLabels []string) error { - m := make(map[string]struct{}, len(presentLabels)) - for _, l := range presentLabels { - m[l] = struct{}{} - } - for _, l := range absentLabels { - if _, ok := m[l]; ok { - return fmt.Errorf("detecting at least one label (e.g., %q) that exist in both the present(%+v) and absent(%+v) label list", l, presentLabels, absentLabels) - } - } - return nil -} - // New initializes a new plugin and returns it. func New(plArgs runtime.Object, handle framework.FrameworkHandle) (framework.Plugin, error) { args, err := getArgs(plArgs) if err != nil { return nil, err } - if err := validateNoConflict(args.PresentLabels, args.AbsentLabels); err != nil { - return nil, err - } - if err := validateNoConflict(args.PresentLabelsPreference, args.AbsentLabelsPreference); err != nil { + + if err := validation.ValidateNodeLabelArgs(args); err != nil { return nil, err } + return &NodeLabel{ handle: handle, args: args, diff --git a/pkg/scheduler/framework/plugins/nodelabel/node_label_test.go b/pkg/scheduler/framework/plugins/nodelabel/node_label_test.go index 327f4d4d64c..af1e4646fa4 100644 --- a/pkg/scheduler/framework/plugins/nodelabel/node_label_test.go +++ b/pkg/scheduler/framework/plugins/nodelabel/node_label_test.go @@ -27,67 +27,6 @@ import ( "k8s.io/kubernetes/pkg/scheduler/internal/cache" ) -func TestValidateNodeLabelArgs(t *testing.T) { - tests := []struct { - name string - args config.NodeLabelArgs - err bool - }{ - { - name: "happy case", - args: config.NodeLabelArgs{ - PresentLabels: []string{"foo", "bar"}, - AbsentLabels: []string{"baz"}, - PresentLabelsPreference: []string{"foo", "bar"}, - AbsentLabelsPreference: []string{"baz"}, - }, - }, - { - name: "label presence conflict", - // "bar" exists in both present and absent labels therefore validation should fail. - args: config.NodeLabelArgs{ - PresentLabels: []string{"foo", "bar"}, - AbsentLabels: []string{"bar", "baz"}, - PresentLabelsPreference: []string{"foo", "bar"}, - AbsentLabelsPreference: []string{"baz"}, - }, - err: true, - }, - { - name: "label preference conflict", - // "bar" exists in both present and absent labels preferences therefore validation should fail. - args: config.NodeLabelArgs{ - PresentLabels: []string{"foo", "bar"}, - AbsentLabels: []string{"baz"}, - PresentLabelsPreference: []string{"foo", "bar"}, - AbsentLabelsPreference: []string{"bar", "baz"}, - }, - err: true, - }, - { - name: "both label presence and preference conflict", - args: config.NodeLabelArgs{ - PresentLabels: []string{"foo", "bar"}, - AbsentLabels: []string{"bar", "baz"}, - PresentLabelsPreference: []string{"foo", "bar"}, - AbsentLabelsPreference: []string{"bar", "baz"}, - }, - err: true, - }, - } - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - _, err := New(&test.args, nil) - if test.err && err == nil { - t.Fatal("Plugin initialization should fail.") - } - if !test.err && err != nil { - t.Fatalf("Plugin initialization shouldn't fail: %v", err) - } - }) - } -} - func TestNodeLabelFilter(t *testing.T) { label := map[string]string{"foo": "any value", "bar": "any value"} var pod *v1.Pod @@ -341,5 +280,4 @@ func TestNodeLabelScoreWithoutNode(t *testing.T) { t.Errorf("Status mismatch. got: %v, want: %v", status.Code(), framework.Error) } }) - } diff --git a/pkg/scheduler/framework/plugins/podtopologyspread/BUILD b/pkg/scheduler/framework/plugins/podtopologyspread/BUILD index ba1bf868c4d..f696c10da00 100644 --- a/pkg/scheduler/framework/plugins/podtopologyspread/BUILD +++ b/pkg/scheduler/framework/plugins/podtopologyspread/BUILD @@ -12,16 +12,15 @@ go_library( visibility = ["//visibility:public"], deps = [ "//pkg/scheduler/apis/config:go_default_library", + "//pkg/scheduler/apis/config/validation:go_default_library", "//pkg/scheduler/framework/plugins/helper:go_default_library", "//pkg/scheduler/framework/v1alpha1:go_default_library", "//pkg/scheduler/internal/parallelize:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", - "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/validation:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library", - "//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library", "//staging/src/k8s.io/client-go/informers:go_default_library", "//staging/src/k8s.io/client-go/listers/apps/v1:go_default_library", "//staging/src/k8s.io/client-go/listers/core/v1:go_default_library", @@ -33,7 +32,6 @@ go_test( name = "go_default_test", srcs = [ "filtering_test.go", - "plugin_test.go", "scoring_test.go", ], embed = [":go_default_library"], diff --git a/pkg/scheduler/framework/plugins/podtopologyspread/plugin.go b/pkg/scheduler/framework/plugins/podtopologyspread/plugin.go index 64a858148f3..3d70276a03e 100644 --- a/pkg/scheduler/framework/plugins/podtopologyspread/plugin.go +++ b/pkg/scheduler/framework/plugins/podtopologyspread/plugin.go @@ -19,15 +19,12 @@ package podtopologyspread import ( "fmt" - v1 "k8s.io/api/core/v1" - metav1validation "k8s.io/apimachinery/pkg/apis/meta/v1/validation" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/util/sets" - "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/client-go/informers" appslisters "k8s.io/client-go/listers/apps/v1" corelisters "k8s.io/client-go/listers/core/v1" "k8s.io/kubernetes/pkg/scheduler/apis/config" + "k8s.io/kubernetes/pkg/scheduler/apis/config/validation" framework "k8s.io/kubernetes/pkg/scheduler/framework/v1alpha1" ) @@ -36,10 +33,6 @@ const ( ErrReasonConstraintsNotMatch = "node(s) didn't match pod topology spread constraints" ) -var ( - supportedScheduleActions = sets.NewString(string(v1.DoNotSchedule), string(v1.ScheduleAnyway)) -) - // PodTopologySpread is a plugin that ensures pod's topologySpreadConstraints is satisfied. type PodTopologySpread struct { args config.PodTopologySpreadArgs @@ -79,7 +72,7 @@ func New(plArgs runtime.Object, h framework.FrameworkHandle) (framework.Plugin, if err != nil { return nil, err } - if err := validateArgs(&args); err != nil { + if err := validation.ValidatePodTopologySpreadArgs(&args); err != nil { return nil, err } pl := &PodTopologySpread{ @@ -109,64 +102,3 @@ func (pl *PodTopologySpread) setListers(factory informers.SharedInformerFactory) pl.replicaSets = factory.Apps().V1().ReplicaSets().Lister() pl.statefulSets = factory.Apps().V1().StatefulSets().Lister() } - -// validateArgs replicates the validation from -// pkg/apis/core/validation.validateTopologySpreadConstraints. -// This has the additional check for .labelSelector to be nil. -func validateArgs(args *config.PodTopologySpreadArgs) error { - var allErrs field.ErrorList - path := field.NewPath("defaultConstraints") - for i, c := range args.DefaultConstraints { - p := path.Index(i) - if c.MaxSkew <= 0 { - f := p.Child("maxSkew") - allErrs = append(allErrs, field.Invalid(f, c.MaxSkew, "must be greater than zero")) - } - allErrs = append(allErrs, validateTopologyKey(p.Child("topologyKey"), c.TopologyKey)...) - if err := validateWhenUnsatisfiable(p.Child("whenUnsatisfiable"), c.WhenUnsatisfiable); err != nil { - allErrs = append(allErrs, err) - } - if c.LabelSelector != nil { - f := field.Forbidden(p.Child("labelSelector"), "constraint must not define a selector, as they deduced for each pod") - allErrs = append(allErrs, f) - } - if err := validateConstraintNotRepeat(path, args.DefaultConstraints, i); err != nil { - allErrs = append(allErrs, err) - } - } - if len(allErrs) == 0 { - return nil - } - return allErrs.ToAggregate() -} - -func validateTopologyKey(p *field.Path, v string) field.ErrorList { - var allErrs field.ErrorList - if len(v) == 0 { - allErrs = append(allErrs, field.Required(p, "can not be empty")) - } else { - allErrs = append(allErrs, metav1validation.ValidateLabelName(v, p)...) - } - return allErrs -} - -func validateWhenUnsatisfiable(p *field.Path, v v1.UnsatisfiableConstraintAction) *field.Error { - if len(v) == 0 { - return field.Required(p, "can not be empty") - } - if !supportedScheduleActions.Has(string(v)) { - return field.NotSupported(p, v, supportedScheduleActions.List()) - } - return nil -} - -func validateConstraintNotRepeat(path *field.Path, constraints []v1.TopologySpreadConstraint, idx int) *field.Error { - c := &constraints[idx] - for i := range constraints[:idx] { - other := &constraints[i] - if c.TopologyKey == other.TopologyKey && c.WhenUnsatisfiable == other.WhenUnsatisfiable { - return field.Duplicate(path.Index(idx), fmt.Sprintf("{%v, %v}", c.TopologyKey, c.WhenUnsatisfiable)) - } - } - return nil -} diff --git a/pkg/scheduler/framework/plugins/podtopologyspread/plugin_test.go b/pkg/scheduler/framework/plugins/podtopologyspread/plugin_test.go deleted file mode 100644 index 9cc2fd9e0c0..00000000000 --- a/pkg/scheduler/framework/plugins/podtopologyspread/plugin_test.go +++ /dev/null @@ -1,153 +0,0 @@ -/* -Copyright 2020 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 podtopologyspread - -import ( - "strings" - "testing" - - v1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/informers" - "k8s.io/client-go/kubernetes/fake" - "k8s.io/kubernetes/pkg/scheduler/apis/config" - framework "k8s.io/kubernetes/pkg/scheduler/framework/v1alpha1" - "k8s.io/kubernetes/pkg/scheduler/internal/cache" -) - -func TestNew(t *testing.T) { - cases := []struct { - name string - args config.PodTopologySpreadArgs - wantErr string - }{ - {name: "empty args"}, - { - name: "valid constraints", - args: config.PodTopologySpreadArgs{ - DefaultConstraints: []v1.TopologySpreadConstraint{ - { - MaxSkew: 1, - TopologyKey: "node", - WhenUnsatisfiable: v1.ScheduleAnyway, - }, - { - MaxSkew: 5, - TopologyKey: "zone", - WhenUnsatisfiable: v1.DoNotSchedule, - }, - }, - }, - }, - { - name: "repeated constraints", - args: config.PodTopologySpreadArgs{ - DefaultConstraints: []v1.TopologySpreadConstraint{ - { - MaxSkew: 1, - TopologyKey: "node", - WhenUnsatisfiable: v1.ScheduleAnyway, - }, - { - MaxSkew: 5, - TopologyKey: "node", - WhenUnsatisfiable: v1.ScheduleAnyway, - }, - }, - }, - wantErr: "Duplicate value", - }, - { - name: "unknown whenUnsatisfiable", - args: config.PodTopologySpreadArgs{ - DefaultConstraints: []v1.TopologySpreadConstraint{ - { - MaxSkew: 1, - TopologyKey: "node", - WhenUnsatisfiable: "Unknown", - }, - }, - }, - wantErr: "Unsupported value", - }, - { - name: "negative maxSkew", - args: config.PodTopologySpreadArgs{ - DefaultConstraints: []v1.TopologySpreadConstraint{ - { - MaxSkew: -1, - TopologyKey: "node", - WhenUnsatisfiable: v1.ScheduleAnyway, - }, - }, - }, - wantErr: "must be greater than zero", - }, - { - name: "empty topologyKey", - args: config.PodTopologySpreadArgs{ - DefaultConstraints: []v1.TopologySpreadConstraint{ - { - MaxSkew: 1, - WhenUnsatisfiable: v1.ScheduleAnyway, - }, - }, - }, - wantErr: "can not be empty", - }, - { - name: "with label selector", - args: config.PodTopologySpreadArgs{ - DefaultConstraints: []v1.TopologySpreadConstraint{ - { - MaxSkew: 1, - TopologyKey: "rack", - WhenUnsatisfiable: v1.ScheduleAnyway, - LabelSelector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "foo": "bar", - }, - }, - }, - }, - }, - wantErr: "constraint must not define a selector", - }, - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - informerFactory := informers.NewSharedInformerFactory(fake.NewSimpleClientset(), 0) - f, err := framework.NewFramework(nil, nil, nil, - framework.WithSnapshotSharedLister(cache.NewSnapshot(nil, nil)), - framework.WithInformerFactory(informerFactory), - ) - if err != nil { - t.Fatal(err) - } - _, err = New(&tc.args, f) - if len(tc.wantErr) != 0 { - if err == nil || !strings.Contains(err.Error(), tc.wantErr) { - t.Errorf("must fail, got error %q, want %q", err, tc.wantErr) - } - return - } - if err != nil { - t.Fatal(err) - } - }) - } -}