fix(dr): use Status.CompletionTimestamp for sorting backups (#1679)

Author: emosbaugh

cmd/installer/cli/restore.go

@@ -192,8 +192,8 @@ func RestoreCmd(ctx context.Context, name string) *cobra.Command {
 					return fmt.Errorf("unable to resume: %w", err)
 				}
 				if backupToRestore != nil {
-					creationTimestamp := backupToRestore.GetCreationTimestamp().Format("2006-01-02 15:04:05 UTC")
-					logrus.Infof("Resuming restore from backup %q (%s)\n", backupToRestore.GetName(), creationTimestamp)
+					completionTimestamp := backupToRestore.GetCompletionTimestamp().Format("2006-01-02 15:04:05 UTC")
+					logrus.Infof("Resuming restore from backup %q (%s)\n", backupToRestore.GetName(), completionTimestamp)
 
 					if err := overrideRuntimeConfigFromBackup(localArtifactMirrorPort, *backupToRestore); err != nil {
 						return fmt.Errorf("unable to override runtime config from backup: %w", err)
@@ -339,10 +339,11 @@ func RestoreCmd(ctx context.Context, name string) *cobra.Command {
 
 				logrus.Debugf("picking backup to restore")
 				backupToRestore = pickBackupToRestore(backups)
+				logrus.Debugf("backup to restore: %s", backupToRestore.GetName())
 
 				logrus.Info("")
-				creationTimestamp := backupToRestore.GetCreationTimestamp().Time.Format("2006-01-02 15:04:05 UTC")
-				shouldRestore := prompts.New().Confirm(fmt.Sprintf("Restore from backup %q (%s)?", backupToRestore.GetName(), creationTimestamp), true)
+				completionTimestamp := backupToRestore.GetCompletionTimestamp().Format("2006-01-02 15:04:05 UTC")
+				shouldRestore := prompts.New().Confirm(fmt.Sprintf("Restore from backup %q (%s)?", backupToRestore.GetName(), completionTimestamp), true)
 				logrus.Info("")
 				if !shouldRestore {
 					logrus.Infof("Aborting restore...")
@@ -1015,6 +1016,7 @@ func waitForBackups(ctx context.Context, out io.Writer, kcli client.Client, k0sC
 		}
 	}
 
+	logrus.Debugf("Found %d restorable backup(s)", len(validBackups))
 	if len(validBackups) == 1 {
 		loading.Infof("Found 1 restorable backup!")
 	} else {
@@ -1053,7 +1055,8 @@ func pickBackupToRestore(backups []disasterrecovery.ReplicatedBackup) *disasterr
 			latestBackup = &b
 			continue
 		}
-		if b.GetCreationTimestamp().After(latestBackup.GetCreationTimestamp().Time) {
+		// Should this use Status.StartTimestamp instead of Status.CompletionTimestamp?
+		if b.GetCompletionTimestamp().After(latestBackup.GetCompletionTimestamp().Time) {
 			latestBackup = &b
 		}
 	}

pkg/disasterrecovery/backup.go

@@ -63,22 +63,32 @@ var (
 )
 
 // ReplicatedBackups implements sort.Interface for []ReplicatedBackup based on the
-// CreationTimestamp of the infrastructure backup.
+// Status.StartTimestamp of the infrastructure backup.
 type ReplicatedBackups []ReplicatedBackup
 
 func (a ReplicatedBackups) Len() int      { return len(a) }
 func (a ReplicatedBackups) Swap(i, j int) { a[i], a[j] = a[j], a[i] }
 func (a ReplicatedBackups) Less(i, j int) bool {
 	var iTime, jTime time.Time
-	if a[i].GetInfraBackup() != nil {
-		iTime = a[i].GetInfraBackup().GetCreationTimestamp().Time
-	} else if len(a[i]) > 0 {
-		iTime = a[i][0].GetCreationTimestamp().Time
+	if a[i].GetInfraBackup() != nil && a[i].GetInfraBackup().Status.StartTimestamp != nil {
+		iTime = a[i].GetInfraBackup().Status.StartTimestamp.Time
+	} else {
+		for _, backup := range a[i] {
+			if backup.Status.StartTimestamp != nil {
+				iTime = backup.Status.StartTimestamp.Time
+				break
+			}
+		}
 	}
-	if a[j].GetInfraBackup() != nil {
-		jTime = a[j].GetInfraBackup().GetCreationTimestamp().Time
-	} else if len(a[j]) > 0 {
-		jTime = a[j][0].GetCreationTimestamp().Time
+	if a[j].GetInfraBackup() != nil && a[j].GetInfraBackup().Status.StartTimestamp != nil {
+		jTime = a[j].GetInfraBackup().Status.StartTimestamp.Time
+	} else {
+		for _, backup := range a[j] {
+			if backup.Status.StartTimestamp != nil {
+				jTime = backup.Status.StartTimestamp.Time
+				break
+			}
+		}
 	}
 	return iTime.Before(jTime)
 }
@@ -181,18 +191,22 @@ func (b ReplicatedBackup) GetRestore() (*velerov1.Restore, error) {
 	return nil, fmt.Errorf("missing restore spec annotation")
 }
 
-// GetCreationTimestamp returns the creation timestamp of the velero infra backup object or the
-// first backup in the slice if the infra backup is not found.
-func (b ReplicatedBackup) GetCreationTimestamp() metav1.Time {
-	backup := b.GetInfraBackup()
-	if backup != nil {
-		return backup.GetCreationTimestamp()
+// GetCompletionTimestamp returns the completion timestamp of the last velero backup to be
+// completed or zero if the expected backup count is not met or any of the backups in the slice is
+// not completed.
+func (b ReplicatedBackup) GetCompletionTimestamp() metav1.Time {
+	if b.GetExpectedBackupCount() != len(b) {
+		return metav1.Time{}
 	}
-	// if the infra backup is not found, we return the creation timestamp of the first backup
+	var completionTimestamp metav1.Time
 	for _, backup := range b {
-		return backup.GetCreationTimestamp()
+		if backup.Status.CompletionTimestamp == nil {
+			return metav1.Time{}
+		} else if backup.Status.CompletionTimestamp.Time.After(completionTimestamp.Time) {
+			completionTimestamp = *backup.Status.CompletionTimestamp
+		}
 	}
-	return metav1.Time{}
+	return completionTimestamp
 }
 
 // GetAnnotation returns the value of the specified annotation key from the velero infra backup