diff --git a/changelogs/unreleased/1517-skriss b/changelogs/unreleased/1517-skriss new file mode 100644 index 000000000..7dce5518d --- /dev/null +++ b/changelogs/unreleased/1517-skriss @@ -0,0 +1 @@ +Allow individual backup storage locations to be read-only diff --git a/pkg/apis/velero/v1/backup_storage_location.go b/pkg/apis/velero/v1/backup_storage_location.go index bc2cc41ff..24e1e2585 100644 --- a/pkg/apis/velero/v1/backup_storage_location.go +++ b/pkg/apis/velero/v1/backup_storage_location.go @@ -66,6 +66,9 @@ type BackupStorageLocationSpec struct { Config map[string]string `json:"config"` StorageType `json:",inline"` + + // AccessMode defines the permissions for the backup storage location. + AccessMode BackupStorageLocationAccessMode `json:"accessMode,omitempty"` } // BackupStorageLocationPhase is the lifecyle phase of a Velero BackupStorageLocation. @@ -90,10 +93,17 @@ const ( BackupStorageLocationAccessModeReadWrite BackupStorageLocationAccessMode = "ReadWrite" ) +// TODO(2.0): remove the AccessMode field from BackupStorageLocationStatus. + // BackupStorageLocationStatus describes the current status of a Velero BackupStorageLocation. type BackupStorageLocationStatus struct { - Phase BackupStorageLocationPhase `json:"phase,omitempty"` - AccessMode BackupStorageLocationAccessMode `json:"accessMode,omitempty"` - LastSyncedRevision types.UID `json:"lastSyncedRevision,omitempty"` - LastSyncedTime metav1.Time `json:"lastSyncedTime,omitempty"` + Phase BackupStorageLocationPhase `json:"phase,omitempty"` + LastSyncedRevision types.UID `json:"lastSyncedRevision,omitempty"` + LastSyncedTime metav1.Time `json:"lastSyncedTime,omitempty"` + + // AccessMode is an unused field. + // + // Deprecated: there is now an AccessMode field on the Spec and this field + // will be removed entirely as of v2.0. + AccessMode BackupStorageLocationAccessMode `json:"accessMode,omitempty"` } diff --git a/pkg/cmd/cli/backuplocation/create.go b/pkg/cmd/cli/backuplocation/create.go index 3a9de95e6..3df861bff 100644 --- a/pkg/cmd/cli/backuplocation/create.go +++ b/pkg/cmd/cli/backuplocation/create.go @@ -18,13 +18,14 @@ package backuplocation import ( "fmt" + "strings" "github.com/pkg/errors" "github.com/spf13/cobra" "github.com/spf13/pflag" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - api "github.com/heptio/velero/pkg/apis/velero/v1" + velerov1api "github.com/heptio/velero/pkg/apis/velero/v1" "github.com/heptio/velero/pkg/client" "github.com/heptio/velero/pkg/cmd" "github.com/heptio/velero/pkg/cmd/util/flag" @@ -53,17 +54,23 @@ func NewCreateCommand(f client.Factory, use string) *cobra.Command { } type CreateOptions struct { - Name string - Provider string - Bucket string - Prefix string - Config flag.Map - Labels flag.Map + Name string + Provider string + Bucket string + Prefix string + Config flag.Map + Labels flag.Map + AccessMode *flag.Enum } func NewCreateOptions() *CreateOptions { return &CreateOptions{ Config: flag.NewMap(), + AccessMode: flag.NewEnum( + string(velerov1api.BackupStorageLocationAccessModeReadWrite), + string(velerov1api.BackupStorageLocationAccessModeReadWrite), + string(velerov1api.BackupStorageLocationAccessModeReadOnly), + ), } } @@ -73,6 +80,11 @@ func (o *CreateOptions) BindFlags(flags *pflag.FlagSet) { flags.StringVar(&o.Prefix, "prefix", o.Prefix, "prefix under which all Velero data should be stored within the bucket. Optional.") flags.Var(&o.Config, "config", "configuration key-value pairs") flags.Var(&o.Labels, "labels", "labels to apply to the backup storage location") + flags.Var( + o.AccessMode, + "access-mode", + fmt.Sprintf("access mode for the backup storage location. Valid values are %s", strings.Join(o.AccessMode.AllowedValues(), ",")), + ) } func (o *CreateOptions) Validate(c *cobra.Command, args []string, f client.Factory) error { @@ -97,21 +109,22 @@ func (o *CreateOptions) Complete(args []string, f client.Factory) error { } func (o *CreateOptions) Run(c *cobra.Command, f client.Factory) error { - backupStorageLocation := &api.BackupStorageLocation{ + backupStorageLocation := &velerov1api.BackupStorageLocation{ ObjectMeta: metav1.ObjectMeta{ Namespace: f.Namespace(), Name: o.Name, Labels: o.Labels.Data(), }, - Spec: api.BackupStorageLocationSpec{ + Spec: velerov1api.BackupStorageLocationSpec{ Provider: o.Provider, - StorageType: api.StorageType{ - ObjectStorage: &api.ObjectStorageLocation{ + StorageType: velerov1api.StorageType{ + ObjectStorage: &velerov1api.ObjectStorageLocation{ Bucket: o.Bucket, Prefix: o.Prefix, }, }, - Config: o.Config.Data(), + Config: o.Config.Data(), + AccessMode: velerov1api.BackupStorageLocationAccessMode(o.AccessMode.String()), }, } diff --git a/pkg/cmd/server/server.go b/pkg/cmd/server/server.go index 13e35fef3..90c3dfe59 100644 --- a/pkg/cmd/server/server.go +++ b/pkg/cmd/server/server.go @@ -194,7 +194,7 @@ func NewCommand() *cobra.Command { command.Flags().StringVar(&config.metricsAddress, "metrics-address", config.metricsAddress, "the address to expose prometheus metrics") command.Flags().DurationVar(&config.backupSyncPeriod, "backup-sync-period", config.backupSyncPeriod, "how often to ensure all Velero backups in object storage exist as Backup API objects in the cluster") command.Flags().DurationVar(&config.podVolumeOperationTimeout, "restic-timeout", config.podVolumeOperationTimeout, "how long backups/restores of pod volumes should be allowed to run before timing out") - command.Flags().BoolVar(&config.restoreOnly, "restore-only", config.restoreOnly, "run in a mode where only restores are allowed; backups, schedules, and garbage-collection are all disabled") + command.Flags().BoolVar(&config.restoreOnly, "restore-only", config.restoreOnly, "run in a mode where only restores are allowed; backups, schedules, and garbage-collection are all disabled. DEPRECATED: this flag will be removed in v2.0. Use read-only backup storage locations instead.") command.Flags().StringSliceVar(&config.disabledControllers, "disable-controllers", config.disabledControllers, fmt.Sprintf("list of controllers to disable on startup. Valid values are %s", strings.Join(disableControllerList, ","))) command.Flags().StringSliceVar(&config.restoreResourcePriorities, "restore-resource-priorities", config.restoreResourcePriorities, "desired order of resource restores; any resource not in the list will be restored alphabetically after the prioritized resources") command.Flags().StringVar(&config.defaultBackupLocation, "default-backup-storage-location", config.defaultBackupLocation, "name of the default backup storage location") @@ -629,6 +629,7 @@ func (s *server) runControllers(defaultVolumeSnapshotLocations map[string]string s.sharedInformerFactory.Velero().V1().Backups(), s.sharedInformerFactory.Velero().V1().DeleteBackupRequests(), s.veleroClient.VeleroV1(), + s.sharedInformerFactory.Velero().V1().BackupStorageLocations(), ) return controllerRunInfo{ diff --git a/pkg/cmd/util/output/backup_storage_location_printer.go b/pkg/cmd/util/output/backup_storage_location_printer.go index 0d444e742..47dd8555b 100644 --- a/pkg/cmd/util/output/backup_storage_location_printer.go +++ b/pkg/cmd/util/output/backup_storage_location_printer.go @@ -26,7 +26,7 @@ import ( ) var ( - backupStorageLocationColumns = []string{"NAME", "PROVIDER", "BUCKET/PREFIX"} + backupStorageLocationColumns = []string{"NAME", "PROVIDER", "BUCKET/PREFIX", "ACCESS MODE"} ) func printBackupStorageLocationList(list *v1.BackupStorageLocationList, w io.Writer, options printers.PrintOptions) error { @@ -52,12 +52,18 @@ func printBackupStorageLocation(location *v1.BackupStorageLocation, w io.Writer, bucketAndPrefix += "/" + location.Spec.ObjectStorage.Prefix } + accessMode := location.Spec.AccessMode + if accessMode == "" { + accessMode = v1.BackupStorageLocationAccessModeReadWrite + } + if _, err := fmt.Fprintf( w, - "%s\t%s\t%s", + "%s\t%s\t%s\t%s", name, location.Spec.Provider, bucketAndPrefix, + accessMode, ); err != nil { return err } diff --git a/pkg/controller/backup_controller.go b/pkg/controller/backup_controller.go index 74397ea03..def1dac13 100644 --- a/pkg/controller/backup_controller.go +++ b/pkg/controller/backup_controller.go @@ -318,6 +318,11 @@ func (c *backupController) prepareBackupRequest(backup *velerov1api.Backup) *pkg } } else { request.StorageLocation = storageLocation + + if request.StorageLocation.Spec.AccessMode == velerov1api.BackupStorageLocationAccessModeReadOnly { + request.Status.ValidationErrors = append(request.Status.ValidationErrors, + fmt.Sprintf("backup can't be created because backup storage location %s is currently in read-only mode", request.StorageLocation.Name)) + } } // validate and get the backup's VolumeSnapshotLocations, and store the diff --git a/pkg/controller/backup_controller_test.go b/pkg/controller/backup_controller_test.go index 76b5e1c5b..d032ea447 100644 --- a/pkg/controller/backup_controller_test.go +++ b/pkg/controller/backup_controller_test.go @@ -1,5 +1,5 @@ /* -Copyright 2017 the Velero contributors. +Copyright 2017, 2019 the Velero contributors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -146,6 +146,12 @@ func TestProcessBackupValidationFailures(t *testing.T) { backup: velerotest.NewTestBackup().WithName("backup-1").WithStorageLocation("nonexistent").Backup, expectedErrs: []string{"a BackupStorageLocation CRD with the name specified in the backup spec needs to be created before this backup can be executed. Error: backupstoragelocation.velero.io \"nonexistent\" not found"}, }, + { + name: "backup for read-only backup location fails validation", + backup: velerotest.NewTestBackup().WithName("backup-1").WithStorageLocation("read-only").Backup, + backupLocation: velerotest.NewTestBackupStorageLocation().WithName("read-only").WithAccessMode(velerov1api.BackupStorageLocationAccessModeReadOnly).BackupStorageLocation, + expectedErrs: []string{"backup can't be created because backup storage location read-only is currently in read-only mode"}, + }, } for _, test := range tests { @@ -358,6 +364,34 @@ func TestProcessBackupCompletions(t *testing.T) { }, }, }, + { + name: "backup for a location with ReadWrite access mode gets processed", + backup: velerotest.NewTestBackup().WithName("backup-1").WithStorageLocation("read-write").Backup, + backupLocation: velerotest.NewTestBackupStorageLocation(). + WithName("read-write"). + WithObjectStorage("store-1"). + WithAccessMode(v1.BackupStorageLocationAccessModeReadWrite). + BackupStorageLocation, + expectedResult: &v1.Backup{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: v1.DefaultNamespace, + Name: "backup-1", + Labels: map[string]string{ + "velero.io/storage-location": "read-write", + }, + }, + Spec: v1.BackupSpec{ + StorageLocation: "read-write", + }, + Status: v1.BackupStatus{ + Phase: v1.BackupPhaseCompleted, + Version: 1, + StartTimestamp: metav1.NewTime(now), + CompletionTimestamp: metav1.NewTime(now), + Expiration: metav1.NewTime(now), + }, + }, + }, { name: "backup with a TTL has expiration set", backup: velerotest.NewTestBackup().WithName("backup-1").WithTTL(10 * time.Minute).Backup, diff --git a/pkg/controller/backup_deletion_controller.go b/pkg/controller/backup_deletion_controller.go index 8deb9be20..7c648f3a9 100644 --- a/pkg/controller/backup_deletion_controller.go +++ b/pkg/controller/backup_deletion_controller.go @@ -19,9 +19,10 @@ package controller import ( "context" "encoding/json" + "fmt" "time" - "github.com/evanphx/json-patch" + jsonpatch "github.com/evanphx/json-patch" "github.com/pkg/errors" "github.com/sirupsen/logrus" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -32,7 +33,7 @@ import ( kubeerrs "k8s.io/apimachinery/pkg/util/errors" "k8s.io/client-go/tools/cache" - "github.com/heptio/velero/pkg/apis/velero/v1" + v1 "github.com/heptio/velero/pkg/apis/velero/v1" pkgbackup "github.com/heptio/velero/pkg/backup" velerov1client "github.com/heptio/velero/pkg/generated/clientset/versioned/typed/velero/v1" informers "github.com/heptio/velero/pkg/generated/informers/externalversions/velero/v1" @@ -192,18 +193,6 @@ func (c *backupDeletionController) processRequest(req *v1.DeleteBackupRequest) e return err } - // Update status to InProgress and set backup-name label if needed - req, err = c.patchDeleteBackupRequest(req, func(r *v1.DeleteBackupRequest) { - r.Status.Phase = v1.DeleteBackupRequestPhaseInProgress - - if req.Labels[v1.BackupNameLabel] == "" { - req.Labels[v1.BackupNameLabel] = label.GetValidName(req.Spec.BackupName) - } - }) - if err != nil { - return err - } - // Get the backup we're trying to delete backup, err := c.backupClient.Backups(req.Namespace).Get(req.Spec.BackupName, metav1.GetOptions{}) if apierrors.IsNotFound(err) { @@ -216,7 +205,40 @@ func (c *backupDeletionController) processRequest(req *v1.DeleteBackupRequest) e return err } if err != nil { - return errors.Wrap(err, "error getting Backup") + return errors.Wrap(err, "error getting backup") + } + + // Don't allow deleting backups in read-only storage locations + location, err := c.backupLocationLister.BackupStorageLocations(backup.Namespace).Get(backup.Spec.StorageLocation) + if apierrors.IsNotFound(err) { + _, err := c.patchDeleteBackupRequest(req, func(r *v1.DeleteBackupRequest) { + r.Status.Phase = v1.DeleteBackupRequestPhaseProcessed + r.Status.Errors = append(r.Status.Errors, fmt.Sprintf("backup storage location %s not found", backup.Spec.StorageLocation)) + }) + return err + } + if err != nil { + return errors.Wrap(err, "error getting backup storage location") + } + + if location.Spec.AccessMode == v1.BackupStorageLocationAccessModeReadOnly { + _, err := c.patchDeleteBackupRequest(req, func(r *v1.DeleteBackupRequest) { + r.Status.Phase = v1.DeleteBackupRequestPhaseProcessed + r.Status.Errors = append(r.Status.Errors, fmt.Sprintf("cannot delete backup because backup storage location %s is currently in read-only mode", location.Name)) + }) + return err + } + + // Update status to InProgress and set backup-name label if needed + req, err = c.patchDeleteBackupRequest(req, func(r *v1.DeleteBackupRequest) { + r.Status.Phase = v1.DeleteBackupRequestPhaseInProgress + + if req.Labels[v1.BackupNameLabel] == "" { + req.Labels[v1.BackupNameLabel] = label.GetValidName(req.Spec.BackupName) + } + }) + if err != nil { + return err } // Set backup-uid label if needed @@ -246,9 +268,9 @@ func (c *backupDeletionController) processRequest(req *v1.DeleteBackupRequest) e pluginManager := c.newPluginManager(log) defer pluginManager.CleanupClients() - backupStore, backupStoreErr := c.backupStoreForBackup(backup, pluginManager, log) - if backupStoreErr != nil { - errs = append(errs, backupStoreErr.Error()) + backupStore, err := c.newBackupStore(location, pluginManager, log) + if err != nil { + errs = append(errs, err.Error()) } if backupStore != nil { @@ -375,19 +397,6 @@ func volumeSnapshotterForSnapshotLocation( return volumeSnapshotter, nil } -func (c *backupDeletionController) backupStoreForBackup(backup *v1.Backup, pluginManager clientmgmt.Manager, log logrus.FieldLogger) (persistence.BackupStore, error) { - backupLocation, err := c.backupLocationLister.BackupStorageLocations(backup.Namespace).Get(backup.Spec.StorageLocation) - if err != nil { - return nil, errors.WithStack(err) - } - - backupStore, err := c.newBackupStore(backupLocation, pluginManager, log) - if err != nil { - return nil, err - } - - return backupStore, nil -} func (c *backupDeletionController) deleteExistingDeletionRequests(req *v1.DeleteBackupRequest, log logrus.FieldLogger) []error { log.Info("Removing existing deletion requests for backup") selector := labels.SelectorFromSet(labels.Set(map[string]string{ diff --git a/pkg/controller/backup_deletion_controller_test.go b/pkg/controller/backup_deletion_controller_test.go index 0c26cbd10..90b02a40d 100644 --- a/pkg/controller/backup_deletion_controller_test.go +++ b/pkg/controller/backup_deletion_controller_test.go @@ -1,5 +1,5 @@ /* -Copyright 2018 the Velero contributors. +Copyright 2018, 2019 the Velero contributors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -25,7 +25,6 @@ import ( "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -267,7 +266,12 @@ func TestBackupDeletionControllerProcessRequest(t *testing.T) { }) t.Run("patching to InProgress fails", func(t *testing.T) { - td := setupBackupDeletionControllerTest() + backup := velerotest.NewTestBackup().WithName("foo").WithStorageLocation("default").Backup + location := velerotest.NewTestBackupStorageLocation().WithName("default").BackupStorageLocation + + td := setupBackupDeletionControllerTest(backup) + + td.sharedInformers.Velero().V1().BackupStorageLocations().Informer().GetStore().Add(location) td.client.PrependReactor("patch", "deletebackuprequests", func(action core.Action) (bool, runtime.Object, error) { return true, nil, errors.New("bad") @@ -275,12 +279,32 @@ func TestBackupDeletionControllerProcessRequest(t *testing.T) { err := td.controller.processRequest(td.req) assert.EqualError(t, err, "error patching DeleteBackupRequest: bad") + + expectedActions := []core.Action{ + core.NewGetAction( + v1.SchemeGroupVersion.WithResource("backups"), + backup.Namespace, + backup.Name, + ), + core.NewPatchAction( + v1.SchemeGroupVersion.WithResource("deletebackuprequests"), + td.req.Namespace, + td.req.Name, + types.MergePatchType, + []byte(`{"status":{"phase":"InProgress"}}`), + ), + } + assert.Equal(t, expectedActions, td.client.Actions()) }) t.Run("patching backup to Deleting fails", func(t *testing.T) { - backup := velerotest.NewTestBackup().WithName("foo").Backup + backup := velerotest.NewTestBackup().WithName("foo").WithStorageLocation("default").Backup + location := velerotest.NewTestBackupStorageLocation().WithName("default").BackupStorageLocation + td := setupBackupDeletionControllerTest(backup) + td.sharedInformers.Velero().V1().BackupStorageLocations().Informer().GetStore().Add(location) + td.client.PrependReactor("patch", "deletebackuprequests", func(action core.Action) (bool, runtime.Object, error) { return true, td.req, nil }) @@ -290,23 +314,13 @@ func TestBackupDeletionControllerProcessRequest(t *testing.T) { err := td.controller.processRequest(td.req) assert.EqualError(t, err, "error patching Backup: bad") - }) - - t.Run("unable to find backup", func(t *testing.T) { - td := setupBackupDeletionControllerTest() - - td.client.PrependReactor("get", "backups", func(action core.Action) (bool, runtime.Object, error) { - return true, nil, apierrors.NewNotFound(v1.SchemeGroupVersion.WithResource("backups").GroupResource(), "foo") - }) - - td.client.PrependReactor("patch", "deletebackuprequests", func(action core.Action) (bool, runtime.Object, error) { - return true, td.req, nil - }) - - err := td.controller.processRequest(td.req) - require.NoError(t, err) expectedActions := []core.Action{ + core.NewGetAction( + v1.SchemeGroupVersion.WithResource("backups"), + backup.Namespace, + backup.Name, + ), core.NewPatchAction( v1.SchemeGroupVersion.WithResource("deletebackuprequests"), td.req.Namespace, @@ -314,6 +328,24 @@ func TestBackupDeletionControllerProcessRequest(t *testing.T) { types.MergePatchType, []byte(`{"status":{"phase":"InProgress"}}`), ), + core.NewPatchAction( + v1.SchemeGroupVersion.WithResource("backups"), + backup.Namespace, + backup.Name, + types.MergePatchType, + []byte(`{"status":{"phase":"Deleting"}}`), + ), + } + assert.Equal(t, expectedActions, td.client.Actions()) + }) + + t.Run("unable to find backup", func(t *testing.T) { + td := setupBackupDeletionControllerTest() + + err := td.controller.processRequest(td.req) + require.NoError(t, err) + + expectedActions := []core.Action{ core.NewGetAction( v1.SchemeGroupVersion.WithResource("backups"), td.req.Namespace, @@ -331,6 +363,61 @@ func TestBackupDeletionControllerProcessRequest(t *testing.T) { assert.Equal(t, expectedActions, td.client.Actions()) }) + t.Run("unable to find backup storage location", func(t *testing.T) { + backup := velerotest.NewTestBackup().WithName("foo").WithStorageLocation("default").Backup + + td := setupBackupDeletionControllerTest(backup) + + err := td.controller.processRequest(td.req) + require.NoError(t, err) + + expectedActions := []core.Action{ + core.NewGetAction( + v1.SchemeGroupVersion.WithResource("backups"), + td.req.Namespace, + td.req.Spec.BackupName, + ), + core.NewPatchAction( + v1.SchemeGroupVersion.WithResource("deletebackuprequests"), + td.req.Namespace, + td.req.Name, + types.MergePatchType, + []byte(`{"status":{"errors":["backup storage location default not found"],"phase":"Processed"}}`), + ), + } + + assert.Equal(t, expectedActions, td.client.Actions()) + }) + + t.Run("backup storage location is in read-only mode", func(t *testing.T) { + backup := velerotest.NewTestBackup().WithName("foo").WithStorageLocation("default").Backup + location := velerotest.NewTestBackupStorageLocation().WithName("default").WithAccessMode(v1.BackupStorageLocationAccessModeReadOnly).BackupStorageLocation + + td := setupBackupDeletionControllerTest(backup) + + td.sharedInformers.Velero().V1().BackupStorageLocations().Informer().GetStore().Add(location) + + err := td.controller.processRequest(td.req) + require.NoError(t, err) + + expectedActions := []core.Action{ + core.NewGetAction( + v1.SchemeGroupVersion.WithResource("backups"), + td.req.Namespace, + td.req.Spec.BackupName, + ), + core.NewPatchAction( + v1.SchemeGroupVersion.WithResource("deletebackuprequests"), + td.req.Namespace, + td.req.Name, + types.MergePatchType, + []byte(`{"status":{"errors":["cannot delete backup because backup storage location default is currently in read-only mode"],"phase":"Processed"}}`), + ), + } + + assert.Equal(t, expectedActions, td.client.Actions()) + }) + t.Run("full delete, no errors", func(t *testing.T) { backup := velerotest.NewTestBackup().WithName("foo").Backup backup.UID = "uid" diff --git a/pkg/controller/gc_controller.go b/pkg/controller/gc_controller.go index 081a8e631..3f03dbbb0 100644 --- a/pkg/controller/gc_controller.go +++ b/pkg/controller/gc_controller.go @@ -45,6 +45,7 @@ type gcController struct { backupLister listers.BackupLister deleteBackupRequestLister listers.DeleteBackupRequestLister deleteBackupRequestClient velerov1client.DeleteBackupRequestsGetter + backupLocationLister listers.BackupStorageLocationLister clock clock.Clock } @@ -55,6 +56,7 @@ func NewGCController( backupInformer informers.BackupInformer, deleteBackupRequestInformer informers.DeleteBackupRequestInformer, deleteBackupRequestClient velerov1client.DeleteBackupRequestsGetter, + backupLocationInformer informers.BackupStorageLocationInformer, ) Interface { c := &gcController{ genericController: newGenericController("gc-controller", logger), @@ -62,12 +64,14 @@ func NewGCController( backupLister: backupInformer.Lister(), deleteBackupRequestLister: deleteBackupRequestInformer.Lister(), deleteBackupRequestClient: deleteBackupRequestClient, + backupLocationLister: backupLocationInformer.Lister(), } c.syncHandler = c.processQueueItem c.cacheSyncWaiters = append(c.cacheSyncWaiters, backupInformer.Informer().HasSynced, deleteBackupRequestInformer.Informer().HasSynced, + backupLocationInformer.Informer().HasSynced, ) c.resyncPeriod = GCSyncPeriod @@ -133,6 +137,19 @@ func (c *gcController) processQueueItem(key string) error { log.Info("Backup has expired") + loc, err := c.backupLocationLister.BackupStorageLocations(ns).Get(backup.Spec.StorageLocation) + if apierrors.IsNotFound(err) { + log.Warnf("Backup cannot be garbage-collected because backup storage location %s does not exist", backup.Spec.StorageLocation) + } + if err != nil { + return errors.Wrap(err, "error getting backup storage location") + } + + if loc.Spec.AccessMode == velerov1api.BackupStorageLocationAccessModeReadOnly { + log.Infof("Backup cannot be garbage-collected because backup storage location %s is currently in read-only mode", loc.Name) + return nil + } + selector := labels.SelectorFromSet(labels.Set(map[string]string{ velerov1api.BackupNameLabel: label.GetValidName(backup.Name), velerov1api.BackupUIDLabel: string(backup.UID), diff --git a/pkg/controller/gc_controller_test.go b/pkg/controller/gc_controller_test.go index 79c492f5d..647b0bc0a 100644 --- a/pkg/controller/gc_controller_test.go +++ b/pkg/controller/gc_controller_test.go @@ -1,5 +1,5 @@ /* -Copyright 2017 the Velero contributors. +Copyright 2017, 2019 the Velero contributors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -49,6 +49,7 @@ func TestGCControllerEnqueueAllBackups(t *testing.T) { sharedInformers.Velero().V1().Backups(), sharedInformers.Velero().V1().DeleteBackupRequests(), client.VeleroV1(), + sharedInformers.Velero().V1().BackupStorageLocations(), ).(*gcController) ) @@ -112,6 +113,7 @@ func TestGCControllerHasUpdateFunc(t *testing.T) { sharedInformers.Velero().V1().Backups(), sharedInformers.Velero().V1().DeleteBackupRequests(), client.VeleroV1(), + sharedInformers.Velero().V1().BackupStorageLocations(), ).(*gcController) keys := make(chan string) @@ -149,11 +151,13 @@ func TestGCControllerHasUpdateFunc(t *testing.T) { func TestGCControllerProcessQueueItem(t *testing.T) { fakeClock := clock.NewFakeClock(time.Now()) + defaultBackupLocation := velerotest.NewTestBackupStorageLocation().WithName("default").BackupStorageLocation tests := []struct { name string backup *api.Backup deleteBackupRequests []*api.DeleteBackupRequest + backupLocation *api.BackupStorageLocation expectDeletion bool createDeleteBackupRequestError bool expectError bool @@ -163,23 +167,52 @@ func TestGCControllerProcessQueueItem(t *testing.T) { }, { name: "unexpired backup is not deleted", - backup: velerotest.NewTestBackup().WithName("backup-1"). + backup: velerotest.NewTestBackup(). + WithName("backup-1"). WithExpiration(fakeClock.Now().Add(1 * time.Minute)). + WithStorageLocation("default"). Backup, + backupLocation: defaultBackupLocation, expectDeletion: false, }, { - name: "expired backup with no pending deletion requests is deleted", - backup: velerotest.NewTestBackup().WithName("backup-1"). - WithExpiration(fakeClock.Now().Add(-1 * time.Second)). + name: "expired backup in read-only storage location is not deleted", + backup: velerotest.NewTestBackup(). + WithName("backup-1"). + WithExpiration(fakeClock.Now().Add(-1 * time.Minute)). + WithStorageLocation("read-only"). Backup, + backupLocation: velerotest.NewTestBackupStorageLocation().WithName("read-only").WithAccessMode(api.BackupStorageLocationAccessModeReadOnly).BackupStorageLocation, + expectDeletion: false, + }, + { + name: "expired backup in read-write storage location is deleted", + backup: velerotest.NewTestBackup(). + WithName("backup-1"). + WithExpiration(fakeClock.Now().Add(-1 * time.Minute)). + WithStorageLocation("read-write"). + Backup, + backupLocation: velerotest.NewTestBackupStorageLocation().WithName("read-write").WithAccessMode(api.BackupStorageLocationAccessModeReadWrite).BackupStorageLocation, + expectDeletion: true, + }, + { + name: "expired backup with no pending deletion requests is deleted", + backup: velerotest.NewTestBackup(). + WithName("backup-1"). + WithExpiration(fakeClock.Now().Add(-1 * time.Second)). + WithStorageLocation("default"). + Backup, + backupLocation: defaultBackupLocation, expectDeletion: true, }, { name: "expired backup with a pending deletion request is not deleted", - backup: velerotest.NewTestBackup().WithName("backup-1"). + backup: velerotest.NewTestBackup(). + WithName("backup-1"). WithExpiration(fakeClock.Now().Add(-1 * time.Second)). + WithStorageLocation("default"). Backup, + backupLocation: defaultBackupLocation, deleteBackupRequests: []*api.DeleteBackupRequest{ { ObjectMeta: metav1.ObjectMeta{ @@ -199,9 +232,12 @@ func TestGCControllerProcessQueueItem(t *testing.T) { }, { name: "expired backup with only processed deletion requests is deleted", - backup: velerotest.NewTestBackup().WithName("backup-1"). + backup: velerotest.NewTestBackup(). + WithName("backup-1"). WithExpiration(fakeClock.Now().Add(-1 * time.Second)). + WithStorageLocation("default"). Backup, + backupLocation: defaultBackupLocation, deleteBackupRequests: []*api.DeleteBackupRequest{ { ObjectMeta: metav1.ObjectMeta{ @@ -221,9 +257,12 @@ func TestGCControllerProcessQueueItem(t *testing.T) { }, { name: "create DeleteBackupRequest error returns an error", - backup: velerotest.NewTestBackup().WithName("backup-1"). + backup: velerotest.NewTestBackup(). + WithName("backup-1"). WithExpiration(fakeClock.Now().Add(-1 * time.Second)). + WithStorageLocation("default"). Backup, + backupLocation: defaultBackupLocation, expectDeletion: true, createDeleteBackupRequestError: true, expectError: true, @@ -242,6 +281,7 @@ func TestGCControllerProcessQueueItem(t *testing.T) { sharedInformers.Velero().V1().Backups(), sharedInformers.Velero().V1().DeleteBackupRequests(), client.VeleroV1(), + sharedInformers.Velero().V1().BackupStorageLocations(), ).(*gcController) controller.clock = fakeClock @@ -251,6 +291,10 @@ func TestGCControllerProcessQueueItem(t *testing.T) { sharedInformers.Velero().V1().Backups().Informer().GetStore().Add(test.backup) } + if test.backupLocation != nil { + sharedInformers.Velero().V1().BackupStorageLocations().Informer().GetStore().Add(test.backupLocation) + } + for _, dbr := range test.deleteBackupRequests { sharedInformers.Velero().V1().DeleteBackupRequests().Informer().GetStore().Add(dbr) } diff --git a/pkg/util/test/test_backup_storage_location.go b/pkg/util/test/test_backup_storage_location.go index 546386b97..d26ccce12 100644 --- a/pkg/util/test/test_backup_storage_location.go +++ b/pkg/util/test/test_backup_storage_location.go @@ -1,5 +1,5 @@ /* -Copyright 2017 the Velero contributors. +Copyright 2017, 2019 the Velero contributors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -66,3 +66,8 @@ func (b *TestBackupStorageLocation) WithObjectStorage(bucketName string) *TestBa b.Spec.ObjectStorage.Bucket = bucketName return b } + +func (b *TestBackupStorageLocation) WithAccessMode(accessMode v1.BackupStorageLocationAccessMode) *TestBackupStorageLocation { + b.Spec.AccessMode = accessMode + return b +} diff --git a/site/docs/master/about.md b/site/docs/master/about.md index 1d9c5c174..85febb1df 100644 --- a/site/docs/master/about.md +++ b/site/docs/master/about.md @@ -31,7 +31,7 @@ The **restore** operation allows you to restore all of the objects and persisten The default name of a restore is `-`, where `` is formatted as *YYYYMMDDhhmmss*. You can also specify a custom name. A restored object also includes a label with key `velero.io/restore-name` and value ``. -You can also run the Velero server in restore-only mode, which disables backup, schedule, and garbage collection functionality during disaster recovery. +By default, backup storage locations are created in read-write mode. However, during a restore, you can configure a backup storage location to be in read-only mode, which disables backup creation and deletion for the storage location. This is useful to ensure that no backups are inadvertently created or deleted during a restore scenario. ## Backup workflow diff --git a/site/docs/master/disaster-case.md b/site/docs/master/disaster-case.md index 3c66f6286..88f61e7c7 100644 --- a/site/docs/master/disaster-case.md +++ b/site/docs/master/disaster-case.md @@ -1,6 +1,6 @@ # Disaster recovery -*Using Schedules and Restore-Only Mode* +*Using Schedules and Read-Only Backup Storage Locations* If you periodically back up your cluster's resources, you are able to return to a previous state in case of some unexpected mishap, such as a service outage. Doing so with Velero looks like the following: @@ -14,10 +14,26 @@ If you periodically back up your cluster's resources, you are able to return to 1. A disaster happens and you need to recreate your resources. -1. Update the Velero server deployment, adding the argument for the `server` command flag `restore-only` set to `true`. This prevents Backup objects from being created or deleted during your Restore process. +1. Update your backup storage location to read-only mode (this prevents backup objects from being created or deleted in the backup storage location during the restore process): + + ```bash + kubectl patch backupstoragelocation \ + --namespace velero \ + --type merge \ + --patch '{"spec":{"accessMode":"ReadOnly"}}' + ``` 1. Create a restore with your most recent Velero Backup: ``` velero restore create --from-backup - ``` + +1. When ready, revert your backup storage location to read-write mode: + + ```bash + kubectl patch backupstoragelocation \ + --namespace velero \ + --type merge \ + --patch '{"spec":{"accessMode":"ReadWrite"}}' + ``` diff --git a/site/docs/master/faq.md b/site/docs/master/faq.md index 6831cd4e6..8adce5045 100644 --- a/site/docs/master/faq.md +++ b/site/docs/master/faq.md @@ -38,6 +38,7 @@ and not to use prefixes at all. Related to this, if you need to restore a backup that was created in cluster A into cluster B, you may configure cluster B with a backup storage location that points to cluster A's bucket/prefix. If you do -this, you should use restore-only mode in cluster B's Velero instance (via the `--restore-only` flag on -the `velero server` command specified in your Velero deployment) while it's configured to use cluster A's -bucket/prefix. This will ensure no new backups are created, and no existing backups are deleted or overwritten. +this, you should configure the storage location pointing to cluster A's bucket/prefix in `ReadOnly` mode +via the `--access-mode=ReadOnly` flag on the `velero backup-location create` command. This will ensure no +new backups are created from Cluster B in Cluster A's bucket/prefix, and no existing backups are deleted +or overwritten. diff --git a/site/docs/master/migration-case.md b/site/docs/master/migration-case.md index 1296cc6b6..41afcf9ef 100644 --- a/site/docs/master/migration-case.md +++ b/site/docs/master/migration-case.md @@ -12,9 +12,8 @@ Velero can help you port your resources from one cluster to another, as long as The default TTL is 30 days (720 hours); you can use the `--ttl` flag to change this as necessary. -1. *(Cluster 2)* Add the `--restore-only` flag to the server spec in the Velero deployment YAML. - -1. *(Cluster 2)* Make sure that the `BackupStorageLocation` and `VolumeSnapshotLocation` CRDs match the ones from *Cluster 1*, so that your new Velero server instance points to the same bucket. +1. *(Cluster 2)* Configure `BackupStorageLocations` and `VolumeSnapshotLocations`, pointing to the locations used by *Cluster 1*, using `velero backup-location create` and `velero snapshot-location create`. Make sure to configure the `BackupStorageLocations` as read-only + by using the `--access-mode=ReadOnly` flag for `velero backup-location create`. 1. *(Cluster 2)* Make sure that the Velero Backup object is created. Velero resources are synchronized with the backup files in cloud storage.