Skip to content

K
k3s
Commit b86dfcf6 authored by Rudi Chiarito's avatar Rudi Chiarito
Browse files
Options

Fix GPU resource validation

Because of defaulting, requests are always set if limits are. Thus, the check can never succeed. Instead, make sure that the two values are equal. Also, remove a few other error messages and remove unnecessary Sprintf calls.
parent d4df1681
Hide whitespace changes
Inline Side-by-side
Showing with 28 additions and 11 deletions
pkg/api/validation/validation.go
View file @ b86dfcf6
...@@ -1197,11 +1197,11 @@ func validateContainerResourceDivisor(rName string, divisor resource.Quantity, f ...@@ -1197,11 +1197,11 @@ func validateContainerResourceDivisor(rName string, divisor resource.Quantity, f
switch rName { switch rName {
case "limits.cpu", "requests.cpu": case "limits.cpu", "requests.cpu":
if !validContainerResourceDivisorForCPU.Has(divisor.String()) { if !validContainerResourceDivisorForCPU.Has(divisor.String()) {
allErrs = append(allErrs, field.Invalid(fldPath.Child("divisor"), rName, fmt.Sprintf("only divisor's values 1m and 1 are supported with the cpu resource"))) allErrs = append(allErrs, field.Invalid(fldPath.Child("divisor"), rName, "only divisor's values 1m and 1 are supported with the cpu resource"))
} }
case "limits.memory", "requests.memory": case "limits.memory", "requests.memory":
if !validContainerResourceDivisorForMemory.Has(divisor.String()) { if !validContainerResourceDivisorForMemory.Has(divisor.String()) {
allErrs = append(allErrs, field.Invalid(fldPath.Child("divisor"), rName, fmt.Sprintf("only divisor's values 1, 1k, 1M, 1G, 1T, 1P, 1E, 1Ki, 1Mi, 1Gi, 1Ti, 1Pi, 1Ei are supported with the memory resource"))) allErrs = append(allErrs, field.Invalid(fldPath.Child("divisor"), rName, "only divisor's values 1, 1k, 1M, 1G, 1T, 1P, 1E, 1Ki, 1Mi, 1Gi, 1Ti, 1Pi, 1Ei are supported with the memory resource"))
} }
} }
return allErrs return allErrs
...@@ -2780,11 +2780,11 @@ func ValidateResourceRequirements(requirements *api.ResourceRequirements, fldPat ...@@ -2780,11 +2780,11 @@ func ValidateResourceRequirements(requirements *api.ResourceRequirements, fldPat
// Check that request <= limit. // Check that request <= limit.
requestQuantity, exists := requirements.Requests[resourceName] requestQuantity, exists := requirements.Requests[resourceName]
if exists { if exists {
// For GPUs, require that no request be set. // For GPUs, not only requests can't exceed limits, they also can't be lower, i.e. must be equal.
if resourceName == api.ResourceNvidiaGPU { if resourceName == api.ResourceNvidiaGPU && quantity.Cmp(requestQuantity) != 0 {
allErrs = append(allErrs, field.Invalid(reqPath, requestQuantity.String(), "cannot be set")) allErrs = append(allErrs, field.Invalid(reqPath, requestQuantity.String(), fmt.Sprintf("must be equal to %s limit", api.ResourceNvidiaGPU)))
} else if quantity.Cmp(requestQuantity) < 0 { } else if quantity.Cmp(requestQuantity) < 0 {
allErrs = append(allErrs, field.Invalid(fldPath, quantity.String(), "must be greater than or equal to request")) allErrs = append(allErrs, field.Invalid(limPath, quantity.String(), fmt.Sprintf("must be greater than or equal to %s request", resourceName)))
} }
} }
} }
...@@ -2957,7 +2957,7 @@ func validateFinalizerName(stringValue string, fldPath *field.Path) field.ErrorL ...@@ -2957,7 +2957,7 @@ func validateFinalizerName(stringValue string, fldPath *field.Path) field.ErrorL
if len(strings.Split(stringValue, "/")) == 1 { if len(strings.Split(stringValue, "/")) == 1 {
if !api.IsStandardFinalizerName(stringValue) { if !api.IsStandardFinalizerName(stringValue) {
return append(allErrs, field.Invalid(fldPath, stringValue, fmt.Sprintf("name is neither a standard finalizer name nor is it fully qualified"))) return append(allErrs, field.Invalid(fldPath, stringValue, "name is neither a standard finalizer name nor is it fully qualified"))
} }
} }
......
pkg/api/validation/validation_test.go
View file @ b86dfcf6
...@@ -1551,7 +1551,24 @@ func TestValidateContainers(t *testing.T) { ...@@ -1551,7 +1551,24 @@ func TestValidateContainers(t *testing.T) {
ImagePullPolicy: "IfNotPresent", ImagePullPolicy: "IfNotPresent",
}, },
{ {
Name: "resources-test-with-gpu", Name: "resources-test-with-gpu-with-request",
Image: "image",
Resources: api.ResourceRequirements{
Requests: api.ResourceList{
api.ResourceName(api.ResourceCPU): resource.MustParse("10"),
api.ResourceName(api.ResourceMemory): resource.MustParse("10G"),
api.ResourceName(api.ResourceNvidiaGPU): resource.MustParse("1"),
},
Limits: api.ResourceList{
api.ResourceName(api.ResourceCPU): resource.MustParse("10"),
api.ResourceName(api.ResourceMemory): resource.MustParse("10G"),
api.ResourceName(api.ResourceNvidiaGPU): resource.MustParse("1"),
},
},
ImagePullPolicy: "IfNotPresent",
},
{
Name: "resources-test-with-gpu-without-request",
Image: "image", Image: "image",
Resources: api.ResourceRequirements{ Resources: api.ResourceRequirements{
Requests: api.ResourceList{ Requests: api.ResourceList{
...@@ -1789,15 +1806,15 @@ func TestValidateContainers(t *testing.T) { ...@@ -1789,15 +1806,15 @@ func TestValidateContainers(t *testing.T) {
ImagePullPolicy: "IfNotPresent", ImagePullPolicy: "IfNotPresent",
}, },
}, },
"Resource can only have GPU limit": { "Resource GPU limit must match request": {
{ {
Name: "resources-request-limit-edge", Name: "gpu-resource-request-limit",
Image: "image", Image: "image",
Resources: api.ResourceRequirements{ Resources: api.ResourceRequirements{
Requests: api.ResourceList{ Requests: api.ResourceList{
api.ResourceName(api.ResourceCPU): resource.MustParse("10"), api.ResourceName(api.ResourceCPU): resource.MustParse("10"),
api.ResourceName(api.ResourceMemory): resource.MustParse("10G"), api.ResourceName(api.ResourceMemory): resource.MustParse("10G"),
api.ResourceName(api.ResourceNvidiaGPU): resource.MustParse("1"), api.ResourceName(api.ResourceNvidiaGPU): resource.MustParse("0"),
}, },
Limits: api.ResourceList{ Limits: api.ResourceList{
api.ResourceName(api.ResourceCPU): resource.MustParse("10"), api.ResourceName(api.ResourceCPU): resource.MustParse("10"),
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment