Merge pull request #57302 from lichuqiang/resourceQuota4extendedResource

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Support for resource quota on extended resources

**Which issue(s) this PR fixes** :
Fixes #46639 #57300 for  resource quota support

**Special notes for your reviewer**:
One thing to be determined is if it necessary to Explicitly prohibit defining limits for extended resources in quota, like we did for [hugepages](https://github.com/kubernetes/kubernetes/pull/54292#pullrequestreview-74982771), as the resource is not allowed to overcommit.

**Release note**:

```release-note
Support for resource quota on extended resources
```

/cc @jiayingz @vishh @derekwaynecarr

pkg/apis/core/helper/BUILD

@@ -30,6 +30,7 @@ go_library(
         "//vendor/k8s.io/apimachinery/pkg/labels:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/selection:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/util/validation:go_default_library",
     ],
 )
 

pkg/apis/core/helper/helpers.go

@@ -28,6 +28,7 @@ import (
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/selection"
 	"k8s.io/apimachinery/pkg/util/sets"
+	"k8s.io/apimachinery/pkg/util/validation"
 	"k8s.io/kubernetes/pkg/apis/core"
 )
 
@@ -146,10 +147,21 @@ func IsStandardContainerResourceName(str string) bool {
 	return standardContainerResources.Has(str) || IsHugePageResourceName(core.ResourceName(str))
 }
 
-// IsExtendedResourceName returns true if the resource name is not in the
+// IsExtendedResourceName returns true if:
-// default namespace.
+// 1. the resource name is not in the default namespace;
+// 2. resource name does not have "requests." prefix,
+// to avoid confusion with the convention in quota
+// 3. it satisfies the rules in IsQualifiedName() after converted into quota resource name
 func IsExtendedResourceName(name core.ResourceName) bool {
-	return !IsDefaultNamespaceResource(name)
+	if IsDefaultNamespaceResource(name) || strings.HasPrefix(string(name), core.DefaultResourceRequestsPrefix) {
+		return false
+	}
+	// Ensure it satisfies the rules in IsQualifiedName() after converted into quota resource name
+	nameForQuota := fmt.Sprintf("%s%s", core.DefaultResourceRequestsPrefix, string(name))
+	if errs := validation.IsQualifiedName(string(nameForQuota)); len(errs) != 0 {
+		return false
+	}
+	return true
 }
 
 // IsDefaultNamespaceResource returns true if the resource name is in the

pkg/apis/core/types.go

@@ -4212,6 +4212,8 @@ const (
 	// HugePages request, in bytes. (500Gi = 500GiB = 500 * 1024 * 1024 * 1024)
 	// As burst is not supported for HugePages, we would only quota its request, and ignore the limit.
 	ResourceRequestsHugePagesPrefix = "requests.hugepages-"
+	// Default resource requests prefix
+	DefaultResourceRequestsPrefix = "requests."
 )
 
 // A ResourceQuotaScope defines a filter that must match each object tracked by a quota

pkg/apis/core/v1/helper/BUILD

@@ -30,6 +30,7 @@ go_library(
         "//vendor/k8s.io/apimachinery/pkg/labels:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/selection:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/util/validation:go_default_library",
     ],
 )
 

pkg/apis/core/v1/helper/helpers.go

@@ -26,13 +26,25 @@ import (
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/selection"
 	"k8s.io/apimachinery/pkg/util/sets"
+	"k8s.io/apimachinery/pkg/util/validation"
 	"k8s.io/kubernetes/pkg/apis/core/helper"
 )
 
-// IsExtendedResourceName returns true if the resource name is not in the
+// IsExtendedResourceName returns true if:
-// default namespace.
+// 1. the resource name is not in the default namespace;
+// 2. resource name does not have "requests." prefix,
+// to avoid confusion with the convention in quota
+// 3. it satisfies the rules in IsQualifiedName() after converted into quota resource name
 func IsExtendedResourceName(name v1.ResourceName) bool {
-	return !IsDefaultNamespaceResource(name)
+	if IsDefaultNamespaceResource(name) || strings.HasPrefix(string(name), v1.DefaultResourceRequestsPrefix) {
+		return false
+	}
+	// Ensure it satisfies the rules in IsQualifiedName() after converted into quota resource name
+	nameForQuota := fmt.Sprintf("%s%s", v1.DefaultResourceRequestsPrefix, string(name))
+	if errs := validation.IsQualifiedName(string(nameForQuota)); len(errs) != 0 {
+		return false
+	}
+	return true
 }
 
 // IsDefaultNamespaceResource returns true if the resource name is in the

pkg/apis/core/v1/validation/validation.go

@@ -75,6 +75,10 @@ func validateContainerResourceName(value string, fldPath *field.Path) field.Erro
 		if !helper.IsStandardContainerResourceName(value) {
 			return append(allErrs, field.Invalid(fldPath, value, "must be a standard resource for containers"))
 		}
+	} else if !v1helper.IsDefaultNamespaceResource(v1.ResourceName(value)) {
+		if !v1helper.IsExtendedResourceName(v1.ResourceName(value)) {
+			return append(allErrs, field.Invalid(fldPath, value, "doesn't follow extended resource name standard"))
+		}
 	}
 	return allErrs
 }

pkg/apis/core/validation/validation.go

@@ -4084,6 +4084,10 @@ func validateContainerResourceName(value string, fldPath *field.Path) field.Erro
 		if !helper.IsStandardContainerResourceName(value) {
 			return append(allErrs, field.Invalid(fldPath, value, "must be a standard resource for containers"))
 		}
+	} else if !helper.IsDefaultNamespaceResource(core.ResourceName(value)) {
+		if !helper.IsExtendedResourceName(core.ResourceName(value)) {
+			return append(allErrs, field.Invalid(fldPath, value, "doesn't follow extended resource name standard"))
+		}
 	}
 	return allErrs
 }
@@ -4265,7 +4269,8 @@ func ValidateLimitRange(limitRange *core.LimitRange) field.ErrorList {
 				}
 			}
 
-			// for GPU and hugepages, the default value and defaultRequest value must match if both are specified
+			// for GPU, hugepages and other resources that are not allowed to overcommit,
+			// the default value and defaultRequest value must match if both are specified
 			if !helper.IsOvercommitAllowed(core.ResourceName(k)) && defaultQuantityFound && defaultRequestQuantityFound && defaultQuantity.Cmp(defaultRequestQuantity) != 0 {
 				allErrs = append(allErrs, field.Invalid(idxPath.Child("defaultRequest").Key(string(k)), defaultRequestQuantity, fmt.Sprintf("default value %s must equal to defaultRequest value %s in %s", defaultQuantity.String(), defaultRequestQuantity.String(), k)))
 			}

pkg/controller/resourcequota/resource_quota_controller_test.go

@@ -337,6 +337,26 @@ func TestAddQuota(t *testing.T) {
 				},
 			},
 		},
+		{
+			name:             "status, no usage(to validate it works for extended resources)",
+			expectedPriority: true,
+			quota: &v1.ResourceQuota{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "default",
+					Name:      "rq",
+				},
+				Spec: v1.ResourceQuotaSpec{
+					Hard: v1.ResourceList{
+						"requests.example/foobars.example.com": resource.MustParse("4"),
+					},
+				},
+				Status: v1.ResourceQuotaStatus{
+					Hard: v1.ResourceList{
+						"requests.example/foobars.example.com": resource.MustParse("4"),
+					},
+				},
+			},
+		},
 		{
 			name:             "status, mismatch",
 			expectedPriority: true,
@@ -370,12 +390,12 @@ func TestAddQuota(t *testing.T) {
 				},
 				Spec: v1.ResourceQuotaSpec{
 					Hard: v1.ResourceList{
-						"count/foobars.example.com": resource.MustParse("4"),
+						"foobars.example.com": resource.MustParse("4"),
 					},
 				},
 				Status: v1.ResourceQuotaStatus{
 					Hard: v1.ResourceList{
-						"count/foobars.example.com": resource.MustParse("4"),
+						"foobars.example.com": resource.MustParse("4"),
 					},
 				},
 			},

pkg/quota/evaluator/core/pods.go

@@ -31,6 +31,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apiserver/pkg/admission"
 	api "k8s.io/kubernetes/pkg/apis/core"
+	"k8s.io/kubernetes/pkg/apis/core/helper"
 	"k8s.io/kubernetes/pkg/apis/core/helper/qos"
 	k8s_api_v1 "k8s.io/kubernetes/pkg/apis/core/v1"
 	"k8s.io/kubernetes/pkg/kubeapiserver/admission/util"
@@ -69,17 +70,20 @@ var requestedResourcePrefixes = []string{
 	api.ResourceHugePagesPrefix,
 }
 
-const (
-	requestsPrefix = "requests."
-	limitsPrefix   = "limits."
-)
-
 // maskResourceWithPrefix mask resource with certain prefix
 // e.g. hugepages-XXX -> requests.hugepages-XXX
 func maskResourceWithPrefix(resource api.ResourceName, prefix string) api.ResourceName {
 	return api.ResourceName(fmt.Sprintf("%s%s", prefix, string(resource)))
 }
 
+// isExtendedResourceNameForQuota returns true if the extended resource name
+// has the quota related resource prefix.
+func isExtendedResourceNameForQuota(name api.ResourceName) bool {
+	// As overcommit is not supported by extended resources for now,
+	// only quota objects in format of "requests.resourceName" is allowed.
+	return !helper.IsDefaultNamespaceResource(name) && strings.HasPrefix(string(name), api.DefaultResourceRequestsPrefix)
+}
+
 // NOTE: it was a mistake, but if a quota tracks cpu or memory related resources,
 // the incoming pod is required to have those values set.  we should not repeat
 // this mistake for other future resources (gpus, ephemeral-storage,etc).
@@ -164,9 +168,14 @@ func (p *podEvaluator) Matches(resourceQuota *api.ResourceQuota, item runtime.Ob
 func (p *podEvaluator) MatchingResources(input []api.ResourceName) []api.ResourceName {
 	result := quota.Intersection(input, podResources)
 	for _, resource := range input {
+		// for resources with certain prefix, e.g. hugepages
 		if quota.ContainsPrefix(podResourcePrefixes, resource) {
 			result = append(result, resource)
 		}
+		// for extended resources
+		if isExtendedResourceNameForQuota(resource) {
+			result = append(result, resource)
+		}
 	}
 
 	return result
@@ -225,14 +234,15 @@ func podComputeUsageHelper(requests api.ResourceList, limits api.ResourceList) a
 		result[api.ResourceLimitsEphemeralStorage] = limit
 	}
 	for resource, request := range requests {
+		// for resources with certain prefix, e.g. hugepages
 		if quota.ContainsPrefix(requestedResourcePrefixes, resource) {
 			result[resource] = request
-			result[maskResourceWithPrefix(resource, requestsPrefix)] = request
+			result[maskResourceWithPrefix(resource, api.DefaultResourceRequestsPrefix)] = request
 		}
-	}
+		// for extended resources
-	for resource, limit := range limits {
+		if helper.IsExtendedResourceName(resource) {
-		if quota.ContainsPrefix(requestedResourcePrefixes, resource) {
+			// only quota objects in format of "requests.resourceName" is allowed for extended resource.
-			result[maskResourceWithPrefix(resource, limitsPrefix)] = limit
+			result[maskResourceWithPrefix(resource, api.DefaultResourceRequestsPrefix)] = request
 		}
 	}
 

pkg/quota/evaluator/core/pods_test.go

@@ -166,6 +166,23 @@ func TestPodEvaluatorUsage(t *testing.T) {
 				generic.ObjectCountQuotaResourceNameFor(schema.GroupResource{Resource: "pods"}): resource.MustParse("1"),
 			},
 		},
+		"init container extended resources": {
+			pod: &api.Pod{
+				Spec: api.PodSpec{
+					InitContainers: []api.Container{{
+						Resources: api.ResourceRequirements{
+							Requests: api.ResourceList{api.ResourceName("example.com/dongle"): resource.MustParse("3")},
+							Limits:   api.ResourceList{api.ResourceName("example.com/dongle"): resource.MustParse("3")},
+						},
+					}},
+				},
+			},
+			usage: api.ResourceList{
+				api.ResourceName("requests.example.com/dongle"): resource.MustParse("3"),
+				api.ResourcePods:                                resource.MustParse("1"),
+				generic.ObjectCountQuotaResourceNameFor(schema.GroupResource{Resource: "pods"}): resource.MustParse("1"),
+			},
+		},
 		"container CPU": {
 			pod: &api.Pod{
 				Spec: api.PodSpec{
@@ -240,6 +257,23 @@ func TestPodEvaluatorUsage(t *testing.T) {
 				generic.ObjectCountQuotaResourceNameFor(schema.GroupResource{Resource: "pods"}): resource.MustParse("1"),
 			},
 		},
+		"container extended resources": {
+			pod: &api.Pod{
+				Spec: api.PodSpec{
+					Containers: []api.Container{{
+						Resources: api.ResourceRequirements{
+							Requests: api.ResourceList{api.ResourceName("example.com/dongle"): resource.MustParse("3")},
+							Limits:   api.ResourceList{api.ResourceName("example.com/dongle"): resource.MustParse("3")},
+						},
+					}},
+				},
+			},
+			usage: api.ResourceList{
+				api.ResourceName("requests.example.com/dongle"): resource.MustParse("3"),
+				api.ResourcePods:                                resource.MustParse("1"),
+				generic.ObjectCountQuotaResourceNameFor(schema.GroupResource{Resource: "pods"}): resource.MustParse("1"),
+			},
+		},
 		"init container maximums override sum of containers": {
 			pod: &api.Pod{
 				Spec: api.PodSpec{
@@ -249,10 +283,12 @@ func TestPodEvaluatorUsage(t *testing.T) {
 								Requests: api.ResourceList{
 									api.ResourceCPU:                        resource.MustParse("4"),
 									api.ResourceMemory:                     resource.MustParse("100M"),
+									api.ResourceName("example.com/dongle"): resource.MustParse("4"),
 								},
 								Limits: api.ResourceList{
 									api.ResourceCPU:                        resource.MustParse("8"),
 									api.ResourceMemory:                     resource.MustParse("200M"),
+									api.ResourceName("example.com/dongle"): resource.MustParse("4"),
 								},
 							},
 						},
@@ -261,10 +297,12 @@ func TestPodEvaluatorUsage(t *testing.T) {
 								Requests: api.ResourceList{
 									api.ResourceCPU:                        resource.MustParse("1"),
 									api.ResourceMemory:                     resource.MustParse("50M"),
+									api.ResourceName("example.com/dongle"): resource.MustParse("2"),
 								},
 								Limits: api.ResourceList{
 									api.ResourceCPU:                        resource.MustParse("2"),
 									api.ResourceMemory:                     resource.MustParse("100M"),
+									api.ResourceName("example.com/dongle"): resource.MustParse("2"),
 								},
 							},
 						},
@@ -275,10 +313,12 @@ func TestPodEvaluatorUsage(t *testing.T) {
 								Requests: api.ResourceList{
 									api.ResourceCPU:                        resource.MustParse("1"),
 									api.ResourceMemory:                     resource.MustParse("50M"),
+									api.ResourceName("example.com/dongle"): resource.MustParse("1"),
 								},
 								Limits: api.ResourceList{
 									api.ResourceCPU:                        resource.MustParse("2"),
 									api.ResourceMemory:                     resource.MustParse("100M"),
+									api.ResourceName("example.com/dongle"): resource.MustParse("1"),
 								},
 							},
 						},
@@ -287,10 +327,12 @@ func TestPodEvaluatorUsage(t *testing.T) {
 								Requests: api.ResourceList{
 									api.ResourceCPU:                        resource.MustParse("2"),
 									api.ResourceMemory:                     resource.MustParse("25M"),
+									api.ResourceName("example.com/dongle"): resource.MustParse("2"),
 								},
 								Limits: api.ResourceList{
 									api.ResourceCPU:                        resource.MustParse("5"),
 									api.ResourceMemory:                     resource.MustParse("50M"),
+									api.ResourceName("example.com/dongle"): resource.MustParse("2"),
 								},
 							},
 						},
@@ -305,6 +347,7 @@ func TestPodEvaluatorUsage(t *testing.T) {
 				api.ResourcePods:                                                                resource.MustParse("1"),
 				api.ResourceCPU:                                                                 resource.MustParse("4"),
 				api.ResourceMemory:                                                              resource.MustParse("100M"),
+				api.ResourceName("requests.example.com/dongle"):                                 resource.MustParse("4"),
 				generic.ObjectCountQuotaResourceNameFor(schema.GroupResource{Resource: "pods"}): resource.MustParse("1"),
 			},
 		},

staging/src/k8s.io/api/core/v1/types.go

@@ -4745,6 +4745,8 @@ const (
 	// HugePages request, in bytes. (500Gi = 500GiB = 500 * 1024 * 1024 * 1024)
 	// As burst is not supported for HugePages, we would only quota its request, and ignore the limit.
 	ResourceRequestsHugePagesPrefix = "requests.hugepages-"
+	// Default resource requests prefix
+	DefaultResourceRequestsPrefix = "requests."
 )
 
 // A ResourceQuotaScope defines a filter that must match each object tracked by a quota

test/e2e/scheduling/resource_quota.go

@@ -41,6 +41,7 @@ const (
 )
 
 var classGold string = "gold"
+var extendedResourceName string = "example.com/dongle"
 
 var _ = SIGDescribe("ResourceQuota", func() {
 	f := framework.NewDefaultFramework("resourcequota")
@@ -368,9 +369,12 @@ var _ = SIGDescribe("ResourceQuota", func() {
 		By("Creating a Pod that fits quota")
 		podName := "test-pod"
 		requests := v1.ResourceList{}
+		limits := v1.ResourceList{}
 		requests[v1.ResourceCPU] = resource.MustParse("500m")
 		requests[v1.ResourceMemory] = resource.MustParse("252Mi")
-		pod := newTestPodForQuota(f, podName, requests, v1.ResourceList{})
+		requests[v1.ResourceName(extendedResourceName)] = resource.MustParse("2")
+		limits[v1.ResourceName(extendedResourceName)] = resource.MustParse("2")
+		pod := newTestPodForQuota(f, podName, requests, limits)
 		pod, err = f.ClientSet.CoreV1().Pods(f.Namespace.Name).Create(pod)
 		Expect(err).NotTo(HaveOccurred())
 		podToUpdate := pod
@@ -380,6 +384,7 @@ var _ = SIGDescribe("ResourceQuota", func() {
 		usedResources[v1.ResourcePods] = resource.MustParse("1")
 		usedResources[v1.ResourceCPU] = requests[v1.ResourceCPU]
 		usedResources[v1.ResourceMemory] = requests[v1.ResourceMemory]
+		usedResources[v1.ResourceName(v1.DefaultResourceRequestsPrefix+extendedResourceName)] = requests[v1.ResourceName(extendedResourceName)]
 		err = waitForResourceQuota(f.ClientSet, f.Namespace.Name, quotaName, usedResources)
 		Expect(err).NotTo(HaveOccurred())
 
@@ -391,6 +396,17 @@ var _ = SIGDescribe("ResourceQuota", func() {
 		pod, err = f.ClientSet.CoreV1().Pods(f.Namespace.Name).Create(pod)
 		Expect(err).To(HaveOccurred())
 
+		By("Not allowing a pod to be created that exceeds remaining quota(validation on extended resources)")
+		requests = v1.ResourceList{}
+		limits = v1.ResourceList{}
+		requests[v1.ResourceCPU] = resource.MustParse("500m")
+		requests[v1.ResourceMemory] = resource.MustParse("100Mi")
+		requests[v1.ResourceName(extendedResourceName)] = resource.MustParse("2")
+		limits[v1.ResourceName(extendedResourceName)] = resource.MustParse("2")
+		pod = newTestPodForQuota(f, "fail-pod-for-extended-resource", requests, limits)
+		pod, err = f.ClientSet.CoreV1().Pods(f.Namespace.Name).Create(pod)
+		Expect(err).To(HaveOccurred())
+
 		By("Ensuring a pod cannot update its resource requirements")
 		// a pod cannot dynamically update its resource requirements.
 		requests = v1.ResourceList{}
@@ -413,6 +429,7 @@ var _ = SIGDescribe("ResourceQuota", func() {
 		usedResources[v1.ResourcePods] = resource.MustParse("0")
 		usedResources[v1.ResourceCPU] = resource.MustParse("0")
 		usedResources[v1.ResourceMemory] = resource.MustParse("0")
+		usedResources[v1.ResourceName(v1.DefaultResourceRequestsPrefix+extendedResourceName)] = resource.MustParse("0")
 		err = waitForResourceQuota(f.ClientSet, f.Namespace.Name, quotaName, usedResources)
 		Expect(err).NotTo(HaveOccurred())
 	})
@@ -833,6 +850,8 @@ func newTestResourceQuota(name string) *v1.ResourceQuota {
 	hard[core.V1ResourceByStorageClass(classGold, v1.ResourceRequestsStorage)] = resource.MustParse("10Gi")
 	// test quota on discovered resource type
 	hard[v1.ResourceName("count/replicasets.extensions")] = resource.MustParse("5")
+	// test quota on extended resource
+	hard[v1.ResourceName(v1.DefaultResourceRequestsPrefix+extendedResourceName)] = resource.MustParse("3")
 	return &v1.ResourceQuota{
 		ObjectMeta: metav1.ObjectMeta{Name: name},
 		Spec:       v1.ResourceQuotaSpec{Hard: hard},