diff --git a/pkg/apis/core/v1/validation/validation.go b/pkg/apis/core/v1/validation/validation.go index 4f7ca42e40f..8f2786b50df 100644 --- a/pkg/apis/core/v1/validation/validation.go +++ b/pkg/apis/core/v1/validation/validation.go @@ -20,7 +20,7 @@ import ( "fmt" "strings" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation/field" @@ -60,9 +60,9 @@ func ValidateResourceRequirements(requirements *v1.ResourceRequirements, fldPath if exists { // For GPUs, not only requests can't exceed limits, they also can't be lower, i.e. must be equal. if quantity.Cmp(limitQuantity) != 0 && !v1helper.IsOvercommitAllowed(resourceName) { - allErrs = append(allErrs, field.Invalid(reqPath, quantity.String(), fmt.Sprintf("must be equal to %s limit", resourceName))) + allErrs = append(allErrs, field.Invalid(reqPath, quantity.String(), fmt.Sprintf("must be equal to %s limit of %s", resourceName, limitQuantity.String()))) } else if quantity.Cmp(limitQuantity) > 0 { - allErrs = append(allErrs, field.Invalid(reqPath, quantity.String(), fmt.Sprintf("must be less than or equal to %s limit", resourceName))) + allErrs = append(allErrs, field.Invalid(reqPath, quantity.String(), fmt.Sprintf("must be less than or equal to %s limit of %s", resourceName, limitQuantity.String()))) } } } diff --git a/pkg/apis/core/v1/validation/validation_test.go b/pkg/apis/core/v1/validation/validation_test.go index f4bbeacad40..97fd7735861 100644 --- a/pkg/apis/core/v1/validation/validation_test.go +++ b/pkg/apis/core/v1/validation/validation_test.go @@ -17,9 +17,10 @@ limitations under the License. package validation import ( + "strings" "testing" - "k8s.io/api/core/v1" + 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/sets" @@ -89,8 +90,10 @@ func TestValidateResourceRequirements(t *testing.T) { } errorCase := []struct { - name string - requirements v1.ResourceRequirements + name string + requirements v1.ResourceRequirements + skipLimitValueCheck bool + skipRequestValueCheck bool }{ { name: "Resources with Requests Larger Than Limits", @@ -114,6 +117,7 @@ func TestValidateResourceRequirements(t *testing.T) { v1.ResourceName("my.org"): resource.MustParse("10m"), }, }, + skipRequestValueCheck: true, }, { name: "Invalid Resources with Limits", @@ -122,17 +126,44 @@ func TestValidateResourceRequirements(t *testing.T) { v1.ResourceName("my.org"): resource.MustParse("9m"), }, }, + skipLimitValueCheck: true, }, } for _, tc := range errorCase { t.Run(tc.name, func(t *testing.T) { - if errs := ValidateResourceRequirements(&tc.requirements, field.NewPath("resources")); len(errs) == 0 { + errs := ValidateResourceRequirements(&tc.requirements, field.NewPath("resources")) + if len(errs) == 0 { t.Errorf("expected error") } + validateNamesAndValuesInDescription(t, tc.requirements.Limits, errs, tc.skipLimitValueCheck, "limit") + validateNamesAndValuesInDescription(t, tc.requirements.Requests, errs, tc.skipRequestValueCheck, "request") }) } } +func validateNamesAndValuesInDescription(t *testing.T, r v1.ResourceList, errs field.ErrorList, skipValueTest bool, rl string) { + for name, value := range r { + containsName := false + containsValue := false + + for _, e := range errs { + if strings.Contains(e.Error(), name.String()) { + containsName = true + } + + if strings.Contains(e.Error(), value.String()) { + containsValue = true + } + } + if !containsName { + t.Errorf("error must contain %s name", rl) + } + if !containsValue && !skipValueTest { + t.Errorf("error must contain %s value", rl) + } + } +} + func TestValidateContainerResourceName(t *testing.T) { successCase := []struct { name string diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 344b746758c..946147156c7 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -5936,9 +5936,9 @@ func ValidateResourceRequirements(requirements *core.ResourceRequirements, podCl if exists { // For non overcommitable resources, not only requests can't exceed limits, they also can't be lower, i.e. must be equal. if quantity.Cmp(limitQuantity) != 0 && !helper.IsOvercommitAllowed(resourceName) { - allErrs = append(allErrs, field.Invalid(reqPath, quantity.String(), fmt.Sprintf("must be equal to %s limit", resourceName))) + allErrs = append(allErrs, field.Invalid(reqPath, quantity.String(), fmt.Sprintf("must be equal to %s limit of %s", resourceName, limitQuantity.String()))) } else if quantity.Cmp(limitQuantity) > 0 { - allErrs = append(allErrs, field.Invalid(reqPath, quantity.String(), fmt.Sprintf("must be less than or equal to %s limit", resourceName))) + allErrs = append(allErrs, field.Invalid(reqPath, quantity.String(), fmt.Sprintf("must be less than or equal to %s limit of %s", resourceName, limitQuantity.String()))) } } else if !helper.IsOvercommitAllowed(resourceName) { allErrs = append(allErrs, field.Required(limPath, "Limit must be set for non overcommitable resources"))