From c6e65079c70100edc647e3ccfdfa9ead1fe71818 Mon Sep 17 00:00:00 2001 From: Dave Chen Date: Thu, 25 Mar 2021 18:18:21 +0800 Subject: [PATCH] Validate plugin config for KubeSchedulerConfiguration Signed-off-by: Dave Chen --- cmd/kube-scheduler/app/options/options.go | 4 +- .../apis/config/validation/validation.go | 142 ++++++++++++------ .../validation/validation_pluginargs.go | 37 ++--- .../validation/validation_pluginargs_test.go | 20 +-- .../apis/config/validation/validation_test.go | 84 ++++++++++- .../defaultpreemption/default_preemption.go | 2 +- .../plugins/interpodaffinity/plugin.go | 2 +- .../plugins/nodeaffinity/node_affinity.go | 2 +- .../framework/plugins/nodelabel/node_label.go | 3 +- .../framework/plugins/noderesources/fit.go | 3 +- .../plugins/noderesources/least_allocated.go | 3 +- .../plugins/noderesources/most_allocated.go | 3 +- .../requested_to_capacity_ratio.go | 3 +- .../plugins/podtopologyspread/plugin.go | 2 +- .../plugins/volumebinding/volume_binding.go | 2 +- .../scheduler_perf/scheduler_perf_test.go | 2 +- 16 files changed, 218 insertions(+), 96 deletions(-) diff --git a/cmd/kube-scheduler/app/options/options.go b/cmd/kube-scheduler/app/options/options.go index 4874a53e686..765664cf49c 100644 --- a/cmd/kube-scheduler/app/options/options.go +++ b/cmd/kube-scheduler/app/options/options.go @@ -184,7 +184,7 @@ func (o *Options) ApplyTo(c *schedulerappconfig.Config) error { if err != nil { return err } - if err := validation.ValidateKubeSchedulerConfiguration(cfg).ToAggregate(); err != nil { + if err := validation.ValidateKubeSchedulerConfiguration(cfg); err != nil { return err } @@ -234,7 +234,7 @@ func emptySchedulerProfileConfig(profiles []kubeschedulerconfig.KubeSchedulerPro func (o *Options) Validate() []error { var errs []error - if err := validation.ValidateKubeSchedulerConfiguration(&o.ComponentConfig).ToAggregate(); err != nil { + if err := validation.ValidateKubeSchedulerConfiguration(&o.ComponentConfig); err != nil { errs = append(errs, err.Errors()...) } errs = append(errs, o.SecureServing.Validate()...) diff --git a/pkg/scheduler/apis/config/validation/validation.go b/pkg/scheduler/apis/config/validation/validation.go index 759d66e5f70..a236799cfca 100644 --- a/pkg/scheduler/apis/config/validation/validation.go +++ b/pkg/scheduler/apis/config/validation/validation.go @@ -17,11 +17,12 @@ limitations under the License. package validation import ( - "errors" "fmt" + "reflect" "github.com/google/go-cmp/cmp" v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/runtime" utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation" @@ -32,67 +33,123 @@ import ( ) // ValidateKubeSchedulerConfiguration ensures validation of the KubeSchedulerConfiguration struct -func ValidateKubeSchedulerConfiguration(cc *config.KubeSchedulerConfiguration) field.ErrorList { - allErrs := field.ErrorList{} - allErrs = append(allErrs, componentbasevalidation.ValidateClientConnectionConfiguration(&cc.ClientConnection, field.NewPath("clientConnection"))...) - allErrs = append(allErrs, componentbasevalidation.ValidateLeaderElectionConfiguration(&cc.LeaderElection, field.NewPath("leaderElection"))...) - +func ValidateKubeSchedulerConfiguration(cc *config.KubeSchedulerConfiguration) utilerrors.Aggregate { + var errs []error + errs = append(errs, componentbasevalidation.ValidateClientConnectionConfiguration(&cc.ClientConnection, field.NewPath("clientConnection")).ToAggregate()) + errs = append(errs, componentbasevalidation.ValidateLeaderElectionConfiguration(&cc.LeaderElection, field.NewPath("leaderElection")).ToAggregate()) profilesPath := field.NewPath("profiles") if cc.Parallelism <= 0 { - allErrs = append(allErrs, field.Invalid(field.NewPath("parallelism"), cc.Parallelism, "should be an integer value greater than zero")) + errs = append(errs, field.Invalid(field.NewPath("parallelism"), cc.Parallelism, "should be an integer value greater than zero")) } if len(cc.Profiles) == 0 { - allErrs = append(allErrs, field.Required(profilesPath, "")) + errs = append(errs, field.Required(profilesPath, "")) } else { existingProfiles := make(map[string]int, len(cc.Profiles)) for i := range cc.Profiles { profile := &cc.Profiles[i] path := profilesPath.Index(i) - allErrs = append(allErrs, validateKubeSchedulerProfile(path, profile)...) + errs = append(errs, validateKubeSchedulerProfile(path, profile)...) if idx, ok := existingProfiles[profile.SchedulerName]; ok { - allErrs = append(allErrs, field.Duplicate(path.Child("schedulerName"), profilesPath.Index(idx).Child("schedulerName"))) + errs = append(errs, field.Duplicate(path.Child("schedulerName"), profilesPath.Index(idx).Child("schedulerName"))) } existingProfiles[profile.SchedulerName] = i } - allErrs = append(allErrs, validateCommonQueueSort(profilesPath, cc.Profiles)...) + errs = append(errs, validateCommonQueueSort(profilesPath, cc.Profiles)...) } for _, msg := range validation.IsValidSocketAddr(cc.HealthzBindAddress) { - allErrs = append(allErrs, field.Invalid(field.NewPath("healthzBindAddress"), cc.HealthzBindAddress, msg)) + errs = append(errs, field.Invalid(field.NewPath("healthzBindAddress"), cc.HealthzBindAddress, msg)) } for _, msg := range validation.IsValidSocketAddr(cc.MetricsBindAddress) { - allErrs = append(allErrs, field.Invalid(field.NewPath("metricsBindAddress"), cc.MetricsBindAddress, msg)) + errs = append(errs, field.Invalid(field.NewPath("metricsBindAddress"), cc.MetricsBindAddress, msg)) } if cc.PercentageOfNodesToScore < 0 || cc.PercentageOfNodesToScore > 100 { - allErrs = append(allErrs, field.Invalid(field.NewPath("percentageOfNodesToScore"), + errs = append(errs, field.Invalid(field.NewPath("percentageOfNodesToScore"), cc.PercentageOfNodesToScore, "not in valid range [0-100]")) } if cc.PodInitialBackoffSeconds <= 0 { - allErrs = append(allErrs, field.Invalid(field.NewPath("podInitialBackoffSeconds"), + errs = append(errs, field.Invalid(field.NewPath("podInitialBackoffSeconds"), cc.PodInitialBackoffSeconds, "must be greater than 0")) } if cc.PodMaxBackoffSeconds < cc.PodInitialBackoffSeconds { - allErrs = append(allErrs, field.Invalid(field.NewPath("podMaxBackoffSeconds"), + errs = append(errs, field.Invalid(field.NewPath("podMaxBackoffSeconds"), cc.PodMaxBackoffSeconds, "must be greater than or equal to PodInitialBackoffSeconds")) } - allErrs = append(allErrs, validateExtenders(field.NewPath("extenders"), cc.Extenders)...) - return allErrs + errs = append(errs, validateExtenders(field.NewPath("extenders"), cc.Extenders)...) + return utilerrors.Flatten(utilerrors.NewAggregate(errs)) } -func validateKubeSchedulerProfile(path *field.Path, profile *config.KubeSchedulerProfile) field.ErrorList { - allErrs := field.ErrorList{} +func validateKubeSchedulerProfile(path *field.Path, profile *config.KubeSchedulerProfile) []error { + var errs []error if len(profile.SchedulerName) == 0 { - allErrs = append(allErrs, field.Required(path.Child("schedulerName"), "")) + errs = append(errs, field.Required(path.Child("schedulerName"), "")) } - return allErrs + errs = append(errs, validatePluginConfig(path, profile)...) + return errs } -func validateCommonQueueSort(path *field.Path, profiles []config.KubeSchedulerProfile) field.ErrorList { - allErrs := field.ErrorList{} +func validatePluginConfig(path *field.Path, profile *config.KubeSchedulerProfile) []error { + var errs []error + m := map[string]interface{}{ + "DefaultPreemption": ValidateDefaultPreemptionArgs, + "InterPodAffinity": ValidateInterPodAffinityArgs, + "NodeAffinity": ValidateNodeAffinityArgs, + "NodeLabel": ValidateNodeLabelArgs, + "NodeResourcesFitArgs": ValidateNodeResourcesFitArgs, + "NodeResourcesLeastAllocated": ValidateNodeResourcesLeastAllocatedArgs, + "NodeResourcesMostAllocated": ValidateNodeResourcesMostAllocatedArgs, + "PodTopologySpread": ValidatePodTopologySpreadArgs, + "RequestedToCapacityRatio": ValidateRequestedToCapacityRatioArgs, + "VolumeBinding": ValidateVolumeBindingArgs, + } + + seenPluginConfig := make(sets.String) + for i := range profile.PluginConfig { + pluginConfigPath := path.Child("pluginConfig").Index(i) + name := profile.PluginConfig[i].Name + args := profile.PluginConfig[i].Args + if seenPluginConfig.Has(name) { + errs = append(errs, field.Duplicate(pluginConfigPath, name)) + } else { + seenPluginConfig.Insert(name) + } + if validateFunc, ok := m[name]; ok { + // type mismatch, no need to validate the `args`. + if reflect.TypeOf(args) != reflect.ValueOf(validateFunc).Type().In(1) { + errs = append(errs, field.Invalid(pluginConfigPath.Child("args"), args, "has to match plugin args")) + return errs + } + in := []reflect.Value{reflect.ValueOf(pluginConfigPath.Child("args")), reflect.ValueOf(args)} + res := reflect.ValueOf(validateFunc).Call(in) + // It's possible that validation function return a Aggregate, just append here and it will be flattened at the end of CC validation. + if res[0].Interface() != nil { + errs = append(errs, res[0].Interface().(error)) + } + } + } + return errs +} + +func validateCommonQueueSort(path *field.Path, profiles []config.KubeSchedulerProfile) []error { + var errs []error var canon config.PluginSet + var queueSortName string + var queueSortArgs runtime.Object if profiles[0].Plugins != nil { canon = profiles[0].Plugins.QueueSort + if len(profiles[0].Plugins.QueueSort.Enabled) != 0 { + queueSortName = profiles[0].Plugins.QueueSort.Enabled[0].Name + } + length := len(profiles[0].Plugins.QueueSort.Enabled) + if length > 1 { + errs = append(errs, field.Invalid(path.Index(0).Child("plugins", "queueSort", "Enabled"), length, "only one queue sort plugin can be enabled")) + } + } + for _, cfg := range profiles[0].PluginConfig { + if len(queueSortName) > 0 && cfg.Name == queueSortName { + queueSortArgs = cfg.Args + } } for i := 1; i < len(profiles); i++ { var curr config.PluginSet @@ -100,11 +157,15 @@ func validateCommonQueueSort(path *field.Path, profiles []config.KubeSchedulerPr curr = profiles[i].Plugins.QueueSort } if !cmp.Equal(canon, curr) { - allErrs = append(allErrs, field.Invalid(path.Index(i).Child("plugins", "queueSort"), curr, "has to match for all profiles")) + errs = append(errs, field.Invalid(path.Index(i).Child("plugins", "queueSort"), curr, "has to match for all profiles")) + } + for _, cfg := range profiles[i].PluginConfig { + if cfg.Name == queueSortName && !cmp.Equal(queueSortArgs, cfg.Args) { + errs = append(errs, field.Invalid(path.Index(i).Child("pluginConfig", "args"), cfg.Args, "has to match for all profiles")) + } } } - // TODO(#88093): Validate that all plugin configs for the queue sort extension match. - return allErrs + return errs } // ValidatePolicy checks for errors in the Config @@ -121,7 +182,7 @@ func ValidatePolicy(policy config.Policy) error { } if extenderErrs := validateExtenders(field.NewPath("extenders"), policy.Extenders); len(extenderErrs) > 0 { - validationErrors = append(validationErrors, extenderErrs.ToAggregate().Errors()...) + validationErrors = append(validationErrors, extenderErrs...) } if policy.HardPodAffinitySymmetricWeight < 0 || policy.HardPodAffinitySymmetricWeight > 100 { @@ -131,14 +192,14 @@ func ValidatePolicy(policy config.Policy) error { } // validateExtenders validates the configured extenders for the Scheduler -func validateExtenders(fldPath *field.Path, extenders []config.Extender) field.ErrorList { - allErrs := field.ErrorList{} +func validateExtenders(fldPath *field.Path, extenders []config.Extender) []error { + var errs []error binders := 0 extenderManagedResources := sets.NewString() for i, extender := range extenders { path := fldPath.Index(i) if len(extender.PrioritizeVerb) > 0 && extender.Weight <= 0 { - allErrs = append(allErrs, field.Invalid(path.Child("weight"), + errs = append(errs, field.Invalid(path.Child("weight"), extender.Weight, "must have a positive weight applied to it")) } if extender.BindVerb != "" { @@ -146,22 +207,19 @@ func validateExtenders(fldPath *field.Path, extenders []config.Extender) field.E } for j, resource := range extender.ManagedResources { managedResourcesPath := path.Child("managedResources").Index(j) - errs := validateExtendedResourceName(v1.ResourceName(resource.Name)) - for _, err := range errs { - allErrs = append(allErrs, field.Invalid(managedResourcesPath.Child("name"), - resource.Name, fmt.Sprintf("%+v", err))) - } + validationErrors := validateExtendedResourceName(managedResourcesPath.Child("name"), v1.ResourceName(resource.Name)) + errs = append(errs, validationErrors...) if extenderManagedResources.Has(resource.Name) { - allErrs = append(allErrs, field.Invalid(managedResourcesPath.Child("name"), + errs = append(errs, field.Invalid(managedResourcesPath.Child("name"), resource.Name, "duplicate extender managed resource name")) } extenderManagedResources.Insert(resource.Name) } } if binders > 1 { - allErrs = append(allErrs, field.Invalid(fldPath, fmt.Sprintf("found %d extenders implementing bind", binders), "only one extender can implement bind")) + errs = append(errs, field.Invalid(fldPath, fmt.Sprintf("found %d extenders implementing bind", binders), "only one extender can implement bind")) } - return allErrs + return errs } // validateCustomPriorities validates that: @@ -207,16 +265,16 @@ func validateCustomPriorities(priorities map[string]config.PriorityPolicy, prior // validateExtendedResourceName checks whether the specified name is a valid // extended resource name. -func validateExtendedResourceName(name v1.ResourceName) []error { +func validateExtendedResourceName(path *field.Path, name v1.ResourceName) []error { var validationErrors []error for _, msg := range validation.IsQualifiedName(string(name)) { - validationErrors = append(validationErrors, errors.New(msg)) + validationErrors = append(validationErrors, field.Invalid(path, name, msg)) } if len(validationErrors) != 0 { return validationErrors } if !v1helper.IsExtendedResourceName(name) { - validationErrors = append(validationErrors, fmt.Errorf("%s is an invalid extended resource name", name)) + validationErrors = append(validationErrors, field.Invalid(path, string(name), "is an invalid extended resource name")) } return validationErrors } diff --git a/pkg/scheduler/apis/config/validation/validation_pluginargs.go b/pkg/scheduler/apis/config/validation/validation_pluginargs.go index a7ac20f1a86..f4b55027c03 100644 --- a/pkg/scheduler/apis/config/validation/validation_pluginargs.go +++ b/pkg/scheduler/apis/config/validation/validation_pluginargs.go @@ -30,8 +30,7 @@ import ( ) // ValidateDefaultPreemptionArgs validates that DefaultPreemptionArgs are correct. -func ValidateDefaultPreemptionArgs(args config.DefaultPreemptionArgs) error { - var path *field.Path +func ValidateDefaultPreemptionArgs(path *field.Path, args *config.DefaultPreemptionArgs) error { var allErrs field.ErrorList percentagePath := path.Child("minCandidateNodesPercentage") absolutePath := path.Child("minCandidateNodesAbsolute") @@ -68,8 +67,7 @@ func validateMinCandidateNodesAbsolute(minCandidateNodesAbsolute int32, p *field } // ValidateInterPodAffinityArgs validates that InterPodAffinityArgs are correct. -func ValidateInterPodAffinityArgs(args config.InterPodAffinityArgs) error { - var path *field.Path +func ValidateInterPodAffinityArgs(path *field.Path, args *config.InterPodAffinityArgs) error { return ValidateHardPodAffinityWeight(path.Child("hardPodAffinityWeight"), args.HardPodAffinityWeight) } @@ -88,8 +86,7 @@ func ValidateHardPodAffinityWeight(path *field.Path, w int32) error { } // ValidateNodeLabelArgs validates that NodeLabelArgs are correct. -func ValidateNodeLabelArgs(args config.NodeLabelArgs) error { - var path *field.Path +func ValidateNodeLabelArgs(path *field.Path, args *config.NodeLabelArgs) error { var allErrs field.ErrorList allErrs = append(allErrs, validateNoConflict(args.PresentLabels, args.AbsentLabels, @@ -120,8 +117,7 @@ func validateNoConflict(presentLabels, absentLabels []string, presentPath, absen // 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 path *field.Path +func ValidatePodTopologySpreadArgs(path *field.Path, args *config.PodTopologySpreadArgs) error { var allErrs field.ErrorList if err := validateDefaultingType(path.Child("defaultingType"), args.DefaultingType, args.DefaultConstraints); err != nil { allErrs = append(allErrs, err) @@ -196,8 +192,7 @@ func validateConstraintNotRepeat(path *field.Path, constraints []v1.TopologySpre } // ValidateRequestedToCapacityRatioArgs validates that RequestedToCapacityRatioArgs are correct. -func ValidateRequestedToCapacityRatioArgs(args config.RequestedToCapacityRatioArgs) error { - var path *field.Path +func ValidateRequestedToCapacityRatioArgs(path *field.Path, args *config.RequestedToCapacityRatioArgs) error { var allErrs field.ErrorList allErrs = append(allErrs, validateFunctionShape(args.Shape, path.Child("shape"))...) allErrs = append(allErrs, validateResourcesNoMax(args.Resources, path.Child("resources"))...) @@ -254,14 +249,12 @@ func validateResourcesNoMax(resources []config.ResourceSpec, p *field.Path) fiel } // ValidateNodeResourcesLeastAllocatedArgs validates that NodeResourcesLeastAllocatedArgs are correct. -func ValidateNodeResourcesLeastAllocatedArgs(args *config.NodeResourcesLeastAllocatedArgs) error { - var path *field.Path +func ValidateNodeResourcesLeastAllocatedArgs(path *field.Path, args *config.NodeResourcesLeastAllocatedArgs) error { return validateResources(args.Resources, path.Child("resources")).ToAggregate() } // ValidateNodeResourcesMostAllocatedArgs validates that NodeResourcesMostAllocatedArgs are correct. -func ValidateNodeResourcesMostAllocatedArgs(args *config.NodeResourcesMostAllocatedArgs) error { - var path *field.Path +func ValidateNodeResourcesMostAllocatedArgs(path *field.Path, args *config.NodeResourcesMostAllocatedArgs) error { return validateResources(args.Resources, path.Child("resources")).ToAggregate() } @@ -277,22 +270,21 @@ func validateResources(resources []config.ResourceSpec, p *field.Path) field.Err } // ValidateNodeAffinityArgs validates that NodeAffinityArgs are correct. -func ValidateNodeAffinityArgs(args *config.NodeAffinityArgs) error { +func ValidateNodeAffinityArgs(path *field.Path, args *config.NodeAffinityArgs) error { if args.AddedAffinity == nil { return nil } affinity := args.AddedAffinity - path := field.NewPath("addedAffinity") var errs []error if ns := affinity.RequiredDuringSchedulingIgnoredDuringExecution; ns != nil { - _, err := nodeaffinity.NewNodeSelector(ns, field.WithPath(path.Child("requiredDuringSchedulingIgnoredDuringExecution"))) + _, err := nodeaffinity.NewNodeSelector(ns, field.WithPath(path.Child("addedAffinity", "requiredDuringSchedulingIgnoredDuringExecution"))) if err != nil { errs = append(errs, err) } } // TODO: Add validation for requiredDuringSchedulingRequiredDuringExecution when it gets added to the API. if terms := affinity.PreferredDuringSchedulingIgnoredDuringExecution; len(terms) != 0 { - _, err := nodeaffinity.NewPreferredSchedulingTerms(terms, field.WithPath(path.Child("preferredDuringSchedulingIgnoredDuringExecution"))) + _, err := nodeaffinity.NewPreferredSchedulingTerms(terms, field.WithPath(path.Child("addedAffinity", "preferredDuringSchedulingIgnoredDuringExecution"))) if err != nil { errs = append(errs, err) } @@ -301,8 +293,7 @@ func ValidateNodeAffinityArgs(args *config.NodeAffinityArgs) error { } // ValidateVolumeBindingArgs validates that VolumeBindingArgs are set correctly. -func ValidateVolumeBindingArgs(args *config.VolumeBindingArgs) error { - var path *field.Path +func ValidateVolumeBindingArgs(path *field.Path, args *config.VolumeBindingArgs) error { var err error if args.BindTimeoutSeconds < 0 { @@ -312,9 +303,9 @@ func ValidateVolumeBindingArgs(args *config.VolumeBindingArgs) error { return err } -func ValidateNodeResourcesFitArgs(args *config.NodeResourcesFitArgs) error { +func ValidateNodeResourcesFitArgs(path *field.Path, args *config.NodeResourcesFitArgs) error { var allErrs field.ErrorList - resPath := field.NewPath("ignoredResources") + resPath := path.Child("ignoredResources") for i, res := range args.IgnoredResources { path := resPath.Index(i) if errs := metav1validation.ValidateLabelName(res, path); len(errs) != 0 { @@ -322,7 +313,7 @@ func ValidateNodeResourcesFitArgs(args *config.NodeResourcesFitArgs) error { } } - groupPath := field.NewPath("ignoredResourceGroups") + groupPath := path.Child("ignoredResourceGroups") for i, group := range args.IgnoredResourceGroups { path := groupPath.Index(i) if strings.Contains(group, "/") { diff --git a/pkg/scheduler/apis/config/validation/validation_pluginargs_test.go b/pkg/scheduler/apis/config/validation/validation_pluginargs_test.go index 28fb70b5577..bbeab2b9d3b 100644 --- a/pkg/scheduler/apis/config/validation/validation_pluginargs_test.go +++ b/pkg/scheduler/apis/config/validation/validation_pluginargs_test.go @@ -114,7 +114,7 @@ func TestValidateDefaultPreemptionArgs(t *testing.T) { for name, tc := range cases { t.Run(name, func(t *testing.T) { - err := ValidateDefaultPreemptionArgs(tc.args) + err := ValidateDefaultPreemptionArgs(nil, &tc.args) if diff := cmp.Diff(tc.wantErrs.ToAggregate(), err, ignoreBadValueDetail); diff != "" { t.Errorf("ValidateDefaultPreemptionArgs returned err (-want,+got):\n%s", diff) } @@ -154,7 +154,7 @@ func TestValidateInterPodAffinityArgs(t *testing.T) { for name, tc := range cases { t.Run(name, func(t *testing.T) { - err := ValidateInterPodAffinityArgs(tc.args) + err := ValidateInterPodAffinityArgs(nil, &tc.args) if diff := cmp.Diff(tc.wantErr, err, ignoreBadValueDetail); diff != "" { t.Errorf("ValidateInterPodAffinityArgs returned err (-want,+got):\n%s", diff) } @@ -235,7 +235,7 @@ func TestValidateNodeLabelArgs(t *testing.T) { for name, tc := range cases { t.Run(name, func(t *testing.T) { - err := ValidateNodeLabelArgs(tc.args) + err := ValidateNodeLabelArgs(nil, &tc.args) if diff := cmp.Diff(tc.wantErrs.ToAggregate(), err, ignoreBadValueDetail); diff != "" { t.Errorf("ValidateNodeLabelArgs returned err (-want,+got):\n%s", diff) } @@ -426,7 +426,7 @@ func TestValidatePodTopologySpreadArgs(t *testing.T) { for name, tc := range cases { t.Run(name, func(t *testing.T) { - err := ValidatePodTopologySpreadArgs(tc.args) + err := ValidatePodTopologySpreadArgs(nil, tc.args) if diff := cmp.Diff(tc.wantErrs.ToAggregate(), err, ignoreBadValueDetail); diff != "" { t.Errorf("ValidatePodTopologySpreadArgs returned err (-want,+got):\n%s", diff) } @@ -668,7 +668,7 @@ func TestValidateRequestedToCapacityRatioArgs(t *testing.T) { for name, tc := range cases { t.Run(name, func(t *testing.T) { - err := ValidateRequestedToCapacityRatioArgs(tc.args) + err := ValidateRequestedToCapacityRatioArgs(nil, &tc.args) if diff := cmp.Diff(tc.wantErrs.ToAggregate(), err, ignoreBadValueDetail); diff != "" { t.Errorf("ValidateRequestedToCapacityRatioArgs returned err (-want,+got):\n%s", diff) } @@ -755,7 +755,7 @@ func TestValidateNodeResourcesLeastAllocatedArgs(t *testing.T) { for name, tc := range cases { t.Run(name, func(t *testing.T) { - err := ValidateNodeResourcesLeastAllocatedArgs(tc.args) + err := ValidateNodeResourcesLeastAllocatedArgs(nil, tc.args) if diff := cmp.Diff(tc.wantErrs.ToAggregate(), err, ignoreBadValueDetail); diff != "" { t.Errorf("ValidateNodeResourcesLeastAllocatedArgs returned err (-want,+got):\n%s", diff) } @@ -842,7 +842,7 @@ func TestValidateNodeResourcesMostAllocatedArgs(t *testing.T) { for name, tc := range cases { t.Run(name, func(t *testing.T) { - err := ValidateNodeResourcesMostAllocatedArgs(tc.args) + err := ValidateNodeResourcesMostAllocatedArgs(nil, tc.args) if diff := cmp.Diff(tc.wantErrs.ToAggregate(), err, ignoreBadValueDetail); diff != "" { t.Errorf("ValidateNodeResourcesLeastAllocatedArgs returned err (-want,+got):\n%s", diff) } @@ -940,7 +940,7 @@ func TestValidateNodeAffinityArgs(t *testing.T) { } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - err := ValidateNodeAffinityArgs(&tc.args) + err := ValidateNodeAffinityArgs(nil, &tc.args) if diff := cmp.Diff(tc.wantErr, err, ignoreBadValueDetail); diff != "" { t.Errorf("ValidatedNodeAffinityArgs returned err (-want,+got):\n%s", diff) } @@ -979,7 +979,7 @@ func TestValidateVolumeBindingArgs(t *testing.T) { } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - err := ValidateVolumeBindingArgs(&tc.args) + err := ValidateVolumeBindingArgs(nil, &tc.args) if diff := cmp.Diff(tc.wantErr, err, ignoreBadValueDetail); diff != "" { t.Errorf("ValidateVolumeBindingArgs returned err (-want,+got):\n%s", diff) } @@ -1051,7 +1051,7 @@ func TestValidateFitArgs(t *testing.T) { for _, test := range argsTest { t.Run(test.name, func(t *testing.T) { - if err := ValidateNodeResourcesFitArgs(&test.args); err != nil && !strings.Contains(err.Error(), test.expect) { + if err := ValidateNodeResourcesFitArgs(nil, &test.args); err != nil && !strings.Contains(err.Error(), test.expect) { t.Errorf("case[%v]: error details do not include %v", test.name, err) } }) diff --git a/pkg/scheduler/apis/config/validation/validation_test.go b/pkg/scheduler/apis/config/validation/validation_test.go index 56171f7ad3a..c241a3b7ce3 100644 --- a/pkg/scheduler/apis/config/validation/validation_test.go +++ b/pkg/scheduler/apis/config/validation/validation_test.go @@ -71,6 +71,12 @@ func TestValidateKubeSchedulerConfiguration(t *testing.T) { Disabled: []config.Plugin{{Name: "*"}}, }, }, + PluginConfig: []config.PluginConfig{ + { + Name: "DefaultPreemption", + Args: &config.DefaultPreemptionArgs{MinCandidateNodesPercentage: 10, MinCandidateNodesAbsolute: 100}, + }, + }, }, { SchedulerName: "other", @@ -135,6 +141,62 @@ func TestValidateKubeSchedulerConfiguration(t *testing.T) { extenderNegativeWeight := validConfig.DeepCopy() extenderNegativeWeight.Extenders[0].Weight = -1 + invalidNodePercentage := validConfig.DeepCopy() + invalidNodePercentage.Profiles[0].PluginConfig = []config.PluginConfig{ + { + Name: "DefaultPreemption", + Args: &config.DefaultPreemptionArgs{MinCandidateNodesPercentage: 200, MinCandidateNodesAbsolute: 100}, + }, + } + + invalidPluginArgs := validConfig.DeepCopy() + invalidPluginArgs.Profiles[0].PluginConfig = []config.PluginConfig{ + { + Name: "DefaultPreemption", + Args: &config.InterPodAffinityArgs{}, + }, + } + + duplicatedPluginConfig := validConfig.DeepCopy() + duplicatedPluginConfig.Profiles[0].PluginConfig = []config.PluginConfig{ + { + Name: "config", + }, + { + Name: "config", + }, + } + + mismatchQueueSort := validConfig.DeepCopy() + mismatchQueueSort.Profiles = []config.KubeSchedulerProfile{ + { + SchedulerName: "me", + Plugins: &config.Plugins{ + QueueSort: config.PluginSet{ + Enabled: []config.Plugin{{Name: "PrioritySort"}}, + }, + }, + PluginConfig: []config.PluginConfig{ + { + Name: "PrioritySort", + }, + }, + }, + { + SchedulerName: "other", + Plugins: &config.Plugins{ + QueueSort: config.PluginSet{ + Enabled: []config.Plugin{{Name: "CustomSort"}}, + }, + }, + PluginConfig: []config.PluginConfig{ + { + Name: "CustomSort", + }, + }, + }, + } + extenderDuplicateManagedResource := validConfig.DeepCopy() extenderDuplicateManagedResource.Extenders[0].ManagedResources = []config.ExtenderManagedResource{ {Name: "foo", IgnoredByScheduler: false}, @@ -216,15 +278,31 @@ func TestValidateKubeSchedulerConfiguration(t *testing.T) { expectedToFail: true, config: extenderDuplicateBind, }, + "invalid-node-percentage": { + expectedToFail: true, + config: invalidNodePercentage, + }, + "invalid-plugin-args": { + expectedToFail: true, + config: invalidPluginArgs, + }, + "duplicated-plugin-config": { + expectedToFail: true, + config: duplicatedPluginConfig, + }, + "mismatch-queue-sort": { + expectedToFail: true, + config: mismatchQueueSort, + }, } for name, scenario := range scenarios { t.Run(name, func(t *testing.T) { errs := ValidateKubeSchedulerConfiguration(scenario.config) - if len(errs) == 0 && scenario.expectedToFail { + if errs == nil && scenario.expectedToFail { t.Error("Unexpected success") } - if len(errs) > 0 && !scenario.expectedToFail { + if errs != nil && !scenario.expectedToFail { t.Errorf("Unexpected failure: %+v", errs) } }) @@ -306,7 +384,7 @@ func TestValidatePolicy(t *testing.T) { Extenders: []config.Extender{ {URLPrefix: "http://127.0.0.1:8081/extender", ManagedResources: []config.ExtenderManagedResource{{Name: "kubernetes.io/foo"}}}, }}, - expected: errors.New("extenders[0].managedResources[0].name: Invalid value: \"kubernetes.io/foo\": kubernetes.io/foo is an invalid extended resource name"), + expected: errors.New("extenders[0].managedResources[0].name: Invalid value: \"kubernetes.io/foo\": is an invalid extended resource name"), }, { name: "invalid redeclared RequestedToCapacityRatio custom priority", diff --git a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go index ffdd2c4d3e2..7884f289179 100644 --- a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go +++ b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption.go @@ -74,7 +74,7 @@ func New(dpArgs runtime.Object, fh framework.Handle) (framework.Plugin, error) { if !ok { return nil, fmt.Errorf("got args of type %T, want *DefaultPreemptionArgs", dpArgs) } - if err := validation.ValidateDefaultPreemptionArgs(*args); err != nil { + if err := validation.ValidateDefaultPreemptionArgs(nil, args); err != nil { return nil, err } pl := DefaultPreemption{ diff --git a/pkg/scheduler/framework/plugins/interpodaffinity/plugin.go b/pkg/scheduler/framework/plugins/interpodaffinity/plugin.go index 176187cf01e..dd8e23897f9 100644 --- a/pkg/scheduler/framework/plugins/interpodaffinity/plugin.go +++ b/pkg/scheduler/framework/plugins/interpodaffinity/plugin.go @@ -80,7 +80,7 @@ func New(plArgs runtime.Object, h framework.Handle, fts feature.Features) (frame if err != nil { return nil, err } - if err := validation.ValidateInterPodAffinityArgs(args); err != nil { + if err := validation.ValidateInterPodAffinityArgs(nil, &args); err != nil { return nil, err } pl := &InterPodAffinity{ diff --git a/pkg/scheduler/framework/plugins/nodeaffinity/node_affinity.go b/pkg/scheduler/framework/plugins/nodeaffinity/node_affinity.go index 4a4c416ca86..27fa711b9b6 100644 --- a/pkg/scheduler/framework/plugins/nodeaffinity/node_affinity.go +++ b/pkg/scheduler/framework/plugins/nodeaffinity/node_affinity.go @@ -225,7 +225,7 @@ func getArgs(obj runtime.Object) (config.NodeAffinityArgs, error) { if !ok { return config.NodeAffinityArgs{}, fmt.Errorf("args are not of type NodeAffinityArgs, got %T", obj) } - return *ptr, validation.ValidateNodeAffinityArgs(ptr) + return *ptr, validation.ValidateNodeAffinityArgs(nil, ptr) } func getPodPreferredNodeAffinity(pod *v1.Pod) (*nodeaffinity.PreferredSchedulingTerms, error) { diff --git a/pkg/scheduler/framework/plugins/nodelabel/node_label.go b/pkg/scheduler/framework/plugins/nodelabel/node_label.go index ecd648f1892..1de3598f759 100644 --- a/pkg/scheduler/framework/plugins/nodelabel/node_label.go +++ b/pkg/scheduler/framework/plugins/nodelabel/node_label.go @@ -42,8 +42,7 @@ func New(plArgs runtime.Object, handle framework.Handle) (framework.Plugin, erro if err != nil { return nil, err } - - if err := validation.ValidateNodeLabelArgs(args); err != nil { + if err := validation.ValidateNodeLabelArgs(nil, &args); err != nil { return nil, err } diff --git a/pkg/scheduler/framework/plugins/noderesources/fit.go b/pkg/scheduler/framework/plugins/noderesources/fit.go index e634067a7d7..6f2d0f95714 100644 --- a/pkg/scheduler/framework/plugins/noderesources/fit.go +++ b/pkg/scheduler/framework/plugins/noderesources/fit.go @@ -72,8 +72,7 @@ func NewFit(plArgs runtime.Object, _ framework.Handle) (framework.Plugin, error) if !ok { return nil, fmt.Errorf("want args to be of type NodeResourcesFitArgs, got %T", plArgs) } - - if err := validation.ValidateNodeResourcesFitArgs(args); err != nil { + if err := validation.ValidateNodeResourcesFitArgs(nil, args); err != nil { return nil, err } return &Fit{ diff --git a/pkg/scheduler/framework/plugins/noderesources/least_allocated.go b/pkg/scheduler/framework/plugins/noderesources/least_allocated.go index f196f30299a..5257a2ad20f 100644 --- a/pkg/scheduler/framework/plugins/noderesources/least_allocated.go +++ b/pkg/scheduler/framework/plugins/noderesources/least_allocated.go @@ -70,8 +70,7 @@ func NewLeastAllocated(laArgs runtime.Object, h framework.Handle) (framework.Plu if !ok { return nil, fmt.Errorf("want args to be of type NodeResourcesLeastAllocatedArgs, got %T", laArgs) } - - if err := validation.ValidateNodeResourcesLeastAllocatedArgs(args); err != nil { + if err := validation.ValidateNodeResourcesLeastAllocatedArgs(nil, args); err != nil { return nil, err } diff --git a/pkg/scheduler/framework/plugins/noderesources/most_allocated.go b/pkg/scheduler/framework/plugins/noderesources/most_allocated.go index 7a7929dfd92..010643c1145 100644 --- a/pkg/scheduler/framework/plugins/noderesources/most_allocated.go +++ b/pkg/scheduler/framework/plugins/noderesources/most_allocated.go @@ -68,8 +68,7 @@ func NewMostAllocated(maArgs runtime.Object, h framework.Handle) (framework.Plug if !ok { return nil, fmt.Errorf("want args to be of type NodeResourcesMostAllocatedArgs, got %T", args) } - - if err := validation.ValidateNodeResourcesMostAllocatedArgs(args); err != nil { + if err := validation.ValidateNodeResourcesMostAllocatedArgs(nil, args); err != nil { return nil, err } diff --git a/pkg/scheduler/framework/plugins/noderesources/requested_to_capacity_ratio.go b/pkg/scheduler/framework/plugins/noderesources/requested_to_capacity_ratio.go index b0bf7ab4b48..f9d7f9af65d 100644 --- a/pkg/scheduler/framework/plugins/noderesources/requested_to_capacity_ratio.go +++ b/pkg/scheduler/framework/plugins/noderesources/requested_to_capacity_ratio.go @@ -41,8 +41,7 @@ func NewRequestedToCapacityRatio(plArgs runtime.Object, handle framework.Handle) if err != nil { return nil, err } - - if err := validation.ValidateRequestedToCapacityRatioArgs(args); err != nil { + if err := validation.ValidateRequestedToCapacityRatioArgs(nil, &args); err != nil { return nil, err } diff --git a/pkg/scheduler/framework/plugins/podtopologyspread/plugin.go b/pkg/scheduler/framework/plugins/podtopologyspread/plugin.go index e58df1143b5..328137accc0 100644 --- a/pkg/scheduler/framework/plugins/podtopologyspread/plugin.go +++ b/pkg/scheduler/framework/plugins/podtopologyspread/plugin.go @@ -85,7 +85,7 @@ func New(plArgs runtime.Object, h framework.Handle) (framework.Plugin, error) { if err != nil { return nil, err } - if err := validation.ValidatePodTopologySpreadArgs(&args); err != nil { + if err := validation.ValidatePodTopologySpreadArgs(nil, &args); err != nil { return nil, err } pl := &PodTopologySpread{ diff --git a/pkg/scheduler/framework/plugins/volumebinding/volume_binding.go b/pkg/scheduler/framework/plugins/volumebinding/volume_binding.go index c20784b5161..72463c4d90d 100644 --- a/pkg/scheduler/framework/plugins/volumebinding/volume_binding.go +++ b/pkg/scheduler/framework/plugins/volumebinding/volume_binding.go @@ -341,7 +341,7 @@ func New(plArgs runtime.Object, fh framework.Handle) (framework.Plugin, error) { if !ok { return nil, fmt.Errorf("want args to be of type VolumeBindingArgs, got %T", plArgs) } - if err := validation.ValidateVolumeBindingArgs(args); err != nil { + if err := validation.ValidateVolumeBindingArgs(nil, args); err != nil { return nil, err } podInformer := fh.SharedInformerFactory().Core().V1().Pods() diff --git a/test/integration/scheduler_perf/scheduler_perf_test.go b/test/integration/scheduler_perf/scheduler_perf_test.go index 338f948294d..1eda1f4622b 100644 --- a/test/integration/scheduler_perf/scheduler_perf_test.go +++ b/test/integration/scheduler_perf/scheduler_perf_test.go @@ -408,7 +408,7 @@ func runWorkload(b *testing.B, tc *testCase, w *workload) []DataItem { if err != nil { b.Fatalf("error loading scheduler config file: %v", err) } - if err = validation.ValidateKubeSchedulerConfiguration(cfg).ToAggregate(); err != nil { + if err = validation.ValidateKubeSchedulerConfiguration(cfg); err != nil { b.Fatalf("validate scheduler config file failed: %v", err) } }