diff --git a/changelogs/unreleased/2031-skriss b/changelogs/unreleased/2031-skriss new file mode 100644 index 0000000000..d94b727d96 --- /dev/null +++ b/changelogs/unreleased/2031-skriss @@ -0,0 +1 @@ +bug fix: don't try to restore pod volume backups that don't have a snapshot ID diff --git a/pkg/restic/common.go b/pkg/restic/common.go index b30f0c3eb3..54a9af769d 100644 --- 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 index b22545779a..33500ad924 100644 --- a/pkg/restic/common_test.go +++ b/pkg/restic/common_test.go @@ -83,7 +83,7 @@ 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(), @@ -91,6 +91,17 @@ func TestGetVolumeBackupsForPod(t *testing.T) { 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 index 8bee92820b..9db5c3a367 100644 --- 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 index b87bb820b5..5811476298 100644 --- 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").