refactor(runner_controller): cleanup code, extract methods

internal/controller/runner_controller.go

@@ -5,6 +5,7 @@ package controller
 import (
 	"context"
 	"fmt"
+	"github.com/mercedes-benz/garm-operator/pkg/config"
 	"strings"
 	"time"
 
@@ -48,13 +49,7 @@ func (r *RunnerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 	log := log.FromContext(ctx)
 	log.Info("Reconciling runners...", "Request", req)
 
-	instanceClient := garmClient.NewInstanceClient()
-	err := instanceClient.Login(garmClient.GarmScopeParams{
-		BaseURL:  r.BaseURL,
-		Username: r.Username,
-		Password: r.Password,
-		// Debug:    true,
-	})
+	instanceClient, err := r.instanceClient()
 	if err != nil {
 		return ctrl.Result{}, err
 	}
@@ -75,7 +70,7 @@ func (r *RunnerReconciler) reconcile(ctx context.Context, req ctrl.Request, inst
 		return r.handleCreateRunnerCR(ctx, req, err, garmRunner)
 	}
 
-	// delete runner on garm side and remove finalizer if deleted
+	// delete runner in garm db
 	if !runner.ObjectMeta.DeletionTimestamp.IsZero() {
 		return r.reconcileDelete(ctx, instanceClient, runner, garmRunner)
 	}
@@ -239,41 +234,55 @@ func (r *RunnerReconciler) PollRunnerInstances(ctx context.Context, eventChan ch
 }
 
 func (r *RunnerReconciler) EnqueueRunnerInstances(ctx context.Context, eventChan chan event.GenericEvent) error {
-	garmRunnerInstances := params.Instances{}
-	pools := &garmoperatorv1alpha1.PoolList{}
-
-	err := r.List(ctx, pools)
+	instanceClient, err := r.instanceClient()
 	if err != nil {
 		return err
 	}
 
-	instanceClient := garmClient.NewInstanceClient()
-	err = instanceClient.Login(garmClient.GarmScopeParams{
-		BaseURL:  r.BaseURL,
-		Username: r.Username,
-		Password: r.Password,
-	})
+	pools, err := r.fetchPools(ctx)
 	if err != nil {
 		return err
 	}
 
 	if len(pools.Items) < 1 {
 		return nil
 	}
-	namespace := pools.Items[0].Namespace
+
 	// fetching runners by pools to ensure only runners belonging to pools in same namespace are being shown
-	for _, p := range pools.Items {
-		if p.Status.ID == "" {
-			continue
+	garmRunnerInstances, err := r.fetchRunnerInstancesByNamespacedPools(instanceClient, pools)
+	if err != nil {
+		return err
+	}
+
+	// compares garm db with RunnerCRs and deletes RunnerCRs not present in garm db
+	err = r.cleanUpNotMatchingRunnerCRs(ctx, err, garmRunnerInstances)
+	if err != nil {
+		return err
+	}
+
+	// triggers controller to reconcile based on instances in garm db
+	enqeueRunnerEvents(garmRunnerInstances, eventChan)
+	return nil
+}
+
+func enqeueRunnerEvents(garmRunnerInstances params.Instances, eventChan chan event.GenericEvent) {
+	for _, runner := range garmRunnerInstances {
+		runnerObj := garmoperatorv1alpha1.Runner{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      strings.ToLower(runner.Name),
+				Namespace: config.Config.Operator.WatchNamespace,
+			},
 		}
-		poolRunners, err := instanceClient.ListPoolInstances(instances.NewListPoolInstancesParams().WithPoolID(p.Status.ID))
-		if err != nil {
-			return err
+
+		e := event.GenericEvent{
+			Object: &runnerObj,
 		}
-		garmRunnerInstances = append(garmRunnerInstances, poolRunners.Payload...)
+
+		eventChan <- e
 	}
+}
 
-	// compare garmdb with runner cr => delete runnerCRs not present in garmdb
+func (r *RunnerReconciler) cleanUpNotMatchingRunnerCRs(ctx context.Context, err error, garmRunnerInstances params.Instances) error {
 	runnerCRList := &garmoperatorv1alpha1.RunnerList{}
 	err = r.List(ctx, runnerCRList)
 	if err != nil {
@@ -293,7 +302,7 @@ func (r *RunnerReconciler) EnqueueRunnerInstances(ctx context.Context, eventChan
 
 	for _, runnerName := range runnersToDelete {
 		runner := &garmoperatorv1alpha1.Runner{}
-		err := r.Get(ctx, types.NamespacedName{Namespace: namespace, Name: runnerName}, runner)
+		err := r.Get(ctx, types.NamespacedName{Namespace: config.Config.Operator.WatchNamespace, Name: runnerName}, runner)
 		if err != nil {
 			return err
 		}
@@ -312,22 +321,41 @@ func (r *RunnerReconciler) EnqueueRunnerInstances(ctx context.Context, eventChan
 			}
 		}
 	}
+	return nil
+}
 
-	for _, runner := range garmRunnerInstances {
-		runnerObj := garmoperatorv1alpha1.Runner{
-			ObjectMeta: metav1.ObjectMeta{
-				Name:      strings.ToLower(runner.Name),
-				Namespace: namespace,
-			},
-		}
+func (r *RunnerReconciler) fetchPools(ctx context.Context) (*garmoperatorv1alpha1.PoolList, error) {
+	pools := &garmoperatorv1alpha1.PoolList{}
+	err := r.List(ctx, pools)
+	if err != nil {
+		return nil, err
+	}
+	return pools, nil
+}
 
-		e := event.GenericEvent{
-			Object: &runnerObj,
+func (r *RunnerReconciler) fetchRunnerInstancesByNamespacedPools(instanceClient garmClient.InstanceClient, pools *garmoperatorv1alpha1.PoolList) (params.Instances, error) {
+	garmRunnerInstances := params.Instances{}
+	for _, p := range pools.Items {
+		if p.Status.ID == "" {
+			continue
 		}
-
-		eventChan <- e
+		poolRunners, err := instanceClient.ListPoolInstances(instances.NewListPoolInstancesParams().WithPoolID(p.Status.ID))
+		if err != nil {
+			return nil, err
+		}
+		garmRunnerInstances = append(garmRunnerInstances, poolRunners.Payload...)
 	}
-	return nil
+	return garmRunnerInstances, nil
+}
+
+func (r *RunnerReconciler) instanceClient() (garmClient.InstanceClient, error) {
+	instanceClient := garmClient.NewInstanceClient()
+	err := instanceClient.Login(garmClient.GarmScopeParams{
+		BaseURL:  r.BaseURL,
+		Username: r.Username,
+		Password: r.Password,
+	})
+	return instanceClient, err
 }
 
 func getRunnerDiff(runnerCRs, garmRunners []string) []string {

pkg/flags/flags.go

@@ -25,7 +25,6 @@ func InitiateFlags() *pflag.FlagSet {
 	f.Bool("operator-leader-election", defaults.DefaultLeaderElection, "Enable leader election for controller manager. "+"Enabling this will ensure there is only one active controller manager.")
 	f.Duration("operator-sync-period", defaults.DefaultSyncPeriod, "The minimum interval at which watched resources are reconciled (e.g. 5m)")
 	f.String("operator-watch-namespace", defaults.DefaultWatchNamespace, "Namespace that the controller watches to reconcile garm objects. "+"If unspecified, the controller watches for garm objects across all namespaces.")
-	f.Bool("operator-disable-webhooks", defaults.DisableWebhooks, "Disables webhooks for local debugging purposes.")
 
 	f.String("garm-server", "", "The address of the GARM server")
 	f.String("garm-username", "", "The username for the GARM server")