Merge pull request #126894 from carlory/ZeroLimitedNominalConcurrencyShares

remove generally available feature-gate ZeroLimitedNominalConcurrencyShares
This commit is contained in:
Kubernetes Prow Robot 2024-11-04 17:19:29 +00:00 committed by GitHub
commit 6a1a6fd85f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 6 additions and 94 deletions

View File

@ -76,12 +76,7 @@ var supportedLimitResponseType = sets.NewString(
)
// PriorityLevelValidationOptions holds the validation options for a priority level object
type PriorityLevelValidationOptions struct {
// AllowZeroLimitedNominalConcurrencyShares, if true, indicates that we allow
// a zero value for the 'nominalConcurrencyShares' field of the 'limited'
// section of a priority level.
AllowZeroLimitedNominalConcurrencyShares bool
}
type PriorityLevelValidationOptions struct{}
// ValidateFlowSchema validates the content of flow-schema
func ValidateFlowSchema(fs *flowcontrol.FlowSchema) field.ErrorList {
@ -429,14 +424,8 @@ func ValidatePriorityLevelConfigurationSpec(spec *flowcontrol.PriorityLevelConfi
// ValidateLimitedPriorityLevelConfiguration validates the configuration for an execution-limited priority level
func ValidateLimitedPriorityLevelConfiguration(lplc *flowcontrol.LimitedPriorityLevelConfiguration, requestGV schema.GroupVersion, fldPath *field.Path, opts PriorityLevelValidationOptions) field.ErrorList {
var allErrs field.ErrorList
if opts.AllowZeroLimitedNominalConcurrencyShares {
if lplc.NominalConcurrencyShares < 0 {
allErrs = append(allErrs, field.Invalid(fldPath.Child(getVersionedFieldNameForConcurrencyShares(requestGV)), lplc.NominalConcurrencyShares, "must be a non-negative integer"))
}
} else {
if lplc.NominalConcurrencyShares <= 0 {
allErrs = append(allErrs, field.Invalid(fldPath.Child(getVersionedFieldNameForConcurrencyShares(requestGV)), lplc.NominalConcurrencyShares, "must be positive"))
}
if lplc.NominalConcurrencyShares < 0 {
allErrs = append(allErrs, field.Invalid(fldPath.Child(getVersionedFieldNameForConcurrencyShares(requestGV)), lplc.NominalConcurrencyShares, "must be a non-negative integer"))
}
allErrs = append(allErrs, ValidateLimitResponse(lplc.LimitResponse, fldPath.Child("limitResponse"))...)

View File

@ -24,8 +24,6 @@ import (
"github.com/google/go-cmp/cmp"
"github.com/stretchr/testify/assert"
flowcontrolv1 "k8s.io/api/flowcontrol/v1"
flowcontrolv1beta1 "k8s.io/api/flowcontrol/v1beta1"
flowcontrolv1beta2 "k8s.io/api/flowcontrol/v1beta2"
flowcontrolv1beta3 "k8s.io/api/flowcontrol/v1beta3"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
@ -1304,65 +1302,32 @@ func TestValidateLimitedPriorityLevelConfiguration(t *testing.T) {
tests := []struct {
requestVersion schema.GroupVersion
allowZero bool
concurrencyShares int32
errExpected field.ErrorList
}{{
requestVersion: flowcontrolv1beta1.SchemeGroupVersion,
concurrencyShares: 0,
errExpected: errExpectedFn("assuredConcurrencyShares", 0, "must be positive"),
}, {
requestVersion: flowcontrolv1beta2.SchemeGroupVersion,
concurrencyShares: 0,
errExpected: errExpectedFn("assuredConcurrencyShares", 0, "must be positive"),
}, {
requestVersion: flowcontrolv1beta3.SchemeGroupVersion,
concurrencyShares: 0,
errExpected: errExpectedFn("nominalConcurrencyShares", 0, "must be positive"),
}, {
requestVersion: flowcontrolv1.SchemeGroupVersion,
concurrencyShares: 0,
errExpected: errExpectedFn("nominalConcurrencyShares", 0, "must be positive"),
}, {
requestVersion: flowcontrolv1beta3.SchemeGroupVersion,
concurrencyShares: 100,
errExpected: nil,
}, {
requestVersion: flowcontrolv1beta3.SchemeGroupVersion,
allowZero: true,
concurrencyShares: 0,
errExpected: nil,
}, {
requestVersion: flowcontrolv1beta3.SchemeGroupVersion,
allowZero: true,
concurrencyShares: -1,
errExpected: errExpectedFn("nominalConcurrencyShares", -1, "must be a non-negative integer"),
}, {
requestVersion: flowcontrolv1beta3.SchemeGroupVersion,
allowZero: true,
concurrencyShares: 1,
errExpected: nil,
}, {
requestVersion: flowcontrolv1.SchemeGroupVersion,
allowZero: true,
concurrencyShares: 0,
errExpected: nil,
}, {
requestVersion: flowcontrolv1.SchemeGroupVersion,
allowZero: true,
concurrencyShares: -1,
errExpected: errExpectedFn("nominalConcurrencyShares", -1, "must be a non-negative integer"),
}, {
requestVersion: flowcontrolv1.SchemeGroupVersion,
allowZero: true,
concurrencyShares: 1,
errExpected: nil,
}, {
// this should never really happen in real life, the request
// context should always contain the request {group, version}
requestVersion: schema.GroupVersion{},
concurrencyShares: 0,
errExpected: errExpectedFn("nominalConcurrencyShares", 0, "must be positive"),
}}
for _, test := range tests {
@ -1375,7 +1340,7 @@ func TestValidateLimitedPriorityLevelConfiguration(t *testing.T) {
}
specPath := field.NewPath("spec").Child("limited")
errGot := ValidateLimitedPriorityLevelConfiguration(configuration, test.requestVersion, specPath, PriorityLevelValidationOptions{AllowZeroLimitedNominalConcurrencyShares: test.allowZero})
errGot := ValidateLimitedPriorityLevelConfiguration(configuration, test.requestVersion, specPath, PriorityLevelValidationOptions{})
if !cmp.Equal(test.errExpected, errGot) {
t.Errorf("Expected error: %v, diff: %s", test.errExpected, cmp.Diff(test.errExpected, errGot))
}

View File

@ -348,11 +348,6 @@ var defaultVersionedKubernetesFeatureGates = map[featuregate.Feature]featuregate
{Version: version.MustParse("1.32"), Default: true, PreRelease: featuregate.Beta},
},
genericfeatures.ZeroLimitedNominalConcurrencyShares: {
{Version: version.MustParse("1.29"), Default: false, PreRelease: featuregate.Beta},
{Version: version.MustParse("1.30"), Default: true, PreRelease: featuregate.GA, LockToDefault: true},
},
GracefulNodeShutdown: {
{Version: version.MustParse("1.20"), Default: false, PreRelease: featuregate.Alpha},
{Version: version.MustParse("1.21"), Default: true, PreRelease: featuregate.Beta},

View File

@ -94,14 +94,7 @@ func (priorityLevelConfigurationStrategy) Validate(ctx context.Context, obj runt
// That means we should not allow 0 values to be introduced, either
// via v1 or v1beta3(with the roundtrip annotation) until we know
// all servers are at 1.29+ and will honor the zero value correctly.
//
// TODO(121510): 1.29: don't allow a zero value, either via v1 or
// v1beta3 (with the roundtrip annotation) for the
// 'nominalConcurrencyShares' field of 'limited' for CREATE operation.
// 1:30: lift this restriction, allow zero value via v1 or v1beta3
opts := validation.PriorityLevelValidationOptions{
AllowZeroLimitedNominalConcurrencyShares: true,
}
opts := validation.PriorityLevelValidationOptions{}
return validation.ValidatePriorityLevelConfiguration(obj.(*flowcontrol.PriorityLevelConfiguration), getRequestGroupVersion(ctx), opts)
}
@ -134,15 +127,7 @@ func (priorityLevelConfigurationStrategy) ValidateUpdate(ctx context.Context, ob
// That means we should not allow 0 values to be introduced, either
// via v1 or v1beta3(with the roundtrip annotation) until we know
// all servers are at 1.29+ and will honor the zero value correctly.
//
// TODO(121510): 1.29: only allow a zero value, either via v1 or
// v1beta3 (with the roundtrip annotation) for the
// 'nominalConcurrencyShares' field of 'limited' for UPDATE operation,
// 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: true,
}
opts := validation.PriorityLevelValidationOptions{}
return validation.ValidatePriorityLevelConfiguration(newPL, getRequestGroupVersion(ctx), opts)
}

View File

@ -235,13 +235,6 @@ const (
//
// Allow the API server to serve consistent lists from cache
ConsistentListFromCache featuregate.Feature = "ConsistentListFromCache"
// owner: @tkashem
//
// Allow Priority & Fairness in the API server to use a zero value for
// the 'nominalConcurrencyShares' field of the 'limited' section of a
// priority level.
ZeroLimitedNominalConcurrencyShares featuregate.Feature = "ZeroLimitedNominalConcurrencyShares"
)
func init() {
@ -410,11 +403,6 @@ var defaultVersionedKubernetesFeatureGates = map[featuregate.Feature]featuregate
{Version: version.MustParse("1.27"), Default: false, PreRelease: featuregate.Alpha},
{Version: version.MustParse("1.32"), Default: true, PreRelease: featuregate.Beta},
},
ZeroLimitedNominalConcurrencyShares: {
{Version: version.MustParse("1.29"), Default: false, PreRelease: featuregate.Beta},
{Version: version.MustParse("1.30"), Default: true, PreRelease: featuregate.GA, LockToDefault: true},
},
}
// defaultKubernetesFeatureGates consists of legacy unversioned Kubernetes-specific feature keys.

View File

@ -1438,13 +1438,3 @@
lockToDefault: false
preRelease: Beta
version: "1.20"
- name: ZeroLimitedNominalConcurrencyShares
versionedSpecs:
- default: false
lockToDefault: false
preRelease: Beta
version: "1.29"
- default: true
lockToDefault: true
preRelease: GA
version: "1.30"