feat: plan-centric reconcile foundations (Steps 1-4)#87
Merged
Conversation
Lay the groundwork for making Plan the central reconciliation abstraction across all SeiNode phases. This is the non-behavioral half of the refactor — all new code is additive and existing behavior is unchanged. Step 1: Add TargetPhase/FailedPhase to TaskPlan CRD - Executor sets node phase atomically with plan completion/failure - Both fields are optional; empty means no transition (backward compatible) Step 2: Extract resource generators to internal/noderesource/ - Moves StatefulSet, Service, PVC generation out of the controller package - Breaks the import cycle that would exist between task and controller - Controller now calls noderesource.* directly (no delegate wrappers) Step 3: New controller-side tasks (ensure-data-pvc, apply-statefulset, apply-service) - Fire-and-forget tasks that complete synchronously in Execute - Registered in the task registry, not yet wired into any plan Step 4: Executor handles TargetPhase and convergence plan cleanup - On plan completion: applies TargetPhase to node status in same patch - On plan failure: applies FailedPhase to node status in same patch - Convergence plans (target == current phase): nils the plan on completion - OnPlanComplete/OnPlanFailed callbacks for metrics/events Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
bdchatham
commented
Apr 15, 2026
Remove PhaseTransition struct and OnPlanComplete/OnPlanFailed callbacks. The executor directly owns phase transitions via TargetPhase/FailedPhase without needing callback indirection. On plan failure, set a PlanFailed Condition on the node with the failed task type as the Reason and a message describing what went wrong. This follows the standard K8s Conditions pattern already used by monitor tasks. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Two fixes from expert review: 1. PlanFailed Condition Reason now uses "TaskFailed" (CamelCase) instead of the kebab-case task type string. K8s convention requires Reasons to be CamelCase machine-readable tokens. The task type is already in the Message field. 2. nilPlanIfConvergence now captures the pre-mutation phase and compares against it, so only convergence plans (same phase before and after) get nilled. Init plans (Initializing → Running) keep their completed plan visible in status for observability. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Lays the groundwork for making
Planthe central reconciliation abstraction across all SeiNode phases. This PR contains the non-behavioral half — all new code is additive and existing controller behavior is unchanged.TargetPhase/FailedPhasetoTaskPlanso the executor can drive phase transitions atomically with plan completion/failureinternal/noderesource/, breaking the import cycle needed for Step 3ensure-data-pvc,apply-statefulset,apply-service— fire-and-forget controller-side tasks registered in the task registry (not yet wired into plans)TargetPhase/FailedPhaseon plan terminal states, nils convergence plans (target == current phase), addsOnPlanComplete/OnPlanFailedcallbacksWhat's next (separate PR)
Steps 5-6 will wire these foundations into the actual reconcile loop:
BuildPlan(init plans include infrastructure tasks, Running phase gets convergence plans)ForNodethat stamps plans directly ontonode.Status.PlanForNode → ExecutePlanTest plan
make test— all existing tests passmake lint— zero issuesmake build— compiles cleanlymake manifests generate— CRDs regeneratedexecutor.gobootstrap_job.go)🤖 Generated with Claude Code