fix(backup): renaming value from backup volume label

And correct the way to get the BackupVolume CR. ref: longhorn/longhorn 5411 Signed-off-by: James Lu <[email protected]>
longhorn · Dec 18, 2024 · 0fe44bc · 0fe44bc
1 parent be9c91b
commit 0fe44bc
Show file tree

Hide file tree

Showing 8 changed files with 44 additions and 24 deletions.
diff --git a/api/backup.go b/api/backup.go
@@ -243,7 +243,7 @@ func (s *Server) SyncBackupVolume(w http.ResponseWriter, req *http.Request) erro
 		return err
 	}
 
-	backupVolumeName := mux.Vars(req)["name"]
+	backupVolumeName := mux.Vars(req)["backupVolumeName"]
 	if backupVolumeName == "" {
 		return fmt.Errorf("backup volume name is required")
 	}
@@ -286,7 +286,7 @@ func (s *Server) BackupList(w http.ResponseWriter, req *http.Request) error {
 func (s *Server) BackupListByBackupVolume(w http.ResponseWriter, req *http.Request) error {
 	apiContext := api.GetApiContext(req)
 
-	backupVolumeName := mux.Vars(req)["name"]
+	backupVolumeName := mux.Vars(req)["backupVolumeName"]
 
 	bs, err := s.m.ListBackupsForBackupVolumeSorted(backupVolumeName)
 	if err != nil {
@@ -315,7 +315,7 @@ func (s *Server) BackupGet(w http.ResponseWriter, req *http.Request) error {
 	if input.Name == "" {
 		return errors.New("empty backup name is not allowed")
 	}
-	backupVolumeName := mux.Vars(req)["name"]
+	backupVolumeName := mux.Vars(req)["backupVolumeName"]
 
 	backup, err := s.m.GetBackup(input.Name, backupVolumeName)
 	if err != nil {
@@ -342,7 +342,7 @@ func (s *Server) BackupDelete(w http.ResponseWriter, req *http.Request) error {
 		return errors.New("empty backup name is not allowed")
 	}
 
-	backupVolumeName := mux.Vars(req)["name"]
+	backupVolumeName := mux.Vars(req)["backupVolumeName"]
 
 	backup, err := s.m.GetBackup(input.Name, backupVolumeName)
 	if err != nil {

diff --git a/api/model.go b/api/model.go
@@ -1987,9 +1987,9 @@ func toBackupResource(b *longhorn.Backup) *Backup {
 	// This field is empty probably because the backup state is not Ready
 	// or the content of the backup config is empty.
 	if ret.VolumeName == "" {
-		backupVolumeName, ok := b.Labels[types.LonghornLabelBackupVolume]
+		volumeName, ok := b.Labels[types.LonghornLabelBackupVolume]
 		if ok {
-			ret.VolumeName = backupVolumeName
+			ret.VolumeName = volumeName
 		}
 	}
 	return ret

diff --git a/api/router.go b/api/router.go
@@ -143,7 +143,7 @@ func NewRouter(s *Server) *mux.Router {
 		"backupVolumeSync": s.fwd.Handler(s.fwd.HandleProxyRequestByNodeID, s.fwd.GetHTTPAddressByNodeID(NodeHasDefaultEngineImage(s.m)), s.SyncBackupVolume),
 	}
 	for name, action := range backupActions {
-		r.Methods("POST").Path("/v1/backupvolumes/{name}").Queries("action", name).Handler(f(schemas, action))
+		r.Methods("POST").Path("/v1/backupvolumes/{backupVolumeName}").Queries("action", name).Handler(f(schemas, action))
 	}
 
 	r.Methods("GET").Path("/v1/nodes").Handler(f(schemas, s.NodeList))

diff --git a/controller/backup_controller.go b/controller/backup_controller.go
@@ -652,11 +652,11 @@ func (bc *BackupController) getBackupTargetName(backup *longhorn.Backup) (string
 }
 
 func (bc *BackupController) getBackupVolumeName(backup *longhorn.Backup) (string, error) {
-	backupVolumeName, ok := backup.Labels[types.LonghornLabelBackupVolume]
+	volumeName, ok := backup.Labels[types.LonghornLabelBackupVolume]
 	if !ok {
 		return "", fmt.Errorf("cannot find the backup volume label")
 	}
-	return backupVolumeName, nil
+	return volumeName, nil
 }
 
 func (bc *BackupController) getEngineBinaryClient(volumeName string) (*engineapi.EngineBinary, error) {

diff --git a/controller/uninstall_controller.go b/controller/uninstall_controller.go
@@ -770,7 +770,7 @@ func (c *UninstallController) deleteReplicas(replicas map[string]*longhorn.Repli
 
 // deleteLeftBackups deletes the backup having no backup volume
 func (c *UninstallController) deleteLeftBackups(backup *longhorn.Backup) (err error) {
-	backupVolumeName, ok := backup.Labels[types.LonghornLabelBackupVolume]
+	volumeName, ok := backup.Labels[types.LonghornLabelBackupVolume]
 	if !ok {
 		// directly delete it if there is even no backup volume label
 		if err = c.ds.DeleteBackup(backup.Name); err != nil {
@@ -779,7 +779,7 @@ func (c *UninstallController) deleteLeftBackups(backup *longhorn.Backup) (err er
 			}
 		}
 	}
-	_, err = c.ds.GetBackupVolume(backupVolumeName)
+	_, err = c.ds.GetBackupVolumeByBackupTargetAndVolumeRO(backup.Status.BackupTargetName, volumeName)
 	if err != nil && apierrors.IsNotFound(err) {
 		if err = c.ds.DeleteBackup(backup.Name); err != nil {
 			if !apierrors.IsNotFound(err) {

diff --git a/manager/engine.go b/manager/engine.go
@@ -505,6 +505,9 @@ func (m *VolumeManager) ListBackupsForVolumeSorted(volumeName string) ([]*longho
 func (m *VolumeManager) ListBackupsForBackupVolumeSorted(backupVolumeName string) ([]*longhorn.Backup, error) {
 	bv, err := m.ds.GetBackupVolumeRO(backupVolumeName)
 	if err != nil {
+		if apierrors.IsNotFound(err) {
+			return []*longhorn.Backup{}, nil
+		}
 		return []*longhorn.Backup{}, err
 	}
 

diff --git a/manager/kubernetes.go b/manager/kubernetes.go
@@ -10,6 +10,7 @@ import (
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 
+	"github.com/longhorn/backupstore"
 	"github.com/longhorn/longhorn-manager/datastore"
 	"github.com/longhorn/longhorn-manager/types"
 	"github.com/longhorn/longhorn-manager/util"
@@ -35,13 +36,29 @@ func (m *VolumeManager) PVCreate(name, pvName, fsType, secretNamespace, secretNa
 		pvName = v.Name
 	}
 
-	if storageClassName == "" {
-		if backupVolumeName := v.Labels[types.LonghornLabelBackupVolume]; backupVolumeName != "" {
-			backupVolume, _ := m.ds.GetBackupVolumeRO(backupVolumeName)
-			if backupVolume != nil {
-				storageClassName = backupVolume.Status.StorageClassName
+	if storageClassName == "" && v.Spec.FromBackup != "" {
+		bName, canonicalBVName, _, err := backupstore.DecodeBackupURL(v.Spec.FromBackup)
+		if err != nil {
+			return nil, errors.Wrapf(err, "failed to get backup and volume name from backup URL %v", v.Spec.FromBackup)
+		}
+		backupTargetName := v.Labels[types.LonghornLabelBackupTarget]
+		backup, err := m.ds.GetBackupRO(bName)
+		if err != nil && !datastore.ErrorIsNotFound(err) {
+			return nil, errors.Wrapf(err, "failed to get backup %v", bName)
+		}
+		if backup != nil {
+			backupTargetName = backup.Status.BackupTargetName
+			if backupTargetName == "" {
+				backupTargetName = backup.Labels[types.LonghornLabelBackupTarget]
+			}
+			if backupTargetName == "" {
+				return nil, fmt.Errorf("failed to get backup target name for backup %v", bName)
 			}
 		}
+		backupVolume, _ := m.ds.GetBackupVolumeByBackupTargetAndVolumeRO(backupTargetName, canonicalBVName)
+		if backupVolume != nil {
+			storageClassName = backupVolume.Status.StorageClassName
+		}
 	}
 
 	if storageClassName == "" {

diff --git a/manager/volume.go b/manager/volume.go
@@ -433,29 +433,29 @@ func (m *VolumeManager) Activate(volumeName string, frontend string) (v *longhor
 
 func (m *VolumeManager) triggerBackupVolumeToSync(volume *longhorn.Volume) error {
 	if volume.Spec.BackupTargetName == "" {
-		return errors.Errorf("cannot find the backup target label for volume: %v", volume.Name)
+		return errors.Errorf("failed to find the backup target label for volume: %v", volume.Name)
 	}
 	backupTargetName := volume.Spec.BackupTargetName
 
-	backupVolumeName, isExist := volume.Labels[types.LonghornLabelBackupVolume]
-	if !isExist || backupVolumeName == "" {
-		return errors.Errorf("cannot find the backup volume label for volume: %v", volume.Name)
+	volumeName, isExist := volume.Labels[types.LonghornLabelBackupVolume]
+	if !isExist || volumeName == "" {
+		return errors.Errorf("failed to find the backup volume label for volume: %v", volume.Name)
 	}
 
-	backupVolume, err := m.ds.GetBackupVolumeByBackupTargetAndVolume(backupTargetName, backupVolumeName)
+	backupVolume, err := m.ds.GetBackupVolumeByBackupTargetAndVolume(backupTargetName, volumeName)
 	if err != nil {
 		// The backup volume may be deleted already.
 		// hence it's better not to block the caller to continue the handlings like DR volume activation.
 		if apierrors.IsNotFound(err) {
-			logrus.Infof("Cannot find backup volume %v of backup target %s to trigger the sync-up, will skip it", backupVolumeName, backupTargetName)
+			logrus.Infof("failed to find backup volume %v of backup target %s to trigger the sync-up, will skip it", volumeName, backupTargetName)
 			return nil
 		}
-		return errors.Wrapf(err, "failed to get backup volume: %v", backupVolumeName)
+		return errors.Wrapf(err, "failed to get backup volume: %v", volumeName)
 	}
 	requestSyncTime := metav1.Time{Time: time.Now().UTC()}
 	backupVolume.Spec.SyncRequestedAt = requestSyncTime
 	if _, err = m.ds.UpdateBackupVolume(backupVolume); err != nil {
-		return errors.Wrapf(err, "failed to update backup volume: %v", backupVolumeName)
+		return errors.Wrapf(err, "failed to update backup volume: %v", backupVolume.Name)
 	}
 
 	return nil