github/kubernetes
4
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #123001 from tkashem/apf-allow-zero-concurrency

Browse Source 
Allow zero value for the 'nominalConcurrencyShares' field
This commit is contained in:
Kubernetes Prow Robot
2024-02-06 09:08:18 -08:00
committed by GitHub
parent e566bd7769 5f75c35edf
commit 862ff187ba
4 changed files with 64 additions and 184 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
pkg/features/kube_features.go
View File

@@ -1211,7 +1211,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
genericfeatures.WatchList: {Default: false, PreRelease: featuregate.Alpha},
genericfeatures.ZeroLimitedNominalConcurrencyShares: {Default: false, PreRelease: featuregate.Beta},
genericfeatures.ZeroLimitedNominalConcurrencyShares: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.32
// inherited features from apiextensions-apiserver, relisted here to get a conflict if it is changed
// unintentionally on either side:

12
pkg/registry/flowcontrol/prioritylevelconfiguration/strategy.go
View File

@@ -24,9 +24,7 @@ import (
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/validation/field"
genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
"k8s.io/apiserver/pkg/features"
"k8s.io/apiserver/pkg/storage/names"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/kubernetes/pkg/api/legacyscheme"
"k8s.io/kubernetes/pkg/apis/flowcontrol"
"k8s.io/kubernetes/pkg/apis/flowcontrol/validation"
@@ -102,7 +100,7 @@ func (priorityLevelConfigurationStrategy) Validate(ctx context.Context, obj runt
// 'nominalConcurrencyShares' field of 'limited' for CREATE operation.
// 1:30: lift this restriction, allow zero value via v1 or v1beta3
opts := validation.PriorityLevelValidationOptions{
AllowZeroLimitedNominalConcurrencyShares: utilfeature.DefaultFeatureGate.Enabled(features.ZeroLimitedNominalConcurrencyShares),
AllowZeroLimitedNominalConcurrencyShares: true,
}
return validation.ValidatePriorityLevelConfiguration(obj.(*flowcontrol.PriorityLevelConfiguration), getRequestGroupVersion(ctx), opts)
}
@@ -128,7 +126,6 @@ func (priorityLevelConfigurationStrategy) AllowCreateOnUpdate() bool {
// ValidateUpdate is the default update validation for an end user.
func (priorityLevelConfigurationStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList {
newPL := obj.(*flowcontrol.PriorityLevelConfiguration)
oldPL := old.(*flowcontrol.PriorityLevelConfiguration)
// 1.28 server is not aware of the roundtrip annotation, and will
// default any 0 value persisted (for the NominalConcurrencyShares
@@ -144,8 +141,7 @@ func (priorityLevelConfigurationStrategy) ValidateUpdate(ctx context.Context, ob
// only if the existing object already contains a zero value.
// 1:30: lift this restriction, allow zero value via v1 or v1beta3
opts := validation.PriorityLevelValidationOptions{
AllowZeroLimitedNominalConcurrencyShares: utilfeature.DefaultFeatureGate.Enabled(features.ZeroLimitedNominalConcurrencyShares) ||
hasZeroLimitedNominalConcurrencyShares(oldPL),
AllowZeroLimitedNominalConcurrencyShares: true,
}
return validation.ValidatePriorityLevelConfiguration(newPL, getRequestGroupVersion(ctx), opts)
}
@@ -214,7 +210,3 @@ func getRequestGroupVersion(ctx context.Context) schema.GroupVersion {
}
return schema.GroupVersion{}
}
func hasZeroLimitedNominalConcurrencyShares(obj *flowcontrol.PriorityLevelConfiguration) bool {
return obj != nil && obj.Spec.Limited != nil && obj.Spec.Limited.NominalConcurrencyShares == 0
}

231
pkg/registry/flowcontrol/prioritylevelconfiguration/strategy_test.go
View File

@@ -25,9 +25,6 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/apiserver/pkg/features"
utilfeature "k8s.io/apiserver/pkg/util/feature"
featuregatetesting "k8s.io/component-base/featuregate/testing"
"k8s.io/kubernetes/pkg/apis/flowcontrol"
"k8s.io/utils/ptr"
@@ -99,208 +96,98 @@ func TestPriorityLevelConfigurationValidation(t *testing.T) {
}
return scheme
}
errExpectedFn := func(v int32, msg string) field.ErrorList {
return field.ErrorList{
field.Invalid(field.NewPath("spec").Child("limited").Child("nominalConcurrencyShares"), int32(v), msg),
}
}
tests := []struct {
name string
obj runtime.Object
old *flowcontrol.PriorityLevelConfiguration // for UPDATE only
zeroFeatureEnabled bool
scheme *runtime.Scheme
errExpected field.ErrorList
name string
obj runtime.Object
old *flowcontrol.PriorityLevelConfiguration // for UPDATE only
scheme *runtime.Scheme
errExpected field.ErrorList
}{
{
name: "v1, feature disabled, create, zero value, error expected",
obj: v1ObjFn(ptr.To(int32(0))),
zeroFeatureEnabled: false,
scheme: v1SchemeFn(t),
errExpected: errExpectedFn(0, "must be positive"),
name: "v1, create, zero value, no error expected",
obj: v1ObjFn(ptr.To(int32(0))),
scheme: v1SchemeFn(t),
errExpected: nil,
},
{
name: "v1, feature disabled, create, unset, no error expected",
obj: v1ObjFn(nil),
zeroFeatureEnabled: false,
scheme: v1SchemeFn(t),
errExpected: nil,
name: "v1, create, unset, no error expected",
obj: v1ObjFn(nil),
scheme: v1SchemeFn(t),
errExpected: nil,
},
{
name: "v1, feature disabled, create, non-zero, no error expected",
obj: v1ObjFn(ptr.To(int32(1))),
zeroFeatureEnabled: false,
scheme: v1SchemeFn(t),
errExpected: nil,
name: "v1, create, non-zero, no error expected",
obj: v1ObjFn(ptr.To(int32(1))),
scheme: v1SchemeFn(t),
errExpected: nil,
},
{
name: "v1, feature enabled, create, zero value, no error expected",
obj: v1ObjFn(ptr.To(int32(0))),
zeroFeatureEnabled: true,
scheme: v1SchemeFn(t),
errExpected: nil,
name: "v1beta3, create, zero value, no error expected",
obj: v1beta3ObjFn(0, true),
scheme: v1beta3SchemeFn(t),
errExpected: nil,
},
{
name: "v1, feature enabled, create, unset, no error expected",
obj: v1ObjFn(nil),
zeroFeatureEnabled: true,
scheme: v1SchemeFn(t),
errExpected: nil,
name: "v1beta3, create, zero value without annotation, no error expected",
obj: v1beta3ObjFn(0, false),
scheme: v1beta3SchemeFn(t),
errExpected: nil,
},
{
name: "v1, feature enabled, create, non-zero, no error expected",
obj: v1ObjFn(ptr.To(int32(1))),
zeroFeatureEnabled: true,
scheme: v1SchemeFn(t),
errExpected: nil,
},
{
name: "v1beta3, feature disabled, create, zero value, error expected",
obj: v1beta3ObjFn(0, true),
zeroFeatureEnabled: false,
scheme: v1beta3SchemeFn(t),
errExpected: errExpectedFn(0, "must be positive"),
},
{
name: "v1beta3, feature disabled, create, zero value without annotation, no error expected",
obj: v1beta3ObjFn(0, false),
zeroFeatureEnabled: false,
scheme: v1beta3SchemeFn(t),
errExpected: nil,
},
{
name: "v1beta3, feature disabled, create, non-zero, no error expected",
obj: v1beta3ObjFn(1, false),
zeroFeatureEnabled: false,
scheme: v1beta3SchemeFn(t),
errExpected: nil,
},
{
name: "v1beta3, feature enabled, create, zero value, no error expected",
obj: v1beta3ObjFn(0, true),
zeroFeatureEnabled: true,
scheme: v1beta3SchemeFn(t),
errExpected: nil,
},
{
name: "v1beta3, feature enabled, create, zero value without annotation, no error expected",
obj: v1beta3ObjFn(0, false),
zeroFeatureEnabled: true,
scheme: v1beta3SchemeFn(t),
errExpected: nil,
},
{
name: "v1beta3, feature enabled, create, non-zero, no error expected",
obj: v1beta3ObjFn(1, false),
zeroFeatureEnabled: true,
scheme: v1beta3SchemeFn(t),
errExpected: nil,
name: "v1beta3, create, non-zero, no error expected",
obj: v1beta3ObjFn(1, false),
scheme: v1beta3SchemeFn(t),
errExpected: nil,
},
// the following use cases cover UPDATE
{
name: "v1, feature disabled, update, zero value, existing has non-zero, error expected",
obj: v1ObjFn(ptr.To(int32(0))),
old: internalObjFn(1),
zeroFeatureEnabled: false,
scheme: v1SchemeFn(t),
errExpected: errExpectedFn(0, "must be positive"),
name: "v1, update, zero value, existing has non-zero, no error expected",
obj: v1ObjFn(ptr.To(int32(0))),
old: internalObjFn(1),
scheme: v1SchemeFn(t),
errExpected: nil,
},
{
name: "v1, feature disabled, update, zero value, existing has zero, no error expected",
obj: v1ObjFn(ptr.To(int32(0))),
old: internalObjFn(0),
zeroFeatureEnabled: false,
scheme: v1SchemeFn(t),
errExpected: nil,
name: "v1, update, zero value, existing has zero, no error expected",
obj: v1ObjFn(ptr.To(int32(0))),
old: internalObjFn(0),
scheme: v1SchemeFn(t),
errExpected: nil,
},
{
name: "v1, feature disabled, update, non-zero value, existing has zero, no error expected",
obj: v1ObjFn(ptr.To(int32(1))),
old: internalObjFn(0),
zeroFeatureEnabled: false,
scheme: v1SchemeFn(t),
errExpected: nil,
name: "v1, update, non-zero value, existing has zero, no error expected",
obj: v1ObjFn(ptr.To(int32(1))),
old: internalObjFn(0),
scheme: v1SchemeFn(t),
errExpected: nil,
},
{
name: "v1, feature enabled, update, zero value, existing has non-zero, no error expected",
obj: v1ObjFn(ptr.To(int32(0))),
old: internalObjFn(1),
zeroFeatureEnabled: true,
scheme: v1SchemeFn(t),
errExpected: nil,
name: "v1beta3, update, zero value, existing has non-zero, no error expected",
obj: v1beta3ObjFn(0, true),
old: internalObjFn(1),
scheme: v1beta3SchemeFn(t),
errExpected: nil,
},
{
name: "v1, feature enabled, update, zero value, existing has zero, no error expected",
obj: v1ObjFn(ptr.To(int32(0))),
old: internalObjFn(0),
zeroFeatureEnabled: true,
scheme: v1SchemeFn(t),
errExpected: nil,
name: "v1beta3, update, zero value, existing has zero, no error expected",
obj: v1beta3ObjFn(0, true),
old: internalObjFn(0),
scheme: v1beta3SchemeFn(t),
errExpected: nil,
},
{
name: "v1, feature enabled, update, non-zero value, existing has zero, no error expected",
obj: v1ObjFn(ptr.To(int32(1))),
old: internalObjFn(0),
zeroFeatureEnabled: true,
scheme: v1SchemeFn(t),
errExpected: nil,
},
{
name: "v1beta3, feature disabled, update, zero value, existing has non-zero, error expected",
obj: v1beta3ObjFn(0, true),
old: internalObjFn(1),
zeroFeatureEnabled: false,
scheme: v1beta3SchemeFn(t),
errExpected: errExpectedFn(0, "must be positive"),
},
{
name: "v1beta3, feature disabled, update, zero value, existing has zero, no error expected",
obj: v1beta3ObjFn(0, true),
old: internalObjFn(0),
zeroFeatureEnabled: false,
scheme: v1beta3SchemeFn(t),
errExpected: nil,
},
{
name: "v1beta3, feature disabled, update, non-zero value, existing has zero, no error expected",
obj: v1beta3ObjFn(1, false),
old: internalObjFn(0),
zeroFeatureEnabled: false,
scheme: v1beta3SchemeFn(t),
errExpected: nil,
},
{
name: "v1beta3, feature enabled, update, zero value, existing has non-zero, no error expected",
obj: v1beta3ObjFn(0, true),
old: internalObjFn(1),
zeroFeatureEnabled: true,
scheme: v1beta3SchemeFn(t),
errExpected: nil,
},
{
name: "v1beta3, feature enabled, update, zero value, existing has zero, no error expected",
obj: v1beta3ObjFn(0, true),
old: internalObjFn(0),
zeroFeatureEnabled: true,
scheme: v1beta3SchemeFn(t),
errExpected: nil,
},
{
name: "v1beta3, feature enabled, update, non-zero value, existing has zero, no error expected",
obj: v1beta3ObjFn(1, false),
old: internalObjFn(0),
zeroFeatureEnabled: true,
scheme: v1beta3SchemeFn(t),
errExpected: nil,
name: "v1beta3, update, non-zero value, existing has zero, no error expected",
obj: v1beta3ObjFn(1, false),
old: internalObjFn(0),
scheme: v1beta3SchemeFn(t),
errExpected: nil,
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ZeroLimitedNominalConcurrencyShares, test.zeroFeatureEnabled)()
scheme := test.scheme
scheme.Default(test.obj)

3
staging/src/k8s.io/apiserver/pkg/features/kube_features.go
View File

@@ -247,6 +247,7 @@ const (
// owner: @tkashem
// beta: v1.29
// GA: v1.30
//
// Allow Priority & Fairness in the API server to use a zero value for
// the 'nominalConcurrencyShares' field of the 'limited' section of a
@@ -315,5 +316,5 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
ConsistentListFromCache: {Default: false, PreRelease: featuregate.Alpha},
ZeroLimitedNominalConcurrencyShares: {Default: false, PreRelease: featuregate.Beta},
ZeroLimitedNominalConcurrencyShares: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.32
}