Merge pull request #72664 from sbezverk/ResourceQuotaScopeSelectors

Move ResourceQuotaSelector out of validation
pull/564/head
Kubernetes Prow Robot 2019-01-08 02:02:59 -08:00 committed by GitHub
commit 8728ecc27d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 203 additions and 9 deletions

View File

@ -19,6 +19,7 @@ filegroup(
"//pkg/api/podsecuritypolicy:all-srcs",
"//pkg/api/ref:all-srcs",
"//pkg/api/resource:all-srcs",
"//pkg/api/resourcequota:all-srcs",
"//pkg/api/service:all-srcs",
"//pkg/api/testapi:all-srcs",
"//pkg/api/testing:all-srcs",

View File

@ -0,0 +1,40 @@
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/apiserver/pkg/util/feature/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"],
)

View File

@ -0,0 +1,41 @@
/*
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
}

View File

@ -0,0 +1,117 @@
/*
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"
utilfeaturetesting "k8s.io/apiserver/pkg/util/feature/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 utilfeaturetesting.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()))
}
}
})
}
}
}
}

View File

@ -4901,9 +4901,6 @@ func validateScopeSelector(resourceQuotaSpec *core.ResourceQuotaSpec, fld *field
if resourceQuotaSpec.ScopeSelector == nil {
return allErrs
}
if !utilfeature.DefaultFeatureGate.Enabled(features.ResourceQuotaScopeSelectors) && resourceQuotaSpec.ScopeSelector != nil {
allErrs = append(allErrs, field.Forbidden(fld.Child("scopeSelector"), "ResourceQuotaScopeSelectors feature-gate is disabled"))
}
allErrs = append(allErrs, validateScopedResourceSelectorRequirement(resourceQuotaSpec, fld.Child("scopeSelector"))...)
return allErrs
}

View File

@ -11528,13 +11528,11 @@ func TestValidateResourceQuota(t *testing.T) {
Spec: nonBestEffortSpec,
},
}
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ResourceQuotaScopeSelectors, true)()
for _, successCase := range successCases {
if errs := ValidateResourceQuota(&successCase); len(errs) != 0 {
t.Errorf("expected success: %v", errs)
}
}
defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ResourceQuotaScopeSelectors, false)()
errorCases := map[string]struct {
R core.ResourceQuota
@ -11580,10 +11578,6 @@ func TestValidateResourceQuota(t *testing.T) {
core.ResourceQuota{ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: "foo"}, Spec: invalidScopeNameSpec},
"unsupported scope",
},
"forbidden-quota-scope-selector": {
core.ResourceQuota{ObjectMeta: metav1.ObjectMeta{Name: "abc", Namespace: "foo"}, Spec: scopeSelectorSpec},
"feature-gate is disabled",
},
}
for k, v := range errorCases {
errs := ValidateResourceQuota(&v.R)

View File

@ -15,6 +15,7 @@ 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",

View File

@ -23,6 +23,7 @@ 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"
)
@ -46,6 +47,7 @@ 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.
@ -53,6 +55,7 @@ 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.