diff --git a/pkg/quota/v1/evaluator/core/persistent_volume_claims.go b/pkg/quota/v1/evaluator/core/persistent_volume_claims.go index 059dd6d1544..a4da87b496b 100644 --- a/pkg/quota/v1/evaluator/core/persistent_volume_claims.go +++ b/pkg/quota/v1/evaluator/core/persistent_volume_claims.go @@ -90,14 +90,11 @@ func (p *pvcEvaluator) GroupResource() schema.GroupResource { // Handles returns true if the evaluator should handle the specified operation. func (p *pvcEvaluator) Handles(a admission.Attributes) bool { + if a.GetSubresource() != "" { + return false + } op := a.GetOperation() - if op == admission.Create { - return true - } - if op == admission.Update { - return true - } - return false + return admission.Create == op || admission.Update == op } // Matches returns true if the evaluator matches the specified quota with the provided input item diff --git a/pkg/quota/v1/evaluator/core/persistent_volume_claims_test.go b/pkg/quota/v1/evaluator/core/persistent_volume_claims_test.go index 510028674bf..697f679e6ab 100644 --- a/pkg/quota/v1/evaluator/core/persistent_volume_claims_test.go +++ b/pkg/quota/v1/evaluator/core/persistent_volume_claims_test.go @@ -24,6 +24,7 @@ import ( "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apiserver/pkg/admission" quota "k8s.io/apiserver/pkg/quota/v1" "k8s.io/apiserver/pkg/quota/v1/generic" utilfeature "k8s.io/apiserver/pkg/util/feature" @@ -223,3 +224,52 @@ func TestPersistentVolumeClaimEvaluatorMatchingResources(t *testing.T) { } } } + +func TestPersistentVolumeClaimEvaluatorHandles(t *testing.T) { + evaluator := NewPersistentVolumeClaimEvaluator(nil) + testCases := []struct { + name string + attrs admission.Attributes + want bool + }{ + { + name: "create", + attrs: admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{Group: "core", Version: "v1", Kind: "Pod"}, "", "", schema.GroupVersionResource{Group: "core", Version: "v1", Resource: "pods"}, "", admission.Create, nil, false, nil), + want: true, + }, + { + name: "update", + attrs: admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{Group: "core", Version: "v1", Kind: "Pod"}, "", "", schema.GroupVersionResource{Group: "core", Version: "v1", Resource: "pods"}, "", admission.Update, nil, false, nil), + want: true, + }, + { + name: "delete", + attrs: admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{Group: "core", Version: "v1", Kind: "Pod"}, "", "", schema.GroupVersionResource{Group: "core", Version: "v1", Resource: "pods"}, "", admission.Delete, nil, false, nil), + want: false, + }, + { + name: "connect", + attrs: admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{Group: "core", Version: "v1", Kind: "Pod"}, "", "", schema.GroupVersionResource{Group: "core", Version: "v1", Resource: "pods"}, "", admission.Connect, nil, false, nil), + want: false, + }, + { + name: "create-subresource", + attrs: admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{Group: "core", Version: "v1", Kind: "Pod"}, "", "", schema.GroupVersionResource{Group: "core", Version: "v1", Resource: "pods"}, "subresource", admission.Create, nil, false, nil), + want: false, + }, + { + name: "update-subresource", + attrs: admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{Group: "core", Version: "v1", Kind: "Pod"}, "", "", schema.GroupVersionResource{Group: "core", Version: "v1", Resource: "pods"}, "subresource", admission.Update, nil, false, nil), + want: false, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + actual := evaluator.Handles(tc.attrs) + + if tc.want != actual { + t.Errorf("%s expected:\n%v\n, actual:\n%v", tc.name, tc.want, actual) + } + }) + } +} diff --git a/pkg/quota/v1/evaluator/core/pods.go b/pkg/quota/v1/evaluator/core/pods.go index c82fa98437c..0bdc116b954 100644 --- a/pkg/quota/v1/evaluator/core/pods.go +++ b/pkg/quota/v1/evaluator/core/pods.go @@ -155,14 +155,11 @@ func (p *podEvaluator) GroupResource() schema.GroupResource { // Handles returns true if the evaluator should handle the specified attributes. func (p *podEvaluator) Handles(a admission.Attributes) bool { + if a.GetSubresource() != "" && a.GetSubresource() != "resize" { + return false + } op := a.GetOperation() - if op == admission.Create { - return true - } - if feature.DefaultFeatureGate.Enabled(features.InPlacePodVerticalScaling) && op == admission.Update { - return true - } - return false + return op == admission.Create || (a.GetSubresource() == "resize" && op == admission.Update) } // Matches returns true if the evaluator matches the specified quota with the provided input item diff --git a/pkg/quota/v1/evaluator/core/pods_test.go b/pkg/quota/v1/evaluator/core/pods_test.go index f92e7def0f0..11de6569212 100644 --- a/pkg/quota/v1/evaluator/core/pods_test.go +++ b/pkg/quota/v1/evaluator/core/pods_test.go @@ -27,6 +27,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apiserver/pkg/admission" quota "k8s.io/apiserver/pkg/quota/v1" "k8s.io/apiserver/pkg/quota/v1/generic" utilfeature "k8s.io/apiserver/pkg/util/feature" @@ -1195,3 +1196,58 @@ func makePod(name, pcName string, resList api.ResourceList, phase api.PodPhase) }, } } + +func TestPodEvaluatorHandles(t *testing.T) { + fakeClock := testingclock.NewFakeClock(time.Now()) + evaluator := NewPodEvaluator(nil, fakeClock) + testCases := []struct { + name string + attrs admission.Attributes + want bool + }{ + { + name: "create", + attrs: admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{Group: "core", Version: "v1", Kind: "Pod"}, "", "", schema.GroupVersionResource{Group: "core", Version: "v1", Resource: "pods"}, "", admission.Create, nil, false, nil), + want: true, + }, + { + name: "update", + attrs: admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{Group: "core", Version: "v1", Kind: "Pod"}, "", "", schema.GroupVersionResource{Group: "core", Version: "v1", Resource: "pods"}, "", admission.Update, nil, false, nil), + want: false, + }, + { + name: "delete", + attrs: admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{Group: "core", Version: "v1", Kind: "Pod"}, "", "", schema.GroupVersionResource{Group: "core", Version: "v1", Resource: "pods"}, "", admission.Delete, nil, false, nil), + want: false, + }, + { + name: "connect", + attrs: admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{Group: "core", Version: "v1", Kind: "Pod"}, "", "", schema.GroupVersionResource{Group: "core", Version: "v1", Resource: "pods"}, "", admission.Connect, nil, false, nil), + want: false, + }, + { + name: "create-subresource", + attrs: admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{Group: "core", Version: "v1", Kind: "Pod"}, "", "", schema.GroupVersionResource{Group: "core", Version: "v1", Resource: "pods"}, "subresource", admission.Create, nil, false, nil), + want: false, + }, + { + name: "update-subresource", + attrs: admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{Group: "core", Version: "v1", Kind: "Pod"}, "", "", schema.GroupVersionResource{Group: "core", Version: "v1", Resource: "pods"}, "subresource", admission.Update, nil, false, nil), + want: false, + }, + { + name: "update-resize", + attrs: admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{Group: "core", Version: "v1", Kind: "Pod"}, "", "", schema.GroupVersionResource{Group: "core", Version: "v1", Resource: "pods"}, "resize", admission.Update, nil, false, nil), + want: true, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + actual := evaluator.Handles(tc.attrs) + + if tc.want != actual { + t.Errorf("%s expected:\n%v\n, actual:\n%v", tc.name, tc.want, actual) + } + }) + } +} diff --git a/pkg/quota/v1/evaluator/core/resource_claims.go b/pkg/quota/v1/evaluator/core/resource_claims.go index db594ffb5e2..a55715023bc 100644 --- a/pkg/quota/v1/evaluator/core/resource_claims.go +++ b/pkg/quota/v1/evaluator/core/resource_claims.go @@ -68,14 +68,11 @@ func (p *claimEvaluator) GroupResource() schema.GroupResource { // Handles returns true if the evaluator should handle the specified operation. func (p *claimEvaluator) Handles(a admission.Attributes) bool { + if a.GetSubresource() != "" { + return false + } op := a.GetOperation() - if op == admission.Create { - return true - } - if op == admission.Update { - return true - } - return false + return admission.Create == op || admission.Update == op } // Matches returns true if the evaluator matches the specified quota with the provided input item diff --git a/pkg/quota/v1/evaluator/core/resource_claims_test.go b/pkg/quota/v1/evaluator/core/resource_claims_test.go index 244dbb2c28e..33bbbbbb314 100644 --- a/pkg/quota/v1/evaluator/core/resource_claims_test.go +++ b/pkg/quota/v1/evaluator/core/resource_claims_test.go @@ -24,6 +24,8 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apiserver/pkg/admission" api "k8s.io/kubernetes/pkg/apis/resource" "k8s.io/utils/ptr" ) @@ -171,3 +173,52 @@ func TestResourceClaimEvaluatorMatchingResources(t *testing.T) { }) } } + +func TestResourceClaimEvaluatorHandles(t *testing.T) { + evaluator := NewResourceClaimEvaluator(nil) + testCases := []struct { + name string + attrs admission.Attributes + want bool + }{ + { + name: "create", + attrs: admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{Group: "core", Version: "v1", Kind: "Pod"}, "", "", schema.GroupVersionResource{Group: "core", Version: "v1", Resource: "pods"}, "", admission.Create, nil, false, nil), + want: true, + }, + { + name: "update", + attrs: admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{Group: "core", Version: "v1", Kind: "Pod"}, "", "", schema.GroupVersionResource{Group: "core", Version: "v1", Resource: "pods"}, "", admission.Update, nil, false, nil), + want: true, + }, + { + name: "delete", + attrs: admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{Group: "core", Version: "v1", Kind: "Pod"}, "", "", schema.GroupVersionResource{Group: "core", Version: "v1", Resource: "pods"}, "", admission.Delete, nil, false, nil), + want: false, + }, + { + name: "connect", + attrs: admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{Group: "core", Version: "v1", Kind: "Pod"}, "", "", schema.GroupVersionResource{Group: "core", Version: "v1", Resource: "pods"}, "", admission.Connect, nil, false, nil), + want: false, + }, + { + name: "create-subresource", + attrs: admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{Group: "core", Version: "v1", Kind: "Pod"}, "", "", schema.GroupVersionResource{Group: "core", Version: "v1", Resource: "pods"}, "subresource", admission.Create, nil, false, nil), + want: false, + }, + { + name: "update-subresource", + attrs: admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{Group: "core", Version: "v1", Kind: "Pod"}, "", "", schema.GroupVersionResource{Group: "core", Version: "v1", Resource: "pods"}, "subresource", admission.Update, nil, false, nil), + want: false, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + actual := evaluator.Handles(tc.attrs) + + if tc.want != actual { + t.Errorf("%s expected:\n%v\n, actual:\n%v", tc.name, tc.want, actual) + } + }) + } +} diff --git a/pkg/quota/v1/evaluator/core/services.go b/pkg/quota/v1/evaluator/core/services.go index b681a853ac6..7bba26eeb88 100644 --- a/pkg/quota/v1/evaluator/core/services.go +++ b/pkg/quota/v1/evaluator/core/services.go @@ -67,6 +67,9 @@ func (p *serviceEvaluator) GroupResource() schema.GroupResource { // Handles returns true of the evaluator should handle the specified operation. func (p *serviceEvaluator) Handles(a admission.Attributes) bool { + if a.GetSubresource() != "" { + return false + } 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/v1/evaluator/core/services_test.go b/pkg/quota/v1/evaluator/core/services_test.go index ddc9997ffd3..c36fd2cf9ba 100644 --- a/pkg/quota/v1/evaluator/core/services_test.go +++ b/pkg/quota/v1/evaluator/core/services_test.go @@ -22,6 +22,7 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apiserver/pkg/admission" quota "k8s.io/apiserver/pkg/quota/v1" "k8s.io/apiserver/pkg/quota/v1/generic" api "k8s.io/kubernetes/pkg/apis/core" @@ -330,3 +331,52 @@ func TestServiceConstraintsFunc(t *testing.T) { } } } + +func TestServiceEvaluatorHandles(t *testing.T) { + evaluator := NewServiceEvaluator(nil) + testCases := []struct { + name string + attrs admission.Attributes + want bool + }{ + { + name: "create", + attrs: admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{Group: "core", Version: "v1", Kind: "Pod"}, "", "", schema.GroupVersionResource{Group: "core", Version: "v1", Resource: "pods"}, "", admission.Create, nil, false, nil), + want: true, + }, + { + name: "update", + attrs: admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{Group: "core", Version: "v1", Kind: "Pod"}, "", "", schema.GroupVersionResource{Group: "core", Version: "v1", Resource: "pods"}, "", admission.Update, nil, false, nil), + want: true, + }, + { + name: "delete", + attrs: admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{Group: "core", Version: "v1", Kind: "Pod"}, "", "", schema.GroupVersionResource{Group: "core", Version: "v1", Resource: "pods"}, "", admission.Delete, nil, false, nil), + want: false, + }, + { + name: "connect", + attrs: admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{Group: "core", Version: "v1", Kind: "Pod"}, "", "", schema.GroupVersionResource{Group: "core", Version: "v1", Resource: "pods"}, "", admission.Connect, nil, false, nil), + want: false, + }, + { + name: "create-subresource", + attrs: admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{Group: "core", Version: "v1", Kind: "Pod"}, "", "", schema.GroupVersionResource{Group: "core", Version: "v1", Resource: "pods"}, "subresource", admission.Create, nil, false, nil), + want: false, + }, + { + name: "update-subresource", + attrs: admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{Group: "core", Version: "v1", Kind: "Pod"}, "", "", schema.GroupVersionResource{Group: "core", Version: "v1", Resource: "pods"}, "subresource", admission.Update, nil, false, nil), + want: false, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + actual := evaluator.Handles(tc.attrs) + + if tc.want != actual { + t.Errorf("%s expected:\n%v\n, actual:\n%v", tc.name, tc.want, actual) + } + }) + } +} diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/resourcequota/admission.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/resourcequota/admission.go index 25c266479db..e7d59bb707d 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/resourcequota/admission.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/resourcequota/admission.go @@ -155,10 +155,6 @@ func (a *QuotaAdmission) ValidateInitialization() error { // Validate makes admission decisions while enforcing quota func (a *QuotaAdmission) Validate(ctx context.Context, attr admission.Attributes, o admission.ObjectInterfaces) (err error) { - // ignore all operations that correspond to sub-resource actions - if attr.GetSubresource() != "" { - return nil - } // ignore all operations that are not namespaced or creation of namespaces if attr.GetNamespace() == "" || isNamespaceCreation(attr) { return nil diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/resourcequota/admission_test.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/resourcequota/admission_test.go index efe323a079f..0cc3d7be137 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/resourcequota/admission_test.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/resourcequota/admission_test.go @@ -143,10 +143,6 @@ func TestExcludedOperations(t *testing.T) { desc string attr admission.Attributes }{ - { - "subresource", - admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{}, "namespace", "name", schema.GroupVersionResource{}, "subresource", admission.Create, nil, false, nil), - }, { "non-namespaced resource", admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{}, "", "namespace", schema.GroupVersionResource{}, "", admission.Create, nil, false, nil), diff --git a/staging/src/k8s.io/apiserver/pkg/quota/v1/generic/evaluator.go b/staging/src/k8s.io/apiserver/pkg/quota/v1/generic/evaluator.go index e122248f861..7549a2e561f 100644 --- a/staging/src/k8s.io/apiserver/pkg/quota/v1/generic/evaluator.go +++ b/staging/src/k8s.io/apiserver/pkg/quota/v1/generic/evaluator.go @@ -250,6 +250,9 @@ func (o *objectCountEvaluator) Constraints(required []corev1.ResourceName, item // Handles returns true if the object count evaluator needs to track this attributes. func (o *objectCountEvaluator) Handles(a admission.Attributes) bool { + if a.GetSubresource() != "" { + return false + } operation := a.GetOperation() return operation == admission.Create } diff --git a/staging/src/k8s.io/apiserver/pkg/quota/v1/generic/evaluator_test.go b/staging/src/k8s.io/apiserver/pkg/quota/v1/generic/evaluator_test.go index ce97b1bdf2f..deaa5d74b93 100644 --- a/staging/src/k8s.io/apiserver/pkg/quota/v1/generic/evaluator_test.go +++ b/staging/src/k8s.io/apiserver/pkg/quota/v1/generic/evaluator_test.go @@ -22,6 +22,8 @@ import ( "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apiserver/pkg/admission" "k8s.io/client-go/tools/cache" ) @@ -129,3 +131,52 @@ func (f *fakeLister) Get(name string) (runtime.Object, error) { func (f *fakeLister) ByNamespace(namespace string) cache.GenericNamespaceLister { panic("not implemented") } + +func TestObjectCountEvaluatorHandles(t *testing.T) { + evaluator := objectCountEvaluator{} + testCases := []struct { + name string + attrs admission.Attributes + want bool + }{ + { + name: "create", + attrs: admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{Group: "core", Version: "v1", Kind: "Pod"}, "", "", schema.GroupVersionResource{Group: "core", Version: "v1", Resource: "pods"}, "", admission.Create, nil, false, nil), + want: true, + }, + { + name: "update", + attrs: admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{Group: "core", Version: "v1", Kind: "Pod"}, "", "", schema.GroupVersionResource{Group: "core", Version: "v1", Resource: "pods"}, "", admission.Update, nil, false, nil), + want: false, + }, + { + name: "delete", + attrs: admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{Group: "core", Version: "v1", Kind: "Pod"}, "", "", schema.GroupVersionResource{Group: "core", Version: "v1", Resource: "pods"}, "", admission.Delete, nil, false, nil), + want: false, + }, + { + name: "connect", + attrs: admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{Group: "core", Version: "v1", Kind: "Pod"}, "", "", schema.GroupVersionResource{Group: "core", Version: "v1", Resource: "pods"}, "", admission.Connect, nil, false, nil), + want: false, + }, + { + name: "create-subresource", + attrs: admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{Group: "core", Version: "v1", Kind: "Pod"}, "", "", schema.GroupVersionResource{Group: "core", Version: "v1", Resource: "pods"}, "subresource", admission.Create, nil, false, nil), + want: false, + }, + { + name: "update-subresource", + attrs: admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{Group: "core", Version: "v1", Kind: "Pod"}, "", "", schema.GroupVersionResource{Group: "core", Version: "v1", Resource: "pods"}, "subresource", admission.Update, nil, false, nil), + want: false, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + actual := evaluator.Handles(tc.attrs) + + if tc.want != actual { + t.Errorf("%s expected:\n%v\n, actual:\n%v", tc.name, tc.want, actual) + } + }) + } +} diff --git a/test/e2e/node/pod_resize.go b/test/e2e/node/pod_resize.go index 9680d3c3fd4..a9035237c00 100644 --- a/test/e2e/node/pod_resize.go +++ b/test/e2e/node/pod_resize.go @@ -122,7 +122,7 @@ func doPodResizeResourceQuotaTests(f *framework.Framework) { ginkgo.By(fmt.Sprintf("patching pod %s for resize with CPU exceeding resource quota", resizedPod.Name)) _, pErrExceedCPU := f.ClientSet.CoreV1().Pods(resizedPod.Namespace).Patch(ctx, - resizedPod.Name, types.StrategicMergePatchType, []byte(patchStringExceedCPU), metav1.PatchOptions{}) + resizedPod.Name, types.StrategicMergePatchType, []byte(patchStringExceedCPU), metav1.PatchOptions{}, "resize") gomega.Expect(pErrExceedCPU).To(gomega.HaveOccurred(), "exceeded quota: %s, requested: cpu=200m, used: cpu=700m, limited: cpu=800m", resourceQuota.Name)