Skip to content

Conversation

@zijiren233
Copy link
Contributor

@zijiren233 zijiren233 commented Dec 25, 2025

When a Backup is deleted using foreground cascading deletion, the garbage collector actively deletes all child resources with blockOwnerDeletion=true. This caused a race condition where:

  1. Backup Controller creates a deletion job (with blockOwnerDeletion=true)
  2. GC detects the new blocking child resource and deletes it
  3. Backup Controller sees the job is missing and creates a new one
  4. This creates an infinite loop of job creation/deletion

The fix checks if the dataprotection finalizer has already been removed before creating a new deletion job. If the finalizer is gone, the deletion has already succeeded and no new job should be created.

Note that the difference between foreground cascading deletion and background cascading deletion is that foreground cascading deletion adds the foregroundDeletion finalizer to the Backup

… foreground deletion

When a Backup is deleted using foreground cascading deletion, the garbage
collector actively deletes all child resources with blockOwnerDeletion=true.
This caused a race condition where:

1. Backup Controller creates a deletion job (with blockOwnerDeletion=true)
2. GC detects the new blocking child resource and deletes it
3. Backup Controller sees the job is missing and creates a new one
4. This creates an infinite loop of job creation/deletion

The fix checks if the dataprotection finalizer has already been removed before
creating a new deletion job. If the finalizer is gone, the deletion has already
succeeded and no new job should be created.
@zijiren233 zijiren233 requested review from a team, ldming and wangyelei as code owners December 25, 2025 03:15
@apecloud-bot
Copy link
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.0

@github-actions github-actions bot added the size/XS Denotes a PR that changes 0-9 lines. label Dec 25, 2025
@apecloud-bot apecloud-bot added the pre-approve Fork PR Pre Approve Test label Dec 25, 2025
@wangyelei wangyelei added nopick Not auto cherry-pick when PR merged pick-1.0 Auto cherry-pick to release-1.0 when PR merged and removed nopick Not auto cherry-pick when PR merged labels Dec 25, 2025
@codecov
Copy link

codecov bot commented Dec 25, 2025

Codecov Report

❌ Patch coverage is 50.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 44.19%. Comparing base (1cd2f9c) to head (1248dd6).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
controllers/dataprotection/backup_controller.go 50.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9958      +/-   ##
==========================================
- Coverage   51.06%   44.19%   -6.88%     
==========================================
  Files         541      732     +191     
  Lines       58340    68153    +9813     
==========================================
+ Hits        29793    30119     +326     
- Misses      25611    35084    +9473     
- Partials     2936     2950      +14     
Flag Coverage Δ
unittests 44.19% <50.00%> (-6.88%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 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.

@wallyxjh
Copy link

Is it possible to include this PR in the KubeBlocks 0.9.3 release?

@wangyelei wangyelei added the pick-0.9 Auto cherry-pick to release-0.9 when PR merged label Dec 25, 2025
@wangyelei
Copy link
Contributor

@zijiren233 pls accept the license ca

@zijiren233
Copy link
Contributor Author

@wangyelei Hi! Thank you for the quick approval; I have signed the license CA.

@wangyelei wangyelei merged commit 53e5262 into apecloud:main Dec 25, 2025
79 of 82 checks passed
@github-actions github-actions bot added this to the Release 1.1.0 milestone Dec 25, 2025
@apecloud-bot
Copy link
Collaborator

/cherry-pick release-0.9

@apecloud-bot
Copy link
Collaborator

/cherry-pick release-1.0

@apecloud-bot
Copy link
Collaborator

🤖 says: cherry pick action finished successfully 🎉!
See: https://github.com/apecloud/kubeblocks/actions/runs/20501954006

apecloud-bot pushed a commit that referenced this pull request Dec 25, 2025
@apecloud-bot
Copy link
Collaborator

🤖 says: cherry pick action finished successfully 🎉!
See: https://github.com/apecloud/kubeblocks/actions/runs/20501953738

apecloud-bot pushed a commit that referenced this pull request Dec 25, 2025
@JashBook
Copy link
Collaborator

Is it possible to include this PR in the KubeBlocks 0.9.3 release?

We will not be overwriting already released versions, this PR will be released in the next version v0.9.6-beta.9.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pick-0.9 Auto cherry-pick to release-0.9 when PR merged pick-1.0 Auto cherry-pick to release-1.0 when PR merged pre-approve Fork PR Pre Approve Test size/XS Denotes a PR that changes 0-9 lines.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants