From 9e7ff4e3d9f2a3ab17f645f28b05931527fe94e6 Mon Sep 17 00:00:00 2001 From: Steve Kriss Date: Sun, 9 Jun 2019 22:02:49 -0600 Subject: [PATCH] Refactoring for backup item action tests (#1545) * migrate and enhance backup item action tests Signed-off-by: Steve Kriss * migrate terminating resource test Signed-off-by: Steve Kriss * add godoc for test functions Signed-off-by: Steve Kriss --- pkg/backup/backup_new_test.go | 674 ++++++++++++++++++++++++++++++ pkg/backup/backup_test.go | 120 +----- pkg/backup/item_backupper_test.go | 168 +------- 3 files changed, 695 insertions(+), 267 deletions(-) diff --git a/pkg/backup/backup_new_test.go b/pkg/backup/backup_new_test.go index 984ea2404..656be30a9 100644 --- a/pkg/backup/backup_new_test.go +++ b/pkg/backup/backup_new_test.go @@ -20,11 +20,15 @@ import ( "archive/tar" "bytes" "compress/gzip" + "encoding/json" "io" + "io/ioutil" "sort" "strings" "testing" + "time" + "github.com/pkg/errors" "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -42,9 +46,17 @@ import ( "github.com/heptio/velero/pkg/client" "github.com/heptio/velero/pkg/discovery" "github.com/heptio/velero/pkg/generated/clientset/versioned/fake" + "github.com/heptio/velero/pkg/kuberesource" + "github.com/heptio/velero/pkg/plugin/velero" "github.com/heptio/velero/pkg/test" ) +// TestBackupResourceFiltering runs backups with different combinations +// of resource filters (included/excluded resources, included/excluded +// namespaces, label selectors, "include cluster resources" flag), and +// verifies that the set of items written to the backup tarball are +// correct. Validation is done by looking at the names of the files in +// the backup tarball; the contents of the files are not checked. func TestBackupResourceFiltering(t *testing.T) { tests := []struct { name string @@ -423,6 +435,19 @@ func TestBackupResourceFiltering(t *testing.T) { "resources/pods/namespaces/zoo/raz.json", }, }, + { + name: "terminating resources are not backed up", + backup: defaultBackup().Backup(), + apiResources: []*apiResource{ + pods( + newPod("ns-1", "pod-1"), + &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Namespace: "ns-2", Name: "pod-2", DeletionTimestamp: &metav1.Time{Time: time.Now()}}}, + ), + }, + want: []string{ + "resources/pods/namespaces/ns-1/pod-1.json", + }, + }, } for _, tc := range tests { @@ -444,6 +469,10 @@ func TestBackupResourceFiltering(t *testing.T) { } } +// TestBackupResourceCohabitation runs backups for resources that "cohabitate", +// meaning they exist in multiple API groups (e.g. deployments.extensions and +// deployments.apps), and verifies that only one copy of each resource is backed +// up, with preference for the non-"extensions" API group. func TestBackupResourceCohabitation(t *testing.T) { tests := []struct { name string @@ -504,6 +533,10 @@ func TestBackupResourceCohabitation(t *testing.T) { } } +// TestBackupUsesNewCohabitatingResourcesForEachBackup ensures that when two backups are +// run that each include cohabitating resources, one copy of the relevant resources is +// backed up in each backup. Verification is done by looking at the contents of the backup +// tarball. This covers a specific issue that was fixed by https://github.com/heptio/velero/pull/485. func TestBackupUsesNewCohabitatingResourcesForEachBackup(t *testing.T) { h := newHarness(t) @@ -531,6 +564,9 @@ func TestBackupUsesNewCohabitatingResourcesForEachBackup(t *testing.T) { assertTarballContents(t, backup2File, "metadata/version", "resources/deployments.apps/namespaces/ns-1/deploy-1.json") } +// TestBackupResourceOrdering runs backups of the core API group and ensures that items are backed +// up in the expected order (pods, PVCs, PVs, everything else). Verification is done by looking +// at the order of files written to the backup tarball. func TestBackupResourceOrdering(t *testing.T) { tests := []struct { name string @@ -582,6 +618,585 @@ func TestBackupResourceOrdering(t *testing.T) { } } +// TestBackupActionsRunsForCorrectItems runs backups with backup item actions, and +// verifies that each backup item action is run for the correct set of resources based on its +// AppliesTo() resource selector. Verification is done by using the recordResourcesAction struct, +// which records which resources it's executed for. +func TestBackupActionsRunForCorrectItems(t *testing.T) { + tests := []struct { + name string + backup *velerov1.Backup + apiResources []*apiResource + + // actions is a map from a recordResourcesAction (which will record the items it was called for) + // to a slice of expected items, formatted as {namespace}/{name}. + actions map[*recordResourcesAction][]string + }{ + { + name: "single action with no selector runs for all items", + backup: defaultBackup(). + Backup(), + apiResources: []*apiResource{ + pods( + newPod("ns-1", "pod-1"), + newPod("ns-2", "pod-2"), + ), + pvs( + newPV("pv-1"), + newPV("pv-2"), + ), + }, + actions: map[*recordResourcesAction][]string{ + new(recordResourcesAction): {"ns-1/pod-1", "ns-2/pod-2", "pv-1", "pv-2"}, + }, + }, + { + name: "single action with a resource selector for namespaced resources runs only for matching resources", + backup: defaultBackup(). + Backup(), + apiResources: []*apiResource{ + pods( + newPod("ns-1", "pod-1"), + newPod("ns-2", "pod-2"), + ), + pvs( + newPV("pv-1"), + newPV("pv-2"), + ), + }, + actions: map[*recordResourcesAction][]string{ + new(recordResourcesAction).ForResource("pods"): {"ns-1/pod-1", "ns-2/pod-2"}, + }, + }, + { + name: "single action with a resource selector for cluster-scoped resources runs only for matching resources", + backup: defaultBackup(). + Backup(), + apiResources: []*apiResource{ + pods( + newPod("ns-1", "pod-1"), + newPod("ns-2", "pod-2"), + ), + pvs( + newPV("pv-1"), + newPV("pv-2"), + ), + }, + actions: map[*recordResourcesAction][]string{ + new(recordResourcesAction).ForResource("persistentvolumes"): {"pv-1", "pv-2"}, + }, + }, + { + // TODO this seems like a bug + name: "single action with a namespace selector runs for resources in that namespace plus cluster-scoped resources", + backup: defaultBackup(). + Backup(), + apiResources: []*apiResource{ + pods( + newPod("ns-1", "pod-1"), + newPod("ns-2", "pod-2"), + ), + pvcs( + newPVC("ns-1", "pvc-1"), + newPVC("ns-2", "pvc-2"), + ), + pvs( + newPV("pv-1"), + newPV("pv-2"), + ), + }, + actions: map[*recordResourcesAction][]string{ + new(recordResourcesAction).ForNamespace("ns-1"): {"ns-1/pod-1", "ns-1/pvc-1", "pv-1", "pv-2"}, + }, + }, + { + name: "single action with a resource and namespace selector runs only for matching resources", + backup: defaultBackup(). + Backup(), + apiResources: []*apiResource{ + pods( + newPod("ns-1", "pod-1"), + newPod("ns-2", "pod-2"), + ), + pvs( + newPV("pv-1"), + newPV("pv-2"), + ), + }, + actions: map[*recordResourcesAction][]string{ + new(recordResourcesAction).ForResource("pods").ForNamespace("ns-1"): {"ns-1/pod-1"}, + }, + }, + { + name: "multiple actions, each with a different resource selector using short name, run for matching resources", + backup: defaultBackup(). + Backup(), + apiResources: []*apiResource{ + pods( + newPod("ns-1", "pod-1"), + newPod("ns-2", "pod-2"), + ), + pvs( + newPV("pv-1"), + newPV("pv-2"), + ), + }, + actions: map[*recordResourcesAction][]string{ + new(recordResourcesAction).ForResource("po"): {"ns-1/pod-1", "ns-2/pod-2"}, + new(recordResourcesAction).ForResource("pv"): {"pv-1", "pv-2"}, + }, + }, + { + name: "actions with selectors that don't match anything don't run for any resources", + backup: defaultBackup(). + Backup(), + apiResources: []*apiResource{ + pods( + newPod("ns-1", "pod-1"), + ), + pvcs( + newPVC("ns-2", "pvc-2"), + ), + pvs( + newPV("pv-1"), + newPV("pv-2"), + ), + }, + actions: map[*recordResourcesAction][]string{ + new(recordResourcesAction).ForNamespace("ns-1").ForResource("persistentvolumeclaims"): nil, + new(recordResourcesAction).ForNamespace("ns-2").ForResource("pods"): nil, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var ( + h = newHarness(t) + req = &Request{Backup: tc.backup} + backupFile = bytes.NewBuffer([]byte{}) + ) + + for _, resource := range tc.apiResources { + h.addItems(t, resource.group, resource.version, resource.name, resource.shortName, resource.namespaced, resource.items...) + } + + actions := []velero.BackupItemAction{} + for action := range tc.actions { + actions = append(actions, action) + } + + err := h.backupper.Backup(h.log, req, backupFile, actions, nil) + assert.NoError(t, err) + + for action, want := range tc.actions { + assert.Equal(t, want, action.ids) + } + }) + } +} + +// TestBackupWithInvalidActions runs backups with backup item actions that are invalid +// in some way (e.g. an invalid label selector returned from AppliesTo(), an error returned +// from AppliesTo()) and verifies that this causes the backupper.Backup(...) method to +// return an error. +func TestBackupWithInvalidActions(t *testing.T) { + // all test cases in this function are expected to cause the method under test + // to return an error, so no expected results need to be set up. + tests := []struct { + name string + backup *velerov1.Backup + apiResources []*apiResource + actions []velero.BackupItemAction + }{ + { + name: "action with invalid label selector results in an error", + backup: defaultBackup(). + Backup(), + apiResources: []*apiResource{ + pods( + newPod("foo", "bar"), + newPod("zoo", "raz"), + ), + pvs( + newPV("bar"), + newPV("baz"), + ), + }, + actions: []velero.BackupItemAction{ + new(recordResourcesAction).ForLabelSelector("=invalid-selector"), + }, + }, + { + name: "action returning an error from AppliesTo results in an error", + backup: defaultBackup(). + Backup(), + apiResources: []*apiResource{ + pods( + newPod("foo", "bar"), + newPod("zoo", "raz"), + ), + pvs( + newPV("bar"), + newPV("baz"), + ), + }, + actions: []velero.BackupItemAction{ + &appliesToErrorAction{}, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var ( + h = newHarness(t) + req = &Request{Backup: tc.backup} + backupFile = bytes.NewBuffer([]byte{}) + ) + + for _, resource := range tc.apiResources { + h.addItems(t, resource.group, resource.version, resource.name, resource.shortName, resource.namespaced, resource.items...) + } + + assert.Error(t, h.backupper.Backup(h.log, req, backupFile, tc.actions, nil)) + }) + } +} + +// appliesToErrorAction is a backup item action that always returns +// an error when AppliesTo() is called. +type appliesToErrorAction struct{} + +func (a *appliesToErrorAction) AppliesTo() (velero.ResourceSelector, error) { + return velero.ResourceSelector{}, errors.New("error calling AppliesTo") +} + +func (a *appliesToErrorAction) Execute(item runtime.Unstructured, backup *velerov1.Backup) (runtime.Unstructured, []velero.ResourceIdentifier, error) { + panic("not implemented") +} + +// TestBackupActionModifications runs backups with backup item actions that make modifications +// to items in their Execute(...) methods and verifies that these modifications are +// persisted to the backup tarball. Verification is done by inspecting the file contents +// of the tarball. +func TestBackupActionModifications(t *testing.T) { + // modifyingActionGetter is a helper function that returns a *pluggableAction, whose Execute(...) + // method modifies the item being passed in by calling the 'modify' function on it. + modifyingActionGetter := func(modify func(*unstructured.Unstructured)) *pluggableAction { + return &pluggableAction{ + executeFunc: func(item runtime.Unstructured, backup *velerov1.Backup) (runtime.Unstructured, []velero.ResourceIdentifier, error) { + obj, ok := item.(*unstructured.Unstructured) + if !ok { + return nil, nil, errors.Errorf("unexpected type %T", item) + } + + res := obj.DeepCopy() + modify(res) + + return res, nil, nil + }, + } + } + + tests := []struct { + name string + backup *velerov1.Backup + apiResources []*apiResource + actions []velero.BackupItemAction + want map[string]unstructuredObject + }{ + { + name: "action that adds a label to item gets persisted", + backup: defaultBackup().Backup(), + apiResources: []*apiResource{ + pods( + newPod("ns-1", "pod-1"), + ), + }, + actions: []velero.BackupItemAction{ + modifyingActionGetter(func(item *unstructured.Unstructured) { + item.SetLabels(map[string]string{"updated": "true"}) + }), + }, + want: map[string]unstructuredObject{ + "resources/pods/namespaces/ns-1/pod-1.json": toUnstructuredOrFail(t, withLabel(newPod("ns-1", "pod-1"), "updated", "true")), + }, + }, + { + name: "action that removes labels from item gets persisted", + backup: defaultBackup().Backup(), + apiResources: []*apiResource{ + pods( + withLabel(newPod("ns-1", "pod-1"), "should-be-removed", "true"), + ), + }, + actions: []velero.BackupItemAction{ + modifyingActionGetter(func(item *unstructured.Unstructured) { + item.SetLabels(nil) + }), + }, + want: map[string]unstructuredObject{ + "resources/pods/namespaces/ns-1/pod-1.json": toUnstructuredOrFail(t, newPod("ns-1", "pod-1")), + }, + }, + { + name: "action that sets a spec field on item gets persisted", + backup: defaultBackup().Backup(), + apiResources: []*apiResource{ + pods( + newPod("ns-1", "pod-1"), + ), + }, + actions: []velero.BackupItemAction{ + modifyingActionGetter(func(item *unstructured.Unstructured) { + item.Object["spec"].(map[string]interface{})["nodeName"] = "foo" + }), + }, + want: map[string]unstructuredObject{ + "resources/pods/namespaces/ns-1/pod-1.json": toUnstructuredOrFail(t, &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Namespace: "ns-1", Name: "pod-1"}, Spec: corev1.PodSpec{NodeName: "foo"}}), + }, + }, + { + // TODO this seems like a bug + name: "modifications to name and namespace in an action are persisted in JSON but not in filename", + backup: defaultBackup(). + Backup(), + apiResources: []*apiResource{ + pods( + newPod("ns-1", "pod-1"), + ), + }, + actions: []velero.BackupItemAction{ + modifyingActionGetter(func(item *unstructured.Unstructured) { + item.SetName(item.GetName() + "-updated") + item.SetNamespace(item.GetNamespace() + "-updated") + }), + }, + want: map[string]unstructuredObject{ + "resources/pods/namespaces/ns-1/pod-1.json": toUnstructuredOrFail(t, newPod("ns-1-updated", "pod-1-updated")), + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var ( + h = newHarness(t) + req = &Request{Backup: tc.backup} + backupFile = bytes.NewBuffer([]byte{}) + ) + + for _, resource := range tc.apiResources { + h.addItems(t, resource.group, resource.version, resource.name, resource.shortName, resource.namespaced, resource.items...) + } + + err := h.backupper.Backup(h.log, req, backupFile, tc.actions, nil) + assert.NoError(t, err) + + assertTarballFileContents(t, backupFile, tc.want) + }) + } + +} + +// TestBackupActionAdditionalItems runs backups with backup item actions that return +// additional items to be backed up, and verifies that those items are included in the +// backup tarball as appropriate. Verification is done by looking at the files that exist +// in the backup tarball. +func TestBackupActionAdditionalItems(t *testing.T) { + tests := []struct { + name string + backup *velerov1.Backup + apiResources []*apiResource + actions []velero.BackupItemAction + want []string + }{ + { + name: "additional items that are already being backed up are not backed up twice", + backup: defaultBackup().Backup(), + apiResources: []*apiResource{ + pods( + newPod("ns-1", "pod-1"), + newPod("ns-2", "pod-2"), + newPod("ns-3", "pod-3"), + ), + }, + actions: []velero.BackupItemAction{ + &pluggableAction{ + selector: velero.ResourceSelector{IncludedNamespaces: []string{"ns-1"}}, + executeFunc: func(item runtime.Unstructured, backup *velerov1.Backup) (runtime.Unstructured, []velero.ResourceIdentifier, error) { + additionalItems := []velero.ResourceIdentifier{ + {GroupResource: kuberesource.Pods, Namespace: "ns-2", Name: "pod-2"}, + {GroupResource: kuberesource.Pods, Namespace: "ns-3", Name: "pod-3"}, + } + + return item, additionalItems, nil + }, + }, + }, + want: []string{ + "resources/pods/namespaces/ns-1/pod-1.json", + "resources/pods/namespaces/ns-2/pod-2.json", + "resources/pods/namespaces/ns-3/pod-3.json", + }, + }, + { + name: "when using a backup namespace filter, additional items that are in a non-included namespace are not backed up", + backup: defaultBackup().IncludedNamespaces("ns-1").Backup(), + apiResources: []*apiResource{ + pods( + newPod("ns-1", "pod-1"), + newPod("ns-2", "pod-2"), + newPod("ns-3", "pod-3"), + ), + }, + actions: []velero.BackupItemAction{ + &pluggableAction{ + executeFunc: func(item runtime.Unstructured, backup *velerov1.Backup) (runtime.Unstructured, []velero.ResourceIdentifier, error) { + additionalItems := []velero.ResourceIdentifier{ + {GroupResource: kuberesource.Pods, Namespace: "ns-2", Name: "pod-2"}, + {GroupResource: kuberesource.Pods, Namespace: "ns-3", Name: "pod-3"}, + } + + return item, additionalItems, nil + }, + }, + }, + want: []string{ + "resources/pods/namespaces/ns-1/pod-1.json", + }, + }, + { + name: "when using a backup namespace filter, additional items that are cluster-scoped are backed up", + backup: defaultBackup().IncludedNamespaces("ns-1").Backup(), + apiResources: []*apiResource{ + pods( + newPod("ns-1", "pod-1"), + newPod("ns-2", "pod-2"), + ), + pvs( + newPV("pv-1"), + newPV("pv-2"), + ), + }, + actions: []velero.BackupItemAction{ + &pluggableAction{ + executeFunc: func(item runtime.Unstructured, backup *velerov1.Backup) (runtime.Unstructured, []velero.ResourceIdentifier, error) { + additionalItems := []velero.ResourceIdentifier{ + {GroupResource: kuberesource.PersistentVolumes, Name: "pv-1"}, + {GroupResource: kuberesource.PersistentVolumes, Name: "pv-2"}, + } + + return item, additionalItems, nil + }, + }, + }, + want: []string{ + "resources/pods/namespaces/ns-1/pod-1.json", + "resources/persistentvolumes/cluster/pv-1.json", + "resources/persistentvolumes/cluster/pv-2.json", + }, + }, + { + name: "when using a backup resource filter, additional items that are non-included resources are not backed up", + backup: defaultBackup().IncludedResources("pods").Backup(), + apiResources: []*apiResource{ + pods( + newPod("ns-1", "pod-1"), + ), + pvs( + newPV("pv-1"), + newPV("pv-2"), + ), + }, + actions: []velero.BackupItemAction{ + &pluggableAction{ + executeFunc: func(item runtime.Unstructured, backup *velerov1.Backup) (runtime.Unstructured, []velero.ResourceIdentifier, error) { + additionalItems := []velero.ResourceIdentifier{ + {GroupResource: kuberesource.PersistentVolumes, Name: "pv-1"}, + {GroupResource: kuberesource.PersistentVolumes, Name: "pv-2"}, + } + + return item, additionalItems, nil + }, + }, + }, + want: []string{ + "resources/pods/namespaces/ns-1/pod-1.json", + }, + }, + { + name: "when IncludeClusterResources=false, additional items that are cluster-scoped are not backed up", + backup: defaultBackup().IncludeClusterResources(false).Backup(), + apiResources: []*apiResource{ + pods( + newPod("ns-1", "pod-1"), + newPod("ns-2", "pod-2"), + ), + pvs( + newPV("pv-1"), + newPV("pv-2"), + ), + }, + actions: []velero.BackupItemAction{ + &pluggableAction{ + executeFunc: func(item runtime.Unstructured, backup *velerov1.Backup) (runtime.Unstructured, []velero.ResourceIdentifier, error) { + additionalItems := []velero.ResourceIdentifier{ + {GroupResource: kuberesource.PersistentVolumes, Name: "pv-1"}, + {GroupResource: kuberesource.PersistentVolumes, Name: "pv-2"}, + } + + return item, additionalItems, nil + }, + }, + }, + want: []string{ + "resources/pods/namespaces/ns-1/pod-1.json", + "resources/pods/namespaces/ns-2/pod-2.json", + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var ( + h = newHarness(t) + req = &Request{Backup: tc.backup} + backupFile = bytes.NewBuffer([]byte{}) + ) + + for _, resource := range tc.apiResources { + h.addItems(t, resource.group, resource.version, resource.name, resource.shortName, resource.namespaced, resource.items...) + } + + err := h.backupper.Backup(h.log, req, backupFile, tc.actions, nil) + assert.NoError(t, err) + + assertTarballContents(t, backupFile, append(tc.want, "metadata/version")...) + }) + } +} + +// pluggableAction is a backup item action that can be plugged with an Execute +// function body at runtime. +type pluggableAction struct { + selector velero.ResourceSelector + executeFunc func(runtime.Unstructured, *velerov1.Backup) (runtime.Unstructured, []velero.ResourceIdentifier, error) +} + +func (a *pluggableAction) Execute(item runtime.Unstructured, backup *velerov1.Backup) (runtime.Unstructured, []velero.ResourceIdentifier, error) { + if a.executeFunc == nil { + return item, nil, nil + } + + return a.executeFunc(item, backup) +} + +func (a *pluggableAction) AppliesTo() (velero.ResourceSelector, error) { + return a.selector, nil +} + type apiResource struct { group string version string @@ -787,6 +1402,17 @@ func defaultBackup() *Builder { return NewNamedBuilder(velerov1.DefaultNamespace, "backup-1") } +func toUnstructuredOrFail(t *testing.T, obj interface{}) map[string]interface{} { + t.Helper() + + res, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj) + require.NoError(t, err) + + return res +} + +// assertTarballContents verifies that the gzipped tarball stored in the provided +// backupFile contains exactly the file names specified. func assertTarballContents(t *testing.T, backupFile io.Reader, items ...string) { t.Helper() @@ -811,6 +1437,54 @@ func assertTarballContents(t *testing.T, backupFile io.Reader, items ...string) assert.Equal(t, items, files) } +// unstructuredObject is a type alias to improve readability. +type unstructuredObject map[string]interface{} + +// assertTarballFileContents verifies that the gzipped tarball stored in the provided +// backupFile contains the files specified as keys in 'want', and for each of those +// files verifies that the content of the file is JSON and is equivalent to the JSON +// content stored as values in 'want'. +func assertTarballFileContents(t *testing.T, backupFile io.Reader, want map[string]unstructuredObject) { + t.Helper() + + gzr, err := gzip.NewReader(backupFile) + require.NoError(t, err) + + r := tar.NewReader(gzr) + items := make(map[string][]byte) + + for { + hdr, err := r.Next() + if err == io.EOF { + break + } + require.NoError(t, err) + + bytes, err := ioutil.ReadAll(r) + require.NoError(t, err) + + items[hdr.Name] = bytes + } + + for name, wantItem := range want { + gotData, ok := items[name] + assert.True(t, ok, "did not find item %s in tarball", name) + if !ok { + continue + } + + // json-unmarshal the data from the tarball + var got unstructuredObject + err := json.Unmarshal(gotData, &got) + assert.NoError(t, err) + if err != nil { + continue + } + + assert.Equal(t, wantItem, got) + } +} + // assertTarballOrdering ensures that resources were written to the tarball in the expected // order. Any resources *not* in orderedResources are required to come *after* all resources // in orderedResources, in any order. diff --git a/pkg/backup/backup_test.go b/pkg/backup/backup_test.go index a15d31950..3c10cb4d7 100644 --- a/pkg/backup/backup_test.go +++ b/pkg/backup/backup_test.go @@ -51,20 +51,17 @@ var ( falsePointer = &falseVal ) -type fakeAction struct { +// recordResourcesAction is a backup item action that can be configured +// to run for specific resources/namespaces and simply records the items +// that it is executed for. +type recordResourcesAction struct { selector velero.ResourceSelector ids []string backups []v1.Backup additionalItems []velero.ResourceIdentifier } -var _ velero.BackupItemAction = &fakeAction{} - -func newFakeAction(resource string) *fakeAction { - return (&fakeAction{}).ForResource(resource) -} - -func (a *fakeAction) Execute(item runtime.Unstructured, backup *v1.Backup) (runtime.Unstructured, []velero.ResourceIdentifier, error) { +func (a *recordResourcesAction) Execute(item runtime.Unstructured, backup *v1.Backup) (runtime.Unstructured, []velero.ResourceIdentifier, error) { metadata, err := meta.Accessor(item) if err != nil { return item, a.additionalItems, err @@ -75,77 +72,28 @@ func (a *fakeAction) Execute(item runtime.Unstructured, backup *v1.Backup) (runt return item, a.additionalItems, nil } -func (a *fakeAction) AppliesTo() (velero.ResourceSelector, error) { +func (a *recordResourcesAction) AppliesTo() (velero.ResourceSelector, error) { return a.selector, nil } -func (a *fakeAction) ForResource(resource string) *fakeAction { - a.selector.IncludedResources = []string{resource} +func (a *recordResourcesAction) ForResource(resource string) *recordResourcesAction { + a.selector.IncludedResources = append(a.selector.IncludedResources, resource) return a } -func TestResolveActions(t *testing.T) { - tests := []struct { - name string - input []velero.BackupItemAction - expected []resolvedAction - resourcesWithErrors []string - expectError bool - }{ - { - name: "empty input", - input: []velero.BackupItemAction{}, - expected: nil, - }, - { - name: "resolve error", - input: []velero.BackupItemAction{&fakeAction{selector: velero.ResourceSelector{LabelSelector: "=invalid-selector"}}}, - expected: nil, - expectError: true, - }, - { - name: "resolved", - input: []velero.BackupItemAction{newFakeAction("foo"), newFakeAction("bar")}, - expected: []resolvedAction{ - { - BackupItemAction: newFakeAction("foo"), - resourceIncludesExcludes: collections.NewIncludesExcludes().Includes("foodies.somegroup"), - namespaceIncludesExcludes: collections.NewIncludesExcludes(), - selector: labels.Everything(), - }, - { - BackupItemAction: newFakeAction("bar"), - resourceIncludesExcludes: collections.NewIncludesExcludes().Includes("barnacles.anothergroup"), - namespaceIncludesExcludes: collections.NewIncludesExcludes(), - selector: labels.Everything(), - }, - }, - }, - } +func (a *recordResourcesAction) ForNamespace(namespace string) *recordResourcesAction { + a.selector.IncludedNamespaces = append(a.selector.IncludedNamespaces, namespace) + return a +} - 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 := velerotest.NewFakeDiscoveryHelper(false, resources) +func (a *recordResourcesAction) ForLabelSelector(selector string) *recordResourcesAction { + a.selector.LabelSelector = selector + return a +} - actual, err := resolveActions(test.input, discoveryHelper) - gotError := err != nil - - if e, a := test.expectError, gotError; e != a { - t.Fatalf("error: expected %t, got %t", e, a) - } - if test.expectError { - return - } - - assert.Equal(t, test.expected, actual) - }) - } +func (a *recordResourcesAction) WithAdditionalItems(items []velero.ResourceIdentifier) *recordResourcesAction { + a.additionalItems = items + return a } var ( @@ -277,24 +225,6 @@ func TestBackup(t *testing.T) { backupGroupErrors map[*metav1.APIResourceList]error expectedError error }{ - { - name: "error resolving actions returns an error", - backup: &v1.Backup{ - Spec: v1.BackupSpec{ - // cm - shortcut in legacy api group - // csr - shortcut in certificates.k8s.io api group - // roles - fully qualified in rbac.authorization.k8s.io api group - IncludedResources: []string{"cm", "csr", "roles"}, - IncludedNamespaces: []string{"a", "b"}, - ExcludedNamespaces: []string{"c", "d"}, - }, - }, - actions: []velero.BackupItemAction{new(appliesToErrorAction)}, - expectedNamespaces: collections.NewIncludesExcludes().Includes("a", "b").Excludes("c", "d"), - expectedResources: collections.NewIncludesExcludes().Includes("configmaps", "certificatesigningrequests.certificates.k8s.io", "roles.rbac.authorization.k8s.io"), - expectedHooks: []resourceHook{}, - expectedError: errors.New("error calling AppliesTo"), - }, { name: "error resolving hooks returns an error", backup: &v1.Backup{ @@ -466,18 +396,6 @@ func TestBackup(t *testing.T) { } } -// appliesToErrorAction is a backup item action that always returns -// an error when AppliesTo() is called. -type appliesToErrorAction struct{} - -func (a *appliesToErrorAction) AppliesTo() (velero.ResourceSelector, error) { - return velero.ResourceSelector{}, errors.New("error calling AppliesTo") -} - -func (a *appliesToErrorAction) Execute(item runtime.Unstructured, backup *v1.Backup) (runtime.Unstructured, []velero.ResourceIdentifier, error) { - panic("not implemented") -} - type mockGroupBackupperFactory struct { mock.Mock } diff --git a/pkg/backup/item_backupper_test.go b/pkg/backup/item_backupper_test.go index 6093989e9..88f349ccd 100644 --- a/pkg/backup/item_backupper_test.go +++ b/pkg/backup/item_backupper_test.go @@ -29,7 +29,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" - corev1api "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -45,68 +44,6 @@ import ( velerotest "github.com/heptio/velero/pkg/util/test" ) -func TestBackupItemSkips(t *testing.T) { - tests := []struct { - testName string - namespace string - name string - namespaces *collections.IncludesExcludes - groupResource schema.GroupResource - resources *collections.IncludesExcludes - terminating bool - backedUpItems map[itemKey]struct{} - }{ - { - testName: "resource already backed up", - namespace: "ns", - name: "foo", - groupResource: schema.GroupResource{Group: "foo", Resource: "bar"}, - namespaces: collections.NewIncludesExcludes(), - resources: collections.NewIncludesExcludes(), - backedUpItems: map[itemKey]struct{}{ - {resource: "bar.foo", namespace: "ns", name: "foo"}: {}, - }, - }, - { - testName: "terminating resource", - namespace: "ns", - name: "foo", - groupResource: schema.GroupResource{Group: "foo", Resource: "bar"}, - namespaces: collections.NewIncludesExcludes(), - resources: collections.NewIncludesExcludes(), - terminating: true, - }, - } - - for _, test := range tests { - t.Run(test.testName, func(t *testing.T) { - req := &Request{ - NamespaceIncludesExcludes: test.namespaces, - ResourceIncludesExcludes: test.resources, - } - - ib := &defaultItemBackupper{ - backupRequest: req, - backedUpItems: test.backedUpItems, - } - - pod := &corev1api.Pod{ - TypeMeta: metav1.TypeMeta{APIVersion: "v1", Kind: "Pod"}, - ObjectMeta: metav1.ObjectMeta{Namespace: test.namespace, Name: test.name}, - } - - if test.terminating { - pod.ObjectMeta.DeletionTimestamp = &metav1.Time{Time: time.Now()} - } - unstructuredObj, unmarshalErr := runtime.DefaultUnstructuredConverter.ToUnstructured(pod) - require.NoError(t, unmarshalErr) - u := &unstructured.Unstructured{Object: unstructuredObj} - err := ib.backupItem(velerotest.NewLogger(), u, test.groupResource) - assert.NoError(t, err) - }) - } -} - func TestBackupItemNoSkips(t *testing.T) { tests := []struct { name string @@ -140,52 +77,6 @@ func TestBackupItemNoSkips(t *testing.T) { expectError: true, tarWriteError: true, }, - { - name: "action invoked - cluster-scoped", - namespaceIncludesExcludes: collections.NewIncludesExcludes().Includes("*"), - item: `{"metadata":{"name":"bar"}}`, - expectError: false, - expectExcluded: false, - expectedTarHeaderName: "resources/resource.group/cluster/bar.json", - customAction: true, - expectedActionID: "bar", - }, - { - name: "action invoked - namespaced", - namespaceIncludesExcludes: collections.NewIncludesExcludes().Includes("*"), - item: `{"metadata":{"namespace": "myns", "name":"bar"}}`, - expectError: false, - expectExcluded: false, - expectedTarHeaderName: "resources/resource.group/namespaces/myns/bar.json", - customAction: true, - expectedActionID: "myns/bar", - }, - { - name: "action invoked - additional items", - namespaceIncludesExcludes: collections.NewIncludesExcludes().Includes("*"), - item: `{"metadata":{"namespace": "myns", "name":"bar"}}`, - expectError: false, - expectExcluded: false, - expectedTarHeaderName: "resources/resource.group/namespaces/myns/bar.json", - customAction: true, - expectedActionID: "myns/bar", - customActionAdditionalItemIdentifiers: []velero.ResourceIdentifier{ - { - GroupResource: schema.GroupResource{Group: "g1", Resource: "r1"}, - Namespace: "ns1", - Name: "n1", - }, - { - GroupResource: schema.GroupResource{Group: "g2", Resource: "r2"}, - Namespace: "ns2", - Name: "n2", - }, - }, - customActionAdditionalItems: []runtime.Unstructured{ - velerotest.UnstructuredOrDie(`{"apiVersion":"g1/v1","kind":"r1","metadata":{"namespace":"ns1","name":"n1"}}`), - velerotest.UnstructuredOrDie(`{"apiVersion":"g2/v1","kind":"r1","metadata":{"namespace":"ns2","name":"n2"}}`), - }, - }, { name: "action invoked - additional items - error", namespaceIncludesExcludes: collections.NewIncludesExcludes().Includes("*"), @@ -286,7 +177,7 @@ func TestBackupItemNoSkips(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { var ( - action *fakeAction + action *recordResourcesAction backup = new(Request) groupResource = schema.ParseGroupResource("resource.group") backedUpItems = make(map[itemKey]struct{}) @@ -322,9 +213,7 @@ func TestBackupItemNoSkips(t *testing.T) { } if test.customAction { - action = &fakeAction{ - additionalItems: test.customActionAdditionalItemIdentifiers, - } + action = new(recordResourcesAction).WithAdditionalItems(test.customActionAdditionalItemIdentifiers) backup.ResolvedActions = []resolvedAction{ { BackupItemAction: action, @@ -514,59 +403,6 @@ func (a *addAnnotationAction) AppliesTo() (velero.ResourceSelector, error) { panic("not implemented") } -func TestItemActionModificationsToItemPersist(t *testing.T) { - var ( - w = &fakeTarWriter{} - obj = &unstructured.Unstructured{ - Object: map[string]interface{}{ - "metadata": map[string]interface{}{ - "namespace": "myns", - "name": "bar", - }, - }, - } - req = &Request{ - NamespaceIncludesExcludes: collections.NewIncludesExcludes(), - ResourceIncludesExcludes: collections.NewIncludesExcludes(), - ResolvedActions: []resolvedAction{ - { - BackupItemAction: &addAnnotationAction{}, - namespaceIncludesExcludes: collections.NewIncludesExcludes(), - resourceIncludesExcludes: collections.NewIncludesExcludes(), - selector: labels.Everything(), - }, - }, - } - - b = (&defaultItemBackupperFactory{}).newItemBackupper( - req, - make(map[itemKey]struct{}), - nil, - w, - &velerotest.FakeDynamicFactory{}, - velerotest.NewFakeDiscoveryHelper(true, nil), - nil, - newPVCSnapshotTracker(), - nil, - ).(*defaultItemBackupper) - ) - - // our expected backed-up object is the passed-in object plus the annotation - // that the backup item action adds. - expected := obj.DeepCopy() - expected.SetAnnotations(map[string]string{"foo": "bar"}) - - // method under test - require.NoError(t, b.backupItem(velerotest.NewLogger(), obj, schema.ParseGroupResource("resource.group"))) - - // get the actual backed-up item - require.Len(t, w.data, 1) - actual, err := velerotest.GetAsMap(string(w.data[0])) - require.NoError(t, err) - - assert.EqualValues(t, expected.Object, actual) -} - func TestResticAnnotationsPersist(t *testing.T) { var ( w = &fakeTarWriter{}