diff --git a/pkg/scheduler/apis/config/validation/validation.go b/pkg/scheduler/apis/config/validation/validation.go index 50bab297b7a..b832340f80b 100644 --- a/pkg/scheduler/apis/config/validation/validation.go +++ b/pkg/scheduler/apis/config/validation/validation.go @@ -89,13 +89,7 @@ func ValidatePolicy(policy config.Policy) error { if priority.Weight <= 0 || priority.Weight >= config.MaxWeight { validationErrors = append(validationErrors, fmt.Errorf("Priority %s should have a positive weight applied to it or it has overflown", priority.Name)) } - - validationErrors = append(validationErrors, validatePriorityRedeclared(priorities, priority)) - } - - predicates := make(map[string]config.PredicatePolicy, len(policy.Predicates)) - for _, predicate := range policy.Predicates { - validationErrors = append(validationErrors, validatePredicateRedeclared(predicates, predicate)) + validationErrors = append(validationErrors, validateCustomPriorities(priorities, priority)) } binders := 0 @@ -124,44 +118,43 @@ func ValidatePolicy(policy config.Policy) error { return utilerrors.NewAggregate(validationErrors) } -// validatePriorityRedeclared checks if any custom priorities have been declared multiple times in the policy config -// by examining the specified priority arguments -func validatePriorityRedeclared(priorities map[string]config.PriorityPolicy, priority config.PriorityPolicy) error { - var priorityType string +// validateCustomPriorities validates that: +// 1. RequestedToCapacityRatioRedeclared custom priority cannot be declared multiple times, +// 2. LabelPreference/ServiceAntiAffinity custom priorities can be declared multiple times, +// however the weights for each custom priority type should be the same. +func validateCustomPriorities(priorities map[string]config.PriorityPolicy, priority config.PriorityPolicy) error { + verifyRedeclaration := func(priorityType string) error { + if existing, alreadyDeclared := priorities[priorityType]; alreadyDeclared { + return fmt.Errorf("Priority %q redeclares custom priority %q, from:%q", priority.Name, priorityType, existing.Name) + } + priorities[priorityType] = priority + return nil + } + verifyDifferentWeights := func(priorityType string) error { + if existing, alreadyDeclared := priorities[priorityType]; alreadyDeclared { + if existing.Weight != priority.Weight { + return fmt.Errorf("%s priority %q has a different weight with %q", priorityType, priority.Name, existing.Name) + } + } + priorities[priorityType] = priority + return nil + } if priority.Argument != nil { if priority.Argument.LabelPreference != nil { - priorityType = "LabelPreference" - } else if priority.Argument.RequestedToCapacityRatioArguments != nil { - priorityType = "RequestedToCapacityRatioArguments" + if err := verifyDifferentWeights("LabelPreference"); err != nil { + return err + } } else if priority.Argument.ServiceAntiAffinity != nil { - priorityType = "ServiceAntiAffinity" + if err := verifyDifferentWeights("ServiceAntiAffinity"); err != nil { + return err + } + } else if priority.Argument.RequestedToCapacityRatioArguments != nil { + if err := verifyRedeclaration("RequestedToCapacityRatio"); err != nil { + return err + } } else { return fmt.Errorf("No priority arguments set for priority %s", priority.Name) } - if existing, alreadyDeclared := priorities[priorityType]; alreadyDeclared { - return fmt.Errorf("Priority '%s' redeclares custom priority '%s', from:'%s'", priority.Name, priorityType, existing.Name) - } - priorities[priorityType] = priority - } - return nil -} - -// validatePredicateRedeclared checks if any custom predicates have been declared multiple times in the policy config -// by examining the specified predicate arguments -func validatePredicateRedeclared(predicates map[string]config.PredicatePolicy, predicate config.PredicatePolicy) error { - var predicateType string - if predicate.Argument != nil { - if predicate.Argument.LabelsPresence != nil { - predicateType = "LabelsPresence" - } else if predicate.Argument.ServiceAffinity != nil { - predicateType = "ServiceAffinity" - } else { - return fmt.Errorf("No priority arguments set for priority %s", predicate.Name) - } - if existing, alreadyDeclared := predicates[predicateType]; alreadyDeclared { - return fmt.Errorf("Predicate '%s' redeclares custom predicate '%s', from:'%s'", predicate.Name, predicateType, existing.Name) - } - predicates[predicateType] = predicate } return nil } diff --git a/pkg/scheduler/apis/config/validation/validation_test.go b/pkg/scheduler/apis/config/validation/validation_test.go index 90226b335b4..1f6bec50fb6 100644 --- a/pkg/scheduler/apis/config/validation/validation_test.go +++ b/pkg/scheduler/apis/config/validation/validation_test.go @@ -240,24 +240,34 @@ func TestValidatePolicy(t *testing.T) { expected: errors.New("kubernetes.io/foo is an invalid extended resource name"), }, { - name: "invalid redeclared custom predicate", - policy: config.Policy{ - Predicates: []config.PredicatePolicy{ - {Name: "customPredicate1", Argument: &config.PredicateArgument{ServiceAffinity: &config.ServiceAffinity{Labels: []string{"label1"}}}}, - {Name: "customPredicate2", Argument: &config.PredicateArgument{ServiceAffinity: &config.ServiceAffinity{Labels: []string{"label2"}}}}, - }, - }, - expected: errors.New("Predicate 'customPredicate2' redeclares custom predicate 'ServiceAffinity', from:'customPredicate1'"), - }, - { - name: "invalid redeclared custom priority", + name: "invalid redeclared RequestedToCapacityRatio custom priority", policy: config.Policy{ Priorities: []config.PriorityPolicy{ - {Name: "customPriority1", Weight: 1, Argument: &config.PriorityArgument{ServiceAntiAffinity: &config.ServiceAntiAffinity{Label: "label1"}}}, - {Name: "customPriority2", Weight: 1, Argument: &config.PriorityArgument{ServiceAntiAffinity: &config.ServiceAntiAffinity{Label: "label2"}}}, + {Name: "customPriority1", Weight: 1, Argument: &config.PriorityArgument{RequestedToCapacityRatioArguments: &config.RequestedToCapacityRatioArguments{}}}, + {Name: "customPriority2", Weight: 1, Argument: &config.PriorityArgument{RequestedToCapacityRatioArguments: &config.RequestedToCapacityRatioArguments{}}}, }, }, - expected: errors.New("Priority 'customPriority2' redeclares custom priority 'ServiceAntiAffinity', from:'customPriority1'"), + expected: errors.New("Priority \"customPriority2\" redeclares custom priority \"RequestedToCapacityRatio\", from:\"customPriority1\""), + }, + { + name: "different weights for LabelPreference custom priority", + policy: config.Policy{ + Priorities: []config.PriorityPolicy{ + {Name: "customPriority1", Weight: 1, Argument: &config.PriorityArgument{LabelPreference: &config.LabelPreference{}}}, + {Name: "customPriority2", Weight: 2, Argument: &config.PriorityArgument{LabelPreference: &config.LabelPreference{}}}, + }, + }, + expected: errors.New("LabelPreference priority \"customPriority2\" has a different weight with \"customPriority1\""), + }, + { + name: "different weights for ServiceAntiAffinity custom priority", + policy: config.Policy{ + Priorities: []config.PriorityPolicy{ + {Name: "customPriority1", Weight: 1, Argument: &config.PriorityArgument{ServiceAntiAffinity: &config.ServiceAntiAffinity{}}}, + {Name: "customPriority2", Weight: 2, Argument: &config.PriorityArgument{ServiceAntiAffinity: &config.ServiceAntiAffinity{}}}, + }, + }, + expected: errors.New("ServiceAntiAffinity priority \"customPriority2\" has a different weight with \"customPriority1\""), }, }