🐛 Use namespace and name to match PVB to Pod restore (vmware-tanzu#3051)

Signed-off-by: Ashish Amarnath <[email protected]>

changelogs/unreleased/3051-ashish-amarnath

@@ -0,0 +1 @@
+🐛  Use namespace and name to match PVB to Pod restore

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

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
 		}
 

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)

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").

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").