chore: refactor

Author: tchinmai7

internal/controller/linodemachine_controller.go

@@ -717,7 +717,6 @@ func (r *LinodeMachineReconciler) reconcilePreflightReady(ctx context.Context, i
 	return ctrl.Result{}, nil
 }
 
-//nolint:cyclop // making it simpler complicates things more
 func (r *LinodeMachineReconciler) reconcileUpdate(ctx context.Context, logger logr.Logger, machineScope *scope.MachineScope) (ctrl.Result, error) {
 	logger.Info("updating machine")
 	instanceID, err := util.GetInstanceID(machineScope.LinodeMachine.Spec.ProviderID)

internal/controller/linodemachinetemplate_controller.go

@@ -126,36 +126,23 @@ func (lmtr *LinodeMachineTemplateReconciler) reconcile(ctx context.Context, lmtS
 		}
 
 		machinesFoundForTemplate = true
-
-		// Define reconciliation tasks
-		reconciliationTasks := []struct {
-			shouldReconcile bool
-			fieldType       string
-			failureReason   string
-			logMessage      string
-		}{
-			{
-				shouldReconcile: !slices.Equal(lmtScope.LinodeMachineTemplate.Spec.Template.Spec.Tags, lmtScope.LinodeMachineTemplate.Status.Tags),
-				fieldType:       "tags",
-				failureReason:   "FailedToPatchLinodeMachineWithTags",
-				logMessage:      "Failed to update tags on LinodeMachine",
-			},
-			{
-				shouldReconcile: lmtScope.LinodeMachineTemplate.Spec.Template.Spec.FirewallID != lmtScope.LinodeMachineTemplate.Status.FirewallID,
-				fieldType:       "firewallID",
-				failureReason:   "FailedToPatchLinodeMachineWithFirewallID",
-				logMessage:      "Failed to update firewall ID on LinodeMachine",
-			},
+		if !slices.Equal(lmtScope.LinodeMachineTemplate.Spec.Template.Spec.Tags, lmtScope.LinodeMachineTemplate.Status.Tags) {
+			err := lmtr.reconcileTags(ctx, lmtScope.LinodeMachineTemplate, &machine)
+			if err != nil {
+				lmtr.Logger.Error(err, "Failed to update tags on LinodeMachine", "template", lmtScope.LinodeMachineTemplate.Name, "machine", machine.Name)
+				outErr = errors.Join(outErr, err)
+				failureReason = "FailedToPatchLinodeMachine"
+				return ctrl.Result{}, outErr
+			}
 		}
 
-		// Execute reconciliation tasks
-		for _, task := range reconciliationTasks {
-			if task.shouldReconcile {
-				if err := lmtr.reconcileField(ctx, lmtScope.LinodeMachineTemplate, &machine, task.fieldType); err != nil {
-					lmtr.Logger.Error(err, task.logMessage, "template", lmtScope.LinodeMachineTemplate.Name, "machine", machine.Name)
-					outErr = errors.Join(outErr, err)
-					failureReason = task.failureReason
-				}
+		if lmtScope.LinodeMachineTemplate.Spec.Template.Spec.FirewallID != lmtScope.LinodeMachineTemplate.Status.FirewallID {
+			err := lmtr.reconcileFirewallID(ctx, lmtScope.LinodeMachineTemplate, &machine)
+			if err != nil {
+				lmtr.Logger.Error(err, "Failed to update FirewallID on LinodeMachine", "template", lmtScope.LinodeMachineTemplate.Name, "machine", machine.Name)
+				outErr = errors.Join(outErr, err)
+				failureReason = "FailedToPatchLinodeMachine"
+				return ctrl.Result{}, outErr
 			}
 		}
 	}
@@ -176,30 +163,30 @@ func (lmtr *LinodeMachineTemplateReconciler) reconcile(ctx context.Context, lmtS
 	return ctrl.Result{}, outErr
 }
 
-// reconcileField updates a specific field on a LinodeMachine based on the field type
-func (lmtr *LinodeMachineTemplateReconciler) reconcileField(ctx context.Context, lmt *infrav1alpha2.LinodeMachineTemplate, machine *infrav1alpha2.LinodeMachine, fieldType string) error {
+func (lmtr *LinodeMachineTemplateReconciler) reconcileTags(ctx context.Context, lmt *infrav1alpha2.LinodeMachineTemplate, machine *infrav1alpha2.LinodeMachine) error {
 	helper, err := patch.NewHelper(machine, lmtr.Client)
 	if err != nil {
 		return fmt.Errorf("failed to init patch helper: %w", err)
 	}
 
-	switch fieldType {
-	case "tags":
-		machine.Spec.Tags = lmt.Spec.Template.Spec.Tags
-		if err := helper.Patch(ctx, machine); err != nil {
-			return fmt.Errorf("failed to patch LinodeMachine %s with new %s: %w", machine.Name, fieldType, err)
-		}
-		lmtr.Logger.Info("Patched LinodeMachine with new tags", "machine", machine.Name, "tags", lmt.Spec.Template.Spec.Tags)
-	case "firewallID":
-		machine.Spec.FirewallID = lmt.Spec.Template.Spec.FirewallID
-		if err := helper.Patch(ctx, machine); err != nil {
-			return fmt.Errorf("failed to patch LinodeMachine %s with new %s: %w", machine.Name, fieldType, err)
-		}
-		lmtr.Logger.Info("Patched LinodeMachine with new firewall ID", "machine", machine.Name, "firewallID", lmt.Spec.Template.Spec.FirewallID)
-	default:
-		return fmt.Errorf("unsupported field type: %s", fieldType)
+	machine.Spec.Tags = lmt.Spec.Template.Spec.Tags
+	if err := helper.Patch(ctx, machine); err != nil {
+		return fmt.Errorf("failed to patch LinodeMachine %s with new tags: %w", machine.Name, err)
 	}
+	lmtr.Logger.Info("Patched LinodeMachine with new tags", "machine", machine.Name, "tags", lmt.Spec.Template.Spec.Tags)
+
+	return nil
+}
 
+func (lmtr *LinodeMachineTemplateReconciler) reconcileFirewallID(ctx context.Context, lmt *infrav1alpha2.LinodeMachineTemplate, machine *infrav1alpha2.LinodeMachine) error {
+	helper, err := patch.NewHelper(machine, lmtr.Client)
+	if err != nil {
+		return fmt.Errorf("failed to init patch helper: %w", err)
+	}
+	machine.Spec.FirewallID = lmt.Spec.Template.Spec.FirewallID
+	if err := helper.Patch(ctx, machine); err != nil {
+		return fmt.Errorf("failed to patch LinodeMachine %s with new firewallID: %w", machine.Name, err)
+	}
 	return nil
 }