ctrl: nrop: pack the common return args in a struct

the reconciliation steps are returning a common
(and growing) set of values, let's pack them
in a struct, since we always want to return
the same tuple anyway for consistency.

Signed-off-by: Francesco Romani <fromani@redhat.com>

controllers/controllers.go

@@ -18,10 +18,45 @@ package controllers
 
 import (
 	"errors"
+	"time"
+
+	ctrl "sigs.k8s.io/controller-runtime"
 
 	"github.com/openshift-kni/numaresources-operator/pkg/status"
 )
 
+type reconcileStep struct {
+	Done          bool // is reconciliation process done after this step?
+	Result        ctrl.Result
+	ConditionInfo conditionInfo
+	Error         error
+}
+
+func reconcileStepSuccess() reconcileStep {
+	return reconcileStep{
+		Done:          false,
+		Result:        ctrl.Result{},
+		ConditionInfo: availableConditionInfo(),
+	}
+}
+
+func reconcileStepOngoing(after time.Duration, reason, message string) reconcileStep {
+	return reconcileStep{
+		Done:          true,
+		Result:        ctrl.Result{RequeueAfter: after},
+		ConditionInfo: progressingConditionInfo(reason, message),
+	}
+}
+
+func reconcileStepFailed(err error) reconcileStep {
+	return reconcileStep{
+		Done:          true,
+		Result:        ctrl.Result{},
+		ConditionInfo: degradedConditionInfoFromError(err),
+		Error:         err,
+	}
+}
+
 type conditionInfo struct {
 	Type    string // like metav1.Condition.Type
 	Reason  string

controllers/numaresourcesoperator_controller.go

@@ -208,27 +208,27 @@ func (r *NUMAResourcesOperatorReconciler) degradeStatus(ctx context.Context, ins
 	return ctrl.Result{}, nil
 }
 
-func (r *NUMAResourcesOperatorReconciler) reconcileResourceAPI(ctx context.Context, instance *nropv1.NUMAResourcesOperator, trees []nodegroupv1.Tree) (bool, ctrl.Result, conditionInfo, error) {
+func (r *NUMAResourcesOperatorReconciler) reconcileResourceAPI(ctx context.Context, instance *nropv1.NUMAResourcesOperator, trees []nodegroupv1.Tree) reconcileStep {
 	applied, err := r.syncNodeResourceTopologyAPI(ctx)
 	if err != nil {
 		r.Recorder.Eventf(instance, corev1.EventTypeWarning, "FailedCRDInstall", "Failed to install Node Resource Topology CRD: %v", err)
 		err = fmt.Errorf("FailedAPISync: %w", err)
-		return true, ctrl.Result{}, degradedConditionInfoFromError(err), err
+		return reconcileStepFailed(err)
 	}
 	if applied {
 		r.Recorder.Eventf(instance, corev1.EventTypeNormal, "SuccessfulCRDInstall", "Node Resource Topology CRD installed")
 	}
-	return false, ctrl.Result{}, availableConditionInfo(), nil
+	return reconcileStepSuccess()
 }
 
-func (r *NUMAResourcesOperatorReconciler) reconcileResourceMachineConfig(ctx context.Context, instance *nropv1.NUMAResourcesOperator, trees []nodegroupv1.Tree) (bool, ctrl.Result, conditionInfo, error) {
+func (r *NUMAResourcesOperatorReconciler) reconcileResourceMachineConfig(ctx context.Context, instance *nropv1.NUMAResourcesOperator, trees []nodegroupv1.Tree) reconcileStep {
 	// we need to sync machine configs first and wait for the MachineConfigPool updates
 	// before checking additional components for updates
 	mcpUpdatedFunc, err := r.syncMachineConfigs(ctx, instance, trees)
 	if err != nil {
 		r.Recorder.Eventf(instance, corev1.EventTypeWarning, "FailedMCSync", "Failed to set up machine configuration for worker nodes: %v", err)
 		err = fmt.Errorf("failed to sync machine configs: %w", err)
-		return true, ctrl.Result{}, degradedConditionInfoFromError(err), err
+		return reconcileStepFailed(err)
 	}
 	r.Recorder.Eventf(instance, corev1.EventTypeNormal, "SuccessfulMCSync", "Enabled machine configuration for worker nodes")
 
@@ -239,22 +239,22 @@ func (r *NUMAResourcesOperatorReconciler) reconcileResourceMachineConfig(ctx con
 
 	if !allMCPsUpdated {
 		// the Machine Config Pool still did not apply the machine config, wait for one minute
-		return true, ctrl.Result{RequeueAfter: numaResourcesRetryPeriod}, progressingConditionInfo("", ""), nil
+		return reconcileStepOngoing(numaResourcesRetryPeriod, "", "")
 	}
 	instance.Status.MachineConfigPools = syncMachineConfigPoolNodeGroupConfigStatuses(instance.Status.MachineConfigPools, trees)
 
-	return false, ctrl.Result{}, availableConditionInfo(), nil
+	return reconcileStepSuccess()
 }
 
-func (r *NUMAResourcesOperatorReconciler) reconcileResourceDaemonSet(ctx context.Context, instance *nropv1.NUMAResourcesOperator, trees []nodegroupv1.Tree) ([]poolDaemonSet, bool, ctrl.Result, conditionInfo, error) {
+func (r *NUMAResourcesOperatorReconciler) reconcileResourceDaemonSet(ctx context.Context, instance *nropv1.NUMAResourcesOperator, trees []nodegroupv1.Tree) ([]poolDaemonSet, reconcileStep) {
 	daemonSetsInfoPerMCP, err := r.syncNUMAResourcesOperatorResources(ctx, instance, trees)
 	if err != nil {
 		r.Recorder.Eventf(instance, corev1.EventTypeWarning, "FailedRTECreate", "Failed to create Resource-Topology-Exporter DaemonSets: %v", err)
 		err = fmt.Errorf("FailedRTESync: %w", err)
-		return nil, true, ctrl.Result{}, degradedConditionInfoFromError(err), err
+		return nil, reconcileStepFailed(err)
 	}
 	if len(daemonSetsInfoPerMCP) == 0 {
-		return nil, false, ctrl.Result{}, availableConditionInfo(), nil
+		return nil, reconcileStepSuccess()
 	}
 
 	r.Recorder.Eventf(instance, corev1.EventTypeNormal, "SuccessfulRTECreate", "Created Resource-Topology-Exporter DaemonSets")
@@ -263,32 +263,32 @@ func (r *NUMAResourcesOperatorReconciler) reconcileResourceDaemonSet(ctx context
 	instance.Status.DaemonSets = dssWithReadyStatus
 	instance.Status.RelatedObjects = relatedobjects.ResourceTopologyExporter(r.Namespace, dssWithReadyStatus)
 	if err != nil {
-		return nil, true, ctrl.Result{}, degradedConditionInfoFromError(err), err
+		return nil, reconcileStepFailed(err)
 	}
 	if !allDSsUpdated {
-		return nil, true, ctrl.Result{RequeueAfter: 5 * time.Second}, progressingConditionInfo("", ""), nil
+		return nil, reconcileStepOngoing(5*time.Second, "", "")
 	}
 
-	return daemonSetsInfoPerMCP, false, ctrl.Result{}, availableConditionInfo(), nil
+	return daemonSetsInfoPerMCP, reconcileStepSuccess()
 }
 
 func (r *NUMAResourcesOperatorReconciler) reconcileResource(ctx context.Context, instance *nropv1.NUMAResourcesOperator, trees []nodegroupv1.Tree) (ctrl.Result, error) {
-	if done, res, cond, err := r.reconcileResourceAPI(ctx, instance, trees); done {
-		updateStatusConditionsIfNeeded(instance, fillConditionInfoFromError(cond, err))
-		return res, err
+	if step := r.reconcileResourceAPI(ctx, instance, trees); step.Done {
+		updateStatusConditionsIfNeeded(instance, fillConditionInfoFromError(step.ConditionInfo, step.Error))
+		return step.Result, step.Error
 	}
 
 	if r.Platform == platform.OpenShift {
-		if done, res, cond, err := r.reconcileResourceMachineConfig(ctx, instance, trees); done {
-			updateStatusConditionsIfNeeded(instance, fillConditionInfoFromError(cond, err))
-			return res, err
+		if step := r.reconcileResourceMachineConfig(ctx, instance, trees); step.Done {
+			updateStatusConditionsIfNeeded(instance, fillConditionInfoFromError(step.ConditionInfo, step.Error))
+			return step.Result, step.Error
 		}
 	}
 
-	dsPerMCP, done, res, cond, err := r.reconcileResourceDaemonSet(ctx, instance, trees)
-	if done {
-		updateStatusConditionsIfNeeded(instance, fillConditionInfoFromError(cond, err))
-		return res, err
+	dsPerMCP, step := r.reconcileResourceDaemonSet(ctx, instance, trees)
+	if step.Done {
+		updateStatusConditionsIfNeeded(instance, fillConditionInfoFromError(step.ConditionInfo, step.Error))
+		return step.Result, step.Error
 	}
 
 	// all fields of NodeGroupStatus are required so publish the status only when all daemonset and MCPs are updated which