Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 Use namespace and name to match PVB to Pod restore #3051

Merged
merged 1 commit into from
Nov 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelogs/unreleased/3051-ashish-amarnath
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
🐛 Use namespace and name to match PVB to Pod restore
6 changes: 6 additions & 0 deletions pkg/builder/pod_volume_backup_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 5 additions & 1 deletion pkg/restic/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,13 +95,17 @@ func getPodSnapshotAnnotations(obj metav1.Object) map[string]string {
return res
}

func isPVBMatchPod(pvb *velerov1api.PodVolumeBackup, pod metav1.Object) bool {
ashish-amarnath marked this conversation as resolved.
Show resolved Hide resolved
return pod.GetName() == pvb.Spec.Pod.Name && pod.GetNamespace() == pvb.Spec.Pod.Namespace
nrb marked this conversation as resolved.
Show resolved Hide resolved
}

// 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
}

Expand Down
82 changes: 82 additions & 0 deletions pkg/restic/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions pkg/restore/restic_restore_action_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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").
Expand Down
6 changes: 3 additions & 3 deletions pkg/restore/restore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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").
Expand Down