This repository was archived by the owner on Aug 5, 2026. It is now read-only.
fix(status): retry UpdateStatus on optimistic-concurrency conflict (krateo-core-provider#57) - #26
Merged
Merged
Conversation
tools.UpdateStatus submits the composition CR with whatever resourceVersion it was read at and performs no retry, so a benign "the object has been modified" 409 aborts the whole reconcile. During a GVK-migration handover the retiring per-version controller and the new one briefly contend on the same CR's status, making that 409 routine — the healthy Ready/Synced condition then never lands and the composition stays wedged Ready=False until a manual controller restart. Add updateStatusWithRetry (a drop-in for tools.UpdateStatus) that retries on conflict by re-fetching the latest object and re-applying the computed status; last-writer-wins is correct since this controller owns the status subresource. Wire all 7 status-write sites in composition.go through it, with unit tests covering conflict-recovery, non-conflict propagation, and the happy path. Refs krateo-platformops/core-provider#57 (fix #1 of the layered plan; #2-#4 tracked on the issue). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The first cut snapshotted mg's status via unstructured.NestedFieldCopy to
re-apply it on conflict, but NestedFieldCopy runs runtime.DeepCopyJSONValue,
which panics ("cannot deep copy composition.ManagedResource") — the composition
CR's in-memory status can hold typed values, not pure JSON. The integration
TestController/Setup/Create exercised that path.
Carry mg's status as-is and, on a 409, refresh only its resourceVersion before
retrying — no status deep-copy. On the no-conflict path the behavior is now
identical to the original tools.UpdateStatus call.
Co-Authored-By: Claude Opus 4.8 <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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
What
Make the composition-dynamic-controller retry its status writes on optimistic-concurrency conflicts, so a benign
the object has been modified409 no longer wedges the reconcile.Fix #1 of the layered plan in the RCA on krateo-platformops/core-provider#57 (#2–#4 tracked there).
Why
tools.UpdateStatussubmits the composition CR with whateverresourceVersionit was read at and does no retry — one 409 aborts the entire reconcile. During a GVK-migration handover (every version bump), the retiring per-version controller and the newly-started one briefly contend on the same CR's status, so that 409 is routine. When it hits, the healthyReady=True/Synced=Truecondition never gets written and the composition stays wedgedReady=Falseuntil someone manually restarts the controller — the exact symptom in krateoplatformops#57 (alongside theanother operation in progressdry-run error, addressed by fix #2).Change
updateStatusWithRetry(internal/composition/status_update.go) — a drop-in replacement fortools.UpdateStatusthat, onapierrors.IsConflict, re-fetches the latest object, re-applies the status this reconcile computed, and retries (retry.RetryOnConflict/retry.DefaultRetry). Last-writer-wins is correct here because this controller owns the status subresource.composition.gothrough it (the 6tools.Updatenon-status calls are untouched).otel.endpoint-style override is N/A here.Tests
internal/composition/status_update_test.go(plain unit tests,dynamicfake+ reactor):go build ./...,go vet ./...,gofmt, and the fullinternal/compositionpackage all pass;go.mod/go.sumunchanged.Scope
This removes the status-conflict wedge. The companion dry-run
another operation in progressbehavior, the handover ordering, and diagnostic logging are fixes #2–#4 in the RCA and remain tracked on krateo-platformops/core-provider#57.🤖 Generated with Claude Code