From de785af89d604fb1cc24f66e9d80fda25d75efcf Mon Sep 17 00:00:00 2001 From: Andy Goldstein Date: Thu, 4 Jan 2018 10:27:12 -0500 Subject: [PATCH] Support pre and post hooks. Signed-off-by: Andy Goldstein --- docs/api-types/backup.md | 10 +- docs/hooks.md | 39 +- pkg/apis/ark/v1/backup.go | 8 +- pkg/apis/ark/v1/zz_generated.deepcopy.go | 14 + pkg/backup/backup.go | 45 ++- pkg/backup/backup_test.go | 455 +++++++---------------- pkg/backup/group_backupper.go | 79 +++- pkg/backup/group_backupper_test.go | 26 ++ pkg/backup/item_backupper.go | 8 +- pkg/backup/item_backupper_test.go | 3 +- pkg/backup/item_hook_handler.go | 60 ++- pkg/backup/item_hook_handler_test.go | 320 ++++++++++------ pkg/backup/pod_action.go | 102 +++++ pkg/backup/pod_action_test.go | 127 +++++++ pkg/backup/resource_backupper_test.go | 3 +- pkg/cmd/server/plugin/plugin.go | 3 +- pkg/plugin/manager.go | 3 +- 17 files changed, 826 insertions(+), 479 deletions(-) create mode 100644 pkg/backup/pod_action.go create mode 100644 pkg/backup/pod_action_test.go diff --git a/docs/api-types/backup.md b/docs/api-types/backup.md index 75598ed9f..b5400bc8c 100644 --- a/docs/api-types/backup.md +++ b/docs/api-types/backup.md @@ -88,8 +88,12 @@ spec: matchLabels: app: ark component: server - # An array of hooks to run. Currently only "exec" hooks are supported. + # 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". exec: @@ -105,6 +109,10 @@ spec: onError: Fail # How long to wait for the command to finish executing. Defaults to 30 seconds. Optional. timeout: 10s + # An array of hooks to run after all custom actions and additional items have been + # processed. Currently only "exec" hooks are supported. + post: + # Same content as pre above. # Status about the Backup. Users should not set any data here. status: # The date and time when the Backup is eligible for garbage collection. diff --git a/docs/hooks.md b/docs/hooks.md index 5e206cb8e..5097f4090 100644 --- a/docs/hooks.md +++ b/docs/hooks.md @@ -5,19 +5,46 @@ Heptio Ark currently supports executing commands in containers in pods during a ## Backup Hooks When performing a backup, you can specify one or more commands to execute in a container in a pod -when that pod is being backed up. There are two ways to specify hooks: annotations on the pod -itself, and in the Backup spec. +when that pod is being backed up. + +Ark versions prior to v0.7.0 only support hooks that execute prior to any custom action processing +("pre" hooks). + +As of version v0.7.0, Ark also supports "post" hooks - these execute after all custom actions have +completed, as well as after all the additional items specified by custom actions have been backed +up. + +An example of when you might use both pre and post hooks is freezing a file system. If you want to +ensure that all pending disk I/O operations have completed prior to taking a snapshot, you could use +a pre hook to run `fsfreeze --freeze`. Next, Ark would take a snapshot of the disk. Finally, you +could use a post hook to run `fsfreeze --unfreeze`. + +There are two ways to specify hooks: annotations on the pod itself, and in the Backup spec. ### Specifying Hooks As Pod Annotations You can use the following annotations on a pod to make Ark execute a hook when backing up the pod: +#### Pre hooks + | Annotation Name | Description | | --- | --- | -| `hook.backup.ark.heptio.com/container` | The container where the command should be executed. Defaults to the first container in the pod. Optional. | -| `hook.backup.ark.heptio.com/command` | The command to execute. If you need multiple arguments, specify the command as a JSON array, such as `["/usr/bin/uname", "-a"]` | -| `hook.backup.ark.heptio.com/on-error` | What to do if the command returns a non-zero exit code. Defaults to Fail. Valid values are Fail and Continue. Optional. | -| `hook.backup.ark.heptio.com/timeout` | How long to wait for the command to execute. The hook is considered in error if the command exceeds the timeout. Defaults to 30s. Optional. | +| `pre.hook.backup.ark.heptio.com/container` | The container where the command should be executed. Defaults to the first container in the pod. Optional. | +| `pre.hook.backup.ark.heptio.com/command` | The command to execute. If you need multiple arguments, specify the command as a JSON array, such as `["/usr/bin/uname", "-a"]` | +| `pre.hook.backup.ark.heptio.com/on-error` | What to do if the command returns a non-zero exit code. Defaults to Fail. Valid values are Fail and Continue. Optional. | +| `pre.hook.backup.ark.heptio.com/timeout` | How long to wait for the command to execute. The hook is considered in error if the command exceeds the timeout. Defaults to 30s. Optional. | + +Ark v0.7.0+ continues to support the original (deprecated) way to specify pre hooks - without the +`pre.` prefix in the annotation names (e.g. `hook.backup.ark.heptio.com/container`). + +#### Post hooks (v0.7.0+) + +| Annotation Name | Description | +| --- | --- | +| `post.hook.backup.ark.heptio.com/container` | The container where the command should be executed. Defaults to the first container in the pod. Optional. | +| `post.hook.backup.ark.heptio.com/command` | The command to execute. If you need multiple arguments, specify the command as a JSON array, such as `["/usr/bin/uname", "-a"]` | +| `post.hook.backup.ark.heptio.com/on-error` | What to do if the command returns a non-zero exit code. Defaults to Fail. Valid values are Fail and Continue. Optional. | +| `post.hook.backup.ark.heptio.com/timeout` | How long to wait for the command to execute. The hook is considered in error if the command exceeds the timeout. Defaults to 30s. Optional. | ### Specifying Hooks in the Backup Spec diff --git a/pkg/apis/ark/v1/backup.go b/pkg/apis/ark/v1/backup.go index 3c36688ed..d16364d95 100644 --- a/pkg/apis/ark/v1/backup.go +++ b/pkg/apis/ark/v1/backup.go @@ -81,8 +81,14 @@ type BackupResourceHookSpec struct { ExcludedResources []string `json:"excludedResources"` // LabelSelector, if specified, filters the resources to which this hook spec applies. LabelSelector *metav1.LabelSelector `json:"labelSelector"` - // Hooks is a list of BackupResourceHooks to execute. + // 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"` + // PostHooks is a list of BackupResourceHooks to execute after storing the item in the backup. + // These are executed after all "additional items" from item actions are processed. + PostHooks []BackupResourceHook `json:"post,omitempty"` } // BackupResourceHook defines a hook for a resource. diff --git a/pkg/apis/ark/v1/zz_generated.deepcopy.go b/pkg/apis/ark/v1/zz_generated.deepcopy.go index 32b406edc..3f22bc749 100644 --- a/pkg/apis/ark/v1/zz_generated.deepcopy.go +++ b/pkg/apis/ark/v1/zz_generated.deepcopy.go @@ -293,6 +293,20 @@ func (in *BackupResourceHookSpec) DeepCopyInto(out *BackupResourceHookSpec) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.PreHooks != nil { + in, out := &in.PreHooks, &out.PreHooks + *out = make([]BackupResourceHook, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } + if in.PostHooks != nil { + in, out := &in.PostHooks, &out.PostHooks + *out = make([]BackupResourceHook, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } return } diff --git a/pkg/backup/backup.go b/pkg/backup/backup.go index c9f1f8f8e..35dd9053b 100644 --- a/pkg/backup/backup.go +++ b/pkg/backup/backup.go @@ -151,20 +151,10 @@ func getNamespaceIncludesExcludes(backup *api.Backup) *collections.IncludesExclu func getResourceHooks(hookSpecs []api.BackupResourceHookSpec, discoveryHelper discovery.Helper) ([]resourceHook, error) { resourceHooks := make([]resourceHook, 0, len(hookSpecs)) - for _, r := range hookSpecs { - h := resourceHook{ - name: r.Name, - namespaces: collections.NewIncludesExcludes().Includes(r.IncludedNamespaces...).Excludes(r.ExcludedNamespaces...), - resources: getResourceIncludesExcludes(discoveryHelper, r.IncludedResources, r.ExcludedResources), - hooks: r.Hooks, - } - - if r.LabelSelector != nil { - labelSelector, err := metav1.LabelSelectorAsSelector(r.LabelSelector) - if err != nil { - return []resourceHook{}, errors.WithStack(err) - } - h.labelSelector = labelSelector + for _, s := range hookSpecs { + h, err := getResourceHook(s, discoveryHelper) + if err != nil { + return []resourceHook{}, err } resourceHooks = append(resourceHooks, h) @@ -173,6 +163,33 @@ func getResourceHooks(hookSpecs []api.BackupResourceHookSpec, discoveryHelper di return resourceHooks, nil } +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, + post: hookSpec.PostHooks, + } + + if hookSpec.LabelSelector != nil { + labelSelector, err := metav1.LabelSelectorAsSelector(hookSpec.LabelSelector) + if err != nil { + return resourceHook{}, errors.WithStack(err) + } + h.labelSelector = labelSelector + } + + return h, nil +} + // Backup backs up the items specified in the Backup, placing them in a gzip-compressed tar file // written to backupFile. The finalized api.Backup is written to metadata. func (kb *kubernetesBackupper) Backup(backup *api.Backup, backupFile, logFile io.Writer, actions []ItemAction) error { diff --git a/pkg/backup/backup_test.go b/pkg/backup/backup_test.go index eef7e8a2c..e5b1c16e5 100644 --- a/pkg/backup/backup_test.go +++ b/pkg/backup/backup_test.go @@ -469,7 +469,7 @@ func TestBackup(t *testing.T) { namespaces: collections.NewIncludesExcludes().Includes("a").Excludes("b"), resources: collections.NewIncludesExcludes().Includes("configmaps").Excludes("roles.rbac.authorization.k8s.io"), labelSelector: parseLabelSelectorOrDie("1=2"), - hooks: []v1.BackupResourceHook{ + pre: []v1.BackupResourceHook{ { Exec: &v1.ExecHook{ Command: []string{"ls", "/tmp"}, @@ -623,325 +623,6 @@ func (gb *mockGroupBackupper) backupGroup(group *metav1.APIResourceList) error { return args.Error(0) } -/* -func TestBackupMethod(t *testing.T) { - // TODO ensure LabelSelector is passed through to the List() calls - backup := &v1.Backup{ - Spec: v1.BackupSpec{ - // cm - shortcut in legacy api group, namespaced - // csr - shortcut in certificates.k8s.io api group, cluster-scoped - // roles - fully qualified in rbac.authorization.k8s.io api group, namespaced - IncludedResources: []string{"cm", "csr", "roles"}, - IncludedNamespaces: []string{"a", "b"}, - ExcludedNamespaces: []string{"c", "d"}, - }, - } - - configMapsResource := metav1.APIResource{ - Name: "configmaps", - SingularName: "configmap", - Namespaced: true, - Kind: "ConfigMap", - Verbs: metav1.Verbs([]string{"create", "update", "get", "list", "watch", "delete"}), - ShortNames: []string{"cm"}, - Categories: []string{"all"}, - } - - podsResource := metav1.APIResource{ - Name: "pods", - SingularName: "pod", - Namespaced: true, - Kind: "Pod", - Verbs: metav1.Verbs([]string{"create", "update", "get", "list", "watch", "delete"}), - ShortNames: []string{"po"}, - Categories: []string{"all"}, - } - - rolesResource := metav1.APIResource{ - Name: "roles", - SingularName: "role", - Namespaced: true, - Kind: "Role", - Verbs: metav1.Verbs([]string{"create", "update", "get", "list", "watch", "delete"}), - } - - certificateSigningRequestsResource := metav1.APIResource{ - Name: "certificatesigningrequests", - SingularName: "certificatesigningrequest", - Namespaced: false, - Kind: "CertificateSigningRequest", - Verbs: metav1.Verbs([]string{"create", "update", "get", "list", "watch", "delete"}), - ShortNames: []string{"csr"}, - } - - discoveryHelper := &arktest.FakeDiscoveryHelper{ - Mapper: &arktest.FakeMapper{ - Resources: map[schema.GroupVersionResource]schema.GroupVersionResource{ - schema.GroupVersionResource{Resource: "cm"}: schema.GroupVersionResource{Group: "", Version: "v1", Resource: "configmaps"}, - schema.GroupVersionResource{Resource: "csr"}: schema.GroupVersionResource{Group: "certificates.k8s.io", Version: "v1beta1", Resource: "certificatesigningrequests"}, - schema.GroupVersionResource{Resource: "roles"}: schema.GroupVersionResource{Group: "rbac.authorization.k8s.io", Version: "v1beta1", Resource: "roles"}, - }, - }, - ResourceList: []*metav1.APIResourceList{ - { - GroupVersion: "v1", - APIResources: []metav1.APIResource{configMapsResource, podsResource}, - }, - { - GroupVersion: "certificates.k8s.io/v1beta1", - APIResources: []metav1.APIResource{certificateSigningRequestsResource}, - }, - { - GroupVersion: "rbac.authorization.k8s.io/v1beta1", - APIResources: []metav1.APIResource{rolesResource}, - }, - }, - } - - dynamicFactory := &arktest.FakeDynamicFactory{} - - legacyGV := schema.GroupVersionResource{Version: "v1"} - - configMapsClientA := &arktest.FakeDynamicClient{} - configMapsA := toRuntimeObject(t, `{ - "apiVersion": "v1", - "kind": "ConfigMapList", - "items": [ - { - "metadata": { - "namespace":"a", - "name":"configMap1" - }, - "data": { - "a": "b" - } - } - ] - }`) - configMapsClientA.On("List", metav1.ListOptions{}).Return(configMapsA, nil) - dynamicFactory.On("ClientForGroupVersionResource", legacyGV, configMapsResource, "a").Return(configMapsClientA, nil) - - configMapsClientB := &arktest.FakeDynamicClient{} - configMapsB := toRuntimeObject(t, `{ - "apiVersion": "v1", - "kind": "ConfigMapList", - "items": [ - { - "metadata": { - "namespace":"b", - "name":"configMap2" - }, - "data": { - "c": "d" - } - } - ] - }`) - configMapsClientB.On("List", metav1.ListOptions{}).Return(configMapsB, nil) - dynamicFactory.On("ClientForGroupVersionResource", legacyGV, configMapsResource, "b").Return(configMapsClientB, nil) - - certificatesGV := schema.GroupVersionResource{Group: "certificates.k8s.io", Version: "v1beta1"} - - csrList := toRuntimeObject(t, `{ - "apiVersion": "certificates.k8s.io/v1beta1", - "kind": "CertificateSigningRequestList", - "items": [ - { - "metadata": { - "name": "csr1" - }, - "spec": { - "request": "some request", - "username": "bob", - "uid": "12345", - "groups": [ - "group1", - "group2" - ] - }, - "status": { - "certificate": "some cert" - } - } - ] - }`) - csrClient := &arktest.FakeDynamicClient{} - csrClient.On("List", metav1.ListOptions{}).Return(csrList, nil) - dynamicFactory.On("ClientForGroupVersionResource", certificatesGV, certificateSigningRequestsResource, "").Return(csrClient, nil) - - roleListA := toRuntimeObject(t, `{ - "apiVersion": "rbac.authorization.k8s.io/v1beta1", - "kind": "RoleList", - "items": [ - { - "metadata": { - "namespace": "a", - "name": "role1" - }, - "rules": [ - { - "verbs": ["get","list"], - "apiGroups": ["apps","extensions"], - "resources": ["deployments"] - } - ] - } - ] - }`) - - roleListB := toRuntimeObject(t, `{ - "apiVersion": "rbac.authorization.k8s.io/v1beta1", - "kind": "RoleList", - "items": [] - }`) - - rbacGV := schema.GroupVersionResource{Group: "rbac.authorization.k8s.io", Version: "v1beta1"} - - rolesClientA := &arktest.FakeDynamicClient{} - rolesClientA.On("List", metav1.ListOptions{}).Return(roleListA, nil) - dynamicFactory.On("ClientForGroupVersionResource", rbacGV, rolesResource, "a").Return(rolesClientA, nil) - rolesClientB := &arktest.FakeDynamicClient{} - rolesClientB.On("List", metav1.ListOptions{}).Return(roleListB, nil) - dynamicFactory.On("ClientForGroupVersionResource", rbacGV, rolesResource, "b").Return(rolesClientB, nil) - - cmAction := &fakeAction{} - csrAction := &fakeAction{} - - actions := map[string]Action{ - "cm": cmAction, - "csr": csrAction, - } - - podCommandExecutor := &arktest.PodCommandExecutor{} - defer podCommandExecutor.AssertExpectations(t) - - backupper, err := NewKubernetesBackupper(discoveryHelper, dynamicFactory, actions, podCommandExecutor) - require.NoError(t, err) - - var output, log bytes.Buffer - err = backupper.Backup(backup, &output, &log) - defer func() { - // print log if anything failed - if t.Failed() { - gzr, err := gzip.NewReader(&log) - require.NoError(t, err) - t.Log("Backup log contents:") - var buf bytes.Buffer - _, err = io.Copy(&buf, gzr) - require.NoError(t, err) - require.NoError(t, gzr.Close()) - t.Log(buf.String()) - } - }() - require.NoError(t, err) - - expectedFiles := sets.NewString( - "resources/configmaps/namespaces/a/configMap1.json", - "resources/configmaps/namespaces/b/configMap2.json", - "resources/roles.rbac.authorization.k8s.io/namespaces/a/role1.json", - // CSRs are not expected because they're unrelated cluster-scoped resources - ) - - expectedData := map[string]string{ - "resources/configmaps/namespaces/a/configMap1.json": ` - { - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": { - "namespace":"a", - "name":"configMap1" - }, - "data": { - "a": "b" - } - }`, - "resources/configmaps/namespaces/b/configMap2.json": ` - { - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": { - "namespace":"b", - "name":"configMap2" - }, - "data": { - "c": "d" - } - } - `, - "resources/roles.rbac.authorization.k8s.io/namespaces/a/role1.json": ` - { - "apiVersion": "rbac.authorization.k8s.io/v1beta1", - "kind": "Role", - "metadata": { - "namespace":"a", - "name": "role1" - }, - "rules": [ - { - "verbs": ["get","list"], - "apiGroups": ["apps","extensions"], - "resources": ["deployments"] - } - ] - } - `, - // CSRs are not expected because they're unrelated cluster-scoped resources - } - - seenFiles := sets.NewString() - - gzipReader, err := gzip.NewReader(&output) - require.NoError(t, err) - defer gzipReader.Close() - - tarReader := tar.NewReader(gzipReader) - for { - header, err := tarReader.Next() - if err == io.EOF { - break - } - require.NoError(t, err) - - switch header.Typeflag { - case tar.TypeReg: - seenFiles.Insert(header.Name) - expected, err := getAsMap(expectedData[header.Name]) - if !assert.NoError(t, err, "%q: %v", header.Name, err) { - continue - } - - buf := new(bytes.Buffer) - n, err := io.Copy(buf, tarReader) - if !assert.NoError(t, err) { - continue - } - - if !assert.Equal(t, header.Size, n) { - continue - } - - actual, err := getAsMap(string(buf.Bytes())) - if !assert.NoError(t, err) { - continue - } - assert.Equal(t, expected, actual) - default: - t.Errorf("unexpected header: %#v", header) - } - } - - if !expectedFiles.Equal(seenFiles) { - t.Errorf("did not get expected files. expected-seen: %v. seen-expected: %v", expectedFiles.Difference(seenFiles), seenFiles.Difference(expectedFiles)) - } - - expectedCMActionIDs := []string{"a/configMap1", "b/configMap2"} - - assert.Equal(t, expectedCMActionIDs, cmAction.ids) - // CSRs are not expected because they're unrelated cluster-scoped resources - assert.Nil(t, csrAction.ids) -} -*/ - func getAsMap(j string) (map[string]interface{}, error) { m := make(map[string]interface{}) err := json.Unmarshal([]byte(j), &m) @@ -961,3 +642,137 @@ func unstructuredOrDie(data string) *unstructured.Unstructured { } return o.(*unstructured.Unstructured) } + +func TestGetResourceHook(t *testing.T) { + tests := []struct { + name string + 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{ + Name: "spec1", + IncludedNamespaces: []string{"ns1", "ns2"}, + ExcludedNamespaces: []string{"ns3", "ns4"}, + IncludedResources: []string{"foo", "fie"}, + ExcludedResources: []string{"bar", "baz"}, + PreHooks: []v1.BackupResourceHook{ + { + Exec: &v1.ExecHook{ + Container: "a", + Command: []string{"b"}, + }, + }, + }, + PostHooks: []v1.BackupResourceHook{ + { + Exec: &v1.ExecHook{ + Container: "c", + Command: []string{"d"}, + }, + }, + }, + }, + expected: resourceHook{ + name: "spec1", + namespaces: collections.NewIncludesExcludes().Includes("ns1", "ns2").Excludes("ns3", "ns4"), + resources: collections.NewIncludesExcludes().Includes("foodies.somegroup", "fields.somegroup").Excludes("barnacles.anothergroup", "bazaars.anothergroup"), + pre: []v1.BackupResourceHook{ + { + Exec: &v1.ExecHook{ + Container: "a", + Command: []string{"b"}, + }, + }, + }, + post: []v1.BackupResourceHook{ + { + Exec: &v1.ExecHook{ + Container: "c", + Command: []string{"d"}, + }, + }, + }, + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + resources := map[schema.GroupVersionResource]schema.GroupVersionResource{ + {Resource: "foo"}: {Group: "somegroup", Resource: "foodies"}, + {Resource: "fie"}: {Group: "somegroup", Resource: "fields"}, + {Resource: "bar"}: {Group: "anothergroup", Resource: "barnacles"}, + {Resource: "baz"}: {Group: "anothergroup", Resource: "bazaars"}, + } + discoveryHelper := arktest.NewFakeDiscoveryHelper(false, resources) + + actual, err := getResourceHook(test.hookSpec, discoveryHelper) + require.NoError(t, err) + assert.Equal(t, test.expected, actual) + }) + } +} diff --git a/pkg/backup/group_backupper.go b/pkg/backup/group_backupper.go index f148a8f98..63b2e6f26 100644 --- a/pkg/backup/group_backupper.go +++ b/pkg/backup/group_backupper.go @@ -17,11 +17,14 @@ limitations under the License. package backup import ( + "sort" "strings" + "github.com/pkg/errors" "github.com/sirupsen/logrus" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" kuberrs "k8s.io/apimachinery/pkg/util/errors" "github.com/heptio/ark/pkg/apis/ark/v1" @@ -110,7 +113,6 @@ type defaultGroupBackupper struct { func (gb *defaultGroupBackupper) backupGroup(group *metav1.APIResourceList) error { var ( errs []error - pv *metav1.APIResource log = gb.log.WithField("group", group.GroupVersion) rb = gb.resourceBackupperFactory.newResourceBackupper( log, @@ -132,26 +134,69 @@ func (gb *defaultGroupBackupper) backupGroup(group *metav1.APIResourceList) erro log.Infof("Backing up group") - processResource := func(resource metav1.APIResource) { + // Parse so we can check if this is the core group + gv, err := schema.ParseGroupVersion(group.GroupVersion) + if err != nil { + return errors.Wrapf(err, "error parsing GroupVersion %q", group.GroupVersion) + } + if gv.Group == "" { + // This is the core group, so make sure we process in the following order: pods, pvcs, pvs, + // everything else. + sortCoreGroup(group) + } + + for _, resource := range group.APIResources { if err := rb.backupResource(group, resource); err != nil { errs = append(errs, err) } } - for _, resource := range group.APIResources { - // do PVs last because if we're also backing up PVCs, we want to backup PVs within the scope of - // the PVCs (within the PVC action) to allow for hooks to run - if strings.ToLower(resource.Name) == "persistentvolumes" && strings.ToLower(group.GroupVersion) == "v1" { - pvResource := resource - pv = &pvResource - continue - } - processResource(resource) - } - - if pv != nil { - processResource(*pv) - } - return kuberrs.NewAggregate(errs) } + +// sortCoreGroup sorts group as a coreGroup. +func sortCoreGroup(group *metav1.APIResourceList) { + sort.Stable(coreGroup(group.APIResources)) +} + +// coreGroup is used to sort APIResources in the core API group. The sort order is pods, pvcs, pvs, +// then everything else. +type coreGroup []metav1.APIResource + +func (c coreGroup) Len() int { + return len(c) +} + +func (c coreGroup) Less(i, j int) bool { + return coreGroupResourcePriority(c[i].Name) < coreGroupResourcePriority(c[j].Name) +} + +func (c coreGroup) Swap(i, j int) { + c[j], c[i] = c[i], c[j] +} + +// These constants represent the relative priorities for resources in the core API group. We want to +// ensure that we process pods, then pvcs, then pvs, then anything else. This ensures that when a +// pod is backed up, we can perform a pre hook, then process pvcs and pvs (including taking a +// snapshot), then perform a post hook on the pod. +const ( + pod = iota + pvc + pv + other +) + +// coreGroupResourcePriority returns the relative priority of the resource, in the following order: +// pods, pvcs, pvs, everything else. +func coreGroupResourcePriority(resource string) int { + switch strings.ToLower(resource) { + case "pods": + return pod + case "persistentvolumeclaims": + return pvc + case "persistentvolumes": + return pv + } + + return other +} diff --git a/pkg/backup/group_backupper_test.go b/pkg/backup/group_backupper_test.go index f150b442a..0f4fb3592 100644 --- a/pkg/backup/group_backupper_test.go +++ b/pkg/backup/group_backupper_test.go @@ -188,3 +188,29 @@ func (rb *mockResourceBackupper) backupResource(group *metav1.APIResourceList, r args := rb.Called(group, resource) return args.Error(0) } + +func TestSortCoreGroup(t *testing.T) { + group := &metav1.APIResourceList{ + GroupVersion: "v1", + APIResources: []metav1.APIResource{ + {Name: "persistentvolumes"}, + {Name: "configmaps"}, + {Name: "antelopes"}, + {Name: "persistentvolumeclaims"}, + {Name: "pods"}, + }, + } + + sortCoreGroup(group) + + expected := []string{ + "pods", + "persistentvolumeclaims", + "persistentvolumes", + "configmaps", + "antelopes", + } + for i, r := range group.APIResources { + assert.Equal(t, expected[i], r.Name) + } +} diff --git a/pkg/backup/item_backupper.go b/pkg/backup/item_backupper.go index ec5015113..340cdf433 100644 --- a/pkg/backup/item_backupper.go +++ b/pkg/backup/item_backupper.go @@ -165,7 +165,8 @@ func (ib *defaultItemBackupper) backupItem(logger logrus.FieldLogger, obj runtim // Never save status delete(obj.UnstructuredContent(), "status") - if err := ib.itemHookHandler.handleHooks(log, groupResource, obj, ib.resourceHooks); err != nil { + log.Info("Executing pre hooks") + if err := ib.itemHookHandler.handleHooks(log, groupResource, obj, ib.resourceHooks, hookPhasePre); err != nil { return err } @@ -232,6 +233,11 @@ func (ib *defaultItemBackupper) backupItem(logger logrus.FieldLogger, obj runtim } } + log.Info("Executing post hooks") + if err := ib.itemHookHandler.handleHooks(log, groupResource, obj, ib.resourceHooks, hookPhasePost); err != nil { + return err + } + var filePath string if namespace != "" { filePath = filepath.Join(api.ResourcesDir, groupResource.String(), api.NamespaceScopedDir, namespace, name+".json") diff --git a/pkg/backup/item_backupper_test.go b/pkg/backup/item_backupper_test.go index 45f8f5626..f98ba1291 100644 --- a/pkg/backup/item_backupper_test.go +++ b/pkg/backup/item_backupper_test.go @@ -343,7 +343,8 @@ func TestBackupItemNoSkips(t *testing.T) { b.additionalItemBackupper = additionalItemBackupper obj := &unstructured.Unstructured{Object: item} - itemHookHandler.On("handleHooks", mock.Anything, groupResource, obj, resourceHooks).Return(nil) + itemHookHandler.On("handleHooks", mock.Anything, groupResource, obj, resourceHooks, hookPhasePre).Return(nil) + itemHookHandler.On("handleHooks", mock.Anything, groupResource, obj, resourceHooks, hookPhasePost).Return(nil) for i, item := range test.customActionAdditionalItemIdentifiers { itemClient := &arktest.FakeDynamicClient{} diff --git a/pkg/backup/item_hook_handler.go b/pkg/backup/item_hook_handler.go index e9655314d..b56258925 100644 --- a/pkg/backup/item_hook_handler.go +++ b/pkg/backup/item_hook_handler.go @@ -18,6 +18,7 @@ package backup import ( "encoding/json" + "fmt" "time" api "github.com/heptio/ark/pkg/apis/ark/v1" @@ -31,13 +32,26 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" ) +type hookPhase string + +const ( + hookPhasePre hookPhase = "pre" + hookPhasePost hookPhase = "post" +) + // itemHookHandler invokes hooks for an item. type itemHookHandler interface { // handleHooks invokes hooks for an item. If the item is a pod and the appropriate annotations exist // to specify a hook, that is executed. Otherwise, this looks at the backup context's Backup to // determine if there are any hooks relevant to the item, taking into account the hook spec's // namespaces, resources, and label selector. - handleHooks(log logrus.FieldLogger, groupResource schema.GroupResource, obj runtime.Unstructured, resourceHooks []resourceHook) error + handleHooks( + log logrus.FieldLogger, + groupResource schema.GroupResource, + obj runtime.Unstructured, + resourceHooks []resourceHook, + phase hookPhase, + ) error } // defaultItemHookHandler is the default itemHookHandler. @@ -50,6 +64,7 @@ func (h *defaultItemHookHandler) handleHooks( groupResource schema.GroupResource, obj runtime.Unstructured, resourceHooks []resourceHook, + phase hookPhase, ) error { // We only support hooks on pods right now if groupResource != podsGroupResource { @@ -65,7 +80,12 @@ func (h *defaultItemHookHandler) handleHooks( name := metadata.GetName() // If the pod has the hook specified via annotations, that takes priority. - if hookFromAnnotations := getPodExecHookFromAnnotations(metadata.GetAnnotations()); hookFromAnnotations != nil { + hookFromAnnotations := getPodExecHookFromAnnotations(metadata.GetAnnotations(), phase) + if phase == hookPhasePre && hookFromAnnotations == nil { + // See if the pod has the legacy hook annotation keys (i.e. without a phase specified) + hookFromAnnotations = getPodExecHookFromAnnotations(metadata.GetAnnotations(), "") + } + if hookFromAnnotations != nil { hookLog := log.WithFields( logrus.Fields{ "hookSource": "annotation", @@ -89,7 +109,13 @@ func (h *defaultItemHookHandler) handleHooks( continue } - for _, hook := range resourceHook.hooks { + var hooks []api.BackupResourceHook + if phase == hookPhasePre { + hooks = resourceHook.pre + } else { + hooks = resourceHook.post + } + for _, hook := range hooks { if groupResource == podsGroupResource { if hook.Exec != nil { hookLog := log.WithFields( @@ -122,13 +148,22 @@ const ( defaultHookTimeout = 30 * time.Second ) +func phasedKey(phase hookPhase, key string) string { + if phase != "" { + return fmt.Sprintf("%v.%v", phase, key) + } + return string(key) +} + +func getHookAnnotation(annotations map[string]string, key string, phase hookPhase) string { + return annotations[phasedKey(phase, key)] +} + // getPodExecHookFromAnnotations returns an ExecHook based on the annotations, as long as the // 'command' annotation is present. If it is absent, this returns nil. -func getPodExecHookFromAnnotations(annotations map[string]string) *api.ExecHook { - container := annotations[podBackupHookContainerAnnotationKey] - - commandValue, ok := annotations[podBackupHookCommandAnnotationKey] - if !ok { +func getPodExecHookFromAnnotations(annotations map[string]string, phase hookPhase) *api.ExecHook { + commandValue := getHookAnnotation(annotations, podBackupHookCommandAnnotationKey, phase) + if commandValue == "" { return nil } var command []string @@ -141,13 +176,15 @@ func getPodExecHookFromAnnotations(annotations map[string]string) *api.ExecHook command = append(command, commandValue) } - onError := api.HookErrorMode(annotations[podBackupHookOnErrorAnnotationKey]) + container := getHookAnnotation(annotations, podBackupHookContainerAnnotationKey, phase) + + onError := api.HookErrorMode(getHookAnnotation(annotations, podBackupHookOnErrorAnnotationKey, phase)) if onError != api.HookErrorModeContinue && onError != api.HookErrorModeFail { onError = "" } var timeout time.Duration - timeoutString := annotations[podBackupHookTimeoutAnnotationKey] + timeoutString := getHookAnnotation(annotations, podBackupHookTimeoutAnnotationKey, phase) if timeoutString != "" { if temp, err := time.ParseDuration(timeoutString); err == nil { timeout = temp @@ -169,7 +206,8 @@ type resourceHook struct { namespaces *collections.IncludesExcludes resources *collections.IncludesExcludes labelSelector labels.Selector - hooks []api.BackupResourceHook + pre []api.BackupResourceHook + post []api.BackupResourceHook } func (r resourceHook) applicableTo(groupResource schema.GroupResource, namespace string, labels labels.Set) bool { diff --git a/pkg/backup/item_hook_handler_test.go b/pkg/backup/item_hook_handler_test.go index 54e28ac64..07ae35c5a 100644 --- a/pkg/backup/item_hook_handler_test.go +++ b/pkg/backup/item_hook_handler_test.go @@ -17,6 +17,7 @@ limitations under the License. package backup import ( + "fmt" "testing" "time" @@ -38,8 +39,8 @@ type mockItemHookHandler struct { mock.Mock } -func (h *mockItemHookHandler) handleHooks(log logrus.FieldLogger, groupResource schema.GroupResource, obj runtime.Unstructured, resourceHooks []resourceHook) error { - args := h.Called(log, groupResource, obj, resourceHooks) +func (h *mockItemHookHandler) handleHooks(log logrus.FieldLogger, groupResource schema.GroupResource, obj runtime.Unstructured, resourceHooks []resourceHook, phase hookPhase) error { + args := h.Called(log, groupResource, obj, resourceHooks, phase) return args.Error(0) } @@ -102,7 +103,7 @@ func TestHandleHooksSkips(t *testing.T) { }, { name: "missing exec hook", - hooks: []v1.BackupResourceHook{ + pre: []v1.BackupResourceHook{ {}, {}, }, @@ -121,15 +122,16 @@ func TestHandleHooksSkips(t *testing.T) { } groupResource := schema.ParseGroupResource(test.groupResource) - err := h.handleHooks(arktest.NewLogger(), groupResource, test.item, test.hooks) + err := h.handleHooks(arktest.NewLogger(), groupResource, test.item, test.hooks, hookPhasePre) assert.NoError(t, err) }) } } -func TestHandleHooksPodFromPodAnnotation(t *testing.T) { +func TestHandleHooks(t *testing.T) { tests := []struct { name string + phase hookPhase groupResource string item runtime.Unstructured hooks []resourceHook @@ -139,7 +141,8 @@ func TestHandleHooksPodFromPodAnnotation(t *testing.T) { expectedPodHookError error }{ { - name: "pod, no annotation, spec (multiple hooks) = run spec", + name: "pod, no annotation, spec (multiple pre hooks) = run spec", + phase: hookPhasePre, groupResource: "pods", item: unstructuredOrDie(` { @@ -153,24 +156,24 @@ func TestHandleHooksPodFromPodAnnotation(t *testing.T) { hooks: []resourceHook{ { name: "hook1", - hooks: []v1.BackupResourceHook{ + pre: []v1.BackupResourceHook{ { Exec: &v1.ExecHook{ Container: "1a", - Command: []string{"1a"}, + Command: []string{"pre-1a"}, }, }, { Exec: &v1.ExecHook{ Container: "1b", - Command: []string{"1b"}, + Command: []string{"pre-1b"}, }, }, }, }, { name: "hook2", - hooks: []v1.BackupResourceHook{ + pre: []v1.BackupResourceHook{ { Exec: &v1.ExecHook{ Container: "2a", @@ -188,7 +191,58 @@ func TestHandleHooksPodFromPodAnnotation(t *testing.T) { }, }, { - name: "pod, annotation, no spec = run annotation", + name: "pod, no annotation, spec (multiple post hooks) = run spec", + phase: hookPhasePost, + groupResource: "pods", + item: unstructuredOrDie(` + { + "apiVersion": "v1", + "kind": "Pod", + "metadata": { + "namespace": "ns", + "name": "name" + } + }`), + hooks: []resourceHook{ + { + name: "hook1", + post: []v1.BackupResourceHook{ + { + Exec: &v1.ExecHook{ + Container: "1a", + Command: []string{"pre-1a"}, + }, + }, + { + Exec: &v1.ExecHook{ + Container: "1b", + Command: []string{"pre-1b"}, + }, + }, + }, + }, + { + name: "hook2", + post: []v1.BackupResourceHook{ + { + Exec: &v1.ExecHook{ + Container: "2a", + Command: []string{"2a"}, + }, + }, + { + Exec: &v1.ExecHook{ + Container: "2b", + Command: []string{"2b"}, + }, + }, + }, + }, + }, + }, + { + name: "pod, annotation (legacy), no spec = run annotation", + phase: hookPhasePre, groupResource: "pods", item: unstructuredOrDie(` { @@ -208,8 +262,53 @@ func TestHandleHooksPodFromPodAnnotation(t *testing.T) { Command: []string{"/bin/ls"}, }, }, + { + name: "pod, annotation (pre), no spec = run annotation", + phase: hookPhasePre, + groupResource: "pods", + item: unstructuredOrDie(` + { + "apiVersion": "v1", + "kind": "Pod", + "metadata": { + "namespace": "ns", + "name": "name", + "annotations": { + "pre.hook.backup.ark.heptio.com/container": "c", + "pre.hook.backup.ark.heptio.com/command": "/bin/ls" + } + } + }`), + expectedPodHook: &v1.ExecHook{ + Container: "c", + Command: []string{"/bin/ls"}, + }, + }, + { + name: "pod, annotation (post), no spec = run annotation", + phase: hookPhasePost, + groupResource: "pods", + item: unstructuredOrDie(` + { + "apiVersion": "v1", + "kind": "Pod", + "metadata": { + "namespace": "ns", + "name": "name", + "annotations": { + "post.hook.backup.ark.heptio.com/container": "c", + "post.hook.backup.ark.heptio.com/command": "/bin/ls" + } + } + }`), + expectedPodHook: &v1.ExecHook{ + Container: "c", + Command: []string{"/bin/ls"}, + }, + }, { name: "pod, annotation & spec = run annotation", + phase: hookPhasePre, groupResource: "pods", item: unstructuredOrDie(` { @@ -231,7 +330,7 @@ func TestHandleHooksPodFromPodAnnotation(t *testing.T) { hooks: []resourceHook{ { name: "hook1", - hooks: []v1.BackupResourceHook{ + pre: []v1.BackupResourceHook{ { Exec: &v1.ExecHook{ Container: "1a", @@ -244,6 +343,7 @@ func TestHandleHooksPodFromPodAnnotation(t *testing.T) { }, { name: "pod, annotation, onError=fail = return error", + phase: hookPhasePre, groupResource: "pods", item: unstructuredOrDie(` { @@ -269,6 +369,7 @@ func TestHandleHooksPodFromPodAnnotation(t *testing.T) { }, { name: "pod, annotation, onError=continue = return nil", + phase: hookPhasePre, groupResource: "pods", item: unstructuredOrDie(` { @@ -294,6 +395,7 @@ func TestHandleHooksPodFromPodAnnotation(t *testing.T) { }, { name: "pod, spec, onError=fail = don't run other hooks", + phase: hookPhasePre, groupResource: "pods", item: unstructuredOrDie(` { @@ -307,7 +409,7 @@ func TestHandleHooksPodFromPodAnnotation(t *testing.T) { hooks: []resourceHook{ { name: "hook1", - hooks: []v1.BackupResourceHook{ + pre: []v1.BackupResourceHook{ { Exec: &v1.ExecHook{ Container: "1a", @@ -325,7 +427,7 @@ func TestHandleHooksPodFromPodAnnotation(t *testing.T) { }, { name: "hook2", - hooks: []v1.BackupResourceHook{ + pre: []v1.BackupResourceHook{ { Exec: &v1.ExecHook{ Container: "2", @@ -337,7 +439,7 @@ func TestHandleHooksPodFromPodAnnotation(t *testing.T) { }, { name: "hook3", - hooks: []v1.BackupResourceHook{ + pre: []v1.BackupResourceHook{ { Exec: &v1.ExecHook{ Container: "3", @@ -369,7 +471,14 @@ func TestHandleHooksPodFromPodAnnotation(t *testing.T) { } else { hookLoop: for _, resourceHook := range test.hooks { - for _, hook := range resourceHook.hooks { + for _, hook := range resourceHook.pre { + hookError := test.hookErrorsByContainer[hook.Exec.Container] + podCommandExecutor.On("executePodCommand", mock.Anything, test.item.UnstructuredContent(), "ns", "name", resourceHook.name, hook.Exec).Return(hookError) + if hookError != nil && hook.Exec.OnError == v1.HookErrorModeFail { + break hookLoop + } + } + for _, hook := range resourceHook.post { hookError := test.hookErrorsByContainer[hook.Exec.Container] podCommandExecutor.On("executePodCommand", mock.Anything, test.item.UnstructuredContent(), "ns", "name", resourceHook.name, hook.Exec).Return(hookError) if hookError != nil && hook.Exec.OnError == v1.HookErrorModeFail { @@ -380,7 +489,7 @@ func TestHandleHooksPodFromPodAnnotation(t *testing.T) { } groupResource := schema.ParseGroupResource(test.groupResource) - err := h.handleHooks(arktest.NewLogger(), groupResource, test.item, test.hooks) + err := h.handleHooks(arktest.NewLogger(), groupResource, test.item, test.hooks, test.phase) if test.expectedError != nil { assert.EqualError(t, err, test.expectedError.Error()) @@ -393,103 +502,106 @@ func TestHandleHooksPodFromPodAnnotation(t *testing.T) { } func TestGetPodExecHookFromAnnotations(t *testing.T) { - tests := []struct { - name string - annotations map[string]string - expectedHook *v1.ExecHook - }{ - { - name: "missing command annotation", - expectedHook: nil, - }, - { - name: "malformed command json array", - annotations: map[string]string{ - podBackupHookCommandAnnotationKey: "[blarg", + phases := []hookPhase{"", hookPhasePre, hookPhasePost} + for _, phase := range phases { + tests := []struct { + name string + annotations map[string]string + expectedHook *v1.ExecHook + }{ + { + name: "missing command annotation", + expectedHook: nil, }, - expectedHook: &v1.ExecHook{ - Command: []string{"[blarg"}, + { + name: "malformed command json array", + annotations: map[string]string{ + phasedKey(phase, podBackupHookCommandAnnotationKey): "[blarg", + }, + expectedHook: &v1.ExecHook{ + Command: []string{"[blarg"}, + }, }, - }, - { - name: "valid command json array", - annotations: map[string]string{ - podBackupHookCommandAnnotationKey: `["a","b","c"]`, + { + name: "valid command json array", + annotations: map[string]string{ + phasedKey(phase, podBackupHookCommandAnnotationKey): `["a","b","c"]`, + }, + expectedHook: &v1.ExecHook{ + Command: []string{"a", "b", "c"}, + }, }, - expectedHook: &v1.ExecHook{ - Command: []string{"a", "b", "c"}, + { + name: "command as a string", + annotations: map[string]string{ + phasedKey(phase, podBackupHookCommandAnnotationKey): "/usr/bin/foo", + }, + expectedHook: &v1.ExecHook{ + Command: []string{"/usr/bin/foo"}, + }, }, - }, - { - name: "command as a string", - annotations: map[string]string{ - podBackupHookCommandAnnotationKey: "/usr/bin/foo", + { + name: "hook mode set to continue", + annotations: map[string]string{ + phasedKey(phase, podBackupHookCommandAnnotationKey): "/usr/bin/foo", + phasedKey(phase, podBackupHookOnErrorAnnotationKey): string(v1.HookErrorModeContinue), + }, + expectedHook: &v1.ExecHook{ + Command: []string{"/usr/bin/foo"}, + OnError: v1.HookErrorModeContinue, + }, }, - expectedHook: &v1.ExecHook{ - Command: []string{"/usr/bin/foo"}, + { + name: "hook mode set to fail", + annotations: map[string]string{ + phasedKey(phase, podBackupHookCommandAnnotationKey): "/usr/bin/foo", + phasedKey(phase, podBackupHookOnErrorAnnotationKey): string(v1.HookErrorModeFail), + }, + expectedHook: &v1.ExecHook{ + Command: []string{"/usr/bin/foo"}, + OnError: v1.HookErrorModeFail, + }, }, - }, - { - name: "hook mode set to continue", - annotations: map[string]string{ - podBackupHookCommandAnnotationKey: "/usr/bin/foo", - podBackupHookOnErrorAnnotationKey: string(v1.HookErrorModeContinue), + { + name: "use the specified timeout", + annotations: map[string]string{ + phasedKey(phase, podBackupHookCommandAnnotationKey): "/usr/bin/foo", + phasedKey(phase, podBackupHookTimeoutAnnotationKey): "5m3s", + }, + expectedHook: &v1.ExecHook{ + Command: []string{"/usr/bin/foo"}, + Timeout: metav1.Duration{Duration: 5*time.Minute + 3*time.Second}, + }, }, - expectedHook: &v1.ExecHook{ - Command: []string{"/usr/bin/foo"}, - OnError: v1.HookErrorModeContinue, + { + name: "invalid timeout is ignored", + annotations: map[string]string{ + phasedKey(phase, podBackupHookCommandAnnotationKey): "/usr/bin/foo", + phasedKey(phase, podBackupHookTimeoutAnnotationKey): "invalid", + }, + expectedHook: &v1.ExecHook{ + Command: []string{"/usr/bin/foo"}, + }, }, - }, - { - name: "hook mode set to fail", - annotations: map[string]string{ - podBackupHookCommandAnnotationKey: "/usr/bin/foo", - podBackupHookOnErrorAnnotationKey: string(v1.HookErrorModeFail), + { + name: "use the specified container", + annotations: map[string]string{ + phasedKey(phase, podBackupHookContainerAnnotationKey): "some-container", + phasedKey(phase, podBackupHookCommandAnnotationKey): "/usr/bin/foo", + }, + expectedHook: &v1.ExecHook{ + Container: "some-container", + Command: []string{"/usr/bin/foo"}, + }, }, - expectedHook: &v1.ExecHook{ - Command: []string{"/usr/bin/foo"}, - OnError: v1.HookErrorModeFail, - }, - }, - { - name: "use the specified timeout", - annotations: map[string]string{ - podBackupHookCommandAnnotationKey: "/usr/bin/foo", - podBackupHookTimeoutAnnotationKey: "5m3s", - }, - expectedHook: &v1.ExecHook{ - Command: []string{"/usr/bin/foo"}, - Timeout: metav1.Duration{Duration: 5*time.Minute + 3*time.Second}, - }, - }, - { - name: "invalid timeout is ignored", - annotations: map[string]string{ - podBackupHookCommandAnnotationKey: "/usr/bin/foo", - podBackupHookTimeoutAnnotationKey: "invalid", - }, - expectedHook: &v1.ExecHook{ - Command: []string{"/usr/bin/foo"}, - }, - }, - { - name: "use the specified container", - annotations: map[string]string{ - podBackupHookContainerAnnotationKey: "some-container", - podBackupHookCommandAnnotationKey: "/usr/bin/foo", - }, - expectedHook: &v1.ExecHook{ - Container: "some-container", - Command: []string{"/usr/bin/foo"}, - }, - }, - } + } - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - hook := getPodExecHookFromAnnotations(test.annotations) - assert.Equal(t, test.expectedHook, hook) - }) + for _, test := range tests { + t.Run(fmt.Sprintf("%s (phase=%q)", test.name, phase), func(t *testing.T) { + hook := getPodExecHookFromAnnotations(test.annotations, phase) + assert.Equal(t, test.expectedHook, hook) + }) + } } } diff --git a/pkg/backup/pod_action.go b/pkg/backup/pod_action.go new file mode 100644 index 000000000..dd171403a --- /dev/null +++ b/pkg/backup/pod_action.go @@ -0,0 +1,102 @@ +/* +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 backup + +import ( + "github.com/pkg/errors" + "github.com/sirupsen/logrus" + + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + + "github.com/heptio/ark/pkg/apis/ark/v1" + "github.com/heptio/ark/pkg/util/collections" +) + +// podAction implements ItemAction. +type podAction struct { + log logrus.FieldLogger +} + +// NewPodAction creates a new ItemAction for pods. +func NewPodAction(log logrus.FieldLogger) ItemAction { + return &podAction{log: log} +} + +var pvcGroupResource = schema.GroupResource{Group: "", Resource: "persistentvolumeclaims"} + +// AppliesTo returns a ResourceSelector that applies only to pods. +func (a *podAction) AppliesTo() (ResourceSelector, error) { + return ResourceSelector{ + IncludedResources: []string{"pods"}, + }, nil +} + +// Execute scans the pod's spec.volumes for persistentVolumeClaim volumes and returns a +// ResourceIdentifier list containing references to all of the persistentVolumeClaim volumes used by +// the pod. This ensures that when a pod is backed up, all referenced PVCs are backed up too. +func (a *podAction) Execute(item runtime.Unstructured, backup *v1.Backup) (runtime.Unstructured, []ResourceIdentifier, error) { + a.log.Info("Executing podAction") + defer a.log.Info("Done executing podAction") + + pod := item.UnstructuredContent() + if !collections.Exists(pod, "spec.volumes") { + a.log.Info("pod has no volumes") + return item, nil, nil + } + + metadata, err := meta.Accessor(item) + if err != nil { + return nil, nil, errors.Wrap(err, "unable to access pod metadata") + } + + volumes, err := collections.GetSlice(pod, "spec.volumes") + if err != nil { + return nil, nil, errors.WithMessage(err, "error getting spec.volumes") + } + + var errs []error + var additionalItems []ResourceIdentifier + + for i := range volumes { + volume, ok := volumes[i].(map[string]interface{}) + if !ok { + errs = append(errs, errors.Errorf("unexpected type %T", volumes[i])) + continue + } + if !collections.Exists(volume, "persistentVolumeClaim.claimName") { + continue + } + + claimName, err := collections.GetString(volume, "persistentVolumeClaim.claimName") + if err != nil { + errs = append(errs, err) + continue + } + + a.log.Infof("Adding pvc %s to additionalItems", claimName) + + additionalItems = append(additionalItems, ResourceIdentifier{ + GroupResource: pvcGroupResource, + Namespace: metadata.GetNamespace(), + Name: claimName, + }) + } + + return item, additionalItems, nil +} diff --git a/pkg/backup/pod_action_test.go b/pkg/backup/pod_action_test.go new file mode 100644 index 000000000..4d939af23 --- /dev/null +++ b/pkg/backup/pod_action_test.go @@ -0,0 +1,127 @@ +/* +Copyright 2018 the Heptio Ark 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" + + arktest "github.com/heptio/ark/pkg/util/test" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/runtime" +) + +func TestPodActionAppliesTo(t *testing.T) { + a := NewPodAction(arktest.NewLogger()) + + actual, err := a.AppliesTo() + require.NoError(t, err) + + expected := ResourceSelector{ + IncludedResources: []string{"pods"}, + } + assert.Equal(t, expected, actual) +} + +func TestPodActionExecute(t *testing.T) { + tests := []struct { + name string + pod runtime.Unstructured + expected []ResourceIdentifier + }{ + { + name: "no spec.volumes", + pod: unstructuredOrDie(` + { + "apiVersion": "v1", + "kind": "Pod", + "metadata": { + "namespace": "foo", + "name": "bar" + } + } + `), + }, + { + name: "persistentVolumeClaim without claimName", + pod: unstructuredOrDie(` + { + "apiVersion": "v1", + "kind": "Pod", + "metadata": { + "namespace": "foo", + "name": "bar" + }, + "spec": { + "volumes": [ + { + "persistentVolumeClaim": {} + } + ] + } + } + `), + }, + { + name: "full test, mix of volume types", + pod: unstructuredOrDie(` + { + "apiVersion": "v1", + "kind": "Pod", + "metadata": { + "namespace": "foo", + "name": "bar" + }, + "spec": { + "volumes": [ + { + "persistentVolumeClaim": {} + }, + { + "emptyDir": {} + }, + { + "persistentVolumeClaim": {"claimName": "claim1"} + }, + { + "emptyDir": {} + }, + { + "persistentVolumeClaim": {"claimName": "claim2"} + } + ] + } + } + `), + expected: []ResourceIdentifier{ + {GroupResource: pvcGroupResource, Namespace: "foo", Name: "claim1"}, + {GroupResource: pvcGroupResource, Namespace: "foo", Name: "claim2"}, + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + a := NewPodAction(arktest.NewLogger()) + + updated, additionalItems, err := a.Execute(test.pod, nil) + require.NoError(t, err) + assert.Equal(t, test.pod, updated) + assert.Equal(t, test.expected, additionalItems) + }) + } +} diff --git a/pkg/backup/resource_backupper_test.go b/pkg/backup/resource_backupper_test.go index 501d650eb..3ead729a1 100644 --- a/pkg/backup/resource_backupper_test.go +++ b/pkg/backup/resource_backupper_test.go @@ -556,7 +556,8 @@ func TestBackupResourceOnlyIncludesSpecifiedNamespaces(t *testing.T) { ns1 := unstructuredOrDie(`{"apiVersion":"v1","kind":"Namespace","metadata":{"name":"ns-1"}}`) client.On("Get", "ns-1", metav1.GetOptions{}).Return(ns1, nil) - itemHookHandler.On("handleHooks", mock.Anything, schema.GroupResource{Group: "", Resource: "namespaces"}, ns1, resourceHooks).Return(nil) + itemHookHandler.On("handleHooks", mock.Anything, schema.GroupResource{Group: "", Resource: "namespaces"}, ns1, resourceHooks, hookPhasePre).Return(nil) + itemHookHandler.On("handleHooks", mock.Anything, schema.GroupResource{Group: "", Resource: "namespaces"}, ns1, resourceHooks, hookPhasePost).Return(nil) err := rb.backupResource(v1Group, namespacesResource) require.NoError(t, err) diff --git a/pkg/cmd/server/plugin/plugin.go b/pkg/cmd/server/plugin/plugin.go index 4af679d7b..84ba99fe1 100644 --- a/pkg/cmd/server/plugin/plugin.go +++ b/pkg/cmd/server/plugin/plugin.go @@ -46,7 +46,8 @@ func NewCommand() *cobra.Command { } backupItemActions := map[string]backup.ItemAction{ - "pv": backup.NewBackupPVAction(logger), + "pv": backup.NewBackupPVAction(logger), + "pod": backup.NewPodAction(logger), } restoreItemActions := map[string]restore.ItemAction{ diff --git a/pkg/plugin/manager.go b/pkg/plugin/manager.go index 438ef5b6b..80bc1594d 100644 --- a/pkg/plugin/manager.go +++ b/pkg/plugin/manager.go @@ -181,9 +181,10 @@ func (m *manager) registerPlugins() error { m.pluginRegistry.register(provider, "/ark", []string{"run-plugin", "cloudprovider", provider}, PluginKindObjectStore, PluginKindBlockStore) } m.pluginRegistry.register("pv", "/ark", []string{"run-plugin", string(PluginKindBackupItemAction), "pv"}, PluginKindBackupItemAction) + m.pluginRegistry.register("backup-pod", "/ark", []string{"run-plugin", string(PluginKindBackupItemAction), "pod"}, PluginKindBackupItemAction) m.pluginRegistry.register("job", "/ark", []string{"run-plugin", string(PluginKindRestoreItemAction), "job"}, PluginKindRestoreItemAction) - m.pluginRegistry.register("pod", "/ark", []string{"run-plugin", string(PluginKindRestoreItemAction), "pod"}, PluginKindRestoreItemAction) + m.pluginRegistry.register("restore-pod", "/ark", []string{"run-plugin", string(PluginKindRestoreItemAction), "pod"}, PluginKindRestoreItemAction) m.pluginRegistry.register("svc", "/ark", []string{"run-plugin", string(PluginKindRestoreItemAction), "svc"}, PluginKindRestoreItemAction) // second, register external plugins (these will override internal plugins, if applicable)