Merge pull request #83963 from damemi/duplicate-policies

Add Scheduler validation check for redeclared policy configs

pkg/scheduler/api/validation/validation.go

@@ -34,10 +34,18 @@ import (
 func ValidatePolicy(policy schedulerapi.Policy) error {
 	var validationErrors []error
 
+	priorities := make(map[string]schedulerapi.PriorityPolicy, len(policy.Priorities))
 	for _, priority := range policy.Priorities {
 		if priority.Weight <= 0 || priority.Weight >= framework.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]schedulerapi.PredicatePolicy, len(policy.Predicates))
+	for _, predicate := range policy.Predicates {
+		validationErrors = append(validationErrors, validatePredicateRedeclared(predicates, predicate))
 	}
 
 	binders := 0
@@ -66,6 +74,48 @@ func ValidatePolicy(policy schedulerapi.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]schedulerapi.PriorityPolicy, priority schedulerapi.PriorityPolicy) error {
+	var priorityType string
+	if priority.Argument != nil {
+		if priority.Argument.LabelPreference != nil {
+			priorityType = "LabelPreference"
+		} else if priority.Argument.RequestedToCapacityRatioArguments != nil {
+			priorityType = "RequestedToCapacityRatioArguments"
+		} else if priority.Argument.ServiceAntiAffinity != nil {
+			priorityType = "ServiceAntiAffinity"
+		} 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]schedulerapi.PredicatePolicy, predicate schedulerapi.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
+}
+
 // validateExtendedResourceName checks whether the specified name is a valid
 // extended resource name.
 func validateExtendedResourceName(name v1.ResourceName) []error {

pkg/scheduler/api/validation/validation_test.go

@@ -102,6 +102,26 @@ func TestValidatePolicy(t *testing.T) {
 				}},
 			expected: errors.New("kubernetes.io/foo is an invalid extended resource name"),
 		},
+		{
+			name: "invalid redeclared custom predicate",
+			policy: api.Policy{
+				Predicates: []api.PredicatePolicy{
+					{Name: "customPredicate1", Argument: &api.PredicateArgument{ServiceAffinity: &api.ServiceAffinity{Labels: []string{"label1"}}}},
+					{Name: "customPredicate2", Argument: &api.PredicateArgument{ServiceAffinity: &api.ServiceAffinity{Labels: []string{"label2"}}}},
+				},
+			},
+			expected: errors.New("Predicate 'customPredicate2' redeclares custom predicate 'ServiceAffinity', from:'customPredicate1'"),
+		},
+		{
+			name: "invalid redeclared custom priority",
+			policy: api.Policy{
+				Priorities: []api.PriorityPolicy{
+					{Name: "customPriority1", Weight: 1, Argument: &api.PriorityArgument{ServiceAntiAffinity: &api.ServiceAntiAffinity{Label: "label1"}}},
+					{Name: "customPriority2", Weight: 1, Argument: &api.PriorityArgument{ServiceAntiAffinity: &api.ServiceAntiAffinity{Label: "label2"}}},
+				},
+			},
+			expected: errors.New("Priority 'customPriority2' redeclares custom priority 'ServiceAntiAffinity', from:'customPriority1'"),
+		},
 	}
 
 	for _, test := range tests {