Merge pull request kubernetes#107106 from tkashem/apf-comment

apf: clarify with comment

Author: k8s-ci-robot

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
 		}
 	}

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)
 			}

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
 		}
 	}

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)
 			}

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
 		}
 

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