diff --git a/pkg/apis/core/helper/BUILD b/pkg/apis/core/helper/BUILD index e7ffdb0f82..72df5635cb 100644 --- a/pkg/apis/core/helper/BUILD +++ b/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", ], ) diff --git a/pkg/apis/core/helper/helpers.go b/pkg/apis/core/helper/helpers.go index d0c004cf99..96899f1841 100644 --- a/pkg/apis/core/helper/helpers.go +++ b/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 -// default namespace. +// IsExtendedResourceName returns true if: +// 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 diff --git a/pkg/apis/core/types.go b/pkg/apis/core/types.go index 574bd240c2..cc5c7740d4 100644 --- a/pkg/apis/core/types.go +++ b/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 diff --git a/pkg/apis/core/v1/helper/BUILD b/pkg/apis/core/v1/helper/BUILD index 1e6735f7b9..6c74f416ac 100644 --- a/pkg/apis/core/v1/helper/BUILD +++ b/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", ], ) diff --git a/pkg/apis/core/v1/helper/helpers.go b/pkg/apis/core/v1/helper/helpers.go index b90a6de1f9..bb6d9385f9 100644 --- a/pkg/apis/core/v1/helper/helpers.go +++ b/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 -// default namespace. +// IsExtendedResourceName returns true if: +// 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 diff --git a/pkg/apis/core/v1/validation/validation.go b/pkg/apis/core/v1/validation/validation.go index 8e41b6c963..cba05040c3 100644 --- a/pkg/apis/core/v1/validation/validation.go +++ b/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 } diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index fd183027b1..68b3d0056c 100644 --- a/pkg/apis/core/validation/validation.go +++ b/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))) } diff --git a/pkg/controller/resourcequota/resource_quota_controller_test.go b/pkg/controller/resourcequota/resource_quota_controller_test.go index 6ed7ba9c96..992a946421 100644 --- a/pkg/controller/resourcequota/resource_quota_controller_test.go +++ b/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"), }, }, }, diff --git a/pkg/quota/evaluator/core/pods.go b/pkg/quota/evaluator/core/pods.go index be7d330718..a2d94c96c7 100644 --- a/pkg/quota/evaluator/core/pods.go +++ b/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 resource, limit := range limits { - if quota.ContainsPrefix(requestedResourcePrefixes, resource) { - result[maskResourceWithPrefix(resource, limitsPrefix)] = limit + // for extended resources + if helper.IsExtendedResourceName(resource) { + // only quota objects in format of "requests.resourceName" is allowed for extended resource. + result[maskResourceWithPrefix(resource, api.DefaultResourceRequestsPrefix)] = request } } diff --git a/pkg/quota/evaluator/core/pods_test.go b/pkg/quota/evaluator/core/pods_test.go index 35febe9374..4d0744373f 100644 --- a/pkg/quota/evaluator/core/pods_test.go +++ b/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{ @@ -247,24 +281,28 @@ func TestPodEvaluatorUsage(t *testing.T) { { Resources: api.ResourceRequirements{ Requests: api.ResourceList{ - api.ResourceCPU: resource.MustParse("4"), - api.ResourceMemory: resource.MustParse("100M"), + 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.ResourceCPU: resource.MustParse("8"), + api.ResourceMemory: resource.MustParse("200M"), + api.ResourceName("example.com/dongle"): resource.MustParse("4"), }, }, }, { Resources: api.ResourceRequirements{ Requests: api.ResourceList{ - api.ResourceCPU: resource.MustParse("1"), - api.ResourceMemory: resource.MustParse("50M"), + 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.ResourceCPU: resource.MustParse("2"), + api.ResourceMemory: resource.MustParse("100M"), + api.ResourceName("example.com/dongle"): resource.MustParse("2"), }, }, }, @@ -273,24 +311,28 @@ func TestPodEvaluatorUsage(t *testing.T) { { Resources: api.ResourceRequirements{ Requests: api.ResourceList{ - api.ResourceCPU: resource.MustParse("1"), - api.ResourceMemory: resource.MustParse("50M"), + 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.ResourceCPU: resource.MustParse("2"), + api.ResourceMemory: resource.MustParse("100M"), + api.ResourceName("example.com/dongle"): resource.MustParse("1"), }, }, }, { Resources: api.ResourceRequirements{ Requests: api.ResourceList{ - api.ResourceCPU: resource.MustParse("2"), - api.ResourceMemory: resource.MustParse("25M"), + 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.ResourceCPU: resource.MustParse("5"), + api.ResourceMemory: resource.MustParse("50M"), + api.ResourceName("example.com/dongle"): resource.MustParse("2"), }, }, }, @@ -298,13 +340,14 @@ func TestPodEvaluatorUsage(t *testing.T) { }, }, usage: api.ResourceList{ - api.ResourceRequestsCPU: resource.MustParse("4"), - api.ResourceRequestsMemory: resource.MustParse("100M"), - api.ResourceLimitsCPU: resource.MustParse("8"), - api.ResourceLimitsMemory: resource.MustParse("200M"), - api.ResourcePods: resource.MustParse("1"), - api.ResourceCPU: resource.MustParse("4"), - api.ResourceMemory: resource.MustParse("100M"), + api.ResourceRequestsCPU: resource.MustParse("4"), + api.ResourceRequestsMemory: resource.MustParse("100M"), + api.ResourceLimitsCPU: resource.MustParse("8"), + api.ResourceLimitsMemory: resource.MustParse("200M"), + 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"), }, }, diff --git a/staging/src/k8s.io/api/core/v1/types.go b/staging/src/k8s.io/api/core/v1/types.go index 314816f171..36219b1a2c 100644 --- a/staging/src/k8s.io/api/core/v1/types.go +++ b/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 diff --git a/test/e2e/scheduling/resource_quota.go b/test/e2e/scheduling/resource_quota.go index b06438d112..9c8203d6ad 100644 --- a/test/e2e/scheduling/resource_quota.go +++ b/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},