diff --git a/docs/cli-reference/ark_backup_create.md b/docs/cli-reference/ark_backup_create.md index 706888358..25f177361 100644 --- a/docs/cli-reference/ark_backup_create.md +++ b/docs/cli-reference/ark_backup_create.md @@ -14,17 +14,17 @@ ark backup create NAME ### Options ``` - --exclude-namespaces stringArray namespaces to exclude from the backup - --exclude-resources stringArray resources to exclude from the backup, formatted as resource.group, such as storageclasses.storage.k8s.io - --include-namespaces stringArray namespaces to include in the backup (use '*' for all namespaces) (default *) - --include-resources stringArray resources to include in the backup, formatted as resource.group, such as storageclasses.storage.k8s.io (use '*' for all resources) - --label-columns stringArray a comma-separated list of labels to be displayed as columns - --labels mapStringString labels to apply to the backup - -o, --output string Output display format. For create commands, display the object but do not send it to the server. Valid formats are 'table', 'json', and 'yaml'. - -l, --selector labelSelector only back up resources matching this label selector (default ) - --show-labels show labels in the last column - --snapshot-volumes take snapshots of PersistentVolumes as part of the backup (default true) - --ttl duration how long before the backup can be garbage collected (default 24h0m0s) + --exclude-namespaces stringArray namespaces to exclude from the backup + --exclude-resources stringArray resources to exclude from the backup, formatted as resource.group, such as storageclasses.storage.k8s.io + --include-namespaces stringArray namespaces to include in the backup (use '*' for all namespaces) (default *) + --include-resources stringArray resources to include in the backup, formatted as resource.group, such as storageclasses.storage.k8s.io (use '*' for all resources) + --label-columns stringArray a comma-separated list of labels to be displayed as columns + --labels mapStringString labels to apply to the backup + -o, --output string Output display format. For create commands, display the object but do not send it to the server. Valid formats are 'table', 'json', and 'yaml'. + -l, --selector labelSelector only back up resources matching this label selector (default ) + --show-labels show labels in the last column + --snapshot-volumes optionalBool[=true] take snapshots of PersistentVolumes as part of the backup + --ttl duration how long before the backup can be garbage collected (default 24h0m0s) ``` ### Options inherited from parent commands diff --git a/docs/cli-reference/ark_restore_create.md b/docs/cli-reference/ark_restore_create.md index 2943f943e..7190ab667 100644 --- a/docs/cli-reference/ark_restore_create.md +++ b/docs/cli-reference/ark_restore_create.md @@ -14,14 +14,14 @@ ark restore create BACKUP ### Options ``` - --label-columns stringArray a comma-separated list of labels to be displayed as columns - --labels mapStringString labels to apply to the restore - --namespace-mappings mapStringString namespace mappings from name in the backup to desired restored name in the form src1:dst1,src2:dst2,... - --namespaces stringArray comma-separated list of namespaces to restore - -o, --output string Output display format. For create commands, display the object but do not send it to the server. Valid formats are 'table', 'json', and 'yaml'. - --restore-volumes whether to restore volumes from snapshots (default true) - -l, --selector labelSelector only restore resources matching this label selector (default ) - --show-labels show labels in the last column + --label-columns stringArray a comma-separated list of labels to be displayed as columns + --labels mapStringString labels to apply to the restore + --namespace-mappings mapStringString namespace mappings from name in the backup to desired restored name in the form src1:dst1,src2:dst2,... + --namespaces stringArray comma-separated list of namespaces to restore + -o, --output string Output display format. For create commands, display the object but do not send it to the server. Valid formats are 'table', 'json', and 'yaml'. + --restore-volumes optionalBool[=true] whether to restore volumes from snapshots + -l, --selector labelSelector only restore resources matching this label selector (default ) + --show-labels show labels in the last column ``` ### Options inherited from parent commands diff --git a/docs/cli-reference/ark_schedule_create.md b/docs/cli-reference/ark_schedule_create.md index 694610e18..3ba7ec9e8 100644 --- a/docs/cli-reference/ark_schedule_create.md +++ b/docs/cli-reference/ark_schedule_create.md @@ -14,18 +14,18 @@ ark schedule create NAME ### Options ``` - --exclude-namespaces stringArray namespaces to exclude from the backup - --exclude-resources stringArray resources to exclude from the backup, formatted as resource.group, such as storageclasses.storage.k8s.io - --include-namespaces stringArray namespaces to include in the backup (use '*' for all namespaces) (default *) - --include-resources stringArray resources to include in the backup, formatted as resource.group, such as storageclasses.storage.k8s.io (use '*' for all resources) - --label-columns stringArray a comma-separated list of labels to be displayed as columns - --labels mapStringString labels to apply to the backup - -o, --output string Output display format. For create commands, display the object but do not send it to the server. Valid formats are 'table', 'json', and 'yaml'. - --schedule string a cron expression specifying a recurring schedule for this backup to run - -l, --selector labelSelector only back up resources matching this label selector (default ) - --show-labels show labels in the last column - --snapshot-volumes take snapshots of PersistentVolumes as part of the backup (default true) - --ttl duration how long before the backup can be garbage collected (default 24h0m0s) + --exclude-namespaces stringArray namespaces to exclude from the backup + --exclude-resources stringArray resources to exclude from the backup, formatted as resource.group, such as storageclasses.storage.k8s.io + --include-namespaces stringArray namespaces to include in the backup (use '*' for all namespaces) (default *) + --include-resources stringArray resources to include in the backup, formatted as resource.group, such as storageclasses.storage.k8s.io (use '*' for all resources) + --label-columns stringArray a comma-separated list of labels to be displayed as columns + --labels mapStringString labels to apply to the backup + -o, --output string Output display format. For create commands, display the object but do not send it to the server. Valid formats are 'table', 'json', and 'yaml'. + --schedule string a cron expression specifying a recurring schedule for this backup to run + -l, --selector labelSelector only back up resources matching this label selector (default ) + --show-labels show labels in the last column + --snapshot-volumes optionalBool[=true] take snapshots of PersistentVolumes as part of the backup + --ttl duration how long before the backup can be garbage collected (default 24h0m0s) ``` ### Options inherited from parent commands diff --git a/pkg/apis/ark/v1/backup.go b/pkg/apis/ark/v1/backup.go index adfe40b41..7dd266ad5 100644 --- a/pkg/apis/ark/v1/backup.go +++ b/pkg/apis/ark/v1/backup.go @@ -41,10 +41,10 @@ type BackupSpec struct { // or nil, all objects are included. Optional. LabelSelector *metav1.LabelSelector `json:"labelSelector"` - // SnapshotVolumes is a bool which specifies whether to take - // cloud snapshots of any PV's referenced in the set of objects - // included in the Backup. - SnapshotVolumes bool `json:"snapshotVolumes"` + // SnapshotVolumes specifies whether to take cloud snapshots + // of any PV's referenced in the set of objects included + // in the Backup. + SnapshotVolumes *bool `json:"snapshotVolumes"` // TTL is a time.Duration-parseable string describing how long // the Backup should be retained for. diff --git a/pkg/apis/ark/v1/restore.go b/pkg/apis/ark/v1/restore.go index 368c04981..4e254f865 100644 --- a/pkg/apis/ark/v1/restore.go +++ b/pkg/apis/ark/v1/restore.go @@ -38,9 +38,9 @@ type RestoreSpec struct { // or nil, all objects are included. Optional. LabelSelector *metav1.LabelSelector `json:"labelSelector"` - // RestorePVs is a bool defining whether to restore all included - // PVs from snapshot (via the cloudprovider). Default false. - RestorePVs bool `json:"restorePVs"` + // RestorePVs specifies whether to restore all included + // PVs from snapshot (via the cloudprovider). + RestorePVs *bool `json:"restorePVs"` } // RestorePhase is a string representation of the lifecycle phase diff --git a/pkg/backup/backup.go b/pkg/backup/backup.go index 2d5dbd19f..f44a00093 100644 --- a/pkg/backup/backup.go +++ b/pkg/backup/backup.go @@ -364,7 +364,10 @@ func (*realItemBackupper) backupItem(ctx *backupContext, item map[string]interfa if action != nil { glog.V(4).Infof("Executing action on %s, ns=%s, name=%s", groupResource, namespace, name) - action.Execute(item, ctx.backup) + + if err := action.Execute(item, ctx.backup); err != nil { + return err + } } glog.V(2).Infof("Backing up resource=%s, ns=%s, name=%s", groupResource, namespace, name) diff --git a/pkg/backup/volume_snapshot_action.go b/pkg/backup/volume_snapshot_action.go index 053108318..d42cd32cc 100644 --- a/pkg/backup/volume_snapshot_action.go +++ b/pkg/backup/volume_snapshot_action.go @@ -19,7 +19,6 @@ package backup import ( "errors" "fmt" - "regexp" "github.com/golang/glog" @@ -27,7 +26,7 @@ import ( api "github.com/heptio/ark/pkg/apis/ark/v1" "github.com/heptio/ark/pkg/cloudprovider" - "github.com/heptio/ark/pkg/util/collections" + kubeutil "github.com/heptio/ark/pkg/util/kube" ) // volumeSnapshotAction is a struct that knows how to take snapshots of PersistentVolumes @@ -55,7 +54,7 @@ func NewVolumeSnapshotAction(snapshotService cloudprovider.SnapshotService) (Act // disk type and IOPS (if applicable) to be able to restore to current state later. func (a *volumeSnapshotAction) Execute(volume map[string]interface{}, backup *api.Backup) error { backupName := fmt.Sprintf("%s/%s", backup.Namespace, backup.Name) - if !backup.Spec.SnapshotVolumes { + if backup.Spec.SnapshotVolumes != nil && !*backup.Spec.SnapshotVolumes { glog.V(2).Infof("Backup %q has volume snapshots disabled; skipping volume snapshot action.", backupName) return nil } @@ -63,14 +62,20 @@ func (a *volumeSnapshotAction) Execute(volume map[string]interface{}, backup *ap metadata := volume["metadata"].(map[string]interface{}) name := metadata["name"].(string) - volumeID := getVolumeID(volume) + volumeID, err := kubeutil.GetVolumeID(volume) + // non-nil error means it's a supported PV source but volume ID can't be found + if err != nil { + return fmt.Errorf("error getting volume ID for backup %q, PersistentVolume %q: %v", backupName, name, err) + } + // no volumeID / nil error means unsupported PV source if volumeID == "" { - return fmt.Errorf("unable to determine volume ID for backup %q, PersistentVolume %q", backupName, name) + glog.V(2).Infof("Backup %q: PersistentVolume %q is not a supported volume type for snapshots, skipping.", backupName, name) + return nil } expiration := a.clock.Now().Add(backup.Spec.TTL.Duration) - glog.Infof("Backup %q: snapshotting PersistenVolume %q, volume-id %q, expiration %v", backupName, name, volumeID, expiration) + glog.Infof("Backup %q: snapshotting PersistentVolume %q, volume-id %q, expiration %v", backupName, name, volumeID, expiration) snapshotID, err := a.snapshotService.CreateSnapshot(volumeID) if err != nil { @@ -96,38 +101,3 @@ func (a *volumeSnapshotAction) Execute(volume map[string]interface{}, backup *ap return nil } - -var ebsVolumeIDRegex = regexp.MustCompile("vol-.*") - -func getVolumeID(pv map[string]interface{}) string { - spec, err := collections.GetMap(pv, "spec") - if err != nil { - return "" - } - - if aws, err := collections.GetMap(spec, "awsElasticBlockStore"); err == nil { - volumeID, err := collections.GetString(aws, "volumeID") - if err != nil { - return "" - } - return ebsVolumeIDRegex.FindString(volumeID) - } - - if gce, err := collections.GetMap(spec, "gcePersistentDisk"); err == nil { - volumeID, err := collections.GetString(gce, "pdName") - if err != nil { - return "" - } - return volumeID - } - - if gce, err := collections.GetMap(spec, "azureDisk"); err == nil { - volumeID, err := collections.GetString(gce, "diskName") - if err != nil { - return "" - } - return volumeID - } - - return "" -} diff --git a/pkg/backup/volume_snapshot_action_test.go b/pkg/backup/volume_snapshot_action_test.go index 77407d2a0..3c1c4f05f 100644 --- a/pkg/backup/volume_snapshot_action_test.go +++ b/pkg/backup/volume_snapshot_action_test.go @@ -34,14 +34,15 @@ func TestVolumeSnapshotAction(t *testing.T) { iops := int64(1000) tests := []struct { - name string - snapshotEnabled bool - pv string - ttl time.Duration - expectError bool - expectedVolumeID string - existingVolumeBackups map[string]*v1.VolumeBackupInfo - volumeInfo map[string]v1.VolumeBackupInfo + name string + snapshotEnabled bool + pv string + ttl time.Duration + expectError bool + expectedVolumeID string + expectedSnapshotsTaken int + existingVolumeBackups map[string]*v1.VolumeBackupInfo + volumeInfo map[string]v1.VolumeBackupInfo }{ { name: "snapshot disabled", @@ -55,10 +56,10 @@ func TestVolumeSnapshotAction(t *testing.T) { expectError: true, }, { - name: "can't find volume id - spec but no volume source defined", + name: "unsupported PV source type", snapshotEnabled: true, - pv: `{"apiVersion": "v1", "kind": "PersistentVolume", "metadata": {"name": "mypv"}, "spec": {}}`, - expectError: true, + pv: `{"apiVersion": "v1", "kind": "PersistentVolume", "metadata": {"name": "mypv"}, "spec": {"unsupportedPVSource": {}}}`, + expectError: false, }, { name: "can't find volume id - aws but no volume id", @@ -73,56 +74,73 @@ func TestVolumeSnapshotAction(t *testing.T) { expectError: true, }, { - name: "aws - simple volume id", - snapshotEnabled: true, - pv: `{"apiVersion": "v1", "kind": "PersistentVolume", "metadata": {"name": "mypv"}, "spec": {"awsElasticBlockStore": {"volumeID": "vol-abc123"}}}`, - expectError: false, - expectedVolumeID: "vol-abc123", - ttl: 5 * time.Minute, + name: "aws - simple volume id", + snapshotEnabled: true, + pv: `{"apiVersion": "v1", "kind": "PersistentVolume", "metadata": {"name": "mypv"}, "spec": {"awsElasticBlockStore": {"volumeID": "vol-abc123"}}}`, + expectError: false, + expectedSnapshotsTaken: 1, + expectedVolumeID: "vol-abc123", + ttl: 5 * time.Minute, volumeInfo: map[string]v1.VolumeBackupInfo{ "vol-abc123": v1.VolumeBackupInfo{Type: "gp", SnapshotID: "snap-1"}, }, }, { - name: "aws - simple volume id with provisioned IOPS", - snapshotEnabled: true, - pv: `{"apiVersion": "v1", "kind": "PersistentVolume", "metadata": {"name": "mypv"}, "spec": {"awsElasticBlockStore": {"volumeID": "vol-abc123"}}}`, - expectError: false, - expectedVolumeID: "vol-abc123", - ttl: 5 * time.Minute, + name: "aws - simple volume id with provisioned IOPS", + snapshotEnabled: true, + pv: `{"apiVersion": "v1", "kind": "PersistentVolume", "metadata": {"name": "mypv"}, "spec": {"awsElasticBlockStore": {"volumeID": "vol-abc123"}}}`, + expectError: false, + expectedSnapshotsTaken: 1, + expectedVolumeID: "vol-abc123", + ttl: 5 * time.Minute, volumeInfo: map[string]v1.VolumeBackupInfo{ "vol-abc123": v1.VolumeBackupInfo{Type: "io1", Iops: &iops, SnapshotID: "snap-1"}, }, }, { - name: "aws - dynamically provisioned volume id", - snapshotEnabled: true, - pv: `{"apiVersion": "v1", "kind": "PersistentVolume", "metadata": {"name": "mypv"}, "spec": {"awsElasticBlockStore": {"volumeID": "aws://us-west-2a/vol-abc123"}}}`, - expectError: false, - expectedVolumeID: "vol-abc123", - ttl: 5 * time.Minute, + name: "aws - dynamically provisioned volume id", + snapshotEnabled: true, + pv: `{"apiVersion": "v1", "kind": "PersistentVolume", "metadata": {"name": "mypv"}, "spec": {"awsElasticBlockStore": {"volumeID": "aws://us-west-2a/vol-abc123"}}}`, + expectError: false, + expectedSnapshotsTaken: 1, + expectedVolumeID: "vol-abc123", + ttl: 5 * time.Minute, volumeInfo: map[string]v1.VolumeBackupInfo{ "vol-abc123": v1.VolumeBackupInfo{Type: "gp", SnapshotID: "snap-1"}, }, }, { - name: "gce", - snapshotEnabled: true, - pv: `{"apiVersion": "v1", "kind": "PersistentVolume", "metadata": {"name": "mypv"}, "spec": {"gcePersistentDisk": {"pdName": "pd-abc123"}}}`, - expectError: false, - expectedVolumeID: "pd-abc123", - ttl: 5 * time.Minute, + name: "gce", + snapshotEnabled: true, + pv: `{"apiVersion": "v1", "kind": "PersistentVolume", "metadata": {"name": "mypv"}, "spec": {"gcePersistentDisk": {"pdName": "pd-abc123"}}}`, + expectError: false, + expectedSnapshotsTaken: 1, + expectedVolumeID: "pd-abc123", + ttl: 5 * time.Minute, volumeInfo: map[string]v1.VolumeBackupInfo{ "pd-abc123": v1.VolumeBackupInfo{Type: "gp", SnapshotID: "snap-1"}, }, }, { - name: "preexisting volume backup info in backup status", - snapshotEnabled: true, - pv: `{"apiVersion": "v1", "kind": "PersistentVolume", "metadata": {"name": "mypv"}, "spec": {"gcePersistentDisk": {"pdName": "pd-abc123"}}}`, - expectError: false, - expectedVolumeID: "pd-abc123", - ttl: 5 * time.Minute, + name: "azure", + snapshotEnabled: true, + pv: `{"apiVersion": "v1", "kind": "PersistentVolume", "metadata": {"name": "mypv"}, "spec": {"azureDisk": {"diskName": "foo-disk"}}}`, + expectError: false, + expectedSnapshotsTaken: 1, + expectedVolumeID: "foo-disk", + ttl: 5 * time.Minute, + volumeInfo: map[string]v1.VolumeBackupInfo{ + "foo-disk": v1.VolumeBackupInfo{Type: "gp", SnapshotID: "snap-1"}, + }, + }, + { + name: "preexisting volume backup info in backup status", + snapshotEnabled: true, + pv: `{"apiVersion": "v1", "kind": "PersistentVolume", "metadata": {"name": "mypv"}, "spec": {"gcePersistentDisk": {"pdName": "pd-abc123"}}}`, + expectError: false, + expectedSnapshotsTaken: 1, + expectedVolumeID: "pd-abc123", + ttl: 5 * time.Minute, existingVolumeBackups: map[string]*v1.VolumeBackupInfo{ "anotherpv": &v1.VolumeBackupInfo{SnapshotID: "anothersnap"}, }, @@ -146,7 +164,7 @@ func TestVolumeSnapshotAction(t *testing.T) { Name: "mybackup", }, Spec: v1.BackupSpec{ - SnapshotVolumes: test.snapshotEnabled, + SnapshotVolumes: &test.snapshotEnabled, TTL: metav1.Duration{Duration: test.ttl}, }, Status: v1.BackupStatus{ @@ -188,20 +206,22 @@ func TestVolumeSnapshotAction(t *testing.T) { } // we should have one snapshot taken exactly - require.Equal(t, 1, snapshotService.SnapshotsTaken.Len()) + require.Equal(t, test.expectedSnapshotsTaken, snapshotService.SnapshotsTaken.Len()) - // the snapshotID should be the one in the entry in snapshotService.SnapshottableVolumes - // for the volume we ran the test for - snapshotID, _ := snapshotService.SnapshotsTaken.PopAny() + if test.expectedSnapshotsTaken > 0 { + // the snapshotID should be the one in the entry in snapshotService.SnapshottableVolumes + // for the volume we ran the test for + snapshotID, _ := snapshotService.SnapshotsTaken.PopAny() - expectedVolumeBackups["mypv"] = &v1.VolumeBackupInfo{ - SnapshotID: snapshotID, - Type: test.volumeInfo[test.expectedVolumeID].Type, - Iops: test.volumeInfo[test.expectedVolumeID].Iops, - } + expectedVolumeBackups["mypv"] = &v1.VolumeBackupInfo{ + SnapshotID: snapshotID, + Type: test.volumeInfo[test.expectedVolumeID].Type, + Iops: test.volumeInfo[test.expectedVolumeID].Iops, + } - if e, a := expectedVolumeBackups, backup.Status.VolumeBackups; !reflect.DeepEqual(e, a) { - t.Errorf("backup.status.VolumeBackups: expected %v, got %v", e, a) + if e, a := expectedVolumeBackups, backup.Status.VolumeBackups; !reflect.DeepEqual(e, a) { + t.Errorf("backup.status.VolumeBackups: expected %v, got %v", e, a) + } } }) } diff --git a/pkg/cmd/cli/backup/create.go b/pkg/cmd/cli/backup/create.go index 1ce21c3ed..e25c66526 100644 --- a/pkg/cmd/cli/backup/create.go +++ b/pkg/cmd/cli/backup/create.go @@ -56,7 +56,7 @@ func NewCreateCommand(f client.Factory) *cobra.Command { type CreateOptions struct { Name string TTL time.Duration - SnapshotVolumes bool + SnapshotVolumes flag.OptionalBool IncludeNamespaces flag.StringArray ExcludeNamespaces flag.StringArray IncludeResources flag.StringArray @@ -70,19 +70,22 @@ func NewCreateOptions() *CreateOptions { TTL: 24 * time.Hour, IncludeNamespaces: flag.NewStringArray("*"), Labels: flag.NewMap(), - SnapshotVolumes: true, + SnapshotVolumes: flag.NewOptionalBool(nil), } } func (o *CreateOptions) BindFlags(flags *pflag.FlagSet) { flags.DurationVar(&o.TTL, "ttl", o.TTL, "how long before the backup can be garbage collected") - flags.BoolVar(&o.SnapshotVolumes, "snapshot-volumes", o.SnapshotVolumes, "take snapshots of PersistentVolumes as part of the backup") flags.Var(&o.IncludeNamespaces, "include-namespaces", "namespaces to include in the backup (use '*' for all namespaces)") flags.Var(&o.ExcludeNamespaces, "exclude-namespaces", "namespaces to exclude from the backup") flags.Var(&o.IncludeResources, "include-resources", "resources to include in the backup, formatted as resource.group, such as storageclasses.storage.k8s.io (use '*' for all resources)") flags.Var(&o.ExcludeResources, "exclude-resources", "resources to exclude from the backup, formatted as resource.group, such as storageclasses.storage.k8s.io") flags.Var(&o.Labels, "labels", "labels to apply to the backup") flags.VarP(&o.Selector, "selector", "l", "only back up resources matching this label selector") + f := flags.VarPF(&o.SnapshotVolumes, "snapshot-volumes", "", "take snapshots of PersistentVolumes as part of the backup") + // this allows the user to just specify "--snapshot-volumes" as shorthand for "--snapshot-volumes=true" + // like a normal bool flag + f.NoOptDefVal = "true" } func (o *CreateOptions) Validate(c *cobra.Command, args []string) error { @@ -120,7 +123,7 @@ func (o *CreateOptions) Run(c *cobra.Command, f client.Factory) error { IncludedResources: o.IncludeResources, ExcludedResources: o.ExcludeResources, LabelSelector: o.Selector.LabelSelector, - SnapshotVolumes: o.SnapshotVolumes, + SnapshotVolumes: o.SnapshotVolumes.Value, TTL: metav1.Duration{Duration: o.TTL}, }, } diff --git a/pkg/cmd/cli/restore/create.go b/pkg/cmd/cli/restore/create.go index 959a66883..8dd7bbf79 100644 --- a/pkg/cmd/cli/restore/create.go +++ b/pkg/cmd/cli/restore/create.go @@ -55,7 +55,7 @@ func NewCreateCommand(f client.Factory) *cobra.Command { type CreateOptions struct { BackupName string - RestoreVolumes bool + RestoreVolumes flag.OptionalBool Labels flag.Map Namespaces flag.StringArray NamespaceMappings flag.Map @@ -66,16 +66,19 @@ func NewCreateOptions() *CreateOptions { return &CreateOptions{ Labels: flag.NewMap(), NamespaceMappings: flag.NewMap().WithEntryDelimiter(",").WithKeyValueDelimiter(":"), - RestoreVolumes: true, + RestoreVolumes: flag.NewOptionalBool(nil), } } func (o *CreateOptions) BindFlags(flags *pflag.FlagSet) { - flags.BoolVar(&o.RestoreVolumes, "restore-volumes", o.RestoreVolumes, "whether to restore volumes from snapshots") flags.Var(&o.Labels, "labels", "labels to apply to the restore") flags.Var(&o.Namespaces, "namespaces", "comma-separated list of namespaces to restore") flags.Var(&o.NamespaceMappings, "namespace-mappings", "namespace mappings from name in the backup to desired restored name in the form src1:dst1,src2:dst2,...") flags.VarP(&o.Selector, "selector", "l", "only restore resources matching this label selector") + f := flags.VarPF(&o.RestoreVolumes, "restore-volumes", "", "whether to restore volumes from snapshots") + // this allows the user to just specify "--restore-volumes" as shorthand for "--restore-volumes=true" + // like a normal bool flag + f.NoOptDefVal = "true" } func (o *CreateOptions) Validate(c *cobra.Command, args []string) error { @@ -112,7 +115,7 @@ func (o *CreateOptions) Run(c *cobra.Command, f client.Factory) error { Namespaces: o.Namespaces, NamespaceMapping: o.NamespaceMappings.Data(), LabelSelector: o.Selector.LabelSelector, - RestorePVs: o.RestoreVolumes, + RestorePVs: o.RestoreVolumes.Value, }, } diff --git a/pkg/cmd/cli/schedule/create.go b/pkg/cmd/cli/schedule/create.go index feebb9793..674421f3c 100644 --- a/pkg/cmd/cli/schedule/create.go +++ b/pkg/cmd/cli/schedule/create.go @@ -103,7 +103,7 @@ func (o *CreateOptions) Run(c *cobra.Command, f client.Factory) error { IncludedResources: o.BackupOptions.IncludeResources, ExcludedResources: o.BackupOptions.ExcludeResources, LabelSelector: o.BackupOptions.Selector.LabelSelector, - SnapshotVolumes: o.BackupOptions.SnapshotVolumes, + SnapshotVolumes: o.BackupOptions.SnapshotVolumes.Value, TTL: metav1.Duration{Duration: o.BackupOptions.TTL}, }, Schedule: o.Schedule, diff --git a/pkg/cmd/server/server.go b/pkg/cmd/server/server.go index 4c5485692..0b70cc08d 100644 --- a/pkg/cmd/server/server.go +++ b/pkg/cmd/server/server.go @@ -250,7 +250,7 @@ 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") + glog.Infof("PersistentVolumeProvider config not provided, volume snapshots and restores are disabled") return nil } diff --git a/pkg/cmd/util/flag/enum.go b/pkg/cmd/util/flag/enum.go new file mode 100644 index 000000000..841097e2b --- /dev/null +++ b/pkg/cmd/util/flag/enum.go @@ -0,0 +1,65 @@ +/* +Copyright 2017 Heptio Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package flag + +import ( + "fmt" + + "k8s.io/apimachinery/pkg/util/sets" +) + +// Enum is a Cobra-compatible wrapper for defining +// a string flag that can be one of a specified set +// of values. +type Enum struct { + allowedValues sets.String + value string +} + +// NewEnum returns a new enum flag with the specified list +// of allowed values. The first value specified is used +// as the default. +func NewEnum(allowedValues ...string) Enum { + return Enum{ + allowedValues: sets.NewString(allowedValues...), + value: allowedValues[0], + } +} + +// String returns a string representation of the +// enum flag. +func (e *Enum) String() string { + return e.value +} + +// Set assigns the provided string to the enum +// receiver. It returns an error if the string +// is not an allowed value. +func (e *Enum) Set(s string) error { + if !e.allowedValues.Has(s) { + return fmt.Errorf("invalid value: %q", s) + } + + e.value = s + return nil +} + +// Type returns a string representation of the +// Enum type. +func (e *Enum) Type() string { + return "enum" +} diff --git a/pkg/cmd/util/flag/optional_bool.go b/pkg/cmd/util/flag/optional_bool.go new file mode 100644 index 000000000..ed9fa4f58 --- /dev/null +++ b/pkg/cmd/util/flag/optional_bool.go @@ -0,0 +1,60 @@ +/* +Copyright 2017 Heptio Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package flag + +import "strconv" + +type OptionalBool struct { + Value *bool +} + +func NewOptionalBool(defaultValue *bool) OptionalBool { + return OptionalBool{ + Value: defaultValue, + } +} + +// String returns a string representation of the +// enum flag. +func (f *OptionalBool) String() string { + switch f.Value { + case nil: + return "" + default: + return strconv.FormatBool(*f.Value) + } +} + +func (f *OptionalBool) Set(val string) error { + if val == "" { + f.Value = nil + return nil + } + + parsed, err := strconv.ParseBool(val) + if err != nil { + return err + } + + f.Value = &parsed + + return nil +} + +func (f *OptionalBool) Type() string { + return "optionalBool" +} diff --git a/pkg/controller/backup_controller.go b/pkg/controller/backup_controller.go index 3e7d7b019..3d7303202 100644 --- a/pkg/controller/backup_controller.go +++ b/pkg/controller/backup_controller.go @@ -49,10 +49,10 @@ import ( const backupVersion = 1 type backupController struct { - backupper backup.Backupper - backupService cloudprovider.BackupService - bucket string - allowSnapshots bool + backupper backup.Backupper + backupService cloudprovider.BackupService + bucket string + pvProviderExists bool lister listers.BackupLister listerSynced cache.InformerSynced @@ -69,13 +69,13 @@ func NewBackupController( backupper backup.Backupper, backupService cloudprovider.BackupService, bucket string, - allowSnapshots bool, + pvProviderExists bool, ) Interface { c := &backupController{ - backupper: backupper, - backupService: backupService, - bucket: bucket, - allowSnapshots: allowSnapshots, + backupper: backupper, + backupService: backupService, + bucket: bucket, + pvProviderExists: pvProviderExists, lister: backupInformer.Lister(), listerSynced: backupInformer.Informer().HasSynced, @@ -300,7 +300,7 @@ 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 { + if !controller.pvProviderExists && itm.Spec.SnapshotVolumes != nil && *itm.Spec.SnapshotVolumes { validationErrors = append(validationErrors, "Server is not configured for PV snapshots") } diff --git a/pkg/controller/backup_controller_test.go b/pkg/controller/backup_controller_test.go index 34ee5de31..261b61bcf 100644 --- a/pkg/controller/backup_controller_test.go +++ b/pkg/controller/backup_controller_test.go @@ -243,7 +243,7 @@ func TestProcessBackup(t *testing.T) { WithExcludedResources(test.expectedExcludes...). WithIncludedNamespaces(expectedNSes...). WithTTL(test.backup.Spec.TTL.Duration). - WithSnapshotVolumes(test.backup.Spec.SnapshotVolumes). + WithSnapshotVolumesPointer(test.backup.Spec.SnapshotVolumes). WithExpiration(expiration). WithVersion(1). Backup, @@ -259,7 +259,7 @@ func TestProcessBackup(t *testing.T) { WithExcludedResources(test.expectedExcludes...). WithIncludedNamespaces(expectedNSes...). WithTTL(test.backup.Spec.TTL.Duration). - WithSnapshotVolumes(test.backup.Spec.SnapshotVolumes). + WithSnapshotVolumesPointer(test.backup.Spec.SnapshotVolumes). WithExpiration(expiration). WithVersion(1). Backup, diff --git a/pkg/controller/restore_controller.go b/pkg/controller/restore_controller.go index c3f34c2a4..68350e6de 100644 --- a/pkg/controller/restore_controller.go +++ b/pkg/controller/restore_controller.go @@ -42,12 +42,12 @@ import ( ) type restoreController struct { - restoreClient arkv1client.RestoresGetter - backupClient arkv1client.BackupsGetter - restorer restore.Restorer - backupService cloudprovider.BackupService - bucket string - allowSnapshotRestores bool + restoreClient arkv1client.RestoresGetter + backupClient arkv1client.BackupsGetter + restorer restore.Restorer + backupService cloudprovider.BackupService + bucket string + pvProviderExists bool backupLister listers.BackupLister backupListerSynced cache.InformerSynced @@ -65,20 +65,20 @@ func NewRestoreController( backupService cloudprovider.BackupService, bucket string, backupInformer informers.BackupInformer, - allowSnapshotRestores bool, + pvProviderExists bool, ) Interface { c := &restoreController{ - 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"), + restoreClient: restoreClient, + backupClient: backupClient, + restorer: restorer, + backupService: backupService, + bucket: bucket, + pvProviderExists: pvProviderExists, + backupLister: backupInformer.Lister(), + backupListerSynced: backupInformer.Informer().HasSynced, + restoreLister: restoreInformer.Lister(), + restoreListerSynced: restoreInformer.Informer().HasSynced, + queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "restore"), } c.syncHandler = c.processRestore @@ -278,7 +278,7 @@ 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 { + if !controller.pvProviderExists && itm.Spec.RestorePVs != nil && *itm.Spec.RestorePVs { validationErrors = append(validationErrors, "Server is not configured for PV snapshot restores") } diff --git a/pkg/controller/schedule_controller_test.go b/pkg/controller/schedule_controller_test.go index 5e48d034d..240e4152d 100644 --- a/pkg/controller/schedule_controller_test.go +++ b/pkg/controller/schedule_controller_test.go @@ -382,7 +382,6 @@ func TestGetBackup(t *testing.T) { IncludedResources: []string{"foo", "bar"}, ExcludedResources: []string{"baz"}, LabelSelector: &metav1.LabelSelector{MatchLabels: map[string]string{"label": "value"}}, - SnapshotVolumes: true, TTL: metav1.Duration{Duration: time.Duration(300)}, }, }, @@ -399,7 +398,6 @@ func TestGetBackup(t *testing.T) { IncludedResources: []string{"foo", "bar"}, ExcludedResources: []string{"baz"}, LabelSelector: &metav1.LabelSelector{MatchLabels: map[string]string{"label": "value"}}, - SnapshotVolumes: true, TTL: metav1.Duration{Duration: time.Duration(300)}, }, }, diff --git a/pkg/restore/restore.go b/pkg/restore/restore.go index cb6df7309..a75a8b3dd 100644 --- a/pkg/restore/restore.go +++ b/pkg/restore/restore.go @@ -432,7 +432,10 @@ func (kr *kubernetesRestorer) restoreResourceForNamespace( continue } - preparedObj, err := restorer.Prepare(obj, restore, backup) + preparedObj, warning, err := restorer.Prepare(obj, restore, backup) + if warning != nil { + addToResult(&warnings, namespace, fmt.Errorf("warning preparing %s: %v", fullPath, warning)) + } if err != nil { addToResult(&errors, namespace, fmt.Errorf("error preparing %s: %v", fullPath, err)) continue diff --git a/pkg/restore/restore_test.go b/pkg/restore/restore_test.go index 99860061f..a88b2d37d 100644 --- a/pkg/restore/restore_test.go +++ b/pkg/restore/restore_test.go @@ -607,10 +607,10 @@ func newFakeCustomRestorer() *fakeCustomRestorer { } } -func (r *fakeCustomRestorer) Prepare(obj runtime.Unstructured, restore *api.Restore, backup *api.Backup) (runtime.Unstructured, error) { +func (r *fakeCustomRestorer) Prepare(obj runtime.Unstructured, restore *api.Restore, backup *api.Backup) (runtime.Unstructured, error, error) { metadata, err := collections.GetMap(obj.UnstructuredContent(), "metadata") if err != nil { - return nil, err + return nil, nil, err } if _, found := metadata["labels"]; !found { diff --git a/pkg/restore/restorers/job_restorer.go b/pkg/restore/restorers/job_restorer.go index 5242ef0f0..90f8e5960 100644 --- a/pkg/restore/restorers/job_restorer.go +++ b/pkg/restore/restorers/job_restorer.go @@ -37,11 +37,11 @@ func (r *jobRestorer) Handles(obj runtime.Unstructured, restore *api.Restore) bo return true } -func (r *jobRestorer) Prepare(obj runtime.Unstructured, restore *api.Restore, backup *api.Backup) (runtime.Unstructured, error) { +func (r *jobRestorer) Prepare(obj runtime.Unstructured, restore *api.Restore, backup *api.Backup) (runtime.Unstructured, error, error) { glog.V(4).Infof("resetting metadata and status") _, err := resetMetadataAndStatus(obj, true) if err != nil { - return nil, err + return nil, nil, err } glog.V(4).Infof("getting spec.selector.matchLabels") @@ -59,7 +59,7 @@ func (r *jobRestorer) Prepare(obj runtime.Unstructured, restore *api.Restore, ba delete(templateLabels, "controller-uid") } - return obj, nil + return obj, nil, nil } func (r *jobRestorer) Wait() bool { diff --git a/pkg/restore/restorers/job_restorer_test.go b/pkg/restore/restorers/job_restorer_test.go index 29039af29..63094418c 100644 --- a/pkg/restore/restorers/job_restorer_test.go +++ b/pkg/restore/restorers/job_restorer_test.go @@ -128,7 +128,7 @@ func TestJobRestorerPrepare(t *testing.T) { t.Run(test.name, func(t *testing.T) { restorer := NewJobRestorer() - res, err := restorer.Prepare(test.obj, nil, nil) + res, _, err := restorer.Prepare(test.obj, nil, nil) if assert.Equal(t, test.expectedErr, err != nil) { assert.Equal(t, test.expectedRes, res) diff --git a/pkg/restore/restorers/namespace_restorer.go b/pkg/restore/restorers/namespace_restorer.go index b2a679ff7..a9b9960aa 100644 --- a/pkg/restore/restorers/namespace_restorer.go +++ b/pkg/restore/restorers/namespace_restorer.go @@ -46,27 +46,27 @@ func (nsr *namespaceRestorer) Handles(obj runtime.Unstructured, restore *api.Res return false } -func (nsr *namespaceRestorer) Prepare(obj runtime.Unstructured, restore *api.Restore, backup *api.Backup) (runtime.Unstructured, error) { +func (nsr *namespaceRestorer) Prepare(obj runtime.Unstructured, restore *api.Restore, backup *api.Backup) (runtime.Unstructured, error, error) { updated, err := resetMetadataAndStatus(obj, true) if err != nil { - return nil, err + return nil, nil, err } metadata, err := collections.GetMap(obj.UnstructuredContent(), "metadata") if err != nil { - return nil, err + return nil, nil, err } currentName, err := collections.GetString(obj.UnstructuredContent(), "metadata.name") if err != nil { - return nil, err + return nil, nil, err } if newName, mapped := restore.Spec.NamespaceMapping[currentName]; mapped { metadata["name"] = newName } - return updated, nil + return updated, nil, nil } func (nsr *namespaceRestorer) Wait() bool { diff --git a/pkg/restore/restorers/namespace_restorer_test.go b/pkg/restore/restorers/namespace_restorer_test.go index 43552c89b..486106f5c 100644 --- a/pkg/restore/restorers/namespace_restorer_test.go +++ b/pkg/restore/restorers/namespace_restorer_test.go @@ -19,11 +19,12 @@ package restorers import ( "testing" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/stretchr/testify/assert" + "k8s.io/apimachinery/pkg/runtime" api "github.com/heptio/ark/pkg/apis/ark/v1" - "github.com/stretchr/testify/assert" + testutil "github.com/heptio/ark/pkg/util/test" ) func TestHandles(t *testing.T) { @@ -36,19 +37,19 @@ func TestHandles(t *testing.T) { { name: "restorable NS", obj: NewTestUnstructured().WithName("ns-1").Unstructured, - restore: newTestRestore().WithRestorableNamespace("ns-1").Restore, + restore: testutil.NewDefaultTestRestore().WithRestorableNamespace("ns-1").Restore, expect: true, }, { name: "non-restorable NS", obj: NewTestUnstructured().WithName("ns-1").Unstructured, - restore: newTestRestore().WithRestorableNamespace("ns-2").Restore, + restore: testutil.NewDefaultTestRestore().WithRestorableNamespace("ns-2").Restore, expect: false, }, { name: "namespace obj doesn't have name", obj: NewTestUnstructured().WithMetadata().Unstructured, - restore: newTestRestore().WithRestorableNamespace("ns-1").Restore, + restore: testutil.NewDefaultTestRestore().WithRestorableNamespace("ns-1").Restore, expect: false, }, } @@ -72,27 +73,27 @@ func TestPrepare(t *testing.T) { { name: "standard non-mapped namespace", obj: NewTestUnstructured().WithStatus().WithName("ns-1").Unstructured, - restore: newTestRestore().Restore, + restore: testutil.NewDefaultTestRestore().Restore, expectedErr: false, expectedRes: NewTestUnstructured().WithName("ns-1").Unstructured, }, { name: "standard mapped namespace", obj: NewTestUnstructured().WithStatus().WithName("ns-1").Unstructured, - restore: newTestRestore().WithMappedNamespace("ns-1", "ns-2").Restore, + restore: testutil.NewDefaultTestRestore().WithMappedNamespace("ns-1", "ns-2").Restore, expectedErr: false, expectedRes: NewTestUnstructured().WithName("ns-2").Unstructured, }, { name: "object without name results in error", obj: NewTestUnstructured().WithMetadata().WithStatus().Unstructured, - restore: newTestRestore().Restore, + restore: testutil.NewDefaultTestRestore().Restore, expectedErr: true, }, { name: "annotations are kept", obj: NewTestUnstructured().WithName("ns-1").WithAnnotations().Unstructured, - restore: newTestRestore().Restore, + restore: testutil.NewDefaultTestRestore().Restore, expectedErr: false, expectedRes: NewTestUnstructured().WithName("ns-1").WithAnnotations().Unstructured, }, @@ -102,7 +103,7 @@ func TestPrepare(t *testing.T) { t.Run(test.name, func(t *testing.T) { restorer := NewNamespaceRestorer() - res, err := restorer.Prepare(test.obj, test.restore, nil) + res, _, err := restorer.Prepare(test.obj, test.restore, nil) if assert.Equal(t, test.expectedErr, err != nil) { assert.Equal(t, test.expectedRes, res) @@ -110,36 +111,3 @@ func TestPrepare(t *testing.T) { }) } } - -type testRestore struct { - *api.Restore -} - -func newTestRestore() *testRestore { - return &testRestore{ - Restore: &api.Restore{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: api.DefaultNamespace, - }, - Spec: api.RestoreSpec{}, - }, - } -} - -func (r *testRestore) WithRestorableNamespace(namespace string) *testRestore { - r.Spec.Namespaces = append(r.Spec.Namespaces, namespace) - return r -} - -func (r *testRestore) WithMappedNamespace(from string, to string) *testRestore { - if r.Spec.NamespaceMapping == nil { - r.Spec.NamespaceMapping = make(map[string]string) - } - r.Spec.NamespaceMapping[from] = to - return r -} - -func (r *testRestore) WithRestorePVs(restorePVs bool) *testRestore { - r.Spec.RestorePVs = restorePVs - return r -} diff --git a/pkg/restore/restorers/pod_restorer.go b/pkg/restore/restorers/pod_restorer.go index 1f25c03a4..072c1ce15 100644 --- a/pkg/restore/restorers/pod_restorer.go +++ b/pkg/restore/restorers/pod_restorer.go @@ -43,17 +43,17 @@ var ( defaultTokenRegex = regexp.MustCompile("default-token-.*") ) -func (nsr *podRestorer) Prepare(obj runtime.Unstructured, restore *api.Restore, backup *api.Backup) (runtime.Unstructured, error) { +func (nsr *podRestorer) Prepare(obj runtime.Unstructured, restore *api.Restore, backup *api.Backup) (runtime.Unstructured, error, error) { glog.V(4).Infof("resetting metadata and status") _, err := resetMetadataAndStatus(obj, true) if err != nil { - return nil, err + return nil, nil, err } glog.V(4).Infof("getting spec") spec, err := collections.GetMap(obj.UnstructuredContent(), "spec") if err != nil { - return nil, err + return nil, nil, err } glog.V(4).Infof("deleting spec.NodeName") @@ -79,7 +79,7 @@ func (nsr *podRestorer) Prepare(obj runtime.Unstructured, restore *api.Restore, return nil }) if err != nil { - return nil, err + return nil, nil, err } glog.V(4).Infof("setting spec.volumes") @@ -114,10 +114,10 @@ func (nsr *podRestorer) Prepare(obj runtime.Unstructured, restore *api.Restore, return nil }) if err != nil { - return nil, err + return nil, nil, err } - return obj, nil + return obj, nil, nil } func (nsr *podRestorer) Wait() bool { diff --git a/pkg/restore/restorers/pod_restorer_test.go b/pkg/restore/restorers/pod_restorer_test.go index 5dbfd4d16..ac9027c47 100644 --- a/pkg/restore/restorers/pod_restorer_test.go +++ b/pkg/restore/restorers/pod_restorer_test.go @@ -98,7 +98,7 @@ func TestPodRestorerPrepare(t *testing.T) { t.Run(test.name, func(t *testing.T) { restorer := NewPodRestorer() - res, err := restorer.Prepare(test.obj, nil, nil) + res, _, err := restorer.Prepare(test.obj, nil, nil) if assert.Equal(t, test.expectedErr, err != nil) { assert.Equal(t, test.expectedRes, res) diff --git a/pkg/restore/restorers/pv_restorer.go b/pkg/restore/restorers/pv_restorer.go index 1f65d1f17..d255738e3 100644 --- a/pkg/restore/restorers/pv_restorer.go +++ b/pkg/restore/restorers/pv_restorer.go @@ -25,6 +25,7 @@ import ( api "github.com/heptio/ark/pkg/apis/ark/v1" "github.com/heptio/ark/pkg/cloudprovider" "github.com/heptio/ark/pkg/util/collections" + kubeutil "github.com/heptio/ark/pkg/util/kube" ) type persistentVolumeRestorer struct { @@ -43,35 +44,79 @@ func (sr *persistentVolumeRestorer) Handles(obj runtime.Unstructured, restore *a return true } -func (sr *persistentVolumeRestorer) Prepare(obj runtime.Unstructured, restore *api.Restore, backup *api.Backup) (runtime.Unstructured, error) { +func (sr *persistentVolumeRestorer) Prepare(obj runtime.Unstructured, restore *api.Restore, backup *api.Backup) (runtime.Unstructured, error, error) { if _, err := resetMetadataAndStatus(obj, false); err != nil { - return nil, err + return nil, nil, err } spec, err := collections.GetMap(obj.UnstructuredContent(), "spec") if err != nil { - return nil, err + return nil, nil, err } delete(spec, "claimRef") delete(spec, "storageClassName") - if restore.Spec.RestorePVs { + pvName, err := collections.GetString(obj.UnstructuredContent(), "metadata.name") + if err != nil { + return nil, nil, err + } + + // if it's an unsupported volume type for snapshot restores, we're done + if sourceType, _ := kubeutil.GetPVSource(spec); sourceType == "" { + return obj, nil, nil + } + + restoreFromSnapshot := false + + if restore.Spec.RestorePVs != nil && *restore.Spec.RestorePVs { + // when RestorePVs = yes, it's an error if we don't have a snapshot service if sr.snapshotService == nil { - return nil, errors.New("PV restorer is not configured for PV snapshot restores") + return nil, nil, errors.New("PV restorer is not configured for PV snapshot restores") } - volumeID, err := sr.restoreVolume(obj.UnstructuredContent(), restore, backup) + // if there are no snapshots in the backup, return without error + if backup.Status.VolumeBackups == nil { + return obj, nil, nil + } + + // if there are snapshots, and this is a supported PV type, but there's no + // snapshot for this PV, it's an error + if backup.Status.VolumeBackups[pvName] == nil { + return nil, nil, fmt.Errorf("no snapshot found to restore volume %s from", pvName) + } + + restoreFromSnapshot = true + } + if restore.Spec.RestorePVs == nil && sr.snapshotService != nil { + // when RestorePVs = Auto, don't error if the backup doesn't have snapshots + if backup.Status.VolumeBackups == nil || backup.Status.VolumeBackups[pvName] == nil { + return obj, nil, nil + } + + restoreFromSnapshot = true + } + + if restoreFromSnapshot { + backupInfo := backup.Status.VolumeBackups[pvName] + + volumeID, err := sr.snapshotService.CreateVolumeFromSnapshot(backupInfo.SnapshotID, backupInfo.Type, backupInfo.Iops) if err != nil { - return nil, err + return nil, nil, err } - if err := setVolumeID(spec, volumeID); err != nil { - return nil, err + if err := kubeutil.SetVolumeID(spec, volumeID); err != nil { + return nil, nil, err } } - return obj, nil + var warning error + + if sr.snapshotService == nil && len(backup.Status.VolumeBackups) > 0 { + warning = errors.New("unable to restore PV snapshots: Ark server is not configured with a PersistentVolumeProvider") + } + + return obj, warning, nil } func (sr *persistentVolumeRestorer) Wait() bool { @@ -83,39 +128,3 @@ func (sr *persistentVolumeRestorer) Ready(obj runtime.Unstructured) bool { return err == nil && phase == "Available" } - -func setVolumeID(spec map[string]interface{}, volumeID string) error { - if pvSource, found := spec["awsElasticBlockStore"]; found { - pvSourceObj := pvSource.(map[string]interface{}) - pvSourceObj["volumeID"] = volumeID - return nil - } else if pvSource, found := spec["gcePersistentDisk"]; found { - pvSourceObj := pvSource.(map[string]interface{}) - pvSourceObj["pdName"] = volumeID - return nil - } else if pvSource, found := spec["azureDisk"]; found { - pvSourceObj := pvSource.(map[string]interface{}) - pvSourceObj["diskName"] = volumeID - return nil - } - - return errors.New("persistent volume source is not compatible") -} - -func (sr *persistentVolumeRestorer) restoreVolume(item map[string]interface{}, restore *api.Restore, backup *api.Backup) (string, error) { - pvName, err := collections.GetString(item, "metadata.name") - if err != nil { - return "", err - } - - if backup.Status.VolumeBackups == nil { - return "", fmt.Errorf("VolumeBackups map not found for persistent volume %s", pvName) - } - - backupInfo, found := backup.Status.VolumeBackups[pvName] - if !found { - return "", fmt.Errorf("BackupInfo not found for PersistentVolume %s", pvName) - } - - return sr.snapshotService.CreateVolumeFromSnapshot(backupInfo.SnapshotID, backupInfo.Type, backupInfo.Iops) -} diff --git a/pkg/restore/restorers/pv_restorer_test.go b/pkg/restore/restorers/pv_restorer_test.go index 12ce423cd..a16f3fb43 100644 --- a/pkg/restore/restorers/pv_restorer_test.go +++ b/pkg/restore/restorers/pv_restorer_test.go @@ -25,6 +25,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" api "github.com/heptio/ark/pkg/apis/ark/v1" + "github.com/heptio/ark/pkg/cloudprovider" . "github.com/heptio/ark/pkg/util/test" ) @@ -32,41 +33,50 @@ func TestPVRestorerPrepare(t *testing.T) { iops := int64(1000) tests := []struct { - name string - obj runtime.Unstructured - restore *api.Restore - backup *api.Backup - volumeMap map[api.VolumeBackupInfo]string - expectedErr bool - expectedRes runtime.Unstructured + name string + obj runtime.Unstructured + restore *api.Restore + backup *api.Backup + volumeMap map[api.VolumeBackupInfo]string + noSnapshotService bool + expectedWarn bool + expectedErr bool + expectedRes runtime.Unstructured }{ { name: "no name should error", obj: NewTestUnstructured().WithMetadata().Unstructured, - restore: newTestRestore().Restore, + restore: NewDefaultTestRestore().Restore, expectedErr: true, }, { name: "no spec should error", obj: NewTestUnstructured().WithName("pv-1").Unstructured, - restore: newTestRestore().Restore, + restore: NewDefaultTestRestore().Restore, expectedErr: true, }, { name: "when RestorePVs=false, should not error if there is no PV->BackupInfo map", obj: NewTestUnstructured().WithName("pv-1").WithSpec().Unstructured, - restore: newTestRestore().WithRestorePVs(false).Restore, + restore: NewDefaultTestRestore().WithRestorePVs(false).Restore, backup: &api.Backup{Status: api.BackupStatus{}}, expectedErr: false, expectedRes: NewTestUnstructured().WithName("pv-1").WithSpec().Unstructured, }, { - name: "when RestorePVs=true, error if there is no PV->BackupInfo map", - obj: NewTestUnstructured().WithName("pv-1").WithSpec().Unstructured, - restore: newTestRestore().WithRestorePVs(true).Restore, + name: "when RestorePVs=true, return without error if there is no PV->BackupInfo map", + obj: NewTestUnstructured().WithName("pv-1").WithSpecField("awsElasticBlockStore", make(map[string]interface{})).Unstructured, + restore: NewDefaultTestRestore().WithRestorePVs(true).Restore, backup: &api.Backup{Status: api.BackupStatus{}}, + expectedErr: false, + expectedRes: NewTestUnstructured().WithName("pv-1").WithSpecField("awsElasticBlockStore", make(map[string]interface{})).Unstructured, + }, + { + name: "when RestorePVs=true, error if there is PV->BackupInfo map but no entry for this PV", + obj: NewTestUnstructured().WithName("pv-1").WithSpecField("awsElasticBlockStore", make(map[string]interface{})).Unstructured, + restore: NewDefaultTestRestore().WithRestorePVs(true).Restore, + backup: &api.Backup{Status: api.BackupStatus{VolumeBackups: map[string]*api.VolumeBackupInfo{"another-pv": &api.VolumeBackupInfo{}}}}, expectedErr: true, - expectedRes: nil, }, { name: "claimRef and storageClassName (only) should be cleared from spec", @@ -76,7 +86,7 @@ func TestPVRestorerPrepare(t *testing.T) { WithSpecField("storageClassName", "foo"). WithSpecField("foo", "bar"). Unstructured, - restore: newTestRestore().WithRestorePVs(false).Restore, + restore: NewDefaultTestRestore().WithRestorePVs(false).Restore, expectedErr: false, expectedRes: NewTestUnstructured(). WithName("pv-1"). @@ -86,7 +96,7 @@ func TestPVRestorerPrepare(t *testing.T) { { name: "when RestorePVs=true, AWS volume ID should be set correctly", obj: NewTestUnstructured().WithName("pv-1").WithSpecField("awsElasticBlockStore", make(map[string]interface{})).Unstructured, - restore: newTestRestore().WithRestorePVs(true).Restore, + restore: NewDefaultTestRestore().WithRestorePVs(true).Restore, backup: &api.Backup{Status: api.BackupStatus{VolumeBackups: map[string]*api.VolumeBackupInfo{"pv-1": &api.VolumeBackupInfo{SnapshotID: "snap-1"}}}}, volumeMap: map[api.VolumeBackupInfo]string{api.VolumeBackupInfo{SnapshotID: "snap-1"}: "volume-1"}, expectedErr: false, @@ -95,7 +105,7 @@ func TestPVRestorerPrepare(t *testing.T) { { name: "when RestorePVs=true, GCE pdName should be set correctly", obj: NewTestUnstructured().WithName("pv-1").WithSpecField("gcePersistentDisk", make(map[string]interface{})).Unstructured, - restore: newTestRestore().WithRestorePVs(true).Restore, + restore: NewDefaultTestRestore().WithRestorePVs(true).Restore, backup: &api.Backup{Status: api.BackupStatus{VolumeBackups: map[string]*api.VolumeBackupInfo{"pv-1": &api.VolumeBackupInfo{SnapshotID: "snap-1"}}}}, volumeMap: map[api.VolumeBackupInfo]string{api.VolumeBackupInfo{SnapshotID: "snap-1"}: "volume-1"}, expectedErr: false, @@ -104,37 +114,54 @@ func TestPVRestorerPrepare(t *testing.T) { { name: "when RestorePVs=true, Azure pdName should be set correctly", obj: NewTestUnstructured().WithName("pv-1").WithSpecField("azureDisk", make(map[string]interface{})).Unstructured, - restore: newTestRestore().WithRestorePVs(true).Restore, + restore: NewDefaultTestRestore().WithRestorePVs(true).Restore, backup: &api.Backup{Status: api.BackupStatus{VolumeBackups: map[string]*api.VolumeBackupInfo{"pv-1": &api.VolumeBackupInfo{SnapshotID: "snap-1"}}}}, volumeMap: map[api.VolumeBackupInfo]string{api.VolumeBackupInfo{SnapshotID: "snap-1"}: "volume-1"}, expectedErr: false, expectedRes: NewTestUnstructured().WithName("pv-1").WithSpecField("azureDisk", map[string]interface{}{"diskName": "volume-1"}).Unstructured, }, { - name: "when RestorePVs=true, unsupported PV source should cause error", + name: "when RestorePVs=true, unsupported PV source should not get snapshot restored", obj: NewTestUnstructured().WithName("pv-1").WithSpecField("unsupportedPVSource", make(map[string]interface{})).Unstructured, - restore: newTestRestore().WithRestorePVs(true).Restore, + restore: NewDefaultTestRestore().WithRestorePVs(true).Restore, backup: &api.Backup{Status: api.BackupStatus{VolumeBackups: map[string]*api.VolumeBackupInfo{"pv-1": &api.VolumeBackupInfo{SnapshotID: "snap-1"}}}}, volumeMap: map[api.VolumeBackupInfo]string{api.VolumeBackupInfo{SnapshotID: "snap-1"}: "volume-1"}, - expectedErr: true, + expectedErr: false, + expectedRes: NewTestUnstructured().WithName("pv-1").WithSpecField("unsupportedPVSource", make(map[string]interface{})).Unstructured, }, { name: "volume type and IOPS are correctly passed to CreateVolume", obj: NewTestUnstructured().WithName("pv-1").WithSpecField("awsElasticBlockStore", make(map[string]interface{})).Unstructured, - restore: newTestRestore().WithRestorePVs(true).Restore, + restore: NewDefaultTestRestore().WithRestorePVs(true).Restore, backup: &api.Backup{Status: api.BackupStatus{VolumeBackups: map[string]*api.VolumeBackupInfo{"pv-1": &api.VolumeBackupInfo{SnapshotID: "snap-1", Type: "gp", Iops: &iops}}}}, volumeMap: map[api.VolumeBackupInfo]string{api.VolumeBackupInfo{SnapshotID: "snap-1", Type: "gp", Iops: &iops}: "volume-1"}, expectedErr: false, expectedRes: NewTestUnstructured().WithName("pv-1").WithSpecField("awsElasticBlockStore", map[string]interface{}{"volumeID": "volume-1"}).Unstructured, }, + { + name: "When no SnapshotService, warn if backup has snapshots that will not be restored", + obj: NewTestUnstructured().WithName("pv-1").WithSpecField("awsElasticBlockStore", make(map[string]interface{})).Unstructured, + restore: NewDefaultTestRestore().Restore, + backup: &api.Backup{Status: api.BackupStatus{VolumeBackups: map[string]*api.VolumeBackupInfo{"pv-1": &api.VolumeBackupInfo{SnapshotID: "snap-1"}}}}, + volumeMap: map[api.VolumeBackupInfo]string{api.VolumeBackupInfo{SnapshotID: "snap-1"}: "volume-1"}, + noSnapshotService: true, + expectedErr: false, + expectedWarn: true, + expectedRes: NewTestUnstructured().WithName("pv-1").WithSpecField("awsElasticBlockStore", make(map[string]interface{})).Unstructured, + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - snapService := &FakeSnapshotService{RestorableVolumes: test.volumeMap} - restorer := NewPersistentVolumeRestorer(snapService) + var snapshotService cloudprovider.SnapshotService + if !test.noSnapshotService { + snapshotService = &FakeSnapshotService{RestorableVolumes: test.volumeMap} + } + restorer := NewPersistentVolumeRestorer(snapshotService) - res, err := restorer.Prepare(test.obj, test.restore, test.backup) + res, warn, err := restorer.Prepare(test.obj, test.restore, test.backup) + + assert.Equal(t, test.expectedWarn, warn != nil) if assert.Equal(t, test.expectedErr, err != nil) { assert.Equal(t, test.expectedRes, res) diff --git a/pkg/restore/restorers/pvc_restorer.go b/pkg/restore/restorers/pvc_restorer.go index 54b324f8b..8e7e20391 100644 --- a/pkg/restore/restorers/pvc_restorer.go +++ b/pkg/restore/restorers/pvc_restorer.go @@ -35,8 +35,10 @@ func (sr *persistentVolumeClaimRestorer) Handles(obj runtime.Unstructured, resto return true } -func (sr *persistentVolumeClaimRestorer) Prepare(obj runtime.Unstructured, restore *api.Restore, backup *api.Backup) (runtime.Unstructured, error) { - return resetMetadataAndStatus(obj, true) +func (sr *persistentVolumeClaimRestorer) Prepare(obj runtime.Unstructured, restore *api.Restore, backup *api.Backup) (runtime.Unstructured, error, error) { + res, err := resetMetadataAndStatus(obj, true) + + return res, nil, err } func (sr *persistentVolumeClaimRestorer) Wait() bool { diff --git a/pkg/restore/restorers/resource_restorer.go b/pkg/restore/restorers/resource_restorer.go index 27f94eb4f..fc18cc694 100644 --- a/pkg/restore/restorers/resource_restorer.go +++ b/pkg/restore/restorers/resource_restorer.go @@ -29,8 +29,8 @@ type ResourceRestorer interface { // Handles returns true if the Restorer should restore this object. Handles(obj runtime.Unstructured, restore *api.Restore) bool - // Prepare gets an item ready to be restored - Prepare(obj runtime.Unstructured, restore *api.Restore, backup *api.Backup) (runtime.Unstructured, error) + // Prepare gets an item ready to be restored. + Prepare(obj runtime.Unstructured, restore *api.Restore, backup *api.Backup) (res runtime.Unstructured, warning error, err error) // Wait returns true if restoration should wait for all of this restorer's resources to be ready before moving on to the next restorer. Wait() bool @@ -66,8 +66,10 @@ func (br *basicRestorer) Handles(obj runtime.Unstructured, restore *api.Restore) return true } -func (br *basicRestorer) Prepare(obj runtime.Unstructured, restore *api.Restore, backup *api.Backup) (runtime.Unstructured, error) { - return resetMetadataAndStatus(obj, br.saveAnnotations) +func (br *basicRestorer) Prepare(obj runtime.Unstructured, restore *api.Restore, backup *api.Backup) (runtime.Unstructured, error, error) { + obj, err := resetMetadataAndStatus(obj, br.saveAnnotations) + + return obj, err, nil } func (br *basicRestorer) Wait() bool { diff --git a/pkg/restore/restorers/service_restorer.go b/pkg/restore/restorers/service_restorer.go index 6c0724444..8f2d28e5d 100644 --- a/pkg/restore/restorers/service_restorer.go +++ b/pkg/restore/restorers/service_restorer.go @@ -35,21 +35,21 @@ func (sr *serviceRestorer) Handles(obj runtime.Unstructured, restore *api.Restor return true } -func (sr *serviceRestorer) Prepare(obj runtime.Unstructured, restore *api.Restore, backup *api.Backup) (runtime.Unstructured, error) { +func (sr *serviceRestorer) Prepare(obj runtime.Unstructured, restore *api.Restore, backup *api.Backup) (runtime.Unstructured, error, error) { if _, err := resetMetadataAndStatus(obj, true); err != nil { - return nil, err + return nil, nil, err } spec, err := collections.GetMap(obj.UnstructuredContent(), "spec") if err != nil { - return nil, err + return nil, nil, err } delete(spec, "clusterIP") ports, err := collections.GetSlice(obj.UnstructuredContent(), "spec.ports") if err != nil { - return nil, err + return nil, nil, err } for _, port := range ports { @@ -57,7 +57,7 @@ func (sr *serviceRestorer) Prepare(obj runtime.Unstructured, restore *api.Restor delete(p, "nodePort") } - return obj, nil + return obj, nil, nil } func (sr *serviceRestorer) Wait() bool { diff --git a/pkg/restore/restorers/service_restorer_test.go b/pkg/restore/restorers/service_restorer_test.go index ad8590535..d7c22ba5c 100644 --- a/pkg/restore/restorers/service_restorer_test.go +++ b/pkg/restore/restorers/service_restorer_test.go @@ -62,7 +62,7 @@ func TestServiceRestorerPrepare(t *testing.T) { t.Run(test.name, func(t *testing.T) { restorer := NewServiceRestorer() - res, err := restorer.Prepare(test.obj, nil, nil) + res, _, err := restorer.Prepare(test.obj, nil, nil) if assert.Equal(t, test.expectedErr, err != nil) { assert.Equal(t, test.expectedRes, res) diff --git a/pkg/util/kube/utils.go b/pkg/util/kube/utils.go index babfdebe5..3f029c720 100644 --- a/pkg/util/kube/utils.go +++ b/pkg/util/kube/utils.go @@ -17,9 +17,14 @@ limitations under the License. package kube import ( + "errors" + "regexp" + apierrors "k8s.io/apimachinery/pkg/api/errors" corev1 "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/client-go/pkg/api/v1" + + "github.com/heptio/ark/pkg/util/collections" ) // EnsureNamespaceExists attempts to create the provided Kubernetes namespace. It returns two values: @@ -35,3 +40,67 @@ func EnsureNamespaceExists(namespace *v1.Namespace, client corev1.NamespaceInter return false, err } } + +var ebsVolumeIDRegex = regexp.MustCompile("vol-.*") + +var supportedVolumeTypes = map[string]string{ + "awsElasticBlockStore": "volumeID", + "gcePersistentDisk": "pdName", + "azureDisk": "diskName", +} + +// GetVolumeID looks for a supported PV source within the provided PV unstructured +// data. It returns the appropriate volume ID field if found. If the PV source +// is supported but a volume ID cannot be found, an error is returned; if the PV +// source is not supported, zero values are returned. +func GetVolumeID(pv map[string]interface{}) (string, error) { + spec, err := collections.GetMap(pv, "spec") + if err != nil { + return "", err + } + + for volumeType, volumeIDKey := range supportedVolumeTypes { + if pvSource, err := collections.GetMap(spec, volumeType); err == nil { + volumeID, err := collections.GetString(pvSource, volumeIDKey) + if err != nil { + return "", err + } + + if volumeType == "awsElasticBlockStore" { + return ebsVolumeIDRegex.FindString(volumeID), nil + } + + return volumeID, nil + } + } + + return "", nil +} + +// GetPVSource looks for a supported PV source within the provided PV spec data. +// It returns the name of the PV source type and the unstructured source data if +// one is found, or zero values otherwise. +func GetPVSource(spec map[string]interface{}) (string, map[string]interface{}) { + for volumeType := range supportedVolumeTypes { + if pvSource, found := spec[volumeType]; found { + return volumeType, pvSource.(map[string]interface{}) + } + } + + return "", nil +} + +// SetVolumeID looks for a supported PV source within the provided PV spec data. +// If sets the appropriate ID field within the source if found, and returns an +// error if a supported PV source is not found. +func SetVolumeID(spec map[string]interface{}, volumeID string) error { + sourceType, source := GetPVSource(spec) + + if sourceType == "" { + return errors.New("persistent volume source is not compatible") + } + + source[supportedVolumeTypes[sourceType]] = volumeID + + return nil +} diff --git a/pkg/util/test/test_backup.go b/pkg/util/test/test_backup.go index d6163c724..540594808 100644 --- a/pkg/util/test/test_backup.go +++ b/pkg/util/test/test_backup.go @@ -106,6 +106,11 @@ func (b *TestBackup) WithSnapshot(pv string, snapshot string) *TestBackup { } func (b *TestBackup) WithSnapshotVolumes(value bool) *TestBackup { + b.Spec.SnapshotVolumes = &value + return b +} + +func (b *TestBackup) WithSnapshotVolumesPointer(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 996ea162a..0b1ac3904 100644 --- a/pkg/util/test/test_restore.go +++ b/pkg/util/test/test_restore.go @@ -41,6 +41,10 @@ func NewTestRestore(ns, name string, phase api.RestorePhase) *TestRestore { } } +func NewDefaultTestRestore() *TestRestore { + return NewTestRestore(api.DefaultNamespace, "", api.RestorePhase("")) +} + func (r *TestRestore) WithRestorableNamespace(name string) *TestRestore { r.Spec.Namespaces = append(r.Spec.Namespaces, name) return r @@ -62,6 +66,14 @@ func (r *TestRestore) WithErrors(e api.RestoreResult) *TestRestore { } func (r *TestRestore) WithRestorePVs(value bool) *TestRestore { - r.Spec.RestorePVs = value + r.Spec.RestorePVs = &value + return r +} + +func (r *TestRestore) WithMappedNamespace(from string, to string) *TestRestore { + if r.Spec.NamespaceMapping == nil { + r.Spec.NamespaceMapping = make(map[string]string) + } + r.Spec.NamespaceMapping[from] = to return r }