Document ReconcileResult type (fixes k8ssandra#1419)

controllers/k8ssandra/medusa_reconciler.go

@@ -109,11 +109,7 @@ func (r *K8ssandraClusterReconciler) reconcileMedusa(
 				return result.Error(err)
 			}
 			purgeCronJob.SetLabels(labels.CleanedUpByLabels(kcKey))
-			recRes := reconciliation.ReconcileObject(ctx, remoteClient, r.DefaultDelay, *purgeCronJob)
-			switch {
-			case recRes.IsError():
-				return recRes
-			case recRes.IsRequeue():
+			if recRes := reconciliation.ReconcileObject(ctx, remoteClient, r.DefaultDelay, *purgeCronJob); recRes.Completed() {
 				return recRes
 			}
 		} else {
@@ -173,12 +169,7 @@ func (r *K8ssandraClusterReconciler) reconcileMedusaSecrets(
 			return result.Error(err)
 		}
 
-		res := r.reconcileRemoteBucketSecretsDeprecated(ctx, r.ClientCache.GetLocalClient(), kc, logger)
-		switch {
-		case res.IsError():
-			logger.Error(res.GetError(), "Failed to reconcile Medusa bucket secrets")
-			return res
-		case res.IsRequeue():
+		if res := r.reconcileRemoteBucketSecretsDeprecated(ctx, r.ClientCache.GetLocalClient(), kc, logger); res.Completed() {
 			return res
 		}
 	}
@@ -202,11 +193,7 @@ func (r *K8ssandraClusterReconciler) reconcileMedusaConfigMap(
 		desiredConfigMap := medusa.CreateMedusaConfigMap(namespace, kc.SanitizedName(), medusaIni)
 		kcKey := utils.GetKey(kc)
 		desiredConfigMap.SetLabels(labels.CleanedUpByLabels(kcKey))
-		recRes := reconciliation.ReconcileObject(ctx, remoteClient, r.DefaultDelay, *desiredConfigMap)
-		switch {
-		case recRes.IsError():
-			return recRes
-		case recRes.IsRequeue():
+		if recRes := reconciliation.ReconcileObject(ctx, remoteClient, r.DefaultDelay, *desiredConfigMap); recRes.Completed() {
 			return recRes
 		}
 	}

controllers/k8ssandra/vector.go

@@ -41,11 +41,7 @@ func (r *K8ssandraClusterReconciler) reconcileVector(
 
 		// Check if the vector config map already exists
 		desiredVectorConfigMap.SetLabels(labels.CleanedUpByLabels(kcKey))
-		recRes := reconciliation.ReconcileObject(ctx, remoteClient, r.DefaultDelay, *desiredVectorConfigMap)
-		switch {
-		case recRes.IsError():
-			return recRes
-		case recRes.IsRequeue():
+		if recRes := reconciliation.ReconcileObject(ctx, remoteClient, r.DefaultDelay, *desiredVectorConfigMap); recRes.Completed() {
 			return recRes
 		}
 	} else {

controllers/reaper/vector.go

@@ -42,14 +42,9 @@ func (r *ReaperReconciler) reconcileVectorConfigMap(
 			dcLogger.Error(err, "Failed to set controller reference on new Reaper Vector ConfigMap", "ConfigMap", configMapKey)
 			return ctrl.Result{}, err
 		}
-		recRes := reconciliation.ReconcileObject(ctx, remoteClient, r.DefaultDelay, *desiredVectorConfigMap)
-		switch {
-		case recRes.IsError():
-			return ctrl.Result{}, recRes.GetError()
-		case recRes.IsRequeue():
-			return ctrl.Result{RequeueAfter: r.DefaultDelay}, nil
+		if recRes := reconciliation.ReconcileObject(ctx, remoteClient, r.DefaultDelay, *desiredVectorConfigMap); recRes.Completed() {
+			return recRes.Output()
 		}
-
 	} else {
 		if err := shared.DeleteConfigMapIfExists(ctx, remoteClient, configMapKey, dcLogger); err != nil {
 			return ctrl.Result{}, err

controllers/stargate/stargate_controller.go

@@ -446,12 +446,8 @@ func (r *StargateReconciler) reconcileStargateConfigMap(
 		return ctrl.Result{}, err
 	}
 
-	recRes := reconciliation.ReconcileObject(ctx, r.Client, r.DefaultDelay, *desiredConfigMap)
-	switch {
-	case recRes.IsError():
-		return ctrl.Result{}, recRes.GetError()
-	case recRes.IsRequeue():
-		return ctrl.Result{RequeueAfter: r.DefaultDelay}, nil
+	if recRes := reconciliation.ReconcileObject(ctx, r.Client, r.DefaultDelay, *desiredConfigMap); recRes.Completed() {
+		return recRes.Output()
 	}
 	logger.Info("Stargate ConfigMap successfully reconciled")
 	return ctrl.Result{}, nil

controllers/stargate/vector.go

@@ -43,12 +43,8 @@ func (r *StargateReconciler) reconcileVectorConfigMap(
 			dcLogger.Error(err, "Failed to set controller reference on new Stargate Vector ConfigMap", "ConfigMap", configMapKey)
 			return ctrl.Result{}, err
 		}
-		recRes := reconciliation.ReconcileObject(ctx, remoteClient, r.DefaultDelay, *desiredVectorConfigMap)
-		switch {
-		case recRes.IsError():
-			return ctrl.Result{}, recRes.GetError()
-		case recRes.IsRequeue():
-			return ctrl.Result{RequeueAfter: r.DefaultDelay}, nil
+		if recRes := reconciliation.ReconcileObject(ctx, remoteClient, r.DefaultDelay, *desiredVectorConfigMap); recRes.Completed() {
+			return recRes.Output()
 		}
 	} else {
 		if err := deleteConfigMapIfExists(ctx, remoteClient, configMapKey, dcLogger); err != nil {

pkg/medusa/refresh_secrets.go

@@ -52,7 +52,7 @@ func RefreshSecrets(dc *cassdcapi.CassandraDatacenter, ctx context.Context, clie
 		case recRes.IsRequeue():
 			requeues++
 			continue
-		case recRes.IsDone():
+		case !recRes.Completed():
 			continue
 		}
 		if requeues > 0 {

pkg/reconciliation/generic.go

@@ -18,7 +18,8 @@ type Reconcileable[T any] interface {
 	*T
 }
 
-// Try with U, a type of any whose POINTER still fulfils Reoncilable...
+// ReconcileObject ensures that desiredObject exists in the given state, either by creating it, or updating it if it
+// already exists.
 func ReconcileObject[U any, T Reconcileable[U]](ctx context.Context, kClient client.Client, requeueDelay time.Duration, desiredObject U) result.ReconcileResult {
 	objectKey := types.NamespacedName{
 		Name:      T(&desiredObject).GetName(),
@@ -53,5 +54,5 @@ func ReconcileObject[U any, T Reconcileable[U]](ctx context.Context, kClient cli
 		}
 		return result.RequeueSoon(requeueDelay)
 	}
-	return result.Done()
+	return result.Continue()
 }

pkg/reconciliation/generic_test.go

@@ -44,7 +44,7 @@ func Test_ReconcileObject_UpdateDone(t *testing.T) {
 	assert.NoError(t, err)
 	// If we reconcile again, we should move into the Done state.
 	recRes = ReconcileObject(ctx, kClient, requeueDelay, desiredObject)
-	assert.True(t, recRes.IsDone())
+	assert.False(t, recRes.Completed())
 }
 
 func Test_ReconcileObject_CreateSuccess(t *testing.T) {

pkg/result/result_helper.go

@@ -1,20 +1,54 @@
 package result
 
 import (
+	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 	"time"
 
 	ctrl "sigs.k8s.io/controller-runtime"
 )
 
-// Copyright DataStax, Inc.
-// Please see the included license file for details.
-
+// This is just so that we can reference TerminalError in the Godoc of [Error]
+var _ error = reconcile.TerminalError(nil)
+
+// ReconcileResult represents the result of a step in the reconciliation process.
+//
+// We typically split the top-level Reconcile() method of a controller into multiple sub-functions. Each of these
+// functions uses ReconcileResult to communicate to its caller how the current iteration should proceed. There are 4
+// possible implementations: [Continue], [Done], [RequeueSoon], and [Error].
+//
+// The idiomatic way to handle a ReconcileResult in an intermediary sub-function is:
+//
+//	if recResult := callStep1(); recResult.Completed() {
+//	   // Global success, requeue or error: stop what we're doing and propagate up
+//	   return recResult
+//	}
+//	// Otherwise, proceed with the next step(s)
+//	if recResult := callStep2(); recResult.Completed() {
+//	   // etc...
+//
+// The idiomatic way to handle a ReconcileResult in the top-level Reconcile() method is:
+//
+//	recResult := callSomeSubmethod()
+//	// Possibly inspect the result (e.g. to set an error field in the status)
+//	return recResult.Output()
 type ReconcileResult interface {
+	// Completed indicates that this result is *NOT* a [Continue].
+	//
+	// In other words, it means that the current iteration of the reconciliation loop is complete, and the top-level
+	// Reconcile() method should return to the controller runtime (either with a success or terminal error that will
+	// stop the entire reconciliation loop, or a requeue or regular error that will trigger a retry).
 	Completed() bool
+	// Output converts this result into a format that the main Reconcile() method can return to the controller runtime.
+	//
+	// Calling this method on a [Continue] will panic.
 	Output() (ctrl.Result, error)
+	// IsError indicates whether this result is an [Error].
 	IsError() bool
+	// IsRequeue indicates whether this result is a [RequeueSoon].
 	IsRequeue() bool
+	// IsDone indicates whether this result is a [Done].
 	IsDone() bool
+	// GetError returns the wrapped error if the result is an [Error], otherwise it returns nil.
 	GetError() error
 }
 
@@ -121,18 +155,31 @@ func (r errorOut) GetError() error {
 	return r.err
 }
 
+// Continue indicates that the current step in the reconciliation is done. The caller should proceed with the next step.
 func Continue() ReconcileResult {
 	return continueReconcile{}
 }
 
+// Done indicates that the entire reconciliation loop was successful.
+// The caller should skip the next steps (if any), and propagate the result up the stack. This will eventually reach the
+// top-level Reconcile() method, which should stop the reconciliation.
 func Done() ReconcileResult {
 	return done{}
 }
 
+// RequeueSoon indicates that the current step in the reconciliation requires a requeue after a certain amount of time.
+// The caller should skip the next steps (if any), and propagate the result up the stack. This will eventually reach the
+// top-level Reconcile() method, which should schedule a requeue with the given delay.
 func RequeueSoon(after time.Duration) ReconcileResult {
 	return callBackSoon{after: after}
 }
 
+// Error indicates that the current step in the reconciliation has failed.
+// The caller should skip the next steps (if any), and propagate the result up the stack. This will eventually reach the
+// top-level Reconcile() method, which should return the error to the controller runtime.
+//
+// If the argument is wrapped with [reconcile.TerminalError], the reconciliation loop will stop; otherwise, it will be
+// retried with exponential backoff.
 func Error(e error) ReconcileResult {
 	return errorOut{err: e}
 }

pkg/telemetry/cassandra_agent/cassandra_agent_config.go

@@ -158,11 +158,7 @@ func (c Configurator) ReconcileTelemetryAgentConfig(dc *cassdcapi.CassandraDatac
 	}
 	desiredCm.SetLabels(labels.CleanedUpByLabels(KlKey))
 
-	recRes := reconciliation.ReconcileObject(c.Ctx, c.RemoteClient, c.RequeueDelay, *desiredCm)
-	switch {
-	case recRes.IsError():
-		return recRes
-	case recRes.IsRequeue():
+	if recRes := reconciliation.ReconcileObject(c.Ctx, c.RemoteClient, c.RequeueDelay, *desiredCm); recRes.Completed() {
 		return recRes
 	}