Skip to content

feat(ops): select backup source target for scale-out restore - #10594

Merged
leon-ape merged 11 commits into
mainfrom
bugfix/10593-scaleout-source-target
Aug 31, 2026
Merged

feat(ops): select backup source target for scale-out restore#10594
leon-ape merged 11 commits into
mainfrom
bugfix/10593-scaleout-source-target

Conversation

@weicao

@weicao weicao commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Fixes #10595.

Problem

scaleOut.fromBackup can restore new replicas from a Backup, but the Ops API did not expose a source-target selector even though Restore.spec.backup.sourceTargetName already supports one.

For a Backup with multiple source targets, horizontal scale-out had no way to choose the target whose data should be restored.

What changed

  • Add scaleOut.fromBackup.sourceTargetName to the Ops API, generated CRDs, and API documentation.
  • Propagate the selector into Restore.spec.backup.sourceTargetName for the PrepareData/PVC scale-out restore path.
  • Preserve the OneToOne data-restore policy required when scale-out explicitly selects a source target.
  • Keep existing DataProtection target lookup semantics and the postReady restore path unchanged.
  • Synchronize the branch with the latest main.

Scope

DataProtection already supports selecting a Backup target through Restore.spec.backup.sourceTargetName. Annotation-based restore is one existing producer of that field.

This PR does not add or change DataProtection target-selection capability. It only exposes the existing selector on OpsRequest.scaleOut.fromBackup and passes the OpsRequest value to the generated PrepareData Restore.

The two correctness/safety fixes originally developed here were split into:

Validation

  • scripts/codex-go-test.sh ./pkg/controller/plan -count=1
  • scripts/codex-go-test.sh ./pkg/operations -count=1
  • scripts/codex-go-test.sh ./controllers/dataprotection -count=1
  • git diff --check

All three affected packages pass on the exact branch tree after synchronizing with main. This includes the VolumePopulator case that failed on the previous head.

Backport analysis

DataProtection in release-1.1 already supports Restore.spec.backup.sourceTargetName. The annotation-based restore flow also already populates this field from kubeblocks.io/backup-source-target, including automatic target allocation for sharded restores.

The 1.1 gap is specifically in Operations:

  • OpsRequest.scaleOut.fromBackup has no sourceTargetName field;
  • horizontal scale-out cannot explicitly select or override a target for the referenced Backup;
  • it can only reuse a target already present on the Component annotation.

Recommendation: backport only if explicit per-OpsRequest source-target selection is required in 1.1. That requires an independent 1.1 PR because the direct cherry-pick conflicts with the release-specific annotation-based plan implementation. No DataProtection capability backport is needed.

As a feature extension, release-1.0 was intentionally not evaluated.

@weicao
weicao requested review from a team, leon-ape and wangyelei as code owners July 9, 2026 08:56
@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

@codecov

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 65.76%. Comparing base (6d7e10c) to head (742b549).

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10594      +/-   ##
==========================================
+ Coverage   65.61%   65.76%   +0.15%     
==========================================
  Files         510      510              
  Lines       64400    64405       +5     
==========================================
+ Hits        42254    42357     +103     
+ Misses      18388    18314      -74     
+ Partials     3758     3734      -24     
Flag Coverage Δ
unittests 65.76% <100.00%> (+0.15%) ⬆️

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

Copy link
Copy Markdown
Collaborator

This change has a data-safety blocker and also duplicates the DataProtection owner's resolution contract.

[P0] The generated Restore points at the wrong namespace for cross-namespace Backups. FromBackup.Namespace is used to read the source Backup, but BuildPrepareDataRestore sets Restore.spec.backup.namespace to the Cluster/RestoreManager namespace instead of backupObj.Namespace (pkg/controller/plan/restore.go:190). The Restore controller strictly follows that reference. A missing same-name Backup causes failure; an existing same-name Backup can cause the wrong data to be restored. Please cover this with a cross-namespace end-to-end test.

[P1] Source-target resolution belongs to the DataProtection Restore path, not Ops/plan. This PR adds backupSourceTargetByName and terminal target-existence checks in BuildPrepareDataRestore (pkg/controller/plan/restore.go:182,309), while DataProtection already has the target resolver and can resolve an omitted selector from Restore/PVC context. It also creates inconsistent semantics for legacy singular status.target: the existing DP resolver returns the singular target regardless of selector, while this new resolver requires its name to match. The safe boundary is to pass sourceTargetName through Restore.spec.backup and let the DP owner validate and resolve it; do not create a second resolver in the Ops plan path.

@weicao
weicao marked this pull request as draft July 14, 2026 07:30
@weicao

weicao commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the P0/P1 blockers in exact head 6ba5a873399d68fafb3a94d1aa6bac567fe5b3e6.

  • Data safety: prepareData and postReady Restore objects now preserve the actual source Backup.Namespace; they are still created in the target Cluster namespace. The focused Ops regression now uses a real second namespace and asserts every generated Restore keeps that source namespace.
  • Ownership boundary: RestoreManager no longer resolves or terminally validates an explicit sourceTargetName. It only carries the public selector into Restore.spec.backup.sourceTargetName; DataProtection remains the validator/resolver. The added test proves an unknown selector is passed through rather than becoming an Ops/plan FatalError.
  • The Ops-side multi-target/no-selector fatal check was also removed, so omitted-selector semantics remain with DataProtection instead of a second resolver.

RED before implementation reproduced both failures: the generated Restore used the Cluster namespace, and the plan rejected the explicit selector before DataProtection could observe it.

GREEN on the exact head:

  • focused plan tests;
  • cross-namespace focused Ops envtest;
  • focused race;
  • full pkg/controller/plan + pkg/operations tests;
  • go vet ./pkg/controller/plan ./pkg/operations;
  • git diff --check.

The PR remains Draft until exact-head CI and review verification close.

@weicao

weicao commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

/nopick

@apecloud-bot apecloud-bot added the nopick Not auto cherry-pick when PR merged label Jul 15, 2026
@weicao
weicao marked this pull request as ready for review July 16, 2026 17:31
Comment thread pkg/controller/plan/restore.go
Comment thread pkg/controller/plan/restore.go Outdated
Comment thread pkg/controller/plan/restore.go Outdated
Comment thread pkg/controller/plan/restore.go Outdated
@weicao
weicao force-pushed the bugfix/10593-scaleout-source-target branch from 6ba5a87 to 1cd4d06 Compare July 20, 2026 10:52
@weicao
weicao requested a review from ldming as a code owner July 20, 2026 10:52
@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 Jul 20, 2026
Comment thread pkg/operations/horizontal_scaling.go
Comment thread pkg/controller/plan/restore.go Outdated
Use restore template ordinals when selecting source pods for snapshot and prepare-data paths. Propagate explicit backup targets without resolving them in Ops so DataProtection remains the validation owner.
Comment thread pkg/controller/plan/restore.go Outdated
weicao and others added 2 commits July 23, 2026 11:45
…-source-target

# Conflicts:
#	cmd/dataprotection/main.go
#	cmd/manager/main.go
#	pkg/dataprotection/restore/manager.go
#	pkg/operations/horizontal_scaling_test.go
@github-actions github-actions Bot added size/L Denotes a PR that changes 100-499 lines. and removed size/XL Denotes a PR that changes 500-999 lines. labels Aug 31, 2026
@leon-ape leon-ape changed the title feat: support source target for scale-out restore feat: select backup source target for scale-out restore Aug 31, 2026
@leon-ape

Copy link
Copy Markdown
Collaborator

/approve

@apecloud-bot apecloud-bot added the approved PR Approved Test label Aug 31, 2026
@leon-ape leon-ape changed the title feat: select backup source target for scale-out restore feat(ops): select backup source target for scale-out restore Aug 31, 2026
@leon-ape
leon-ape merged commit 27618c3 into main Aug 31, 2026
80 of 87 checks passed
@leon-ape
leon-ape deleted the bugfix/10593-scaleout-source-target branch August 31, 2026 08:24
@github-actions github-actions Bot added this to the Release 1.2.0 milestone Aug 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved PR Approved Test 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.

scaleOut.fromBackup needs source target selector for multi-target backups

3 participants