fix: separate plan creation from execution into atomic steps (#91)

A new plan is now persisted before any tasks execute. This prevents a
crash-safety issue where side effects (e.g., StatefulSet updates) could
occur before the plan and its conditions are flushed to the API server.

Before: ResolvePlan builds a plan and ExecutePlan runs tasks in the same
reconcile. A crash after task execution but before the flush loses the
plan and any conditions — external observers never see the update.

After: When ResolvePlan creates a new plan, the reconciler flushes it
immediately and requeues. Execution starts on the next reconcile, after
the plan is persisted. This guarantees external controllers (e.g.,
SeiNodeDeployment) see the plan and conditions before any side effects.

The nodedeployment controller already followed this pattern (startPlan
returns ResultRequeueImmediate without executing). This change aligns
the node controller to the same atomic guarantee.

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: bdchatham

internal/controller/node/controller.go

@@ -105,18 +105,18 @@ func (r *SeiNodeReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 		return ctrl.Result{}, fmt.Errorf("resolving plan: %w", err)
 	}
 
-	// Execute the plan. The executor advances tasks in-memory — synchronous
-	// tasks complete in a loop, async tasks return Running with a poll interval.
 	var result ctrl.Result
 	var execErr error
-	if node.Status.Plan != nil && node.Status.Plan.Phase == seiv1alpha1.TaskPlanActive {
-		result, execErr = r.PlanExecutor.ExecutePlan(ctx, node, node.Status.Plan)
-		statusDirty = true
-	}
 
-	// If ResolvePlan built a new plan (but there's nothing to execute yet
-	// because it was just created this reconcile), mark dirty so it gets persisted.
 	if !planAlreadyActive && node.Status.Plan != nil {
+		// New plan — persist it before executing. Execution starts on the
+		// next reconcile. This guarantees external observers see the plan
+		// (and any conditions) before side effects occur.
+		statusDirty = true
+		result = planner.ResultRequeueImmediate
+	} else if node.Status.Plan != nil && node.Status.Plan.Phase == seiv1alpha1.TaskPlanActive {
+		// Existing plan — execute tasks in-memory.
+		result, execErr = r.PlanExecutor.ExecutePlan(ctx, node, node.Status.Plan)
 		statusDirty = true
 	}
 

internal/controller/node/plan_execution_integration_test.go

@@ -77,14 +77,17 @@ func TestIntegrationFullProgressionSnapshotMode(t *testing.T) {
 		return n
 	}
 
-	// First Reconcile: builds plan, transitions to Initializing, completes
-	// all synchronous infrastructure tasks, and submits the first sidecar task.
+	// First Reconcile: builds and persists the plan.
 	reconcileOnce(t, g, r, node.Name, node.Namespace)
 	node = fetch()
 	g.Expect(node.Status.Plan).NotTo(BeNil())
 	g.Expect(node.Status.Phase).To(Equal(seiv1alpha1.PhaseInitializing))
 
-	// Drive sidecar tasks (infrastructure tasks already completed in first reconcile).
+	// Second Reconcile: executes all synchronous infrastructure tasks
+	// and submits the first sidecar task.
+	reconcileOnce(t, g, r, node.Name, node.Namespace)
+
+	// Drive sidecar tasks.
 	driveTask(t, g, r, mock, fetch, planner.TaskSnapshotRestore)
 	driveTask(t, g, r, mock, fetch, planner.TaskConfigureGenesis)
 	driveTask(t, g, r, mock, fetch, planner.TaskConfigApply)
@@ -111,13 +114,15 @@ func TestIntegrationFullProgressionGenesisMode(t *testing.T) {
 		return n
 	}
 
-	// First Reconcile: builds plan, transitions to Initializing, completes
-	// all synchronous infrastructure tasks, and submits the first sidecar task.
+	// First Reconcile: builds and persists the plan.
 	reconcileOnce(t, g, r, node.Name, node.Namespace)
 	node = fetch()
 	g.Expect(node.Status.Plan).NotTo(BeNil())
 
-	// Drive sidecar tasks (infrastructure tasks already completed in first reconcile).
+	// Second Reconcile: executes infrastructure tasks, submits first sidecar task.
+	reconcileOnce(t, g, r, node.Name, node.Namespace)
+
+	// Drive sidecar tasks.
 	driveTask(t, g, r, mock, fetch, planner.TaskConfigureGenesis)
 	// Completing config-apply also chain-completes the trailing fire-and-forget
 	// tasks (config-validate, mark-ready) and the plan, transitioning to Running.
@@ -141,13 +146,13 @@ func TestIntegrationTaskFailure_FailsPlan(t *testing.T) {
 		return n
 	}
 
-	// First Reconcile: builds plan, transitions to Initializing, completes
-	// all synchronous infrastructure tasks, and submits the first sidecar task.
+	// First Reconcile: builds and persists the plan.
 	reconcileOnce(t, g, r, node.Name, node.Namespace)
 	node = fetch()
 	g.Expect(node.Status.Plan).NotTo(BeNil())
 
-	// Drive snapshot-restore: the first reconcile already submitted it.
+	// Second Reconcile: executes infrastructure tasks, submits snapshot-restore.
+	reconcileOnce(t, g, r, node.Name, node.Namespace)
 	node = fetch()
 	ct := planner.CurrentTask(node.Status.Plan)
 	g.Expect(ct).NotTo(BeNil())