diff --git a/pkg/apis/flowcontrol/validation/validation.go b/pkg/apis/flowcontrol/validation/validation.go index a3545e0e26b..db3d8d69951 100644 --- a/pkg/apis/flowcontrol/validation/validation.go +++ b/pkg/apis/flowcontrol/validation/validation.go @@ -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"))...) diff --git a/pkg/apis/flowcontrol/validation/validation_test.go b/pkg/apis/flowcontrol/validation/validation_test.go index 98b7e72b7a8..65dcd0e5a71 100644 --- a/pkg/apis/flowcontrol/validation/validation_test.go +++ b/pkg/apis/flowcontrol/validation/validation_test.go @@ -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)) } diff --git a/pkg/features/versioned_kube_features.go b/pkg/features/versioned_kube_features.go index d6351bf2d31..24b47f49ba4 100644 --- a/pkg/features/versioned_kube_features.go +++ b/pkg/features/versioned_kube_features.go @@ -334,11 +334,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}, diff --git a/pkg/registry/flowcontrol/prioritylevelconfiguration/strategy.go b/pkg/registry/flowcontrol/prioritylevelconfiguration/strategy.go index 7f63e3223b6..35f5637636b 100644 --- a/pkg/registry/flowcontrol/prioritylevelconfiguration/strategy.go +++ b/pkg/registry/flowcontrol/prioritylevelconfiguration/strategy.go @@ -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) } diff --git a/staging/src/k8s.io/apiserver/pkg/features/kube_features.go b/staging/src/k8s.io/apiserver/pkg/features/kube_features.go index c17f2849a62..7a698736da5 100644 --- a/staging/src/k8s.io/apiserver/pkg/features/kube_features.go +++ b/staging/src/k8s.io/apiserver/pkg/features/kube_features.go @@ -226,13 +226,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() { @@ -398,11 +391,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. diff --git a/test/featuregates_linter/test_data/versioned_feature_list.yaml b/test/featuregates_linter/test_data/versioned_feature_list.yaml index 251d40830d1..f604a97a5a6 100644 --- a/test/featuregates_linter/test_data/versioned_feature_list.yaml +++ b/test/featuregates_linter/test_data/versioned_feature_list.yaml @@ -1350,13 +1350,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"