From 8a2a852b8768848f21dc9d44cbb1810854cc179a Mon Sep 17 00:00:00 2001 From: Ashish Amarnath Date: Wed, 3 Jun 2020 17:15:59 -0700 Subject: [PATCH] use backup's defaultRestic flag to identify pod volumes using restic Signed-off-by: Ashish Amarnath --- pkg/backup/item_backupper.go | 2 +- pkg/cmd/server/server.go | 1 + pkg/controller/backup_controller.go | 7 ++ pkg/controller/backup_controller_test.go | 93 +++++++++++++++++++++ pkg/restic/common.go | 51 ++++++++++++ pkg/restic/common_test.go | 102 +++++++++++++++++++++++ 6 files changed, 255 insertions(+), 1 deletion(-) diff --git a/pkg/backup/item_backupper.go b/pkg/backup/item_backupper.go index e0889ff0c..11691ef65 100644 --- a/pkg/backup/item_backupper.go +++ b/pkg/backup/item_backupper.go @@ -141,7 +141,7 @@ func (ib *itemBackupper) backupItem(logger logrus.FieldLogger, obj runtime.Unstr // Get the list of volumes to back up using restic from the pod's annotations. Remove from this list // any volumes that use a PVC that we've already backed up (this would be in a read-write-many scenario, // where it's been backed up from another pod), since we don't need >1 backup per PVC. - for _, volume := range restic.GetVolumesToBackup(pod) { + for _, volume := range restic.GetPodVolumesUsingRestic(pod, boolptr.IsSetToTrue(ib.backupRequest.Spec.DefaultRestic)) { if found, pvcName := ib.resticSnapshotTracker.HasPVCForPodVolume(pod, volume); found { log.WithFields(map[string]interface{}{ "podVolume": volume, diff --git a/pkg/cmd/server/server.go b/pkg/cmd/server/server.go index f4ecc866f..6861f4ce1 100644 --- a/pkg/cmd/server/server.go +++ b/pkg/cmd/server/server.go @@ -651,6 +651,7 @@ func (s *server) runControllers(defaultVolumeSnapshotLocations map[string]string backupTracker, s.sharedInformerFactory.Velero().V1().BackupStorageLocations().Lister(), s.config.defaultBackupLocation, + s.config.defaultRestic, s.config.defaultBackupTTL, s.sharedInformerFactory.Velero().V1().VolumeSnapshotLocations().Lister(), defaultVolumeSnapshotLocations, diff --git a/pkg/controller/backup_controller.go b/pkg/controller/backup_controller.go index 05f549df5..c9edf49f0 100644 --- a/pkg/controller/backup_controller.go +++ b/pkg/controller/backup_controller.go @@ -71,6 +71,7 @@ type backupController struct { backupTracker BackupTracker backupLocationLister velerov1listers.BackupStorageLocationLister defaultBackupLocation string + defaultRestic bool defaultBackupTTL time.Duration snapshotLocationLister velerov1listers.VolumeSnapshotLocationLister defaultSnapshotLocations map[string]string @@ -92,6 +93,7 @@ func NewBackupController( backupTracker BackupTracker, backupLocationLister velerov1listers.BackupStorageLocationLister, defaultBackupLocation string, + defaultRestic bool, defaultBackupTTL time.Duration, volumeSnapshotLocationLister velerov1listers.VolumeSnapshotLocationLister, defaultSnapshotLocations map[string]string, @@ -120,6 +122,7 @@ func NewBackupController( volumeSnapshotLister: volumeSnapshotLister, volumeSnapshotContentLister: volumeSnapshotContentLister, newBackupStore: persistence.NewObjectBackupStore, + defaultRestic: defaultRestic, } c.syncHandler = c.processBackup @@ -339,6 +342,10 @@ func (c *backupController) prepareBackupRequest(backup *velerov1api.Backup) *pkg request.Spec.StorageLocation = c.defaultBackupLocation } + if request.Spec.DefaultRestic == nil { + request.Spec.DefaultRestic = &c.defaultRestic + } + // add the storage location as a label for easy filtering later. if request.Labels == nil { request.Labels = make(map[string]string) diff --git a/pkg/controller/backup_controller_test.go b/pkg/controller/backup_controller_test.go index 87d457a93..9f6561ab7 100644 --- a/pkg/controller/backup_controller_test.go +++ b/pkg/controller/backup_controller_test.go @@ -48,6 +48,7 @@ import ( pluginmocks "github.com/vmware-tanzu/velero/pkg/plugin/mocks" "github.com/vmware-tanzu/velero/pkg/plugin/velero" velerotest "github.com/vmware-tanzu/velero/pkg/test" + "github.com/vmware-tanzu/velero/pkg/util/boolptr" "github.com/vmware-tanzu/velero/pkg/util/logging" ) @@ -342,6 +343,7 @@ func TestProcessBackupCompletions(t *testing.T) { name string backup *velerov1api.Backup backupLocation *velerov1api.BackupStorageLocation + defaultRestic bool expectedResult *velerov1api.Backup backupExists bool existenceCheckError error @@ -351,6 +353,7 @@ func TestProcessBackupCompletions(t *testing.T) { name: "backup with no backup location gets the default", backup: defaultBackup().Result(), backupLocation: defaultBackupLocation, + defaultRestic: true, expectedResult: &velerov1api.Backup{ TypeMeta: metav1.TypeMeta{ Kind: "Backup", @@ -370,6 +373,7 @@ func TestProcessBackupCompletions(t *testing.T) { }, Spec: velerov1api.BackupSpec{ StorageLocation: defaultBackupLocation.Name, + DefaultRestic: boolptr.True(), }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseCompleted, @@ -385,6 +389,7 @@ func TestProcessBackupCompletions(t *testing.T) { name: "backup with a specific backup location keeps it", backup: defaultBackup().StorageLocation("alt-loc").Result(), backupLocation: builder.ForBackupStorageLocation("velero", "alt-loc").Bucket("store-1").Result(), + defaultRestic: false, expectedResult: &velerov1api.Backup{ TypeMeta: metav1.TypeMeta{ Kind: "Backup", @@ -404,6 +409,7 @@ func TestProcessBackupCompletions(t *testing.T) { }, Spec: velerov1api.BackupSpec{ StorageLocation: "alt-loc", + DefaultRestic: boolptr.False(), }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseCompleted, @@ -422,6 +428,7 @@ func TestProcessBackupCompletions(t *testing.T) { Bucket("store-1"). AccessMode(velerov1api.BackupStorageLocationAccessModeReadWrite). Result(), + defaultRestic: true, expectedResult: &velerov1api.Backup{ TypeMeta: metav1.TypeMeta{ Kind: "Backup", @@ -441,6 +448,7 @@ func TestProcessBackupCompletions(t *testing.T) { }, Spec: velerov1api.BackupSpec{ StorageLocation: "read-write", + DefaultRestic: boolptr.True(), }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseCompleted, @@ -456,6 +464,7 @@ func TestProcessBackupCompletions(t *testing.T) { name: "backup with a TTL has expiration set", backup: defaultBackup().TTL(10 * time.Minute).Result(), backupLocation: defaultBackupLocation, + defaultRestic: false, expectedResult: &velerov1api.Backup{ TypeMeta: metav1.TypeMeta{ Kind: "Backup", @@ -476,6 +485,7 @@ func TestProcessBackupCompletions(t *testing.T) { Spec: velerov1api.BackupSpec{ TTL: metav1.Duration{Duration: 10 * time.Minute}, StorageLocation: defaultBackupLocation.Name, + DefaultRestic: boolptr.False(), }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseCompleted, @@ -492,6 +502,7 @@ func TestProcessBackupCompletions(t *testing.T) { backupExists: false, backup: defaultBackup().Result(), backupLocation: defaultBackupLocation, + defaultRestic: true, expectedResult: &velerov1api.Backup{ TypeMeta: metav1.TypeMeta{ Kind: "Backup", @@ -511,6 +522,83 @@ func TestProcessBackupCompletions(t *testing.T) { }, Spec: velerov1api.BackupSpec{ StorageLocation: defaultBackupLocation.Name, + DefaultRestic: boolptr.True(), + }, + Status: velerov1api.BackupStatus{ + Phase: velerov1api.BackupPhaseCompleted, + Version: 1, + FormatVersion: "1.1.0", + StartTimestamp: ×tamp, + CompletionTimestamp: ×tamp, + Expiration: ×tamp, + }, + }, + }, + { + name: "backup specifying a false value for 'DefaultRestic' keeps it", + backupExists: false, + backup: defaultBackup().DefaultRestic(false).Result(), + backupLocation: defaultBackupLocation, + // value set in the controller is different from that specified in the backup + defaultRestic: true, + expectedResult: &velerov1api.Backup{ + TypeMeta: metav1.TypeMeta{ + Kind: "Backup", + APIVersion: "velero.io/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: velerov1api.DefaultNamespace, + Name: "backup-1", + Annotations: map[string]string{ + "velero.io/source-cluster-k8s-major-version": "1", + "velero.io/source-cluster-k8s-minor-version": "16", + "velero.io/source-cluster-k8s-gitversion": "v1.16.4", + }, + Labels: map[string]string{ + "velero.io/storage-location": "loc-1", + }, + }, + Spec: velerov1api.BackupSpec{ + StorageLocation: defaultBackupLocation.Name, + DefaultRestic: boolptr.False(), + }, + Status: velerov1api.BackupStatus{ + Phase: velerov1api.BackupPhaseCompleted, + Version: 1, + FormatVersion: "1.1.0", + StartTimestamp: ×tamp, + CompletionTimestamp: ×tamp, + Expiration: ×tamp, + }, + }, + }, + { + name: "backup specifying a true value for 'DefaultRestic' keeps it", + backupExists: false, + backup: defaultBackup().DefaultRestic(true).Result(), + backupLocation: defaultBackupLocation, + // value set in the controller is different from that specified in the backup + defaultRestic: false, + expectedResult: &velerov1api.Backup{ + TypeMeta: metav1.TypeMeta{ + Kind: "Backup", + APIVersion: "velero.io/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: velerov1api.DefaultNamespace, + Name: "backup-1", + Annotations: map[string]string{ + "velero.io/source-cluster-k8s-major-version": "1", + "velero.io/source-cluster-k8s-minor-version": "16", + "velero.io/source-cluster-k8s-gitversion": "v1.16.4", + }, + Labels: map[string]string{ + "velero.io/storage-location": "loc-1", + }, + }, + Spec: velerov1api.BackupSpec{ + StorageLocation: defaultBackupLocation.Name, + DefaultRestic: boolptr.True(), }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseCompleted, @@ -529,6 +617,7 @@ func TestProcessBackupCompletions(t *testing.T) { backupExists: true, backup: defaultBackup().Result(), backupLocation: defaultBackupLocation, + defaultRestic: true, expectedResult: &velerov1api.Backup{ TypeMeta: metav1.TypeMeta{ Kind: "Backup", @@ -548,6 +637,7 @@ func TestProcessBackupCompletions(t *testing.T) { }, Spec: velerov1api.BackupSpec{ StorageLocation: defaultBackupLocation.Name, + DefaultRestic: boolptr.True(), }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFailed, @@ -564,6 +654,7 @@ func TestProcessBackupCompletions(t *testing.T) { backup: defaultBackup().Result(), existenceCheckError: errors.New("Backup already exists in object storage"), backupLocation: defaultBackupLocation, + defaultRestic: true, expectedResult: &velerov1api.Backup{ TypeMeta: metav1.TypeMeta{ Kind: "Backup", @@ -583,6 +674,7 @@ func TestProcessBackupCompletions(t *testing.T) { }, Spec: velerov1api.BackupSpec{ StorageLocation: defaultBackupLocation.Name, + DefaultRestic: boolptr.True(), }, Status: velerov1api.BackupStatus{ Phase: velerov1api.BackupPhaseFailed, @@ -633,6 +725,7 @@ func TestProcessBackupCompletions(t *testing.T) { backupLocationLister: sharedInformers.Velero().V1().BackupStorageLocations().Lister(), snapshotLocationLister: sharedInformers.Velero().V1().VolumeSnapshotLocations().Lister(), defaultBackupLocation: defaultBackupLocation.Name, + defaultRestic: test.defaultRestic, backupTracker: NewBackupTracker(), metrics: metrics.NewServerMetrics(), clock: clock.NewFakeClock(now), diff --git a/pkg/restic/common.go b/pkg/restic/common.go index 41f5c9f89..a73e95910 100644 --- a/pkg/restic/common.go +++ b/pkg/restic/common.go @@ -23,6 +23,7 @@ import ( "time" "github.com/pkg/errors" + corev1api "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" corev1listers "k8s.io/client-go/listers/core/v1" @@ -53,6 +54,10 @@ const ( // need to be backed up using restic. VolumesToBackupAnnotation = "backup.velero.io/backup-volumes" + // VolumesToExcludeAnnotation is the annotation on a pod whose mounted volumes + // should be excluded from restic backup. + VolumesToExcludeAnnotation = "backup.velero.io/backup-volumes-excludes" + // Deprecated. // // TODO(2.0): remove @@ -111,6 +116,7 @@ func GetVolumeBackupsForPod(podVolumeBackups []*velerov1api.PodVolumeBackup, pod // GetVolumesToBackup returns a list of volume names to backup for // the provided pod. +// Deprecated: Use GetPodVolumesUsingRestic instead. func GetVolumesToBackup(obj metav1.Object) []string { annotations := obj.GetAnnotations() if annotations == nil { @@ -125,6 +131,51 @@ func GetVolumesToBackup(obj metav1.Object) []string { return strings.Split(backupsValue, ",") } +func getVolumesToExclude(obj metav1.Object) []string { + annotations := obj.GetAnnotations() + if annotations == nil { + return nil + } + + return strings.Split(annotations[VolumesToExcludeAnnotation], ",") +} + +func contains(list []string, k string) bool { + for _, i := range list { + if i == k { + return true + } + } + return false +} + +// GetPodVolumesUsingRestic returns a list of volume names to backup for the provided pod. +func GetPodVolumesUsingRestic(pod *corev1api.Pod, defaultRestic bool) []string { + if !defaultRestic { + return GetVolumesToBackup(pod) + } + + volsToExclude := getVolumesToExclude(pod) + podVolumes := []string{} + for _, pv := range pod.Spec.Volumes { + // cannot backup hostpath volumes as they are not mounted into /var/lib/kubelet/pods + // and therefore not accessible to the restic daemon set. + if pv.HostPath != nil { + continue + } + // don't backup volumes that are included in the exclude list. + if contains(volsToExclude, pv.Name) { + continue + } + // don't include volumes that mount the default service account token. + if strings.HasPrefix(pv.Name, "default-token") { + continue + } + podVolumes = append(podVolumes, pv.Name) + } + return podVolumes +} + // SnapshotIdentifier uniquely identifies a restic snapshot // taken by Velero. type SnapshotIdentifier struct { diff --git a/pkg/restic/common_test.go b/pkg/restic/common_test.go index 2a73c6039..1e223a2e2 100644 --- a/pkg/restic/common_test.go +++ b/pkg/restic/common_test.go @@ -419,3 +419,105 @@ func TestTempCACertFile(t *testing.T) { os.Remove(fileName) } + +func TestGetPodVolumesUsingRestic(t *testing.T) { + testCases := []struct { + name string + pod *corev1api.Pod + expected []string + defaultRestic bool + }{ + { + name: "should get PVs from VolumesToBackupAnnotation when defaultRestic is false", + defaultRestic: false, + pod: &corev1api.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + VolumesToBackupAnnotation: "resticPV1,resticPV2,resticPV3", + }, + }, + }, + expected: []string{"resticPV1", "resticPV2", "resticPV3"}, + }, + { + name: "should get all pod volumes when defaultRestic is true and no PVs are excluded", + defaultRestic: true, + pod: &corev1api.Pod{ + Spec: corev1api.PodSpec{ + Volumes: []corev1api.Volume{ + // Restic Volumes + {Name: "resticPV1"}, {Name: "resticPV2"}, {Name: "resticPV3"}, + }, + }, + }, + expected: []string{"resticPV1", "resticPV2", "resticPV3"}, + }, + { + name: "should get all pod volumes except ones excluded when defaultRestic is true", + defaultRestic: true, + pod: &corev1api.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + VolumesToExcludeAnnotation: "nonResticPV1,nonResticPV2,nonResticPV3", + }, + }, + Spec: corev1api.PodSpec{ + Volumes: []corev1api.Volume{ + // Restic Volumes + {Name: "resticPV1"}, {Name: "resticPV2"}, {Name: "resticPV3"}, + /// Excluded from restic through annotation + {Name: "nonResticPV1"}, {Name: "nonResticPV2"}, {Name: "nonResticPV3"}, + }, + }, + }, + expected: []string{"resticPV1", "resticPV2", "resticPV3"}, + }, + { + name: "should exclude default service account token from restic backup", + defaultRestic: true, + pod: &corev1api.Pod{ + Spec: corev1api.PodSpec{ + Volumes: []corev1api.Volume{ + // Restic Volumes + {Name: "resticPV1"}, {Name: "resticPV2"}, {Name: "resticPV3"}, + /// Excluded from restic because colume mounting default service account token + {Name: "default-token-5xq45"}, + }, + }, + }, + expected: []string{"resticPV1", "resticPV2", "resticPV3"}, + }, + { + name: "should exclude host path volumes from restic backups", + defaultRestic: true, + pod: &corev1api.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + VolumesToExcludeAnnotation: "nonResticPV1,nonResticPV2,nonResticPV3", + }, + }, + Spec: corev1api.PodSpec{ + Volumes: []corev1api.Volume{ + // Restic Volumes + {Name: "resticPV1"}, {Name: "resticPV2"}, {Name: "resticPV3"}, + /// Excluded from restic through annotation + {Name: "nonResticPV1"}, {Name: "nonResticPV2"}, {Name: "nonResticPV3"}, + // Excluded from restic because hostpath + {Name: "hostPath1", VolumeSource: corev1api.VolumeSource{HostPath: &corev1api.HostPathVolumeSource{Path: "/hostpathVol"}}}, + }, + }, + }, + expected: []string{"resticPV1", "resticPV2", "resticPV3"}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + actual := GetPodVolumesUsingRestic(tc.pod, tc.defaultRestic) + + sort.Strings(tc.expected) + sort.Strings(actual) + assert.Equal(t, tc.expected, actual) + }) + } +}