Validate custom priority policy config.

- Do not validate redeclartion of custom predicates.
- Validate no duplicate declaration of RequestedToCapacityRatio
- Validate the weights across multiple declarations of LabelPreference/ServiceAntiAffinity are the same.

Author: liu-cong

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
-	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)
+// 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 {
-			return fmt.Errorf("Priority '%s' redeclares custom priority '%s', from:'%s'", priority.Name, priorityType, existing.Name)
+			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
 	}
-	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"
+	if priority.Argument != nil {
+		if priority.Argument.LabelPreference != nil {
+			if err := verifyDifferentWeights("LabelPreference"); err != nil {
+				return err
+			}
+		} else if priority.Argument.ServiceAntiAffinity != nil {
+			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", predicate.Name)
-		}
-		if existing, alreadyDeclared := predicates[predicateType]; alreadyDeclared {
-			return fmt.Errorf("Predicate '%s' redeclares custom predicate '%s', from:'%s'", predicate.Name, predicateType, existing.Name)
+			return fmt.Errorf("No priority arguments set for priority %s", priority.Name)
 		}
-		predicates[predicateType] = predicate
 	}
 	return nil
 }

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",
+			name: "invalid redeclared RequestedToCapacityRatio custom priority",
 			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"}}}},
+				Priorities: []config.PriorityPolicy{
+					{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 \"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("Predicate 'customPredicate2' redeclares custom predicate 'ServiceAffinity', from:'customPredicate1'"),
+			expected: errors.New("LabelPreference  priority \"customPriority2\" has a different weight with \"customPriority1\""),
 		},
 		{
-			name: "invalid redeclared custom priority",
+			name: "different weights for ServiceAntiAffinity 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{ServiceAntiAffinity: &config.ServiceAntiAffinity{}}},
+					{Name: "customPriority2", Weight: 2, Argument: &config.PriorityArgument{ServiceAntiAffinity: &config.ServiceAntiAffinity{}}},
 				},
 			},
-			expected: errors.New("Priority 'customPriority2' redeclares custom priority 'ServiceAntiAffinity', from:'customPriority1'"),
+			expected: errors.New("ServiceAntiAffinity  priority \"customPriority2\" has a different weight with \"customPriority1\""),
 		},
 	}