Modify restore logic.

Signed-off-by: Xun Jiang <jxun@vmware.com>

pkg/cmd/util/downloadrequest/downloadrequest.go

@@ -123,7 +123,7 @@ func Stream(ctx context.Context, kbClient kbclient.Client, namespace, name strin
 		ExpectContinueTimeout: defaultTransport.ExpectContinueTimeout,
 	}
 
-	httpReq, err := http.NewRequest(http.MethodGet, created.Status.DownloadURL, nil)
+	httpReq, err := http.NewRequestWithContext(context.TODO(), http.MethodGet, created.Status.DownloadURL, nil)
 	if err != nil {
 		return err
 	}

pkg/controller/restore_controller.go

@@ -523,8 +523,9 @@ func (r *restoreReconciler) runValidatedRestore(restore *api.Restore, info backu
 
 	backupVolumeInfoMap := make(map[string]internalVolume.VolumeInfo)
 	volumeInfos, err := backupStore.GetBackupVolumeInfos(restore.Spec.BackupName)
-	if err != nil || len(volumeInfos) == 0 {
-		restoreLog.Infof("Backup %s doesn't have volumeinfos metadata file.", restore.Spec.BackupName)
+	if err != nil {
+		restoreLog.WithError(err).Errorf("fail to get VolumeInfos metadata file for backup %s", restore.Spec.BackupName)
+		return errors.WithStack(err)
 	} else {
 		for _, volumeInfo := range volumeInfos {
 			backupVolumeInfoMap[volumeInfo.PVName] = *volumeInfo

pkg/restore/restore.go

@@ -81,15 +81,6 @@ var resourceMustHave = []string{
 	"datauploads.velero.io",
 }
 
-const (
-	pvHasNativeSnapshot = iota + 1
-	pvHasPodVolumeBackup
-	pvHasCSIVolumeSnapshot
-	pvHasDataUpload
-	pvHasReclaimPolicyAsDelete
-	pvHasReclaimPolicyAsRetain
-)
-
 type VolumeSnapshotterGetter interface {
 	GetVolumeSnapshotter(name string) (vsv1.VolumeSnapshotter, error)
 }
@@ -1244,28 +1235,56 @@ func (ctx *restoreContext) restoreItem(obj *unstructured.Unstructured, groupReso
 	}
 
 	if groupResource == kuberesource.PersistentVolumes {
-		pvCondition := 0
 		if volumeInfo, ok := ctx.volumeInfoMap[obj.GetName()]; ok {
 			ctx.log.Infof("Find VolumeInfo for PV %s.", obj.GetName())
 
 			switch volumeInfo.BackupMethod {
 			case internalVolume.NativeSnapshot:
-				pvCondition = pvHasNativeSnapshot
+				obj, err = ctx.handlePVHasNativeSnapshot(obj, resourceClient)
+				if err != nil {
+					errs.Add(namespace, err)
+					return warnings, errs, itemExists
+				}
+
+				name = obj.GetName()
+
 			case internalVolume.PodVolumeBackup:
-				pvCondition = pvHasPodVolumeBackup
+				restoreLogger.Infof("Dynamically re-provisioning persistent volume because it has a pod volume backup to be restored.")
+				ctx.pvsToProvision.Insert(name)
+
+				// Return early because we don't want to restore the PV itself, we
+				// want to dynamically re-provision it.
+				return warnings, errs, itemExists
+
 			case internalVolume.CSISnapshot:
-				if volumeInfo.SnapshotDataMoved {
-					pvCondition = pvHasDataUpload
-				} else {
-					pvCondition = pvHasCSIVolumeSnapshot
+				restoreLogger.Infof("Dynamically re-provisioning persistent volume because it has a CSI VolumeSnapshot or a related snapshot DataUpload.")
+				ctx.pvsToProvision.Insert(name)
+
+				if ready, err := ctx.featureVerifier.Verify(velerov1api.CSIFeatureFlag); !ready {
+					ctx.log.Errorf("Failed to verify CSI modules, ready %v, err %v", ready, err)
+					errs.Add(namespace, fmt.Errorf("CSI modules are not ready for restore. Check CSI feature is enabled and CSI plugin is installed"))
 				}
+
+				// Return early because we don't want to restore the PV itself, we
+				// want to dynamically re-provision it.
+				return warnings, errs, itemExists
+
 			// When the PV data is skipped from backup, it's VolumeInfo BackupMethod
 			// is not set, and it will fall into the default case.
 			default:
 				if hasDeleteReclaimPolicy(obj.Object) {
-					pvCondition = pvHasReclaimPolicyAsDelete
+					restoreLogger.Infof("Dynamically re-provisioning persistent volume because it doesn't have a snapshot and its reclaim policy is Delete.")
+					ctx.pvsToProvision.Insert(name)
+
+					// Return early because we don't want to restore the PV itself, we
+					// want to dynamically re-provision it.
+					return warnings, errs, itemExists
 				} else {
-					pvCondition = pvHasReclaimPolicyAsRetain
+					obj, err = ctx.handleSkippedPVHasRetainPolicy(obj, resourceID, restoreLogger)
+					if err != nil {
+						errs.Add(namespace, err)
+						return warnings, errs, itemExists
+					}
 				}
 			}
 		} else {
@@ -1275,79 +1294,52 @@ func (ctx *restoreContext) restoreItem(obj *unstructured.Unstructured, groupReso
 
 			switch {
 			case hasSnapshot(name, ctx.volumeSnapshots):
-				pvCondition = pvHasNativeSnapshot
-			case hasPodVolumeBackup(obj, ctx):
-				pvCondition = pvHasPodVolumeBackup
-			case hasCSIVolumeSnapshot(ctx, obj):
-				pvCondition = pvHasCSIVolumeSnapshot
-			case hasSnapshotDataUpload(ctx, obj):
-				pvCondition = pvHasDataUpload
-			case hasDeleteReclaimPolicy(obj.Object):
-				pvCondition = pvHasReclaimPolicyAsDelete
-			default:
-				pvCondition = pvHasReclaimPolicyAsRetain
-			}
-		}
-
-		switch pvCondition {
-		case pvHasNativeSnapshot:
-			obj, err = ctx.handlePVHasNativeSnapshot(obj, resourceClient)
-			if err != nil {
-				errs.Add(namespace, err)
-				return warnings, errs, itemExists
-			}
-
-			name = obj.GetName()
-
-		case pvHasPodVolumeBackup:
-			restoreLogger.Infof("Dynamically re-provisioning persistent volume because it has a pod volume backup to be restored.")
-			ctx.pvsToProvision.Insert(name)
+				obj, err = ctx.handlePVHasNativeSnapshot(obj, resourceClient)
+				if err != nil {
+					errs.Add(namespace, err)
+					return warnings, errs, itemExists
+				}
 
-			// Return early because we don't want to restore the PV itself, we
-			// want to dynamically re-provision it.
-			return warnings, errs, itemExists
+				name = obj.GetName()
 
-		case pvHasCSIVolumeSnapshot:
-			fallthrough
-		case pvHasDataUpload:
-			restoreLogger.Infof("Dynamically re-provisioning persistent volume because it has a CSI VolumeSnapshot or a related snapshot DataUpload.")
-			ctx.pvsToProvision.Insert(name)
+			case hasPodVolumeBackup(obj, ctx):
+				restoreLogger.Infof("Dynamically re-provisioning persistent volume because it has a pod volume backup to be restored.")
+				ctx.pvsToProvision.Insert(name)
 
-			if ready, err := ctx.featureVerifier.Verify(velerov1api.CSIFeatureFlag); !ready {
-				ctx.log.Errorf("Failed to verify CSI modules, ready %v, err %v", ready, err)
-				errs.Add(namespace, fmt.Errorf("CSI modules are not ready for restore. Check CSI feature is enabled and CSI plugin is installed"))
-			}
+				// Return early because we don't want to restore the PV itself, we
+				// want to dynamically re-provision it.
+				return warnings, errs, itemExists
 
-			// Return early because we don't want to restore the PV itself, we
-			// want to dynamically re-provision it.
-			return warnings, errs, itemExists
+			case hasCSIVolumeSnapshot(ctx, obj):
+				fallthrough
+			case hasSnapshotDataUpload(ctx, obj):
+				restoreLogger.Infof("Dynamically re-provisioning persistent volume because it has a CSI VolumeSnapshot or a related snapshot DataUpload.")
+				ctx.pvsToProvision.Insert(name)
 
-		case pvHasReclaimPolicyAsDelete:
-			restoreLogger.Infof("Dynamically re-provisioning persistent volume because it doesn't have a snapshot and its reclaim policy is Delete.")
-			ctx.pvsToProvision.Insert(name)
+				if ready, err := ctx.featureVerifier.Verify(velerov1api.CSIFeatureFlag); !ready {
+					ctx.log.Errorf("Failed to verify CSI modules, ready %v, err %v", ready, err)
+					errs.Add(namespace, fmt.Errorf("CSI modules are not ready for restore. Check CSI feature is enabled and CSI plugin is installed"))
+				}
 
-			// Return early because we don't want to restore the PV itself, we
-			// want to dynamically re-provision it.
-			return warnings, errs, itemExists
+				// Return early because we don't want to restore the PV itself, we
+				// want to dynamically re-provision it.
+				return warnings, errs, itemExists
 
-		case pvHasReclaimPolicyAsRetain:
-			restoreLogger.Infof("Restoring persistent volume as-is because it doesn't have a snapshot and its reclaim policy is not Delete.")
+			case hasDeleteReclaimPolicy(obj.Object):
+				restoreLogger.Infof("Dynamically re-provisioning persistent volume because it doesn't have a snapshot and its reclaim policy is Delete.")
+				ctx.pvsToProvision.Insert(name)
 
-			// Check to see if the claimRef.namespace field needs to be remapped, and do so if necessary.
-			_, err = remapClaimRefNS(ctx, obj)
-			if err != nil {
-				errs.Add(namespace, err)
-				return warnings, errs, itemExists
-			}
-			obj = resetVolumeBindingInfo(obj)
-			// We call the pvRestorer here to clear out the PV's claimRef.UID,
-			// so it can be re-claimed when its PVC is restored and gets a new UID.
-			updatedObj, err := ctx.pvRestorer.executePVAction(obj)
-			if err != nil {
-				errs.Add(namespace, fmt.Errorf("error executing PVAction for %s: %v", resourceID, err))
+				// Return early because we don't want to restore the PV itself, we
+				// want to dynamically re-provision it.
 				return warnings, errs, itemExists
+
+			default:
+				obj, err = ctx.handleSkippedPVHasRetainPolicy(obj, resourceID, restoreLogger)
+				if err != nil {
+					errs.Add(namespace, err)
+					return warnings, errs, itemExists
+				}
 			}
-			obj = updatedObj
 		}
 	}
 
@@ -2517,3 +2509,27 @@ func (ctx *restoreContext) handlePVHasNativeSnapshot(obj *unstructured.Unstructu
 
 	return retObj, nil
 }
+
+func (ctx *restoreContext) handleSkippedPVHasRetainPolicy(
+	obj *unstructured.Unstructured,
+	resourceID string,
+	logger logrus.FieldLogger,
+) (*unstructured.Unstructured, error) {
+	logger.Infof("Restoring persistent volume as-is because it doesn't have a snapshot and its reclaim policy is not Delete.")
+
+	// Check to see if the claimRef.namespace field needs to be remapped, and do so if necessary.
+	if _, err := remapClaimRefNS(ctx, obj); err != nil {
+		return nil, err
+	}
+
+	obj = resetVolumeBindingInfo(obj)
+
+	// We call the pvRestorer here to clear out the PV's claimRef.UID,
+	// so it can be re-claimed when its PVC is restored and gets a new UID.
+	updatedObj, err := ctx.pvRestorer.executePVAction(obj)
+	if err != nil {
+		return nil, fmt.Errorf("error executing PVAction for %s: %v", resourceID, err)
+	}
+
+	return updatedObj, nil
+}