Merge pull request #322 from chanwit/refactor_indexers_to_use_HOLs

Refactor indexers to use high-order functions
fluxcd · Apr 21, 2021 · 39f86cc · 39f86cc
2 parents 273bf03 + 147df26
commit 39f86cc
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 102 deletions.
diff --git a/controllers/kustomization_controller.go b/controllers/kustomization_controller.go
@@ -88,13 +88,13 @@ type KustomizationReconcilerOptions struct {
 func (r *KustomizationReconciler) SetupWithManager(mgr ctrl.Manager, opts KustomizationReconcilerOptions) error {
 	// Index the Kustomizations by the GitRepository references they (may) point at.
 	if err := mgr.GetCache().IndexField(context.TODO(), &kustomizev1.Kustomization{}, kustomizev1.GitRepositoryIndexKey,
-		r.indexByGitRepository); err != nil {
+		r.indexBy(sourcev1.GitRepositoryKind)); err != nil {
 		return fmt.Errorf("failed setting index fields: %w", err)
 	}
 
 	// Index the Kustomizations by the Bucket references they (may) point at.
 	if err := mgr.GetCache().IndexField(context.TODO(), &kustomizev1.Kustomization{}, kustomizev1.BucketIndexKey,
-		r.indexByBucket); err != nil {
+		r.indexBy(sourcev1.BucketKind)); err != nil {
 		return fmt.Errorf("failed setting index fields: %w", err)
 	}
 
@@ -115,12 +115,12 @@ func (r *KustomizationReconciler) SetupWithManager(mgr ctrl.Manager, opts Kustom
 		)).
 		Watches(
 			&source.Kind{Type: &sourcev1.GitRepository{}},
-			handler.EnqueueRequestsFromMapFunc(r.requestsForGitRepositoryRevisionChange),
+			handler.EnqueueRequestsFromMapFunc(r.requestsForRevisionChangeOf(kustomizev1.GitRepositoryIndexKey)),
 			builder.WithPredicates(SourceRevisionChangePredicate{}),
 		).
 		Watches(
 			&source.Kind{Type: &sourcev1.Bucket{}},
-			handler.EnqueueRequestsFromMapFunc(r.requestsForBucketRevisionChange),
+			handler.EnqueueRequestsFromMapFunc(r.requestsForRevisionChangeOf(kustomizev1.BucketIndexKey)),
 			builder.WithPredicates(SourceRevisionChangePredicate{}),
 		).
 		WithOptions(controller.Options{MaxConcurrentReconciles: opts.MaxConcurrentReconciles}).

diff --git a/controllers/kustomization_indexers.go b/controllers/kustomization_indexers.go
@@ -28,112 +28,63 @@ import (
 	sourcev1 "github.com/fluxcd/source-controller/api/v1beta1"
 )
 
-func (r *KustomizationReconciler) requestsForGitRepositoryRevisionChange(obj client.Object) []reconcile.Request {
-	repo, ok := obj.(*sourcev1.GitRepository)
-	if !ok {
-		panic(fmt.Sprintf("Expected a GitRepository but got a %T", obj))
-	}
-	// If we do not have an artifact, we have no requests to make
-	if repo.GetArtifact() == nil {
-		return nil
-	}
-
-	ctx := context.Background()
-	var list kustomizev1.KustomizationList
-	if err := r.List(ctx, &list, client.MatchingFields{
-		kustomizev1.GitRepositoryIndexKey: ObjectKey(obj).String(),
-	}); err != nil {
-		return nil
-	}
-	var dd []dependency.Dependent
-	for _, d := range list.Items {
-		// If the revision of the artifact equals to the last attempted revision,
-		// we should not make a request for this Kustomization
-		if repo.GetArtifact().Revision == d.Status.LastAttemptedRevision {
-			continue
+func (r *KustomizationReconciler) requestsForRevisionChangeOf(indexKey string) func(obj client.Object) []reconcile.Request {
+	return func(obj client.Object) []reconcile.Request {
+		repo, ok := obj.(interface {
+			GetArtifact() *sourcev1.Artifact
+		})
+		if !ok {
+			panic(fmt.Sprintf("Expected an object conformed with GetArtifact() method, but got a %T", obj))
 		}
-		dd = append(dd, d)
-	}
-	sorted, err := dependency.Sort(dd)
-	if err != nil {
-		return nil
-	}
-	reqs := make([]reconcile.Request, len(sorted), len(sorted))
-	for i := range sorted {
-		reqs[i].NamespacedName.Name = sorted[i].Name
-		reqs[i].NamespacedName.Namespace = sorted[i].Namespace
-	}
-	return reqs
-}
-
-func (r *KustomizationReconciler) indexByGitRepository(o client.Object) []string {
-	k, ok := o.(*kustomizev1.Kustomization)
-	if !ok {
-		panic(fmt.Sprintf("Expected a Kustomization, got %T", o))
-	}
-
-	if k.Spec.SourceRef.Kind == sourcev1.GitRepositoryKind {
-		namespace := k.GetNamespace()
-		if k.Spec.SourceRef.Namespace != "" {
-			namespace = k.Spec.SourceRef.Namespace
+		// If we do not have an artifact, we have no requests to make
+		if repo.GetArtifact() == nil {
+			return nil
 		}
-		return []string{fmt.Sprintf("%s/%s", namespace, k.Spec.SourceRef.Name)}
-	}
-
-	return nil
-}
 
-func (r *KustomizationReconciler) requestsForBucketRevisionChange(obj client.Object) []reconcile.Request {
-	bucket, ok := obj.(*sourcev1.Bucket)
-	if !ok {
-		panic(fmt.Sprintf("Expected a Bucket but got a %T", obj))
-	}
-	// If we do not have an artifact, we have no requests to make
-	if bucket.GetArtifact() == nil {
-		return nil
-	}
-
-	ctx := context.Background()
-	var list kustomizev1.KustomizationList
-	if err := r.List(ctx, &list, client.MatchingFields{
-		kustomizev1.BucketIndexKey: ObjectKey(obj).String(),
-	}); err != nil {
-		return nil
-	}
-	var dd []dependency.Dependent
-	for _, d := range list.Items {
-		// If the revision of the artifact equals to the last attempted revision,
-		// we should not make a request for this Kustomization
-		if bucket.GetArtifact().Revision == d.Status.LastAttemptedRevision {
-			continue
+		ctx := context.Background()
+		var list kustomizev1.KustomizationList
+		if err := r.List(ctx, &list, client.MatchingFields{
+			indexKey: ObjectKey(obj).String(),
+		}); err != nil {
+			return nil
 		}
-		dd = append(dd, d)
-	}
-	sorted, err := dependency.Sort(dd)
-	if err != nil {
-		return nil
-	}
-	reqs := make([]reconcile.Request, len(sorted), len(sorted))
-	for i := range sorted {
-		reqs[i].NamespacedName.Name = sorted[i].Name
-		reqs[i].NamespacedName.Namespace = sorted[i].Namespace
+		var dd []dependency.Dependent
+		for _, d := range list.Items {
+			// If the revision of the artifact equals to the last attempted revision,
+			// we should not make a request for this Kustomization
+			if repo.GetArtifact().Revision == d.Status.LastAttemptedRevision {
+				continue
+			}
+			dd = append(dd, d)
+		}
+		sorted, err := dependency.Sort(dd)
+		if err != nil {
+			return nil
+		}
+		reqs := make([]reconcile.Request, len(sorted), len(sorted))
+		for i := range sorted {
+			reqs[i].NamespacedName.Name = sorted[i].Name
+			reqs[i].NamespacedName.Namespace = sorted[i].Namespace
+		}
+		return reqs
 	}
-	return reqs
 }
 
-func (r *KustomizationReconciler) indexByBucket(o client.Object) []string {
-	k, ok := o.(*kustomizev1.Kustomization)
-	if !ok {
-		panic(fmt.Sprintf("Expected a Kustomization, got %T", o))
-	}
+func (r *KustomizationReconciler) indexBy(kind string) func(o client.Object) []string {
+	return func(o client.Object) []string {
+		k, ok := o.(*kustomizev1.Kustomization)
+		if !ok {
+			panic(fmt.Sprintf("Expected a Kustomization, got %T", o))
+		}
 
-	if k.Spec.SourceRef.Kind == sourcev1.BucketKind {
-		namespace := k.GetNamespace()
-		if k.Spec.SourceRef.Namespace != "" {
-			namespace = k.Spec.SourceRef.Namespace
+		if k.Spec.SourceRef.Kind == kind {
+			namespace := k.GetNamespace()
+			if k.Spec.SourceRef.Namespace != "" {
+				namespace = k.Spec.SourceRef.Namespace
+			}
+			return []string{fmt.Sprintf("%s/%s", namespace, k.Spec.SourceRef.Name)}
 		}
-		return []string{fmt.Sprintf("%s/%s", namespace, k.Spec.SourceRef.Name)}
-	}
 
-	return nil
+		return nil
+	}
 }