fix(dataprotection): defer restore source resolution

Author: weicao

pkg/controller/plan/restore.go

@@ -182,17 +182,16 @@ func (r *RestoreManager) BuildPrepareDataRestore(comp *component.SynthesizedComp
 	sourceTargetName, sourceTarget := backupSourceTargetForRestore(backupObj)
 	if r.SourceTargetName != "" {
 		sourceTargetName = r.SourceTargetName
-		sourceTarget = backupSourceTargetByName(backupObj, r.SourceTargetName)
-		if sourceTarget == nil {
-			return nil, intctrlutil.NewFatalError(fmt.Sprintf(`source target "%s" not found in backup "%s"`, r.SourceTargetName, backupObj.Name))
-		}
+		// Source-target validation and resolution belong to the DataProtection
+		// Restore path. This layer only carries the public Restore intent.
+		sourceTarget = nil
 	}
 	restore := &dpv1alpha1.Restore{
 		ObjectMeta: r.GetRestoreObjectMeta(comp, dpv1alpha1.PrepareData, templateName),
 		Spec: dpv1alpha1.RestoreSpec{
 			Backup: dpv1alpha1.BackupRef{
 				Name:             backupObj.Name,
-				Namespace:        r.namespace,
+				Namespace:        backupObj.Namespace,
 				SourceTargetName: sourceTargetName,
 			},
 			RestoreTime: r.RestoreTime,
@@ -254,7 +253,7 @@ func (r *RestoreManager) DoPostReady(comp *component.SynthesizedComponent,
 		Spec: dpv1alpha1.RestoreSpec{
 			Backup: dpv1alpha1.BackupRef{
 				Name:             backupObj.Name,
-				Namespace:        r.namespace,
+				Namespace:        backupObj.Namespace,
 				SourceTargetName: sourceTargetName,
 			},
 			RestoreTime: r.RestoreTime,
@@ -306,24 +305,6 @@ func backupSourceTargetForRestore(backupObj *dpv1alpha1.Backup) (string, *dpv1al
 	return "", nil
 }
 
-func backupSourceTargetByName(backupObj *dpv1alpha1.Backup, sourceTargetName string) *dpv1alpha1.BackupStatusTarget {
-	if backupObj == nil || sourceTargetName == "" {
-		return nil
-	}
-	if backupObj.Status.Target != nil {
-		if backupObj.Status.Target.Name == sourceTargetName {
-			return backupObj.Status.Target
-		}
-		return nil
-	}
-	for i := range backupObj.Status.Targets {
-		if backupObj.Status.Targets[i].Name == sourceTargetName {
-			return &backupObj.Status.Targets[i]
-		}
-	}
-	return nil
-}
-
 func (r *RestoreManager) buildRequiredPolicy(sourceTarget *dpv1alpha1.BackupStatusTarget) *dpv1alpha1.RequiredPolicyForAllPodSelection {
 	var requiredPolicy *dpv1alpha1.RequiredPolicyForAllPodSelection
 	if sourceTarget != nil && sourceTarget.PodSelector.Strategy == dpv1alpha1.PodSelectionStrategyAll {

pkg/controller/plan/restore_test.go

@@ -31,7 +31,6 @@ import (
 	dpv1alpha1 "github.com/apecloud/kubeblocks/apis/dataprotection/v1alpha1"
 	"github.com/apecloud/kubeblocks/pkg/constant"
 	"github.com/apecloud/kubeblocks/pkg/controller/component"
-	intctrlutil "github.com/apecloud/kubeblocks/pkg/controllerutil"
 )
 
 func newRestoreManagerForTest() *RestoreManager {
@@ -235,7 +234,7 @@ func TestRestoreManagerBuildPrepareDataRestore(t *testing.T) {
 		SchedulingPolicy: &appsv1.SchedulingPolicy{NodeName: "node-a"},
 	}
 	backup := &dpv1alpha1.Backup{
-		ObjectMeta: metav1.ObjectMeta{Name: "backup"},
+		ObjectMeta: metav1.ObjectMeta{Name: "backup", Namespace: "backup-source"},
 		Status: dpv1alpha1.BackupStatus{
 			Targets: []dpv1alpha1.BackupStatusTarget{{
 				BackupTarget: dpv1alpha1.BackupTarget{
@@ -263,7 +262,7 @@ func TestRestoreManagerBuildPrepareDataRestore(t *testing.T) {
 		return
 	}
 	if restore.Spec.Backup.Name != "backup" ||
-		restore.Spec.Backup.Namespace != "default" ||
+		restore.Spec.Backup.Namespace != "backup-source" ||
 		restore.Spec.Backup.SourceTargetName != "target-a" {
 		t.Fatalf("unexpected backup ref: %#v", restore.Spec.Backup)
 	}
@@ -338,7 +337,7 @@ func TestRestoreManagerBuildPrepareDataRestoreWithExplicitSourceTarget(t *testin
 	}
 }
 
-func TestRestoreManagerBuildPrepareDataRestoreRejectsUnknownSourceTarget(t *testing.T) {
+func TestRestoreManagerBuildPrepareDataRestorePassesUnknownSourceTargetToDataProtection(t *testing.T) {
 	manager := newRestoreManagerForTest()
 	manager.SourceTargetName = "missing-target"
 	comp := &component.SynthesizedComponent{
@@ -366,12 +365,15 @@ func TestRestoreManagerBuildPrepareDataRestoreRejectsUnknownSourceTarget(t *test
 		},
 	}
 
-	_, err := manager.BuildPrepareDataRestore(comp, backup, nil)
-	if err == nil {
-		t.Fatal("expected unknown source target error")
+	restore, err := manager.BuildPrepareDataRestore(comp, backup, nil)
+	if err != nil {
+		t.Fatalf("BuildPrepareDataRestore() error = %v", err)
+	}
+	if restore == nil {
+		t.Fatal("restore is nil")
 	}
-	if !intctrlutil.IsTargetError(err, intctrlutil.ErrorTypeFatal) {
-		t.Fatalf("expected fatal error, got %v", err)
+	if restore.Spec.Backup.SourceTargetName != "missing-target" {
+		t.Fatalf("source target = %q, want missing-target", restore.Spec.Backup.SourceTargetName)
 	}
 }
 

pkg/operations/horizontal_scaling.go

@@ -224,9 +224,6 @@ func (hs horizontalScalingOpsHandler) createRestore(reqCtx intctrlutil.RequestCt
 		}
 		return nil
 	}
-	if len(backupObj.Status.Targets) > 1 && restoreMGR.SourceTargetName == "" {
-		return intctrlutil.NewFatalError(fmt.Sprintf("scale-out from backup %s/%s requires sourceTargetName because it has multiple source targets", backupObj.Namespace, backupObj.Name))
-	}
 	// create restore
 	restore, err := restoreMGR.BuildPrepareDataRestore(synthesizedComponent, backupObj, getTemplate(templateName))
 	if err != nil {

pkg/operations/horizontal_scaling_test.go

@@ -268,9 +268,14 @@ var _ = Describe("HorizontalScaling OpsRequest", func() {
 		})
 
 		It("test to scale out replicas from a full backup", func() {
+			backupNamespace := "backup-source-" + randomStr
+			Expect(k8sClient.Create(ctx, &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: backupNamespace}})).Should(Succeed())
+			DeferCleanup(func() {
+				_ = k8sClient.Delete(ctx, &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: backupNamespace}})
+			})
 			By("create Backup")
 			backupName := "backup-for-ops-" + randomStr
-			backup := testdp.NewBackupFactory(testCtx.DefaultNamespace, backupName).
+			backup := testdp.NewBackupFactory(backupNamespace, backupName).
 				SetBackupPolicyName(testdp.BackupPolicyName).
 				SetBackupMethod(testdp.VSBackupMethodName).
 				Create(&testCtx).GetObject()
@@ -299,6 +304,7 @@ var _ = Describe("HorizontalScaling OpsRequest", func() {
 			horizontalScaling := opsv1alpha1.HorizontalScaling{ScaleOut: &opsv1alpha1.ScaleOut{
 				FromBackup: &opsv1alpha1.FromBackup{
 					Name:             backupName,
+					Namespace:        backupNamespace,
 					SourceTargetName: "target-b",
 				},
 			}}
@@ -312,6 +318,7 @@ var _ = Describe("HorizontalScaling OpsRequest", func() {
 			}, client.InNamespace(opsRes.OpsRequest.Namespace))).Should(Succeed())
 			Expect(restoreList.Items).Should(HaveLen(2))
 			for i := range restoreList.Items {
+				Expect(restoreList.Items[i].Spec.Backup.Namespace).Should(Equal(backupNamespace))
 				Expect(restoreList.Items[i].Spec.Backup.SourceTargetName).Should(Equal("target-b"))
 			}
 
@@ -347,35 +354,6 @@ var _ = Describe("HorizontalScaling OpsRequest", func() {
 			checkOpsRequestPhaseIsSucceed(reqCtx, opsRes)
 		})
 
-		It("fails scale-out from a multi-target backup without source target name", func() {
-			backup := &dpv1alpha1.Backup{
-				ObjectMeta: metav1.ObjectMeta{
-					Namespace: testCtx.DefaultNamespace,
-					Name:      "multi-target-backup-" + randomStr,
-				},
-				Status: dpv1alpha1.BackupStatus{
-					Targets: []dpv1alpha1.BackupStatusTarget{
-						{BackupTarget: dpv1alpha1.BackupTarget{Name: "target-a"}},
-						{BackupTarget: dpv1alpha1.BackupTarget{Name: "target-b"}},
-					},
-				},
-			}
-			restoreMGR := plan.NewRestoreManager(ctx, k8sClient, &appsv1.Cluster{
-				ObjectMeta: metav1.ObjectMeta{Name: clusterName, Namespace: testCtx.DefaultNamespace},
-			}, model.GetScheme(), nil, 1, 0)
-			err := horizontalScalingOpsHandler{}.createRestore(
-				intctrlutil.RequestCtx{Ctx: ctx},
-				k8sClient,
-				&OpsResource{OpsRequest: &opsv1alpha1.OpsRequest{}},
-				nil,
-				restoreMGR,
-				&appsv1.ClusterComponentSpec{},
-				backup,
-				"")
-			Expect(err).Should(HaveOccurred())
-			Expect(err.Error()).Should(ContainSubstring("requires sourceTargetName"))
-		})
-
 		It("test to scale in replicas with `scaleIn`", func() {
 			By("scale in replicas from 3 to 1")
 			horizontalScaling := opsv1alpha1.HorizontalScaling{ScaleIn: &opsv1alpha1.ScaleIn{}}