fix(backup): only delete backups' resources when synchronization

Only delete backups' resources (BackupVolume, BackupBackingImage,
SystemBackup, and Backup) when synchronization.

Add the label `DeleteCustomResourceOnly`, and delete remote backup
data when deleting the corresponding backup resource in the cluster
and the label does not exist.

ref: longhorn/longhorn 9530

Signed-off-by: James Lu <james.lu@suse.com>
(cherry picked from commit 8fadbfb)

# Conflicts:
#	controller/utils.go

controller/backup_backing_image_controller.go

@@ -214,7 +214,11 @@ func (bc *BackupBackingImageController) reconcile(backupBackingImageName string)
 
 	// Examine DeletionTimestamp to determine if object is under deletion
 	if !bbi.DeletionTimestamp.IsZero() {
-		if backupTarget.Spec.BackupTargetURL != "" {
+		needsCleanupRemoteData, err := checkIfRemoteDataCleanupIsNeeded(bbi, backupTarget)
+		if err != nil {
+			return errors.Wrap(err, "failed to check if it needs to delete remote backup backing image data")
+		}
+		if needsCleanupRemoteData {
 			backupTargetClient, err := newBackupTargetClientFromDefaultEngineImage(bc.ds, backupTarget)
 			if err != nil {
 				log.WithError(err).Warn("Failed to init backup target clients")

controller/backup_controller.go

@@ -270,8 +270,12 @@ func (bc *BackupController) reconcile(backupName string) (err error) {
 			return err
 		}
 
-		if backupTarget.Spec.BackupTargetURL != "" &&
-			backupVolume != nil && backupVolume.DeletionTimestamp == nil {
+		needsCleanupRemoteData, err := checkIfRemoteDataCleanupIsNeeded(backup, backupTarget)
+		if err != nil {
+			return errors.Wrap(err, "failed to check if it needs to delete remote backup data")
+		}
+
+		if needsCleanupRemoteData && backupVolume != nil && backupVolume.DeletionTimestamp == nil {
 			backupTargetClient, err := newBackupTargetClientFromDefaultEngineImage(bc.ds, backupTarget)
 			if err != nil {
 				log.WithError(err).Warn("Failed to init backup target clients")

controller/backup_target_controller.go

@@ -499,9 +499,12 @@ func (btc *BackupTargetController) syncBackupVolume(backupStoreBackupVolumeNames
 		log.Infof("Found %d backup volumes in the backup target that do not exist in the cluster and need to be deleted from the cluster", count)
 	}
 	for backupVolumeName := range backupVolumesToDelete {
-		log.WithField("backupVolume", backupVolumeName).Info("Deleting backup volume from cluster")
+		log.WithField("backupVolume", backupVolumeName).Info("Deleting BackupVolume not exist in backupstore")
+		if err = datastore.AddBackupVolumeDeleteCustomResourceOnlyLabel(btc.ds, backupVolumeName); err != nil {
+			return errors.Wrapf(err, "failed to add label delete-custom-resource-only to Backupvolume %s", backupVolumeName)
+		}
 		if err = btc.ds.DeleteBackupVolume(backupVolumeName); err != nil {
-			return errors.Wrapf(err, "failed to delete backup volume %s from cluster", backupVolumeName)
+			return errors.Wrapf(err, "failed to delete BackupVolume %s not exist in backupstore", backupVolumeName)
 		}
 	}
 
@@ -554,9 +557,12 @@ func (btc *BackupTargetController) syncBackupBackingImage(backupStoreBackingImag
 		log.Infof("Found %d backup backing images in the cluster that do not exist in the backup target and need to be deleted", count)
 	}
 	for backupBackingImageName := range backupBackingImagesToDelete {
-		log.WithField("backupBackingImage", backupBackingImageName).Info("Deleting backup backing image from cluster")
+		log.WithField("backupBackingImage", backupBackingImageName).Info("Deleting BackupBackingImage not exist in backupstore")
+		if err = datastore.AddBackupBackingImageDeleteCustomResourceOnlyLabel(btc.ds, backupBackingImageName); err != nil {
+			return errors.Wrapf(err, "failed to add label delete-custom-resource-only to BackupBackingImage %s", backupBackingImageName)
+		}
 		if err = btc.ds.DeleteBackupBackingImage(backupBackingImageName); err != nil {
-			return errors.Wrapf(err, "failed to delete backup backing image %s from cluster", backupBackingImageName)
+			return errors.Wrapf(err, "failed to delete BackupBackingImage %s not exist in backupstore", backupBackingImageName)
 		}
 	}
 
@@ -610,6 +616,9 @@ func (btc *BackupTargetController) syncSystemBackup(backupStoreSystemBackups sys
 	delSystemBackupsInCluster := clusterReadySystemBackupNames.Difference(backupstoreSystemBackupNames)
 	for name := range delSystemBackupsInCluster {
 		log.WithField("systemBackup", name).Info("Deleting SystemBackup not exist in backupstore")
+		if err = datastore.AddSystemBackupDeleteCustomResourceOnlyLabel(btc.ds, name); err != nil {
+			return errors.Wrapf(err, "failed to add label delete-custom-resource-only to SystemBackup %v", name)
+		}
 		if err = btc.ds.DeleteSystemBackup(name); err != nil {
 			return errors.Wrapf(err, "failed to delete SystemBackup %v not exist in backupstore", name)
 		}
@@ -657,6 +666,10 @@ func (btc *BackupTargetController) cleanupBackupVolumes() error {
 
 	var errs []string
 	for backupVolumeName := range clusterBackupVolumes {
+		if err = datastore.AddBackupVolumeDeleteCustomResourceOnlyLabel(btc.ds, backupVolumeName); err != nil {
+			errs = append(errs, err.Error())
+			continue
+		}
 		if err = btc.ds.DeleteBackupVolume(backupVolumeName); err != nil && !apierrors.IsNotFound(err) {
 			errs = append(errs, err.Error())
 			continue
@@ -677,6 +690,10 @@ func (btc *BackupTargetController) cleanupBackupBackingImages() error {
 
 	var errs []string
 	for backupBackingImageName := range clusterBackupBackingImages {
+		if err = datastore.AddBackupBackingImageDeleteCustomResourceOnlyLabel(btc.ds, backupBackingImageName); err != nil {
+			errs = append(errs, err.Error())
+			continue
+		}
 		if err = btc.ds.DeleteBackupBackingImage(backupBackingImageName); err != nil && !apierrors.IsNotFound(err) {
 			errs = append(errs, err.Error())
 			continue
@@ -697,6 +714,10 @@ func (btc *BackupTargetController) cleanupSystemBackups() error {
 
 	var errs []string
 	for systemBackup := range systemBackups {
+		if err = datastore.AddSystemBackupDeleteCustomResourceOnlyLabel(btc.ds, systemBackup); err != nil {
+			errs = append(errs, err.Error())
+			continue
+		}
 		if err = btc.ds.DeleteSystemBackup(systemBackup); err != nil && !apierrors.IsNotFound(err) {
 			errs = append(errs, err.Error())
 			continue

controller/backup_volume_controller.go

@@ -223,13 +223,16 @@ func (bvc *BackupVolumeController) reconcile(backupVolumeName string) (err error
 
 	// Examine DeletionTimestamp to determine if object is under deletion
 	if !backupVolume.DeletionTimestamp.IsZero() {
-
 		if err := bvc.ds.DeleteAllBackupsForBackupVolume(backupVolumeName); err != nil {
 			return errors.Wrap(err, "failed to delete backups")
 		}
 
+		needsCleanupRemoteData, err := checkIfRemoteDataCleanupIsNeeded(backupVolume, backupTarget)
+		if err != nil {
+			return errors.Wrap(err, "failed to check if it needs to delete remote backup volume data")
+		}
 		// Delete the backup volume from the remote backup target
-		if backupTarget.Spec.BackupTargetURL != "" {
+		if needsCleanupRemoteData {
 			engineClientProxy, backupTargetClient, err := getBackupTarget(bvc.controllerID, backupTarget, bvc.ds, log, bvc.proxyConnCounter)
 			if err != nil || engineClientProxy == nil {
 				log.WithError(err).Error("Failed to init backup target clients")
@@ -354,6 +357,9 @@ func (bvc *BackupVolumeController) reconcile(backupVolumeName string) (err error
 		log.Infof("Found %d backups in the backup target that do not exist in the cluster and need to be deleted from the cluster", count)
 	}
 	for backupName := range backupsToDelete {
+		if err = datastore.AddBackupDeleteCustomResourceOnlyLabel(bvc.ds, backupName); err != nil {
+			return errors.Wrapf(err, "failed to add label delete-custom-resource-only to backup %s", backupName)
+		}
 		if err = bvc.ds.DeleteBackup(backupName); err != nil {
 			return errors.Wrapf(err, "failed to delete backup %s from cluster", backupName)
 		}

controller/system_backup_controller.go

@@ -276,7 +276,7 @@ func (c *SystemBackupController) syncSystemBackup(key string) (err error) {
 		return err
 	}
 
-	return c.reconcile(name, backupTargetClient)
+	return c.reconcile(name, backupTargetClient, backupTarget)
 }
 
 func getLoggerForSystemBackup(logger logrus.FieldLogger, systemBackup *longhorn.SystemBackup) logrus.FieldLogger {
@@ -319,7 +319,7 @@ func (c *SystemBackupController) updateSystemBackupRecord(record *systemBackupRe
 	record.message = message
 }
 
-func (c *SystemBackupController) reconcile(name string, backupTargetClient engineapi.SystemBackupOperationInterface) (err error) {
+func (c *SystemBackupController) reconcile(name string, backupTargetClient engineapi.SystemBackupOperationInterface, backupTarget *longhorn.BackupTarget) (err error) {
 	systemBackup, err := c.ds.GetSystemBackup(name)
 	if err != nil {
 		if !apierrors.IsNotFound(err) {
@@ -446,7 +446,7 @@ func (c *SystemBackupController) reconcile(name string, backupTargetClient engin
 		cleanupLocalSystemBackupFiles(tempBackupArchivePath, tempBackupDir, log)
 
 	case longhorn.SystemBackupStateDeleting:
-		cleanupRemoteSystemBackupFiles(systemBackup, backupTargetClient, log)
+		cleanupRemoteSystemBackupFiles(systemBackup, backupTargetClient, backupTarget, log)
 
 		cleanupLocalSystemBackupFiles(tempBackupArchivePath, tempBackupDir, log)
 
@@ -581,11 +581,17 @@ func (c *SystemBackupController) UploadSystemBackup(systemBackup *longhorn.Syste
 	}
 }
 
-func cleanupRemoteSystemBackupFiles(systemBackup *longhorn.SystemBackup, backupTargetClient engineapi.SystemBackupOperationInterface, log logrus.FieldLogger) {
+func cleanupRemoteSystemBackupFiles(systemBackup *longhorn.SystemBackup, backupTargetClient engineapi.SystemBackupOperationInterface, backupTarget *longhorn.BackupTarget, log logrus.FieldLogger) {
 	if systemBackup.Status.Version == "" {
 		// The backup store sync might not have finished
 		return
 	}
+	if needsCleanupRemoteData, err := checkIfRemoteDataCleanupIsNeeded(systemBackup, backupTarget); err != nil {
+		log.WithError(err).Warn("failed to check if it needs to delete remote system backup data")
+		return
+	} else if !needsCleanupRemoteData {
+		return
+	}
 
 	systemBackupsFromBackupTarget, err := backupTargetClient.ListSystemBackup()
 	if err != nil {

controller/system_backup_controller_test.go

@@ -264,7 +264,7 @@ func (s *TestSuite) TestReconcileSystemBackup(c *C) {
 			systemBackupController.UploadSystemBackup(systemBackup, archievePath, tempDir, backupTargetClient)
 
 		default:
-			err = systemBackupController.reconcile(tc.systemBackupName, backupTargetClient)
+			err = systemBackupController.reconcile(tc.systemBackupName, backupTargetClient, nil)
 			if tc.expectError {
 				c.Assert(err, NotNil)
 			} else {

controller/utils.go

@@ -1,10 +1,15 @@
 package controller
 
 import (
-	"github.com/longhorn/longhorn-manager/types"
 	"github.com/sirupsen/logrus"
+
+	"k8s.io/apimachinery/pkg/runtime"
+
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 
+	"github.com/longhorn/longhorn-manager/datastore"
+	"github.com/longhorn/longhorn-manager/types"
+
 	longhorn "github.com/longhorn/longhorn-manager/k8s/pkg/apis/longhorn/v1beta2"
 )
 
@@ -76,3 +81,25 @@ func setReplicaFailedAt(r *longhorn.Replica, timestamp string) {
 		r.Spec.LastFailedAt = timestamp
 	}
 }
+<<<<<<< HEAD
+=======
+
+func isRegularRWXVolume(v *longhorn.Volume) bool {
+	if v == nil {
+		return false
+	}
+	return v.Spec.AccessMode == longhorn.AccessModeReadWriteMany && !v.Spec.Migratable
+}
+
+func checkIfRemoteDataCleanupIsNeeded(obj runtime.Object, bt *longhorn.BackupTarget) (bool, error) {
+	if obj == nil || bt == nil {
+		return false, nil
+	}
+	exists, err := datastore.IsLabelLonghornDeleteCustomResourceOnlyExisting(obj)
+	if err != nil {
+		return false, err
+	}
+
+	return !exists && bt.Spec.BackupTargetURL != "", nil
+}
+>>>>>>> 8fadbfbe (fix(backup): only delete backups' resources when synchronization)

datastore/longhorn.go

@@ -2927,6 +2927,121 @@ func labelBackupVolume(backupVolumeName string, obj runtime.Object) error {
 	return nil
 }
 
+// labelLonghornDeleteCustomResourceOnly labels the object with the label `longhorn.io/delete-custom-resource-only: true`
+func labelLonghornDeleteCustomResourceOnly(obj k8sruntime.Object) error {
+	metadata, err := meta.Accessor(obj)
+	if err != nil {
+		return err
+	}
+
+	labels := metadata.GetLabels()
+	if labels == nil {
+		labels = map[string]string{}
+	}
+	labels[types.GetLonghornLabelKey(types.DeleteCustomResourceOnly)] = "true"
+	metadata.SetLabels(labels)
+	return nil
+}
+
+// IsLabelLonghornDeleteCustomResourceOnlyExisting check if the object label `longhorn.io/delete-custom-resource-only` exists
+func IsLabelLonghornDeleteCustomResourceOnlyExisting(obj k8sruntime.Object) (bool, error) {
+	metadata, err := meta.Accessor(obj)
+	if err != nil {
+		return false, err
+	}
+
+	labels := metadata.GetLabels()
+	if labels == nil {
+		return false, nil
+	}
+	_, ok := labels[types.GetLonghornLabelKey(types.DeleteCustomResourceOnly)]
+	return ok, nil
+}
+
+// AddBackupVolumeDeleteCustomResourceOnlyLabel adds the label `longhorn.io/delete-custom-resource-only: true` to the BackupVolume
+func AddBackupVolumeDeleteCustomResourceOnlyLabel(ds *DataStore, backupVolumeName string) error {
+	bv, err := ds.GetBackupVolume(backupVolumeName)
+	if err != nil {
+		return err
+	}
+	if exists, err := IsLabelLonghornDeleteCustomResourceOnlyExisting(bv); err != nil {
+		return err
+	} else if exists {
+		return nil
+	}
+	if err := labelLonghornDeleteCustomResourceOnly(bv); err != nil {
+		return err
+	}
+	if _, err = ds.UpdateBackupVolume(bv); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+// AddBackupDeleteCustomResourceOnlyLabel adds the label `longhorn.io/delete-custom-resource-only: true` to the Backup
+func AddBackupDeleteCustomResourceOnlyLabel(ds *DataStore, backupName string) error {
+	backup, err := ds.GetBackup(backupName)
+	if err != nil {
+		return err
+	}
+	if exists, err := IsLabelLonghornDeleteCustomResourceOnlyExisting(backup); err != nil {
+		return err
+	} else if exists {
+		return nil
+	}
+	if err := labelLonghornDeleteCustomResourceOnly(backup); err != nil {
+		return err
+	}
+	if _, err = ds.UpdateBackup(backup); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+// AddBackupBackingImageDeleteCustomResourceOnlyLabel adds the label `longhorn.io/delete-custom-resource-only: true` to the BackupBackingImage
+func AddBackupBackingImageDeleteCustomResourceOnlyLabel(ds *DataStore, backupBackingImageName string) error {
+	bbi, err := ds.GetBackupBackingImage(backupBackingImageName)
+	if err != nil {
+		return err
+	}
+	if exists, err := IsLabelLonghornDeleteCustomResourceOnlyExisting(bbi); err != nil {
+		return err
+	} else if exists {
+		return nil
+	}
+	if err := labelLonghornDeleteCustomResourceOnly(bbi); err != nil {
+		return err
+	}
+	if _, err = ds.UpdateBackupBackingImage(bbi); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+// AddSystemBackupDeleteCustomResourceOnlyLabel adds the label `longhorn.io/delete-custom-resource-only: true` to the SystemBackup
+func AddSystemBackupDeleteCustomResourceOnlyLabel(ds *DataStore, systemBackupName string) error {
+	sb, err := ds.GetSystemBackup(systemBackupName)
+	if err != nil {
+		return err
+	}
+	if exists, err := IsLabelLonghornDeleteCustomResourceOnlyExisting(sb); err != nil {
+		return err
+	} else if exists {
+		return nil
+	}
+	if err := labelLonghornDeleteCustomResourceOnly(sb); err != nil {
+		return err
+	}
+	if _, err = ds.UpdateSystemBackup(sb); err != nil {
+		return err
+	}
+
+	return nil
+}
+
 func FixupRecurringJob(v *longhorn.Volume) error {
 	if err := labelRecurringJobDefault(v); err != nil {
 		return err

types/types.go

@@ -128,6 +128,8 @@ const (
 	ConfigMapResourceVersionKey = "configmap-resource-version"
 	UpdateSettingFromLonghorn   = "update-setting-from-longhorn"
 
+	DeleteCustomResourceOnly = "delete-custom-resource-only"
+
 	KubernetesStatusLabel = "KubernetesStatus"
 	KubernetesReplicaSet  = "ReplicaSet"
 	KubernetesStatefulSet = "StatefulSet"

webhook/resources/backupvolume/mutator.go

@@ -45,9 +45,40 @@ func (b *backupVolumeMutator) Create(request *admission.Request, newObj runtime.
 }
 
 func (b *backupVolumeMutator) Update(request *admission.Request, oldObj runtime.Object, newObj runtime.Object) (admission.PatchOps, error) {
+	backupVolume, ok := newObj.(*longhorn.BackupVolume)
+	if !ok {
+		return nil, werror.NewInvalidError(fmt.Sprintf("%v is not a *longhorn.BackupVolume", newObj), "")
+	}
+
+	deleteCustomResourceOnlyLabelExists, err := datastore.IsLabelLonghornDeleteCustomResourceOnlyExisting(backupVolume)
+	if err != nil {
+		err := errors.Wrap(err, "failed to check if the label longhorn.io/delete-custom-resource-only exists")
+		return nil, werror.NewInvalidError(err.Error(), "")
+	}
+	if deleteCustomResourceOnlyLabelExists {
+		if err := b.addBackupsDeleteCustomResourceLabel(backupVolume); err != nil {
+			err := errors.Wrapf(err, "failed to add the label longhorn.io/delete-custom-resource-only to backups of the backup volume %v", backupVolume.Name)
+			return nil, werror.NewInvalidError(err.Error(), "")
+		}
+	}
+
 	return mutate(newObj)
 }
 
+func (b *backupVolumeMutator) addBackupsDeleteCustomResourceLabel(bv *longhorn.BackupVolume) error {
+	backups, err := b.ds.ListBackupsWithBackupVolumeName(bv.Name)
+	if err != nil {
+		return errors.Wrap(err, "failed to list backups of the backup volume")
+	}
+	for _, backup := range backups {
+		if err = datastore.AddBackupDeleteCustomResourceOnlyLabel(b.ds, backup.Name); err != nil {
+			return errors.Wrapf(err, "failed to add the label longhorn.io/delete-custom-resource-only to backup %s", backup.Name)
+		}
+	}
+
+	return nil
+}
+
 // mutate contains functionality shared by Create and Update.
 func mutate(newObj runtime.Object) (admission.PatchOps, error) {
 	backupVolume, ok := newObj.(*longhorn.BackupVolume)