diff --git a/docs/config-definition.md b/docs/config-definition.md index f5a170e9b..b430ddf5a 100644 --- a/docs/config-definition.md +++ b/docs/config-definition.md @@ -25,17 +25,12 @@ metadata: name: default persistentVolumeProvider: aws: - region: minio - availabilityZone: minio - s3ForcePathStyle: true - s3Url: http://minio:9000 + region: us-west-2 + availabilityZone: us-west-2a backupStorageProvider: bucket: ark aws: - region: minio - availabilityZone: minio - s3ForcePathStyle: true - s3Url: http://minio:9000 + region: us-west-2 backupSyncPeriod: 60m gcSyncPeriod: 60m scheduleSyncPeriod: 1m @@ -50,7 +45,7 @@ The configurable parameters are as follows: | Key | Type | Default | Meaning | | --- | --- | --- | --- | -| `persistentVolumeProvider` | CloudProviderConfig

(Supported key values are `aws`, `gcp`, and `azure`, but only one can be present. See the corresponding [AWS][0], [GCP][1], and [Azure][2]-specific configs.) | Required Field | The specification for whichever cloud provider the cluster is using for persistent volumes (to be snapshotted).

*NOTE*: For Azure, your Kubernetes cluster needs to be version 1.7.2+ in order to support PV snapshotting of its managed disks. | +| `persistentVolumeProvider` | CloudProviderConfig

(Supported key values are `aws`, `gcp`, and `azure`, but only one can be present. See the corresponding [AWS][0], [GCP][1], and [Azure][2]-specific configs.) | None (Optional) | The specification for whichever cloud provider the cluster is using for persistent volumes (to be snapshotted), if any.

If not specified, Backups and Restores requesting PV snapshots & restores, respectively, are considered invalid.

*NOTE*: For Azure, your Kubernetes cluster needs to be version 1.7.2+ in order to support PV snapshotting of its managed disks. | | `backupStorageProvider`/(inline) | CloudProviderConfig

(Supported key values are `aws`, `gcp`, and `azure`, but only one can be present. See the corresponding [AWS][0], [GCP][1], and [Azure][2]-specific configs.) | Required Field | The specification for whichever cloud provider will be used to actually store the backups. | | `backupStorageProvider/bucket` | String | Required Field | The storage bucket where backups are to be uploaded. | | `backupSyncPeriod` | metav1.Duration | 60m0s | How frequently Ark queries the object storage to make sure that the appropriate Backup resources have been created for existing backup files. | @@ -63,22 +58,44 @@ The configurable parameters are as follows: **(Or other S3-compatible storage)** +#### backupStorageProvider + +| Key | Type | Default | Meaning | +| --- | --- | --- | --- | +| `region` | string | Required Field | *Example*: "us-east-1"

See [AWS documentation][3] for the full list. | +| `disableSSL` | bool | `false` | Set this to `true` if you are using Minio (or another local, S3-compatible storage service) and your deployment is not secured. | +| `s3ForcePathStyle` | bool | `false` | Set this to `true` if you are using a local storage service like Minio. | +| `s3Url` | string | Required field for non-AWS-hosted storage| *Example*: http://minio:9000

You can specify the AWS S3 URL here for explicitness, but Ark can already generate it from `region`, `availabilityZone`, and `bucket`. This field is primarily for local storage services like Minio.| +| `kmsKeyID` | string | Empty | *Example*: "502b409c-4da1-419f-a16e-eif453b3i49f"

Specify an [AWS KMS key][12] id to enable encryption of the backups stored in S3. Only works with AWS S3 and may require explicitly granting key usage rights.| + +#### persistentVolumeProvider (AWS Only) + | Key | Type | Default | Meaning | | --- | --- | --- | --- | | `region` | string | Required Field | *Example*: "us-east-1"

See [AWS documentation][3] for the full list. | | `availabilityZone` | string | Required Field | *Example*: "us-east-1a"

See [AWS documentation][4] for details. | -| `disableSSL` | bool | `false` | Set this to `true` if you are using Minio (or another local, S3-compatible storage service) and your deployment is not secured. | -| `s3ForcePathStyle` | bool | `false` | Set this to `true` if you are using a local storage service like Minio. | -| `s3Url` | string | Required field for non-AWS-hosted storage| *Example*: http://minio:9000

You can specify the AWS S3 URL here for explicitness, but Ark can already generate it from `region`, `availabilityZone`, and `bucket`. This field is primarily for local sotrage services like Minio.| -| `kmsKeyID` | string | Empty | *Example*: "502b409c-4da1-419f-a16e-eif453b3i49f"

Specify an [AWS KMS key][12] id to enable encryption of the backups stored in S3. Only works with AWS S3 and may require explicitly granting key usage rights.| ### GCP + +#### backupStorageProvider + +No parameters required; specify an empty object per [example file][13]. + +#### persistentVolumeProvider + | Key | Type | Default | Meaning | | --- | --- | --- | --- | | `project` | string | Required Field | *Example*: "project-example-3jsn23"

See the [Project ID documentation][5] for details. | | `zone` | string | Required Field | *Example*: "us-central1-a"

See [GCP documentation][6] for the full list. | ### Azure + +#### backupStorageProvider + +No parameters required; specify an empty object per [example file][14]. + +#### persistentVolumeProvider + | Key | Type | Default | Meaning | | --- | --- | --- | --- | | `location` | string | Required Field | *Example*: "Canada East"

See [the list of available locations][7] (note that this particular page refers to them as "Regions"). | @@ -97,4 +114,6 @@ The configurable parameters are as follows: [10]: #overview [11]: #example [12]: http://docs.aws.amazon.com/kms/latest/developerguide/overview.html +[13]: ../examples/gcp/00-ark-config.yaml +[14]: ../examples/azure/10-ark-config.yaml diff --git a/examples/aws/00-ark-config.yaml b/examples/aws/00-ark-config.yaml index 6c6a69ab1..183f983c8 100644 --- a/examples/aws/00-ark-config.yaml +++ b/examples/aws/00-ark-config.yaml @@ -26,7 +26,6 @@ backupStorageProvider: bucket: aws: region: - availabilityZone: backupSyncPeriod: 30m gcSyncPeriod: 30m scheduleSyncPeriod: 1m diff --git a/examples/azure/10-ark-config.yaml b/examples/azure/10-ark-config.yaml index d90647a50..4edcdfe37 100644 --- a/examples/azure/10-ark-config.yaml +++ b/examples/azure/10-ark-config.yaml @@ -24,9 +24,7 @@ persistentVolumeProvider: apiTimeout: backupStorageProvider: bucket: - azure: - location: - apiTimeout: + azure: {} backupSyncPeriod: 30m gcSyncPeriod: 30m scheduleSyncPeriod: 1m diff --git a/examples/gcp/00-ark-config.yaml b/examples/gcp/00-ark-config.yaml index 00cba5cf5..3b7579847 100644 --- a/examples/gcp/00-ark-config.yaml +++ b/examples/gcp/00-ark-config.yaml @@ -24,9 +24,7 @@ persistentVolumeProvider: zone: backupStorageProvider: bucket: - gcp: - project: - zone: + gcp: {} backupSyncPeriod: 30m gcSyncPeriod: 30m scheduleSyncPeriod: 1m diff --git a/examples/minio/10-ark-config.yaml b/examples/minio/10-ark-config.yaml index 58759fb0f..66a6dce55 100644 --- a/examples/minio/10-ark-config.yaml +++ b/examples/minio/10-ark-config.yaml @@ -18,17 +18,10 @@ kind: Config metadata: namespace: heptio-ark name: default -persistentVolumeProvider: - aws: - region: minio - availabilityZone: minio - s3ForcePathStyle: true - s3Url: http://minio:9000 backupStorageProvider: bucket: ark aws: region: minio - availabilityZone: minio s3ForcePathStyle: true s3Url: http://minio:9000 backupSyncPeriod: 1m diff --git a/pkg/apis/ark/v1/config.go b/pkg/apis/ark/v1/config.go index 60399f4c1..29ad9b1f3 100644 --- a/pkg/apis/ark/v1/config.go +++ b/pkg/apis/ark/v1/config.go @@ -35,8 +35,8 @@ type Config struct { metav1.ObjectMeta `json:"metadata"` // PersistentVolumeProvider is the configuration information for the cloud where - // the cluster is running and has PersistentVolumes to snapshot or restore. - PersistentVolumeProvider CloudProviderConfig `json:"persistentVolumeProvider"` + // the cluster is running and has PersistentVolumes to snapshot or restore. Optional. + PersistentVolumeProvider *CloudProviderConfig `json:"persistentVolumeProvider"` // BackupStorageProvider is the configuration information for the cloud where // Ark backups are stored in object storage. This may be a different cloud than diff --git a/pkg/backup/volume_snapshot_action.go b/pkg/backup/volume_snapshot_action.go index 152a3bc2c..053108318 100644 --- a/pkg/backup/volume_snapshot_action.go +++ b/pkg/backup/volume_snapshot_action.go @@ -17,6 +17,7 @@ limitations under the License. package backup import ( + "errors" "fmt" "regexp" @@ -38,11 +39,15 @@ type volumeSnapshotAction struct { var _ Action = &volumeSnapshotAction{} -func NewVolumeSnapshotAction(snapshotService cloudprovider.SnapshotService) Action { +func NewVolumeSnapshotAction(snapshotService cloudprovider.SnapshotService) (Action, error) { + if snapshotService == nil { + return nil, errors.New("snapshotService cannot be nil") + } + return &volumeSnapshotAction{ snapshotService: snapshotService, clock: clock.RealClock{}, - } + }, nil } // Execute triggers a snapshot for the volume/disk underlying a PersistentVolume if the provided diff --git a/pkg/backup/volume_snapshot_action_test.go b/pkg/backup/volume_snapshot_action_test.go index ad1f788cd..77407d2a0 100644 --- a/pkg/backup/volume_snapshot_action_test.go +++ b/pkg/backup/volume_snapshot_action_test.go @@ -155,7 +155,10 @@ func TestVolumeSnapshotAction(t *testing.T) { } snapshotService := &FakeSnapshotService{SnapshottableVolumes: test.volumeInfo} - action := NewVolumeSnapshotAction(snapshotService).(*volumeSnapshotAction) + + vsa, _ := NewVolumeSnapshotAction(snapshotService) + action := vsa.(*volumeSnapshotAction) + fakeClock := clock.NewFakeClock(time.Now()) action.clock = fakeClock diff --git a/pkg/cmd/server/server.go b/pkg/cmd/server/server.go index 1d1a2044b..375cb3707 100644 --- a/pkg/cmd/server/server.go +++ b/pkg/cmd/server/server.go @@ -251,8 +251,13 @@ func (s *server) initBackupService(config *api.Config) error { } func (s *server) initSnapshotService(config *api.Config) error { + if config.PersistentVolumeProvider == nil { + glog.Infof("PersistentVolumeProvider config not provided, skipping SnapshotService creation") + return nil + } + glog.Infof("Configuring cloud provider for snapshot service") - cloud, err := initCloud(config.PersistentVolumeProvider, "persistentVolumeProvider") + cloud, err := initCloud(*config.PersistentVolumeProvider, "persistentVolumeProvider") if err != nil { return err } @@ -408,6 +413,7 @@ func (s *server) runControllers(config *api.Config) error { backupper, s.backupService, config.BackupStorageProvider.Bucket, + s.snapshotService != nil, ) wg.Add(1) go func() { @@ -461,6 +467,7 @@ func (s *server) runControllers(config *api.Config) error { s.backupService, config.BackupStorageProvider.Bucket, s.sharedInformerFactory.Ark().V1().Backups(), + s.snapshotService != nil, ) wg.Add(1) go func() { @@ -490,7 +497,12 @@ func newBackupper( actions := map[string]backup.Action{} if snapshotService != nil { - actions["persistentvolumes"] = backup.NewVolumeSnapshotAction(snapshotService) + action, err := backup.NewVolumeSnapshotAction(snapshotService) + if err != nil { + return nil, err + } + + actions["persistentvolumes"] = action } return backup.NewKubernetesBackupper( diff --git a/pkg/controller/backup_controller.go b/pkg/controller/backup_controller.go index ce3d89e81..3e7d7b019 100644 --- a/pkg/controller/backup_controller.go +++ b/pkg/controller/backup_controller.go @@ -49,9 +49,10 @@ import ( const backupVersion = 1 type backupController struct { - backupper backup.Backupper - backupService cloudprovider.BackupService - bucket string + backupper backup.Backupper + backupService cloudprovider.BackupService + bucket string + allowSnapshots bool lister listers.BackupLister listerSynced cache.InformerSynced @@ -68,11 +69,13 @@ func NewBackupController( backupper backup.Backupper, backupService cloudprovider.BackupService, bucket string, + allowSnapshots bool, ) Interface { c := &backupController{ - backupper: backupper, - backupService: backupService, - bucket: bucket, + backupper: backupper, + backupService: backupService, + bucket: bucket, + allowSnapshots: allowSnapshots, lister: backupInformer.Lister(), listerSynced: backupInformer.Informer().HasSynced, @@ -297,6 +300,10 @@ func (controller *backupController) getValidationErrors(itm *api.Backup) []strin validationErrors = append(validationErrors, fmt.Sprintf("Invalid included/excluded namespace lists: %v", err)) } + if !controller.allowSnapshots && itm.Spec.SnapshotVolumes { + validationErrors = append(validationErrors, "Server is not configured for PV snapshots") + } + return validationErrors } diff --git a/pkg/controller/backup_controller_test.go b/pkg/controller/backup_controller_test.go index 83786c71c..34ee5de31 100644 --- a/pkg/controller/backup_controller_test.go +++ b/pkg/controller/backup_controller_test.go @@ -54,6 +54,7 @@ func TestProcessBackup(t *testing.T) { expectedExcludes []string backup *TestBackup expectBackup bool + allowSnapshots bool }{ { name: "bad key", @@ -129,6 +130,20 @@ func TestProcessBackup(t *testing.T) { expectedIncludes: []string{"*"}, expectBackup: true, }, + { + name: "backup with SnapshotVolumes when allowSnapshots=false fails validation", + key: "heptio-ark/backup1", + backup: NewTestBackup().WithName("backup1").WithPhase(v1.BackupPhaseNew).WithSnapshotVolumes(true), + expectBackup: false, + }, + { + name: "backup with SnapshotVolumes when allowSnapshots=true gets executed", + key: "heptio-ark/backup1", + backup: NewTestBackup().WithName("backup1").WithPhase(v1.BackupPhaseNew).WithSnapshotVolumes(true), + allowSnapshots: true, + expectedIncludes: []string{"*"}, + expectBackup: true, + }, } // flag.Set("logtostderr", "true") @@ -150,6 +165,7 @@ func TestProcessBackup(t *testing.T) { backupper, cloudBackups, "bucket", + test.allowSnapshots, ).(*backupController) c.clock = clock.NewFakeClock(time.Now()) @@ -180,6 +196,7 @@ func TestProcessBackup(t *testing.T) { } backup.Spec.IncludedNamespaces = expectedNSes + backup.Spec.SnapshotVolumes = test.backup.Spec.SnapshotVolumes backup.Status.Phase = v1.BackupPhaseInProgress backup.Status.Expiration.Time = expiration backup.Status.Version = 1 @@ -226,6 +243,7 @@ func TestProcessBackup(t *testing.T) { WithExcludedResources(test.expectedExcludes...). WithIncludedNamespaces(expectedNSes...). WithTTL(test.backup.Spec.TTL.Duration). + WithSnapshotVolumes(test.backup.Spec.SnapshotVolumes). WithExpiration(expiration). WithVersion(1). Backup, @@ -241,6 +259,7 @@ func TestProcessBackup(t *testing.T) { WithExcludedResources(test.expectedExcludes...). WithIncludedNamespaces(expectedNSes...). WithTTL(test.backup.Spec.TTL.Duration). + WithSnapshotVolumes(test.backup.Spec.SnapshotVolumes). WithExpiration(expiration). WithVersion(1). Backup, diff --git a/pkg/controller/gc_controller.go b/pkg/controller/gc_controller.go index 18d30fb19..91b6b1d47 100644 --- a/pkg/controller/gc_controller.go +++ b/pkg/controller/gc_controller.go @@ -108,26 +108,36 @@ func (c *gcController) cleanBackups() { // storage should happen first because otherwise there's a possibility the backup sync // controller would re-create the API object after deletion. for _, backup := range backups { - if backup.Status.Expiration.Time.Before(now) { - glog.Infof("Removing backup %s/%s", backup.Namespace, backup.Name) - if err := c.backupService.DeleteBackup(c.bucket, backup.Name); err != nil { - glog.Errorf("error deleting backup %s/%s: %v", backup.Namespace, backup.Name, err) - } - - for _, volumeBackup := range backup.Status.VolumeBackups { - glog.Infof("Removing snapshot %s associated with backup %s/%s", volumeBackup.SnapshotID, backup.Namespace, backup.Name) - if err := c.snapshotService.DeleteSnapshot(volumeBackup.SnapshotID); err != nil { - glog.Errorf("error deleting snapshot %v: %v", volumeBackup.SnapshotID, err) - } - } - - glog.Infof("Removing backup API object %s/%s", backup.Namespace, backup.Name) - if err := c.client.Backups(backup.Namespace).Delete(backup.Name, &metav1.DeleteOptions{}); err != nil { - glog.Errorf("error deleting backup API object %s/%s: %v", backup.Namespace, backup.Name, err) - } - } else { + if !backup.Status.Expiration.Time.Before(now) { glog.Infof("Backup %s/%s has not expired yet, skipping", backup.Namespace, backup.Name) + continue } + + // if the backup includes snapshots but we don't currently have a PVProvider, we don't + // want to orphan the snapshots so skip garbage-collection entirely. + if c.snapshotService == nil && len(backup.Status.VolumeBackups) > 0 { + glog.Warningf("Cannot garbage-collect backup %s/%s because backup includes snapshots and server is not configured with PersistentVolumeProvider", + backup.Namespace, backup.Name) + continue + } + + glog.Infof("Removing backup %s/%s", backup.Namespace, backup.Name) + if err := c.backupService.DeleteBackup(c.bucket, backup.Name); err != nil { + glog.Errorf("error deleting backup %s/%s: %v", backup.Namespace, backup.Name, err) + } + + for _, volumeBackup := range backup.Status.VolumeBackups { + glog.Infof("Removing snapshot %s associated with backup %s/%s", volumeBackup.SnapshotID, backup.Namespace, backup.Name) + if err := c.snapshotService.DeleteSnapshot(volumeBackup.SnapshotID); err != nil { + glog.Errorf("error deleting snapshot %v: %v", volumeBackup.SnapshotID, err) + } + } + + glog.Infof("Removing backup API object %s/%s", backup.Namespace, backup.Name) + if err := c.client.Backups(backup.Namespace).Delete(backup.Name, &metav1.DeleteOptions{}); err != nil { + glog.Errorf("error deleting backup API object %s/%s: %v", backup.Namespace, backup.Name, err) + } + } // also GC any Backup API objects without files in object storage diff --git a/pkg/controller/gc_controller_test.go b/pkg/controller/gc_controller_test.go index adca51f16..0eb85d793 100644 --- a/pkg/controller/gc_controller_test.go +++ b/pkg/controller/gc_controller_test.go @@ -31,16 +31,18 @@ import ( "k8s.io/apimachinery/pkg/util/sets" api "github.com/heptio/ark/pkg/apis/ark/v1" + "github.com/heptio/ark/pkg/cloudprovider" "github.com/heptio/ark/pkg/generated/clientset/fake" informers "github.com/heptio/ark/pkg/generated/informers/externalversions" . "github.com/heptio/ark/pkg/util/test" ) type gcTest struct { - name string - bucket string - backups map[string][]*api.Backup - snapshots sets.String + name string + bucket string + backups map[string][]*api.Backup + snapshots sets.String + nilSnapshotService bool expectedBackupsRemaining map[string]sets.String expectedSnapshotsRemaining sets.String @@ -149,11 +151,38 @@ func TestGarbageCollect(t *testing.T) { expectedBackupsRemaining: make(map[string]sets.String), expectedSnapshotsRemaining: sets.NewString("snapshot-3", "snapshot-4"), }, + gcTest{ + name: "no snapshot service only GC's backups without snapshots", + bucket: "bucket-1", + backups: map[string][]*api.Backup{ + "bucket-1": []*api.Backup{ + NewTestBackup().WithName("backup-1"). + WithExpiration(fakeClock.Now().Add(-1*time.Second)). + WithSnapshot("pv-1", "snapshot-1"). + WithSnapshot("pv-2", "snapshot-2"). + Backup, + NewTestBackup().WithName("backup-2"). + WithExpiration(fakeClock.Now().Add(-1 * time.Second)). + Backup, + }, + }, + snapshots: sets.NewString("snapshot-1", "snapshot-2"), + nilSnapshotService: true, + expectedBackupsRemaining: map[string]sets.String{ + "bucket-1": sets.NewString("backup-1"), + }, + }, } for _, test := range tests { - backupService := &fakeBackupService{} - snapshotService := &FakeSnapshotService{} + var ( + backupService = &fakeBackupService{} + snapshotService *FakeSnapshotService + ) + + if !test.nilSnapshotService { + snapshotService = &FakeSnapshotService{SnapshotsTaken: test.snapshots} + } t.Run(test.name, func(t *testing.T) { backupService.backupsByBucket = make(map[string][]*api.Backup) @@ -167,16 +196,19 @@ func TestGarbageCollect(t *testing.T) { backupService.backupsByBucket[bucket] = data } - snapshotService.SnapshotsTaken = test.snapshots - var ( client = fake.NewSimpleClientset() sharedInformers = informers.NewSharedInformerFactory(client, 0) + snapSvc cloudprovider.SnapshotService ) + if snapshotService != nil { + snapSvc = snapshotService + } + controller := NewGCController( backupService, - snapshotService, + snapSvc, test.bucket, 1*time.Millisecond, sharedInformers.Ark().V1().Backups(), @@ -202,7 +234,9 @@ func TestGarbageCollect(t *testing.T) { assert.Equal(t, test.expectedBackupsRemaining[bucket], backupNames) } - assert.Equal(t, test.expectedSnapshotsRemaining, snapshotService.SnapshotsTaken) + if !test.nilSnapshotService { + assert.Equal(t, test.expectedSnapshotsRemaining, snapshotService.SnapshotsTaken) + } }) } } diff --git a/pkg/controller/restore_controller.go b/pkg/controller/restore_controller.go index d25963c47..c3f34c2a4 100644 --- a/pkg/controller/restore_controller.go +++ b/pkg/controller/restore_controller.go @@ -42,11 +42,12 @@ import ( ) type restoreController struct { - restoreClient arkv1client.RestoresGetter - backupClient arkv1client.BackupsGetter - restorer restore.Restorer - backupService cloudprovider.BackupService - bucket string + restoreClient arkv1client.RestoresGetter + backupClient arkv1client.BackupsGetter + restorer restore.Restorer + backupService cloudprovider.BackupService + bucket string + allowSnapshotRestores bool backupLister listers.BackupLister backupListerSynced cache.InformerSynced @@ -64,18 +65,20 @@ func NewRestoreController( backupService cloudprovider.BackupService, bucket string, backupInformer informers.BackupInformer, + allowSnapshotRestores bool, ) Interface { c := &restoreController{ - restoreClient: restoreClient, - backupClient: backupClient, - restorer: restorer, - backupService: backupService, - bucket: bucket, - backupLister: backupInformer.Lister(), - backupListerSynced: backupInformer.Informer().HasSynced, - restoreLister: restoreInformer.Lister(), - restoreListerSynced: restoreInformer.Informer().HasSynced, - queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "restore"), + restoreClient: restoreClient, + backupClient: backupClient, + restorer: restorer, + backupService: backupService, + bucket: bucket, + allowSnapshotRestores: allowSnapshotRestores, + backupLister: backupInformer.Lister(), + backupListerSynced: backupInformer.Informer().HasSynced, + restoreLister: restoreInformer.Lister(), + restoreListerSynced: restoreInformer.Informer().HasSynced, + queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "restore"), } c.syncHandler = c.processRestore @@ -275,6 +278,10 @@ func (controller *restoreController) getValidationErrors(itm *api.Restore) []str validationErrors = append(validationErrors, "BackupName must be non-empty and correspond to the name of a backup in object storage.") } + if !controller.allowSnapshotRestores && itm.Spec.RestorePVs { + validationErrors = append(validationErrors, "Server is not configured for PV snapshot restores") + } + return validationErrors } diff --git a/pkg/controller/restore_controller_test.go b/pkg/controller/restore_controller_test.go index 0fb88f890..d8b2ab797 100644 --- a/pkg/controller/restore_controller_test.go +++ b/pkg/controller/restore_controller_test.go @@ -42,6 +42,7 @@ func TestProcessRestore(t *testing.T) { restore *api.Restore backup *api.Backup restorerError error + allowRestoreSnapshots bool expectedErr bool expectedRestoreUpdates []*api.Restore expectedRestorerCall *api.Restore @@ -137,6 +138,28 @@ func TestProcessRestore(t *testing.T) { }, expectedRestorerCall: NewTestRestore("foo", "bar", api.RestorePhaseInProgress).WithBackup("backup-1").WithRestorableNamespace("*").Restore, }, + { + name: "valid restore with RestorePVs=true gets executed when allowRestoreSnapshots=true", + restore: NewTestRestore("foo", "bar", api.RestorePhaseNew).WithBackup("backup-1").WithRestorableNamespace("ns-1").WithRestorePVs(true).Restore, + backup: NewTestBackup().WithName("backup-1").Backup, + allowRestoreSnapshots: true, + expectedErr: false, + expectedRestoreUpdates: []*api.Restore{ + NewTestRestore("foo", "bar", api.RestorePhaseInProgress).WithBackup("backup-1").WithRestorableNamespace("ns-1").WithRestorePVs(true).Restore, + NewTestRestore("foo", "bar", api.RestorePhaseCompleted).WithBackup("backup-1").WithRestorableNamespace("ns-1").WithRestorePVs(true).Restore, + }, + expectedRestorerCall: NewTestRestore("foo", "bar", api.RestorePhaseInProgress).WithBackup("backup-1").WithRestorableNamespace("ns-1").WithRestorePVs(true).Restore, + }, + { + name: "restore with RestorePVs=true fails validation when allowRestoreSnapshots=false", + restore: NewTestRestore("foo", "bar", api.RestorePhaseNew).WithBackup("backup-1").WithRestorableNamespace("ns-1").WithRestorePVs(true).Restore, + backup: NewTestBackup().WithName("backup-1").Backup, + expectedErr: false, + expectedRestoreUpdates: []*api.Restore{ + NewTestRestore("foo", "bar", api.RestorePhaseFailedValidation).WithBackup("backup-1").WithRestorableNamespace("ns-1").WithRestorePVs(true). + WithValidationError("Server is not configured for PV snapshot restores").Restore, + }, + }, } // flag.Set("logtostderr", "true") @@ -160,6 +183,7 @@ func TestProcessRestore(t *testing.T) { backupSvc, "bucket", sharedInformers.Ark().V1().Backups(), + test.allowRestoreSnapshots, ).(*restoreController) if test.restore != nil { diff --git a/pkg/restore/restorers/pv_restorer.go b/pkg/restore/restorers/pv_restorer.go index c5ce09def..1f65d1f17 100644 --- a/pkg/restore/restorers/pv_restorer.go +++ b/pkg/restore/restorers/pv_restorer.go @@ -57,6 +57,10 @@ func (sr *persistentVolumeRestorer) Prepare(obj runtime.Unstructured, restore *a delete(spec, "storageClassName") if restore.Spec.RestorePVs { + if sr.snapshotService == nil { + return nil, errors.New("PV restorer is not configured for PV snapshot restores") + } + volumeID, err := sr.restoreVolume(obj.UnstructuredContent(), restore, backup) if err != nil { return nil, err diff --git a/pkg/util/test/test_backup.go b/pkg/util/test/test_backup.go index c29f105f0..d6163c724 100644 --- a/pkg/util/test/test_backup.go +++ b/pkg/util/test/test_backup.go @@ -104,3 +104,8 @@ func (b *TestBackup) WithSnapshot(pv string, snapshot string) *TestBackup { b.Status.VolumeBackups[pv] = &v1.VolumeBackupInfo{SnapshotID: snapshot} return b } + +func (b *TestBackup) WithSnapshotVolumes(value bool) *TestBackup { + b.Spec.SnapshotVolumes = value + return b +} diff --git a/pkg/util/test/test_restore.go b/pkg/util/test/test_restore.go index a686d5f2d..996ea162a 100644 --- a/pkg/util/test/test_restore.go +++ b/pkg/util/test/test_restore.go @@ -60,3 +60,8 @@ func (r *TestRestore) WithErrors(e api.RestoreResult) *TestRestore { r.Status.Errors = e return r } + +func (r *TestRestore) WithRestorePVs(value bool) *TestRestore { + r.Spec.RestorePVs = value + return r +}