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