fix(ops): guard scale-out restore without target volumes (#10475)

Author: weicao

pkg/controller/plan/restore.go

@@ -118,6 +118,10 @@ func (r *RestoreManager) DoPrepareData(comp *component.SynthesizedComponent,
 	return r.createRestoreAndWait(compObj, restores...)
 }
 
+// BuildPrepareDataRestore returns (nil, nil) when the backup method has no
+// target volumes matching the component's volume claim templates, i.e. there
+// is nothing to prepareData-restore for this component. Callers must handle
+// the nil Restore instead of using it.
 func (r *RestoreManager) BuildPrepareDataRestore(comp *component.SynthesizedComponent, backupObj *dpv1alpha1.Backup, template *appsv1.InstanceTemplate) (*dpv1alpha1.Restore, error) {
 	templateName := ""
 	startingIndex := r.startingIndex

pkg/operations/horizontal_scaling.go

@@ -233,6 +233,10 @@ func (hs horizontalScalingOpsHandler) createRestore(reqCtx intctrlutil.RequestCt
 	if err != nil {
 		return err
 	}
+	if restore == nil {
+		return intctrlutil.NewFatalError(fmt.Sprintf("scale-out from backup %s/%s is not supported because backup method %q has no target volumes matching component %q",
+			backupObj.Namespace, backupObj.Name, backupObj.Status.BackupMethod.Name, synthesizedComponent.Name))
+	}
 	scheme, _ := opsv1alpha1.SchemeBuilder.Build()
 	if err := intctrlutil.SetOwnership(opsRes.OpsRequest, restore, scheme, ""); err != nil {
 		return err

pkg/operations/horizontal_scaling_test.go

@@ -20,18 +20,23 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 package operations
 
 import (
+	"context"
 	"fmt"
+	"strings"
+	"testing"
 	"time"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/utils/pointer"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 
 	appsv1 "github.com/apecloud/kubeblocks/apis/apps/v1"
 	dpv1alpha1 "github.com/apecloud/kubeblocks/apis/dataprotection/v1alpha1"
@@ -998,3 +1003,101 @@ func mockConsensusCompToRunning(opsRes *OpsResource) {
 	compStatus.Phase = appsv1.RunningComponentPhase
 	opsRes.Cluster.Status.Components[defaultCompName] = compStatus
 }
+
+func TestHorizontalScalingCreateRestoreReturnsFatalWhenNoRestoreBuilt(t *testing.T) {
+	testCases := []struct {
+		name         string
+		backupMethod *dpv1alpha1.BackupMethod
+	}{{
+		// logical backups (e.g. TiDB BR) have no targetVolumes at all
+		name:         "backup method without target volumes",
+		backupMethod: &dpv1alpha1.BackupMethod{Name: "br"},
+	}, {
+		// targetVolumes exist but none match the component's volume claim templates
+		name: "backup method target volumes match no component volume",
+		backupMethod: &dpv1alpha1.BackupMethod{
+			Name: "br",
+			TargetVolumes: &dpv1alpha1.TargetVolumeInfo{
+				Volumes: []string{"other-data"},
+			},
+		},
+	}}
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			ctx := context.Background()
+			scheme := runtime.NewScheme()
+			for _, addToScheme := range []func(*runtime.Scheme) error{
+				corev1.AddToScheme,
+				appsv1.AddToScheme,
+				dpv1alpha1.AddToScheme,
+				opsv1alpha1.AddToScheme,
+			} {
+				if err := addToScheme(scheme); err != nil {
+					t.Fatalf("add scheme: %v", err)
+				}
+			}
+
+			cluster := &appsv1.Cluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "tidb",
+					Namespace: "default",
+					UID:       types.UID("cluster1"),
+				},
+			}
+			opsRequest := &opsv1alpha1.OpsRequest{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "scale-out-from-backup",
+					Namespace: "default",
+					UID:       types.UID("opsreq1"),
+				},
+			}
+			backup := &dpv1alpha1.Backup{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "br-full",
+					Namespace: "default",
+				},
+				Status: dpv1alpha1.BackupStatus{
+					Phase:        dpv1alpha1.BackupPhaseCompleted,
+					BackupMethod: tc.backupMethod,
+				},
+			}
+			cli := fake.NewClientBuilder().WithScheme(scheme).WithObjects(cluster, opsRequest, backup).Build()
+			opsRes := &OpsResource{
+				Cluster:    cluster,
+				OpsRequest: opsRequest,
+			}
+			synthesizedComponent := &component.SynthesizedComponent{
+				Name: "tikv",
+				VolumeClaimTemplates: []corev1.PersistentVolumeClaimTemplate{{
+					ObjectMeta: metav1.ObjectMeta{Name: "data"},
+					Spec: corev1.PersistentVolumeClaimSpec{
+						AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce},
+					},
+				}},
+			}
+			restoreMGR := plan.NewRestoreManager(ctx, cli, cluster, scheme, map[string]string{
+				constant.OpsRequestNameLabelKey: opsRequest.Name,
+			}, 1, 3)
+
+			err := horizontalScalingOpsHandler{}.createRestore(intctrlutil.RequestCtx{Ctx: ctx}, cli, opsRes,
+				synthesizedComponent, restoreMGR, &appsv1.ClusterComponentSpec{Name: "tikv"}, backup, "")
+			if err == nil {
+				t.Fatal("expected fatal error when backup method cannot build prepareData restore")
+			}
+			if !intctrlutil.IsTargetError(err, intctrlutil.ErrorTypeFatal) {
+				t.Fatalf("expected fatal error, got %T: %v", err, err)
+			}
+			if !strings.Contains(err.Error(), "has no target volumes matching component") {
+				t.Fatalf("unexpected error: %v", err)
+			}
+
+			restoreList := &dpv1alpha1.RestoreList{}
+			if err := cli.List(ctx, restoreList, client.InNamespace("default")); err != nil {
+				t.Fatalf("list restores: %v", err)
+			}
+			if len(restoreList.Items) != 0 {
+				t.Fatalf("expected no restore to be created, got %d", len(restoreList.Items))
+			}
+		})
+	}
+}