Skip to content

Commit e3e70fc

Browse files
MikeSpreitzerMikeSpreitzer
committed
Select APF config by FeatureGate
.. instead of feature values.
1 parent ea7fcda commit e3e70fc

File tree

20 files changed

+295
-243
lines changed

20 files changed

+295
-243
lines changed

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

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ import (
2020
flowcontrolv1 "k8s.io/api/flowcontrol/v1"
2121
"k8s.io/apimachinery/pkg/runtime"
2222
"k8s.io/apiserver/pkg/apis/flowcontrol/bootstrap"
23+
"k8s.io/apiserver/pkg/features"
24+
"k8s.io/component-base/featuregate"
2325
"k8s.io/kubernetes/pkg/apis/flowcontrol"
2426
"k8s.io/kubernetes/pkg/apis/flowcontrol/install"
2527
)
@@ -30,18 +32,20 @@ type FlowSchemasMap = map[string]*flowcontrol.FlowSchema
3032
// GetMandatoryFlowSchemasMap returns the unversioned (internal) mandatory FlowSchema objects,
3133
// as a deeply immutable map.
3234
// The arguments are the values of the features that control which config collection to use.
33-
func GetMandatoryFlowSchemasMap(v134 bool) FlowSchemasMap {
34-
return MandatoryFlowSchemasMap[bootstrap.CollectionID{V134: v134}]
35+
func GetMandatoryFlowSchemasMap(featureGate featuregate.FeatureGate) FlowSchemasMap {
36+
return MandatoryFlowSchemasMap[bootstrap.CollectionID{V134: featureGate.Enabled(features.APFv134Config)}]
3537
}
3638

39+
var oldConfig = bootstrap.GetV1ConfigCollection(bootstrap.MakeGate(false))
40+
3741
// MandatoryFlowSchemasMap holds the unversioned (internal) renditions of the mandatory
3842
// flow schemas. In the outer map the key is CollectionId and
3943
// in the inner map the key is the schema's name and the
4044
// value is the `*FlowSchema`. Nobody should mutate anything
4145
// reachable from this map.
4246
var MandatoryFlowSchemasMap = map[bootstrap.CollectionID]FlowSchemasMap{
43-
{V134: false}: internalizeFSes(bootstrap.GetV1ConfigCollection(false).Mandatory.FlowSchemas),
44-
{V134: true}: internalizeFSes(bootstrap.GetV1ConfigCollection(true).Mandatory.FlowSchemas),
47+
{V134: false}: internalizeFSes(oldConfig.Mandatory.FlowSchemas),
48+
{V134: true}: internalizeFSes(bootstrap.Latest.Mandatory.FlowSchemas),
4549
}
4650

4751
// PriorityLevelConfigurationsMap is a collection of unversioned (internal) PriorityLevelConfiguration objects, indexed by name.
@@ -50,8 +54,8 @@ type PriorityLevelConfigurationsMap = map[string]*flowcontrol.PriorityLevelConfi
5054
// GetMandatoryPriorityLevelConfigurationsMap returns the mandatory PriorityLevelConfiguration objects,
5155
// as a deeply immutable map.
5256
// The arguments are the values of the features that control which config collection to use.
53-
func GetMandatoryPriorityLevelConfigurationsMap(v134 bool) PriorityLevelConfigurationsMap {
54-
return MandatoryPriorityLevelConfigurationsMap[bootstrap.CollectionID{V134: v134}]
57+
func GetMandatoryPriorityLevelConfigurationsMap(featureGate featuregate.FeatureGate) PriorityLevelConfigurationsMap {
58+
return MandatoryPriorityLevelConfigurationsMap[bootstrap.CollectionID{V134: featureGate.Enabled(features.APFv134Config)}]
5559
}
5660

5761
// MandatoryPriorityLevelConfigurationsMap holds the untyped renditions of the
@@ -60,8 +64,8 @@ func GetMandatoryPriorityLevelConfigurationsMap(v134 bool) PriorityLevelConfigur
6064
// `*PriorityLevelConfiguration`. Nobody should mutate anything
6165
// reachable from this map.
6266
var MandatoryPriorityLevelConfigurationsMap = map[bootstrap.CollectionID]PriorityLevelConfigurationsMap{
63-
{V134: false}: internalizePLs(bootstrap.GetV1ConfigCollection(false).Mandatory.PriorityLevelConfigurations),
64-
{V134: true}: internalizePLs(bootstrap.GetV1ConfigCollection(true).Mandatory.PriorityLevelConfigurations),
67+
{V134: false}: internalizePLs(oldConfig.Mandatory.PriorityLevelConfigurations),
68+
{V134: true}: internalizePLs(bootstrap.Latest.Mandatory.PriorityLevelConfigurations),
6569
}
6670

6771
func internalizeFSes(exts []*flowcontrolv1.FlowSchema) FlowSchemasMap {

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

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,17 +25,18 @@ import (
2525
flowcontrol "k8s.io/api/flowcontrol/v1"
2626
apiequality "k8s.io/apimachinery/pkg/api/equality"
2727
"k8s.io/apiserver/pkg/apis/flowcontrol/bootstrap"
28+
"k8s.io/component-base/featuregate"
2829
)
2930

3031
func TestBootstrapConfigurationWithDefaulted(t *testing.T) {
31-
t.Run("false", caseFn(false))
32-
t.Run("true", caseFn(true))
32+
t.Run("v134Config=false", caseFn(bootstrap.MakeGate(false)))
33+
t.Run("v134Config=true", caseFn(bootstrap.LatestFeatureGate))
3334
}
3435

35-
func caseFn(v134 bool) func(*testing.T) {
36+
func caseFn(featureGate featuregate.FeatureGate) func(*testing.T) {
3637
return func(t *testing.T) {
3738
scheme := NewAPFScheme()
38-
bootstrapFlowSchemas := bootstrap.GetFlowSchemas(v134)
39+
bootstrapFlowSchemas := bootstrap.GetFlowSchemas(featureGate)
3940
for _, original := range bootstrapFlowSchemas {
4041
t.Run(fmt.Sprintf("FlowSchema/%s", original.Name), func(t *testing.T) {
4142
defaulted := original.DeepCopyObject().(*flowcontrol.FlowSchema)
@@ -48,7 +49,7 @@ func caseFn(v134 bool) func(*testing.T) {
4849
})
4950
}
5051

51-
bootstrapPriorityLevels := bootstrap.GetPrioritylevelConfigurations(v134)
52+
bootstrapPriorityLevels := bootstrap.GetPrioritylevelConfigurations(featureGate)
5253
for _, original := range bootstrapPriorityLevels {
5354
t.Run(fmt.Sprintf("PriorityLevelConfiguration/%s", original.Name), func(t *testing.T) {
5455
defaulted := original.DeepCopyObject().(*flowcontrol.PriorityLevelConfiguration)

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
"k8s.io/apimachinery/pkg/runtime/schema"
2929
"k8s.io/apimachinery/pkg/util/sets"
3030
"k8s.io/apimachinery/pkg/util/validation/field"
31+
bootstrapv1 "k8s.io/apiserver/pkg/apis/flowcontrol/bootstrap"
3132
"k8s.io/apiserver/pkg/util/shufflesharding"
3233
apivalidation "k8s.io/kubernetes/pkg/apis/core/validation"
3334
"k8s.io/kubernetes/pkg/apis/flowcontrol"
@@ -78,15 +79,18 @@ var supportedLimitResponseType = sets.NewString(
7879
// PriorityLevelValidationOptions holds the validation options for a priority level object
7980
type PriorityLevelValidationOptions struct{}
8081

82+
var oldGate = bootstrapv1.MakeGate(false)
83+
var newGate = bootstrapv1.LatestFeatureGate
84+
8185
// ValidateFlowSchema validates the content of flow-schema
8286
func ValidateFlowSchema(fs *flowcontrol.FlowSchema) field.ErrorList {
8387
allErrs := apivalidation.ValidateObjectMeta(&fs.ObjectMeta, false, ValidateFlowSchemaName, field.NewPath("metadata"))
8488
specPath := field.NewPath("spec")
8589
allErrs = append(allErrs, ValidateFlowSchemaSpec(fs.Name, &fs.Spec, specPath)...)
86-
if mand, ok := internalbootstrap.GetMandatoryFlowSchemasMap(true)[fs.Name]; ok {
90+
if mand, ok := internalbootstrap.GetMandatoryFlowSchemasMap(newGate)[fs.Name]; ok {
8791
// The old config objects are a subset of the new config objects,
8892
// as far as identify is concerned. The specs may differ.
89-
mandOld := internalbootstrap.GetMandatoryFlowSchemasMap(false)[fs.Name]
93+
mandOld := internalbootstrap.GetMandatoryFlowSchemasMap(oldGate)[fs.Name]
9094
// For now, accept either the old or the new spec.
9195
// In a later release, change this to accept only the new spec.
9296
// Check for almost exact equality. This is a pretty
@@ -368,7 +372,7 @@ func ValidatePriorityLevelConfiguration(pl *flowcontrol.PriorityLevelConfigurati
368372

369373
func ValidateIfMandatoryPriorityLevelConfigurationObject(pl *flowcontrol.PriorityLevelConfiguration, fldPath *field.Path) field.ErrorList {
370374
var allErrs field.ErrorList
371-
mand, ok := internalbootstrap.GetMandatoryPriorityLevelConfigurationsMap(true)[pl.Name]
375+
mand, ok := internalbootstrap.GetMandatoryPriorityLevelConfigurationsMap(newGate)[pl.Name]
372376
if !ok {
373377
// The old config objects are a subset of the new, as far as identity is concerned.
374378
return allErrs
@@ -389,7 +393,7 @@ func ValidateIfMandatoryPriorityLevelConfigurationObject(pl *flowcontrol.Priorit
389393

390394
// For now, accept either the old or the new default config.
391395
// In a later release, accept only the new.
392-
mandOld := internalbootstrap.GetMandatoryPriorityLevelConfigurationsMap(false)[pl.Name]
396+
mandOld := internalbootstrap.GetMandatoryPriorityLevelConfigurationsMap(oldGate)[pl.Name]
393397
// Check for almost exact equality. This is a pretty
394398
// strict test, and it is OK in this context because both
395399
// sides of this comparison are intended to ultimately

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -769,7 +769,7 @@ func TestPriorityLevelConfigurationValidation(t *testing.T) {
769769
}
770770

771771
validChangesInExemptFieldOfExemptPLFn := func() flowcontrol.PriorityLevelConfigurationSpec {
772-
have := fcboot.GetV1ConfigCollection(true).PriorityLevelConfigurationExempt
772+
have := fcboot.Latest.PriorityLevelConfigurationExempt
773773
return flowcontrol.PriorityLevelConfigurationSpec{
774774
Type: flowcontrol.PriorityLevelEnablementExempt,
775775
Exempt: &flowcontrol.ExemptPriorityLevelConfiguration{

‎pkg/registry/flowcontrol/rest/storage_flowcontrol.go

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ import (
2525
"k8s.io/apimachinery/pkg/runtime/schema"
2626
"k8s.io/apimachinery/pkg/util/wait"
2727
flowcontrolbootstrap "k8s.io/apiserver/pkg/apis/flowcontrol/bootstrap"
28-
"k8s.io/apiserver/pkg/features"
2928
"k8s.io/apiserver/pkg/registry/generic"
3029
"k8s.io/apiserver/pkg/registry/rest"
3130
genericapiserver "k8s.io/apiserver/pkg/server"
@@ -58,6 +57,8 @@ const PostStartHookName = "priority-and-fairness-config-producer"
5857

5958
// NewRESTStorage creates a new rest storage for flow-control api models.
6059
func (p RESTStorageProvider) NewRESTStorage(apiResourceConfigSource serverstorage.APIResourceConfigSource, restOptionsGetter generic.RESTOptionsGetter) (genericapiserver.APIGroupInfo, error) {
60+
fg := featuregate.NewFeatureGate()
61+
fg.Set("foo=bar")
6162
apiGroupInfo := genericapiserver.NewDefaultAPIGroupInfo(flowcontrol.GroupName, legacyscheme.Scheme, legacyscheme.ParameterCodec, legacyscheme.Codecs)
6263

6364
if storageMap, err := p.storage(apiResourceConfigSource, restOptionsGetter, flowcontrolapisv1beta3.SchemeGroupVersion); err != nil {
@@ -109,7 +110,7 @@ func (p RESTStorageProvider) GroupName() string {
109110
// PostStartHook returns the hook func that launches the config provider
110111
func (p RESTStorageProvider) PostStartHook() (string, genericapiserver.PostStartHookFunc, error) {
111112
bce := &bootstrapConfigurationEnsurer{
112-
v134Config: p.FeatureGate.Enabled(features.APFv134Config),
113+
featureGate: p.FeatureGate,
113114
informersSynced: []cache.InformerSynced{
114115
p.InformerFactory.Flowcontrol().V1().PriorityLevelConfigurations().Informer().HasSynced,
115116
p.InformerFactory.Flowcontrol().V1().FlowSchemas().Informer().HasSynced,
@@ -121,7 +122,7 @@ func (p RESTStorageProvider) PostStartHook() (string, genericapiserver.PostStart
121122
}
122123

123124
type bootstrapConfigurationEnsurer struct {
124-
v134Config bool
125+
featureGate featuregate.FeatureGate
125126
informersSynced []cache.InformerSynced
126127
fsLister flowcontrollisters.FlowSchemaLister
127128
plcLister flowcontrollisters.PriorityLevelConfigurationLister
@@ -145,7 +146,7 @@ func (bce *bootstrapConfigurationEnsurer) ensureAPFBootstrapConfiguration(hookCo
145146

146147
err = wait.PollUntilContextCancel(ctx, time.Second, true,
147148
func(context.Context) (bool, error) {
148-
if err := ensure(ctx, bce.v134Config, clientset, bce.fsLister, bce.plcLister); err != nil {
149+
if err := ensure(ctx, bce.featureGate, clientset, bce.fsLister, bce.plcLister); err != nil {
149150
klog.ErrorS(err, "APF bootstrap ensurer ran into error, will retry later")
150151
return false, nil
151152
}
@@ -165,7 +166,7 @@ func (bce *bootstrapConfigurationEnsurer) ensureAPFBootstrapConfiguration(hookCo
165166
go func() {
166167
_ = wait.PollUntilContextCancel(hookContext, time.Minute, true,
167168
func(ctx context.Context) (bool, error) {
168-
if err := ensure(ctx, bce.v134Config, clientset, bce.fsLister, bce.plcLister); err != nil {
169+
if err := ensure(ctx, bce.featureGate, clientset, bce.fsLister, bce.plcLister); err != nil {
169170
klog.ErrorS(err, "APF bootstrap ensurer ran into error, will retry later")
170171
}
171172
// always auto update both suggested and mandatory configuration
@@ -177,29 +178,29 @@ func (bce *bootstrapConfigurationEnsurer) ensureAPFBootstrapConfiguration(hookCo
177178
return nil
178179
}
179180

180-
func ensure(ctx context.Context, v134Config bool, clientset flowcontrolclient.FlowcontrolV1Interface, fsLister flowcontrollisters.FlowSchemaLister, plcLister flowcontrollisters.PriorityLevelConfigurationLister) error {
181+
func ensure(ctx context.Context, featureGate featuregate.FeatureGate, clientset flowcontrolclient.FlowcontrolV1Interface, fsLister flowcontrollisters.FlowSchemaLister, plcLister flowcontrollisters.PriorityLevelConfigurationLister) error {
181182

182-
if err := ensureSuggestedConfiguration(ctx, v134Config, clientset, fsLister, plcLister); err != nil {
183+
if err := ensureSuggestedConfiguration(ctx, featureGate, clientset, fsLister, plcLister); err != nil {
183184
// We should not attempt creation of mandatory objects if ensuring the suggested
184185
// configuration resulted in an error.
185186
// This only happens when the stop channel is closed.
186187
return fmt.Errorf("failed ensuring suggested settings - %w", err)
187188
}
188189

189-
if err := ensureMandatoryConfiguration(ctx, v134Config, clientset, fsLister, plcLister); err != nil {
190+
if err := ensureMandatoryConfiguration(ctx, featureGate, clientset, fsLister, plcLister); err != nil {
190191
return fmt.Errorf("failed ensuring mandatory settings - %w", err)
191192
}
192193

193-
if err := removeDanglingBootstrapConfiguration(ctx, v134Config, clientset, fsLister, plcLister); err != nil {
194+
if err := removeDanglingBootstrapConfiguration(ctx, featureGate, clientset, fsLister, plcLister); err != nil {
194195
return fmt.Errorf("failed to delete removed settings - %w", err)
195196
}
196197

197198
return nil
198199
}
199200

200-
func ensureSuggestedConfiguration(ctx context.Context, v134Config bool, clientset flowcontrolclient.FlowcontrolV1Interface, fsLister flowcontrollisters.FlowSchemaLister, plcLister flowcontrollisters.PriorityLevelConfigurationLister) error {
201+
func ensureSuggestedConfiguration(ctx context.Context, featureGate featuregate.FeatureGate, clientset flowcontrolclient.FlowcontrolV1Interface, fsLister flowcontrollisters.FlowSchemaLister, plcLister flowcontrollisters.PriorityLevelConfigurationLister) error {
201202
plcOps := ensurer.NewPriorityLevelConfigurationOps(clientset.PriorityLevelConfigurations(), plcLister)
202-
config := flowcontrolbootstrap.GetV1ConfigCollection(v134Config).Suggested
203+
config := flowcontrolbootstrap.GetV1ConfigCollection(featureGate).Suggested
203204
if err := ensurer.EnsureConfigurations(ctx, plcOps, config.PriorityLevelConfigurations, ensurer.NewSuggestedEnsureStrategy[*flowcontrolv1.PriorityLevelConfiguration]()); err != nil {
204205
return err
205206
}
@@ -208,9 +209,9 @@ func ensureSuggestedConfiguration(ctx context.Context, v134Config bool, clientse
208209
return ensurer.EnsureConfigurations(ctx, fsOps, config.FlowSchemas, ensurer.NewSuggestedEnsureStrategy[*flowcontrolv1.FlowSchema]())
209210
}
210211

211-
func ensureMandatoryConfiguration(ctx context.Context, v134Config bool, clientset flowcontrolclient.FlowcontrolV1Interface, fsLister flowcontrollisters.FlowSchemaLister, plcLister flowcontrollisters.PriorityLevelConfigurationLister) error {
212+
func ensureMandatoryConfiguration(ctx context.Context, featureGate featuregate.FeatureGate, clientset flowcontrolclient.FlowcontrolV1Interface, fsLister flowcontrollisters.FlowSchemaLister, plcLister flowcontrollisters.PriorityLevelConfigurationLister) error {
212213
plcOps := ensurer.NewPriorityLevelConfigurationOps(clientset.PriorityLevelConfigurations(), plcLister)
213-
config := flowcontrolbootstrap.GetV1ConfigCollection(v134Config).Mandatory
214+
config := flowcontrolbootstrap.GetV1ConfigCollection(featureGate).Mandatory
214215
if err := ensurer.EnsureConfigurations(ctx, plcOps, config.PriorityLevelConfigurations, ensurer.NewMandatoryEnsureStrategy[*flowcontrolv1.PriorityLevelConfiguration]()); err != nil {
215216
return err
216217
}
@@ -219,22 +220,22 @@ func ensureMandatoryConfiguration(ctx context.Context, v134Config bool, clientse
219220
return ensurer.EnsureConfigurations(ctx, fsOps, config.FlowSchemas, ensurer.NewMandatoryEnsureStrategy[*flowcontrolv1.FlowSchema]())
220221
}
221222

222-
func removeDanglingBootstrapConfiguration(ctx context.Context, v134Config bool, clientset flowcontrolclient.FlowcontrolV1Interface, fsLister flowcontrollisters.FlowSchemaLister, plcLister flowcontrollisters.PriorityLevelConfigurationLister) error {
223-
if err := removeDanglingBootstrapFlowSchema(ctx, v134Config, clientset, fsLister); err != nil {
223+
func removeDanglingBootstrapConfiguration(ctx context.Context, featureGate featuregate.FeatureGate, clientset flowcontrolclient.FlowcontrolV1Interface, fsLister flowcontrollisters.FlowSchemaLister, plcLister flowcontrollisters.PriorityLevelConfigurationLister) error {
224+
if err := removeDanglingBootstrapFlowSchema(ctx, featureGate, clientset, fsLister); err != nil {
224225
return err
225226
}
226227

227-
return removeDanglingBootstrapPriorityLevel(ctx, v134Config, clientset, plcLister)
228+
return removeDanglingBootstrapPriorityLevel(ctx, featureGate, clientset, plcLister)
228229
}
229230

230-
func removeDanglingBootstrapFlowSchema(ctx context.Context, v134Config bool, clientset flowcontrolclient.FlowcontrolV1Interface, fsLister flowcontrollisters.FlowSchemaLister) error {
231+
func removeDanglingBootstrapFlowSchema(ctx context.Context, featureGate featuregate.FeatureGate, clientset flowcontrolclient.FlowcontrolV1Interface, fsLister flowcontrollisters.FlowSchemaLister) error {
231232
fsOps := ensurer.NewFlowSchemaOps(clientset.FlowSchemas(), fsLister)
232-
return ensurer.RemoveUnwantedObjects(ctx, fsOps, flowcontrolbootstrap.GetFlowSchemas(v134Config))
233+
return ensurer.RemoveUnwantedObjects(ctx, fsOps, flowcontrolbootstrap.GetFlowSchemas(featureGate))
233234
}
234235

235-
func removeDanglingBootstrapPriorityLevel(ctx context.Context, v134Config bool, clientset flowcontrolclient.FlowcontrolV1Interface, plcLister flowcontrollisters.PriorityLevelConfigurationLister) error {
236+
func removeDanglingBootstrapPriorityLevel(ctx context.Context, featureGate featuregate.FeatureGate, clientset flowcontrolclient.FlowcontrolV1Interface, plcLister flowcontrollisters.PriorityLevelConfigurationLister) error {
236237
plcOps := ensurer.NewPriorityLevelConfigurationOps(clientset.PriorityLevelConfigurations(), plcLister)
237-
return ensurer.RemoveUnwantedObjects(ctx, plcOps, flowcontrolbootstrap.GetPrioritylevelConfigurations(v134Config))
238+
return ensurer.RemoveUnwantedObjects(ctx, plcOps, flowcontrolbootstrap.GetPrioritylevelConfigurations(featureGate))
238239
}
239240

240241
// contextFromChannelAndMaxWaitDuration returns a Context that is bound to the

0 commit comments

Comments
 (0)