Fix Stack wedging when status.currentUpdate is never cleared - #1295
Open
bpalermo wants to merge 1 commit into
Open
Fix Stack wedging when status.currentUpdate is never cleared#1295bpalermo wants to merge 1 commit into
bpalermo wants to merge 1 commit into
Conversation
A Stack could be left with status.currentUpdate naming an Update that would
never resolve, after which it never started another update: Reconciling=True /
Ready=False forever, with the resources it manages silently unreconciled. Two
shapes, same effect.
Shape A -- the Update was garbage-collected. The controller logged "update
object not found; will retry" and returned with no requeue and without clearing
the reference, so nothing self-healed. The reconciliation-request annotation
could not rescue it either: the check runs before any sync logic, and
observedReconcileRequest is persisted by that same branch, so the request was
marked observed with nothing done.
- Absence is now confirmed against the API server via an uncached reader; a
cold or lagging informer is no longer mistaken for a deleted object, and an
Update that exists but is Terminating is left to complete normally.
- If the Stack's own update is still running it is adopted rather than
abandoned, so we never start a second pulumi operation on one workspace.
- Otherwise the reference is cleared, an event is emitted, and an error is
returned so the retry is rate-limited. That bounds the case where updates
are deleted systemically, which would otherwise be an unbounded
create/delete loop.
status.lastUpdate is deliberately left untouched: the lost update's outcome is
unknown -- most often it never ran -- so a synthetic failure would misreport
history and spend the Stack's retry budget on an operator-side fault.
Shape B -- the Update was present and terminal, but its completion was never
processed, leaving lastUpdate describing the previous (often successful) update
so the Stack looked healthy while being stuck. The cause is a generation gate:
isUpdateComplete required observedGeneration to equal generation, but nothing
writes an Update's status once it is Complete (the update controller
short-circuits), while the apiserver bumps generation when a deletion is blocked
by a finalizer. A completed Update that is then deleted therefore sits at
generation N+1 / observedGeneration N permanently, looking forever unfinished.
Worse, updateCompletePredicate uses the same helper, so operator restarts did
not even enqueue the Stack. The gate is dropped -- an Update's spec is never
modified after creation, so a generation bump cannot mean the result is stale.
Two of the windows that produce a dangling reference needed no crash at all: the
recovery paths after a failed Create and a failed finalizer patch both discarded
their status-write error, leaving currentUpdate naming an Update that was
deleted or never created. Those errors are now returned.
Also sweep Updates left Terminating while still holding our finalizer, which can
never finish deleting. This uses a read-modify-write rather than the happy path's
server-side apply, because that apply can only drop a finalizer owned by its own
field manager -- so one written by an older operator version survived silently,
leaving an object removable only by hand-patching a controller-owned field.
Finally, requeue on a slow timer while an update is in flight, so an Update that
never reaches a terminal state cannot strand the Stack.
Fixes pulumi#1292
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
|
PR is now waiting for a maintainer to run the acceptance tests. This PR will only perform build and linting. |
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.
Proposed changes
A
Stackcould be left withstatus.currentUpdatenaming anUpdatethat would never resolve, after which it never started another update —Reconciling=True/Ready=Falseindefinitely, with the resources it manages silently unreconciled. #1292 reports 77 wedged Stacks across two clusters, the oldest at 174h, each managing live DNS records.Two shapes, same effect and same manual recovery.
Shape A — the Update was garbage-collected
stack_controller.gologgedupdate object not found; will retryand returned with no requeue and without clearing the reference, so nothing self-healed. Thepulumi.com/reconciliation-requestannotation could not rescue it either: the check runs before any sync logic, andobservedReconcileRequestis persisted by that same branch — so the request was marked observed with nothing done.readCurrentUpdategoes through the cache andIsNotFoundthere only means "not in the informer". A cold or lagging cache is no longer mistaken for a deleted object.Terminatingis left alone: its deletion bumpsgeneration, which wakes the update controller, which drives it to a terminal state this controller can absorb normally.pulumioperation against one workspace.StackUpdateLostevent is emitted, and an error is returned so the retry goes through the workqueue's exponential backoff. That backoff is load-bearing: returningnilmakes controller-runtimeForget()the item and reset the rate limiter, so an Update being deleted systemically (its Workspace is gone, and every replacement is cascade-deleted in turn) would become an unbounded create/delete loop.status.lastUpdateis deliberately left untouched. The lost update's real outcome is unknown — most often it never ran — so recording a synthetic failure would both misreport history and spend the Stack's retry budget (maxUpdateFailures) on an operator-side fault. Visibility comes from the event and condition message instead, which is the "nothing distinguishes this from a healthy Stack" complaint in the issue.Shape B — the Update was present and terminal, but never processed
Here
lastUpdatestill described the previous update — often a successful one — so the Stack looked healthy while being completely stuck.The cause is a generation gate.
isUpdateCompleterequiredstatus.observedGeneration == metadata.generation, but:Complete— the update controller short-circuits on that condition and returns without callingupdateStatus; whilegenerationwhen a deletion is blocked by a finalizer (registry/store.go, generation is incremented on the transition into deleting).So an Update that completes and is then deleted — by its TTL, or by a workspace cascade — sits at generation N+1 / observedGeneration N permanently, looking forever unfinished. The issue hypothesised "a missed edge on the completion watch"; it is slightly worse than that, because
updateCompletePredicateuses the same helper, soCreatereturned false for such an Update and operator restarts did not even enqueue the Stack. The edge was unreachable, not merely missed.The gate is dropped. This is safe because an Update's spec is never modified after creation, so a generation bump cannot mean the recorded result is stale.
Two windows that needed no crash at all
The recovery paths after a failed
Createand a failedaddUpdateFinalizerboth did_ = saveStatus(). A conflict on either leftstatus.currentUpdatenaming an Update that was deleted or never created — no crash required, which likely makes these the highest-probability cause in a busy fleet. Those errors are now returned.Releasing Updates stuck in
TerminatingAlso sweeps Updates that are being deleted while still holding
finalizer.stack.pulumi.com, which can otherwise never finish deleting. The sweep uses a read-modify-write rather than the happy path's server-side apply, because that apply can only drop a finalizer owned by its own field manager — so a finalizer written by an older operator version survives silently, leaving an object removable only by hand-patching a controller-owned field. (This is what the unusedisFinalizerOwnedByLegacyManagerhelper was anticipating.) It skips the update currently being waited on, and skips non-terminal ones, which the update controller marksCanceledfirst.This part addresses the deletion deadlock described in #1293 rather than #1292, but it lives in the same function as the changes above; splitting it out would mean two PRs editing
saveStatus/Reconcileconcurrently. Its changelog entry cites #1293 accordingly.Also
Requeue on a slow timer while an update is in flight, so an Update that never reaches a terminal state cannot strand the Stack. Kept coarse (5m) because each pass re-resolves the source, which for a git source is a network round trip per in-flight Stack.
Testing
New envtest specs in
stack_controller_test.go:Terminatingwith a legacy-owned finalizer → released; and the update still being waited on → untouchedlastUpdate.Failuresunchanged across a healI confirmed the shape-B and reaper specs fail against the pre-fix code rather than passing vacuously.
make test(operator + agent) andgolangci-lintare clean, with no codegen drift.make test-e2ehas not been run, and neither failure mode has been reproduced end-to-end against a live cluster.Related issues
Fixes #1292. The finalizer sweep also addresses the deletion deadlock in #1293.
🤖 Generated with Claude Code