Merge pull request #112925 from SergeyKanzhelev/addValueOfLimitToErr

added limit value to the pod validation error to simplify debugging
This commit is contained in:
Kubernetes Prow Robot 2023-01-19 11:52:15 -08:00 committed by GitHub
commit 7913e135a5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 40 additions and 9 deletions

View File

@ -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())))
}
}
}

View File

@ -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"
@ -91,6 +92,8 @@ func TestValidateResourceRequirements(t *testing.T) {
errorCase := []struct {
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

View File

@ -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"))