allow individual backup storage locations to be read-only (#1517)

* allow individual backup storage locations to be read-only

Signed-off-by: Steve Kriss <krisss@vmware.com>
This commit is contained in:
Steve Kriss
2019-05-29 14:21:25 -04:00
committed by Nolan Brubaker
parent 4e2e4cd5c4
commit 411d44a673
16 changed files with 336 additions and 88 deletions
+5
View File
@@ -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
+35 -1
View File
@@ -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,
+40 -31
View File
@@ -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{
+106 -19
View File
@@ -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"
+17
View File
@@ -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),
+52 -8
View File
@@ -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)
}