Repository navigation
fix: a concurrent push must not wedge a LocalResource forever (#16) - #17
Merged
Merged
Conversation
A publish fans out one LocalResource PER FILE and they all push to the SAME branch at once. Under
that fan-out some wedge permanently with "cannot determine creation result - remove the
krateo.io/external-create-pending annotation if it is safe to proceed". Seen four times on one
cluster, always on the LATER indices, recovered by hand every time.
In every case THE COMMIT HAD ALREADY SUCCEEDED — the file was on the branch. Only the recording of
the outcome failed. Two stacked causes, both in the controller logs:
1. Git only advances a ref from the commit the pusher expects, so the losers of the race get
"cannot lock ref 'refs/heads/builder/<slug>': is at <sha>". That is the ordinary outcome of
concurrent writers, not a failure of the resource.
2. The status write then loses an optimistic-concurrency race — "the object has been modified;
please apply your changes to the latest version". It runs AFTER the push, so returning that
error aborts the sync with the work already done: external-create-pending stays set,
external-create-succeeded is never written, and every later reconcile correctly refuses to guess.
That is what makes it PERMANENT rather than transient.
Fixes:
* syncWithRetry re-runs the whole sync on contention, jittered. It retries the SYNC, not the push,
because the local ref is stale — pushing it again fails identically; only a re-clone helps.
isRefContention is deliberately narrow: auth failures, a missing repo, a clone failure and the
.krateoignore path bug are NOT retried, so a genuine error still surfaces immediately instead of
hiding behind an exhausted backoff.
* updateStatusWithRetry re-reads the object on conflict, keeps the status this sync produced and the
external name just set, and retries. A lost race is no longer permanent.
* Events now set `action`, which the events API REQUIRES. It was "", so the apiserver rejected every
event this controller emitted ("Event ... is invalid: action: Required value") and there was no
usable event trail at all — part of why this was hard to diagnose from the cluster.
Verified: go build, go vet clean, full suite 9 packages ok. New tests pin the classifier in both
directions (the verbatim observed error retries; auth/missing-repo/clone/ignore-path do not) and
that the backoff is multi-step and jittered, since siblings start milliseconds apart and a lockstep
retry would just collide again.
Refs #16.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JumEqTGGJ2hf3vjE3xFQsv
braghettos
added a commit
that referenced
this pull request
Sep 17, 2026
#17 fixed the status write on the committed path and left the identical bare `e.kube.Status().Update(ctx, cr)` in the `git.NoErrAlreadyUpToDate` branch a few lines above — same function, same failure mode, missed. Caught in review by the installer session before tagging. That branch needs it MORE, not less: * It is the variant that cannot self-heal. It makes NO commit, so a later Observe has no footer to recognise the resource by and cannot adopt it — it falls through to the terminal default branch. * #17 makes this path MORE likely to be taken. A retried sync re-clones a tree that may already contain the file from the attempt that actually won the push, so the commit becomes a no-op and lands exactly here. Also adds a SOURCE guard rather than a behavioural test, because the bug is a missed call site, not a wrong result: a behavioural test would need to mock git deeply enough to reach each branch and would still say nothing about a branch added later. The guard asserts no bare Status().Update survives outside updateStatusWithRetry. Verified it actually catches the regression — reverting line 515 fails it, restoring passes. go build, go vet clean; full suite 9 packages ok. Claude-Session: https://claude.ai/code/session_01JumEqTGGJ2hf3vjE3xFQsv Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
braghettos
added a commit
that referenced
this pull request
Sep 28, 2026
…onger wedges the Repo (#26) (#27) A Repo whose push had LANDED wedged permanently on krateo.io/external-create-pending. The status write after the push lost an optimistic-concurrency race, Create returned that 409, and provider-runtime answers a conflict from Create by requeueing without recording an outcome (reconciler.go:997-1008) — pending stays with neither -succeeded nor -failed, and every later reconcile refuses with "cannot determine creation result". Seen on krateo-057 (0.15.4) on the first publish of a blueprint, 14ms after a successful push. - Both status writes in SyncRepos (the committed path and the already-up-to-date path) go through updateStatusWithRetry: RetryOnConflict that re-reads the object and keeps the status the sync produced and the external name it set — the LocalResource fix from #16, which Repo never got. - A status-subresource write returns the server's object, whose annotations replace the copy's, so the helper sets the external name again after the write: the runtime persists it as the create's critical annotation, the one record of the push that does not depend on the status write. - Observe reads the target commit from the external name when status.targetCommitId is empty, instead of checking the history for "" — so a Repo whose status write was lost converges. Tests: a REAL conflict (a concurrent write makes the reconciler's resourceVersion stale) — the status lands, the concurrent write survives, the external name survives; with the refresh removed the test fails. A source guard pins that no bare Status().Update exists outside the helper (the LocalResource guard, #17). go test -race ./... passes. Not fixed here: Repos wedged before this release (#22) — resolving an existing pending annotation needs the runtime-side confirm-the-negative. Claude-Session: https://claude.ai/code/session_016V8EVEvyty3XvXMHUWJvPg Co-authored-by: Claude Opus 5.5 <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
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.
Fixes #16.
A publish fans out one
LocalResourceper file and they all push to the same branch at once. Under that fan-out some wedge permanently:Seen four times on one cluster, always on the later indices, recovered by hand every time. In every case the commit had already succeeded — the file was on the branch. Only the recording of the outcome failed.
Two stacked causes, both in the controller logs
1. Ref contention makes the push fail. Git only advances a ref from the commit the pusher expects:
That is the ordinary outcome of concurrent writers, not a failure of the resource.
2. The status write then loses an optimistic-concurrency race — and this is what makes it permanent:
It runs after the push, so returning that error aborts the sync with the work already done.
external-create-pendingstays set,external-create-succeededis never written, and every later reconcile correctly refuses to guess whether the create happened.The timing shows it cleanly — healthy
-000: pending07:58:57→ succeeded07:58:59, uncontended. Wedged-005: pending07:58:59, straight into the window its own siblings created, succeeded never.Fixes
syncWithRetryre-runs the sync on contention, jittered. It retries the sync, not the push — the local ref is stale, so pushing it again fails identically; only a re-clone helps.isRefContentionis deliberately narrow. Auth failures, a missing repo, a clone failure and the.krateoignorepath bug are not retried, so a genuine error still surfaces immediately instead of hiding behind an exhausted backoff. Both directions are tested.updateStatusWithRetryre-reads the object on conflict, keeps the status this sync produced and the external name just set, then retries. A lost race is no longer permanent.Events set
action. The events API requires it; it was"", so the apiserver rejected every event this controller emitted:There was no usable event trail at all, which is part of why this was hard to diagnose from the cluster.
Verification
go build ./...,go vet ./internal/...cleanNote on blast radius
The retry is bounded (6 steps, ~400ms base, factor 2, jitter 0.5). A sustained genuine outage still fails, just later; that trade is deliberate, and the narrow classifier keeps it to cases where a re-clone is the actual remedy.
🤖 Generated with Claude Code
https://claude.ai/code/session_01JumEqTGGJ2hf3vjE3xFQsv