From 61896ab186ef909297aead05ed7e61dee574afff Mon Sep 17 00:00:00 2001 From: Adnan Abdulhussein Date: Tue, 6 Aug 2019 14:34:41 -0700 Subject: [PATCH 01/12] add --from-schedule to `velero backup create` to create backups from schedules Signed-off-by: Adnan Abdulhussein --- pkg/builder/backup_builder.go | 13 +++++++++ pkg/builder/object_meta.go | 17 +++++++++++ pkg/cmd/cli/backup/create.go | 41 +++++++++++++++++++++------ pkg/cmd/cli/schedule/create.go | 3 ++ pkg/controller/schedule_controller.go | 26 ++++------------- 5 files changed, 71 insertions(+), 29 deletions(-) diff --git a/pkg/builder/backup_builder.go b/pkg/builder/backup_builder.go index ec80f7c2e5..5bbd224ac5 100644 --- a/pkg/builder/backup_builder.go +++ b/pkg/builder/backup_builder.go @@ -73,6 +73,19 @@ func (b *BackupBuilder) ObjectMeta(opts ...ObjectMetaOpt) *BackupBuilder { return b } +// FromSchedule sets the Backup's spec and labels from the Schedule template +func (b *BackupBuilder) FromSchedule(schedule *velerov1api.Schedule) *BackupBuilder { + labels := schedule.Labels + if labels == nil { + labels = make(map[string]string) + } + labels[velerov1api.ScheduleNameLabel] = schedule.Name + + b.object.Spec = schedule.Spec.Template + b.ObjectMeta(WithLabelsMap(labels)) + return b +} + // IncludedNamespaces sets the Backup's included namespaces. func (b *BackupBuilder) IncludedNamespaces(namespaces ...string) *BackupBuilder { b.object.Spec.IncludedNamespaces = namespaces diff --git a/pkg/builder/object_meta.go b/pkg/builder/object_meta.go index 1a2b56089f..dfa6dd0c56 100644 --- a/pkg/builder/object_meta.go +++ b/pkg/builder/object_meta.go @@ -42,6 +42,23 @@ func WithLabels(vals ...string) func(obj metav1.Object) { } } +// WithLabelsMap is a functional option that applies the specified labels map to +// an object. +func WithLabelsMap(labels map[string]string) func(obj metav1.Object) { + return func(obj metav1.Object) { + objLabels := obj.GetLabels() + if objLabels == nil { + objLabels = make(map[string]string) + } + + for k, v := range labels { + objLabels[k] = v + } + + obj.SetLabels(objLabels) + } +} + // WithAnnotations is a functional option that applies the specified // annotation keys/values to an object. func WithAnnotations(vals ...string) func(obj metav1.Object) { diff --git a/pkg/cmd/cli/backup/create.go b/pkg/cmd/cli/backup/create.go index 61bc105b1b..2b72364554 100644 --- a/pkg/cmd/cli/backup/create.go +++ b/pkg/cmd/cli/backup/create.go @@ -17,9 +17,12 @@ limitations under the License. package backup import ( + "errors" "fmt" "time" + "github.com/heptio/velero/pkg/builder" + "github.com/spf13/cobra" "github.com/spf13/pflag" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -34,6 +37,8 @@ import ( v1 "github.com/heptio/velero/pkg/generated/informers/externalversions/velero/v1" ) +const DefaultBackupTTL time.Duration = 30 * 24 * time.Hour + func NewCreateCommand(f client.Factory, use string) *cobra.Command { o := NewCreateOptions() @@ -84,13 +89,14 @@ type CreateOptions struct { Wait bool StorageLocation string SnapshotLocations []string + FromSchedule string client veleroclient.Interface } func NewCreateOptions() *CreateOptions { return &CreateOptions{ - TTL: 30 * 24 * time.Hour, + TTL: DefaultBackupTTL, IncludeNamespaces: flag.NewStringArray("*"), Labels: flag.NewMap(), SnapshotVolumes: flag.NewOptionalBool(nil), @@ -108,6 +114,7 @@ func (o *CreateOptions) BindFlags(flags *pflag.FlagSet) { flags.StringVar(&o.StorageLocation, "storage-location", "", "location in which to store the backup") flags.StringSliceVar(&o.SnapshotLocations, "volume-snapshot-locations", o.SnapshotLocations, "list of locations (at most one per provider) where volume snapshots should be stored") flags.VarP(&o.Selector, "selector", "l", "only back up resources matching this label selector") + flags.StringVar(&o.FromSchedule, "from-schedule", "", "create a backup from a Schedule. Cannot be used with any other filters.") 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 @@ -128,6 +135,17 @@ func (o *CreateOptions) Validate(c *cobra.Command, args []string, f client.Facto return err } + if o.FromSchedule != "" { + if (len(o.IncludeNamespaces) > 1 || o.IncludeNamespaces[0] != "*") || + len(o.ExcludeNamespaces) > 0 || len(o.IncludeResources) > 0 || + len(o.ExcludeResources) > 0 || o.Selector.LabelSelector != nil || + o.SnapshotVolumes.Value != nil || o.TTL != DefaultBackupTTL || + o.IncludeClusterResources.Value != nil || + o.StorageLocation != "" || len(o.SnapshotLocations) > 0 { + return errors.New("backup filters cannot be set when creating a Backup from a Schedule") + } + } + if o.StorageLocation != "" { if _, err := o.client.VeleroV1().BackupStorageLocations(f.Namespace()).Get(o.StorageLocation, metav1.GetOptions{}); err != nil { return err @@ -154,13 +172,18 @@ func (o *CreateOptions) Complete(args []string, f client.Factory) error { } func (o *CreateOptions) Run(c *cobra.Command, f client.Factory) error { - backup := &api.Backup{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: f.Namespace(), - Name: o.Name, - Labels: o.Labels.Data(), - }, - Spec: api.BackupSpec{ + var backup *api.Backup + backupBuilder := builder.ForBackup(f.Namespace(), o.Name).ObjectMeta(builder.WithLabelsMap(o.Labels.Data())) + + if o.FromSchedule != "" { + schedule, err := o.client.VeleroV1().Schedules(f.Namespace()).Get(o.FromSchedule, metav1.GetOptions{}) + if err != nil { + return err + } + backup = backupBuilder.FromSchedule(schedule).Result() + } else { + backup = backupBuilder.Result() + backup.Spec = api.BackupSpec{ IncludedNamespaces: o.IncludeNamespaces, ExcludedNamespaces: o.ExcludeNamespaces, IncludedResources: o.IncludeResources, @@ -171,7 +194,7 @@ func (o *CreateOptions) Run(c *cobra.Command, f client.Factory) error { IncludeClusterResources: o.IncludeClusterResources.Value, StorageLocation: o.StorageLocation, VolumeSnapshotLocations: o.SnapshotLocations, - }, + } } if printed, err := output.PrintWithFormat(c, backup); printed || err != nil { diff --git a/pkg/cmd/cli/schedule/create.go b/pkg/cmd/cli/schedule/create.go index fc7350645a..48e5fa8635 100644 --- a/pkg/cmd/cli/schedule/create.go +++ b/pkg/cmd/cli/schedule/create.go @@ -93,6 +93,9 @@ func NewCreateOptions() *CreateOptions { func (o *CreateOptions) BindFlags(flags *pflag.FlagSet) { o.BackupOptions.BindFlags(flags) + // hide the Backup options --from-schedule flag + // TODO: see if there is a way to remove the flag + flags.MarkHidden("from-schedule") flags.StringVar(&o.Schedule, "schedule", o.Schedule, "a cron expression specifying a recurring schedule for this backup to run") } diff --git a/pkg/controller/schedule_controller.go b/pkg/controller/schedule_controller.go index ad0336e87f..e0bf31c761 100644 --- a/pkg/controller/schedule_controller.go +++ b/pkg/controller/schedule_controller.go @@ -33,7 +33,7 @@ import ( "k8s.io/client-go/tools/cache" api "github.com/heptio/velero/pkg/apis/velero/v1" - velerov1api "github.com/heptio/velero/pkg/apis/velero/v1" + "github.com/heptio/velero/pkg/builder" velerov1client "github.com/heptio/velero/pkg/generated/clientset/versioned/typed/velero/v1" informers "github.com/heptio/velero/pkg/generated/informers/externalversions/velero/v1" listers "github.com/heptio/velero/pkg/generated/listers/velero/v1" @@ -286,29 +286,15 @@ func getNextRunTime(schedule *api.Schedule, cronSchedule cron.Schedule, asOf tim } func getBackup(item *api.Schedule, timestamp time.Time) *api.Backup { - backup := &api.Backup{ - Spec: item.Spec.Template, - ObjectMeta: metav1.ObjectMeta{ - Namespace: item.Namespace, - Name: fmt.Sprintf("%s-%s", item.Name, timestamp.Format("20060102150405")), - }, - } - - addLabelsToBackup(item, backup) + name := fmt.Sprintf("%s-%s", item.Name, timestamp.Format("20060102150405")) + backup := builder. + ForBackup(item.Namespace, name). + FromSchedule(item). + Result() return backup } -func addLabelsToBackup(item *api.Schedule, backup *api.Backup) { - labels := item.Labels - if labels == nil { - labels = make(map[string]string) - } - labels[velerov1api.ScheduleNameLabel] = item.Name - - backup.Labels = labels -} - func patchSchedule(original, updated *api.Schedule, client velerov1client.SchedulesGetter) (*api.Schedule, error) { origBytes, err := json.Marshal(original) if err != nil { From bff4154a16e5048eb90d388f3c17cbe48366f315 Mon Sep 17 00:00:00 2001 From: Adnan Abdulhussein Date: Tue, 6 Aug 2019 15:01:07 -0700 Subject: [PATCH 02/12] make update Signed-off-by: Adnan Abdulhussein --- pkg/cmd/cli/backup/create.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/cmd/cli/backup/create.go b/pkg/cmd/cli/backup/create.go index 2b72364554..13e27db0da 100644 --- a/pkg/cmd/cli/backup/create.go +++ b/pkg/cmd/cli/backup/create.go @@ -21,14 +21,13 @@ import ( "fmt" "time" - "github.com/heptio/velero/pkg/builder" - "github.com/spf13/cobra" "github.com/spf13/pflag" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/tools/cache" api "github.com/heptio/velero/pkg/apis/velero/v1" + "github.com/heptio/velero/pkg/builder" "github.com/heptio/velero/pkg/client" "github.com/heptio/velero/pkg/cmd" "github.com/heptio/velero/pkg/cmd/util/flag" From fb2cb4d69d8b22ab05f9669f670be6b3cfe23132 Mon Sep 17 00:00:00 2001 From: Adnan Abdulhussein Date: Tue, 6 Aug 2019 15:02:45 -0700 Subject: [PATCH 03/12] add changelog Signed-off-by: Adnan Abdulhussein --- changelogs/unreleased/1734-prydonius | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelogs/unreleased/1734-prydonius diff --git a/changelogs/unreleased/1734-prydonius b/changelogs/unreleased/1734-prydonius new file mode 100644 index 0000000000..8d13fcbca3 --- /dev/null +++ b/changelogs/unreleased/1734-prydonius @@ -0,0 +1 @@ +adds --from-schedule flag to the `velero create backup` command to create a Backup from an existing Schedule From cab30dde499114f8be2404ce7a024ac36f30de9e Mon Sep 17 00:00:00 2001 From: Adnan Abdulhussein Date: Tue, 6 Aug 2019 15:27:05 -0700 Subject: [PATCH 04/12] reorder backupBuilder to ensure CLI-provided labels take precedence Signed-off-by: Adnan Abdulhussein --- pkg/cmd/cli/backup/create.go | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/pkg/cmd/cli/backup/create.go b/pkg/cmd/cli/backup/create.go index 13e27db0da..a023081529 100644 --- a/pkg/cmd/cli/backup/create.go +++ b/pkg/cmd/cli/backup/create.go @@ -171,31 +171,35 @@ func (o *CreateOptions) Complete(args []string, f client.Factory) error { } func (o *CreateOptions) Run(c *cobra.Command, f client.Factory) error { - var backup *api.Backup - backupBuilder := builder.ForBackup(f.Namespace(), o.Name).ObjectMeta(builder.WithLabelsMap(o.Labels.Data())) + backupBuilder := builder.ForBackup(f.Namespace(), o.Name) if o.FromSchedule != "" { schedule, err := o.client.VeleroV1().Schedules(f.Namespace()).Get(o.FromSchedule, metav1.GetOptions{}) if err != nil { return err } - backup = backupBuilder.FromSchedule(schedule).Result() + backupBuilder.FromSchedule(schedule) } else { - backup = backupBuilder.Result() - backup.Spec = api.BackupSpec{ - IncludedNamespaces: o.IncludeNamespaces, - ExcludedNamespaces: o.ExcludeNamespaces, - IncludedResources: o.IncludeResources, - ExcludedResources: o.ExcludeResources, - LabelSelector: o.Selector.LabelSelector, - SnapshotVolumes: o.SnapshotVolumes.Value, - TTL: metav1.Duration{Duration: o.TTL}, - IncludeClusterResources: o.IncludeClusterResources.Value, - StorageLocation: o.StorageLocation, - VolumeSnapshotLocations: o.SnapshotLocations, + backupBuilder. + IncludedNamespaces(o.IncludeNamespaces...). + ExcludedNamespaces(o.ExcludeNamespaces...). + IncludedResources(o.IncludeResources...). + ExcludedResources(o.ExcludeResources...). + LabelSelector(o.Selector.LabelSelector). + TTL(o.TTL). + StorageLocation(o.StorageLocation). + VolumeSnapshotLocations(o.SnapshotLocations...) + + if o.SnapshotVolumes.Value != nil { + backupBuilder.SnapshotVolumes(*o.SnapshotVolumes.Value) + } + if o.IncludeClusterResources.Value != nil { + backupBuilder.IncludeClusterResources(*o.IncludeClusterResources.Value) } } + backup := backupBuilder.ObjectMeta(builder.WithLabelsMap(o.Labels.Data())).Result() + if printed, err := output.PrintWithFormat(c, backup); printed || err != nil { return err } From dea6283dec1955342621377198c978c61cfe3c59 Mon Sep 17 00:00:00 2001 From: Adnan Abdulhussein Date: Tue, 6 Aug 2019 15:28:57 -0700 Subject: [PATCH 05/12] add comment about overwriting labels Signed-off-by: Adnan Abdulhussein --- pkg/builder/object_meta.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/builder/object_meta.go b/pkg/builder/object_meta.go index dfa6dd0c56..931da8034f 100644 --- a/pkg/builder/object_meta.go +++ b/pkg/builder/object_meta.go @@ -51,6 +51,7 @@ func WithLabelsMap(labels map[string]string) func(obj metav1.Object) { objLabels = make(map[string]string) } + // If the label already exists in the object, it will be overwritten for k, v := range labels { objLabels[k] = v } @@ -83,6 +84,7 @@ func setMapEntries(m map[string]string, vals ...string) map[string]string { key := vals[i] val := vals[i+1] + // If the label already exists in the object, it will be overwritten m[key] = val } From eb339f5457ae79eade6ccf53be613a84592df510 Mon Sep 17 00:00:00 2001 From: Adnan Abdulhussein Date: Tue, 6 Aug 2019 15:47:38 -0700 Subject: [PATCH 06/12] fix tests Signed-off-by: Adnan Abdulhussein --- pkg/controller/schedule_controller_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/controller/schedule_controller_test.go b/pkg/controller/schedule_controller_test.go index 9fc33de881..046a30141d 100644 --- a/pkg/controller/schedule_controller_test.go +++ b/pkg/controller/schedule_controller_test.go @@ -97,7 +97,7 @@ func TestProcessSchedule(t *testing.T) { fakeClockTime: "2017-01-01 12:00:00", expectedErr: false, expectedPhase: string(velerov1api.SchedulePhaseEnabled), - expectedBackupCreate: builder.ForBackup("ns", "name-20170101120000").ObjectMeta(builder.WithLabels(velerov1api.ScheduleNameLabel, "name")).NoTypeMeta().Result(), + expectedBackupCreate: builder.ForBackup("ns", "name-20170101120000").ObjectMeta(builder.WithLabels(velerov1api.ScheduleNameLabel, "name")).Result(), expectedLastBackup: "2017-01-01 12:00:00", }, { @@ -105,7 +105,7 @@ func TestProcessSchedule(t *testing.T) { schedule: newScheduleBuilder(velerov1api.SchedulePhaseEnabled).CronSchedule("@every 5m").Result(), fakeClockTime: "2017-01-01 12:00:00", expectedErr: false, - expectedBackupCreate: builder.ForBackup("ns", "name-20170101120000").ObjectMeta(builder.WithLabels(velerov1api.ScheduleNameLabel, "name")).NoTypeMeta().Result(), + expectedBackupCreate: builder.ForBackup("ns", "name-20170101120000").ObjectMeta(builder.WithLabels(velerov1api.ScheduleNameLabel, "name")).Result(), expectedLastBackup: "2017-01-01 12:00:00", }, { @@ -113,7 +113,7 @@ func TestProcessSchedule(t *testing.T) { schedule: newScheduleBuilder(velerov1api.SchedulePhaseEnabled).CronSchedule("@every 5m").LastBackupTime("2000-01-01 00:00:00").Result(), fakeClockTime: "2017-01-01 12:00:00", expectedErr: false, - expectedBackupCreate: builder.ForBackup("ns", "name-20170101120000").ObjectMeta(builder.WithLabels(velerov1api.ScheduleNameLabel, "name")).NoTypeMeta().Result(), + expectedBackupCreate: builder.ForBackup("ns", "name-20170101120000").ObjectMeta(builder.WithLabels(velerov1api.ScheduleNameLabel, "name")).Result(), expectedLastBackup: "2017-01-01 12:00:00", }, } From 74fc26587f31e817f3d19aa01c7df102c69924df Mon Sep 17 00:00:00 2001 From: Adnan Abdulhussein Date: Tue, 6 Aug 2019 15:48:05 -0700 Subject: [PATCH 07/12] builder: remove unused NoTypeMeta helper Signed-off-by: Adnan Abdulhussein --- pkg/builder/backup_builder.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/pkg/builder/backup_builder.go b/pkg/builder/backup_builder.go index 5bbd224ac5..4f3a4f27dc 100644 --- a/pkg/builder/backup_builder.go +++ b/pkg/builder/backup_builder.go @@ -164,12 +164,6 @@ func (b *BackupBuilder) StartTimestamp(val time.Time) *BackupBuilder { return b } -// NoTypeMeta removes the type meta from the Backup. -func (b *BackupBuilder) NoTypeMeta() *BackupBuilder { - b.object.TypeMeta = metav1.TypeMeta{} - return b -} - // Hooks sets the Backup's hooks. func (b *BackupBuilder) Hooks(hooks velerov1api.BackupHooks) *BackupBuilder { b.object.Spec.Hooks = hooks From 74d9af0001e9b1e961341a22e6197d91c8063365 Mon Sep 17 00:00:00 2001 From: Adnan Abdulhussein Date: Mon, 19 Aug 2019 18:05:48 -0700 Subject: [PATCH 08/12] use separate function to bind from-schedule flag Signed-off-by: Adnan Abdulhussein --- pkg/cmd/cli/backup/create.go | 8 +++++++- pkg/cmd/cli/schedule/create.go | 3 --- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/cli/backup/create.go b/pkg/cmd/cli/backup/create.go index a023081529..b41e18eaad 100644 --- a/pkg/cmd/cli/backup/create.go +++ b/pkg/cmd/cli/backup/create.go @@ -68,6 +68,7 @@ func NewCreateCommand(f client.Factory, use string) *cobra.Command { o.BindFlags(c.Flags()) o.BindWait(c.Flags()) + o.BindFromSchedule(c.Flags()) output.BindFlags(c.Flags()) output.ClearOutputFlagDefault(c) @@ -113,7 +114,6 @@ func (o *CreateOptions) BindFlags(flags *pflag.FlagSet) { flags.StringVar(&o.StorageLocation, "storage-location", "", "location in which to store the backup") flags.StringSliceVar(&o.SnapshotLocations, "volume-snapshot-locations", o.SnapshotLocations, "list of locations (at most one per provider) where volume snapshots should be stored") flags.VarP(&o.Selector, "selector", "l", "only back up resources matching this label selector") - flags.StringVar(&o.FromSchedule, "from-schedule", "", "create a backup from a Schedule. Cannot be used with any other filters.") 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 @@ -129,6 +129,12 @@ func (o *CreateOptions) BindWait(flags *pflag.FlagSet) { flags.BoolVarP(&o.Wait, "wait", "w", o.Wait, "wait for the operation to complete") } +// BindFromSchedule binds the from-schedule flag separately so it is not called +// by other create commands that reuse CreateOptions's BindFlags method. +func (o *CreateOptions) BindFromSchedule(flags *pflag.FlagSet) { + flags.StringVar(&o.FromSchedule, "from-schedule", "", "create a backup from a Schedule. Cannot be used with any other filters.") +} + func (o *CreateOptions) Validate(c *cobra.Command, args []string, f client.Factory) error { if err := output.ValidateFlags(c); err != nil { return err diff --git a/pkg/cmd/cli/schedule/create.go b/pkg/cmd/cli/schedule/create.go index 48e5fa8635..fc7350645a 100644 --- a/pkg/cmd/cli/schedule/create.go +++ b/pkg/cmd/cli/schedule/create.go @@ -93,9 +93,6 @@ func NewCreateOptions() *CreateOptions { func (o *CreateOptions) BindFlags(flags *pflag.FlagSet) { o.BackupOptions.BindFlags(flags) - // hide the Backup options --from-schedule flag - // TODO: see if there is a way to remove the flag - flags.MarkHidden("from-schedule") flags.StringVar(&o.Schedule, "schedule", o.Schedule, "a cron expression specifying a recurring schedule for this backup to run") } From 67f05f50a85d2a409f36a5aa505c139759e05416 Mon Sep 17 00:00:00 2001 From: Adnan Abdulhussein Date: Wed, 21 Aug 2019 15:54:24 -0700 Subject: [PATCH 09/12] log about ignored filters Signed-off-by: Adnan Abdulhussein --- pkg/cmd/cli/backup/create.go | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/pkg/cmd/cli/backup/create.go b/pkg/cmd/cli/backup/create.go index b41e18eaad..d663c67d84 100644 --- a/pkg/cmd/cli/backup/create.go +++ b/pkg/cmd/cli/backup/create.go @@ -17,7 +17,6 @@ limitations under the License. package backup import ( - "errors" "fmt" "time" @@ -140,17 +139,6 @@ func (o *CreateOptions) Validate(c *cobra.Command, args []string, f client.Facto return err } - if o.FromSchedule != "" { - if (len(o.IncludeNamespaces) > 1 || o.IncludeNamespaces[0] != "*") || - len(o.ExcludeNamespaces) > 0 || len(o.IncludeResources) > 0 || - len(o.ExcludeResources) > 0 || o.Selector.LabelSelector != nil || - o.SnapshotVolumes.Value != nil || o.TTL != DefaultBackupTTL || - o.IncludeClusterResources.Value != nil || - o.StorageLocation != "" || len(o.SnapshotLocations) > 0 { - return errors.New("backup filters cannot be set when creating a Backup from a Schedule") - } - } - if o.StorageLocation != "" { if _, err := o.client.VeleroV1().BackupStorageLocations(f.Namespace()).Get(o.StorageLocation, metav1.GetOptions{}); err != nil { return err @@ -210,6 +198,10 @@ func (o *CreateOptions) Run(c *cobra.Command, f client.Factory) error { return err } + if o.FromSchedule != "" { + fmt.Println("Creating backup from schedule, all other filters are ignored.") + } + var backupInformer cache.SharedIndexInformer var updates chan *api.Backup if o.Wait { From b966d18376f3d128cc11b8207ddc02e9ee9e6891 Mon Sep 17 00:00:00 2001 From: Adnan Abdulhussein Date: Thu, 22 Aug 2019 14:25:47 -0700 Subject: [PATCH 10/12] add tests for BuildBackup Signed-off-by: Adnan Abdulhussein --- pkg/cmd/cli/backup/create.go | 65 ++++++++++++---------- pkg/cmd/cli/backup/create_test.go | 89 +++++++++++++++++++++++++++++++ 2 files changed, 126 insertions(+), 28 deletions(-) create mode 100644 pkg/cmd/cli/backup/create_test.go diff --git a/pkg/cmd/cli/backup/create.go b/pkg/cmd/cli/backup/create.go index d663c67d84..b075f11319 100644 --- a/pkg/cmd/cli/backup/create.go +++ b/pkg/cmd/cli/backup/create.go @@ -26,6 +26,7 @@ import ( "k8s.io/client-go/tools/cache" api "github.com/heptio/velero/pkg/apis/velero/v1" + velerov1api "github.com/heptio/velero/pkg/apis/velero/v1" "github.com/heptio/velero/pkg/builder" "github.com/heptio/velero/pkg/client" "github.com/heptio/velero/pkg/cmd" @@ -165,35 +166,11 @@ func (o *CreateOptions) Complete(args []string, f client.Factory) error { } func (o *CreateOptions) Run(c *cobra.Command, f client.Factory) error { - backupBuilder := builder.ForBackup(f.Namespace(), o.Name) - - if o.FromSchedule != "" { - schedule, err := o.client.VeleroV1().Schedules(f.Namespace()).Get(o.FromSchedule, metav1.GetOptions{}) - if err != nil { - return err - } - backupBuilder.FromSchedule(schedule) - } else { - backupBuilder. - IncludedNamespaces(o.IncludeNamespaces...). - ExcludedNamespaces(o.ExcludeNamespaces...). - IncludedResources(o.IncludeResources...). - ExcludedResources(o.ExcludeResources...). - LabelSelector(o.Selector.LabelSelector). - TTL(o.TTL). - StorageLocation(o.StorageLocation). - VolumeSnapshotLocations(o.SnapshotLocations...) - - if o.SnapshotVolumes.Value != nil { - backupBuilder.SnapshotVolumes(*o.SnapshotVolumes.Value) - } - if o.IncludeClusterResources.Value != nil { - backupBuilder.IncludeClusterResources(*o.IncludeClusterResources.Value) - } + backup, err := o.BuildBackup(f.Namespace()) + if err != nil { + return err } - backup := backupBuilder.ObjectMeta(builder.WithLabelsMap(o.Labels.Data())).Result() - if printed, err := output.PrintWithFormat(c, backup); printed || err != nil { return err } @@ -242,7 +219,7 @@ func (o *CreateOptions) Run(c *cobra.Command, f client.Factory) error { go backupInformer.Run(stop) } - _, err := o.client.VeleroV1().Backups(backup.Namespace).Create(backup) + _, err = o.client.VeleroV1().Backups(backup.Namespace).Create(backup) if err != nil { return err } @@ -277,3 +254,35 @@ func (o *CreateOptions) Run(c *cobra.Command, f client.Factory) error { return nil } + +func (o *CreateOptions) BuildBackup(namespace string) (*velerov1api.Backup, error) { + backupBuilder := builder.ForBackup(namespace, o.Name) + + if o.FromSchedule != "" { + schedule, err := o.client.VeleroV1().Schedules(namespace).Get(o.FromSchedule, metav1.GetOptions{}) + if err != nil { + return nil, err + } + backupBuilder.FromSchedule(schedule) + } else { + backupBuilder. + IncludedNamespaces(o.IncludeNamespaces...). + ExcludedNamespaces(o.ExcludeNamespaces...). + IncludedResources(o.IncludeResources...). + ExcludedResources(o.ExcludeResources...). + LabelSelector(o.Selector.LabelSelector). + TTL(o.TTL). + StorageLocation(o.StorageLocation). + VolumeSnapshotLocations(o.SnapshotLocations...) + + if o.SnapshotVolumes.Value != nil { + backupBuilder.SnapshotVolumes(*o.SnapshotVolumes.Value) + } + if o.IncludeClusterResources.Value != nil { + backupBuilder.IncludeClusterResources(*o.IncludeClusterResources.Value) + } + } + + backup := backupBuilder.ObjectMeta(builder.WithLabelsMap(o.Labels.Data())).Result() + return backup, nil +} diff --git a/pkg/cmd/cli/backup/create_test.go b/pkg/cmd/cli/backup/create_test.go new file mode 100644 index 0000000000..393f27631d --- /dev/null +++ b/pkg/cmd/cli/backup/create_test.go @@ -0,0 +1,89 @@ +/* +Copyright 2017 the Velero contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +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 backup + +import ( + "testing" + + "github.com/heptio/velero/pkg/builder" + + "github.com/heptio/velero/pkg/generated/clientset/versioned/fake" + + velerov1api "github.com/heptio/velero/pkg/apis/velero/v1" + "github.com/stretchr/testify/assert" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +const testNamespace = "velero" + +func TestCreateOptions_BuildBackup(t *testing.T) { + o := NewCreateOptions() + o.Labels.Set("velero.io/test=true") + + backup, err := o.BuildBackup(testNamespace) + assert.NoError(t, err) + + assert.Equal(t, velerov1api.BackupSpec{ + TTL: metav1.Duration{Duration: o.TTL}, + IncludedNamespaces: []string(o.IncludeNamespaces), + SnapshotVolumes: o.SnapshotVolumes.Value, + IncludeClusterResources: o.IncludeClusterResources.Value, + }, backup.Spec) + + assert.Equal(t, map[string]string{ + "velero.io/test": "true", + }, backup.GetLabels()) +} + +func TestCreateOptions_BuildBackupFromSchedule(t *testing.T) { + o := NewCreateOptions() + o.FromSchedule = "test" + o.client = fake.NewSimpleClientset() + + t.Run("inexistant schedule", func(t *testing.T) { + _, err := o.BuildBackup(testNamespace) + assert.Error(t, err) + }) + + expectedBackupSpec := builder.ForBackup("test", testNamespace).IncludedNamespaces("test").Result().Spec + schedule := builder.ForSchedule(testNamespace, "test").Template(expectedBackupSpec).ObjectMeta(builder.WithLabels("velero.io/test", "true")).Result() + o.client.VeleroV1().Schedules(testNamespace).Create(schedule) + + t.Run("existing schedule", func(t *testing.T) { + backup, err := o.BuildBackup(testNamespace) + assert.NoError(t, err) + + assert.Equal(t, expectedBackupSpec, backup.Spec) + assert.Equal(t, map[string]string{ + "velero.io/test": "true", + velerov1api.ScheduleNameLabel: "test", + }, backup.GetLabels()) + }) + + t.Run("command line labels take precedence over schedule labels", func(t *testing.T) { + o.Labels.Set("velero.io/test=yes,custom-label=true") + backup, err := o.BuildBackup(testNamespace) + assert.NoError(t, err) + + assert.Equal(t, expectedBackupSpec, backup.Spec) + assert.Equal(t, map[string]string{ + "velero.io/test": "yes", + velerov1api.ScheduleNameLabel: "test", + "custom-label": "true", + }, backup.GetLabels()) + }) +} From 08f01a4bd690c6c51a8926e6d3001764e352a65d Mon Sep 17 00:00:00 2001 From: Adnan Abdulhussein Date: Thu, 22 Aug 2019 14:28:05 -0700 Subject: [PATCH 11/12] update flag description Signed-off-by: Adnan Abdulhussein --- pkg/cmd/cli/backup/create.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/cli/backup/create.go b/pkg/cmd/cli/backup/create.go index b075f11319..516b716e0b 100644 --- a/pkg/cmd/cli/backup/create.go +++ b/pkg/cmd/cli/backup/create.go @@ -132,7 +132,7 @@ func (o *CreateOptions) BindWait(flags *pflag.FlagSet) { // BindFromSchedule binds the from-schedule flag separately so it is not called // by other create commands that reuse CreateOptions's BindFlags method. func (o *CreateOptions) BindFromSchedule(flags *pflag.FlagSet) { - flags.StringVar(&o.FromSchedule, "from-schedule", "", "create a backup from a Schedule. Cannot be used with any other filters.") + flags.StringVar(&o.FromSchedule, "from-schedule", "", "create a backup from the template of an existing schedule. Cannot be used with any other filters.") } func (o *CreateOptions) Validate(c *cobra.Command, args []string, f client.Factory) error { From 6e5853e16fd06481d35a25d2943007bbd917476a Mon Sep 17 00:00:00 2001 From: Adnan Abdulhussein Date: Thu, 22 Aug 2019 15:57:01 -0700 Subject: [PATCH 12/12] fix duplicate import Signed-off-by: Adnan Abdulhussein --- pkg/cmd/cli/backup/create.go | 13 ++++++------- pkg/cmd/cli/backup/create_test.go | 11 +++++------ 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/pkg/cmd/cli/backup/create.go b/pkg/cmd/cli/backup/create.go index 516b716e0b..aa46bdeced 100644 --- a/pkg/cmd/cli/backup/create.go +++ b/pkg/cmd/cli/backup/create.go @@ -25,7 +25,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/tools/cache" - api "github.com/heptio/velero/pkg/apis/velero/v1" velerov1api "github.com/heptio/velero/pkg/apis/velero/v1" "github.com/heptio/velero/pkg/builder" "github.com/heptio/velero/pkg/client" @@ -180,19 +179,19 @@ func (o *CreateOptions) Run(c *cobra.Command, f client.Factory) error { } var backupInformer cache.SharedIndexInformer - var updates chan *api.Backup + var updates chan *velerov1api.Backup if o.Wait { stop := make(chan struct{}) defer close(stop) - updates = make(chan *api.Backup) + updates = make(chan *velerov1api.Backup) backupInformer = v1.NewBackupInformer(o.client, f.Namespace(), 0, nil) backupInformer.AddEventHandler( cache.FilteringResourceEventHandler{ FilterFunc: func(obj interface{}) bool { - backup, ok := obj.(*api.Backup) + backup, ok := obj.(*velerov1api.Backup) if !ok { return false } @@ -200,14 +199,14 @@ func (o *CreateOptions) Run(c *cobra.Command, f client.Factory) error { }, Handler: cache.ResourceEventHandlerFuncs{ UpdateFunc: func(_, obj interface{}) { - backup, ok := obj.(*api.Backup) + backup, ok := obj.(*velerov1api.Backup) if !ok { return } updates <- backup }, DeleteFunc: func(obj interface{}) { - backup, ok := obj.(*api.Backup) + backup, ok := obj.(*velerov1api.Backup) if !ok { return } @@ -240,7 +239,7 @@ func (o *CreateOptions) Run(c *cobra.Command, f client.Factory) error { return nil } - if backup.Status.Phase != api.BackupPhaseNew && backup.Status.Phase != api.BackupPhaseInProgress { + if backup.Status.Phase != velerov1api.BackupPhaseNew && backup.Status.Phase != velerov1api.BackupPhaseInProgress { fmt.Printf("\nBackup completed with status: %s. You may check for more information using the commands `velero backup describe %s` and `velero backup logs %s`.\n", backup.Status.Phase, backup.Name, backup.Name) return nil } diff --git a/pkg/cmd/cli/backup/create_test.go b/pkg/cmd/cli/backup/create_test.go index 393f27631d..4797a8d868 100644 --- a/pkg/cmd/cli/backup/create_test.go +++ b/pkg/cmd/cli/backup/create_test.go @@ -1,5 +1,5 @@ /* -Copyright 2017 the Velero contributors. +Copyright 2019 the Velero contributors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -19,13 +19,12 @@ package backup import ( "testing" - "github.com/heptio/velero/pkg/builder" - - "github.com/heptio/velero/pkg/generated/clientset/versioned/fake" - - velerov1api "github.com/heptio/velero/pkg/apis/velero/v1" "github.com/stretchr/testify/assert" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + velerov1api "github.com/heptio/velero/pkg/apis/velero/v1" + "github.com/heptio/velero/pkg/builder" + "github.com/heptio/velero/pkg/generated/clientset/versioned/fake" ) const testNamespace = "velero"