Merge pull request #17061 from derekwaynecarr/quota_dirty_error

plugin/pkg/admission/resourcequota/admission.go

@@ -168,6 +168,12 @@ func (q *quota) Admit(a admission.Attributes) (err error) {
 // Return true if the usage must be recorded prior to admitting the new resource
 // Return an error if the operation should not pass admission control
 func IncrementUsage(a admission.Attributes, status *api.ResourceQuotaStatus, client client.Interface) (bool, error) {
+	// on update, the only resource that can modify the value of a quota is pods
+	// so if your not a pod, we exit quickly
+	if a.GetOperation() == admission.Update && a.GetResource() != "pods" {
+		return false, nil
+	}
+
 	var errs []error
 	dirty := true
 	set := map[api.ResourceName]bool{}

plugin/pkg/admission/resourcequota/admission_test.go

@@ -26,6 +26,7 @@ import (
 	"k8s.io/kubernetes/pkg/client/cache"
 	"k8s.io/kubernetes/pkg/client/unversioned/testclient"
 	resourcequotacontroller "k8s.io/kubernetes/pkg/controller/resourcequota"
+	"k8s.io/kubernetes/pkg/runtime"
 )
 
 func getResourceList(cpu, memory string) api.ResourceList {
@@ -388,3 +389,59 @@ func TestExceedUsagePersistentVolumeClaims(t *testing.T) {
 		t.Errorf("Expected error for exceeding hard limits")
 	}
 }
+
+func TestIncrementUsageOnUpdateIgnoresNonPodResources(t *testing.T) {
+	testCase := []struct {
+		kind        string
+		resource    string
+		subresource string
+		object      runtime.Object
+	}{
+		{
+			kind:     "Service",
+			resource: "services",
+			object:   &api.Service{},
+		},
+		{
+			kind:     "ReplicationController",
+			resource: "replicationcontrollers",
+			object:   &api.ReplicationController{},
+		},
+		{
+			kind:     "ResourceQuota",
+			resource: "resourcequotas",
+			object:   &api.ResourceQuota{},
+		},
+		{
+			kind:     "Secret",
+			resource: "secrets",
+			object:   &api.Secret{},
+		},
+		{
+			kind:     "PersistentVolumeClaim",
+			resource: "persistentvolumeclaims",
+			object:   &api.PersistentVolumeClaim{},
+		},
+	}
+
+	for _, testCase := range testCase {
+		client := testclient.NewSimpleFake()
+		status := &api.ResourceQuotaStatus{
+			Hard: api.ResourceList{},
+			Used: api.ResourceList{},
+		}
+		r := api.ResourceName(testCase.resource)
+		status.Hard[r] = resource.MustParse("2")
+		status.Used[r] = resource.MustParse("1")
+
+		attributesRecord := admission.NewAttributesRecord(testCase.object, testCase.kind, "my-ns", "new-thing",
+			testCase.resource, testCase.subresource, admission.Update, nil)
+		dirty, err := IncrementUsage(attributesRecord, status, client)
+		if err != nil {
+			t.Errorf("Increment usage of resource %v had unexpected error: %v", testCase.resource, err)
+		}
+		if dirty {
+			t.Errorf("Increment usage of resource %v should not result in a dirty quota on update", testCase.resource)
+		}
+	}
+}