Merge pull request #91241 from pancernik/scheduler-plugin-args-validation

Move Scheduler plugin args validation to apis/config/validation

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)
 	}
 

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",
     ],

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
+}

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())
+		}
+	}
+}

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",
     ],
 )

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
-}

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",

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,

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)
 		}
 	})
-
 }

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"],

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
-}

pkg/scheduler/framework/plugins/podtopologyspread/plugin_test.go

@@ -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)
-			}
-		})
-	}
-}