From 208df08ea6648b295bb394ef7a14ad0a442f6672 Mon Sep 17 00:00:00 2001 From: Cao Shufeng Date: Mon, 11 Dec 2017 16:42:31 +0800 Subject: [PATCH] remove useless validation from pod's resourcequota admission ResourceQuota is a validating admission plugin. Before it runs, pods has already been validated. It's not necessary to validate it again. --- pkg/quota/evaluator/core/BUILD | 2 -- pkg/quota/evaluator/core/pods.go | 19 -------------- pkg/quota/evaluator/core/pods_test.go | 26 ------------------- .../admission/resourcequota/admission_test.go | 6 ----- 4 files changed, 53 deletions(-) diff --git a/pkg/quota/evaluator/core/BUILD b/pkg/quota/evaluator/core/BUILD index 85d50d0855..00962bb02f 100644 --- a/pkg/quota/evaluator/core/BUILD +++ b/pkg/quota/evaluator/core/BUILD @@ -21,7 +21,6 @@ go_library( "//pkg/apis/core/helper:go_default_library", "//pkg/apis/core/helper/qos:go_default_library", "//pkg/apis/core/v1:go_default_library", - "//pkg/apis/core/validation:go_default_library", "//pkg/features:go_default_library", "//pkg/kubeapiserver/admission/util:go_default_library", "//pkg/quota:go_default_library", @@ -34,7 +33,6 @@ go_library( "//vendor/k8s.io/apimachinery/pkg/util/initialization:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/runtime:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library", - "//vendor/k8s.io/apimachinery/pkg/util/validation/field:go_default_library", "//vendor/k8s.io/apiserver/pkg/admission:go_default_library", "//vendor/k8s.io/apiserver/pkg/features:go_default_library", "//vendor/k8s.io/apiserver/pkg/util/feature:go_default_library", diff --git a/pkg/quota/evaluator/core/pods.go b/pkg/quota/evaluator/core/pods.go index ba935eab5a..be7d330718 100644 --- a/pkg/quota/evaluator/core/pods.go +++ b/pkg/quota/evaluator/core/pods.go @@ -29,12 +29,10 @@ import ( "k8s.io/apimachinery/pkg/util/clock" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/sets" - "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/qos" k8s_api_v1 "k8s.io/kubernetes/pkg/apis/core/v1" - "k8s.io/kubernetes/pkg/apis/core/validation" "k8s.io/kubernetes/pkg/kubeapiserver/admission/util" "k8s.io/kubernetes/pkg/quota" "k8s.io/kubernetes/pkg/quota/generic" @@ -118,23 +116,6 @@ func (p *podEvaluator) Constraints(required []api.ResourceName, item runtime.Obj return fmt.Errorf("Unexpected input object %v", item) } - // Pod level resources are often set during admission control - // As a consequence, we want to verify that resources are valid prior - // to ever charging quota prematurely in case they are not. - // TODO remove this entire section when we have a validation step in admission. - allErrs := field.ErrorList{} - fldPath := field.NewPath("spec").Child("containers") - for i, ctr := range pod.Spec.Containers { - allErrs = append(allErrs, validation.ValidateResourceRequirements(&ctr.Resources, fldPath.Index(i).Child("resources"))...) - } - fldPath = field.NewPath("spec").Child("initContainers") - for i, ctr := range pod.Spec.InitContainers { - allErrs = append(allErrs, validation.ValidateResourceRequirements(&ctr.Resources, fldPath.Index(i).Child("resources"))...) - } - if len(allErrs) > 0 { - return allErrs.ToAggregate() - } - // BACKWARD COMPATIBILITY REQUIREMENT: if we quota cpu or memory, then each container // must make an explicit request for the resource. this was a mistake. it coupled // validation with resource counting, but we did this before QoS was even defined. diff --git a/pkg/quota/evaluator/core/pods_test.go b/pkg/quota/evaluator/core/pods_test.go index 2c06bdcb4b..35febe9374 100644 --- a/pkg/quota/evaluator/core/pods_test.go +++ b/pkg/quota/evaluator/core/pods_test.go @@ -36,32 +36,6 @@ func TestPodConstraintsFunc(t *testing.T) { required []api.ResourceName err string }{ - "init container resource invalid": { - pod: &api.Pod{ - Spec: api.PodSpec{ - InitContainers: []api.Container{{ - Resources: api.ResourceRequirements{ - Requests: api.ResourceList{api.ResourceCPU: resource.MustParse("2m")}, - Limits: api.ResourceList{api.ResourceCPU: resource.MustParse("1m")}, - }, - }}, - }, - }, - err: `spec.initContainers[0].resources.requests: Invalid value: "2m": must be less than or equal to cpu limit`, - }, - "container resource invalid": { - pod: &api.Pod{ - Spec: api.PodSpec{ - Containers: []api.Container{{ - Resources: api.ResourceRequirements{ - Requests: api.ResourceList{api.ResourceCPU: resource.MustParse("2m")}, - Limits: api.ResourceList{api.ResourceCPU: resource.MustParse("1m")}, - }, - }}, - }, - }, - err: `spec.containers[0].resources.requests: Invalid value: "2m": must be less than or equal to cpu limit`, - }, "init container resource missing": { pod: &api.Pod{ Spec: api.PodSpec{ diff --git a/plugin/pkg/admission/resourcequota/admission_test.go b/plugin/pkg/admission/resourcequota/admission_test.go index 755d0f5d00..09855f2ead 100644 --- a/plugin/pkg/admission/resourcequota/admission_test.go +++ b/plugin/pkg/admission/resourcequota/admission_test.go @@ -724,12 +724,6 @@ func TestAdmitEnforceQuotaConstraints(t *testing.T) { if err == nil { t.Errorf("Expected an error because the pod does not specify a memory limit") } - // verify the requests and limits are actually valid (in this case, we fail because the limits < requests) - newPod = validPod("not-allowed-pod", 1, getResourceRequirements(getResourceList("200m", "2Gi"), getResourceList("100m", "1Gi"))) - err = handler.Validate(admission.NewAttributesRecord(newPod, nil, api.Kind("Pod").WithVersion("version"), newPod.Namespace, newPod.Name, api.Resource("pods").WithVersion("version"), "", admission.Create, nil)) - if err == nil { - t.Errorf("Expected an error because the pod does not specify a memory limit") - } } // TestAdmitPodInNamespaceWithoutQuota ensures that if a namespace has no quota, that a pod can get in