diff --git a/pkg/quota/evaluator/core/BUILD b/pkg/quota/evaluator/core/BUILD index efc0798d80a..b413ddac201 100644 --- a/pkg/quota/evaluator/core/BUILD +++ b/pkg/quota/evaluator/core/BUILD @@ -25,6 +25,7 @@ go_library( "//pkg/api/helper/qos:go_default_library", "//pkg/api/v1:go_default_library", "//pkg/api/validation:go_default_library", + "//pkg/kubeapiserver/admission/util:go_default_library", "//pkg/quota:go_default_library", "//pkg/quota/generic:go_default_library", "//vendor/k8s.io/api/core/v1:go_default_library", diff --git a/pkg/quota/evaluator/core/persistent_volume_claims.go b/pkg/quota/evaluator/core/persistent_volume_claims.go index 86e7350dee7..ed2d8bef0d0 100644 --- a/pkg/quota/evaluator/core/persistent_volume_claims.go +++ b/pkg/quota/evaluator/core/persistent_volume_claims.go @@ -32,6 +32,7 @@ import ( "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/api/helper" k8s_api_v1 "k8s.io/kubernetes/pkg/api/v1" + "k8s.io/kubernetes/pkg/kubeapiserver/admission/util" "k8s.io/kubernetes/pkg/quota" "k8s.io/kubernetes/pkg/quota/generic" ) @@ -141,8 +142,18 @@ func (p *pvcEvaluator) GroupKind() schema.GroupKind { } // Handles returns true if the evaluator should handle the specified operation. -func (p *pvcEvaluator) Handles(operation admission.Operation) bool { - return admission.Create == operation +func (p *pvcEvaluator) Handles(a admission.Attributes) bool { + op := a.GetOperation() + if op == admission.Create { + return true + } + updateUninitialized, err := util.IsUpdatingUninitializedObject(a) + if err != nil { + // fail closed, will try to give an evaluation. + return true + } + // only uninitialized pvc might be updated. + return updateUninitialized } // Matches returns true if the evaluator matches the specified quota with the provided input item diff --git a/pkg/quota/evaluator/core/pods.go b/pkg/quota/evaluator/core/pods.go index 5773a4a229d..da398860df7 100644 --- a/pkg/quota/evaluator/core/pods.go +++ b/pkg/quota/evaluator/core/pods.go @@ -34,6 +34,7 @@ import ( "k8s.io/kubernetes/pkg/api/helper/qos" k8s_api_v1 "k8s.io/kubernetes/pkg/api/v1" "k8s.io/kubernetes/pkg/api/validation" + "k8s.io/kubernetes/pkg/kubeapiserver/admission/util" "k8s.io/kubernetes/pkg/quota" "k8s.io/kubernetes/pkg/quota/generic" ) @@ -131,10 +132,19 @@ func (p *podEvaluator) GroupKind() schema.GroupKind { return api.Kind("Pod") } -// Handles returns true of the evaluator should handle the specified operation. -func (p *podEvaluator) Handles(operation admission.Operation) bool { - // TODO: update this if/when pods support resizing resource requirements. - return admission.Create == operation +// Handles returns true of the evaluator should handle the specified attributes. +func (p *podEvaluator) Handles(a admission.Attributes) bool { + op := a.GetOperation() + if op == admission.Create { + return true + } + updateUninitialized, err := util.IsUpdatingUninitializedObject(a) + if err != nil { + // fail closed, will try to give an evaluation. + return true + } + // only uninitialized pods might be updated. + return updateUninitialized } // Matches returns true if the evaluator matches the specified quota with the provided input item diff --git a/pkg/quota/evaluator/core/services.go b/pkg/quota/evaluator/core/services.go index 457d8b41735..91a5ef0a059 100644 --- a/pkg/quota/evaluator/core/services.go +++ b/pkg/quota/evaluator/core/services.go @@ -108,7 +108,8 @@ func (p *serviceEvaluator) GroupKind() schema.GroupKind { } // Handles returns true of the evaluator should handle the specified operation. -func (p *serviceEvaluator) Handles(operation admission.Operation) bool { +func (p *serviceEvaluator) Handles(a admission.Attributes) bool { + operation := a.GetOperation() // We handle create and update because a service type can change. return admission.Create == operation || admission.Update == operation } diff --git a/pkg/quota/generic/evaluator.go b/pkg/quota/generic/evaluator.go index ab3bbb531b4..1b574bb6b52 100644 --- a/pkg/quota/generic/evaluator.go +++ b/pkg/quota/generic/evaluator.go @@ -150,8 +150,9 @@ func (o *ObjectCountEvaluator) GroupKind() schema.GroupKind { return o.InternalGroupKind } -// Handles returns true if the object count evaluator needs to track this operation. -func (o *ObjectCountEvaluator) Handles(operation admission.Operation) bool { +// Handles returns true if the object count evaluator needs to track this attributes. +func (o *ObjectCountEvaluator) Handles(a admission.Attributes) bool { + operation := a.GetOperation() return operation == admission.Create || (o.AllowCreateOnUpdate && operation == admission.Update) } diff --git a/pkg/quota/interfaces.go b/pkg/quota/interfaces.go index 393e89791b5..56eacfe2d47 100644 --- a/pkg/quota/interfaces.go +++ b/pkg/quota/interfaces.go @@ -45,9 +45,9 @@ type Evaluator interface { Constraints(required []api.ResourceName, item runtime.Object) error // GroupKind returns the groupKind that this object knows how to evaluate GroupKind() schema.GroupKind - // Handles determines if quota could be impacted by the specified operation. + // Handles determines if quota could be impacted by the specified attribute. // If true, admission control must perform quota processing for the operation, otherwise it is safe to ignore quota. - Handles(operation admission.Operation) bool + Handles(operation admission.Attributes) bool // Matches returns true if the specified quota matches the input item Matches(resourceQuota *api.ResourceQuota, item runtime.Object) (bool, error) // MatchingResources takes the input specified list of resources and returns the set of resources evaluator matches. diff --git a/plugin/pkg/admission/resourcequota/controller.go b/plugin/pkg/admission/resourcequota/controller.go index ab32bbf31ba..84f196ba60d 100644 --- a/plugin/pkg/admission/resourcequota/controller.go +++ b/plugin/pkg/admission/resourcequota/controller.go @@ -375,8 +375,7 @@ func (e *quotaEvaluator) checkRequest(quotas []api.ResourceQuota, a admission.At return quotas, nil } - op := a.GetOperation() - if !evaluator.Handles(op) { + if !evaluator.Handles(a) { return quotas, nil } @@ -463,7 +462,7 @@ func (e *quotaEvaluator) checkRequest(quotas []api.ResourceQuota, a admission.At return nil, admission.NewForbidden(a, fmt.Errorf("quota usage is negative for resource(s): %s", prettyPrintResourceNames(negativeUsage))) } - if admission.Update == op { + if admission.Update == a.GetOperation() { prevItem := a.GetOldObject() if prevItem == nil { return nil, admission.NewForbidden(a, fmt.Errorf("unable to get previous usage since prior version of object was not found")) @@ -529,8 +528,7 @@ func (e *quotaEvaluator) Evaluate(a admission.Attributes) error { } // for this kind, check if the operation could mutate any quota resources // if no resources tracked by quota are impacted, then just return - op := a.GetOperation() - if !evaluator.Handles(op) { + if !evaluator.Handles(a) { return nil } diff --git a/test/e2e/resource_quota.go b/test/e2e/resource_quota.go index c666a54406d..e70f8fb65b9 100644 --- a/test/e2e/resource_quota.go +++ b/test/e2e/resource_quota.go @@ -147,6 +147,85 @@ var _ = framework.KubeDescribe("ResourceQuota", func() { Expect(err).NotTo(HaveOccurred()) }) + It("should create a ResourceQuota and capture the life of an uninitialized pod.", func() { + // TODO: uncomment the test when #50344 is merged. + // By("Creating a ResourceQuota") + // quotaName := "test-quota" + // resourceQuota := newTestResourceQuota(quotaName) + // resourceQuota, err := createResourceQuota(f.ClientSet, f.Namespace.Name, resourceQuota) + // Expect(err).NotTo(HaveOccurred()) + + // By("Ensuring resource quota status is calculated") + // usedResources := v1.ResourceList{} + // usedResources[v1.ResourceQuotas] = resource.MustParse("1") + // err = waitForResourceQuota(f.ClientSet, f.Namespace.Name, quotaName, usedResources) + // Expect(err).NotTo(HaveOccurred()) + + // By("Creating an uninitialized Pod that fits quota") + // podName := "test-pod" + // requests := v1.ResourceList{} + // requests[v1.ResourceCPU] = resource.MustParse("500m") + // requests[v1.ResourceMemory] = resource.MustParse("252Mi") + // pod := newTestPodForQuota(f, podName, requests, v1.ResourceList{}) + // pod.Initializers = &metav1.Initializers{Pending: []metav1.Initializer{{Name: "unhandled"}}} + // _, err = f.ClientSet.Core().Pods(f.Namespace.Name).Create(pod) + // // because no one is handling the initializer, server will return a 504 timeout + // if err != nil && !errors.IsTimeout(err) { + // framework.Failf("expect err to be timeout error, got %v", err) + // } + // podToUpdate, err := f.ClientSet.Core().Pods(f.Namespace.Name).Get(podName, metav1.GetOptions{}) + // Expect(err).NotTo(HaveOccurred()) + + // By("Ensuring ResourceQuota status captures the pod usage") + // usedResources[v1.ResourceQuotas] = resource.MustParse("1") + // usedResources[v1.ResourcePods] = resource.MustParse("1") + // usedResources[v1.ResourceCPU] = requests[v1.ResourceCPU] + // usedResources[v1.ResourceMemory] = requests[v1.ResourceMemory] + // err = waitForResourceQuota(f.ClientSet, f.Namespace.Name, quotaName, usedResources) + // Expect(err).NotTo(HaveOccurred()) + + // By("Not allowing an uninitialized pod to be created that exceeds remaining quota") + // requests = v1.ResourceList{} + // requests[v1.ResourceCPU] = resource.MustParse("600m") + // requests[v1.ResourceMemory] = resource.MustParse("100Mi") + // pod = newTestPodForQuota(f, "fail-pod", requests, v1.ResourceList{}) + // pod.Initializers = &metav1.Initializers{Pending: []metav1.Initializer{{Name: "unhandled"}}} + // pod, err = f.ClientSet.Core().Pods(f.Namespace.Name).Create(pod) + // Expect(err).To(HaveOccurred()) + // fmt.Println("CHAO: err=", err) + + // By("Ensuring an uninitialized pod can update its resource requirements") + // // a pod cannot dynamically update its resource requirements. + // requests = v1.ResourceList{} + // requests[v1.ResourceCPU] = resource.MustParse("100m") + // requests[v1.ResourceMemory] = resource.MustParse("100Mi") + // podToUpdate.Spec.Containers[0].Resources.Requests = requests + // _, err = f.ClientSet.Core().Pods(f.Namespace.Name).Update(podToUpdate) + // Expect(err).NotTo(HaveOccurred()) + + // By("Ensuring attempts to update pod resource requirements did change quota usage") + // usedResources[v1.ResourceQuotas] = resource.MustParse("1") + // usedResources[v1.ResourcePods] = resource.MustParse("1") + // usedResources[v1.ResourceCPU] = requests[v1.ResourceCPU] + // usedResources[v1.ResourceMemory] = requests[v1.ResourceMemory] + // err = waitForResourceQuota(f.ClientSet, f.Namespace.Name, quotaName, usedResources) + // Expect(err).NotTo(HaveOccurred()) + + // TODO: uncomment the test when the replenishment_controller uses the + // sharedInformer that list/watches uninitialized objects. + // By("Deleting the pod") + // err = f.ClientSet.Core().Pods(f.Namespace.Name).Delete(podName, metav1.NewDeleteOptions(0)) + // Expect(err).NotTo(HaveOccurred()) + // + // By("Ensuring resource quota status released the pod usage") + // usedResources[v1.ResourceQuotas] = resource.MustParse("1") + // usedResources[v1.ResourcePods] = resource.MustParse("0") + // usedResources[v1.ResourceCPU] = resource.MustParse("0") + // usedResources[v1.ResourceMemory] = resource.MustParse("0") + // err = waitForResourceQuota(f.ClientSet, f.Namespace.Name, quotaName, usedResources) + // Expect(err).NotTo(HaveOccurred()) + }) + It("should create a ResourceQuota and capture the life of a pod.", func() { By("Creating a ResourceQuota") quotaName := "test-quota"