Skip to content

fix(dataprotection): surface terminating namespace cleanup blocker#10693

Open
weicao wants to merge 3 commits into
mainfrom
bugfix/dp-terminating-ns-observability
Open

fix(dataprotection): surface terminating namespace cleanup blocker#10693
weicao wants to merge 3 commits into
mainfrom
bugfix/dp-terminating-ns-observability

Conversation

@weicao

@weicao weicao commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

When a Backup with deletionPolicy: Delete is deleted after its namespace has entered Terminating, DataProtection cannot create the worker resources needed to delete the backup files. The controller intentionally retains the finalizer and retries, but the Backup API has no durable indication of why deletion is blocked; the reason is visible only in controller logs.

Fixes #10694.

What is changed and how does it work?

  • return a dedicated internal error when the backup namespace is Terminating
  • persist that exact cleanup-order blocker in Backup.status.failureReason
  • avoid duplicate status patches for the same reason while continuing to return the original error so reconciliation keeps retrying
  • leave other Deleting/Unknown errors, finalizer behavior, worker-resource creation, and deletion semantics unchanged
  • do not emit a namespaced Event because Event creation is not reliable after namespace termination begins

Tests

  • go test ./controllers/dataprotection -run 'TestDeleteBackupFiles(RecordsTerminatingNamespaceBlockerWithoutStoppingRetry|DoesNotRecordOrdinaryDeletingError|ReturnsStatusPatchError)$' -count=3
  • KUBEBUILDER_ASSETS=... go test ./controllers/dataprotection -run TestAPIs -ginkgo.focus 'should not create worker resources when backup namespace is terminating' -count=1
  • KUBEBUILDER_ASSETS=... go test ./controllers/dataprotection -count=1
  • KUBEBUILDER_ASSETS=... go test ./pkg/dataprotection/backup -count=1
  • go vet ./controllers/dataprotection
  • make test-go-generate

@weicao
weicao requested review from a team, ldming and wangyelei as code owners July 17, 2026 20:23
@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 Jul 17, 2026
@weicao weicao added the nopick Not auto cherry-pick when PR merged label Jul 17, 2026
@codecov

codecov Bot commented Jul 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.47368% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.00%. Comparing base (116b639) to head (828b093).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
controllers/dataprotection/backup_controller.go 89.47% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10693      +/-   ##
==========================================
+ Coverage   63.98%   64.00%   +0.02%     
==========================================
  Files         520      520              
  Lines       62995    63012      +17     
==========================================
+ Hits        40305    40332      +27     
+ Misses      19101    19080      -21     
- Partials     3589     3600      +11     
Flag Coverage Δ
unittests 64.00% <89.47%> (+0.02%) ⬆️

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.

Comment thread controllers/dataprotection/backup_controller.go Outdated
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/L Denotes a PR that changes 100-499 lines.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Deleting Backup does not expose terminating namespace blocker in status

3 participants