diff --git a/pkg/scheduler/apis/config/validation/validation_pluginargs.go b/pkg/scheduler/apis/config/validation/validation_pluginargs.go index 19236aafe5b..860c1abe535 100644 --- a/pkg/scheduler/apis/config/validation/validation_pluginargs.go +++ b/pkg/scheduler/apis/config/validation/validation_pluginargs.go @@ -133,3 +133,82 @@ func validateConstraintNotRepeat(path *field.Path, constraints []v1.TopologySpre } return nil } + +// ValidateRequestedToCapacityRatioArgs validates that RequestedToCapacityRatioArgs are correct. +func ValidateRequestedToCapacityRatioArgs(args config.RequestedToCapacityRatioArgs) error { + if err := validateFunctionShape(args.Shape); err != nil { + return err + } + if err := validateResourcesNoMax(args.Resources); err != nil { + return err + } + return nil +} + +func validateFunctionShape(shape []config.UtilizationShapePoint) error { + const ( + minUtilization = 0 + maxUtilization = 100 + minScore = 0 + maxScore = int32(config.MaxCustomPriorityScore) + ) + + if len(shape) == 0 { + return fmt.Errorf("at least one point must be specified") + } + + for i := 1; i < len(shape); i++ { + if shape[i-1].Utilization >= shape[i].Utilization { + return fmt.Errorf("utilization values must be sorted. Utilization[%d]==%d >= Utilization[%d]==%d", i-1, shape[i-1].Utilization, i, shape[i].Utilization) + } + } + + for i, point := range shape { + if point.Utilization < minUtilization { + return fmt.Errorf("utilization values must not be less than %d. Utilization[%d]==%d", minUtilization, i, point.Utilization) + } + if point.Utilization > maxUtilization { + return fmt.Errorf("utilization values must not be greater than %d. Utilization[%d]==%d", maxUtilization, i, point.Utilization) + } + if point.Score < minScore { + return fmt.Errorf("score values must not be less than %d. Score[%d]==%d", minScore, i, point.Score) + } + if point.Score > maxScore { + return fmt.Errorf("score values must not be greater than %d. Score[%d]==%d", maxScore, i, point.Score) + } + } + + return nil +} + +// TODO potentially replace with validateResources +func validateResourcesNoMax(resources []config.ResourceSpec) error { + for _, r := range resources { + if r.Weight < 1 { + return fmt.Errorf("resource %s weight %d must not be less than 1", string(r.Name), r.Weight) + } + } + return nil +} + +// ValidateNodeResourcesLeastAllocatedArgs validates that NodeResourcesLeastAllocatedArgs are correct. +func ValidateNodeResourcesLeastAllocatedArgs(args *config.NodeResourcesLeastAllocatedArgs) error { + return validateResources(args.Resources) +} + +// ValidateNodeResourcesMostAllocatedArgs validates that NodeResourcesMostAllocatedArgs are correct. +func ValidateNodeResourcesMostAllocatedArgs(args *config.NodeResourcesMostAllocatedArgs) error { + return validateResources(args.Resources) +} + +func validateResources(resources []config.ResourceSpec) error { + for _, resource := range resources { + if resource.Weight <= 0 { + return fmt.Errorf("resource Weight of %v should be a positive value, got %v", resource.Name, resource.Weight) + } + if resource.Weight > 100 { + return fmt.Errorf("resource Weight of %v should be less than 100, got %v", resource.Name, resource.Weight) + } + } + return nil +} diff --git a/pkg/scheduler/apis/config/validation/validation_pluginargs_test.go b/pkg/scheduler/apis/config/validation/validation_pluginargs_test.go index e1ceaa7745a..652ccf0d131 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 TestValidatePodTopologySpreadArgs(t *testing.T) { }, }, }, - "max skew less than zero": { + "maxSkew less than zero": { args: &config.PodTopologySpreadArgs{ DefaultConstraints: []v1.TopologySpreadConstraint{ { @@ -138,7 +138,7 @@ func TestValidatePodTopologySpreadArgs(t *testing.T) { }, wantErr: `defaultConstraints[0].topologyKey: Required value: can not be empty`, }, - "WhenUnsatisfiable is empty": { + "whenUnsatisfiable is empty": { args: &config.PodTopologySpreadArgs{ DefaultConstraints: []v1.TopologySpreadConstraint{ { @@ -150,7 +150,7 @@ func TestValidatePodTopologySpreadArgs(t *testing.T) { }, wantErr: `defaultConstraints[0].whenUnsatisfiable: Required value: can not be empty`, }, - "WhenUnsatisfiable contains unsupported action": { + "whenUnsatisfiable contains unsupported action": { args: &config.PodTopologySpreadArgs{ DefaultConstraints: []v1.TopologySpreadConstraint{ { @@ -206,17 +206,285 @@ func TestValidatePodTopologySpreadArgs(t *testing.T) { } } +func TestValidateRequestedToCapacityRatioArgs(t *testing.T) { + cases := map[string]struct { + args config.RequestedToCapacityRatioArgs + wantErr string + }{ + "valid config": { + args: config.RequestedToCapacityRatioArgs{ + Shape: []config.UtilizationShapePoint{ + { + Utilization: 20, + Score: 5, + }, + { + Utilization: 30, + Score: 3, + }, + { + Utilization: 50, + Score: 2, + }, + }, + Resources: []config.ResourceSpec{ + { + Name: "custom-resource", + Weight: 5, + }, + }, + }, + }, + "no shape points": { + args: config.RequestedToCapacityRatioArgs{ + Shape: []config.UtilizationShapePoint{}, + Resources: []config.ResourceSpec{ + { + Name: "custom", + Weight: 5, + }, + }, + }, + wantErr: `at least one point must be specified`, + }, + "utilization less than min": { + args: config.RequestedToCapacityRatioArgs{ + Shape: []config.UtilizationShapePoint{ + { + Utilization: -10, + Score: 3, + }, + { + Utilization: 10, + Score: 2, + }, + }, + }, + wantErr: `utilization values must not be less than 0. Utilization[0]==-10`, + }, + "utilization greater than max": { + args: config.RequestedToCapacityRatioArgs{ + Shape: []config.UtilizationShapePoint{ + { + Utilization: 10, + Score: 3, + }, + { + Utilization: 110, + Score: 2, + }, + }, + }, + wantErr: `utilization values must not be greater than 100. Utilization[1]==110`, + }, + "Utilization values in non-increasing order": { + args: config.RequestedToCapacityRatioArgs{ + Shape: []config.UtilizationShapePoint{ + { + Utilization: 30, + Score: 3, + }, + { + Utilization: 20, + Score: 2, + }, + { + Utilization: 10, + Score: 1, + }, + }, + }, + wantErr: `utilization values must be sorted. Utilization[0]==30 >= Utilization[1]==20`, + }, + "duplicated utilization values": { + args: config.RequestedToCapacityRatioArgs{ + Shape: []config.UtilizationShapePoint{ + { + Utilization: 10, + Score: 3, + }, + { + Utilization: 20, + Score: 2, + }, + { + Utilization: 20, + Score: 1, + }, + }, + }, + wantErr: `utilization values must be sorted. Utilization[1]==20 >= Utilization[2]==20`, + }, + "score less than min": { + args: config.RequestedToCapacityRatioArgs{ + Shape: []config.UtilizationShapePoint{ + { + Utilization: 10, + Score: -1, + }, + { + Utilization: 20, + Score: 2, + }, + }, + }, + wantErr: `score values must not be less than 0. Score[0]==-1`, + }, + "score greater than max": { + args: config.RequestedToCapacityRatioArgs{ + Shape: []config.UtilizationShapePoint{ + { + Utilization: 10, + Score: 3, + }, + { + Utilization: 20, + Score: 11, + }, + }, + }, + wantErr: `score values must not be greater than 10. Score[1]==11`, + }, + "resources weight less than 1": { + args: config.RequestedToCapacityRatioArgs{ + Shape: []config.UtilizationShapePoint{ + { + Utilization: 10, + Score: 1, + }, + }, + Resources: []config.ResourceSpec{ + { + Name: "custom", + Weight: 0, + }, + }, + }, + wantErr: `resource custom weight 0 must not be less than 1`, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + err := ValidateRequestedToCapacityRatioArgs(tc.args) + assertErr(t, tc.wantErr, err) + }) + } +} + +func TestValidateNodeResourcesLeastAllocatedArgs(t *testing.T) { + cases := map[string]struct { + args *config.NodeResourcesLeastAllocatedArgs + wantErr string + }{ + "valid config": { + args: &config.NodeResourcesLeastAllocatedArgs{ + Resources: []config.ResourceSpec{ + { + Name: "cpu", + Weight: 50, + }, + { + Name: "memory", + Weight: 30, + }, + }, + }, + }, + "weight less than min": { + args: &config.NodeResourcesLeastAllocatedArgs{ + Resources: []config.ResourceSpec{ + { + Name: "cpu", + Weight: 0, + }, + }, + }, + wantErr: `resource Weight of cpu should be a positive value, got 0`, + }, + "weight more than max": { + args: &config.NodeResourcesLeastAllocatedArgs{ + Resources: []config.ResourceSpec{ + { + Name: "memory", + Weight: 101, + }, + }, + }, + wantErr: `resource Weight of memory should be less than 100, got 101`, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + err := ValidateNodeResourcesLeastAllocatedArgs(tc.args) + assertErr(t, tc.wantErr, err) + }) + } +} + +func TestValidateNodeResourcesMostAllocatedArgs(t *testing.T) { + cases := map[string]struct { + args *config.NodeResourcesMostAllocatedArgs + wantErr string + }{ + "valid config": { + args: &config.NodeResourcesMostAllocatedArgs{ + Resources: []config.ResourceSpec{ + { + Name: "cpu", + Weight: 70, + }, + { + Name: "memory", + Weight: 40, + }, + }, + }, + }, + "weight less than min": { + args: &config.NodeResourcesMostAllocatedArgs{ + Resources: []config.ResourceSpec{ + { + Name: "cpu", + Weight: -1, + }, + }, + }, + wantErr: `resource Weight of cpu should be a positive value, got -1`, + }, + "weight more than max": { + args: &config.NodeResourcesMostAllocatedArgs{ + Resources: []config.ResourceSpec{ + { + Name: "memory", + Weight: 110, + }, + }, + }, + wantErr: `resource Weight of memory should be less than 100, got 110`, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + err := ValidateNodeResourcesMostAllocatedArgs(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()) + t.Fatalf("\nwant err to be:\n\tnil\ngot:\n\t%s", gotErr.Error()) } } else { if gotErr == nil { - t.Fatalf("wanted err to be: '%s', got: nil", wantErr) + t.Fatalf("\nwant err to be:\n\t%s\ngot:\n\tnil", wantErr) } if gotErr.Error() != wantErr { - t.Errorf("wanted err to be: '%s', got '%s'", wantErr, gotErr.Error()) + t.Errorf("\nwant err to be:\n\t%s\ngot:\n\t%s", wantErr, gotErr.Error()) } } } diff --git a/pkg/scheduler/framework/plugins/noderesources/BUILD b/pkg/scheduler/framework/plugins/noderesources/BUILD index d15f5e1a598..f2cf84c75f1 100644 --- a/pkg/scheduler/framework/plugins/noderesources/BUILD +++ b/pkg/scheduler/framework/plugins/noderesources/BUILD @@ -18,6 +18,7 @@ go_library( "//pkg/apis/core/v1/helper:go_default_library", "//pkg/features:go_default_library", "//pkg/scheduler/apis/config:go_default_library", + "//pkg/scheduler/apis/config/validation:go_default_library", "//pkg/scheduler/framework/v1alpha1:go_default_library", "//pkg/scheduler/util:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", diff --git a/pkg/scheduler/framework/plugins/noderesources/least_allocated.go b/pkg/scheduler/framework/plugins/noderesources/least_allocated.go index 6829daf8fb4..e8d186e0eb9 100644 --- a/pkg/scheduler/framework/plugins/noderesources/least_allocated.go +++ b/pkg/scheduler/framework/plugins/noderesources/least_allocated.go @@ -23,6 +23,7 @@ import ( v1 "k8s.io/api/core/v1" "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" ) @@ -70,14 +71,12 @@ func NewLeastAllocated(laArgs runtime.Object, h framework.FrameworkHandle) (fram return nil, fmt.Errorf("want args to be of type NodeResourcesLeastAllocatedArgs, got %T", laArgs) } + if err := validation.ValidateNodeResourcesLeastAllocatedArgs(args); err != nil { + return nil, err + } + resToWeightMap := make(resourceToWeightMap) for _, resource := range (*args).Resources { - if resource.Weight <= 0 { - return nil, fmt.Errorf("resource Weight of %v should be a positive value, got %v", resource.Name, resource.Weight) - } - if resource.Weight > framework.MaxNodeScore { - return nil, fmt.Errorf("resource Weight of %v should be less than 100, got %v", resource.Name, resource.Weight) - } resToWeightMap[v1.ResourceName(resource.Name)] = resource.Weight } diff --git a/pkg/scheduler/framework/plugins/noderesources/most_allocated.go b/pkg/scheduler/framework/plugins/noderesources/most_allocated.go index 9dc2b860db1..67aa2a6394f 100644 --- a/pkg/scheduler/framework/plugins/noderesources/most_allocated.go +++ b/pkg/scheduler/framework/plugins/noderesources/most_allocated.go @@ -23,6 +23,7 @@ import ( v1 "k8s.io/api/core/v1" "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" ) @@ -68,15 +69,12 @@ func NewMostAllocated(maArgs runtime.Object, h framework.FrameworkHandle) (frame return nil, fmt.Errorf("want args to be of type NodeResourcesMostAllocatedArgs, got %T", args) } - resToWeightMap := make(resourceToWeightMap) + if err := validation.ValidateNodeResourcesMostAllocatedArgs(args); err != nil { + return nil, err + } + resToWeightMap := make(resourceToWeightMap) for _, resource := range (*args).Resources { - if resource.Weight <= 0 { - return nil, fmt.Errorf("resource Weight of %v should be a positive value, got %v", resource.Name, resource.Weight) - } - if resource.Weight > framework.MaxNodeScore { - return nil, fmt.Errorf("resource Weight of %v should be less than 100, got %v", resource.Name, resource.Weight) - } resToWeightMap[v1.ResourceName(resource.Name)] = resource.Weight } 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 60b3120e9c9..436cf09cf56 100644 --- a/pkg/scheduler/framework/plugins/noderesources/requested_to_capacity_ratio.go +++ b/pkg/scheduler/framework/plugins/noderesources/requested_to_capacity_ratio.go @@ -23,8 +23,8 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/klog/v2" "k8s.io/kubernetes/pkg/scheduler/apis/config" + "k8s.io/kubernetes/pkg/scheduler/apis/config/validation" framework "k8s.io/kubernetes/pkg/scheduler/framework/v1alpha1" ) @@ -33,8 +33,6 @@ const ( RequestedToCapacityRatioName = "RequestedToCapacityRatio" minUtilization = 0 maxUtilization = 100 - minScore = 0 - maxScore = framework.MaxNodeScore ) type functionShape []functionShapePoint @@ -53,6 +51,10 @@ func NewRequestedToCapacityRatio(plArgs runtime.Object, handle framework.Framewo return nil, err } + if err := validation.ValidateRequestedToCapacityRatioArgs(args); err != nil { + return nil, err + } + shape := make([]functionShapePoint, 0, len(args.Shape)) for _, point := range args.Shape { shape = append(shape, functionShapePoint{ @@ -64,10 +66,6 @@ func NewRequestedToCapacityRatio(plArgs runtime.Object, handle framework.Framewo }) } - if err := validateFunctionShape(shape); err != nil { - return nil, err - } - resourceToWeightMap := make(resourceToWeightMap) for _, resource := range args.Resources { resourceToWeightMap[v1.ResourceName(resource.Name)] = resource.Weight @@ -123,54 +121,8 @@ func (pl *RequestedToCapacityRatio) ScoreExtensions() framework.ScoreExtensions return nil } -func validateFunctionShape(shape functionShape) error { - if len(shape) == 0 { - return fmt.Errorf("at least one point must be specified") - } - - for i := 1; i < len(shape); i++ { - if shape[i-1].utilization >= shape[i].utilization { - return fmt.Errorf("utilization values must be sorted. Utilization[%d]==%d >= Utilization[%d]==%d", i-1, shape[i-1].utilization, i, shape[i].utilization) - } - } - - for i, point := range shape { - if point.utilization < minUtilization { - return fmt.Errorf("utilization values must not be less than %d. Utilization[%d]==%d", minUtilization, i, point.utilization) - } - if point.utilization > maxUtilization { - return fmt.Errorf("utilization values must not be greater than %d. Utilization[%d]==%d", maxUtilization, i, point.utilization) - } - if point.score < minScore { - return fmt.Errorf("score values must not be less than %d. Score[%d]==%d", minScore, i, point.score) - } - if int64(point.score) > maxScore { - return fmt.Errorf("score values not be greater than %d. Score[%d]==%d", maxScore, i, point.score) - } - } - - return nil -} - -func validateResourceWeightMap(resourceToWeightMap resourceToWeightMap) error { - if len(resourceToWeightMap) == 0 { - return fmt.Errorf("resourceToWeightMap cannot be nil") - } - - for resource, weight := range resourceToWeightMap { - if weight < 1 { - return fmt.Errorf("resource %s weight %d must not be less than 1", string(resource), weight) - } - } - return nil -} - func buildRequestedToCapacityRatioScorerFunction(scoringFunctionShape functionShape, resourceToWeightMap resourceToWeightMap) func(resourceToValueMap, resourceToValueMap, bool, int, int) int64 { rawScoringFunction := buildBrokenLinearFunction(scoringFunctionShape) - err := validateResourceWeightMap(resourceToWeightMap) - if err != nil { - klog.Error(err) - } resourceScoringFunction := func(requested, capacity int64) int64 { if capacity == 0 || requested > capacity { return rawScoringFunction(maxUtilization) diff --git a/pkg/scheduler/framework/plugins/noderesources/requested_to_capacity_ratio_test.go b/pkg/scheduler/framework/plugins/noderesources/requested_to_capacity_ratio_test.go index d8288f69754..4afac0bfca0 100644 --- a/pkg/scheduler/framework/plugins/noderesources/requested_to_capacity_ratio_test.go +++ b/pkg/scheduler/framework/plugins/noderesources/requested_to_capacity_ratio_test.go @@ -116,47 +116,6 @@ func makePod(node string, milliCPU, memory int64) *v1.Pod { } } -func TestCreatingFunctionShapeErrorsIfEmptyPoints(t *testing.T) { - var err error - err = validateFunctionShape([]functionShapePoint{}) - assert.Equal(t, "at least one point must be specified", err.Error()) -} - -func TestCreatingResourceNegativeWeight(t *testing.T) { - err := validateResourceWeightMap(resourceToWeightMap{v1.ResourceCPU: -1}) - assert.Equal(t, "resource cpu weight -1 must not be less than 1", err.Error()) -} - -func TestCreatingResourceDefaultWeight(t *testing.T) { - err := validateResourceWeightMap(resourceToWeightMap{}) - assert.Equal(t, "resourceToWeightMap cannot be nil", err.Error()) - -} - -func TestCreatingFunctionShapeErrorsIfXIsNotSorted(t *testing.T) { - var err error - err = validateFunctionShape([]functionShapePoint{{10, 1}, {15, 2}, {20, 3}, {19, 4}, {25, 5}}) - assert.Equal(t, "utilization values must be sorted. Utilization[2]==20 >= Utilization[3]==19", err.Error()) - - err = validateFunctionShape([]functionShapePoint{{10, 1}, {20, 2}, {20, 3}, {22, 4}, {25, 5}}) - assert.Equal(t, "utilization values must be sorted. Utilization[1]==20 >= Utilization[2]==20", err.Error()) -} - -func TestCreatingFunctionPointNotInAllowedRange(t *testing.T) { - var err error - err = validateFunctionShape([]functionShapePoint{{-1, 0}, {100, 100}}) - assert.Equal(t, "utilization values must not be less than 0. Utilization[0]==-1", err.Error()) - - err = validateFunctionShape([]functionShapePoint{{0, 0}, {101, 100}}) - assert.Equal(t, "utilization values must not be greater than 100. Utilization[1]==101", err.Error()) - - err = validateFunctionShape([]functionShapePoint{{0, -1}, {100, 100}}) - assert.Equal(t, "score values must not be less than 0. Score[0]==-1", err.Error()) - - err = validateFunctionShape([]functionShapePoint{{0, 0}, {100, 101}}) - assert.Equal(t, "score values not be greater than 100. Score[1]==101", err.Error()) -} - func TestBrokenLinearFunction(t *testing.T) { type Assertion struct { p int64