fix: clear resourceVersion before apply-patching SpiceDBCluster#440
Open
miparnisari wants to merge 4 commits into
Open
fix: clear resourceVersion before apply-patching SpiceDBCluster#440miparnisari wants to merge 4 commits into
miparnisari wants to merge 4 commits into
Conversation
The self-pause flow patches status (bumping the object's resourceVersion) and then patches metadata to add the controller-paused label using the same in-memory object. Since the marshaled apply patch embeds the stale resourceVersion, the API server rejects the label patch with a conflict every time, so self-pause never sticks: the next reconcile sees the Paused condition without the label, removes it, re-detects the failed migration, re-adds it, and loops forever (~25 status writes/sec). Clear resourceVersion before applying, matching how managedFields is already cleared, so the force apply is not subject to optimistic locking.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Regression test for the self-pause conflict loop: fails against the previous Patch implementation, which embedded the informer-cached resourceVersion in the apply payload.
A failed migration job should park the cluster: the Paused condition on the status and the authzed.com/controller-paused label on metadata. Before the resourceVersion fix, the label patch was rejected with a conflict every time, so the label never landed and the operator hot- looped adding/removing the Paused condition. This spec fails on main and passes with the fix.
The pod-error spec ends right after a rollout starts, then deletes the SpiceDBCluster asynchronously; DeferCleanup dropped the logical database while spicedb pods still held connections, failing with SQLSTATE 55006. Wait for dependents to be GCd before dropping, like the other specs do.
miparnisari
force-pushed
the
fix/self-pause-label-conflict
branch
2 times, most recently
from
July 17, 2026 02:01
c4cedbf to
e1da1a2
Compare
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.
Fix a self-pause hot loop. When a migration job fails, the operator pauses the cluster in two steps: a status patch adding the
Pausedcondition, then a metadata patch adding theauthzed.com/controller-pausedlabel — both from the same in-memory object. The status patch bumps the object's resourceVersion, so the label patch always carried a stale resourceVersion, which server-side apply treats as an optimistic-lock precondition and rejects with a 409 — every time. The label never landed, so each reconcile interpreted the labellessPausedcondition as a manual unpause and started over: a hot loop of ~360 conflicts/minute with the status flickering every second. Fixed by clearingresourceVersionbefore apply-patching, the same waymanagedFieldsis already cleared. Covered by a unit test on the patch payload and an e2e test that fails onmainand passes with the fix.Fix a flaky e2e teardown. The postgres pod-error spec deleted the SpiceDBCluster asynchronously and then immediately dropped the logical database, while spicedb pods still held connections — failing cleanup with
ERROR: database is being accessed by other users (SQLSTATE 55006). The spec now waits for dependent resources to be GC'd before the drop, like the other specs do.