diff --git a/controllers/k8ssandra/medusa_reconciler.go b/controllers/k8ssandra/medusa_reconciler.go index e55f7ca5e..357c1b52f 100644 --- a/controllers/k8ssandra/medusa_reconciler.go +++ b/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 } } diff --git a/controllers/k8ssandra/vector.go b/controllers/k8ssandra/vector.go index 730d4a2ea..69e44ee65 100644 --- a/controllers/k8ssandra/vector.go +++ b/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 { diff --git a/controllers/reaper/vector.go b/controllers/reaper/vector.go index c0a6f572f..c8518821a 100644 --- a/controllers/reaper/vector.go +++ b/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 diff --git a/controllers/stargate/stargate_controller.go b/controllers/stargate/stargate_controller.go index 6dfe7dfa8..00316a8fe 100644 --- a/controllers/stargate/stargate_controller.go +++ b/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 diff --git a/controllers/stargate/vector.go b/controllers/stargate/vector.go index 0884c378c..1ec9bb299 100644 --- a/controllers/stargate/vector.go +++ b/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 { diff --git a/pkg/medusa/refresh_secrets.go b/pkg/medusa/refresh_secrets.go index 659b24aee..132ca43ad 100644 --- a/pkg/medusa/refresh_secrets.go +++ b/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 { diff --git a/pkg/reconciliation/generic.go b/pkg/reconciliation/generic.go index 2d3c9cbb7..9771400d7 100644 --- a/pkg/reconciliation/generic.go +++ b/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() } diff --git a/pkg/reconciliation/generic_test.go b/pkg/reconciliation/generic_test.go index a2bc5abfa..a014686a8 100644 --- a/pkg/reconciliation/generic_test.go +++ b/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) { diff --git a/pkg/result/result_helper.go b/pkg/result/result_helper.go index 0133074f1..3558a6382 100644 --- a/pkg/result/result_helper.go +++ b/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} } diff --git a/pkg/telemetry/cassandra_agent/cassandra_agent_config.go b/pkg/telemetry/cassandra_agent/cassandra_agent_config.go index c69062d80..fa75ae21b 100644 --- a/pkg/telemetry/cassandra_agent/cassandra_agent_config.go +++ b/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 }