Skip to content

fix(dataprotection): terminate cluster restores through resource owners - #10777

Open
leon-ape wants to merge 48 commits into
mainfrom
bugfix/10755-cluster-restore-finalizer
Open

fix(dataprotection): terminate cluster restores through resource owners#10777
leon-ape wants to merge 48 commits into
mainfrom
bugfix/10755-cluster-restore-finalizer

Conversation

@leon-ape

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

Copy link
Copy Markdown
Collaborator

Problem

Cluster or Component deletion is the restore cancellation signal. The target PVC deletion timestamp is not. Cancellation must preserve controller ownership: each controller deletes only the objects it created and removes only the finalizer it added.

Changes

Cluster restore protection

  • add a restore-specific Cluster finalizer owned only by ClusterRestoreReconciler
  • add it while Cluster restore is active or PVC-scoped DP resources remain
  • retain protection for Restore=False while the live Cluster still carries initial-restore intent
  • on Cluster deletion, observe cached Restore/PVC state and wait for resource owners to finish cleanup
  • treat every internal Restore, including Completed and Failed objects, as pending cleanup while Cluster deletion is in progress
  • never delete Restore/helper PVC objects or patch target PVC finalizers from ClusterRestoreReconciler

VolumePopulator termination

  • treat Cluster deletion as Cluster-wide cancellation and Component deletion as Component-scoped cancellation
  • ignore target PVC deletion as a business signal
  • delete VP-owned execution and postReady Restore objects, then wait for RestoreReconciler
  • delete the VP-owned helper PVC and wait for disappearance
  • finally remove only the VP-owned target PVC DP finalizer
  • continue teardown for retained PVCs using identity recorded before restore resources are created
  • verify exact UID/owner identity before every destructive operation

Restore creation ordering

  • wait for the Cluster restore-protection finalizer before entering the PVC restore state machine
  • before creating a postReady Restore, use the controller cache to confirm that the Cluster is not deleting and still carries restore protection
  • stop postReady creation when the target Component is deleting
  • use watches and normal requeue for dependency progression; no direct API reader is used as a correctness mechanism

Identity

  • propagate Cluster UID through Cluster-owned VCT restore intent and VP-owned DP labels
  • validate the live PVC → Instance/InstanceSet → Component → Cluster owner chain before restore starts
  • record the verified Cluster and Component UID on a new target PVC before VP creates helper/Restore resources
  • do not migrate or adopt resources from older controller versions in this deletion-focused PR

Ownership invariants

  • ClusterRestoreReconciler only patches its Cluster restore-protection finalizer
  • VolumePopulator never adds/removes the Cluster finalizer and never deletes target PVC/Component/Cluster
  • RestoreReconciler remains the sole owner of Restore Job/Pod cleanup and Restore finalizers
  • App/InstanceSet remains the sole deletion path for target PVCs

Dependencies

Tests

  • full controllers/dataprotection package
  • Cluster restore-intent UID injection/cleanup tests
  • cmd/dataprotection compile test
  • ownership, ordered termination, Retain owner detachment, failed/terminal Restore protection, target deletion neutrality, postReady creation guards, and UID mismatch coverage

Fixes #10755

@leon-ape leon-ape added the nopick Not auto cherry-pick when PR merged label Aug 13, 2026
@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 13, 2026
@codecov

codecov Bot commented Aug 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 75.79737% with 129 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.85%. Comparing base (63373ca) to head (9614838).

Files with missing lines Patch % Lines
...llers/dataprotection/volumepopulator_controller.go 73.01% 77 Missing and 35 partials ⚠️
...llers/dataprotection/cluster_restore_controller.go 85.21% 11 Missing and 6 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10777      +/-   ##
==========================================
+ Coverage   65.76%   65.85%   +0.09%     
==========================================
  Files         510      511       +1     
  Lines       64958    65459     +501     
==========================================
+ Hits        42721    43111     +390     
- Misses      18449    18523      +74     
- Partials     3788     3825      +37     
Flag Coverage Δ
unittests 65.85% <75.79%> (+0.09%) ⬆️

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.

@leon-ape leon-ape changed the title fix: protect Cluster lifecycle during restore execution fix(dataprotection): protect Cluster lifecycle during restore execution Aug 13, 2026
@leon-ape
leon-ape marked this pull request as ready for review August 13, 2026 10:01
@leon-ape
leon-ape requested review from a team, ldming and wangyelei as code owners August 13, 2026 10:01
@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 14, 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 14, 2026
@leon-ape leon-ape changed the title fix(dataprotection): protect Cluster lifecycle during restore execution fix(dataprotection): terminate Cluster restores through resource owners Aug 31, 2026
@leon-ape
leon-ape changed the base branch from main to bugfix/volume-populator-dependency-watches August 31, 2026 07:34
@leon-ape leon-ape changed the title fix(dataprotection): terminate Cluster restores through resource owners fix(dataprotection): terminate cluster restores through resource owners Aug 31, 2026
Base automatically changed from bugfix/volume-populator-dependency-watches to main August 31, 2026 08:25
…restore-finalizer

# Conflicts:
#	controllers/dataprotection/volumepopulator_controller.go
#	controllers/dataprotection/volumepopulator_controller_test.go
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/XXL Denotes a PR that changes 1000+ lines.

Projects

None yet

2 participants