Quota usage checking ignores unrelated resources

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 := &quotaAdmission{
 		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 := &quotaAdmission{
 		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 := &quotaAdmission{
 		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 := &quotaAdmission{
 		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 := &quotaAdmission{
 		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 := &quotaAdmission{
 		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 := &quotaAdmission{
 		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 := &quotaAdmission{
 		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 := &quotaAdmission{
 		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 := &quotaAdmission{
 		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 := &quotaAdmission{
+		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)
+	}
+}

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)