From 5cb92260a693e0b71269bfb941891f8e3df0d95b Mon Sep 17 00:00:00 2001 From: draveness Date: Wed, 13 Nov 2019 11:12:43 +0800 Subject: [PATCH] feat: graduate ResourceQuotaScopeSelectors to GA --- pkg/BUILD | 1 - pkg/api/resourcequota/BUILD | 40 ------ pkg/api/resourcequota/util.go | 41 ------ pkg/api/resourcequota/util_test.go | 117 ------------------ pkg/features/kube_features.go | 4 +- pkg/registry/core/resourcequota/BUILD | 1 - pkg/registry/core/resourcequota/strategy.go | 3 - plugin/pkg/admission/priority/BUILD | 2 - plugin/pkg/admission/priority/admission.go | 33 +---- .../pkg/admission/priority/admission_test.go | 1 - 10 files changed, 5 insertions(+), 238 deletions(-) delete mode 100644 pkg/api/resourcequota/BUILD delete mode 100644 pkg/api/resourcequota/util.go delete mode 100644 pkg/api/resourcequota/util_test.go diff --git a/pkg/BUILD b/pkg/BUILD index a4a97d2c97e..4586fb55fd4 100644 --- a/pkg/BUILD +++ b/pkg/BUILD @@ -17,7 +17,6 @@ filegroup( "//pkg/api/persistentvolumeclaim:all-srcs", "//pkg/api/pod:all-srcs", "//pkg/api/podsecuritypolicy:all-srcs", - "//pkg/api/resourcequota:all-srcs", "//pkg/api/service:all-srcs", "//pkg/api/testapi:all-srcs", "//pkg/api/testing:all-srcs", diff --git a/pkg/api/resourcequota/BUILD b/pkg/api/resourcequota/BUILD deleted file mode 100644 index 93c0eb48a3e..00000000000 --- a/pkg/api/resourcequota/BUILD +++ /dev/null @@ -1,40 +0,0 @@ -load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") - -go_library( - name = "go_default_library", - srcs = ["util.go"], - importpath = "k8s.io/kubernetes/pkg/api/resourcequota", - visibility = ["//visibility:public"], - deps = [ - "//pkg/apis/core:go_default_library", - "//pkg/features:go_default_library", - "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", - ], -) - -go_test( - name = "go_default_test", - srcs = ["util_test.go"], - embed = [":go_default_library"], - deps = [ - "//pkg/apis/core:go_default_library", - "//pkg/features:go_default_library", - "//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library", - "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", - "//staging/src/k8s.io/component-base/featuregate/testing:go_default_library", - ], -) - -filegroup( - name = "package-srcs", - srcs = glob(["**"]), - tags = ["automanaged"], - visibility = ["//visibility:private"], -) - -filegroup( - name = "all-srcs", - srcs = [":package-srcs"], - tags = ["automanaged"], - visibility = ["//visibility:public"], -) diff --git a/pkg/api/resourcequota/util.go b/pkg/api/resourcequota/util.go deleted file mode 100644 index 322472011f3..00000000000 --- a/pkg/api/resourcequota/util.go +++ /dev/null @@ -1,41 +0,0 @@ -/* -Copyright 2019 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package resourcequota - -import ( - utilfeature "k8s.io/apiserver/pkg/util/feature" - api "k8s.io/kubernetes/pkg/apis/core" - "k8s.io/kubernetes/pkg/features" -) - -// DropDisabledFields removes disabled fields from the ResourceQuota spec. -// This should be called from PrepareForCreate/PrepareForUpdate for all resources containing a ResourceQuota spec. -func DropDisabledFields(resSpec *api.ResourceQuotaSpec, oldResSpec *api.ResourceQuotaSpec) { - if !utilfeature.DefaultFeatureGate.Enabled(features.ResourceQuotaScopeSelectors) && !resourceQuotaScopeSelectorInUse(oldResSpec) { - resSpec.ScopeSelector = nil - } -} - -func resourceQuotaScopeSelectorInUse(oldResSpec *api.ResourceQuotaSpec) bool { - if oldResSpec == nil { - return false - } - if oldResSpec.ScopeSelector != nil { - return true - } - return false -} diff --git a/pkg/api/resourcequota/util_test.go b/pkg/api/resourcequota/util_test.go deleted file mode 100644 index 53b856e4af5..00000000000 --- a/pkg/api/resourcequota/util_test.go +++ /dev/null @@ -1,117 +0,0 @@ -/* -Copyright 2019 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package resourcequota - -import ( - "fmt" - "reflect" - "testing" - - "k8s.io/apimachinery/pkg/util/diff" - utilfeature "k8s.io/apiserver/pkg/util/feature" - featuregatetesting "k8s.io/component-base/featuregate/testing" - api "k8s.io/kubernetes/pkg/apis/core" - "k8s.io/kubernetes/pkg/features" -) - -func TestDropDisabledFields(t *testing.T) { - rqWithScopeSelector := func() *api.ResourceQuota { - return &api.ResourceQuota{Spec: api.ResourceQuotaSpec{Scopes: []api.ResourceQuotaScope{"scope-1"}, ScopeSelector: &api.ScopeSelector{ - MatchExpressions: []api.ScopedResourceSelectorRequirement{ - { - ScopeName: api.ResourceQuotaScopePriorityClass, - Operator: api.ScopeSelectorOpIn, - Values: []string{"scope-1"}, - }, - }, - }}} - } - rqWithoutScopeSelector := func() *api.ResourceQuota { - return &api.ResourceQuota{Spec: api.ResourceQuotaSpec{Scopes: []api.ResourceQuotaScope{"scope-1"}, ScopeSelector: nil}} - } - - rqInfo := []struct { - description string - hasScopeSelector bool - resourceQuota func() *api.ResourceQuota - }{ - { - description: "ResourceQuota without Scopes Selector", - hasScopeSelector: false, - resourceQuota: rqWithoutScopeSelector, - }, - { - description: "ResourceQuota with Scope Selector", - hasScopeSelector: true, - resourceQuota: rqWithScopeSelector, - }, - { - description: "is nil", - hasScopeSelector: false, - resourceQuota: func() *api.ResourceQuota { return nil }, - }, - } - - for _, enabled := range []bool{true, false} { - for _, oldRQInfo := range rqInfo { - for _, newRQInfo := range rqInfo { - oldRQHasSelector, oldrq := oldRQInfo.hasScopeSelector, oldRQInfo.resourceQuota() - newRQHasSelector, newrq := newRQInfo.hasScopeSelector, newRQInfo.resourceQuota() - if newrq == nil { - continue - } - - t.Run(fmt.Sprintf("feature enabled=%v, old ResourceQuota %v, new ResourceQuota %v", enabled, oldRQInfo.description, newRQInfo.description), func(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ResourceQuotaScopeSelectors, enabled)() - - var oldRQSpec *api.ResourceQuotaSpec - if oldrq != nil { - oldRQSpec = &oldrq.Spec - } - DropDisabledFields(&newrq.Spec, oldRQSpec) - - // old ResourceQuota should never be changed - if !reflect.DeepEqual(oldrq, oldRQInfo.resourceQuota()) { - t.Errorf("old ResourceQuota changed: %v", diff.ObjectReflectDiff(oldrq, oldRQInfo.resourceQuota())) - } - - switch { - case enabled || oldRQHasSelector: - // new ResourceQuota should not be changed if the feature is enabled, or if the old ResourceQuota had ScopeSelector - if !reflect.DeepEqual(newrq, newRQInfo.resourceQuota()) { - t.Errorf("new ResourceQuota changed: %v", diff.ObjectReflectDiff(newrq, newRQInfo.resourceQuota())) - } - case newRQHasSelector: - // new ResourceQuota should be changed - if reflect.DeepEqual(newrq, newRQInfo.resourceQuota()) { - t.Errorf("new ResourceQuota was not changed") - } - // new ResourceQuota should not have ScopeSelector - if !reflect.DeepEqual(newrq, rqWithoutScopeSelector()) { - t.Errorf("new ResourceQuota had ScopeSelector: %v", diff.ObjectReflectDiff(newrq, rqWithoutScopeSelector())) - } - default: - // new ResourceQuota should not need to be changed - if !reflect.DeepEqual(newrq, newRQInfo.resourceQuota()) { - t.Errorf("new ResourceQuota changed: %v", diff.ObjectReflectDiff(newrq, newRQInfo.resourceQuota())) - } - } - }) - } - } - } -} diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index a3f887d6a91..9eabe5995c9 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -308,7 +308,7 @@ const ( // owner: @vikaschoudhary16 // beta: v1.12 - // + // ga: v1.17 // // Enable resource quota scope selectors ResourceQuotaScopeSelectors featuregate.Feature = "ResourceQuotaScopeSelectors" @@ -556,7 +556,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS VolumeSubpath: {Default: true, PreRelease: featuregate.GA}, BalanceAttachedNodeVolumes: {Default: false, PreRelease: featuregate.Alpha}, VolumeSubpathEnvExpansion: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.19, - ResourceQuotaScopeSelectors: {Default: true, PreRelease: featuregate.Beta}, + ResourceQuotaScopeSelectors: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.18 CSIBlockVolume: {Default: true, PreRelease: featuregate.Beta}, CSIInlineVolume: {Default: true, PreRelease: featuregate.Beta}, RuntimeClass: {Default: true, PreRelease: featuregate.Beta}, diff --git a/pkg/registry/core/resourcequota/BUILD b/pkg/registry/core/resourcequota/BUILD index a8eac9a9764..54bba4eaa48 100644 --- a/pkg/registry/core/resourcequota/BUILD +++ b/pkg/registry/core/resourcequota/BUILD @@ -15,7 +15,6 @@ go_library( importpath = "k8s.io/kubernetes/pkg/registry/core/resourcequota", deps = [ "//pkg/api/legacyscheme:go_default_library", - "//pkg/api/resourcequota:go_default_library", "//pkg/apis/core:go_default_library", "//pkg/apis/core/validation:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", diff --git a/pkg/registry/core/resourcequota/strategy.go b/pkg/registry/core/resourcequota/strategy.go index da259dfbffc..aa123e19ebe 100644 --- a/pkg/registry/core/resourcequota/strategy.go +++ b/pkg/registry/core/resourcequota/strategy.go @@ -23,7 +23,6 @@ import ( "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/apiserver/pkg/storage/names" "k8s.io/kubernetes/pkg/api/legacyscheme" - resourcequotautil "k8s.io/kubernetes/pkg/api/resourcequota" api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/core/validation" ) @@ -47,7 +46,6 @@ func (resourcequotaStrategy) NamespaceScoped() bool { func (resourcequotaStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) { resourcequota := obj.(*api.ResourceQuota) resourcequota.Status = api.ResourceQuotaStatus{} - resourcequotautil.DropDisabledFields(&resourcequota.Spec, nil) } // PrepareForUpdate clears fields that are not allowed to be set by end users on update. @@ -55,7 +53,6 @@ func (resourcequotaStrategy) PrepareForUpdate(ctx context.Context, obj, old runt newResourcequota := obj.(*api.ResourceQuota) oldResourcequota := old.(*api.ResourceQuota) newResourcequota.Status = oldResourcequota.Status - resourcequotautil.DropDisabledFields(&newResourcequota.Spec, &oldResourcequota.Spec) } // Validate validates a new resourcequota. diff --git a/plugin/pkg/admission/priority/BUILD b/plugin/pkg/admission/priority/BUILD index 62f9d619c94..daf148908fb 100644 --- a/plugin/pkg/admission/priority/BUILD +++ b/plugin/pkg/admission/priority/BUILD @@ -35,12 +35,10 @@ go_library( deps = [ "//pkg/apis/core:go_default_library", "//pkg/apis/scheduling:go_default_library", - "//pkg/apis/scheduling/v1:go_default_library", "//pkg/features:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/api/scheduling/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/labels:go_default_library", "//staging/src/k8s.io/apiserver/pkg/admission:go_default_library", "//staging/src/k8s.io/apiserver/pkg/admission/initializer:go_default_library", diff --git a/plugin/pkg/admission/priority/admission.go b/plugin/pkg/admission/priority/admission.go index 87318394af0..c414535aa25 100644 --- a/plugin/pkg/admission/priority/admission.go +++ b/plugin/pkg/admission/priority/admission.go @@ -24,7 +24,6 @@ import ( apiv1 "k8s.io/api/core/v1" schedulingv1 "k8s.io/api/scheduling/v1" "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apiserver/pkg/admission" genericadmissioninitializers "k8s.io/apiserver/pkg/admission/initializer" @@ -35,7 +34,6 @@ import ( "k8s.io/kubernetes/pkg/apis/core" api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/scheduling" - schedulingapiv1 "k8s.io/kubernetes/pkg/apis/scheduling/v1" "k8s.io/kubernetes/pkg/features" ) @@ -54,10 +52,9 @@ func Register(plugins *admission.Plugins) { // Plugin is an implementation of admission.Interface. type Plugin struct { *admission.Handler - client kubernetes.Interface - lister schedulingv1listers.PriorityClassLister - resourceQuotaFeatureGateEnabled bool - nonPreemptingPriority bool + client kubernetes.Interface + lister schedulingv1listers.PriorityClassLister + nonPreemptingPriority bool } var _ admission.MutationInterface = &Plugin{} @@ -87,7 +84,6 @@ func (p *Plugin) ValidateInitialization() error { // 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. @@ -147,20 +143,6 @@ func (p *Plugin) Validate(ctx context.Context, a admission.Attributes, o admissi } } -// priorityClassPermittedInNamespace returns true if we allow the given priority class name in the -// given namespace. It currently checks that system priorities are created only in the system namespace. -func priorityClassPermittedInNamespace(priorityClassName string, namespace string) bool { - // Only allow system priorities in the system namespace. This is to prevent abuse or incorrect - // usage of these priorities. Pods created at these priorities could preempt system critical - // components. - for _, spc := range schedulingapiv1.SystemPriorityClasses() { - if spc.Name == priorityClassName && namespace != metav1.NamespaceSystem { - return false - } - } - return true -} - // admitPod makes sure a new pod does not set spec.Priority field. It also makes sure that the PriorityClassName exists if it is provided and resolves the pod priority from the PriorityClassName. func (p *Plugin) admitPod(a admission.Attributes) error { operation := a.GetOperation() @@ -196,15 +178,6 @@ func (p *Plugin) admitPod(a admission.Attributes) error { } pod.Spec.PriorityClassName = pcName } else { - pcName := pod.Spec.PriorityClassName - // 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. pc, err := p.lister.Get(pod.Spec.PriorityClassName) if err != nil { diff --git a/plugin/pkg/admission/priority/admission_test.go b/plugin/pkg/admission/priority/admission_test.go index 37d00f48b9f..42af7cb748b 100644 --- a/plugin/pkg/admission/priority/admission_test.go +++ b/plugin/pkg/admission/priority/admission_test.go @@ -682,7 +682,6 @@ 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 {