From debcbd2f8e1090648c2d753d272f6ffca0f03bfd Mon Sep 17 00:00:00 2001
From: Pawan Prakash Sharma <pawanprakash101@gmail.com>
Date: Wed, 2 Sep 2020 02:55:13 +0530
Subject: [PATCH] fix: rename the PV if VolumeSnapshotter has modified the PV
 name (#2835)

* fix: rename the PV if VolumeSnapshotter has modified the PV name

When VolumeSnapshotter sets the PV name via SetVolumeID and PV is
not there in the cluster, velero does not rename the PV. Which causes
the pvc to be in the lost state as pvc points to the old PV but pv object
has been renamed by VolumeSnapshotter.

Signed-off-by: Pawan <pawan@mayadata.io>

* adding a test case for pv rename

Signed-off-by: Pawan <pawan@mayadata.io>
---
 changelogs/unreleased/2835-pawanpraka1 |  1 +
 pkg/restore/restore.go                 |  5 +++
 pkg/restore/restore_test.go            | 60 ++++++++++++++++++++++++++
 3 files changed, 66 insertions(+)
 create mode 100644 changelogs/unreleased/2835-pawanpraka1

diff --git a/changelogs/unreleased/2835-pawanpraka1 b/changelogs/unreleased/2835-pawanpraka1
new file mode 100644
index 000000000..8bbc9ef85
--- /dev/null
+++ b/changelogs/unreleased/2835-pawanpraka1
@@ -0,0 +1 @@
+rename the PV if VolumeSnapshotter has modified the PV name
diff --git a/pkg/restore/restore.go b/pkg/restore/restore.go
index 13a1185ab..300f1548a 100644
--- a/pkg/restore/restore.go
+++ b/pkg/restore/restore.go
@@ -866,6 +866,11 @@ func (ctx *restoreContext) restoreItem(obj *unstructured.Unstructured, groupReso
 					return warnings, errs
 				}
 				obj = updatedObj
+
+				// VolumeSnapshotter has modified the PV name, we should rename the PV
+				if oldName != obj.GetName() {
+					shouldRenamePV = true
+				}
 			}
 
 			if shouldRenamePV {
diff --git a/pkg/restore/restore_test.go b/pkg/restore/restore_test.go
index 0c53cdbe4..8e8d3b3fb 100644
--- a/pkg/restore/restore_test.go
+++ b/pkg/restore/restore_test.go
@@ -2174,6 +2174,66 @@ func TestRestorePersistentVolumes(t *testing.T) {
 				),
 			},
 		},
+		{
+			name:    "when a PV is renamed and the original PV does not exist in-cluster, the PV should be renamed",
+			restore: defaultRestore().NamespaceMappings("source-ns", "target-ns").Result(),
+			backup:  defaultBackup().Result(),
+			tarball: test.NewTarWriter(t).
+				AddItems(
+					"persistentvolumes",
+					builder.ForPersistentVolume("source-pv").AWSEBSVolumeID("source-volume").ClaimRef("source-ns", "pvc-1").Result(),
+				).
+				AddItems(
+					"persistentvolumeclaims",
+					builder.ForPersistentVolumeClaim("source-ns", "pvc-1").VolumeName("source-pv").Result(),
+				).
+				Done(),
+			apiResources: []*test.APIResource{
+				test.PVs(),
+				test.PVCs(),
+			},
+			volumeSnapshots: []*volume.Snapshot{
+				{
+					Spec: volume.SnapshotSpec{
+						BackupName:           "backup-1",
+						Location:             "default",
+						PersistentVolumeName: "source-pv",
+					},
+					Status: volume.SnapshotStatus{
+						Phase:              volume.SnapshotPhaseCompleted,
+						ProviderSnapshotID: "snapshot-1",
+					},
+				},
+			},
+			volumeSnapshotLocations: []*velerov1api.VolumeSnapshotLocation{
+				builder.ForVolumeSnapshotLocation(velerov1api.DefaultNamespace, "default").Provider("provider-1").Result(),
+			},
+			volumeSnapshotterGetter: map[string]velero.VolumeSnapshotter{
+				"provider-1": &volumeSnapshotter{
+					snapshotVolumes: map[string]string{"snapshot-1": "new-pvname"},
+					pvName:          map[string]string{"new-pvname": "new-pvname"},
+				},
+			},
+			want: []*test.APIResource{
+				test.PVs(
+					builder.ForPersistentVolume("new-pvname").
+						ObjectMeta(
+							builder.WithLabels("velero.io/backup-name", "backup-1", "velero.io/restore-name", "restore-1"),
+							builder.WithAnnotations("velero.io/original-pv-name", "source-pv"),
+						).
+						AWSEBSVolumeID("new-pvname").
+						Result(),
+				),
+				test.PVCs(
+					builder.ForPersistentVolumeClaim("target-ns", "pvc-1").
+						ObjectMeta(
+							builder.WithLabels("velero.io/backup-name", "backup-1", "velero.io/restore-name", "restore-1"),
+						).
+						VolumeName("new-pvname").
+						Result(),
+				),
+			},
+		},
 		{
 			name:    "when a PV with a reclaim policy of retain has a snapshot and exists in-cluster, neither the snapshot nor the PV are restored",
 			restore: defaultRestore().Result(),