From edeec848d3600bfd8320f9399081d34fa0e5b12f Mon Sep 17 00:00:00 2001 From: Bridget McErlean Date: Wed, 1 Sep 2021 01:51:44 -0400 Subject: [PATCH] Skip restic backup/restore of DownwardAPI volumes (#4076) Velero was including DownwardAPI volumes when backing up with restic. When restoring these volumes, it triggered a known issue with restic (as seen in #3863). Like projected volumes, these volumes should be skipped as their contents are populated by the Kubernetes API server. With this change, we are now skipping the restic backup of volumes with a DownwardAPI source. We are also skipping the restore of any volume that had a DownwardAPI source as there will exist backups that were taken prior to this fix being introduced. This will allow these backups to be restored succesfully. Signed-off-by: Bridget McErlean --- changelogs/unreleased/4076-zubron | 1 + pkg/restic/common.go | 27 ++++--- pkg/restic/common_test.go | 114 ++++++++++++++++++++++++++++-- 3 files changed, 125 insertions(+), 17 deletions(-) create mode 100644 changelogs/unreleased/4076-zubron diff --git a/changelogs/unreleased/4076-zubron b/changelogs/unreleased/4076-zubron new file mode 100644 index 000000000..386675f4e --- /dev/null +++ b/changelogs/unreleased/4076-zubron @@ -0,0 +1 @@ +Skip the backup and restore of DownwardAPI volumes when using restic. \ No newline at end of file diff --git a/pkg/restic/common.go b/pkg/restic/common.go index e122ff4bb..e8648cd6c 100644 --- a/pkg/restic/common.go +++ b/pkg/restic/common.go @@ -100,15 +100,18 @@ func isPVBMatchPod(pvb *velerov1api.PodVolumeBackup, podName string, namespace s return podName == pvb.Spec.Pod.Name && namespace == pvb.Spec.Pod.Namespace } -// volumeIsProjected checks if the given volume exists in the list of podVolumes -// and returns true if the volume has a projected source -func volumeIsProjected(volumeName string, podVolumes []corev1api.Volume) bool { - for _, volume := range podVolumes { - if volume.Name == volumeName && volume.Projected != nil { - return true +// volumeHasNonRestorableSource checks if the given volume exists in the list of podVolumes +// and returns true if the volume's source is not restorable. This is true for volumes with +// a Projected or DownwardAPI source. +func volumeHasNonRestorableSource(volumeName string, podVolumes []corev1api.Volume) bool { + var volume corev1api.Volume + for _, v := range podVolumes { + if v.Name == volumeName { + volume = v + break } } - return false + return volume.Projected != nil || volume.DownwardAPI != nil } // GetVolumeBackupsForPod returns a map, of volume name -> snapshot id, @@ -127,10 +130,10 @@ func GetVolumeBackupsForPod(podVolumeBackups []*velerov1api.PodVolumeBackup, pod continue } - // If the volume came from a projected source, skip its restore. + // If the volume came from a projected or DownwardAPI source, skip its restore. // This allows backups affected by https://github.com/vmware-tanzu/velero/issues/3863 - // to be restored successfully. - if volumeIsProjected(pvb.Spec.Volume, pod.Spec.Volumes) { + // or https://github.com/vmware-tanzu/velero/issues/4053 to be restored successfully. + if volumeHasNonRestorableSource(pvb.Spec.Volume, pod.Spec.Volumes) { continue } @@ -205,6 +208,10 @@ func GetPodVolumesUsingRestic(pod *corev1api.Pod, defaultVolumesToRestic bool) [ if pv.Projected != nil { continue } + // don't backup DownwardAPI volumes, all data in those come from kube state. + if pv.DownwardAPI != nil { + continue + } // don't backup volumes that are included in the exclude list. if contains(volsToExclude, pv.Name) { continue diff --git a/pkg/restic/common_test.go b/pkg/restic/common_test.go index 7e102ca97..954defee0 100644 --- a/pkg/restic/common_test.go +++ b/pkg/restic/common_test.go @@ -152,6 +152,30 @@ func TestGetVolumeBackupsForPod(t *testing.T) { sourcePodNs: "TestNS", expected: map[string]string{"pvb-non-projected": "snapshot1"}, }, + { + name: "volumes from PVBs that correspond to a pod volume from a DownwardAPI source are not returned", + podVolumeBackups: []*velerov1api.PodVolumeBackup{ + builder.ForPodVolumeBackup("velero", "pvb-1").PodName("TestPod").PodNamespace("TestNS").SnapshotID("snapshot1").Volume("pvb-non-downwardapi").Result(), + builder.ForPodVolumeBackup("velero", "pvb-1").PodName("TestPod").PodNamespace("TestNS").SnapshotID("snapshot2").Volume("pvb-downwardapi").Result(), + }, + podVolumes: []corev1api.Volume{ + { + Name: "pvb-non-downwardapi", + VolumeSource: corev1api.VolumeSource{ + PersistentVolumeClaim: &corev1api.PersistentVolumeClaimVolumeSource{}, + }, + }, + { + Name: "pvb-downwardapi", + VolumeSource: corev1api.VolumeSource{ + DownwardAPI: &corev1api.DownwardAPIVolumeSource{}, + }, + }, + }, + podName: "TestPod", + sourcePodNs: "TestNS", + expected: map[string]string{"pvb-non-downwardapi": "snapshot1"}, + }, } for _, test := range tests { @@ -568,6 +592,39 @@ func TestGetPodVolumesUsingRestic(t *testing.T) { }, expected: []string{"resticPV1", "resticPV2", "resticPV3"}, }, + { + name: "should exclude DownwardAPI volumes", + defaultVolumesToRestic: true, + pod: &corev1api.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + VolumesToExcludeAnnotation: "nonResticPV1,nonResticPV2,nonResticPV3", + }, + }, + Spec: corev1api.PodSpec{ + Volumes: []corev1api.Volume{ + {Name: "resticPV1"}, {Name: "resticPV2"}, {Name: "resticPV3"}, + { + Name: "downwardAPI", + VolumeSource: corev1api.VolumeSource{ + DownwardAPI: &corev1api.DownwardAPIVolumeSource{ + Items: []corev1api.DownwardAPIVolumeFile{ + { + Path: "labels", + FieldRef: &corev1api.ObjectFieldSelector{ + APIVersion: "v1", + FieldPath: "metadata.labels", + }, + }, + }, + }, + }, + }, + }, + }, + }, + expected: []string{"resticPV1", "resticPV2", "resticPV3"}, + }, } for _, tc := range testCases { @@ -656,7 +713,7 @@ func TestIsPVBMatchPod(t *testing.T) { } } -func TestVolumeIsProjected(t *testing.T) { +func TestVolumeHasNonRestorableSource(t *testing.T) { testCases := []struct { name string volumeName string @@ -668,7 +725,7 @@ func TestVolumeIsProjected(t *testing.T) { volumeName: "missing-volume", podVolumes: []corev1api.Volume{ { - Name: "non-projected", + Name: "restorable", VolumeSource: corev1api.VolumeSource{ PersistentVolumeClaim: &corev1api.PersistentVolumeClaimVolumeSource{}, }, @@ -679,15 +736,21 @@ func TestVolumeIsProjected(t *testing.T) { Projected: &corev1api.ProjectedVolumeSource{}, }, }, + { + Name: "downwardapi", + VolumeSource: corev1api.VolumeSource{ + DownwardAPI: &corev1api.DownwardAPIVolumeSource{}, + }, + }, }, expected: false, }, { - name: "volume name in list of volumes but not projected", - volumeName: "non-projected", + name: "volume name in list of volumes but not projected or DownwardAPI", + volumeName: "restorable", podVolumes: []corev1api.Volume{ { - Name: "non-projected", + Name: "restorable", VolumeSource: corev1api.VolumeSource{ PersistentVolumeClaim: &corev1api.PersistentVolumeClaimVolumeSource{}, }, @@ -698,6 +761,12 @@ func TestVolumeIsProjected(t *testing.T) { Projected: &corev1api.ProjectedVolumeSource{}, }, }, + { + Name: "downwardapi", + VolumeSource: corev1api.VolumeSource{ + DownwardAPI: &corev1api.DownwardAPIVolumeSource{}, + }, + }, }, expected: false, }, @@ -706,7 +775,7 @@ func TestVolumeIsProjected(t *testing.T) { volumeName: "projected", podVolumes: []corev1api.Volume{ { - Name: "non-projected", + Name: "restorable", VolumeSource: corev1api.VolumeSource{ PersistentVolumeClaim: &corev1api.PersistentVolumeClaimVolumeSource{}, }, @@ -717,6 +786,37 @@ func TestVolumeIsProjected(t *testing.T) { Projected: &corev1api.ProjectedVolumeSource{}, }, }, + { + Name: "downwardapi", + VolumeSource: corev1api.VolumeSource{ + DownwardAPI: &corev1api.DownwardAPIVolumeSource{}, + }, + }, + }, + expected: true, + }, + { + name: "volume name in list of volumes and is a DownwardAPI volume", + volumeName: "downwardapi", + podVolumes: []corev1api.Volume{ + { + Name: "restorable", + VolumeSource: corev1api.VolumeSource{ + PersistentVolumeClaim: &corev1api.PersistentVolumeClaimVolumeSource{}, + }, + }, + { + Name: "projected", + VolumeSource: corev1api.VolumeSource{ + Projected: &corev1api.ProjectedVolumeSource{}, + }, + }, + { + Name: "downwardapi", + VolumeSource: corev1api.VolumeSource{ + DownwardAPI: &corev1api.DownwardAPIVolumeSource{}, + }, + }, }, expected: true, }, @@ -724,7 +824,7 @@ func TestVolumeIsProjected(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - actual := volumeIsProjected(tc.volumeName, tc.podVolumes) + actual := volumeHasNonRestorableSource(tc.volumeName, tc.podVolumes) assert.Equal(t, tc.expected, actual) })