Merged
Conversation
Refactors both controllers to accumulate all status mutations in-memory and flush with a single Status().Patch() at the end of Reconcile. Executor changes: - Removes Client field — executor is now a pure in-memory mutation engine - Task loop: synchronous tasks advance in sequence without intermediate patches - failTask is now a void function (in-memory mutations only) Node controller changes: - Single statusBase captured after ensureNodeFinalizer - reconcilePeers returns (bool, error) dirty flag, mutates in-memory - observeCurrentImage returns (bool, error), mutates in-memory - Single Status().Patch() at end, gated on statusDirty - Flushes before returning exec errors (preserves partial progress) NodeDeployment controller changes: - reconcilePlan no longer takes statusBase (in-memory mutations) - completePlan/failPlan are now void (in-memory, no re-read) - startPlan is now void (in-memory, caller flushes) Tests need updating to match the new in-memory executor behavior. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Both controllers now accumulate all status mutations in-memory and flush with a single Status().Patch() at the end of Reconcile. This eliminates optimistic lock conflicts between tasks and the executor, and establishes a clean ownership model: tasks mutate owned resources, the executor mutates plan state in-memory, the reconciler flushes once. Executor changes: - Removes Client field — pure in-memory mutation engine - Task loop: synchronous tasks advance in sequence without patches - advanceTask returns ctrl.Result (no error — failures are in-memory) - failTask is void (in-memory mutations only) Node controller changes: - Single statusBase captured after ensureNodeFinalizer - reconcilePeers returns (bool, error) dirty flag, mutates in-memory - observeCurrentImage returns (bool, error), mutates in-memory - Single Status().Patch() at end, gated on statusDirty - Flushes before returning exec errors (preserves partial progress) NodeDeployment controller changes: - reconcilePlan no longer takes statusBase - completePlan/failPlan are void (in-memory, no re-read needed) - startPlan is void (caller flushes) Test updates: - Executor unit tests assert against in-memory object (no API re-fetch) - Integration tests use Reconcile (full pipeline with flush) - peers_test, reconciler_test assert in-memory mutations - nodedeployment plan_test asserts in-memory group status Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
bdchatham
commented
Apr 16, 2026
- Trim verbose strategy comment in Reconcile, add concise method doc - Log exec error when status flush also fails (don't swallow silently) - Remove "Task completed synchronously" comment from executor loop - Use log.FromContext(ctx) instead of log.Log in failTask Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
Both controllers now accumulate all status mutations in-memory and flush with a single
Status().Patch()at the end of Reconcile. This eliminates the class of optimistic lock conflicts between tasks and the executor, and establishes a clean ownership model.Clientfield entirely — pure in-memory mutation engine. Synchronous tasks advance in a loop without intermediate patches.failTaskis void.statusBasecaptured after finalizer.reconcilePeersandobserveCurrentImagereturn dirty flags. One flush at the end.completePlan/failPlanare now void in-memory mutations. No more re-read after executor patches (because there are none).Ownership model
Status().Patch()flush at endWhat this unblocks
Tasks can now freely mutate the owning resource's status fields (e.g.,
status.currentImage, conditions) because everything is in-memory until the flush. This is the foundation for NodeUpdate plans where tasks need to set conditions and stamp image state.Test plan
make build— compiles cleanlymake test— all tests pass (84.6% node controller, 43.5% planner, 32.9% nodedeployment)make lint— zero issues🤖 Generated with Claude Code