Skip to content

Commit 6605f14

Browse files
MikeSpreitzerMikeSpreitzer
committed
Introduce FeatureGate controlling which APF config to use
1 parent 71ed39a commit 6605f14

File tree

25 files changed

+902
-84
lines changed

25 files changed

+902
-84
lines changed

‎pkg/apis/flowcontrol/internalbootstrap/default-internal.go

Lines changed: 37 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,23 +19,54 @@ package internalbootstrap
1919
import (
2020
flowcontrolv1 "k8s.io/api/flowcontrol/v1"
2121
"k8s.io/apimachinery/pkg/runtime"
22-
"k8s.io/apiserver/pkg/apis/flowcontrol/bootstrap"
22+
bootstrapnew "k8s.io/apiserver/pkg/apis/flowcontrol/bootstrap-new"
23+
bootstrapold "k8s.io/apiserver/pkg/apis/flowcontrol/bootstrap-old"
2324
"k8s.io/kubernetes/pkg/apis/flowcontrol"
2425
"k8s.io/kubernetes/pkg/apis/flowcontrol/install"
2526
)
2627

28+
// Concat concatenates the given slices, without reusing any given backing array.
29+
func Concat[T any](slices ...[]T) []T {
30+
var ans []T
31+
for _, slice := range slices {
32+
ans = append(ans, slice...)
33+
}
34+
return ans
35+
}
36+
37+
// PrioritylevelConfigurations maps `newConfig bool` to a slice of all the default objects
38+
// Deeply immutable.
39+
var PrioritylevelConfigurations = map[bool][]*flowcontrolv1.PriorityLevelConfiguration{
40+
false: Concat(bootstrapold.MandatoryPriorityLevelConfigurations, bootstrapold.SuggestedPriorityLevelConfigurations),
41+
true: Concat(bootstrapnew.MandatoryPriorityLevelConfigurations, bootstrapnew.SuggestedPriorityLevelConfigurations),
42+
}
43+
44+
// FlowSchemas maps `newConfig bool` to a slice of all the default objects
45+
// Deeply immutable.
46+
var FlowSchemas = map[bool][]*flowcontrolv1.FlowSchema{
47+
false: Concat(bootstrapold.MandatoryFlowSchemas, bootstrapold.SuggestedFlowSchemas),
48+
true: Concat(bootstrapnew.MandatoryFlowSchemas, bootstrapnew.SuggestedFlowSchemas),
49+
}
50+
2751
// MandatoryFlowSchemas holds the untyped renditions of the mandatory
28-
// flow schemas. In this map the key is the schema's name and the
52+
// flow schemas. In the outer map the key is `newConfig bool` and
53+
// in the inner map the key is the schema's name and the
2954
// value is the `*FlowSchema`. Nobody should mutate anything
3055
// reachable from this map.
31-
var MandatoryFlowSchemas = internalizeFSes(bootstrap.MandatoryFlowSchemas)
56+
var MandatoryFlowSchemas = map[bool]map[string]*flowcontrol.FlowSchema{
57+
false: internalizeFSes(bootstrapold.MandatoryFlowSchemas),
58+
true: internalizeFSes(bootstrapnew.MandatoryFlowSchemas),
59+
}
3260

3361
// MandatoryPriorityLevelConfigurations holds the untyped renditions of the
34-
// mandatory priority level configuration objects. In this map the
35-
// key is the object's name and the value is the
62+
// mandatory priority level configuration objects. In the outer map the key is `newConfig bool` and
63+
// in the inner map the key is the object's name and the value is the
3664
// `*PriorityLevelConfiguration`. Nobody should mutate anything
3765
// reachable from this map.
38-
var MandatoryPriorityLevelConfigurations = internalizePLs(bootstrap.MandatoryPriorityLevelConfigurations)
66+
var MandatoryPriorityLevelConfigurations = map[bool]map[string]*flowcontrol.PriorityLevelConfiguration{
67+
false: internalizePLs(bootstrapold.MandatoryPriorityLevelConfigurations),
68+
true: internalizePLs(bootstrapnew.MandatoryPriorityLevelConfigurations),
69+
}
3970

4071
// NewAPFScheme constructs and returns a Scheme configured to handle
4172
// the API object types that are used to configure API Priority and

‎pkg/apis/flowcontrol/internalbootstrap/defaults_test.go

Lines changed: 31 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -24,39 +24,40 @@ import (
2424

2525
flowcontrol "k8s.io/api/flowcontrol/v1"
2626
apiequality "k8s.io/apimachinery/pkg/api/equality"
27-
"k8s.io/apiserver/pkg/apis/flowcontrol/bootstrap"
2827
)
2928

3029
func TestBootstrapConfigurationWithDefaulted(t *testing.T) {
31-
scheme := NewAPFScheme()
32-
33-
bootstrapFlowSchemas := make([]*flowcontrol.FlowSchema, 0)
34-
bootstrapFlowSchemas = append(bootstrapFlowSchemas, bootstrap.MandatoryFlowSchemas...)
35-
bootstrapFlowSchemas = append(bootstrapFlowSchemas, bootstrap.SuggestedFlowSchemas...)
36-
for _, original := range bootstrapFlowSchemas {
37-
t.Run(fmt.Sprintf("FlowSchema/%s", original.Name), func(t *testing.T) {
38-
defaulted := original.DeepCopyObject().(*flowcontrol.FlowSchema)
39-
scheme.Default(defaulted)
40-
if apiequality.Semantic.DeepEqual(original, defaulted) {
41-
t.Logf("Defaulting makes no change to FlowSchema: %q", original.Name)
42-
return
43-
}
44-
t.Errorf("Expected defaulting to not change FlowSchema: %q, diff: %s", original.Name, cmp.Diff(original, defaulted))
45-
})
46-
}
30+
t.Run("false", caseFn(false))
31+
t.Run("true", caseFn(true))
32+
}
4733

48-
bootstrapPriorityLevels := make([]*flowcontrol.PriorityLevelConfiguration, 0)
49-
bootstrapPriorityLevels = append(bootstrapPriorityLevels, bootstrap.MandatoryPriorityLevelConfigurations...)
50-
bootstrapPriorityLevels = append(bootstrapPriorityLevels, bootstrap.SuggestedPriorityLevelConfigurations...)
51-
for _, original := range bootstrapPriorityLevels {
52-
t.Run(fmt.Sprintf("PriorityLevelConfiguration/%s", original.Name), func(t *testing.T) {
53-
defaulted := original.DeepCopyObject().(*flowcontrol.PriorityLevelConfiguration)
54-
scheme.Default(defaulted)
55-
if apiequality.Semantic.DeepEqual(original, defaulted) {
56-
t.Logf("Defaulting makes no change to PriorityLevelConfiguration: %q", original.Name)
57-
return
58-
}
59-
t.Errorf("Expected defaulting to not change PriorityLevelConfiguration: %q, diff: %s", original.Name, cmp.Diff(original, defaulted))
60-
})
34+
func caseFn(newConfig bool) func(*testing.T) {
35+
return func(t *testing.T) {
36+
scheme := NewAPFScheme()
37+
bootstrapFlowSchemas := FlowSchemas[newConfig]
38+
for _, original := range bootstrapFlowSchemas {
39+
t.Run(fmt.Sprintf("FlowSchema/%s", original.Name), func(t *testing.T) {
40+
defaulted := original.DeepCopyObject().(*flowcontrol.FlowSchema)
41+
scheme.Default(defaulted)
42+
if apiequality.Semantic.DeepEqual(original, defaulted) {
43+
t.Logf("Defaulting makes no change to FlowSchema: %q", original.Name)
44+
return
45+
}
46+
t.Errorf("Expected defaulting to not change FlowSchema: %q, diff: %s", original.Name, cmp.Diff(original, defaulted))
47+
})
48+
}
49+
50+
bootstrapPriorityLevels := PrioritylevelConfigurations[newConfig]
51+
for _, original := range bootstrapPriorityLevels {
52+
t.Run(fmt.Sprintf("PriorityLevelConfiguration/%s", original.Name), func(t *testing.T) {
53+
defaulted := original.DeepCopyObject().(*flowcontrol.PriorityLevelConfiguration)
54+
scheme.Default(defaulted)
55+
if apiequality.Semantic.DeepEqual(original, defaulted) {
56+
t.Logf("Defaulting makes no change to PriorityLevelConfiguration: %q", original.Name)
57+
return
58+
}
59+
t.Errorf("Expected defaulting to not change PriorityLevelConfiguration: %q, diff: %s", original.Name, cmp.Diff(original, defaulted))
60+
})
61+
}
6162
}
6263
}

‎pkg/apis/flowcontrol/validation/validation.go

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -83,13 +83,18 @@ func ValidateFlowSchema(fs *flowcontrol.FlowSchema) field.ErrorList {
8383
allErrs := apivalidation.ValidateObjectMeta(&fs.ObjectMeta, false, ValidateFlowSchemaName, field.NewPath("metadata"))
8484
specPath := field.NewPath("spec")
8585
allErrs = append(allErrs, ValidateFlowSchemaSpec(fs.Name, &fs.Spec, specPath)...)
86-
if mand, ok := internalbootstrap.MandatoryFlowSchemas[fs.Name]; ok {
86+
if mand, ok := internalbootstrap.MandatoryFlowSchemas[true][fs.Name]; ok {
87+
// The old config objects are a subset of the new config objects,
88+
// as far as identify is concerned. The specs may differ.
89+
mandOld := internalbootstrap.MandatoryFlowSchemas[false][fs.Name]
90+
// For now, accept either the old or the new spec.
91+
// In a later release, change this to accept only the new spec.
8792
// Check for almost exact equality. This is a pretty
8893
// strict test, and it is OK in this context because both
8994
// sides of this comparison are intended to ultimately
9095
// come from the same code.
91-
if !apiequality.Semantic.DeepEqual(fs.Spec, mand.Spec) {
92-
allErrs = append(allErrs, field.Invalid(specPath, fs.Spec, fmt.Sprintf("spec of '%s' must equal the fixed value", fs.Name)))
96+
if !(apiequality.Semantic.DeepEqual(fs.Spec, mand.Spec) || apiequality.Semantic.DeepEqual(fs.Spec, mandOld.Spec)) {
97+
allErrs = append(allErrs, field.Invalid(specPath, fs.Spec, fmt.Sprintf("spec of '%s' must equal the old or new fixed value", fs.Name)))
9398
}
9499
}
95100
allErrs = append(allErrs, ValidateFlowSchemaStatus(&fs.Status, field.NewPath("status"))...)
@@ -363,8 +368,9 @@ func ValidatePriorityLevelConfiguration(pl *flowcontrol.PriorityLevelConfigurati
363368

364369
func ValidateIfMandatoryPriorityLevelConfigurationObject(pl *flowcontrol.PriorityLevelConfiguration, fldPath *field.Path) field.ErrorList {
365370
var allErrs field.ErrorList
366-
mand, ok := internalbootstrap.MandatoryPriorityLevelConfigurations[pl.Name]
371+
mand, ok := internalbootstrap.MandatoryPriorityLevelConfigurations[true][pl.Name]
367372
if !ok {
373+
// The old config objects are a subset of the new, as far as identity is concerned.
368374
return allErrs
369375
}
370376

@@ -381,12 +387,15 @@ func ValidateIfMandatoryPriorityLevelConfigurationObject(pl *flowcontrol.Priorit
381387
return allErrs
382388
}
383389

390+
// For now, accept either the old or the new default config.
391+
// In a later release, accept only the new.
392+
mandOld := internalbootstrap.MandatoryPriorityLevelConfigurations[false][pl.Name]
384393
// Check for almost exact equality. This is a pretty
385394
// strict test, and it is OK in this context because both
386395
// sides of this comparison are intended to ultimately
387396
// come from the same code.
388-
if !apiequality.Semantic.DeepEqual(pl.Spec, mand.Spec) {
389-
allErrs = append(allErrs, field.Invalid(fldPath, pl.Spec, fmt.Sprintf("spec of '%s' must equal the fixed value", pl.Name)))
397+
if !(apiequality.Semantic.DeepEqual(pl.Spec, mand.Spec) || apiequality.Semantic.DeepEqual(pl.Spec, mandOld.Spec)) {
398+
allErrs = append(allErrs, field.Invalid(fldPath, pl.Spec, fmt.Sprintf("spec of '%s' must equal the old or new fixed value", pl.Name)))
390399
}
391400
return allErrs
392401
}

‎pkg/apis/flowcontrol/validation/validation_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,7 @@ func TestFlowSchemaValidation(t *testing.T) {
263263
},
264264
Spec: badExempt,
265265
},
266-
expectedErrors: field.ErrorList{field.Invalid(field.NewPath("spec"), badExempt, "spec of 'exempt' must equal the fixed value")},
266+
expectedErrors: field.ErrorList{field.Invalid(field.NewPath("spec"), badExempt, "spec of 'exempt' must equal the old or new fixed value")},
267267
}, {
268268
name: "bad catch-all flow-schema should fail",
269269
flowSchema: &flowcontrol.FlowSchema{
@@ -272,7 +272,7 @@ func TestFlowSchemaValidation(t *testing.T) {
272272
},
273273
Spec: badCatchAll,
274274
},
275-
expectedErrors: field.ErrorList{field.Invalid(field.NewPath("spec"), badCatchAll, "spec of 'catch-all' must equal the fixed value")},
275+
expectedErrors: field.ErrorList{field.Invalid(field.NewPath("spec"), badCatchAll, "spec of 'catch-all' must equal the old or new fixed value")},
276276
}, {
277277
name: "catch-all flow-schema should work",
278278
flowSchema: &flowcontrol.FlowSchema{
@@ -769,7 +769,7 @@ func TestPriorityLevelConfigurationValidation(t *testing.T) {
769769
}
770770

771771
validChangesInExemptFieldOfExemptPLFn := func() flowcontrol.PriorityLevelConfigurationSpec {
772-
have, _ := internalbootstrap.MandatoryPriorityLevelConfigurations[flowcontrol.PriorityLevelConfigurationNameExempt]
772+
have := internalbootstrap.MandatoryPriorityLevelConfigurations[true][flowcontrol.PriorityLevelConfigurationNameExempt]
773773
return flowcontrol.PriorityLevelConfigurationSpec{
774774
Type: flowcontrol.PriorityLevelEnablementExempt,
775775
Exempt: &flowcontrol.ExemptPriorityLevelConfiguration{
@@ -949,7 +949,7 @@ func TestPriorityLevelConfigurationValidation(t *testing.T) {
949949
},
950950
Spec: badSpec,
951951
},
952-
expectedErrors: field.ErrorList{field.Invalid(field.NewPath("spec"), badSpec, "spec of 'catch-all' must equal the fixed value")},
952+
expectedErrors: field.ErrorList{field.Invalid(field.NewPath("spec"), badSpec, "spec of 'catch-all' must equal the old or new fixed value")},
953953
}, {
954954
name: "backstop should work",
955955
priorityLevelConfiguration: &flowcontrol.PriorityLevelConfiguration{

‎pkg/controlplane/apiserver/apis.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ func (c *CompletedConfig) GenericStorageProviders(discovery discovery.DiscoveryI
7878
coordinationrest.RESTStorageProvider{},
7979
rbacrest.RESTStorageProvider{Authorizer: c.Generic.Authorization.Authorizer},
8080
svmrest.RESTStorageProvider{},
81-
flowcontrolrest.RESTStorageProvider{InformerFactory: c.Generic.SharedInformerFactory},
81+
flowcontrolrest.RESTStorageProvider{InformerFactory: c.Generic.SharedInformerFactory, FeatureGate: c.Generic.FeatureGate},
8282
admissionregistrationrest.RESTStorageProvider{Authorizer: c.Generic.Authorization.Authorizer, DiscoveryClient: discovery},
8383
eventsrest.RESTStorageProvider{TTL: c.EventTTL},
8484
}, nil

‎pkg/controlplane/instance.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -419,7 +419,7 @@ func (c CompletedConfig) StorageProviders(discovery clientdiscovery.DiscoveryInt
419419
schedulingrest.RESTStorageProvider{},
420420
storagerest.RESTStorageProvider{},
421421
svmrest.RESTStorageProvider{},
422-
flowcontrolrest.RESTStorageProvider{InformerFactory: c.ControlPlane.Generic.SharedInformerFactory},
422+
flowcontrolrest.RESTStorageProvider{InformerFactory: c.ControlPlane.Generic.SharedInformerFactory, FeatureGate: c.ControlPlane.Generic.FeatureGate},
423423
// keep apps after extensions so legacy clients resolve the extensions versions of shared resource names.
424424
// See https://github.com/kubernetes/kubernetes/issues/42392
425425
appsrest.StorageProvider{},

‎pkg/features/kube_features.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,16 @@ const (
5858
// Enables usage of any object for volume data source in PVCs
5959
AnyVolumeDataSource featuregate.Feature = "AnyVolumeDataSource"
6060

61+
// owner: @MikeSpreitzer, @tkashem, @linxiulei
62+
// beta: 1.33
63+
//
64+
// Make API Priority and Fairness use modern configuration, which
65+
// differs from the old in these ways:
66+
// - introduce priority level and flow schema for events;
67+
// - generally reorganize to stop working around lack of borrowing;
68+
// - increase the nominal concurrency shares for leader election.
69+
APFNewConfig featuregate.Feature = "APFNewConifg"
70+
6171
// owner: @tallclair
6272
AppArmor featuregate.Feature = "AppArmor"
6373

‎pkg/features/versioned_kube_features.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,10 @@ var defaultVersionedKubernetesFeatureGates = map[featuregate.Feature]featuregate
6363
{Version: version.MustParse("1.33"), Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // GA in 1.33 -> remove in 1.36
6464
},
6565

66+
APFNewConfig: {
67+
{Version: version.MustParse("1.33"), Default: true, PreRelease: featuregate.Beta},
68+
},
69+
6670
AppArmor: {
6771
{Version: version.MustParse("1.4"), Default: true, PreRelease: featuregate.Beta},
6872
{Version: version.MustParse("1.31"), Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.33

‎pkg/registry/flowcontrol/ensurer/flowschema_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import (
2424
flowcontrolv1 "k8s.io/api/flowcontrol/v1"
2525
apierrors "k8s.io/apimachinery/pkg/api/errors"
2626
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
27-
"k8s.io/apiserver/pkg/apis/flowcontrol/bootstrap"
27+
bootstrap "k8s.io/apiserver/pkg/apis/flowcontrol/bootstrap-new"
2828
"k8s.io/client-go/kubernetes/fake"
2929
flowcontrollisters "k8s.io/client-go/listers/flowcontrol/v1"
3030
toolscache "k8s.io/client-go/tools/cache"

‎pkg/registry/flowcontrol/ensurer/prioritylevelconfiguration_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import (
2424
flowcontrolv1 "k8s.io/api/flowcontrol/v1"
2525
apierrors "k8s.io/apimachinery/pkg/api/errors"
2626
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
27-
"k8s.io/apiserver/pkg/apis/flowcontrol/bootstrap"
27+
bootstrap "k8s.io/apiserver/pkg/apis/flowcontrol/bootstrap-new"
2828
"k8s.io/client-go/kubernetes/fake"
2929
flowcontrollisters "k8s.io/client-go/listers/flowcontrol/v1"
3030
toolscache "k8s.io/client-go/tools/cache"

0 commit comments

Comments
 (0)