Commit 8677b0c5 authored by k8s-merge-robot's avatar k8s-merge-robot Committed by GitHub

Merge pull request #28743 from Clarifai/gpu-impl

Automatic merge from submit-queue Fix GPU resource validation This fixes scheduling of pods with GPU resources. The change was never upstreamed during the 1.3 beta period, as it got lost in the noise of other changes in our fork. Ooops. I'll submit a cherry-pick request for 1.3.1 as soon as this lands in master. 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.
parents 51629f50 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
...@@ -2819,11 +2819,11 @@ func ValidateResourceRequirements(requirements *api.ResourceRequirements, fldPat ...@@ -2819,11 +2819,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)))
} }
} }
} }
...@@ -2996,7 +2996,7 @@ func validateFinalizerName(stringValue string, fldPath *field.Path) field.ErrorL ...@@ -2996,7 +2996,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"))
} }
} }
......
...@@ -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