restic: don't try to restore PVBs with no snapshotID

Signed-off-by: Steve Kriss <[email protected]>
vmware-tanzu · Nov 1, 2019 · 728930f · 728930f
1 parent 83752d2
commit 728930f
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 6 deletions.
diff --git a/pkg/restic/common.go b/pkg/restic/common.go
@@ -87,9 +87,17 @@ func GetVolumeBackupsForPod(podVolumeBackups []*velerov1api.PodVolumeBackup, pod
 	volumes := make(map[string]string)
 
 	for _, pvb := range podVolumeBackups {
-		if pod.GetName() == pvb.Spec.Pod.Name {
-			volumes[pvb.Spec.Volume] = pvb.Status.SnapshotID
+		if pod.GetName() != pvb.Spec.Pod.Name {
+			continue
 		}
+
+		// skip PVBs without a snapshot ID since there's nothing
+		// to restore (they could be failed, or for empty volumes).
+		if pvb.Status.SnapshotID == "" {
+			continue
+		}
+
+		volumes[pvb.Spec.Volume] = pvb.Status.SnapshotID
 	}
 
 	if len(volumes) > 0 {

diff --git a/pkg/restic/common_test.go b/pkg/restic/common_test.go
@@ -83,14 +83,25 @@ func TestGetVolumeBackupsForPod(t *testing.T) {
 			expected:       map[string]string{"pvbtest1-foo": "bar", "pvbtest2-abc": "123"},
 		},
 		{
-			name: "no snapshot annotation, no suffix, but with PVBs",
+			name: "no snapshot annotation, but with PVBs",
 			podVolumeBackups: []*velerov1api.PodVolumeBackup{
 				builder.ForPodVolumeBackup("velero", "pvb-1").PodName("TestPod").SnapshotID("bar").Volume("pvbtest1-foo").Result(),
 				builder.ForPodVolumeBackup("velero", "pvb-2").PodName("TestPod").SnapshotID("123").Volume("pvbtest2-abc").Result(),
 			},
 			podName:  "TestPod",
 			expected: map[string]string{"pvbtest1-foo": "bar", "pvbtest2-abc": "123"},
 		},
+		{
+			name: "no snapshot annotation, but with PVBs, some of which have snapshot IDs and some of which don't",
+			podVolumeBackups: []*velerov1api.PodVolumeBackup{
+				builder.ForPodVolumeBackup("velero", "pvb-1").PodName("TestPod").SnapshotID("bar").Volume("pvbtest1-foo").Result(),
+				builder.ForPodVolumeBackup("velero", "pvb-2").PodName("TestPod").SnapshotID("123").Volume("pvbtest2-abc").Result(),
+				builder.ForPodVolumeBackup("velero", "pvb-3").PodName("TestPod").Volume("pvbtest3-foo").Result(),
+				builder.ForPodVolumeBackup("velero", "pvb-4").PodName("TestPod").Volume("pvbtest4-abc").Result(),
+			},
+			podName:  "TestPod",
+			expected: map[string]string{"pvbtest1-foo": "bar", "pvbtest2-abc": "123"},
+		},
 		{
 			name: "has snapshot annotation, with suffix, and with PVBs from current pod and a PVB from another pod",
 			podVolumeBackups: []*velerov1api.PodVolumeBackup{

diff --git a/pkg/restore/restic_restore_action_test.go b/pkg/restore/restic_restore_action_test.go
@@ -158,11 +158,13 @@ func TestResticRestoreActionExecute(t *testing.T) {
 					PodName("my-pod").
 					Volume("vol-1").
 					ObjectMeta(builder.WithLabels(velerov1api.BackupNameLabel, backupName)).
+					SnapshotID("foo").
 					Result(),
 				builder.ForPodVolumeBackup(veleroNs, "pvb-2").
 					PodName("my-pod").
 					Volume("vol-2").
 					ObjectMeta(builder.WithLabels(velerov1api.BackupNameLabel, backupName)).
+					SnapshotID("foo").
 					Result(),
 			},
 			want: builder.ForPod("ns-1", "my-pod").

diff --git a/pkg/restore/restore_test.go b/pkg/restore/restore_test.go
@@ -2277,9 +2277,9 @@ func TestRestoreWithRestic(t *testing.T) {
 			backup:       defaultBackup().Result(),
 			apiResources: []*test.APIResource{test.Pods()},
 			podVolumeBackups: []*velerov1api.PodVolumeBackup{
-				builder.ForPodVolumeBackup("velero", "pvb-1").PodName("pod-1").Result(),
-				builder.ForPodVolumeBackup("velero", "pvb-2").PodName("pod-2").Result(),
-				builder.ForPodVolumeBackup("velero", "pvb-3").PodName("pod-4").Result(),
+				builder.ForPodVolumeBackup("velero", "pvb-1").PodName("pod-1").SnapshotID("foo").Result(),
+				builder.ForPodVolumeBackup("velero", "pvb-2").PodName("pod-2").SnapshotID("foo").Result(),
+				builder.ForPodVolumeBackup("velero", "pvb-3").PodName("pod-4").SnapshotID("foo").Result(),
 			},
 			podWithPVBs: []*corev1api.Pod{
 				builder.ForPod("ns-1", "pod-2").