operator v1: don't return both result and error

Controller-runtime forbids returning both result and error.
However, ar.Ensure() code provoked such cases; because it reconciles
multiple "inner" resources, and some of them might return an error, in
addition to some other returning a ctrl.Result.

to over come this, ar.Ensure now makes the deliberate choice to
prioritize errors - if any non-requeue error is returned by an attached
resource, it will return just the error, and ignore the ctrl.Result.

in addition, one case - where a STS cannot reconcile, because "the other
STS" is being decommissioned - was changed to not return an error. It's
not an error we need to report, it's really just an ask for a requeue to
make sure it gets reconciled, after the condition has settled.

operator/internal/controller/vectorized/cluster_controller.go

@@ -233,11 +233,11 @@ func (r *ClusterReconciler) Reconcile(
 	}
 
 	result, errs := ar.Ensure()
-	if !result.IsZero() && errs == nil {
-		return result, nil
-	}
 	if errs != nil {
-		return result, errs
+		return ctrl.Result{}, errs
+	}
+	if !result.IsZero() {
+		return result, nil
 	}
 
 	adminAPI, err := r.AdminAPIClientFactory(ctx, r.Client, &vectorizedCluster, ar.getHeadlessServiceFQDN(), pki.AdminAPIConfigProvider())

operator/internal/controller/vectorized/cluster_controller_attached_resources.go

@@ -92,7 +92,15 @@ func (a *attachedResources) Ensure() (ctrl.Result, error) {
 			errs = errors.Join(errs, err)
 		}
 	}
-	return result, errs
+	// Controller-runtime does not allow returning a result and an error at the same time.
+	// We choose to prioritize the error - if there is an error, this is more important
+	// to us to return than the reconcile.
+	// Downstream functions are expected to only return errors, if there is an
+	// actual error condition, not generally to cause a requeue (must use result for this purpose).
+	if errs != nil {
+		return ctrl.Result{}, errs
+	}
+	return result, nil
 }
 
 func (a *attachedResources) bootstrapService() {

operator/pkg/resources/statefulset_scale.go

@@ -194,8 +194,12 @@ func (r *StatefulSetResource) handleDecommissionInProgress(ctx context.Context,
 
 	// Decom Pod is from another NodePool. Super important to early exit here, as it's not our business (in this StatefulSet handler).
 	if brokerPod == nil {
-		log.Info("decom on other in progress")
-		return fmt.Errorf("decommission on other NodePool in progress, can't handle decom for this one yet")
+		log.Info("decom on other NodePool in progress. asking for requeue so this nodepool can get reconciled afterwards.")
+		return &RequeueAfterError{
+			RequeueAfter: wait.Jitter(r.decommissionWaitInterval, decommissionWaitJitterFactor),
+			Msg:          "decommission on other NodePool in progress, can't handle decom for this one yet",
+		}
+
 	}
 
 	if !r.nodePool.Deleted && *r.nodePool.Replicas >= r.pandaCluster.Status.NodePools[r.nodePool.Name].CurrentReplicas {