Repository: apecloud/kubeblocks

Issue:
[BUG] BackupRepo PreCheck Error for Aliyun OSS: BucketAlreadyExists
**To Reproduce**
1. Install KB 1.0.1-beta 6 (datasafed version 0.2.1)
2. Create BackupRepository 
```bash
kbcli backuprepo create kb-oss \
  --provider oss \
  --region <ERGION> \
  --bucket  <BUCKET_NAME> \
  --access-key-id <AK> \
  --secret-access-key <SK>  
```
3. BackupRepo PreCheck Job failed

<img width="1291" height="110" alt="Image" src="https://github.com/user-attachments/assets/c551628f-cfd8-447c-b144-ee98898e3a27" />



Discussion:
@shanshanying: To workaround at the moment we should use `--no-check-bucket` options
```bash
kbcli backuprepo create kb-oss \
  --provider oss \
  --region <ERGION> \
  --bucket  <BUCKET_NAME> \
  --access-key-id <AK> \
  --secret-access-key <SK>  \
  --no-check-bucket
```
@zjx20: This is the issue introduced by upgrading datasafed to version 0.2.1 in https://github.com/apecloud/kubeblocks/pull/9510, which upgraded the rclone dependency and caused some behavioral changes.

rclone always tries to create a bucket upon startup to ensure its existence. If the bucket already exists, s3 will report an error, but in previous versions, rclone would ignore this error and assume the bucket belongs to you. Later, rclone added a feature (https://github.com/rclone/rclone/commit/f56ea2bee2e7552e273ff679c6f0eb429a0969d3) that checks the s3 return value to determine if the bucket truly belongs to you, and reports an error if it does not.

However, different s3 providers return different error codes in this situation. Some return `AlreadyOwnedByYou`, while others return `BucketAlreadyExists`. Thus, rclone added an option `use_already_exists`. When this option is true, it assumes the provider supports the `AlreadyOwnedByYou` error code, so `BucketAlreadyExists` indicates the bucket belongs to someone else and should trigger an error. rclone sets `use_already_exists=true` by default for OSS, assuming that if the bucket exists and belongs to you, OSS will return 200 Ok, but this is not the actual case.

In https://github.com/apecloud/kubeblocks/pull/9555, we set `use_already_exists=false` for OSS to resolve this issue.


Solution patch:
diff --git a/controllers/dataprotection/backup_controller.go b/controllers/dataprotection/backup_controller.go
index 6c373fd50..0abc1bb02 100644
--- a/controllers/dataprotection/backup_controller.go
+++ b/controllers/dataprotection/backup_controller.go
@@ -609,8 +609,9 @@ func (r *BackupReconciler) handleRunningPhase(
 		// check all actions status, if any action failed, update backup status to failed
 		// if all actions completed, update backup status to completed, otherwise,
 		// continue to handle following actions.
-	actions:
 		for targetPodName, acts := range actions {
+			// the backup actions for selected pod
+		targetPodBackupActions:
 			for _, act := range acts {
 				status, err := act.Execute(actionCtx)
 				if err != nil {
@@ -624,10 +625,10 @@ func (r *BackupReconciler) handleRunningPhase(
 					continue
 				case dpv1alpha1.ActionPhaseFailed:
 					existFailedAction = true
-					break actions
+					break targetPodBackupActions
 				case dpv1alpha1.ActionPhaseRunning:
 					waiting = true
-					break actions
+					break targetPodBackupActions
 				}
 			}
 		}
diff --git a/deploy/helm/templates/storageprovider/oss.yaml b/deploy/helm/templates/storageprovider/oss.yaml
index 5881b803d..303ef2fee 100644
--- a/deploy/helm/templates/storageprovider/oss.yaml
+++ b/deploy/helm/templates/storageprovider/oss.yaml
@@ -48,6 +48,7 @@ spec:
     root = {{ `{{ index .Parameters "bucket" }}` }}
     no_check_certificate = {{ `{{ index .Parameters "insecure" }}` }}
     no_check_bucket = {{ `{{ index .Parameters "noCheckBucket" }}` }}
+    use_already_exists = false
     chunk_size = 50Mi
 
   parametersSchema:
diff --git a/pkg/controller/plan/restore.go b/pkg/controller/plan/restore.go
index 68d628f8d..e0982af41 100644
--- a/pkg/controller/plan/restore.go
+++ b/pkg/controller/plan/restore.go
@@ -133,7 +133,7 @@ func (r *RestoreManager) DoPrepareData(comp *component.SynthesizedComponent,
 	for _, v := range comp.Instances {
 		r.replicas = replicas(v)
 		templateReplicas += r.replicas
-		restore, err := r.BuildPrepareDataRestore(comp, backupObj, v.Name)
+		restore, err := r.BuildPrepareDataRestore(comp, backupObj, &v)
 		if err != nil {
 			return err
 		}
@@ -144,7 +144,7 @@ func (r *RestoreManager) DoPrepareData(comp *component.SynthesizedComponent,
 	compReplicas := comp.Replicas - templateReplicas
 	if compReplicas > 0 {
 		r.replicas = compReplicas
-		restore, err := r.BuildPrepareDataRestore(comp, backupObj, "")
+		restore, err := r.BuildPrepareDataRestore(comp, backupObj, nil)
 		if err != nil {
 			return err
 		}
@@ -155,7 +155,17 @@ func (r *RestoreManager) DoPrepareData(comp *component.SynthesizedComponent,
 	return r.createRestoreAndWait(compObj, restores...)
 }
 
-func (r *RestoreManager) BuildPrepareDataRestore(comp *component.SynthesizedComponent, backupObj *dpv1alpha1.Backup, templateName string) (*dpv1alpha1.Restore, error) {
+func (r *RestoreManager) BuildPrepareDataRestore(comp *component.SynthesizedComponent, backupObj *dpv1alpha1.Backup, template *appsv1.InstanceTemplate) (*dpv1alpha1.Restore, error) {
+	templateName := ""
+	startingIndex := r.startingIndex
+	if template != nil {
+		templateName = template.Name
+		if len(template.Ordinals.Ranges) > 0 {
+			// todo: currently restore api does not support multiple ranges, if implement in current way it
+			// need to use multiple restore objects
+			startingIndex = template.Ordinals.Ranges[0].Start
+		}
+	}
 	backupMethod := backupObj.Status.BackupMethod
 	if backupMethod == nil {
 		return nil, intctrlutil.NewErrorf(intctrlutil.ErrorTypeRestoreFailed, `status.backupMethod of backup "%s" can not be empty`, backupObj.Name)
@@ -217,7 +227,7 @@ func (r *RestoreManager) BuildPrepareDataRestore(comp *component.SynthesizedComp
 				VolumeClaimRestorePolicy:         r.volumeRestorePolicy,
 				RestoreVolumeClaimsTemplate: &dpv1alpha1.RestoreVolumeClaimsTemplate{
 					Replicas:      r.replicas,
-					StartingIndex: r.startingIndex,
+					StartingIndex: startingIndex,
 					Templates:     templates,
 				},
 			},



Test patch:
diff --git a/pkg/controller/plan/restore_test.go b/pkg/controller/plan/restore_test.go
index 29445b73f..8388b3d2f 100644
--- a/pkg/controller/plan/restore_test.go
+++ b/pkg/controller/plan/restore_test.go
@@ -249,6 +249,207 @@ var _ = Describe("Restore", func() {
 				restore.Status.Phase = dpv1alpha1.RestorePhaseCompleted
 			})()).ShouldNot(HaveOccurred())
 
+			By("clean up annotations after cluster running")
+			_ = restoreMGR.DoRestore(synthesizedComponent, compObj, true)
+			Eventually(testapps.CheckObj(&testCtx, client.ObjectKeyFromObject(cluster), func(g Gomega, tmpCluster *appsv1.Cluster) {
+				g.Expect(tmpCluster.Annotations[constant.RestoreFromBackupAnnotationKey]).Should(BeEmpty())
+			})).Should(Succeed())
+		})
+	})
+	Context("Cluster InstanceTemplate ranges Restore", func() {
+		const (
+			compDefName     = "test-compdef-1"
+			defaultCompName = "mysql"
+			topologyKey     = "testTopologyKey"
+			labelKey        = "testNodeLabelKey"
+			labelValue      = "testLabelValue"
+		)
+
+		var (
+			compDef                 *appsv1.ComponentDefinition
+			cluster                 *appsv1.Cluster
+			synthesizedComponent    *component.SynthesizedComponent
+			compObj                 *appsv1.Component
+			pvc                     *corev1.PersistentVolumeClaim
+			backup                  *dpv1alpha1.Backup
+			fullBackupActionSet     *dpv1alpha1.ActionSet
+			fullBackupActionSetName string
+			startIndex = int32(10)
+		)
+
+		BeforeEach(func() {
+			By("By creating backup policyTemplate ")
+			compDef = testapps.NewComponentDefinitionFactory(compDefName).
+				SetDefaultSpec().
+				Create(&testCtx).GetObject()
+
+			testdp.NewBackupPolicyTemplateFactory("backup-policy-template").
+				SetCompDefs(compDef.Name).
+				WithRandomName().
+				AddBackupMethod(testdp.BackupMethodName, false, fullBackupActionSetName).
+				SetBackupMethodVolumeMounts(testapps.DataVolumeName, "/data").Create(&testCtx).Get()
+
+			pvcSpec := testapps.NewPVCSpec("1Gi")
+			cluster = testapps.NewClusterFactory(testCtx.DefaultNamespace, clusterName, "").
+				AddMultipleTemplateComponentRange(defaultCompName, compDefName).
+				SetReplicas(3).
+				AddVolumeClaimTemplate(testapps.DataVolumeName, pvcSpec).
+				Create(&testCtx).GetObject()
+
+			By("By mocking a pvc")
+			pvc = testapps.NewPersistentVolumeClaimFactory(
+				testCtx.DefaultNamespace, "data-"+clusterName+"-"+defaultCompName+"-0", clusterName, defaultCompName, "data").
+				SetStorage("1Gi").
+				Create(&testCtx).GetObject()
+
+			By("By mocking a pod")
+			volume := corev1.Volume{Name: pvc.Name, VolumeSource: corev1.VolumeSource{
+				PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ClaimName: pvc.Name}}}
+			_ = testapps.NewPodFactory(testCtx.DefaultNamespace, clusterName+"-"+defaultCompName+"-0").
+				AddAppInstanceLabel(clusterName).
+				AddAppComponentLabel(defaultCompName).
+				AddAppManagedByLabel().
+				AddVolume(volume).
+				AddContainer(corev1.Container{Name: testapps.DefaultMySQLContainerName, Image: testapps.ApeCloudMySQLImage}).
+				AddNodeName("fake-node-name").
+				Create(&testCtx).GetObject()
+
+			By("create actionset of full backup")
+			fullBackupActionSet = testapps.CreateCustomizedObj(&testCtx, "backup/actionset.yaml", &dpv1alpha1.ActionSet{}, testapps.RandomizedObjName())
+			fullBackupActionSetName = fullBackupActionSet.Name
+			synthesizedComponent = &component.SynthesizedComponent{
+				PodSpec:              &compDef.Spec.Runtime,
+				VolumeClaimTemplates: intctrlutil.ToCoreV1PVCTs(cluster.Spec.ComponentSpecs[0].VolumeClaimTemplates),
+				Name:                 defaultCompName,
+				Replicas:             3,
+				Roles: []appsv1.ReplicaRole{
+					{
+						Name:           "leader",
+						UpdatePriority: 2,
+					},
+					{
+						Name:           "follower",
+						UpdatePriority: 1,
+					},
+				},
+				Instances: []appsv1.InstanceTemplate{{
+					Name:     "foo",
+					Replicas: func() *int32 { replicas := int32(1); return &replicas }(),
+					Ordinals: appsv1.Ordinals{
+						Ranges: []appsv1.Range{
+							{Start: startIndex, End: 20},
+						},
+					},
+				}},
+			}
+			By("create component object")
+			compObj = testapps.NewComponentFactory(testCtx.DefaultNamespace, cluster.Name+"-"+synthesizedComponent.Name, "").
+				AddAnnotations(constant.KBAppClusterUIDKey, string(cluster.UID)).
+				AddLabels(constant.AppInstanceLabelKey, cluster.Name).
+				SetReplicas(1).
+				Create(&testCtx).
+				GetObject()
+
+			By("By creating remote pvc: ")
+			remotePVC := testapps.NewPersistentVolumeClaimFactory(
+				testCtx.DefaultNamespace, "remote-pvc", clusterName, defaultCompName, "log").
+				SetStorage("1Gi").
+				Create(&testCtx).GetObject()
+
+			By("By creating base backup: ")
+			backupLabels := map[string]string{
+				constant.AppInstanceLabelKey:    sourceCluster,
+				constant.KBAppComponentLabelKey: defaultCompName,
+			}
+			backup = testdp.NewBackupFactory(testCtx.DefaultNamespace, backupName).
+				WithRandomName().SetLabels(backupLabels).
+				SetBackupPolicyName("test-fake").
+				SetBackupMethod(testdp.VSBackupMethodName).
+				Create(&testCtx).GetObject()
+			baseStartTime := &startTime
+			baseStopTime := &now
+			backup.Status = dpv1alpha1.BackupStatus{
+				Phase:                     dpv1alpha1.BackupPhaseCompleted,
+				StartTimestamp:            baseStartTime,
+				CompletionTimestamp:       baseStopTime,
+				PersistentVolumeClaimName: remotePVC.Name,
+			}
+			testdp.MockBackupStatusMethod(backup, testdp.VSBackupMethodName, testapps.DataVolumeName, testdp.ActionSetName)
+			patchBackupStatus(backup.Status, client.ObjectKeyFromObject(backup))
+		})
+
+		It("Test restore", func() {
+			By("restore from backup")
+			restoreFromBackup := fmt.Sprintf(`{"%s": {"name":"%s"}}`, defaultCompName, backup.Name)
+			Expect(testapps.ChangeObj(&testCtx, cluster, func(tmpCluster *appsv1.Cluster) {
+				tmpCluster.Annotations = map[string]string{
+					constant.RestoreFromBackupAnnotationKey: restoreFromBackup,
+				}
+			})).Should(Succeed())
+			Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(cluster), cluster)).Should(Succeed())
+			restoreMGR := NewRestoreManager(ctx, k8sClient, cluster, scheme.Scheme, nil, 3, 0)
+
+			By("restore from template")
+			err := restoreMGR.DoRestore(synthesizedComponent, compObj, true)
+			Expect(intctrlutil.IsTargetError(err, intctrlutil.ErrorTypeNeedWaiting)).Should(BeTrue())
+
+			By("verify template restore")
+			restore := &dpv1alpha1.Restore{}
+			restoreMeta := restoreMGR.GetRestoreObjectMeta(synthesizedComponent, dpv1alpha1.PrepareData, "foo")
+			namedspace := types.NamespacedName{Name: restoreMeta.Name, Namespace: restoreMeta.Namespace}
+			Expect(k8sClient.Get(ctx, namedspace, restore)).Should(Succeed())
+			Expect(restore.Spec.PrepareDataConfig.RestoreVolumeClaimsTemplate.StartingIndex).Should(Equal(startIndex))
+			Expect(restore.Spec.PrepareDataConfig.RestoreVolumeClaimsTemplate.Replicas).Should(Equal(int32(1)))
+
+			By("mock template restore of prepareData stage to Completed")
+			Expect(testapps.GetAndChangeObjStatus(&testCtx, namedspace, func(restore *dpv1alpha1.Restore) {
+				restore.Status.Phase = dpv1alpha1.RestorePhaseCompleted
+			})()).ShouldNot(HaveOccurred())
+
+			By("restore from default")
+			err = restoreMGR.DoRestore(synthesizedComponent, compObj, true)
+			Expect(intctrlutil.IsTargetError(err, intctrlutil.ErrorTypeNeedWaiting)).Should(BeTrue())
+
+			By("verify default restore")
+			defaultMeta := restoreMGR.GetRestoreObjectMeta(synthesizedComponent, dpv1alpha1.PrepareData, "")
+			defaultNs := types.NamespacedName{Name: defaultMeta.Name, Namespace: defaultMeta.Namespace}
+			defaultRestore := &dpv1alpha1.Restore{}
+			Expect(k8sClient.Get(ctx, defaultNs, defaultRestore)).Should(Succeed())
+			Expect(defaultRestore.Spec.PrepareDataConfig.RestoreVolumeClaimsTemplate.StartingIndex).Should(Equal(int32(0)))
+			Expect(defaultRestore.Spec.PrepareDataConfig.RestoreVolumeClaimsTemplate.Replicas).Should(Equal(int32(2)))
+
+			By("mock default restore of prepareData stage to Completed")
+			Expect(testapps.GetAndChangeObjStatus(&testCtx, defaultNs, func(restore *dpv1alpha1.Restore) {
+				restore.Status.Phase = dpv1alpha1.RestorePhaseCompleted
+			})()).ShouldNot(HaveOccurred())
+
+			By("mock component and cluster phase to Running")
+			Expect(testapps.ChangeObjStatus(&testCtx, cluster, func() {
+				cluster.Status.Phase = appsv1.RunningClusterPhase
+				cluster.Status.Components = map[string]appsv1.ClusterComponentStatus{
+					defaultCompName: {
+						Phase: appsv1.RunningComponentPhase,
+					},
+				}
+			})).Should(Succeed())
+			Expect(testapps.ChangeObjStatus(&testCtx, compObj, func() {
+				compObj.Status.Phase = appsv1.RunningComponentPhase
+			})).Should(Succeed())
+
+			By("wait for postReady restore created and mock it to Completed")
+			restoreMGR.Cluster = cluster
+			_ = restoreMGR.DoRestore(synthesizedComponent, compObj, true)
+
+			// check if restore CR of postReady stage is created.
+			restoreMeta = restoreMGR.GetRestoreObjectMeta(synthesizedComponent, dpv1alpha1.PostReady, "")
+			namedspace = types.NamespacedName{Name: restoreMeta.Name, Namespace: restoreMeta.Namespace}
+			Eventually(testapps.CheckObjExists(&testCtx, namedspace,
+				&dpv1alpha1.Restore{}, true)).Should(Succeed())
+			// set restore to Completed
+			Expect(testapps.GetAndChangeObjStatus(&testCtx, namedspace, func(restore *dpv1alpha1.Restore) {
+				restore.Status.Phase = dpv1alpha1.RestorePhaseCompleted
+			})()).ShouldNot(HaveOccurred())
+
 			By("clean up annotations after cluster running")
 			_ = restoreMGR.DoRestore(synthesizedComponent, compObj, true)
 			Eventually(testapps.CheckObj(&testCtx, client.ObjectKeyFromObject(cluster), func(g Gomega, tmpCluster *appsv1.Cluster) {
diff --git a/pkg/testutil/apps/cluster_factory.go b/pkg/testutil/apps/cluster_factory.go
index 90eb98ddf..3285dbccf 100644
--- a/pkg/testutil/apps/cluster_factory.go
+++ b/pkg/testutil/apps/cluster_factory.go
@@ -95,6 +95,24 @@ func (factory *MockClusterFactory) AddMultipleTemplateComponent(compName string,
 	return factory
 }
 
+func (factory *MockClusterFactory) AddMultipleTemplateComponentRange(compName string, compDefName string) *MockClusterFactory {
+	comp := appsv1.ClusterComponentSpec{
+		Name:         compName,
+		ComponentDef: compDefName,
+		Instances: []appsv1.InstanceTemplate{{
+			Name:     "foo",
+			Replicas: func() *int32 { replicas := int32(1); return &replicas }(),
+			Ordinals: appsv1.Ordinals{
+				Ranges: []appsv1.Range{
+					{Start: 10, End: 20},
+				},
+			},
+		}},
+	}
+	factory.Get().Spec.ComponentSpecs = append(factory.Get().Spec.ComponentSpecs, comp)
+	return factory
+}
+
 func (factory *MockClusterFactory) AddInstances(compName string, instance appsv1.InstanceTemplate) *MockClusterFactory {
 	for i, compSpec := range factory.Get().Spec.ComponentSpecs {
 		if compSpec.Name != compName {