From 307832dbec129bf6d4cc6d9f7d9bc3c3cbb57d08 Mon Sep 17 00:00:00 2001 From: derekwaynecarr Date: Wed, 17 Aug 2016 15:02:33 -0400 Subject: [PATCH] Quota usage checking ignores unrelated resources --- .../admission/resourcequota/admission_test.go | 50 +++++++++++++++---- .../pkg/admission/resourcequota/controller.go | 4 +- 2 files changed, 42 insertions(+), 12 deletions(-) diff --git a/plugin/pkg/admission/resourcequota/admission_test.go b/plugin/pkg/admission/resourcequota/admission_test.go index 51823c96126..18f74b995fe 100644 --- a/plugin/pkg/admission/resourcequota/admission_test.go +++ b/plugin/pkg/admission/resourcequota/admission_test.go @@ -36,7 +36,6 @@ import ( "k8s.io/kubernetes/pkg/quota/generic" "k8s.io/kubernetes/pkg/quota/install" "k8s.io/kubernetes/pkg/runtime" - utilruntime "k8s.io/kubernetes/pkg/util/runtime" "k8s.io/kubernetes/pkg/util/sets" ) @@ -161,7 +160,6 @@ func TestAdmissionIgnoresSubresources(t *testing.T) { go quotaAccessor.Run(stopCh) evaluator := NewQuotaEvaluator(quotaAccessor, install.NewRegistry(kubeClient), nil, 5, stopCh) - defer utilruntime.HandleCrash() handler := "aAdmission{ Handler: admission.NewHandler(admission.Create, admission.Update), evaluator: evaluator, @@ -205,7 +203,6 @@ func TestAdmitBelowQuotaLimit(t *testing.T) { go quotaAccessor.Run(stopCh) evaluator := NewQuotaEvaluator(quotaAccessor, install.NewRegistry(kubeClient), nil, 5, stopCh) - defer utilruntime.HandleCrash() handler := "aAdmission{ Handler: admission.NewHandler(admission.Create, admission.Update), evaluator: evaluator, @@ -381,7 +378,6 @@ func TestAdmitExceedQuotaLimit(t *testing.T) { go quotaAccessor.Run(stopCh) evaluator := NewQuotaEvaluator(quotaAccessor, install.NewRegistry(kubeClient), nil, 5, stopCh) - defer utilruntime.HandleCrash() handler := "aAdmission{ Handler: admission.NewHandler(admission.Create, admission.Update), evaluator: evaluator, @@ -425,7 +421,6 @@ func TestAdmitEnforceQuotaConstraints(t *testing.T) { go quotaAccessor.Run(stopCh) evaluator := NewQuotaEvaluator(quotaAccessor, install.NewRegistry(kubeClient), nil, 5, stopCh) - defer utilruntime.HandleCrash() handler := "aAdmission{ Handler: admission.NewHandler(admission.Create, admission.Update), evaluator: evaluator, @@ -479,7 +474,6 @@ func TestAdmitPodInNamespaceWithoutQuota(t *testing.T) { go quotaAccessor.Run(stopCh) evaluator := NewQuotaEvaluator(quotaAccessor, install.NewRegistry(kubeClient), nil, 5, stopCh) - defer utilruntime.HandleCrash() handler := "aAdmission{ Handler: admission.NewHandler(admission.Create, admission.Update), evaluator: evaluator, @@ -545,7 +539,6 @@ func TestAdmitBelowTerminatingQuotaLimit(t *testing.T) { go quotaAccessor.Run(stopCh) evaluator := NewQuotaEvaluator(quotaAccessor, install.NewRegistry(kubeClient), nil, 5, stopCh) - defer utilruntime.HandleCrash() handler := "aAdmission{ Handler: admission.NewHandler(admission.Create, admission.Update), evaluator: evaluator, @@ -650,7 +643,6 @@ func TestAdmitBelowBestEffortQuotaLimit(t *testing.T) { go quotaAccessor.Run(stopCh) evaluator := NewQuotaEvaluator(quotaAccessor, install.NewRegistry(kubeClient), nil, 5, stopCh) - defer utilruntime.HandleCrash() handler := "aAdmission{ Handler: admission.NewHandler(admission.Create, admission.Update), evaluator: evaluator, @@ -742,7 +734,6 @@ func TestAdmitBestEffortQuotaLimitIgnoresBurstable(t *testing.T) { go quotaAccessor.Run(stopCh) evaluator := NewQuotaEvaluator(quotaAccessor, install.NewRegistry(kubeClient), nil, 5, stopCh) - defer utilruntime.HandleCrash() handler := "aAdmission{ Handler: admission.NewHandler(admission.Create, admission.Update), evaluator: evaluator, @@ -861,7 +852,6 @@ func TestAdmissionSetsMissingNamespace(t *testing.T) { evaluator := NewQuotaEvaluator(quotaAccessor, install.NewRegistry(kubeClient), nil, 5, stopCh) evaluator.(*quotaEvaluator).registry = registry - defer utilruntime.HandleCrash() handler := "aAdmission{ Handler: admission.NewHandler(admission.Create, admission.Update), evaluator: evaluator, @@ -906,7 +896,6 @@ func TestAdmitRejectsNegativeUsage(t *testing.T) { go quotaAccessor.Run(stopCh) evaluator := NewQuotaEvaluator(quotaAccessor, install.NewRegistry(kubeClient), nil, 5, stopCh) - defer utilruntime.HandleCrash() handler := "aAdmission{ Handler: admission.NewHandler(admission.Create, admission.Update), evaluator: evaluator, @@ -926,3 +915,42 @@ func TestAdmitRejectsNegativeUsage(t *testing.T) { t.Errorf("Unexpected error: %v", err) } } + +// TestAdmitWhenUnrelatedResourceExceedsQuota verifies that if resource X exceeds quota, it does not prohibit resource Y from admission. +func TestAdmitWhenUnrelatedResourceExceedsQuota(t *testing.T) { + resourceQuota := &api.ResourceQuota{ + ObjectMeta: api.ObjectMeta{Name: "quota", Namespace: "test", ResourceVersion: "124"}, + Status: api.ResourceQuotaStatus{ + Hard: api.ResourceList{ + api.ResourceServices: resource.MustParse("3"), + api.ResourcePods: resource.MustParse("4"), + }, + Used: api.ResourceList{ + api.ResourceServices: resource.MustParse("4"), + api.ResourcePods: resource.MustParse("1"), + }, + }, + } + kubeClient := fake.NewSimpleClientset(resourceQuota) + indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{"namespace": cache.MetaNamespaceIndexFunc}) + stopCh := make(chan struct{}) + defer close(stopCh) + + quotaAccessor, _ := newQuotaAccessor(kubeClient) + quotaAccessor.indexer = indexer + go quotaAccessor.Run(stopCh) + evaluator := NewQuotaEvaluator(quotaAccessor, install.NewRegistry(kubeClient), nil, 5, stopCh) + + handler := "aAdmission{ + Handler: admission.NewHandler(admission.Create, admission.Update), + evaluator: evaluator, + } + indexer.Add(resourceQuota) + + // create a pod that should pass existing quota + newPod := validPod("allowed-pod", 1, getResourceRequirements(getResourceList("", ""), getResourceList("", ""))) + err := handler.Admit(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("Unexpected error: %v", err) + } +} diff --git a/plugin/pkg/admission/resourcequota/controller.go b/plugin/pkg/admission/resourcequota/controller.go index 81752684b35..4fc7c9c0849 100644 --- a/plugin/pkg/admission/resourcequota/controller.go +++ b/plugin/pkg/admission/resourcequota/controller.go @@ -382,7 +382,9 @@ func (e *quotaEvaluator) checkRequest(quotas []api.ResourceQuota, a admission.At hardResources := quota.ResourceNames(resourceQuota.Status.Hard) requestedUsage := quota.Mask(deltaUsage, hardResources) newUsage := quota.Add(resourceQuota.Status.Used, requestedUsage) - if allowed, exceeded := quota.LessThanOrEqual(newUsage, resourceQuota.Status.Hard); !allowed { + maskedNewUsage := quota.Mask(newUsage, quota.ResourceNames(requestedUsage)) + + if allowed, exceeded := quota.LessThanOrEqual(maskedNewUsage, resourceQuota.Status.Hard); !allowed { failedRequestedUsage := quota.Mask(requestedUsage, exceeded) failedUsed := quota.Mask(resourceQuota.Status.Used, exceeded) failedHard := quota.Mask(resourceQuota.Status.Hard, exceeded)