From fe4cac73c83ad0fd1460df85a264f9f70307b647 Mon Sep 17 00:00:00 2001 From: ravisantoshgudimetla Date: Mon, 4 Nov 2019 17:26:16 -0500 Subject: [PATCH] Relax namespace restriction for critical pods --- pkg/registry/scheduling/rest/BUILD | 2 ++ plugin/pkg/admission/priority/admission.go | 26 ++++++++++++++----- .../pkg/admission/priority/admission_test.go | 6 +++-- plugin/pkg/admission/resourcequota/BUILD | 1 - test/integration/quota/BUILD | 3 +++ 5 files changed, 28 insertions(+), 10 deletions(-) diff --git a/pkg/registry/scheduling/rest/BUILD b/pkg/registry/scheduling/rest/BUILD index 08e14847ff4..2cba35ebb5c 100644 --- a/pkg/registry/scheduling/rest/BUILD +++ b/pkg/registry/scheduling/rest/BUILD @@ -16,6 +16,7 @@ go_library( "//pkg/apis/scheduling/v1alpha1:go_default_library", "//pkg/apis/scheduling/v1beta1:go_default_library", "//pkg/registry/scheduling/priorityclass/storage:go_default_library", + "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/runtime:go_default_library", @@ -24,6 +25,7 @@ go_library( "//staging/src/k8s.io/apiserver/pkg/registry/rest:go_default_library", "//staging/src/k8s.io/apiserver/pkg/server:go_default_library", "//staging/src/k8s.io/apiserver/pkg/server/storage:go_default_library", + "//staging/src/k8s.io/client-go/kubernetes:go_default_library", "//staging/src/k8s.io/client-go/kubernetes/typed/scheduling/v1:go_default_library", "//vendor/k8s.io/klog:go_default_library", ], diff --git a/plugin/pkg/admission/priority/admission.go b/plugin/pkg/admission/priority/admission.go index b11d0d63879..87318394af0 100644 --- a/plugin/pkg/admission/priority/admission.go +++ b/plugin/pkg/admission/priority/admission.go @@ -28,10 +28,10 @@ import ( "k8s.io/apimachinery/pkg/labels" "k8s.io/apiserver/pkg/admission" genericadmissioninitializers "k8s.io/apiserver/pkg/admission/initializer" - utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes" schedulingv1listers "k8s.io/client-go/listers/scheduling/v1" + "k8s.io/component-base/featuregate" "k8s.io/kubernetes/pkg/apis/core" api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/scheduling" @@ -54,12 +54,15 @@ func Register(plugins *admission.Plugins) { // Plugin is an implementation of admission.Interface. type Plugin struct { *admission.Handler - client kubernetes.Interface - lister schedulingv1listers.PriorityClassLister + client kubernetes.Interface + lister schedulingv1listers.PriorityClassLister + resourceQuotaFeatureGateEnabled bool + nonPreemptingPriority bool } var _ admission.MutationInterface = &Plugin{} var _ admission.ValidationInterface = &Plugin{} +var _ genericadmissioninitializers.WantsFeatures = &Plugin{} var _ = genericadmissioninitializers.WantsExternalKubeInformerFactory(&Plugin{}) var _ = genericadmissioninitializers.WantsExternalKubeClientSet(&Plugin{}) @@ -81,6 +84,12 @@ func (p *Plugin) ValidateInitialization() error { return nil } +// InspectFeatureGates allows setting bools without taking a dep on a global variable +func (p *Plugin) InspectFeatureGates(featureGates featuregate.FeatureGate) { + p.nonPreemptingPriority = featureGates.Enabled(features.NonPreemptingPriority) + p.resourceQuotaFeatureGateEnabled = featureGates.Enabled(features.ResourceQuotaScopeSelectors) +} + // SetExternalKubeClientSet implements the WantsInternalKubeClientSet interface. func (p *Plugin) SetExternalKubeClientSet(client kubernetes.Interface) { p.client = client @@ -106,7 +115,6 @@ func (p *Plugin) Admit(ctx context.Context, a admission.Attributes, o admission. if len(a.GetSubresource()) != 0 { return nil } - switch a.GetResource().GroupResource() { case podResource: if operation == admission.Create || operation == admission.Update { @@ -189,8 +197,12 @@ func (p *Plugin) admitPod(a admission.Attributes) error { pod.Spec.PriorityClassName = pcName } else { pcName := pod.Spec.PriorityClassName - if !priorityClassPermittedInNamespace(pcName, a.GetNamespace()) { - return admission.NewForbidden(a, fmt.Errorf("pods with %v priorityClass is not permitted in %v namespace", pcName, a.GetNamespace())) + // If ResourceQuotaScopeSelectors is enabled, we should let pods with critical priorityClass to be created + // any namespace where administrator wants it to be created. + if !p.resourceQuotaFeatureGateEnabled { + if !priorityClassPermittedInNamespace(pcName, a.GetNamespace()) { + return admission.NewForbidden(a, fmt.Errorf("pods with %v priorityClass is not permitted in %v namespace", pcName, a.GetNamespace())) + } } // Try resolving the priority class name. @@ -212,7 +224,7 @@ func (p *Plugin) admitPod(a admission.Attributes) error { } pod.Spec.Priority = &priority - if utilfeature.DefaultFeatureGate.Enabled(features.NonPreemptingPriority) { + if p.nonPreemptingPriority { var corePolicy core.PreemptionPolicy if preemptionPolicy != nil { corePolicy = core.PreemptionPolicy(*preemptionPolicy) diff --git a/plugin/pkg/admission/priority/admission_test.go b/plugin/pkg/admission/priority/admission_test.go index 2892bcf938e..37d00f48b9f 100644 --- a/plugin/pkg/admission/priority/admission_test.go +++ b/plugin/pkg/admission/priority/admission_test.go @@ -626,7 +626,7 @@ func TestPodAdmission(t *testing.T) { []*scheduling.PriorityClass{systemClusterCritical}, *pods[7], scheduling.SystemCriticalPriority, - true, + false, nil, }, { @@ -681,8 +681,9 @@ func TestPodAdmission(t *testing.T) { for _, test := range tests { klog.V(4).Infof("starting test %q", test.name) - ctrl := NewPlugin() + ctrl.resourceQuotaFeatureGateEnabled = true + ctrl.nonPreemptingPriority = true // Add existing priority classes. if err := addPriorityClasses(ctrl, test.existingClasses); err != nil { t.Errorf("Test %q: unable to add object to informer: %v", test.name, err) @@ -704,6 +705,7 @@ func TestPodAdmission(t *testing.T) { ) err := admissiontesting.WithReinvocationTesting(t, ctrl).Admit(context.TODO(), attrs, nil) klog.Infof("Got %v", err) + if !test.expectError { if err != nil { t.Errorf("Test %q: unexpected error received: %v", test.name, err) diff --git a/plugin/pkg/admission/resourcequota/BUILD b/plugin/pkg/admission/resourcequota/BUILD index a4c8f1ddcae..dda10afe386 100644 --- a/plugin/pkg/admission/resourcequota/BUILD +++ b/plugin/pkg/admission/resourcequota/BUILD @@ -22,7 +22,6 @@ go_library( "//pkg/quota/v1/generic:go_default_library", "//plugin/pkg/admission/resourcequota/apis/resourcequota:go_default_library", "//plugin/pkg/admission/resourcequota/apis/resourcequota/install:go_default_library", - "//plugin/pkg/admission/resourcequota/apis/resourcequota/v1beta1:go_default_library", "//plugin/pkg/admission/resourcequota/apis/resourcequota/validation:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library", diff --git a/test/integration/quota/BUILD b/test/integration/quota/BUILD index 3bf9cd52aab..2c2adb7c101 100644 --- a/test/integration/quota/BUILD +++ b/test/integration/quota/BUILD @@ -17,6 +17,7 @@ go_test( "//pkg/controller:go_default_library", "//pkg/controller/replication:go_default_library", "//pkg/controller/resourcequota:go_default_library", + "//pkg/features:go_default_library", "//pkg/quota/v1/generic:go_default_library", "//pkg/quota/v1/install:go_default_library", "//plugin/pkg/admission/resourcequota:go_default_library", @@ -29,11 +30,13 @@ go_test( "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/wait:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/watch:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", "//staging/src/k8s.io/client-go/informers:go_default_library", "//staging/src/k8s.io/client-go/kubernetes:go_default_library", "//staging/src/k8s.io/client-go/rest:go_default_library", "//staging/src/k8s.io/client-go/tools/record:go_default_library", "//staging/src/k8s.io/client-go/tools/watch:go_default_library", + "//staging/src/k8s.io/component-base/featuregate/testing:go_default_library", "//test/integration/framework:go_default_library", ], )