Merge pull request #107106 from tkashem/apf-comment

apf: clarify with comment
This commit is contained in:
Kubernetes Prow Robot 2022-01-20 01:13:51 -08:00 committed by GitHub
commit 46c5edbc58
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 54 additions and 38 deletions

View File

@ -41,9 +41,14 @@ type FlowSchemaEnsurer interface {
Ensure([]*flowcontrolv1beta2.FlowSchema) error 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 { type FlowSchemaRemover interface {
Remove([]string) error RemoveAutoUpdateEnabledObjects([]string) error
} }
// NewSuggestedFlowSchemaEnsurer returns a FlowSchemaEnsurer instance that // 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. // names that are candidates for deletion from the cluster.
// bootstrap: a set of hard coded FlowSchema configuration objects // bootstrap: a set of hard coded FlowSchema configuration objects
// kube-apiserver maintains in-memory. // 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()) fsList, err := lister.List(labels.Everything())
if err != nil { if err != nil {
return nil, fmt.Errorf("failed to list FlowSchema - %w", err) return nil, fmt.Errorf("failed to list FlowSchema - %w", err)
@ -103,7 +108,7 @@ func GetFlowSchemaRemoveCandidate(lister flowcontrollisters.FlowSchemaLister, bo
currentObjects[i] = fsList[i] currentObjects[i] = fsList[i]
} }
return getRemoveCandidate(bootstrapNames, currentObjects), nil return getDanglingBootstrapObjectNames(bootstrapNames, currentObjects), nil
} }
type fsEnsurer struct { type fsEnsurer struct {
@ -121,9 +126,9 @@ func (e *fsEnsurer) Ensure(flowSchemas []*flowcontrolv1beta2.FlowSchema) error {
return nil return nil
} }
func (e *fsEnsurer) Remove(flowSchemas []string) error { func (e *fsEnsurer) RemoveAutoUpdateEnabledObjects(flowSchemas []string) error {
for _, flowSchema := range flowSchemas { for _, flowSchema := range flowSchemas {
if err := removeConfiguration(e.wrapper, flowSchema); err != nil { if err := removeAutoUpdateEnabledConfiguration(e.wrapper, flowSchema); err != nil {
return err return err
} }
} }

View File

@ -322,7 +322,7 @@ func TestRemoveFlowSchema(t *testing.T) {
} }
remover := NewFlowSchemaRemover(client, flowcontrollisters.NewFlowSchemaLister(indexer)) remover := NewFlowSchemaRemover(client, flowcontrollisters.NewFlowSchemaLister(indexer))
err := remover.Remove([]string{test.bootstrapName}) err := remover.RemoveAutoUpdateEnabledObjects([]string{test.bootstrapName})
if err != nil { if err != nil {
t.Fatalf("Expected no error, but got: %v", err) t.Fatalf("Expected no error, but got: %v", err)
} }
@ -410,7 +410,7 @@ func TestGetFlowSchemaRemoveCandidate(t *testing.T) {
} }
lister := flowcontrollisters.NewFlowSchemaLister(indexer) lister := flowcontrollisters.NewFlowSchemaLister(indexer)
removeListGot, err := GetFlowSchemaRemoveCandidate(lister, test.bootstrap) removeListGot, err := GetFlowSchemaRemoveCandidates(lister, test.bootstrap)
if err != nil { if err != nil {
t.Fatalf("Expected no error, but got: %v", err) t.Fatalf("Expected no error, but got: %v", err)
} }

View File

@ -41,9 +41,15 @@ type PriorityLevelEnsurer interface {
Ensure([]*flowcontrolv1beta2.PriorityLevelConfiguration) error 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 { type PriorityLevelRemover interface {
Remove([]string) error RemoveAutoUpdateEnabledObjects([]string) error
} }
// NewSuggestedPriorityLevelEnsurerEnsurer returns a PriorityLevelEnsurer instance that // 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. // names that are candidates for removal from the cluster.
// bootstrap: a set of hard coded PriorityLevelConfiguration configuration // bootstrap: a set of hard coded PriorityLevelConfiguration configuration
// objects kube-apiserver maintains in-memory. // 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()) plList, err := lister.List(labels.Everything())
if err != nil { if err != nil {
return nil, fmt.Errorf("failed to list PriorityLevelConfiguration - %w", err) return nil, fmt.Errorf("failed to list PriorityLevelConfiguration - %w", err)
@ -103,7 +109,7 @@ func GetPriorityLevelRemoveCandidate(lister flowcontrollisters.PriorityLevelConf
currentObjects[i] = plList[i] currentObjects[i] = plList[i]
} }
return getRemoveCandidate(bootstrapNames, currentObjects), nil return getDanglingBootstrapObjectNames(bootstrapNames, currentObjects), nil
} }
type plEnsurer struct { type plEnsurer struct {
@ -121,9 +127,9 @@ func (e *plEnsurer) Ensure(priorityLevels []*flowcontrolv1beta2.PriorityLevelCon
return nil return nil
} }
func (e *plEnsurer) Remove(priorityLevels []string) error { func (e *plEnsurer) RemoveAutoUpdateEnabledObjects(priorityLevels []string) error {
for _, priorityLevel := range priorityLevels { for _, priorityLevel := range priorityLevels {
if err := removeConfiguration(e.wrapper, priorityLevel); err != nil { if err := removeAutoUpdateEnabledConfiguration(e.wrapper, priorityLevel); err != nil {
return err return err
} }
} }

View File

@ -338,7 +338,7 @@ func TestRemovePriorityLevelConfiguration(t *testing.T) {
} }
remover := NewPriorityLevelRemover(client, flowcontrollisters.NewPriorityLevelConfigurationLister(indexer)) remover := NewPriorityLevelRemover(client, flowcontrollisters.NewPriorityLevelConfigurationLister(indexer))
err := remover.Remove([]string{test.bootstrapName}) err := remover.RemoveAutoUpdateEnabledObjects([]string{test.bootstrapName})
if err != nil { if err != nil {
t.Fatalf("Expected no error, but got: %v", err) t.Fatalf("Expected no error, but got: %v", err)
} }
@ -426,7 +426,7 @@ func TestGetPriorityLevelRemoveCandidate(t *testing.T) {
} }
lister := flowcontrollisters.NewPriorityLevelConfigurationLister(indexer) lister := flowcontrollisters.NewPriorityLevelConfigurationLister(indexer)
removeListGot, err := GetPriorityLevelRemoveCandidate(lister, test.bootstrap) removeListGot, err := GetPriorityLevelRemoveCandidates(lister, test.bootstrap)
if err != nil { if err != nil {
t.Fatalf("Expected no error, but got: %v", err) t.Fatalf("Expected no error, but got: %v", err)
} }

View File

@ -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) 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) current, err := wrapper.Get(name)
if err != nil { if err != nil {
if apierrors.IsNotFound(err) { if apierrors.IsNotFound(err) {
@ -305,16 +307,17 @@ func removeConfiguration(wrapper configurationWrapper, name string) error {
return nil return nil
} }
// getRemoveCandidate returns a list of configuration objects we should delete // getDanglingBootstrapObjectNames returns a list of names of bootstrap
// from the cluster given a set of bootstrap and current configuration. // configuration objects that are potentially candidates for deletion from
// bootstrap: a set of hard coded configuration kube-apiserver maintains in-memory. // the cluster, given a set of bootstrap and current configuration.
// current: a set of configuration objects that exist on the cluster // - bootstrap: a set of hard coded configuration kube-apiserver maintains in-memory.
// Any object present in current is a candidate for removal if both a and b are true: // - 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 // a. the object in current is missing from the bootstrap configuration
// b. the object has the designated auto-update annotation key // b. the object has the designated auto-update annotation key
// This function shares the common logic for both FlowSchema and PriorityLevelConfiguration // This function shares the common logic for both FlowSchema and
// type and hence it accepts metav1.Object only. // PriorityLevelConfiguration type and hence it accepts metav1.Object only.
func getRemoveCandidate(bootstrap sets.String, current []metav1.Object) []string { func getDanglingBootstrapObjectNames(bootstrap sets.String, current []metav1.Object) []string {
if len(current) == 0 { if len(current) == 0 {
return nil return nil
} }
@ -323,7 +326,9 @@ func getRemoveCandidate(bootstrap sets.String, current []metav1.Object) []string
for i := range current { for i := range current {
object := current[i] object := current[i]
if _, ok := object.GetAnnotations()[flowcontrolv1beta2.AutoUpdateAnnotationKey]; !ok { 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 continue
} }

View File

@ -188,7 +188,7 @@ func ensure(clientset flowcontrolclient.FlowcontrolV1beta2Interface, fsLister fl
return fmt.Errorf("failed ensuring mandatory settings - %w", err) 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) return fmt.Errorf("failed to delete removed settings - %w", err)
} }
@ -215,17 +215,17 @@ func ensureMandatoryConfiguration(clientset flowcontrolclient.FlowcontrolV1beta2
return plEnsurer.Ensure(flowcontrolbootstrap.MandatoryPriorityLevelConfigurations) return plEnsurer.Ensure(flowcontrolbootstrap.MandatoryPriorityLevelConfigurations)
} }
func removeConfiguration(clientset flowcontrolclient.FlowcontrolV1beta2Interface, fsLister flowcontrollisters.FlowSchemaLister, plcLister flowcontrollisters.PriorityLevelConfigurationLister) error { func removeDanglingBootstrapConfiguration(clientset flowcontrolclient.FlowcontrolV1beta2Interface, fsLister flowcontrollisters.FlowSchemaLister, plcLister flowcontrollisters.PriorityLevelConfigurationLister) error {
if err := removeFlowSchema(clientset.FlowSchemas(), fsLister); err != nil { if err := removeDanglingBootstrapFlowSchema(clientset.FlowSchemas(), fsLister); err != nil {
return err 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...) bootstrap := append(flowcontrolbootstrap.MandatoryFlowSchemas, flowcontrolbootstrap.SuggestedFlowSchemas...)
candidates, err := ensurer.GetFlowSchemaRemoveCandidate(lister, bootstrap) candidates, err := ensurer.GetFlowSchemaRemoveCandidates(lister, bootstrap)
if err != nil { if err != nil {
return err return err
} }
@ -234,12 +234,12 @@ func removeFlowSchema(client flowcontrolclient.FlowSchemaInterface, lister flowc
} }
fsRemover := ensurer.NewFlowSchemaRemover(client, lister) 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...) bootstrap := append(flowcontrolbootstrap.MandatoryPriorityLevelConfigurations, flowcontrolbootstrap.SuggestedPriorityLevelConfigurations...)
candidates, err := ensurer.GetPriorityLevelRemoveCandidate(lister, bootstrap) candidates, err := ensurer.GetPriorityLevelRemoveCandidates(lister, bootstrap)
if err != nil { if err != nil {
return err return err
} }
@ -248,7 +248,7 @@ func removePriorityLevel(client flowcontrolclient.PriorityLevelConfigurationInte
} }
plRemover := ensurer.NewPriorityLevelRemover(client, lister) plRemover := ensurer.NewPriorityLevelRemover(client, lister)
return plRemover.Remove(candidates) return plRemover.RemoveAutoUpdateEnabledObjects(candidates)
} }
// contextFromChannelAndMaxWaitDuration returns a Context that is bound to the // contextFromChannelAndMaxWaitDuration returns a Context that is bound to the