diff --git a/changelogs/unreleased/3051-ashish-amarnath b/changelogs/unreleased/3051-ashish-amarnath new file mode 100644 index 00000000000..9715242c73d --- /dev/null +++ b/changelogs/unreleased/3051-ashish-amarnath @@ -0,0 +1 @@ +🐛 Use namespace and name to match PVB to Pod restore diff --git a/pkg/builder/pod_volume_backup_builder.go b/pkg/builder/pod_volume_backup_builder.go index a1033893270..82af70bdd32 100644 --- a/pkg/builder/pod_volume_backup_builder.go +++ b/pkg/builder/pod_volume_backup_builder.go @@ -75,6 +75,12 @@ func (b *PodVolumeBackupBuilder) PodName(name string) *PodVolumeBackupBuilder { return b } +// PodNamespace sets the name of the pod associated with this PodVolumeBackup. +func (b *PodVolumeBackupBuilder) PodNamespace(ns string) *PodVolumeBackupBuilder { + b.object.Spec.Pod.Namespace = ns + return b +} + // Volume sets the name of the volume associated with this PodVolumeBackup. func (b *PodVolumeBackupBuilder) Volume(volume string) *PodVolumeBackupBuilder { b.object.Spec.Volume = volume diff --git a/pkg/restic/common.go b/pkg/restic/common.go index f87ba656f88..4c98ea42709 100644 --- a/pkg/restic/common.go +++ b/pkg/restic/common.go @@ -95,13 +95,17 @@ func getPodSnapshotAnnotations(obj metav1.Object) map[string]string { return res } +func isPVBMatchPod(pvb *velerov1api.PodVolumeBackup, pod metav1.Object) bool { + return pod.GetName() == pvb.Spec.Pod.Name && pod.GetNamespace() == pvb.Spec.Pod.Namespace +} + // GetVolumeBackupsForPod returns a map, of volume name -> snapshot id, // of the PodVolumeBackups that exist for the provided pod. func GetVolumeBackupsForPod(podVolumeBackups []*velerov1api.PodVolumeBackup, pod metav1.Object) map[string]string { volumes := make(map[string]string) for _, pvb := range podVolumeBackups { - if pod.GetName() != pvb.Spec.Pod.Name { + if !isPVBMatchPod(pvb, pod) { continue } diff --git a/pkg/restic/common_test.go b/pkg/restic/common_test.go index 99f48dc6ed7..c29ee643f3d 100644 --- a/pkg/restic/common_test.go +++ b/pkg/restic/common_test.go @@ -564,6 +564,88 @@ func TestGetPodVolumesUsingRestic(t *testing.T) { } } +func TestIsPVBMatchPod(t *testing.T) { + testCases := []struct { + name string + pod metav1.Object + pvb velerov1api.PodVolumeBackup + expected bool + }{ + { + name: "should match PVB and pod", + pod: &metav1.ObjectMeta{ + Name: "matching-pod", + Namespace: "matching-namespace", + }, + pvb: velerov1api.PodVolumeBackup{ + Spec: velerov1api.PodVolumeBackupSpec{ + Pod: corev1api.ObjectReference{ + Name: "matching-pod", + Namespace: "matching-namespace", + }, + }, + }, + expected: true, + }, + { + name: "should not match PVB and pod, pod name mismatch", + pod: &metav1.ObjectMeta{ + Name: "not-matching-pod", + Namespace: "matching-namespace", + }, + pvb: velerov1api.PodVolumeBackup{ + Spec: velerov1api.PodVolumeBackupSpec{ + Pod: corev1api.ObjectReference{ + Name: "matching-pod", + Namespace: "matching-namespace", + }, + }, + }, + expected: false, + }, + { + name: "should not match PVB and pod, pod namespace mismatch", + pod: &metav1.ObjectMeta{ + Name: "matching-pod", + Namespace: "not-matching-namespace", + }, + pvb: velerov1api.PodVolumeBackup{ + Spec: velerov1api.PodVolumeBackupSpec{ + Pod: corev1api.ObjectReference{ + Name: "matching-pod", + Namespace: "matching-namespace", + }, + }, + }, + expected: false, + }, + { + name: "should not match PVB and pod, pod name and namespace mismatch", + pod: &metav1.ObjectMeta{ + Name: "not-matching-pod", + Namespace: "not-matching-namespace", + }, + pvb: velerov1api.PodVolumeBackup{ + Spec: velerov1api.PodVolumeBackupSpec{ + Pod: corev1api.ObjectReference{ + Name: "matching-pod", + Namespace: "matching-namespace", + }, + }, + }, + expected: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + actual := isPVBMatchPod(&tc.pvb, tc.pod) + assert.Equal(t, tc.expected, actual) + }) + + } +} + func newFakeClient(t *testing.T, initObjs ...runtime.Object) client.Client { err := velerov1api.AddToScheme(scheme.Scheme) require.NoError(t, err) diff --git a/pkg/restore/restic_restore_action_test.go b/pkg/restore/restic_restore_action_test.go index 6623cd3f5aa..0211cdcbaf0 100644 --- a/pkg/restore/restic_restore_action_test.go +++ b/pkg/restore/restic_restore_action_test.go @@ -173,12 +173,14 @@ func TestResticRestoreActionExecute(t *testing.T) { podVolumeBackups: []*velerov1api.PodVolumeBackup{ builder.ForPodVolumeBackup(veleroNs, "pvb-1"). PodName("my-pod"). + PodNamespace("ns-1"). Volume("vol-1"). ObjectMeta(builder.WithLabels(velerov1api.BackupNameLabel, backupName)). SnapshotID("foo"). Result(), builder.ForPodVolumeBackup(veleroNs, "pvb-2"). PodName("my-pod"). + PodNamespace("ns-1"). Volume("vol-2"). ObjectMeta(builder.WithLabels(velerov1api.BackupNameLabel, backupName)). SnapshotID("foo"). diff --git a/pkg/restore/restore_test.go b/pkg/restore/restore_test.go index a2933107604..ee8db6bf485 100644 --- a/pkg/restore/restore_test.go +++ b/pkg/restore/restore_test.go @@ -2435,14 +2435,14 @@ func TestRestoreWithRestic(t *testing.T) { want map[*test.APIResource][]string }{ { - name: "a pod that exists in given backup and contains associated PVBs should have should have RestorePodVolumes called", + name: "a pod that exists in given backup and contains associated PVBs should have RestorePodVolumes called", restore: defaultRestore().Result(), backup: defaultBackup().Result(), apiResources: []*test.APIResource{test.Pods()}, podVolumeBackups: []*velerov1api.PodVolumeBackup{ 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(), + builder.ForPodVolumeBackup("velero", "pvb-2").PodName("pod-2").PodNamespace("ns-1").SnapshotID("foo").Result(), + builder.ForPodVolumeBackup("velero", "pvb-3").PodName("pod-4").PodNamespace("ns-2").SnapshotID("foo").Result(), }, podWithPVBs: []*corev1api.Pod{ builder.ForPod("ns-1", "pod-2").