diff --git a/docs/config-definition.md b/docs/config-definition.md
index d1c5a2b7b1..46b3c9ea31 100644
--- a/docs/config-definition.md
+++ b/docs/config-definition.md
@@ -50,7 +50,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. |
diff --git a/examples/minio/10-ark-config.yaml b/examples/minio/10-ark-config.yaml
index 58759fb0fe..d8673c6495 100644
--- a/examples/minio/10-ark-config.yaml
+++ b/examples/minio/10-ark-config.yaml
@@ -18,12 +18,6 @@ kind: Config
metadata:
namespace: heptio-ark
name: default
-persistentVolumeProvider:
- aws:
- region: minio
- availabilityZone: minio
- s3ForcePathStyle: true
- s3Url: http://minio:9000
backupStorageProvider:
bucket: ark
aws:
diff --git a/pkg/apis/ark/v1/config.go b/pkg/apis/ark/v1/config.go
index 6f66bc3c35..97f9954030 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 152a3bc2ce..053108318b 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 458e2a791e..ae0c5fd2e2 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 12b2780e82..b41860e8d0 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 ce3d89e815..3e7d7b0199 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 83786c71c8..34ee5de31b 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 18d30fb194..a35fdab72d 100644
--- a/pkg/controller/gc_controller.go
+++ b/pkg/controller/gc_controller.go
@@ -115,6 +115,12 @@ func (c *gcController) cleanBackups() {
}
for _, volumeBackup := range backup.Status.VolumeBackups {
+ if c.snapshotService == nil {
+ glog.Warningf("Cannot garbage-collect snapshot %s for %backup s/%s because server is not configured with PersistentVolumeProvider",
+ volumeBackup.SnapshotID, backup.Namespace, backup.Name)
+ continue
+ }
+
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)
diff --git a/pkg/controller/gc_controller_test.go b/pkg/controller/gc_controller_test.go
index adca51f164..56810d5b47 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,33 @@ func TestGarbageCollect(t *testing.T) {
expectedBackupsRemaining: make(map[string]sets.String),
expectedSnapshotsRemaining: sets.NewString("snapshot-3", "snapshot-4"),
},
+ gcTest{
+ name: "no snapshot service does not error",
+ 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,
+ },
+ },
+ snapshots: sets.NewString("snapshot-1", "snapshot-2"),
+ nilSnapshotService: true,
+ expectedBackupsRemaining: make(map[string]sets.String),
+ },
}
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 +191,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 +229,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 d25963c474..c3f34c2a45 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 0fb88f890a..d8b2ab797d 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 c5ce09defe..1f65d1f175 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 c29f105f08..d6163c724f 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 a686d5f2d4..996ea162a5 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
+}