From fde4f6f9b04fc94a262b609180e1222c46329f90 Mon Sep 17 00:00:00 2001 From: lichuqiang Date: Tue, 30 Jan 2018 09:29:22 +0800 Subject: [PATCH] resourceQuota support for extended resources --- pkg/apis/core/helper/BUILD | 1 + pkg/apis/core/helper/helpers.go | 18 +++- pkg/apis/core/types.go | 2 + pkg/apis/core/v1/helper/BUILD | 1 + pkg/apis/core/v1/helper/helpers.go | 18 +++- pkg/apis/core/v1/validation/validation.go | 4 + pkg/apis/core/validation/validation.go | 7 +- .../resource_quota_controller_test.go | 24 ++++- pkg/quota/evaluator/core/pods.go | 30 ++++--- pkg/quota/evaluator/core/pods_test.go | 89 ++++++++++++++----- staging/src/k8s.io/api/core/v1/types.go | 2 + 11 files changed, 154 insertions(+), 42 deletions(-) diff --git a/pkg/apis/core/helper/BUILD b/pkg/apis/core/helper/BUILD index 81b0a2fbff..1225a57fa1 100644 --- a/pkg/apis/core/helper/BUILD +++ b/pkg/apis/core/helper/BUILD @@ -31,6 +31,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 45dfd90421..6608633f06 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 b6b570b06e..8835334ea0 100644 --- a/pkg/apis/core/types.go +++ b/pkg/apis/core/types.go @@ -4159,6 +4159,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 ea7e2ae974..6832f34b34 100644 --- a/pkg/apis/core/v1/helper/BUILD +++ b/pkg/apis/core/v1/helper/BUILD @@ -31,6 +31,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 4b21aefcfc..ace6fbe124 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 655d0bf216..9e5e788dc6 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -3998,6 +3998,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 } @@ -4179,7 +4183,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 ba935eab5a..7f8966a192 100644 --- a/pkg/quota/evaluator/core/pods.go +++ b/pkg/quota/evaluator/core/pods.go @@ -32,6 +32,7 @@ import ( "k8s.io/apimachinery/pkg/util/validation/field" "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/apis/core/validation" @@ -71,17 +72,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). @@ -183,9 +187,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 @@ -244,14 +253,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 2c06bdcb4b..65653d31c6 100644 --- a/pkg/quota/evaluator/core/pods_test.go +++ b/pkg/quota/evaluator/core/pods_test.go @@ -192,6 +192,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{ @@ -266,6 +283,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{ @@ -273,24 +307,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"), }, }, }, @@ -299,24 +337,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"), }, }, }, @@ -324,13 +366,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 728cbd5a62..73def3ef26 100644 --- a/staging/src/k8s.io/api/core/v1/types.go +++ b/staging/src/k8s.io/api/core/v1/types.go @@ -4684,6 +4684,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