fix(reconcile): refactor pvc reconcillation, add conflict retries#139
Merged
GrigoryPervakov merged 1 commit intomainfrom Mar 20, 2026
Merged
fix(reconcile): refactor pvc reconcillation, add conflict retries#139GrigoryPervakov merged 1 commit intomainfrom
GrigoryPervakov merged 1 commit intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors replica reconciliation to decouple PVC updates from StatefulSet (STS) spec revisions so that PVC update failures no longer silently block STS updates, and adds conflict retry handling to make updates more resilient under frequent reconciles.
Changes:
- Split STS vs PVC revision tracking: ignore
VolumeClaimTemplatesin STS revision hashing and introduce a separate PVC revision/hash check. - Update reconciliation flow so PVC update failures no longer prevent STS updates, while still preventing progression to the next replica.
- Add
RetryOnConflictretries to resource updates and expand envtest coverage to better model PVC immutability behavior.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/controllerutil/annotations.go | Adds a helper to consistently set the spec-hash annotation. |
| internal/controller/testutil/suite.go | Wraps the envtest client with an update interceptor to simulate PVC immutability; enhances STS readiness simulation by creating pods/PVCs. |
| internal/controller/resources.go | Adds conflict retries to updates; introduces PVC lookup by STS; refactors replica reconciliation to update PVCs without blocking STS updates and to track PVC revision separately. |
| internal/controller/keeper/templates.go | Excludes VolumeClaimTemplates from the STS revision hash computation. |
| internal/controller/keeper/templates_test.go | Adds a test ensuring STS revision does not change when the data disk spec changes. |
| internal/controller/keeper/sync.go | Tracks per-replica PVC state and a separate pvcRevision; includes PVC in diff detection and reconciliation input. |
| internal/controller/keeper/sync_test.go | Updates expectations around spec-hash annotations and replica state setup for restart-on-config-change behavior. |
| internal/controller/clickhouse/templates.go | Excludes VolumeClaimTemplates from the STS revision hash computation. |
| internal/controller/clickhouse/templates_test.go | Adds a test ensuring STS revision does not change when the data disk spec changes. |
| internal/controller/clickhouse/sync.go | Tracks per-replica PVC state and a separate pvcRevision; includes PVC in diff detection and reconciliation input. |
| internal/controller/clickhouse/controller_test.go | Adds an integration-style test verifying STS updates proceed even when PVC updates fail, and that reconciliation doesn’t move to the next replica on PVC failure. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
3548619 to
048d725
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.
Why
PVC changes should not trigger pod restart and block other replica resources from updating, but they should block updates to the next replica.
Frequent reconciles may face Conflict errors that can be retried.
What
Decouple STS and PVC revision.
Allow STS update if PVC update failed.
Add retries on conflict.
Related Issues
Related to #133, #131