Fix #6444: remove PVC selector during restore

1. Remove PVC's selector in restore PVC action. 2. Return PVC intactly when CSI is not present in PVC spec to avoid deleting useful annocations. Signed-off-by: Xun Jiang <[email protected]>
vmware-tanzu · Jul 7, 2023 · 7db0f4e · 7db0f4e
1 parent 94f4cae
commit 7db0f4e
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 9 deletions.
diff --git a/internal/restore/pvc_action.go b/internal/restore/pvc_action.go
@@ -96,6 +96,9 @@ func resetPVCSpec(pvc *corev1api.PersistentVolumeClaim, vsName string) {
 	}
 	pvc.Spec.DataSource = dataSourceRef
 	pvc.Spec.DataSourceRef = dataSourceRef
+
+	// Clean the selector to make the PVC to dynamically allocate PV.
+	pvc.Spec.Selector = new(metav1.LabelSelector)
 }
 
 func setPVCStorageResourceRequest(pvc *corev1api.PersistentVolumeClaim, restoreSize resource.Quantity, log logrus.FieldLogger) {
@@ -167,7 +170,15 @@ func (p *PVCRestoreItemAction) Execute(input *velero.RestoreItemActionExecuteInp
 			}
 			logger.Infof("DataDownload %s/%s is created successfully.", dataDownload.Namespace, dataDownload.Name)
 		} else {
-			if err := restoreFromVolumeSnapshot(&pvc, p.SnapshotClient, logger); err != nil {
+			volumeSnapshotName, ok := pvc.Annotations[util.VolumeSnapshotLabel]
+			if !ok {
+				logger.Info("Skipping PVCRestoreItemAction for PVC , PVC does not have a CSI volumesnapshot.")
+				// Make no change in the input PVC.
+				return &velero.RestoreItemActionExecuteOutput{
+					UpdatedItem: input.Item,
+				}, nil
+			}
+			if err := restoreFromVolumeSnapshot(&pvc, p.SnapshotClient, volumeSnapshotName, logger); err != nil {
 				logger.Errorf("Failed to restore PVC from VolumeSnapshot.")
 				return nil, errors.WithStack(err)
 			}
@@ -385,13 +396,7 @@ func newDataDownload(restore *velerov1api.Restore, dataUploadResult *velerov2alp
 }
 
 func restoreFromVolumeSnapshot(pvc *corev1api.PersistentVolumeClaim, snapClient snapshotterClientSet.Interface,
-	logger logrus.FieldLogger) error {
-	volumeSnapshotName, ok := pvc.Annotations[util.VolumeSnapshotLabel]
-	if !ok {
-		logger.Info("Skipping PVCRestoreItemAction for PVC , PVC does not have a CSI volumesnapshot.")
-		return nil
-	}
-
+	volumeSnapshotName string, logger logrus.FieldLogger) error {
 	vs, err := snapClient.SnapshotV1().VolumeSnapshots(pvc.Namespace).Get(context.TODO(), volumeSnapshotName, metav1.GetOptions{})
 	if err != nil {
 		return errors.Wrapf(err, fmt.Sprintf("Failed to get Volumesnapshot %s/%s to restore PVC %s/%s", pvc.Namespace, volumeSnapshotName, pvc.Namespace, pvc.Name))

diff --git a/internal/restore/pvc_action_test.go b/internal/restore/pvc_action_test.go
@@ -242,7 +242,7 @@ func TestResetPVCSpec(t *testing.T) {
 			assert.Equalf(t, tc.pvc.Name, before.Name, "unexpected change to Object.Name, Want: %s; Got %s", before.Name, tc.pvc.Name)
 			assert.Equalf(t, tc.pvc.Namespace, before.Namespace, "unexpected change to Object.Namespace, Want: %s; Got %s", before.Namespace, tc.pvc.Namespace)
 			assert.Equalf(t, tc.pvc.Spec.AccessModes, before.Spec.AccessModes, "unexpected Spec.AccessModes, Want: %v; Got: %v", before.Spec.AccessModes, tc.pvc.Spec.AccessModes)
-			assert.Equalf(t, tc.pvc.Spec.Selector, before.Spec.Selector, "unexpected change to Spec.Selector, Want: %s; Got: %s", before.Spec.Selector.String(), tc.pvc.Spec.Selector.String())
+			assert.Emptyf(t, tc.pvc.Spec.Selector, "expected change to Spec.Selector missing, Want: \"\"; Got: %s", tc.pvc.Spec.Selector.String())
 			assert.Equalf(t, tc.pvc.Spec.Resources, before.Spec.Resources, "unexpected change to Spec.Resources, Want: %s; Got: %s", before.Spec.Resources.String(), tc.pvc.Spec.Resources.String())
 			assert.Emptyf(t, tc.pvc.Spec.VolumeName, "expected change to Spec.VolumeName missing, Want: \"\"; Got: %s", tc.pvc.Spec.VolumeName)
 			assert.Equalf(t, *tc.pvc.Spec.VolumeMode, *before.Spec.VolumeMode, "expected change to Spec.VolumeName missing, Want: \"\"; Got: %s", tc.pvc.Spec.VolumeName)