diff --git a/pkg/registry/flowcontrol/ensurer/flowschema.go b/pkg/registry/flowcontrol/ensurer/flowschema.go index dbbfc5eb10d..2ddceb287bf 100644 --- a/pkg/registry/flowcontrol/ensurer/flowschema.go +++ b/pkg/registry/flowcontrol/ensurer/flowschema.go @@ -41,9 +41,14 @@ type FlowSchemaEnsurer interface { Ensure([]*flowcontrolv1beta2.FlowSchema) error } -// FlowSchemaRemover removes the specified bootstrap configuration objects +// FlowSchemaRemover is the interface that wraps the +// RemoveAutoUpdateEnabledObjects method. +// +// RemoveAutoUpdateEnabledObjects removes a set of bootstrap FlowSchema +// objects specified via their names. The function removes an object +// only if automatic update of the spec is enabled for it. type FlowSchemaRemover interface { - Remove([]string) error + RemoveAutoUpdateEnabledObjects([]string) error } // NewSuggestedFlowSchemaEnsurer returns a FlowSchemaEnsurer instance that @@ -83,11 +88,11 @@ func NewFlowSchemaRemover(client flowcontrolclient.FlowSchemaInterface, lister f } } -// GetFlowSchemaRemoveCandidate returns a list of FlowSchema object +// GetFlowSchemaRemoveCandidates returns a list of FlowSchema object // names that are candidates for deletion from the cluster. // bootstrap: a set of hard coded FlowSchema configuration objects // kube-apiserver maintains in-memory. -func GetFlowSchemaRemoveCandidate(lister flowcontrollisters.FlowSchemaLister, bootstrap []*flowcontrolv1beta2.FlowSchema) ([]string, error) { +func GetFlowSchemaRemoveCandidates(lister flowcontrollisters.FlowSchemaLister, bootstrap []*flowcontrolv1beta2.FlowSchema) ([]string, error) { fsList, err := lister.List(labels.Everything()) if err != nil { return nil, fmt.Errorf("failed to list FlowSchema - %w", err) @@ -103,7 +108,7 @@ func GetFlowSchemaRemoveCandidate(lister flowcontrollisters.FlowSchemaLister, bo currentObjects[i] = fsList[i] } - return getRemoveCandidate(bootstrapNames, currentObjects), nil + return getDanglingBootstrapObjectNames(bootstrapNames, currentObjects), nil } type fsEnsurer struct { @@ -121,9 +126,9 @@ func (e *fsEnsurer) Ensure(flowSchemas []*flowcontrolv1beta2.FlowSchema) error { return nil } -func (e *fsEnsurer) Remove(flowSchemas []string) error { +func (e *fsEnsurer) RemoveAutoUpdateEnabledObjects(flowSchemas []string) error { for _, flowSchema := range flowSchemas { - if err := removeConfiguration(e.wrapper, flowSchema); err != nil { + if err := removeAutoUpdateEnabledConfiguration(e.wrapper, flowSchema); err != nil { return err } } diff --git a/pkg/registry/flowcontrol/ensurer/flowschema_test.go b/pkg/registry/flowcontrol/ensurer/flowschema_test.go index ef2a82e2ca3..1828cce89fa 100644 --- a/pkg/registry/flowcontrol/ensurer/flowschema_test.go +++ b/pkg/registry/flowcontrol/ensurer/flowschema_test.go @@ -322,7 +322,7 @@ func TestRemoveFlowSchema(t *testing.T) { } remover := NewFlowSchemaRemover(client, flowcontrollisters.NewFlowSchemaLister(indexer)) - err := remover.Remove([]string{test.bootstrapName}) + err := remover.RemoveAutoUpdateEnabledObjects([]string{test.bootstrapName}) if err != nil { t.Fatalf("Expected no error, but got: %v", err) } @@ -410,7 +410,7 @@ func TestGetFlowSchemaRemoveCandidate(t *testing.T) { } lister := flowcontrollisters.NewFlowSchemaLister(indexer) - removeListGot, err := GetFlowSchemaRemoveCandidate(lister, test.bootstrap) + removeListGot, err := GetFlowSchemaRemoveCandidates(lister, test.bootstrap) if err != nil { t.Fatalf("Expected no error, but got: %v", err) } diff --git a/pkg/registry/flowcontrol/ensurer/prioritylevelconfiguration.go b/pkg/registry/flowcontrol/ensurer/prioritylevelconfiguration.go index e75bf16eaef..aa9bcbea8e0 100644 --- a/pkg/registry/flowcontrol/ensurer/prioritylevelconfiguration.go +++ b/pkg/registry/flowcontrol/ensurer/prioritylevelconfiguration.go @@ -41,9 +41,15 @@ type PriorityLevelEnsurer interface { Ensure([]*flowcontrolv1beta2.PriorityLevelConfiguration) error } -// PriorityLevelRemover removes the specified bootstrap configuration objects +// PriorityLevelRemover is the interface that wraps the +// RemoveAutoUpdateEnabledObjects method. +// +// RemoveAutoUpdateEnabledObjects removes a set of bootstrap +// PriorityLevelConfiguration objects specified via their names. +// The function removes an object only if automatic update +// of the spec is enabled for it. type PriorityLevelRemover interface { - Remove([]string) error + RemoveAutoUpdateEnabledObjects([]string) error } // NewSuggestedPriorityLevelEnsurerEnsurer returns a PriorityLevelEnsurer instance that @@ -83,11 +89,11 @@ func NewPriorityLevelRemover(client flowcontrolclient.PriorityLevelConfiguration } } -// GetPriorityLevelRemoveCandidate returns a list of PriorityLevelConfiguration +// GetPriorityLevelRemoveCandidates returns a list of PriorityLevelConfiguration // names that are candidates for removal from the cluster. // bootstrap: a set of hard coded PriorityLevelConfiguration configuration // objects kube-apiserver maintains in-memory. -func GetPriorityLevelRemoveCandidate(lister flowcontrollisters.PriorityLevelConfigurationLister, bootstrap []*flowcontrolv1beta2.PriorityLevelConfiguration) ([]string, error) { +func GetPriorityLevelRemoveCandidates(lister flowcontrollisters.PriorityLevelConfigurationLister, bootstrap []*flowcontrolv1beta2.PriorityLevelConfiguration) ([]string, error) { plList, err := lister.List(labels.Everything()) if err != nil { return nil, fmt.Errorf("failed to list PriorityLevelConfiguration - %w", err) @@ -103,7 +109,7 @@ func GetPriorityLevelRemoveCandidate(lister flowcontrollisters.PriorityLevelConf currentObjects[i] = plList[i] } - return getRemoveCandidate(bootstrapNames, currentObjects), nil + return getDanglingBootstrapObjectNames(bootstrapNames, currentObjects), nil } type plEnsurer struct { @@ -121,9 +127,9 @@ func (e *plEnsurer) Ensure(priorityLevels []*flowcontrolv1beta2.PriorityLevelCon return nil } -func (e *plEnsurer) Remove(priorityLevels []string) error { +func (e *plEnsurer) RemoveAutoUpdateEnabledObjects(priorityLevels []string) error { for _, priorityLevel := range priorityLevels { - if err := removeConfiguration(e.wrapper, priorityLevel); err != nil { + if err := removeAutoUpdateEnabledConfiguration(e.wrapper, priorityLevel); err != nil { return err } } diff --git a/pkg/registry/flowcontrol/ensurer/prioritylevelconfiguration_test.go b/pkg/registry/flowcontrol/ensurer/prioritylevelconfiguration_test.go index a0da43a8ebd..f97f141c4e5 100644 --- a/pkg/registry/flowcontrol/ensurer/prioritylevelconfiguration_test.go +++ b/pkg/registry/flowcontrol/ensurer/prioritylevelconfiguration_test.go @@ -338,7 +338,7 @@ func TestRemovePriorityLevelConfiguration(t *testing.T) { } remover := NewPriorityLevelRemover(client, flowcontrollisters.NewPriorityLevelConfigurationLister(indexer)) - err := remover.Remove([]string{test.bootstrapName}) + err := remover.RemoveAutoUpdateEnabledObjects([]string{test.bootstrapName}) if err != nil { t.Fatalf("Expected no error, but got: %v", err) } @@ -426,7 +426,7 @@ func TestGetPriorityLevelRemoveCandidate(t *testing.T) { } lister := flowcontrollisters.NewPriorityLevelConfigurationLister(indexer) - removeListGot, err := GetPriorityLevelRemoveCandidate(lister, test.bootstrap) + removeListGot, err := GetPriorityLevelRemoveCandidates(lister, test.bootstrap) if err != nil { t.Fatalf("Expected no error, but got: %v", err) } diff --git a/pkg/registry/flowcontrol/ensurer/strategy.go b/pkg/registry/flowcontrol/ensurer/strategy.go index 7b92efe11b6..d118a20fbde 100644 --- a/pkg/registry/flowcontrol/ensurer/strategy.go +++ b/pkg/registry/flowcontrol/ensurer/strategy.go @@ -268,7 +268,9 @@ func ensureConfiguration(wrapper configurationWrapper, strategy ensureStrategy, return fmt.Errorf("failed to update the %s, will retry later type=%s name=%q error=%w", wrapper.TypeName(), configurationType, name, err) } -func removeConfiguration(wrapper configurationWrapper, name string) error { +// removeAutoUpdateEnabledConfiguration makes an attempt to remove the given +// configuration object if automatic update of the spec is enabled for this object. +func removeAutoUpdateEnabledConfiguration(wrapper configurationWrapper, name string) error { current, err := wrapper.Get(name) if err != nil { if apierrors.IsNotFound(err) { @@ -305,16 +307,17 @@ func removeConfiguration(wrapper configurationWrapper, name string) error { return nil } -// getRemoveCandidate returns a list of configuration objects we should delete -// from the cluster given a set of bootstrap and current configuration. -// bootstrap: a set of hard coded configuration kube-apiserver maintains in-memory. -// current: a set of configuration objects that exist on the cluster -// Any object present in current is a candidate for removal if both a and b are true: +// getDanglingBootstrapObjectNames returns a list of names of bootstrap +// configuration objects that are potentially candidates for deletion from +// the cluster, given a set of bootstrap and current configuration. +// - bootstrap: a set of hard coded configuration kube-apiserver maintains in-memory. +// - current: a set of configuration objects that exist on the cluster +// Any object present in current is added to the list if both a and b are true: // a. the object in current is missing from the bootstrap configuration // b. the object has the designated auto-update annotation key -// This function shares the common logic for both FlowSchema and PriorityLevelConfiguration -// type and hence it accepts metav1.Object only. -func getRemoveCandidate(bootstrap sets.String, current []metav1.Object) []string { +// This function shares the common logic for both FlowSchema and +// PriorityLevelConfiguration type and hence it accepts metav1.Object only. +func getDanglingBootstrapObjectNames(bootstrap sets.String, current []metav1.Object) []string { if len(current) == 0 { return nil } @@ -323,7 +326,9 @@ func getRemoveCandidate(bootstrap sets.String, current []metav1.Object) []string for i := range current { object := current[i] if _, ok := object.GetAnnotations()[flowcontrolv1beta2.AutoUpdateAnnotationKey]; !ok { - // the configuration object does not have the annotation key + // the configuration object does not have the annotation key, + // it's probably a user defined configuration object, + // so we can skip it. continue } diff --git a/pkg/registry/flowcontrol/rest/storage_flowcontrol.go b/pkg/registry/flowcontrol/rest/storage_flowcontrol.go index 12077700f58..55963684265 100644 --- a/pkg/registry/flowcontrol/rest/storage_flowcontrol.go +++ b/pkg/registry/flowcontrol/rest/storage_flowcontrol.go @@ -188,7 +188,7 @@ func ensure(clientset flowcontrolclient.FlowcontrolV1beta2Interface, fsLister fl return fmt.Errorf("failed ensuring mandatory settings - %w", err) } - if err := removeConfiguration(clientset, fsLister, plcLister); err != nil { + if err := removeDanglingBootstrapConfiguration(clientset, fsLister, plcLister); err != nil { return fmt.Errorf("failed to delete removed settings - %w", err) } @@ -215,17 +215,17 @@ func ensureMandatoryConfiguration(clientset flowcontrolclient.FlowcontrolV1beta2 return plEnsurer.Ensure(flowcontrolbootstrap.MandatoryPriorityLevelConfigurations) } -func removeConfiguration(clientset flowcontrolclient.FlowcontrolV1beta2Interface, fsLister flowcontrollisters.FlowSchemaLister, plcLister flowcontrollisters.PriorityLevelConfigurationLister) error { - if err := removeFlowSchema(clientset.FlowSchemas(), fsLister); err != nil { +func removeDanglingBootstrapConfiguration(clientset flowcontrolclient.FlowcontrolV1beta2Interface, fsLister flowcontrollisters.FlowSchemaLister, plcLister flowcontrollisters.PriorityLevelConfigurationLister) error { + if err := removeDanglingBootstrapFlowSchema(clientset.FlowSchemas(), fsLister); err != nil { return err } - return removePriorityLevel(clientset.PriorityLevelConfigurations(), plcLister) + return removeDanglingBootstrapPriorityLevel(clientset.PriorityLevelConfigurations(), plcLister) } -func removeFlowSchema(client flowcontrolclient.FlowSchemaInterface, lister flowcontrollisters.FlowSchemaLister) error { +func removeDanglingBootstrapFlowSchema(client flowcontrolclient.FlowSchemaInterface, lister flowcontrollisters.FlowSchemaLister) error { bootstrap := append(flowcontrolbootstrap.MandatoryFlowSchemas, flowcontrolbootstrap.SuggestedFlowSchemas...) - candidates, err := ensurer.GetFlowSchemaRemoveCandidate(lister, bootstrap) + candidates, err := ensurer.GetFlowSchemaRemoveCandidates(lister, bootstrap) if err != nil { return err } @@ -234,12 +234,12 @@ func removeFlowSchema(client flowcontrolclient.FlowSchemaInterface, lister flowc } fsRemover := ensurer.NewFlowSchemaRemover(client, lister) - return fsRemover.Remove(candidates) + return fsRemover.RemoveAutoUpdateEnabledObjects(candidates) } -func removePriorityLevel(client flowcontrolclient.PriorityLevelConfigurationInterface, lister flowcontrollisters.PriorityLevelConfigurationLister) error { +func removeDanglingBootstrapPriorityLevel(client flowcontrolclient.PriorityLevelConfigurationInterface, lister flowcontrollisters.PriorityLevelConfigurationLister) error { bootstrap := append(flowcontrolbootstrap.MandatoryPriorityLevelConfigurations, flowcontrolbootstrap.SuggestedPriorityLevelConfigurations...) - candidates, err := ensurer.GetPriorityLevelRemoveCandidate(lister, bootstrap) + candidates, err := ensurer.GetPriorityLevelRemoveCandidates(lister, bootstrap) if err != nil { return err } @@ -248,7 +248,7 @@ func removePriorityLevel(client flowcontrolclient.PriorityLevelConfigurationInte } plRemover := ensurer.NewPriorityLevelRemover(client, lister) - return plRemover.Remove(candidates) + return plRemover.RemoveAutoUpdateEnabledObjects(candidates) } // contextFromChannelAndMaxWaitDuration returns a Context that is bound to the