Address review comments from @unmarshall: simplify `operatorContext.D…

…ata` and `GetBoolValueOrDefault()` function

internal/controller/etcd/reconcile_status.go

@@ -82,10 +82,7 @@ func (r *Reconciler) inspectStatefulSetAndMutateETCDStatus(ctx component.Operato
 }
 
 func (r *Reconciler) mutateObservedGeneration(ctx component.OperatorContext, etcd *druidv1alpha1.Etcd, _ logr.Logger) ctrlutils.ReconcileStepResult {
-	canReconcileSpec := utils.GetBoolValueOrDefault(ctx.Data, reconciliationContextDataKeyCanReconcileSpec, false)
-	shortCircuitSpecReconcile := utils.GetBoolValueOrDefault(ctx.Data, reconciliationContextDataKeyShortCircuitSpecReconcile, true)
-
-	if canReconcileSpec && !shortCircuitSpecReconcile {
+	if utils.GetBoolValueOrDefault(ctx.Data, reconciliationContextDataKeyWasSpecReconciled, false) {
 		etcd.Status.ObservedGeneration = &etcd.Generation
 	}
 	return ctrlutils.ContinueReconcile()

internal/controller/etcd/reconciler.go

@@ -35,10 +35,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 )
 
-const (
-	reconciliationContextDataKeyCanReconcileSpec          = "canReconcileSpec"
-	reconciliationContextDataKeyShortCircuitSpecReconcile = "shortCircuitSpecReconcile"
-)
+const reconciliationContextDataKeyWasSpecReconciled = "wasSpecReconciled"
 
 // Reconciler reconciles the Etcd resource spec and status.
 type Reconciler struct {
@@ -99,7 +96,8 @@ type reconcileFn func(ctx component.OperatorContext, objectKey client.ObjectKey)
 //     and if there is a need then reconcile spec.
 //  3. Status Reconciliation: Always update the status of the Etcd component to reflect its current state,
 //     as well as status fields derived from spec reconciliation.
-//  4. Scheduled Requeue: Requeue the reconciliation request after a defined period (EtcdStatusSyncPeriod) to maintain sync.
+//  4. Remove operation-reconcile annotation if it was set and if spec reconciliation had succeeded.
+//  5. Scheduled Requeue: Requeue the reconciliation request after a defined period (EtcdStatusSyncPeriod) to maintain sync.
 func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
 	runID := string(controller.ReconcileIDFromContext(ctx))
 	operatorCtx := component.NewOperatorContext(ctx, r.logger, runID)
@@ -116,8 +114,9 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 	var reconcileSpecResult ctrlutils.ReconcileStepResult
 	if canReconcileSpec {
 		reconcileSpecResult = r.reconcileSpec(operatorCtx, req.NamespacedName)
-		operatorCtx.Data[reconciliationContextDataKeyCanReconcileSpec] = strconv.FormatBool(canReconcileSpec)
-		operatorCtx.Data[reconciliationContextDataKeyShortCircuitSpecReconcile] = strconv.FormatBool(ctrlutils.ShortCircuitReconcileFlow(reconcileSpecResult))
+		if !ctrlutils.ShortCircuitReconcileFlow(reconcileSpecResult) {
+			operatorCtx.Data[reconciliationContextDataKeyWasSpecReconciled] = strconv.FormatBool(true)
+		}
 	}
 
 	reconcileStatusResult := r.reconcileStatus(operatorCtx, req.NamespacedName)
@@ -126,6 +125,10 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 		return reconcileStatusResult.ReconcileResult()
 	}
 
+	if reconcileSpecResult.NeedsRequeue() {
+		return reconcileSpecResult.ReconcileResult()
+	}
+
 	// Operation annotation is removed at the end of reconciliation flow, after both spec and status
 	// have been successfully reconciled. This ensures that if there are any errors in either spec or status
 	// reconcile flow upon addition of operation annotation, then the next requeue of reconciliation will attempt
@@ -141,9 +144,6 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 		}
 	}
 
-	if reconcileSpecResult.NeedsRequeue() {
-		return reconcileSpecResult.ReconcileResult()
-	}
 	return ctrlutils.ReconcileAfter(r.config.EtcdStatusSyncPeriod, "Periodic Requeue").ReconcileResult()
 }
 

internal/utils/miscellaneous.go

@@ -83,30 +83,14 @@ func ComputeScheduleInterval(cronSchedule string) (time.Duration, error) {
 	return nextNextScheduledTime.Sub(nextScheduledTime), nil
 }
 
-func getValueOrError(data map[string]string, key string) (string, error) {
-	value, ok := data[key]
-	if !ok {
-		return "", fmt.Errorf("key %s not found in data", key)
-	}
-	return value, nil
-}
-
-func parseBoolOrError(value string) (bool, error) {
-	result, err := strconv.ParseBool(value)
-	if err != nil {
-		return false, fmt.Errorf("failed to parse boolean value: %w", err)
-	}
-	return result, nil
-}
-
 // GetBoolValueOrDefault returns the boolean value for the given key from the data map,
 // and defaults to defaultValue if the key is not found or the value is not a valid boolean.
 func GetBoolValueOrDefault(data map[string]string, key string, defaultValue bool) bool {
-	value, err := getValueOrError(data, key)
-	if err != nil {
+	value, ok := data[key]
+	if !ok {
 		return defaultValue
 	}
-	result, err := parseBoolOrError(value)
+	result, err := strconv.ParseBool(value)
 	if err != nil {
 		return defaultValue
 	}