Merge pull request #96745 from lingsamuel/scheduler-validation

Use field.Error(s) in scheduler plugin args validation

pkg/scheduler/apis/config/validation/validation_pluginargs.go

@@ -29,39 +29,46 @@ import (
 
 // ValidateDefaultPreemptionArgs validates that DefaultPreemptionArgs are correct.
 func ValidateDefaultPreemptionArgs(args config.DefaultPreemptionArgs) error {
-	if err := validateMinCandidateNodesPercentage(args.MinCandidateNodesPercentage); err != nil {
+	var path *field.Path
-		return err
+	var allErrs field.ErrorList
+	percentagePath := path.Child("minCandidateNodesPercentage")
+	absolutePath := path.Child("minCandidateNodesAbsolute")
+	if err := validateMinCandidateNodesPercentage(args.MinCandidateNodesPercentage, percentagePath); err != nil {
+		allErrs = append(allErrs, err)
 	}
-	if err := validateMinCandidateNodesAbsolute(args.MinCandidateNodesAbsolute); err != nil {
+	if err := validateMinCandidateNodesAbsolute(args.MinCandidateNodesAbsolute, absolutePath); err != nil {
-		return err
+		allErrs = append(allErrs, err)
 	}
 	if args.MinCandidateNodesPercentage == 0 && args.MinCandidateNodesAbsolute == 0 {
-		return fmt.Errorf("both minCandidateNodesPercentage and minCandidateNodesAbsolute cannot be zero")
+		allErrs = append(allErrs,
+			field.Invalid(percentagePath, args.MinCandidateNodesPercentage, "cannot be zero at the same time as minCandidateNodesAbsolute"),
+			field.Invalid(absolutePath, args.MinCandidateNodesAbsolute, "cannot be zero at the same time as minCandidateNodesPercentage"))
 	}
-	return nil
+	return allErrs.ToAggregate()
 }
 
 // validateMinCandidateNodesPercentage validates that
 // minCandidateNodesPercentage is within the allowed range.
-func validateMinCandidateNodesPercentage(minCandidateNodesPercentage int32) error {
+func validateMinCandidateNodesPercentage(minCandidateNodesPercentage int32, p *field.Path) *field.Error {
 	if minCandidateNodesPercentage < 0 || minCandidateNodesPercentage > 100 {
-		return fmt.Errorf("minCandidateNodesPercentage is not in the range [0, 100]")
+		return field.Invalid(p, minCandidateNodesPercentage, "not in valid range [0, 100]")
 	}
 	return nil
 }
 
 // validateMinCandidateNodesAbsolute validates that minCandidateNodesAbsolute
 // is within the allowed range.
-func validateMinCandidateNodesAbsolute(minCandidateNodesAbsolute int32) error {
+func validateMinCandidateNodesAbsolute(minCandidateNodesAbsolute int32, p *field.Path) *field.Error {
 	if minCandidateNodesAbsolute < 0 {
-		return fmt.Errorf("minCandidateNodesAbsolute is not in the range [0, inf)")
+		return field.Invalid(p, minCandidateNodesAbsolute, "not in valid range [0, inf)")
 	}
 	return nil
 }
 
 // ValidateInterPodAffinityArgs validates that InterPodAffinityArgs are correct.
 func ValidateInterPodAffinityArgs(args config.InterPodAffinityArgs) error {
-	return ValidateHardPodAffinityWeight(field.NewPath("hardPodAffinityWeight"), args.HardPodAffinityWeight)
+	var path *field.Path
+	return ValidateHardPodAffinityWeight(path.Child("hardPodAffinityWeight"), args.HardPodAffinityWeight)
 }
 
 // ValidateHardPodAffinityWeight validates that weight is within allowed range.
@@ -72,7 +79,7 @@ func ValidateHardPodAffinityWeight(path *field.Path, w int32) error {
 	)
 
 	if w < minHardPodAffinityWeight || w > maxHardPodAffinityWeight {
-		msg := fmt.Sprintf("not in valid range [%d-%d]", minHardPodAffinityWeight, maxHardPodAffinityWeight)
+		msg := fmt.Sprintf("not in valid range [%d, %d]", minHardPodAffinityWeight, maxHardPodAffinityWeight)
 		return field.Invalid(path, w, msg)
 	}
 	return nil
@@ -80,44 +87,50 @@ func ValidateHardPodAffinityWeight(path *field.Path, w int32) error {
 
 // ValidateNodeLabelArgs validates that NodeLabelArgs are correct.
 func ValidateNodeLabelArgs(args config.NodeLabelArgs) error {
-	if err := validateNoConflict(args.PresentLabels, args.AbsentLabels); err != nil {
+	var path *field.Path
-		return err
+	var allErrs field.ErrorList
-	}
+
-	if err := validateNoConflict(args.PresentLabelsPreference, args.AbsentLabelsPreference); err != nil {
+	allErrs = append(allErrs, validateNoConflict(args.PresentLabels, args.AbsentLabels,
-		return err
+		path.Child("presentLabels"), path.Child("absentLabels"))...)
-	}
+	allErrs = append(allErrs, validateNoConflict(args.PresentLabelsPreference, args.AbsentLabelsPreference,
-	return nil
+		path.Child("presentLabelsPreference"), path.Child("absentLabelsPreference"))...)
+
+	return allErrs.ToAggregate()
 }
 
 // validateNoConflict validates that presentLabels and absentLabels do not conflict.
-func validateNoConflict(presentLabels []string, absentLabels []string) error {
+func validateNoConflict(presentLabels, absentLabels []string, presentPath, absentPath *field.Path) field.ErrorList {
-	m := make(map[string]struct{}, len(presentLabels))
+	var allErrs field.ErrorList
-	for _, l := range presentLabels {
+
-		m[l] = struct{}{}
+	m := make(map[string]int, len(presentLabels)) // label -> index
+	for i, l := range presentLabels {
+		m[l] = i
 	}
-	for _, l := range absentLabels {
+	for i, l := range absentLabels {
-		if _, ok := m[l]; ok {
+		if j, ok := m[l]; ok {
-			return fmt.Errorf("detecting at least one label (e.g., %q) that exist in both the present(%+v) and absent(%+v) label list", l, presentLabels, absentLabels)
+			allErrs = append(allErrs, field.Invalid(presentPath.Index(j), l,
+				fmt.Sprintf("conflict with %v", absentPath.Index(i).String())))
 		}
 	}
-	return nil
+	return allErrs
 }
 
 // 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
 	var allErrs field.ErrorList
-	if err := validateDefaultingType(field.NewPath("defaultingType"), args.DefaultingType, args.DefaultConstraints); err != nil {
+	if err := validateDefaultingType(path.Child("defaultingType"), args.DefaultingType, args.DefaultConstraints); err != nil {
 		allErrs = append(allErrs, err)
 	}
-	path := field.NewPath("defaultConstraints")
 
+	defaultConstraintsPath := path.Child("defaultConstraints")
 	for i, c := range args.DefaultConstraints {
-		p := path.Index(i)
+		p := defaultConstraintsPath.Index(i)
 		if c.MaxSkew <= 0 {
 			f := p.Child("maxSkew")
-			allErrs = append(allErrs, field.Invalid(f, c.MaxSkew, "must be greater than zero"))
+			allErrs = append(allErrs, field.Invalid(f, c.MaxSkew, "not in valid range (0, inf)"))
 		}
 		allErrs = append(allErrs, validateTopologyKey(p.Child("topologyKey"), c.TopologyKey)...)
 		if err := validateWhenUnsatisfiable(p.Child("whenUnsatisfiable"), c.WhenUnsatisfiable); err != nil {
@@ -127,7 +140,7 @@ func ValidatePodTopologySpreadArgs(args *config.PodTopologySpreadArgs) error {
 			f := field.Forbidden(p.Child("labelSelector"), "constraint must not define a selector, as they deduced for each pod")
 			allErrs = append(allErrs, f)
 		}
-		if err := validateConstraintNotRepeat(path, args.DefaultConstraints, i); err != nil {
+		if err := validateConstraintNotRepeat(defaultConstraintsPath, args.DefaultConstraints, i); err != nil {
 			allErrs = append(allErrs, err)
 		}
 	}
@@ -139,7 +152,7 @@ func ValidatePodTopologySpreadArgs(args *config.PodTopologySpreadArgs) error {
 
 func validateDefaultingType(p *field.Path, v config.PodTopologySpreadConstraintsDefaulting, constraints []v1.TopologySpreadConstraint) *field.Error {
 	if v != config.SystemDefaulting && v != config.ListDefaulting {
-		return field.Invalid(p, v, fmt.Sprintf("must be one of {%q, %q}", config.SystemDefaulting, config.ListDefaulting))
+		return field.NotSupported(p, v, []string{string(config.SystemDefaulting), string(config.ListDefaulting)})
 	}
 	if v == config.SystemDefaulting && len(constraints) > 0 {
 		return field.Invalid(p, v, "when .defaultConstraints are not empty")
@@ -182,16 +195,14 @@ func validateConstraintNotRepeat(path *field.Path, constraints []v1.TopologySpre
 
 // ValidateRequestedToCapacityRatioArgs validates that RequestedToCapacityRatioArgs are correct.
 func ValidateRequestedToCapacityRatioArgs(args config.RequestedToCapacityRatioArgs) error {
-	if err := validateFunctionShape(args.Shape); err != nil {
+	var path *field.Path
-		return err
+	var allErrs field.ErrorList
-	}
+	allErrs = append(allErrs, validateFunctionShape(args.Shape, path.Child("shape"))...)
-	if err := validateResourcesNoMax(args.Resources); err != nil {
+	allErrs = append(allErrs, validateResourcesNoMax(args.Resources, path.Child("resources"))...)
-		return err
+	return allErrs.ToAggregate()
-	}
-	return nil
 }
 
-func validateFunctionShape(shape []config.UtilizationShapePoint) error {
+func validateFunctionShape(shape []config.UtilizationShapePoint, path *field.Path) field.ErrorList {
 	const (
 		minUtilization = 0
 		maxUtilization = 100
@@ -199,64 +210,68 @@ func validateFunctionShape(shape []config.UtilizationShapePoint) error {
 		maxScore       = int32(config.MaxCustomPriorityScore)
 	)
 
+	var allErrs field.ErrorList
+
 	if len(shape) == 0 {
-		return fmt.Errorf("at least one point must be specified")
+		allErrs = append(allErrs, field.Required(path, "at least one point must be specified"))
+		return allErrs
 	}
 
 	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)
+			allErrs = append(allErrs, field.Invalid(path.Index(i).Child("utilization"), shape[i].Utilization, "utilization values must be sorted in increasing order"))
+			break
 		}
 	}
 
 	for i, point := range shape {
-		if point.Utilization < minUtilization {
+		if point.Utilization < minUtilization || point.Utilization > maxUtilization {
-			return fmt.Errorf("utilization values must not be less than %d. Utilization[%d]==%d", minUtilization, i, point.Utilization)
+			msg := fmt.Sprintf("not in valid range [%d, %d]", minUtilization, maxUtilization)
+			allErrs = append(allErrs, field.Invalid(path.Index(i).Child("utilization"), point.Utilization, msg))
 		}
-		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 || point.Score > maxScore {
-		}
+			msg := fmt.Sprintf("not in valid range [%d, %d]", minScore, maxScore)
-		if point.Score < minScore {
+			allErrs = append(allErrs, field.Invalid(path.Index(i).Child("score"), point.Score, msg))
-			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
+	return allErrs
 }
 
 // TODO potentially replace with validateResources
-func validateResourcesNoMax(resources []config.ResourceSpec) error {
+func validateResourcesNoMax(resources []config.ResourceSpec, p *field.Path) field.ErrorList {
-	for _, r := range resources {
+	var allErrs field.ErrorList
+	for i, 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)
+			allErrs = append(allErrs, field.Invalid(p.Index(i).Child("weight"), r.Weight,
+				fmt.Sprintf("resource weight of %s not in valid range [1, inf)", r.Name)))
 		}
 	}
-	return nil
+	return allErrs
 }
 
 // ValidateNodeResourcesLeastAllocatedArgs validates that NodeResourcesLeastAllocatedArgs are correct.
 func ValidateNodeResourcesLeastAllocatedArgs(args *config.NodeResourcesLeastAllocatedArgs) error {
-	return validateResources(args.Resources)
+	var path *field.Path
+	return validateResources(args.Resources, path.Child("resources")).ToAggregate()
 }
 
 // ValidateNodeResourcesMostAllocatedArgs validates that NodeResourcesMostAllocatedArgs are correct.
 func ValidateNodeResourcesMostAllocatedArgs(args *config.NodeResourcesMostAllocatedArgs) error {
-	return validateResources(args.Resources)
+	var path *field.Path
+	return validateResources(args.Resources, path.Child("resources")).ToAggregate()
 }
 
-func validateResources(resources []config.ResourceSpec) error {
+func validateResources(resources []config.ResourceSpec, p *field.Path) field.ErrorList {
-	for _, resource := range resources {
+	var allErrs field.ErrorList
-		if resource.Weight <= 0 {
+	for i, resource := range resources {
-			return fmt.Errorf("resource Weight of %v should be a positive value, got %v", resource.Name, resource.Weight)
+		if resource.Weight <= 0 || resource.Weight > 100 {
-		}
+			msg := fmt.Sprintf("resource weight of %v not in valid range (0, 100]", resource.Name)
-		if resource.Weight > 100 {
+			allErrs = append(allErrs, field.Invalid(p.Index(i).Child("weight"), resource.Weight, msg))
-			return fmt.Errorf("resource Weight of %v should be less than 100, got %v", resource.Name, resource.Weight)
 		}
 	}
-	return nil
+	return allErrs
 }
 
 // ValidateNodeAffinityArgs validates that NodeAffinityArgs are correct.

pkg/scheduler/apis/config/validation/validation_pluginargs_test.go

@@ -34,7 +34,7 @@ var (
 func TestValidateDefaultPreemptionArgs(t *testing.T) {
 	cases := map[string]struct {
 		args     config.DefaultPreemptionArgs
-		wantErr string
+		wantErrs field.ErrorList
 	}{
 		"valid args (default)": {
 			args: config.DefaultPreemptionArgs{
@@ -47,35 +47,75 @@ func TestValidateDefaultPreemptionArgs(t *testing.T) {
 				MinCandidateNodesPercentage: -1,
 				MinCandidateNodesAbsolute:   100,
 			},
-			wantErr: "minCandidateNodesPercentage is not in the range [0, 100]",
+			wantErrs: field.ErrorList{
+				&field.Error{
+					Type:  field.ErrorTypeInvalid,
+					Field: "minCandidateNodesPercentage",
+				},
+			},
 		},
 		"minCandidateNodesPercentage over 100": {
 			args: config.DefaultPreemptionArgs{
 				MinCandidateNodesPercentage: 900,
 				MinCandidateNodesAbsolute:   100,
 			},
-			wantErr: "minCandidateNodesPercentage is not in the range [0, 100]",
+			wantErrs: field.ErrorList{
+				&field.Error{
+					Type:  field.ErrorTypeInvalid,
+					Field: "minCandidateNodesPercentage",
+				},
+			},
 		},
 		"negative minCandidateNodesAbsolute": {
 			args: config.DefaultPreemptionArgs{
 				MinCandidateNodesPercentage: 20,
 				MinCandidateNodesAbsolute:   -1,
 			},
-			wantErr: "minCandidateNodesAbsolute is not in the range [0, inf)",
+			wantErrs: field.ErrorList{
+				&field.Error{
+					Type:  field.ErrorTypeInvalid,
+					Field: "minCandidateNodesAbsolute",
+				},
+			},
 		},
 		"all zero": {
 			args: config.DefaultPreemptionArgs{
 				MinCandidateNodesPercentage: 0,
 				MinCandidateNodesAbsolute:   0,
 			},
-			wantErr: "both minCandidateNodesPercentage and minCandidateNodesAbsolute cannot be zero",
+			wantErrs: field.ErrorList{
+				&field.Error{
+					Type:  field.ErrorTypeInvalid,
+					Field: "minCandidateNodesPercentage",
+				}, &field.Error{
+					Type:  field.ErrorTypeInvalid,
+					Field: "minCandidateNodesAbsolute",
+				},
+			},
+		},
+		"both negative": {
+			args: config.DefaultPreemptionArgs{
+				MinCandidateNodesPercentage: -1,
+				MinCandidateNodesAbsolute:   -1,
+			},
+			wantErrs: field.ErrorList{
+				&field.Error{
+					Type:  field.ErrorTypeInvalid,
+					Field: "minCandidateNodesPercentage",
+				}, &field.Error{
+					Type:  field.ErrorTypeInvalid,
+					Field: "minCandidateNodesAbsolute",
+				},
+			},
 		},
 	}
 
 	for name, tc := range cases {
 		t.Run(name, func(t *testing.T) {
 			err := ValidateDefaultPreemptionArgs(tc.args)
-			assertErr(t, tc.wantErr, err)
+			if diff := cmp.Diff(tc.wantErrs.ToAggregate(), err, ignoreBadValueDetail); diff != "" {
+				t.Fatalf("ValidateDefaultPreemptionArgs returned err (-want,+got):\n%s", diff)
+			}
 		})
 	}
 }
@@ -83,7 +123,7 @@ func TestValidateDefaultPreemptionArgs(t *testing.T) {
 func TestValidateInterPodAffinityArgs(t *testing.T) {
 	cases := map[string]struct {
 		args    config.InterPodAffinityArgs
-		wantErr string
+		wantErr error
 	}{
 		"valid args": {
 			args: config.InterPodAffinityArgs{
@@ -94,20 +134,28 @@ func TestValidateInterPodAffinityArgs(t *testing.T) {
 			args: config.InterPodAffinityArgs{
 				HardPodAffinityWeight: -1,
 			},
-			wantErr: `hardPodAffinityWeight: Invalid value: -1: not in valid range [0-100]`,
+			wantErr: &field.Error{
+				Type:  field.ErrorTypeInvalid,
+				Field: "hardPodAffinityWeight",
+			},
 		},
 		"hardPodAffinityWeight more than max": {
 			args: config.InterPodAffinityArgs{
 				HardPodAffinityWeight: 101,
 			},
-			wantErr: `hardPodAffinityWeight: Invalid value: 101: not in valid range [0-100]`,
+			wantErr: &field.Error{
+				Type:  field.ErrorTypeInvalid,
+				Field: "hardPodAffinityWeight",
+			},
 		},
 	}
 
 	for name, tc := range cases {
 		t.Run(name, func(t *testing.T) {
 			err := ValidateInterPodAffinityArgs(tc.args)
-			assertErr(t, tc.wantErr, err)
+			if diff := cmp.Diff(tc.wantErr, err, ignoreBadValueDetail); diff != "" {
+				t.Fatalf("ValidateInterPodAffinityArgs returned err (-want,+got):\n%s", diff)
+			}
 		})
 	}
 }
@@ -115,7 +163,7 @@ func TestValidateInterPodAffinityArgs(t *testing.T) {
 func TestValidateNodeLabelArgs(t *testing.T) {
 	cases := map[string]struct {
 		args     config.NodeLabelArgs
-		wantErr string
+		wantErrs field.ErrorList
 	}{
 		"valid config": {
 			args: config.NodeLabelArgs{
@@ -130,21 +178,65 @@ func TestValidateNodeLabelArgs(t *testing.T) {
 				PresentLabels: []string{"label"},
 				AbsentLabels:  []string{"label"},
 			},
-			wantErr: `detecting at least one label (e.g., "label") that exist in both the present([label]) and absent([label]) label list`,
+			wantErrs: field.ErrorList{
+				&field.Error{
+					Type:  field.ErrorTypeInvalid,
+					Field: "presentLabels[0]",
+				},
+			},
+		},
+		"multiple labels conflict": {
+			args: config.NodeLabelArgs{
+				PresentLabels: []string{"label", "label3"},
+				AbsentLabels:  []string{"label", "label2", "label3"},
+			},
+			wantErrs: field.ErrorList{
+				&field.Error{
+					Type:  field.ErrorTypeInvalid,
+					Field: "presentLabels[0]",
+				},
+				&field.Error{
+					Type:  field.ErrorTypeInvalid,
+					Field: "presentLabels[1]",
+				},
+			},
 		},
 		"labels preference conflict": {
 			args: config.NodeLabelArgs{
 				PresentLabelsPreference: []string{"label"},
 				AbsentLabelsPreference:  []string{"label"},
 			},
-			wantErr: `detecting at least one label (e.g., "label") that exist in both the present([label]) and absent([label]) label list`,
+			wantErrs: field.ErrorList{
+				&field.Error{
+					Type:  field.ErrorTypeInvalid,
+					Field: "presentLabelsPreference[0]",
+				},
+			},
+		},
+		"multiple labels preference conflict": {
+			args: config.NodeLabelArgs{
+				PresentLabelsPreference: []string{"label", "label3"},
+				AbsentLabelsPreference:  []string{"label", "label2", "label3"},
+			},
+			wantErrs: field.ErrorList{
+				&field.Error{
+					Type:  field.ErrorTypeInvalid,
+					Field: "presentLabelsPreference[0]",
+				},
+				&field.Error{
+					Type:  field.ErrorTypeInvalid,
+					Field: "presentLabelsPreference[1]",
+				},
+			},
 		},
 	}
 
 	for name, tc := range cases {
 		t.Run(name, func(t *testing.T) {
 			err := ValidateNodeLabelArgs(tc.args)
-			assertErr(t, tc.wantErr, err)
+			if diff := cmp.Diff(tc.wantErrs.ToAggregate(), err, ignoreBadValueDetail); diff != "" {
+				t.Fatalf("ValidateNodeLabelArgs returned err (-want,+got):\n%s", diff)
+			}
 		})
 	}
 }
@@ -152,7 +244,7 @@ func TestValidateNodeLabelArgs(t *testing.T) {
 func TestValidatePodTopologySpreadArgs(t *testing.T) {
 	cases := map[string]struct {
 		args     *config.PodTopologySpreadArgs
-		wantErr string
+		wantErrs field.ErrorList
 	}{
 		"valid config": {
 			args: &config.PodTopologySpreadArgs{
@@ -182,7 +274,12 @@ func TestValidatePodTopologySpreadArgs(t *testing.T) {
 				},
 				DefaultingType: config.ListDefaulting,
 			},
-			wantErr: `defaultConstraints[0].maxSkew: Invalid value: -1: must be greater than zero`,
+			wantErrs: field.ErrorList{
+				&field.Error{
+					Type:  field.ErrorTypeInvalid,
+					Field: "defaultConstraints[0].maxSkew",
+				},
+			},
 		},
 		"empty topology key": {
 			args: &config.PodTopologySpreadArgs{
@@ -195,7 +292,12 @@ func TestValidatePodTopologySpreadArgs(t *testing.T) {
 				},
 				DefaultingType: config.ListDefaulting,
 			},
-			wantErr: `defaultConstraints[0].topologyKey: Required value: can not be empty`,
+			wantErrs: field.ErrorList{
+				&field.Error{
+					Type:  field.ErrorTypeRequired,
+					Field: "defaultConstraints[0].topologyKey",
+				},
+			},
 		},
 		"whenUnsatisfiable is empty": {
 			args: &config.PodTopologySpreadArgs{
@@ -208,7 +310,12 @@ func TestValidatePodTopologySpreadArgs(t *testing.T) {
 				},
 				DefaultingType: config.ListDefaulting,
 			},
-			wantErr: `defaultConstraints[0].whenUnsatisfiable: Required value: can not be empty`,
+			wantErrs: field.ErrorList{
+				&field.Error{
+					Type:  field.ErrorTypeRequired,
+					Field: "defaultConstraints[0].whenUnsatisfiable",
+				},
+			},
 		},
 		"whenUnsatisfiable contains unsupported action": {
 			args: &config.PodTopologySpreadArgs{
@@ -221,7 +328,12 @@ func TestValidatePodTopologySpreadArgs(t *testing.T) {
 				},
 				DefaultingType: config.ListDefaulting,
 			},
-			wantErr: `defaultConstraints[0].whenUnsatisfiable: Unsupported value: "unknown action": supported values: "DoNotSchedule", "ScheduleAnyway"`,
+			wantErrs: field.ErrorList{
+				&field.Error{
+					Type:  field.ErrorTypeNotSupported,
+					Field: "defaultConstraints[0].whenUnsatisfiable",
+				},
+			},
 		},
 		"duplicated constraints": {
 			args: &config.PodTopologySpreadArgs{
@@ -239,7 +351,12 @@ func TestValidatePodTopologySpreadArgs(t *testing.T) {
 				},
 				DefaultingType: config.ListDefaulting,
 			},
-			wantErr: `defaultConstraints[1]: Duplicate value: "{node, DoNotSchedule}"`,
+			wantErrs: field.ErrorList{
+				&field.Error{
+					Type:  field.ErrorTypeDuplicate,
+					Field: "defaultConstraints[1]",
+				},
+			},
 		},
 		"label selector present": {
 			args: &config.PodTopologySpreadArgs{
@@ -257,7 +374,12 @@ func TestValidatePodTopologySpreadArgs(t *testing.T) {
 				},
 				DefaultingType: config.ListDefaulting,
 			},
-			wantErr: `defaultConstraints[0].labelSelector: Forbidden: constraint must not define a selector, as they deduced for each pod`,
+			wantErrs: field.ErrorList{
+				&field.Error{
+					Type:  field.ErrorTypeForbidden,
+					Field: "defaultConstraints[0].labelSelector",
+				},
+			},
 		},
 		"list default constraints, no constraints": {
 			args: &config.PodTopologySpreadArgs{
@@ -269,6 +391,17 @@ func TestValidatePodTopologySpreadArgs(t *testing.T) {
 				DefaultingType: config.SystemDefaulting,
 			},
 		},
+		"wrong constraints": {
+			args: &config.PodTopologySpreadArgs{
+				DefaultingType: "unknown",
+			},
+			wantErrs: field.ErrorList{
+				&field.Error{
+					Type:  field.ErrorTypeNotSupported,
+					Field: "defaultingType",
+				},
+			},
+		},
 		"system default constraints, but has constraints": {
 			args: &config.PodTopologySpreadArgs{
 				DefaultConstraints: []v1.TopologySpreadConstraint{
@@ -280,14 +413,21 @@ func TestValidatePodTopologySpreadArgs(t *testing.T) {
 				},
 				DefaultingType: config.SystemDefaulting,
 			},
-			wantErr: `defaultingType: Invalid value: "System": when .defaultConstraints are not empty`,
+			wantErrs: field.ErrorList{
+				&field.Error{
+					Type:  field.ErrorTypeInvalid,
+					Field: "defaultingType",
+				},
+			},
 		},
 	}
 
 	for name, tc := range cases {
 		t.Run(name, func(t *testing.T) {
 			err := ValidatePodTopologySpreadArgs(tc.args)
-			assertErr(t, tc.wantErr, err)
+			if diff := cmp.Diff(tc.wantErrs.ToAggregate(), err, ignoreBadValueDetail); diff != "" {
+				t.Fatalf("ValidatePodTopologySpreadArgs returned err (-want,+got):\n%s", diff)
+			}
 		})
 	}
 }
@@ -295,7 +435,7 @@ func TestValidatePodTopologySpreadArgs(t *testing.T) {
 func TestValidateRequestedToCapacityRatioArgs(t *testing.T) {
 	cases := map[string]struct {
 		args     config.RequestedToCapacityRatioArgs
-		wantErr string
+		wantErrs field.ErrorList
 	}{
 		"valid config": {
 			args: config.RequestedToCapacityRatioArgs{
@@ -331,7 +471,12 @@ func TestValidateRequestedToCapacityRatioArgs(t *testing.T) {
 					},
 				},
 			},
-			wantErr: `at least one point must be specified`,
+			wantErrs: field.ErrorList{
+				&field.Error{
+					Type:  field.ErrorTypeRequired,
+					Field: "shape",
+				},
+			},
 		},
 		"utilization less than min": {
 			args: config.RequestedToCapacityRatioArgs{
@@ -346,7 +491,12 @@ func TestValidateRequestedToCapacityRatioArgs(t *testing.T) {
 					},
 				},
 			},
-			wantErr: `utilization values must not be less than 0. Utilization[0]==-10`,
+			wantErrs: field.ErrorList{
+				&field.Error{
+					Type:  field.ErrorTypeInvalid,
+					Field: "shape[0].utilization",
+				},
+			},
 		},
 		"utilization greater than max": {
 			args: config.RequestedToCapacityRatioArgs{
@@ -361,9 +511,14 @@ func TestValidateRequestedToCapacityRatioArgs(t *testing.T) {
 					},
 				},
 			},
-			wantErr: `utilization values must not be greater than 100. Utilization[1]==110`,
+			wantErrs: field.ErrorList{
+				&field.Error{
+					Type:  field.ErrorTypeInvalid,
+					Field: "shape[1].utilization",
 				},
-		"Utilization values in non-increasing order": {
+			},
+		},
+		"utilization values in non-increasing order": {
 			args: config.RequestedToCapacityRatioArgs{
 				Shape: []config.UtilizationShapePoint{
 					{
@@ -380,7 +535,12 @@ func TestValidateRequestedToCapacityRatioArgs(t *testing.T) {
 					},
 				},
 			},
-			wantErr: `utilization values must be sorted. Utilization[0]==30 >= Utilization[1]==20`,
+			wantErrs: field.ErrorList{
+				&field.Error{
+					Type:  field.ErrorTypeInvalid,
+					Field: "shape[1].utilization",
+				},
+			},
 		},
 		"duplicated utilization values": {
 			args: config.RequestedToCapacityRatioArgs{
@@ -399,7 +559,12 @@ func TestValidateRequestedToCapacityRatioArgs(t *testing.T) {
 					},
 				},
 			},
-			wantErr: `utilization values must be sorted. Utilization[1]==20 >= Utilization[2]==20`,
+			wantErrs: field.ErrorList{
+				&field.Error{
+					Type:  field.ErrorTypeInvalid,
+					Field: "shape[2].utilization",
+				},
+			},
 		},
 		"score less than min": {
 			args: config.RequestedToCapacityRatioArgs{
@@ -414,7 +579,12 @@ func TestValidateRequestedToCapacityRatioArgs(t *testing.T) {
 					},
 				},
 			},
-			wantErr: `score values must not be less than 0. Score[0]==-1`,
+			wantErrs: field.ErrorList{
+				&field.Error{
+					Type:  field.ErrorTypeInvalid,
+					Field: "shape[0].score",
+				},
+			},
 		},
 		"score greater than max": {
 			args: config.RequestedToCapacityRatioArgs{
@@ -429,7 +599,12 @@ func TestValidateRequestedToCapacityRatioArgs(t *testing.T) {
 					},
 				},
 			},
-			wantErr: `score values must not be greater than 10. Score[1]==11`,
+			wantErrs: field.ErrorList{
+				&field.Error{
+					Type:  field.ErrorTypeInvalid,
+					Field: "shape[1].score",
+				},
+			},
 		},
 		"resources weight less than 1": {
 			args: config.RequestedToCapacityRatioArgs{
@@ -446,14 +621,55 @@ func TestValidateRequestedToCapacityRatioArgs(t *testing.T) {
 					},
 				},
 			},
-			wantErr: `resource custom weight 0 must not be less than 1`,
+			wantErrs: field.ErrorList{
+				&field.Error{
+					Type:  field.ErrorTypeInvalid,
+					Field: "resources[0].weight",
+				},
+			},
+		},
+		"multiple errors": {
+			args: config.RequestedToCapacityRatioArgs{
+				Shape: []config.UtilizationShapePoint{
+					{
+						Utilization: 20,
+						Score:       -1,
+					},
+					{
+						Utilization: 10,
+						Score:       2,
+					},
+				},
+				Resources: []config.ResourceSpec{
+					{
+						Name:   "custom",
+						Weight: 0,
+					},
+				},
+			},
+			wantErrs: field.ErrorList{
+				&field.Error{
+					Type:  field.ErrorTypeInvalid,
+					Field: "shape[1].utilization",
+				},
+				&field.Error{
+					Type:  field.ErrorTypeInvalid,
+					Field: "shape[0].score",
+				},
+				&field.Error{
+					Type:  field.ErrorTypeInvalid,
+					Field: "resources[0].weight",
+				},
+			},
 		},
 	}
 
 	for name, tc := range cases {
 		t.Run(name, func(t *testing.T) {
 			err := ValidateRequestedToCapacityRatioArgs(tc.args)
-			assertErr(t, tc.wantErr, err)
+			if diff := cmp.Diff(tc.wantErrs.ToAggregate(), err, ignoreBadValueDetail); diff != "" {
+				t.Fatalf("ValidateRequestedToCapacityRatioArgs returned err (-want,+got):\n%s", diff)
+			}
 		})
 	}
 }
@@ -461,7 +677,7 @@ func TestValidateRequestedToCapacityRatioArgs(t *testing.T) {
 func TestValidateNodeResourcesLeastAllocatedArgs(t *testing.T) {
 	cases := map[string]struct {
 		args     *config.NodeResourcesLeastAllocatedArgs
-		wantErr string
+		wantErrs field.ErrorList
 	}{
 		"valid config": {
 			args: &config.NodeResourcesLeastAllocatedArgs{
@@ -486,7 +702,12 @@ func TestValidateNodeResourcesLeastAllocatedArgs(t *testing.T) {
 					},
 				},
 			},
-			wantErr: `resource Weight of cpu should be a positive value, got 0`,
+			wantErrs: field.ErrorList{
+				&field.Error{
+					Type:  field.ErrorTypeInvalid,
+					Field: "resources[0].weight",
+				},
+			},
 		},
 		"weight more than max": {
 			args: &config.NodeResourcesLeastAllocatedArgs{
@@ -497,14 +718,45 @@ func TestValidateNodeResourcesLeastAllocatedArgs(t *testing.T) {
 					},
 				},
 			},
-			wantErr: `resource Weight of memory should be less than 100, got 101`,
+			wantErrs: field.ErrorList{
+				&field.Error{
+					Type:  field.ErrorTypeInvalid,
+					Field: "resources[0].weight",
+				},
+			},
+		},
+		"multiple error": {
+			args: &config.NodeResourcesLeastAllocatedArgs{
+				Resources: []config.ResourceSpec{
+					{
+						Name:   "memory",
+						Weight: 0,
+					},
+					{
+						Name:   "cpu",
+						Weight: 101,
+					},
+				},
+			},
+			wantErrs: field.ErrorList{
+				&field.Error{
+					Type:  field.ErrorTypeInvalid,
+					Field: "resources[0].weight",
+				},
+				&field.Error{
+					Type:  field.ErrorTypeInvalid,
+					Field: "resources[1].weight",
+				},
+			},
 		},
 	}
 
 	for name, tc := range cases {
 		t.Run(name, func(t *testing.T) {
 			err := ValidateNodeResourcesLeastAllocatedArgs(tc.args)
-			assertErr(t, tc.wantErr, err)
+			if diff := cmp.Diff(tc.wantErrs.ToAggregate(), err, ignoreBadValueDetail); diff != "" {
+				t.Fatalf("ValidateNodeResourcesLeastAllocatedArgs returned err (-want,+got):\n%s", diff)
+			}
 		})
 	}
 }
@@ -512,7 +764,7 @@ func TestValidateNodeResourcesLeastAllocatedArgs(t *testing.T) {
 func TestValidateNodeResourcesMostAllocatedArgs(t *testing.T) {
 	cases := map[string]struct {
 		args     *config.NodeResourcesMostAllocatedArgs
-		wantErr string
+		wantErrs field.ErrorList
 	}{
 		"valid config": {
 			args: &config.NodeResourcesMostAllocatedArgs{
@@ -537,7 +789,12 @@ func TestValidateNodeResourcesMostAllocatedArgs(t *testing.T) {
 					},
 				},
 			},
-			wantErr: `resource Weight of cpu should be a positive value, got -1`,
+			wantErrs: field.ErrorList{
+				&field.Error{
+					Type:  field.ErrorTypeInvalid,
+					Field: "resources[0].weight",
+				},
+			},
 		},
 		"weight more than max": {
 			args: &config.NodeResourcesMostAllocatedArgs{
@@ -548,14 +805,45 @@ func TestValidateNodeResourcesMostAllocatedArgs(t *testing.T) {
 					},
 				},
 			},
-			wantErr: `resource Weight of memory should be less than 100, got 110`,
+			wantErrs: field.ErrorList{
+				&field.Error{
+					Type:  field.ErrorTypeInvalid,
+					Field: "resources[0].weight",
+				},
+			},
+		},
+		"multiple error": {
+			args: &config.NodeResourcesMostAllocatedArgs{
+				Resources: []config.ResourceSpec{
+					{
+						Name:   "memory",
+						Weight: -1,
+					},
+					{
+						Name:   "cpu",
+						Weight: 110,
+					},
+				},
+			},
+			wantErrs: field.ErrorList{
+				&field.Error{
+					Type:  field.ErrorTypeInvalid,
+					Field: "resources[0].weight",
+				},
+				&field.Error{
+					Type:  field.ErrorTypeInvalid,
+					Field: "resources[1].weight",
+				},
+			},
 		},
 	}
 
 	for name, tc := range cases {
 		t.Run(name, func(t *testing.T) {
 			err := ValidateNodeResourcesMostAllocatedArgs(tc.args)
-			assertErr(t, tc.wantErr, err)
+			if diff := cmp.Diff(tc.wantErrs.ToAggregate(), err, ignoreBadValueDetail); diff != "" {
+				t.Fatalf("ValidateNodeResourcesLeastAllocatedArgs returned err (-want,+got):\n%s", diff)
+			}
 		})
 	}
 }
@@ -651,18 +939,3 @@ func TestValidateNodeAffinityArgs(t *testing.T) {
 		})
 	}
 }
-
-func assertErr(t *testing.T, wantErr string, gotErr error) {
-	if wantErr == "" {
-		if gotErr != nil {
-			t.Fatalf("\nwant err to be:\n\tnil\ngot:\n\t%s", gotErr.Error())
-		}
-	} else {
-		if gotErr == nil {
-			t.Fatalf("\nwant err to be:\n\t%s\ngot:\n\tnil", wantErr)
-		}
-		if gotErr.Error() != wantErr {
-			t.Errorf("\nwant err to be:\n\t%s\ngot:\n\t%s", wantErr, gotErr.Error())
-		}
-	}
-}

pkg/scheduler/framework/plugins/noderesources/BUILD

@@ -66,9 +66,12 @@ go_test(
         "//staging/src/k8s.io/api/core/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library",
         "//staging/src/k8s.io/component-base/featuregate:go_default_library",
         "//staging/src/k8s.io/component-base/featuregate/testing:go_default_library",
+        "//vendor/github.com/google/go-cmp/cmp:go_default_library",
+        "//vendor/github.com/google/go-cmp/cmp/cmpopts:go_default_library",
         "//vendor/github.com/stretchr/testify/assert:go_default_library",
     ],
 )

pkg/scheduler/framework/plugins/noderesources/least_allocated_test.go

@@ -21,9 +21,12 @@ import (
 	"reflect"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/kubernetes/pkg/scheduler/apis/config"
 	"k8s.io/kubernetes/pkg/scheduler/framework"
 	"k8s.io/kubernetes/pkg/scheduler/framework/runtime"
@@ -101,7 +104,7 @@ func TestNodeResourcesLeastAllocated(t *testing.T) {
 		pods         []*v1.Pod
 		nodes        []*v1.Node
 		args         config.NodeResourcesLeastAllocatedArgs
-		wantErr      string
+		wantErr      error
 		expectedList framework.NodeScoreList
 		name         string
 	}{
@@ -263,7 +266,12 @@ func TestNodeResourcesLeastAllocated(t *testing.T) {
 			pod:   &v1.Pod{Spec: cpuAndMemory},
 			nodes: []*v1.Node{makeNode("machine", 4000, 10000)},
 			args:  config.NodeResourcesLeastAllocatedArgs{Resources: []config.ResourceSpec{{Name: "memory", Weight: -1}, {Name: "cpu", Weight: 1}}},
-			wantErr: "resource Weight of memory should be a positive value, got -1",
+			wantErr: field.ErrorList{
+				&field.Error{
+					Type:  field.ErrorTypeInvalid,
+					Field: "resources[0].weight",
+				},
+			}.ToAggregate(),
 			name: "resource with negtive weight",
 		},
 		{
@@ -271,7 +279,12 @@ func TestNodeResourcesLeastAllocated(t *testing.T) {
 			pod:   &v1.Pod{Spec: cpuAndMemory},
 			nodes: []*v1.Node{makeNode("machine", 4000, 10000)},
 			args:  config.NodeResourcesLeastAllocatedArgs{Resources: []config.ResourceSpec{{Name: "memory", Weight: 1}, {Name: "cpu", Weight: 0}}},
-			wantErr: "resource Weight of cpu should be a positive value, got 0",
+			wantErr: field.ErrorList{
+				&field.Error{
+					Type:  field.ErrorTypeInvalid,
+					Field: "resources[1].weight",
+				},
+			}.ToAggregate(),
 			name: "resource with zero weight",
 		},
 		{
@@ -279,7 +292,12 @@ func TestNodeResourcesLeastAllocated(t *testing.T) {
 			pod:   &v1.Pod{Spec: cpuAndMemory},
 			nodes: []*v1.Node{makeNode("machine", 4000, 10000)},
 			args:  config.NodeResourcesLeastAllocatedArgs{Resources: []config.ResourceSpec{{Name: "memory", Weight: 120}}},
-			wantErr: "resource Weight of memory should be less than 100, got 120",
+			wantErr: field.ErrorList{
+				&field.Error{
+					Type:  field.ErrorTypeInvalid,
+					Field: "resources[0].weight",
+				},
+			}.ToAggregate(),
 			name: "resource weight larger than MaxNodeScore",
 		},
 	}
@@ -290,16 +308,19 @@ func TestNodeResourcesLeastAllocated(t *testing.T) {
 			fh, _ := runtime.NewFramework(nil, nil, nil, runtime.WithSnapshotSharedLister(snapshot))
 			p, err := NewLeastAllocated(&test.args, fh)
 
-			if len(test.wantErr) != 0 {
+			if test.wantErr != nil {
-				if err != nil && test.wantErr != err.Error() {
+				if err != nil {
-					t.Fatalf("got err %v, want %v", err.Error(), test.wantErr)
+					diff := cmp.Diff(test.wantErr, err, cmpopts.IgnoreFields(field.Error{}, "BadValue", "Detail"))
-				} else if err == nil {
+					if diff != "" {
+						t.Fatalf("got err (-want,+got):\n%s", diff)
+					}
+				} else {
 					t.Fatalf("no error produced, wanted %v", test.wantErr)
 				}
 				return
 			}
 
-			if err != nil && len(test.wantErr) == 0 {
+			if err != nil && test.wantErr == nil {
 				t.Fatalf("failed to initialize plugin NodeResourcesLeastAllocated, got error: %v", err)
 			}
 

pkg/scheduler/framework/plugins/noderesources/most_allocated_test.go

@@ -21,9 +21,12 @@ import (
 	"reflect"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/kubernetes/pkg/scheduler/apis/config"
 	"k8s.io/kubernetes/pkg/scheduler/framework"
 	"k8s.io/kubernetes/pkg/scheduler/framework/runtime"
@@ -116,7 +119,7 @@ func TestNodeResourcesMostAllocated(t *testing.T) {
 		pods         []*v1.Pod
 		nodes        []*v1.Node
 		args         config.NodeResourcesMostAllocatedArgs
-		wantErr      string
+		wantErr      error
 		expectedList framework.NodeScoreList
 		name         string
 	}{
@@ -223,7 +226,12 @@ func TestNodeResourcesMostAllocated(t *testing.T) {
 			pod:   &v1.Pod{Spec: cpuAndMemory},
 			nodes: []*v1.Node{makeNode("machine", 4000, 10000)},
 			args:  config.NodeResourcesMostAllocatedArgs{Resources: []config.ResourceSpec{{Name: "memory", Weight: -1}, {Name: "cpu", Weight: 1}}},
-			wantErr: "resource Weight of memory should be a positive value, got -1",
+			wantErr: field.ErrorList{
+				&field.Error{
+					Type:  field.ErrorTypeInvalid,
+					Field: "resources[0].weight",
+				},
+			}.ToAggregate(),
 			name: "resource with negtive weight",
 		},
 		{
@@ -231,7 +239,12 @@ func TestNodeResourcesMostAllocated(t *testing.T) {
 			pod:   &v1.Pod{Spec: cpuAndMemory},
 			nodes: []*v1.Node{makeNode("machine", 4000, 10000)},
 			args:  config.NodeResourcesMostAllocatedArgs{Resources: []config.ResourceSpec{{Name: "memory", Weight: 1}, {Name: "cpu", Weight: 0}}},
-			wantErr: "resource Weight of cpu should be a positive value, got 0",
+			wantErr: field.ErrorList{
+				&field.Error{
+					Type:  field.ErrorTypeInvalid,
+					Field: "resources[1].weight",
+				},
+			}.ToAggregate(),
 			name: "resource with zero weight",
 		},
 		{
@@ -239,7 +252,12 @@ func TestNodeResourcesMostAllocated(t *testing.T) {
 			pod:   &v1.Pod{Spec: cpuAndMemory},
 			nodes: []*v1.Node{makeNode("machine", 4000, 10000)},
 			args:  config.NodeResourcesMostAllocatedArgs{Resources: []config.ResourceSpec{{Name: "memory", Weight: 120}}},
-			wantErr: "resource Weight of memory should be less than 100, got 120",
+			wantErr: field.ErrorList{
+				&field.Error{
+					Type:  field.ErrorTypeInvalid,
+					Field: "resources[0].weight",
+				},
+			}.ToAggregate(),
 			name: "resource weight larger than MaxNodeScore",
 		},
 	}
@@ -250,16 +268,19 @@ func TestNodeResourcesMostAllocated(t *testing.T) {
 			fh, _ := runtime.NewFramework(nil, nil, nil, runtime.WithSnapshotSharedLister(snapshot))
 			p, err := NewMostAllocated(&test.args, fh)
 
-			if len(test.wantErr) != 0 {
+			if test.wantErr != nil {
-				if err != nil && test.wantErr != err.Error() {
+				if err != nil {
-					t.Fatalf("got err %v, want %v", err.Error(), test.wantErr)
+					diff := cmp.Diff(test.wantErr, err, cmpopts.IgnoreFields(field.Error{}, "BadValue", "Detail"))
-				} else if err == nil {
+					if diff != "" {
+						t.Fatalf("got err (-want,+got):\n%s", diff)
+					}
+				} else {
 					t.Fatalf("no error produced, wanted %v", test.wantErr)
 				}
 				return
 			}
 
-			if err != nil && len(test.wantErr) == 0 {
+			if err != nil && test.wantErr == nil {
 				t.Fatalf("failed to initialize plugin NodeResourcesMostAllocated, got error: %v", err)
 			}