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 <[email protected]>
vmware-tanzu · Sep 1, 2021 · edeec84 · edeec84
1 parent 746cd61
commit edeec84
Show file tree

Hide file tree

Showing 3 changed files with 125 additions and 17 deletions.
diff --git a/changelogs/unreleased/4076-zubron b/changelogs/unreleased/4076-zubron
@@ -0,0 +1 @@
+Skip the backup and restore of DownwardAPI volumes when using restic.
diff --git 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
@@ -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,14 +786,45 @@ 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,
 		},
 	}
 
 	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)
 		})
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		Skip the backup and restore of DownwardAPI volumes when using restic.