diff --git a/changelogs/unreleased/1384-skriss b/changelogs/unreleased/1384-skriss new file mode 100644 index 000000000..bf571bf53 --- /dev/null +++ b/changelogs/unreleased/1384-skriss @@ -0,0 +1 @@ +remove deprecated "hooks" for backups (they've been replaced by "pre hooks") diff --git a/docs/api-types/backup.md b/docs/api-types/backup.md index 561500400..c2d4d96b7 100644 --- a/docs/api-types/backup.md +++ b/docs/api-types/backup.md @@ -97,10 +97,6 @@ spec: app: velero component: server # An array of hooks to run before executing custom actions. Currently only "exec" hooks are supported. - # DEPRECATED. Use pre instead. - hooks: - # Same content as pre below. - # An array of hooks to run before executing custom actions. Currently only "exec" hooks are supported. pre: - # The type of hook. This must be "exec". diff --git a/pkg/apis/velero/v1/backup.go b/pkg/apis/velero/v1/backup.go index aec79aec4..1ec566f39 100644 --- a/pkg/apis/velero/v1/backup.go +++ b/pkg/apis/velero/v1/backup.go @@ -89,8 +89,6 @@ type BackupResourceHookSpec struct { ExcludedResources []string `json:"excludedResources"` // LabelSelector, if specified, filters the resources to which this hook spec applies. LabelSelector *metav1.LabelSelector `json:"labelSelector,omitempty"` - // Hooks is a list of BackupResourceHooks to execute. DEPRECATED. Replaced by PreHooks. - Hooks []BackupResourceHook `json:"hooks"` // PreHooks is a list of BackupResourceHooks to execute prior to storing the item in the backup. // These are executed before any "additional items" from item actions are processed. PreHooks []BackupResourceHook `json:"pre,omitempty"` diff --git a/pkg/apis/velero/v1/zz_generated.deepcopy.go b/pkg/apis/velero/v1/zz_generated.deepcopy.go index 0d40817b3..04b2064fe 100644 --- a/pkg/apis/velero/v1/zz_generated.deepcopy.go +++ b/pkg/apis/velero/v1/zz_generated.deepcopy.go @@ -158,13 +158,6 @@ func (in *BackupResourceHookSpec) DeepCopyInto(out *BackupResourceHookSpec) { *out = new(metav1.LabelSelector) (*in).DeepCopyInto(*out) } - if in.Hooks != nil { - in, out := &in.Hooks, &out.Hooks - *out = make([]BackupResourceHook, len(*in)) - for i := range *in { - (*in)[i].DeepCopyInto(&(*out)[i]) - } - } if in.PreHooks != nil { in, out := &in.PreHooks, &out.PreHooks *out = make([]BackupResourceHook, len(*in)) diff --git a/pkg/backup/backup.go b/pkg/backup/backup.go index 76ba90e28..e0974995a 100644 --- a/pkg/backup/backup.go +++ b/pkg/backup/backup.go @@ -183,18 +183,11 @@ func getResourceHooks(hookSpecs []api.BackupResourceHookSpec, discoveryHelper di } func getResourceHook(hookSpec api.BackupResourceHookSpec, discoveryHelper discovery.Helper) (resourceHook, error) { - // Use newer PreHooks if it's set - preHooks := hookSpec.PreHooks - if len(preHooks) == 0 { - // Fall back to Hooks otherwise (DEPRECATED) - preHooks = hookSpec.Hooks - } - h := resourceHook{ name: hookSpec.Name, namespaces: collections.NewIncludesExcludes().Includes(hookSpec.IncludedNamespaces...).Excludes(hookSpec.ExcludedNamespaces...), resources: getResourceIncludesExcludes(discoveryHelper, hookSpec.IncludedResources, hookSpec.ExcludedResources), - pre: preHooks, + pre: hookSpec.PreHooks, post: hookSpec.PostHooks, } diff --git a/pkg/backup/backup_test.go b/pkg/backup/backup_test.go index 9a5e0a18e..862973bc2 100644 --- a/pkg/backup/backup_test.go +++ b/pkg/backup/backup_test.go @@ -428,7 +428,7 @@ func TestBackup(t *testing.T) { LabelSelector: &metav1.LabelSelector{ MatchLabels: map[string]string{"1": "2"}, }, - Hooks: []v1.BackupResourceHook{ + PreHooks: []v1.BackupResourceHook{ { Exec: &v1.ExecHook{ Command: []string{"ls", "/tmp"}, @@ -652,68 +652,6 @@ func TestGetResourceHook(t *testing.T) { hookSpec v1.BackupResourceHookSpec expected resourceHook }{ - { - name: "PreHooks take priority over Hooks", - hookSpec: v1.BackupResourceHookSpec{ - Name: "spec1", - PreHooks: []v1.BackupResourceHook{ - { - Exec: &v1.ExecHook{ - Container: "a", - Command: []string{"b"}, - }, - }, - }, - Hooks: []v1.BackupResourceHook{ - { - Exec: &v1.ExecHook{ - Container: "c", - Command: []string{"d"}, - }, - }, - }, - }, - expected: resourceHook{ - name: "spec1", - namespaces: collections.NewIncludesExcludes(), - resources: collections.NewIncludesExcludes(), - pre: []v1.BackupResourceHook{ - { - Exec: &v1.ExecHook{ - Container: "a", - Command: []string{"b"}, - }, - }, - }, - }, - }, - { - name: "Use Hooks if PreHooks isn't set", - hookSpec: v1.BackupResourceHookSpec{ - Name: "spec1", - Hooks: []v1.BackupResourceHook{ - { - Exec: &v1.ExecHook{ - Container: "a", - Command: []string{"b"}, - }, - }, - }, - }, - expected: resourceHook{ - name: "spec1", - namespaces: collections.NewIncludesExcludes(), - resources: collections.NewIncludesExcludes(), - pre: []v1.BackupResourceHook{ - { - Exec: &v1.ExecHook{ - Container: "a", - Command: []string{"b"}, - }, - }, - }, - }, - }, { name: "Full test", hookSpec: v1.BackupResourceHookSpec{ diff --git a/pkg/cmd/util/output/backup_describer.go b/pkg/cmd/util/output/backup_describer.go index 7e31dd5e7..eb9b81b29 100644 --- a/pkg/cmd/util/output/backup_describer.go +++ b/pkg/cmd/util/output/backup_describer.go @@ -172,10 +172,21 @@ func DescribeBackupSpec(d *Describer, spec velerov1api.BackupSpec) { } d.Printf("\t\t\tLabel selector:\t%s\n", s) - for _, hook := range backupResourceHookSpec.Hooks { + for _, hook := range backupResourceHookSpec.PreHooks { if hook.Exec != nil { d.Println() - d.Printf("\t\t\tExec Hook:\n") + d.Printf("\t\t\tPre Exec Hook:\n") + d.Printf("\t\t\t\tContainer:\t%s\n", hook.Exec.Container) + d.Printf("\t\t\t\tCommand:\t%s\n", strings.Join(hook.Exec.Command, " ")) + d.Printf("\t\t\t\tOn Error:\t%s\n", hook.Exec.OnError) + d.Printf("\t\t\t\tTimeout:\t%s\n", hook.Exec.Timeout.Duration) + } + } + + for _, hook := range backupResourceHookSpec.PostHooks { + if hook.Exec != nil { + d.Println() + d.Printf("\t\t\tPost Exec Hook:\n") d.Printf("\t\t\t\tContainer:\t%s\n", hook.Exec.Container) d.Printf("\t\t\t\tCommand:\t%s\n", strings.Join(hook.Exec.Command, " ")) d.Printf("\t\t\t\tOn Error:\t%s\n", hook.Exec.OnError)