Skip to content

fix: validate restore volume binding lifecycle - #10765

Closed
leon-ape wants to merge 7 commits into
mainfrom
bugfix/10755-restore-populator-lifecycle
Closed

fix: validate restore volume binding lifecycle#10765
leon-ape wants to merge 7 commits into
mainfrom
bugfix/10755-restore-populator-lifecycle

Conversation

@leon-ape

@leon-ape leon-ape commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

What changed

  • validate the target PVC/PV binding using the PV ClaimRef, helper PVC/PV identity, execution Restore phase, and dataprotection.kubeblocks.io/populate-from
  • keep the Restore condition Unknown and preserve helper resources while the execution Restore is pending or running
  • report Restore=False and preserve the restore artifacts when the execution Restore fails or a completed restore is bound to an unrelated PV
  • preserve dataSourceRef semantics: the normal workload Pod may drive WaitForFirstConsumer node selection, but the target PVC remains unbound (and the Pod Pending) until prepareData completes and the restored PV is handed to the target PVC
  • make Serial prepareData and component/cluster postReady gates consume the verified population result instead of spec.volumeName
  • treat the helper-PVC/new-target-PVC/old-PV informer sequence as transient and requeue with Restore=Unknown instead of recording a permanent failure
  • match execution Restores by their normal owner reference during target deletion, stop the Restore first, and refuse helper cleanup on an ownership mismatch
  • split target deletion cleanup from successful helper release and remove the duplicate broad cleanup call
  • include target PVC/PV, helper PVC/PV, and execution Restore details in wait and failure messages

Why

A target PVC can be bound before the VolumePopulator restore finishes. Treating any non-empty spec.volumeName as restore completion could delete the helper PVC and restore Job. Binding a target PVC before population also violates the dataSourceRef contract: Bound must mean that the volume is ready for the consuming Pod.

Cross-object informer updates can also expose the new target PVC before the updated PV ClaimRef/annotation. That observation is now retried while the helper PVC still proves that the rebind is in flight.

Impact

Invalid early bindings are surfaced through the Restore condition without deleting or replacing existing PVC/PV resources. A provisioner that binds an empty target volume while a custom dataSourceRef is still being populated is not treated as a supported restore path. InstanceSet, Component, and Cluster workload semantics are unchanged.

Validation

  • full dataprotection controller unit/envtest suite
  • full InstanceSet Ginkgo suite (71 specs)
  • dataprotection types and package regression suites
  • Component, Cluster, and workloads controller regression suites
  • focused tests for Running/Completed/Failed Restore phases, informer reordering, Serial/postReady gating, ordinary owner-reference deletion, and keeping the target PVC unbound while prepareData is Running
  • make manifests and make lint (0 issues)

Fixes #10755

@apecloud-bot

Copy link
Copy Markdown
Collaborator

Auto Cherry-pick Instructions

Usage:
  - /nopick: Not auto cherry-pick when PR merged.
  - /pick: release-x.x [release-x.x]: Auto cherry-pick to the specified branch when PR merged.

Example:
  - /nopick
  - /pick release-1.1

CLA Recheck Instructions

Usage:
  - /recheck-cla: Trigger a re-check of CLA status for this pull request.
Example:
  - /recheck-cla

@github-actions github-actions Bot added the size/L Denotes a PR that changes 100-499 lines. label Aug 12, 2026
@leon-ape leon-ape added the nopick Not auto cherry-pick when PR merged label Aug 12, 2026
@codecov

codecov Bot commented Aug 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 68.14621% with 122 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.79%. Comparing base (9ee2a73) to head (bed333e).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
...llers/dataprotection/volumepopulator_controller.go 69.32% 75 Missing and 21 partials ⚠️
...oller/instanceset/reconciler_instance_alignment.go 56.66% 18 Missing and 8 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10765      +/-   ##
==========================================
+ Coverage   64.56%   64.79%   +0.22%     
==========================================
  Files         521      506      -15     
  Lines       63343    63558     +215     
==========================================
+ Hits        40897    41181     +284     
+ Misses      18804    18723      -81     
- Partials     3642     3654      +12     
Flag Coverage Δ
unittests 64.79% <68.14%> (+0.22%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions github-actions Bot added size/XL Denotes a PR that changes 500-999 lines. and removed size/L Denotes a PR that changes 100-499 lines. labels Aug 12, 2026
@github-actions github-actions Bot added size/XXL Denotes a PR that changes 1000+ lines. and removed size/XL Denotes a PR that changes 500-999 lines. labels Aug 12, 2026
@github-actions github-actions Bot added size/XL Denotes a PR that changes 500-999 lines. and removed size/XXL Denotes a PR that changes 1000+ lines. labels Aug 13, 2026
@leon-ape

Copy link
Copy Markdown
Collaborator Author

Superseded by four independent draft PRs so each lifecycle hardening can be reviewed and merged separately:\n\n1. #10779 — wait for Kubernetes formal PVC binding\n2. #10777 — protect Cluster restore execution with a dedicated finalizer and ordered deletion\n3. #10776 — split successful helper release from deleting-PVC cleanup\n4. #10778 — add minimum restored-PV rebind validation\n\nAll four target main, reference #10755, and carry the nopick label.

@leon-ape leon-ape closed this Aug 13, 2026
@github-actions github-actions Bot added this to the Release 1.2.0 milestone Aug 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

nopick Not auto cherry-pick when PR merged size/XL Denotes a PR that changes 500-999 lines.

Projects

None yet

2 participants