From 1a339f06acef2ef4f62e2adf349050e194d5fabc Mon Sep 17 00:00:00 2001 From: Steve Kriss Date: Fri, 21 Jun 2019 11:00:49 -0600 Subject: [PATCH 1/3] migrate restore resource filtering tests Signed-off-by: Steve Kriss --- pkg/backup/backup_new_test.go | 56 ++-- pkg/restore/builder.go | 36 +++ pkg/restore/restore_new_test.go | 447 ++++++++++++++++++++++++++++++-- pkg/restore/restore_test.go | 230 ---------------- pkg/test/resources.go | 131 +++++++++- 5 files changed, 607 insertions(+), 293 deletions(-) diff --git a/pkg/backup/backup_new_test.go b/pkg/backup/backup_new_test.go index e7220fcc6..0c5fe48cd 100644 --- a/pkg/backup/backup_new_test.go +++ b/pkg/backup/backup_new_test.go @@ -196,16 +196,16 @@ func TestBackupResourceFiltering(t *testing.T) { Backup(), apiResources: []*test.APIResource{ test.Pods( - withLabel(test.NewPod("foo", "bar"), "a", "b"), + test.NewPod("foo", "bar", test.WithLabels("a", "b")), test.NewPod("zoo", "raz"), ), test.Deployments( test.NewDeployment("foo", "bar"), - withLabel(test.NewDeployment("zoo", "raz"), "a", "b"), + test.NewDeployment("zoo", "raz", test.WithLabels("a", "b")), ), test.PVs( - withLabel(test.NewPV("bar"), "a", "b"), - withLabel(test.NewPV("baz"), "a", "c"), + test.NewPV("bar", test.WithLabels("a", "b")), + test.NewPV("baz", test.WithLabels("a", "c")), ), }, want: []string{ @@ -220,16 +220,16 @@ func TestBackupResourceFiltering(t *testing.T) { Backup(), apiResources: []*test.APIResource{ test.Pods( - withLabel(test.NewPod("foo", "bar"), "velero.io/exclude-from-backup", "true"), + test.NewPod("foo", "bar", test.WithLabels("velero.io/exclude-from-backup", "true")), test.NewPod("zoo", "raz"), ), test.Deployments( test.NewDeployment("foo", "bar"), - withLabel(test.NewDeployment("zoo", "raz"), "velero.io/exclude-from-backup", "true"), + test.NewDeployment("zoo", "raz", test.WithLabels("velero.io/exclude-from-backup", "true")), ), test.PVs( - withLabel(test.NewPV("bar"), "a", "b"), - withLabel(test.NewPV("baz"), "velero.io/exclude-from-backup", "true"), + test.NewPV("bar", test.WithLabels("a", "b")), + test.NewPV("baz", test.WithLabels("velero.io/exclude-from-backup", "true")), ), }, want: []string{ @@ -245,16 +245,16 @@ func TestBackupResourceFiltering(t *testing.T) { Backup(), apiResources: []*test.APIResource{ test.Pods( - withLabel(test.NewPod("foo", "bar"), "velero.io/exclude-from-backup", "true", "a", "b"), - withLabel(test.NewPod("zoo", "raz"), "a", "b"), + test.NewPod("foo", "bar", test.WithLabels("velero.io/exclude-from-backup", "true", "a", "b")), + test.NewPod("zoo", "raz", test.WithLabels("a", "b")), ), test.Deployments( test.NewDeployment("foo", "bar"), - withLabel(test.NewDeployment("zoo", "raz"), "velero.io/exclude-from-backup", "true", "a", "b"), + test.NewDeployment("zoo", "raz", test.WithLabels("velero.io/exclude-from-backup", "true", "a", "b")), ), test.PVs( - withLabel(test.NewPV("bar"), "a", "b"), - withLabel(test.NewPV("baz"), "a", "b", "velero.io/exclude-from-backup", "true"), + test.NewPV("bar", test.WithLabels("a", "b")), + test.NewPV("baz", test.WithLabels("a", "b", "velero.io/exclude-from-backup", "true")), ), }, want: []string{ @@ -268,16 +268,16 @@ func TestBackupResourceFiltering(t *testing.T) { Backup(), apiResources: []*test.APIResource{ test.Pods( - withLabel(test.NewPod("foo", "bar"), "velero.io/exclude-from-backup", "false"), + test.NewPod("foo", "bar", test.WithLabels("velero.io/exclude-from-backup", "false")), test.NewPod("zoo", "raz"), ), test.Deployments( test.NewDeployment("foo", "bar"), - withLabel(test.NewDeployment("zoo", "raz"), "velero.io/exclude-from-backup", "1"), + test.NewDeployment("zoo", "raz", test.WithLabels("velero.io/exclude-from-backup", "1")), ), test.PVs( - withLabel(test.NewPV("bar"), "a", "b"), - withLabel(test.NewPV("baz"), "velero.io/exclude-from-backup", ""), + test.NewPV("bar", test.WithLabels("a", "b")), + test.NewPV("baz", test.WithLabels("velero.io/exclude-from-backup", "")), ), }, want: []string{ @@ -1042,7 +1042,7 @@ func TestBackupActionModifications(t *testing.T) { }), }, want: map[string]unstructuredObject{ - "resources/pods/namespaces/ns-1/pod-1.json": toUnstructuredOrFail(t, withLabel(test.NewPod("ns-1", "pod-1"), "updated", "true")), + "resources/pods/namespaces/ns-1/pod-1.json": toUnstructuredOrFail(t, test.NewPod("ns-1", "pod-1", test.WithLabels("updated", "true"))), }, }, { @@ -1050,7 +1050,7 @@ func TestBackupActionModifications(t *testing.T) { backup: defaultBackup().Backup(), apiResources: []*test.APIResource{ test.Pods( - withLabel(test.NewPod("ns-1", "pod-1"), "should-be-removed", "true"), + test.NewPod("ns-1", "pod-1", test.WithLabels("should-be-removed", "true")), ), }, actions: []velero.BackupItemAction{ @@ -1509,7 +1509,7 @@ func TestBackupWithSnapshots(t *testing.T) { }, apiResources: []*test.APIResource{ test.PVs( - withLabel(test.NewPV("pv-1"), "failure-domain.beta.kubernetes.io/zone", "zone-1"), + test.NewPV("pv-1", test.WithLabels("failure-domain.beta.kubernetes.io/zone", "zone-1")), ), }, snapshotterGetter: map[string]velero.VolumeSnapshotter{ @@ -2088,22 +2088,6 @@ func newHarness(t *testing.T) *harness { } } -func withLabel(obj metav1.Object, labelPairs ...string) metav1.Object { - if len(labelPairs)%2 != 0 { - panic("withLabel requires a series of key-value pairs") - } - labels := obj.GetLabels() - if labels == nil { - labels = make(map[string]string) - } - for i := 0; i < len(labelPairs); i += 2 { - labels[labelPairs[i]] = labelPairs[i+1] - } - obj.SetLabels(labels) - - return obj -} - func newSnapshotLocation(ns, name, provider string) *velerov1.VolumeSnapshotLocation { return &velerov1.VolumeSnapshotLocation{ ObjectMeta: metav1.ObjectMeta{ diff --git a/pkg/restore/builder.go b/pkg/restore/builder.go index 2daf1afd6..474e04484 100644 --- a/pkg/restore/builder.go +++ b/pkg/restore/builder.go @@ -60,6 +60,42 @@ func (b *Builder) Backup(name string) *Builder { return b } +// IncludedNamespaces sets the Restore's included namespaces. +func (b *Builder) IncludedNamespaces(namespaces ...string) *Builder { + b.restore.Spec.IncludedNamespaces = namespaces + return b +} + +// ExcludedNamespaces sets the Restore's excluded namespaces. +func (b *Builder) ExcludedNamespaces(namespaces ...string) *Builder { + b.restore.Spec.ExcludedNamespaces = namespaces + return b +} + +// IncludedResources sets the Restore's included resources. +func (b *Builder) IncludedResources(resources ...string) *Builder { + b.restore.Spec.IncludedResources = resources + return b +} + +// ExcludedResources sets the Restore's excluded resources. +func (b *Builder) ExcludedResources(resources ...string) *Builder { + b.restore.Spec.ExcludedResources = resources + return b +} + +// IncludeClusterResources sets the Restore's "include cluster resources" flag. +func (b *Builder) IncludeClusterResources(val bool) *Builder { + b.restore.Spec.IncludeClusterResources = &val + return b +} + +// LabelSelector sets the Restore's label selector. +func (b *Builder) LabelSelector(selector *metav1.LabelSelector) *Builder { + b.restore.Spec.LabelSelector = selector + return b +} + // NamespaceMappings sets the Restore's namespace mappings. func (b *Builder) NamespaceMappings(mapping ...string) *Builder { if b.restore.Spec.NamespaceMapping == nil { diff --git a/pkg/restore/restore_new_test.go b/pkg/restore/restore_new_test.go index 8197671a7..d0541292c 100644 --- a/pkg/restore/restore_new_test.go +++ b/pkg/restore/restore_new_test.go @@ -29,12 +29,12 @@ import ( "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + corev1api "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/sets" velerov1api "github.com/heptio/velero/pkg/apis/velero/v1" - "github.com/heptio/velero/pkg/backup" "github.com/heptio/velero/pkg/client" "github.com/heptio/velero/pkg/discovery" "github.com/heptio/velero/pkg/test" @@ -42,7 +42,13 @@ import ( testutil "github.com/heptio/velero/pkg/util/test" ) -func TestRestoreNew(t *testing.T) { +// TestRestoreResourceFiltering runs restores 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 created in the API are correct. +// Validation is done by looking at the namespaces/names of the items in +// the API; contents are not checked. +func TestRestoreResourceFiltering(t *testing.T) { tests := []struct { name string restore *velerov1api.Restore @@ -52,35 +58,428 @@ func TestRestoreNew(t *testing.T) { want map[*test.APIResource][]string }{ { - name: "base case - restore a single resource", - restore: defaultRestore().Backup("backup-1").Restore(), - backup: backup.NewNamedBuilder(velerov1api.DefaultNamespace, "backup-1").Backup(), + name: "no filters restores everything", + restore: defaultRestore().Restore(), + backup: defaultBackup().Backup(), tarball: newTarWriter(t). - add("metadata/version", []byte("1")). - add("resources/pods/namespaces/ns-1/pod-1.json", test.NewPod("ns-1", "pod-1")). + addItems("pods", + test.NewPod("ns-1", "pod-1"), + test.NewPod("ns-2", "pod-2"), + ). + addItems("persistentvolumes", + test.NewPV("pv-1"), + test.NewPV("pv-2"), + ). done(), apiResources: []*test.APIResource{ test.Pods(), + test.PVs(), }, want: map[*test.APIResource][]string{ - test.Pods(): {"ns-1/pod-1"}, + test.Pods(): {"ns-1/pod-1", "ns-2/pod-2"}, + test.PVs(): {"/pv-1", "/pv-2"}, }, }, { - name: "restore a resource to a remapped namespace", - restore: defaultRestore().Backup("backup-1").NamespaceMappings("ns-1", "ns-2").Restore(), - backup: backup.NewNamedBuilder(velerov1api.DefaultNamespace, "backup-1").Backup(), + name: "included resources filter only restores resources of those types", + restore: defaultRestore().IncludedResources("pods").Restore(), + backup: defaultBackup().Backup(), tarball: newTarWriter(t). - add("metadata/version", []byte("1")). - add("resources/pods/namespaces/ns-1/pod-1.json", test.NewPod("ns-1", "pod-1")). + addItems("pods", + test.NewPod("ns-1", "pod-1"), + test.NewPod("ns-2", "pod-2"), + ). + addItems("persistentvolumes", + test.NewPV("pv-1"), + test.NewPV("pv-2"), + ). done(), apiResources: []*test.APIResource{ test.Pods(), + test.PVs(), }, want: map[*test.APIResource][]string{ - test.Pods(): {"ns-2/pod-1"}, + test.Pods(): {"ns-1/pod-1", "ns-2/pod-2"}, }, }, + { + name: "excluded resources filter only restores resources not of those types", + restore: defaultRestore().ExcludedResources("pvs").Restore(), + backup: defaultBackup().Backup(), + tarball: newTarWriter(t). + addItems("pods", + test.NewPod("ns-1", "pod-1"), + test.NewPod("ns-2", "pod-2"), + ). + addItems("persistentvolumes", + test.NewPV("pv-1"), + test.NewPV("pv-2"), + ). + done(), + apiResources: []*test.APIResource{ + test.Pods(), + test.PVs(), + }, + want: map[*test.APIResource][]string{ + test.Pods(): {"ns-1/pod-1", "ns-2/pod-2"}, + }, + }, + { + name: "included namespaces filter only restores resources in those namespaces", + restore: defaultRestore().IncludedNamespaces("ns-1").Restore(), + backup: defaultBackup().Backup(), + tarball: newTarWriter(t). + addItems("pods", + test.NewPod("ns-1", "pod-1"), + test.NewPod("ns-2", "pod-2"), + ). + addItems("deployments.apps", + test.NewDeployment("ns-1", "deploy-1"), + test.NewDeployment("ns-2", "deploy-2"), + ). + addItems("persistentvolumes", + test.NewPV("pv-1"), + test.NewPV("pv-2"), + ). + done(), + apiResources: []*test.APIResource{ + test.Pods(), + test.Deployments(), + test.PVs(), + }, + want: map[*test.APIResource][]string{ + test.Pods(): {"ns-1/pod-1"}, + test.Deployments(): {"ns-1/deploy-1"}, + }, + }, + { + name: "excluded namespaces filter only restores resources not in those namespaces", + restore: defaultRestore().ExcludedNamespaces("ns-2").Restore(), + backup: defaultBackup().Backup(), + tarball: newTarWriter(t). + addItems("pods", + test.NewPod("ns-1", "pod-1"), + test.NewPod("ns-2", "pod-2"), + ). + addItems("deployments.apps", + test.NewDeployment("ns-1", "deploy-1"), + test.NewDeployment("ns-2", "deploy-2"), + ). + addItems("persistentvolumes", + test.NewPV("pv-1"), + test.NewPV("pv-2"), + ). + done(), + apiResources: []*test.APIResource{ + test.Pods(), + test.Deployments(), + test.PVs(), + }, + want: map[*test.APIResource][]string{ + test.Pods(): {"ns-1/pod-1"}, + test.Deployments(): {"ns-1/deploy-1"}, + }, + }, + { + name: "IncludeClusterResources=false only restores namespaced resources", + restore: defaultRestore().IncludeClusterResources(false).Restore(), + backup: defaultBackup().Backup(), + tarball: newTarWriter(t). + addItems("pods", + test.NewPod("ns-1", "pod-1"), + test.NewPod("ns-2", "pod-2"), + ). + addItems("deployments.apps", + test.NewDeployment("ns-1", "deploy-1"), + test.NewDeployment("ns-2", "deploy-2"), + ). + addItems("persistentvolumes", + test.NewPV("pv-1"), + test.NewPV("pv-2"), + ). + done(), + apiResources: []*test.APIResource{ + test.Pods(), + test.Deployments(), + test.PVs(), + }, + want: map[*test.APIResource][]string{ + test.Pods(): {"ns-1/pod-1", "ns-2/pod-2"}, + test.Deployments(): {"ns-1/deploy-1", "ns-2/deploy-2"}, + }, + }, + { + name: "label selector only restores matching resources", + restore: defaultRestore().LabelSelector(&metav1.LabelSelector{MatchLabels: map[string]string{"a": "b"}}).Restore(), + backup: defaultBackup().Backup(), + tarball: newTarWriter(t). + addItems("pods", + test.NewPod("ns-1", "pod-1", test.WithLabels("a", "b")), + test.NewPod("ns-2", "pod-2"), + ). + addItems("deployments.apps", + test.NewDeployment("ns-1", "deploy-1"), + test.NewDeployment("ns-2", "deploy-2", test.WithLabels("a", "b")), + ). + addItems("persistentvolumes", + test.NewPV("pv-1", test.WithLabels("a", "b")), + test.NewPV("pv-2", test.WithLabels("a", "c")), + ). + done(), + apiResources: []*test.APIResource{ + test.Pods(), + test.Deployments(), + test.PVs(), + }, + want: map[*test.APIResource][]string{ + test.Pods(): {"ns-1/pod-1"}, + test.Deployments(): {"ns-2/deploy-2"}, + test.PVs(): {"/pv-1"}, + }, + }, + { + name: "should include cluster-scoped resources if restoring subset of namespaces and IncludeClusterResources=true", + restore: defaultRestore().IncludedNamespaces("ns-1").IncludeClusterResources(true).Restore(), + backup: defaultBackup().Backup(), + tarball: newTarWriter(t). + addItems("pods", + test.NewPod("ns-1", "pod-1"), + test.NewPod("ns-2", "pod-2"), + ). + addItems("deployments.apps", + test.NewDeployment("ns-1", "deploy-1"), + test.NewDeployment("ns-2", "deploy-2"), + ). + addItems("persistentvolumes", + test.NewPV("pv-1"), + test.NewPV("pv-2"), + ). + done(), + apiResources: []*test.APIResource{ + test.Pods(), + test.Deployments(), + test.PVs(), + }, + want: map[*test.APIResource][]string{ + test.Pods(): {"ns-1/pod-1"}, + test.Deployments(): {"ns-1/deploy-1"}, + test.PVs(): {"/pv-1", "/pv-2"}, + }, + }, + { + name: "should not include cluster-scoped resources if restoring subset of namespaces and IncludeClusterResources=false", + restore: defaultRestore().IncludedNamespaces("ns-1").IncludeClusterResources(false).Restore(), + backup: defaultBackup().Backup(), + tarball: newTarWriter(t). + addItems("pods", + test.NewPod("ns-1", "pod-1"), + test.NewPod("ns-2", "pod-2"), + ). + addItems("deployments.apps", + test.NewDeployment("ns-1", "deploy-1"), + test.NewDeployment("ns-2", "deploy-2"), + ). + addItems("persistentvolumes", + test.NewPV("pv-1"), + test.NewPV("pv-2"), + ). + done(), + apiResources: []*test.APIResource{ + test.Pods(), + test.Deployments(), + test.PVs(), + }, + want: map[*test.APIResource][]string{ + test.Pods(): {"ns-1/pod-1"}, + test.Deployments(): {"ns-1/deploy-1"}, + }, + }, + { + name: "should include cluster-scoped resources if restoring all namespaces and IncludeClusterResources=true", + restore: defaultRestore().IncludeClusterResources(true).Restore(), + backup: defaultBackup().Backup(), + tarball: newTarWriter(t). + addItems("pods", + test.NewPod("ns-1", "pod-1"), + test.NewPod("ns-2", "pod-2"), + ). + addItems("deployments.apps", + test.NewDeployment("ns-1", "deploy-1"), + test.NewDeployment("ns-2", "deploy-2"), + ). + addItems("persistentvolumes", + test.NewPV("pv-1"), + test.NewPV("pv-2"), + ). + done(), + apiResources: []*test.APIResource{ + test.Pods(), + test.Deployments(), + test.PVs(), + }, + want: map[*test.APIResource][]string{ + test.Pods(): {"ns-1/pod-1", "ns-2/pod-2"}, + test.Deployments(): {"ns-1/deploy-1", "ns-2/deploy-2"}, + test.PVs(): {"/pv-1", "/pv-2"}, + }, + }, + { + name: "should not include cluster-scoped resources if restoring all namespaces and IncludeClusterResources=false", + restore: defaultRestore().IncludeClusterResources(false).Restore(), + backup: defaultBackup().Backup(), + tarball: newTarWriter(t). + addItems("pods", + test.NewPod("ns-1", "pod-1"), + test.NewPod("ns-2", "pod-2"), + ). + addItems("deployments.apps", + test.NewDeployment("ns-1", "deploy-1"), + test.NewDeployment("ns-2", "deploy-2"), + ). + addItems("persistentvolumes", + test.NewPV("pv-1"), + test.NewPV("pv-2"), + ). + done(), + apiResources: []*test.APIResource{ + test.Pods(), + test.Deployments(), + test.PVs(), + }, + want: map[*test.APIResource][]string{ + test.Pods(): {"ns-1/pod-1", "ns-2/pod-2"}, + test.Deployments(): {"ns-1/deploy-1", "ns-2/deploy-2"}, + }, + }, + { + name: "when a wildcard and a specific resource are included, the wildcard takes precedence", + restore: defaultRestore().IncludedResources("*", "pods").Restore(), + backup: defaultBackup().Backup(), + tarball: newTarWriter(t). + addItems("pods", + test.NewPod("ns-1", "pod-1"), + test.NewPod("ns-2", "pod-2"), + ). + addItems("deployments.apps", + test.NewDeployment("ns-1", "deploy-1"), + test.NewDeployment("ns-2", "deploy-2"), + ). + addItems("persistentvolumes", + test.NewPV("pv-1"), + test.NewPV("pv-2"), + ). + done(), + apiResources: []*test.APIResource{ + test.Pods(), + test.Deployments(), + test.PVs(), + }, + want: map[*test.APIResource][]string{ + test.Pods(): {"ns-1/pod-1", "ns-2/pod-2"}, + test.Deployments(): {"ns-1/deploy-1", "ns-2/deploy-2"}, + test.PVs(): {"/pv-1", "/pv-2"}, + }, + }, + { + name: "wildcard excludes are ignored", + restore: defaultRestore().ExcludedResources("*").Restore(), + backup: defaultBackup().Backup(), + tarball: newTarWriter(t). + addItems("pods", + test.NewPod("ns-1", "pod-1"), + test.NewPod("ns-2", "pod-2"), + ). + addItems("deployments.apps", + test.NewDeployment("ns-1", "deploy-1"), + test.NewDeployment("ns-2", "deploy-2"), + ). + addItems("persistentvolumes", + test.NewPV("pv-1"), + test.NewPV("pv-2"), + ). + done(), + apiResources: []*test.APIResource{ + test.Pods(), + test.Deployments(), + test.PVs(), + }, + want: map[*test.APIResource][]string{ + test.Pods(): {"ns-1/pod-1", "ns-2/pod-2"}, + test.Deployments(): {"ns-1/deploy-1", "ns-2/deploy-2"}, + test.PVs(): {"/pv-1", "/pv-2"}, + }, + }, + { + name: "unresolvable included resources are ignored", + restore: defaultRestore().IncludedResources("pods", "unresolvable").Restore(), + backup: defaultBackup().Backup(), + tarball: newTarWriter(t). + addItems("pods", + test.NewPod("ns-1", "pod-1"), + test.NewPod("ns-2", "pod-2"), + ). + addItems("deployments.apps", + test.NewDeployment("ns-1", "deploy-1"), + test.NewDeployment("ns-2", "deploy-2"), + ). + addItems("persistentvolumes", + test.NewPV("pv-1"), + test.NewPV("pv-2"), + ). + done(), + apiResources: []*test.APIResource{ + test.Pods(), + test.Deployments(), + test.PVs(), + }, + want: map[*test.APIResource][]string{ + test.Pods(): {"ns-1/pod-1", "ns-2/pod-2"}, + }, + }, + { + name: "unresolvable excluded resources are ignored", + restore: defaultRestore().ExcludedResources("deployments", "unresolvable").Restore(), + backup: defaultBackup().Backup(), + tarball: newTarWriter(t). + addItems("pods", + test.NewPod("ns-1", "pod-1"), + test.NewPod("ns-2", "pod-2"), + ). + addItems("deployments.apps", + test.NewDeployment("ns-1", "deploy-1"), + test.NewDeployment("ns-2", "deploy-2"), + ). + addItems("persistentvolumes", + test.NewPV("pv-1"), + test.NewPV("pv-2"), + ). + done(), + apiResources: []*test.APIResource{ + test.Pods(), + test.Deployments(), + test.PVs(), + }, + want: map[*test.APIResource][]string{ + test.Pods(): {"ns-1/pod-1", "ns-2/pod-2"}, + test.PVs(): {"/pv-1", "/pv-2"}, + }, + }, + { + name: "mirror pods are not restored", + restore: defaultRestore().Restore(), + backup: defaultBackup().Backup(), + tarball: newTarWriter(t).addItems("pods", test.NewPod("ns-1", "pod-1", test.WithAnnotations(corev1api.MirrorPodAnnotationKey, "foo"))).done(), + apiResources: []*test.APIResource{test.Pods()}, + want: map[*test.APIResource][]string{test.Pods(): {}}, + }, + { + name: "service accounts are restored", + restore: defaultRestore().Restore(), + backup: defaultBackup().Backup(), + tarball: newTarWriter(t).addItems("serviceaccounts", test.NewServiceAccount("ns-1", "sa-1")).done(), + apiResources: []*test.APIResource{test.ServiceAccounts()}, + want: map[*test.APIResource][]string{test.ServiceAccounts(): {"ns-1/sa-1"}}, + }, } for _, tc := range tests { @@ -110,7 +509,7 @@ func TestRestoreNew(t *testing.T) { } func defaultRestore() *Builder { - return NewNamedBuilder(velerov1api.DefaultNamespace, "restore-1") + return NewNamedBuilder(velerov1api.DefaultNamespace, "restore-1").Backup("backup-1") } // assertAPIContents asserts that the dynamic client on the provided harness contains @@ -160,6 +559,24 @@ func newTarWriter(t *testing.T) *tarWriter { return tw } +func (tw *tarWriter) addItems(groupVersion string, items ...metav1.Object) *tarWriter { + tw.t.Helper() + + for _, obj := range items { + + var path string + if obj.GetNamespace() == "" { + path = fmt.Sprintf("resources/%s/cluster/%s.json", groupVersion, obj.GetName()) + } else { + path = fmt.Sprintf("resources/%s/namespaces/%s/%s.json", groupVersion, obj.GetNamespace(), obj.GetName()) + } + + tw.add(path, obj) + } + + return tw +} + func (tw *tarWriter) add(name string, obj interface{}) *tarWriter { tw.t.Helper() diff --git a/pkg/restore/restore_test.go b/pkg/restore/restore_test.go index 25a6ceb56..7d8158af0 100644 --- a/pkg/restore/restore_test.go +++ b/pkg/restore/restore_test.go @@ -123,96 +123,6 @@ func TestPrioritizeResources(t *testing.T) { } } -func TestRestoreNamespaceFiltering(t *testing.T) { - tests := []struct { - name string - fileSystem *velerotest.FakeFileSystem - baseDir string - restore *api.Restore - expectedReadDirs []string - prioritizedResources []schema.GroupResource - }{ - { - name: "namespacesToRestore having * restores all namespaces", - fileSystem: velerotest.NewFakeFileSystem().WithDirectories("bak/resources/nodes/cluster", "bak/resources/secrets/namespaces/a", "bak/resources/secrets/namespaces/b", "bak/resources/secrets/namespaces/c"), - baseDir: "bak", - restore: &api.Restore{Spec: api.RestoreSpec{IncludedNamespaces: []string{"*"}}}, - expectedReadDirs: []string{"bak/resources", "bak/resources/nodes/cluster", "bak/resources/secrets/namespaces", "bak/resources/secrets/namespaces/a", "bak/resources/secrets/namespaces/b", "bak/resources/secrets/namespaces/c"}, - prioritizedResources: []schema.GroupResource{ - {Resource: "nodes"}, - {Resource: "secrets"}, - }, - }, - { - name: "namespacesToRestore properly filters", - fileSystem: velerotest.NewFakeFileSystem().WithDirectories("bak/resources/nodes/cluster", "bak/resources/secrets/namespaces/a", "bak/resources/secrets/namespaces/b", "bak/resources/secrets/namespaces/c"), - baseDir: "bak", - restore: &api.Restore{Spec: api.RestoreSpec{IncludedNamespaces: []string{"b", "c"}}}, - expectedReadDirs: []string{"bak/resources", "bak/resources/nodes/cluster", "bak/resources/secrets/namespaces", "bak/resources/secrets/namespaces/b", "bak/resources/secrets/namespaces/c"}, - prioritizedResources: []schema.GroupResource{ - {Resource: "nodes"}, - {Resource: "secrets"}, - }, - }, - { - name: "namespacesToRestore properly filters with exclusion filter", - fileSystem: velerotest.NewFakeFileSystem().WithDirectories("bak/resources/nodes/cluster", "bak/resources/secrets/namespaces/a", "bak/resources/secrets/namespaces/b", "bak/resources/secrets/namespaces/c"), - baseDir: "bak", - restore: &api.Restore{Spec: api.RestoreSpec{IncludedNamespaces: []string{"*"}, ExcludedNamespaces: []string{"a"}}}, - expectedReadDirs: []string{"bak/resources", "bak/resources/nodes/cluster", "bak/resources/secrets/namespaces", "bak/resources/secrets/namespaces/b", "bak/resources/secrets/namespaces/c"}, - prioritizedResources: []schema.GroupResource{ - {Resource: "nodes"}, - {Resource: "secrets"}, - }, - }, - { - name: "namespacesToRestore properly filters with inclusion & exclusion filters", - fileSystem: velerotest.NewFakeFileSystem().WithDirectories("bak/resources/nodes/cluster", "bak/resources/secrets/namespaces/a", "bak/resources/secrets/namespaces/b", "bak/resources/secrets/namespaces/c"), - baseDir: "bak", - restore: &api.Restore{ - Spec: api.RestoreSpec{ - IncludedNamespaces: []string{"a", "b", "c"}, - ExcludedNamespaces: []string{"b"}, - }, - }, - expectedReadDirs: []string{"bak/resources", "bak/resources/nodes/cluster", "bak/resources/secrets/namespaces", "bak/resources/secrets/namespaces/a", "bak/resources/secrets/namespaces/c"}, - prioritizedResources: []schema.GroupResource{ - {Resource: "nodes"}, - {Resource: "secrets"}, - }, - }, - } - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - log := velerotest.NewLogger() - - nsClient := &velerotest.FakeNamespaceClient{} - - ctx := &context{ - restore: test.restore, - namespaceClient: nsClient, - fileSystem: test.fileSystem, - log: log, - prioritizedResources: test.prioritizedResources, - restoreDir: test.baseDir, - } - - nsClient.On("Get", mock.Anything, metav1.GetOptions{}).Return(&v1.Namespace{}, nil) - - warnings, errors := ctx.restoreFromDir() - - assert.Empty(t, warnings.Velero) - assert.Empty(t, warnings.Cluster) - assert.Empty(t, warnings.Namespaces) - assert.Empty(t, errors.Velero) - assert.Empty(t, errors.Cluster) - assert.Empty(t, errors.Namespaces) - assert.Equal(t, test.expectedReadDirs, test.fileSystem.ReadDirCalls) - }) - } -} - func TestRestorePriority(t *testing.T) { tests := []struct { name string @@ -374,13 +284,6 @@ func TestNamespaceRemapping(t *testing.T) { } func TestRestoreResourceForNamespace(t *testing.T) { - var ( - trueVal = true - falseVal = false - truePtr = &trueVal - falsePtr = &falseVal - ) - tests := []struct { name string namespace string @@ -392,19 +295,6 @@ func TestRestoreResourceForNamespace(t *testing.T) { expectedErrors Result expectedObjs []unstructured.Unstructured }{ - { - name: "basic normal case", - namespace: "ns-1", - resourcePath: "configmaps", - labelSelector: labels.NewSelector(), - fileSystem: velerotest.NewFakeFileSystem(). - WithFile("configmaps/cm-1.json", newNamedTestConfigMap("cm-1").ToJSON()). - WithFile("configmaps/cm-2.json", newNamedTestConfigMap("cm-2").ToJSON()), - expectedObjs: toUnstructured( - newNamedTestConfigMap("cm-1").ConfigMap, - newNamedTestConfigMap("cm-2").ConfigMap, - ), - }, { name: "no such directory causes error", namespace: "ns-1", @@ -437,21 +327,6 @@ func TestRestoreResourceForNamespace(t *testing.T) { }, expectedObjs: toUnstructured(newNamedTestConfigMap("cm-2").ConfigMap), }, - { - name: "matching label selector correctly includes", - namespace: "ns-1", - resourcePath: "configmaps", - labelSelector: labels.SelectorFromSet(labels.Set(map[string]string{"foo": "bar"})), - fileSystem: velerotest.NewFakeFileSystem().WithFile("configmaps/cm-1.json", newTestConfigMap().WithLabels(map[string]string{"foo": "bar"}).ToJSON()), - expectedObjs: toUnstructured(newTestConfigMap().WithLabels(map[string]string{"foo": "bar"}).ConfigMap), - }, - { - name: "non-matching label selector correctly excludes", - namespace: "ns-1", - resourcePath: "configmaps", - labelSelector: labels.SelectorFromSet(labels.Set(map[string]string{"foo": "not-bar"})), - fileSystem: velerotest.NewFakeFileSystem().WithFile("configmaps/cm-1.json", newTestConfigMap().WithLabels(map[string]string{"foo": "bar"}).ToJSON()), - }, { name: "namespace is remapped", namespace: "ns-2", @@ -492,111 +367,6 @@ func TestRestoreResourceForNamespace(t *testing.T) { }, expectedObjs: toUnstructured(newTestConfigMap().ConfigMap), }, - { - name: "cluster-scoped resources are skipped when IncludeClusterResources=false", - namespace: "", - resourcePath: "persistentvolumes", - labelSelector: labels.NewSelector(), - includeClusterResources: falsePtr, - fileSystem: velerotest.NewFakeFileSystem().WithFile("persistentvolumes/pv-1.json", newTestPV().ToJSON()), - }, - { - name: "namespaced resources are not skipped when IncludeClusterResources=false", - namespace: "ns-1", - resourcePath: "configmaps", - labelSelector: labels.NewSelector(), - includeClusterResources: falsePtr, - fileSystem: velerotest.NewFakeFileSystem().WithFile("configmaps/cm-1.json", newTestConfigMap().ToJSON()), - expectedObjs: toUnstructured(newTestConfigMap().ConfigMap), - }, - { - name: "cluster-scoped resources are not skipped when IncludeClusterResources=true", - namespace: "", - resourcePath: "persistentvolumes", - labelSelector: labels.NewSelector(), - includeClusterResources: truePtr, - fileSystem: velerotest.NewFakeFileSystem().WithFile("persistentvolumes/pv-1.json", newTestPV().ToJSON()), - expectedObjs: toUnstructured(newTestPV().PersistentVolume), - }, - { - name: "namespaced resources are not skipped when IncludeClusterResources=true", - namespace: "ns-1", - resourcePath: "configmaps", - labelSelector: labels.NewSelector(), - includeClusterResources: truePtr, - fileSystem: velerotest.NewFakeFileSystem().WithFile("configmaps/cm-1.json", newTestConfigMap().ToJSON()), - expectedObjs: toUnstructured(newTestConfigMap().ConfigMap), - }, - { - name: "cluster-scoped resources are not skipped when IncludeClusterResources=nil", - namespace: "", - resourcePath: "persistentvolumes", - labelSelector: labels.NewSelector(), - includeClusterResources: nil, - fileSystem: velerotest.NewFakeFileSystem().WithFile("persistentvolumes/pv-1.json", newTestPV().ToJSON()), - expectedObjs: toUnstructured(newTestPV().PersistentVolume), - }, - { - name: "namespaced resources are not skipped when IncludeClusterResources=nil", - namespace: "ns-1", - resourcePath: "configmaps", - labelSelector: labels.NewSelector(), - includeClusterResources: nil, - fileSystem: velerotest.NewFakeFileSystem().WithFile("configmaps/cm-1.json", newTestConfigMap().ToJSON()), - expectedObjs: toUnstructured(newTestConfigMap().ConfigMap), - }, - { - name: "serviceaccounts are restored", - namespace: "ns-1", - resourcePath: "serviceaccounts", - labelSelector: labels.NewSelector(), - includeClusterResources: nil, - fileSystem: velerotest.NewFakeFileSystem().WithFile("serviceaccounts/sa-1.json", newTestServiceAccount().ToJSON()), - expectedObjs: toUnstructured(newTestServiceAccount().ServiceAccount), - }, - { - name: "non-mirror pods are restored", - namespace: "ns-1", - resourcePath: "pods", - labelSelector: labels.NewSelector(), - includeClusterResources: nil, - fileSystem: velerotest.NewFakeFileSystem(). - WithFile( - "pods/pod.json", - NewTestUnstructured(). - WithAPIVersion("v1"). - WithKind("Pod"). - WithNamespace("ns-1"). - WithName("pod1"). - ToJSON(), - ), - expectedObjs: []unstructured.Unstructured{ - *(NewTestUnstructured(). - WithAPIVersion("v1"). - WithKind("Pod"). - WithNamespace("ns-1"). - WithName("pod1"). - Unstructured), - }, - }, - { - name: "mirror pods are not restored", - namespace: "ns-1", - resourcePath: "pods", - labelSelector: labels.NewSelector(), - includeClusterResources: nil, - fileSystem: velerotest.NewFakeFileSystem(). - WithFile( - "pods/pod.json", - NewTestUnstructured(). - WithAPIVersion("v1"). - WithKind("Pod"). - WithNamespace("ns-1"). - WithName("pod1"). - WithAnnotations(v1.MirrorPodAnnotationKey). - ToJSON(), - ), - }, } var ( diff --git a/pkg/test/resources.go b/pkg/test/resources.go index 95f6de044..d3a221813 100644 --- a/pkg/test/resources.go +++ b/pkg/test/resources.go @@ -121,64 +121,129 @@ func Namespaces(items ...metav1.Object) *APIResource { } } -func NewPod(ns, name string) *corev1.Pod { - return &corev1.Pod{ +func ServiceAccounts(items ...metav1.Object) *APIResource { + return &APIResource{ + Group: "", + Version: "v1", + Name: "serviceaccounts", + ShortName: "sa", + Namespaced: true, + Items: items, + } +} + +type ObjectOpts func(metav1.Object) + +func NewPod(ns, name string, opts ...ObjectOpts) *corev1.Pod { + obj := &corev1.Pod{ TypeMeta: metav1.TypeMeta{ Kind: "Pod", APIVersion: "v1", }, ObjectMeta: objectMeta(ns, name), } + + for _, opt := range opts { + opt(obj) + } + + return obj } -func NewPVC(ns, name string) *corev1.PersistentVolumeClaim { - return &corev1.PersistentVolumeClaim{ +func NewPVC(ns, name string, opts ...ObjectOpts) *corev1.PersistentVolumeClaim { + obj := &corev1.PersistentVolumeClaim{ TypeMeta: metav1.TypeMeta{ Kind: "PersistentVolumeClaim", APIVersion: "v1", }, ObjectMeta: objectMeta(ns, name), } + + for _, opt := range opts { + opt(obj) + } + + return obj } -func NewPV(name string) *corev1.PersistentVolume { - return &corev1.PersistentVolume{ +func NewPV(name string, opts ...ObjectOpts) *corev1.PersistentVolume { + obj := &corev1.PersistentVolume{ TypeMeta: metav1.TypeMeta{ Kind: "PersistentVolume", APIVersion: "v1", }, ObjectMeta: objectMeta("", name), } + + for _, opt := range opts { + opt(obj) + } + + return obj } -func NewSecret(ns, name string) *corev1.Secret { - return &corev1.Secret{ +func NewSecret(ns, name string, opts ...ObjectOpts) *corev1.Secret { + obj := &corev1.Secret{ TypeMeta: metav1.TypeMeta{ Kind: "Secret", APIVersion: "v1", }, ObjectMeta: objectMeta(ns, name), } + + for _, opt := range opts { + opt(obj) + } + + return obj } -func NewDeployment(ns, name string) *appsv1.Deployment { - return &appsv1.Deployment{ +func NewDeployment(ns, name string, opts ...ObjectOpts) *appsv1.Deployment { + obj := &appsv1.Deployment{ TypeMeta: metav1.TypeMeta{ Kind: "Deployment", APIVersion: "apps/v1", }, ObjectMeta: objectMeta(ns, name), } + + for _, opt := range opts { + opt(obj) + } + + return obj } -func NewNamespace(name string) *corev1.Namespace { - return &corev1.Namespace{ +func NewServiceAccount(ns, name string, opts ...ObjectOpts) *corev1.ServiceAccount { + obj := &corev1.ServiceAccount{ + TypeMeta: metav1.TypeMeta{ + Kind: "ServiceAccount", + APIVersion: "v1", + }, + ObjectMeta: objectMeta(ns, name), + } + + for _, opt := range opts { + opt(obj) + } + + return obj +} + +func NewNamespace(name string, opts ...ObjectOpts) *corev1.Namespace { + obj := &corev1.Namespace{ TypeMeta: metav1.TypeMeta{ Kind: "Namespace", APIVersion: "v1", }, ObjectMeta: objectMeta("", name), } + + for _, opt := range opts { + opt(obj) + } + + return obj } func objectMeta(ns, name string) metav1.ObjectMeta { @@ -187,3 +252,45 @@ func objectMeta(ns, name string) metav1.ObjectMeta { Name: name, } } + +// WithLabels is a functional option that applies the specified +// label keys/values to an object. +func WithLabels(labels ...string) func(obj metav1.Object) { + return func(obj metav1.Object) { + objLabels := obj.GetLabels() + if objLabels == nil { + objLabels = make(map[string]string) + } + + if len(labels)%2 != 0 { + labels = append(labels, "") + } + + for i := 0; i < len(labels); i += 2 { + objLabels[labels[i]] = labels[i+1] + } + + obj.SetLabels(objLabels) + } +} + +// WithAnnotations is a functional option that applies the specified +// annotation keys/values to an object. +func WithAnnotations(vals ...string) func(obj metav1.Object) { + return func(obj metav1.Object) { + objAnnotations := obj.GetAnnotations() + if objAnnotations == nil { + objAnnotations = make(map[string]string) + } + + if len(vals)%2 != 0 { + vals = append(vals, "") + } + + for i := 0; i < len(vals); i += 2 { + objAnnotations[vals[i]] = vals[i+1] + } + + obj.SetAnnotations(objAnnotations) + } +} From 5d8ba1b90dbe7d6f1c0fec9fb69eafdb8aec0805 Mon Sep 17 00:00:00 2001 From: Steve Kriss Date: Fri, 21 Jun 2019 12:38:54 -0600 Subject: [PATCH 2/3] migrate namespace mapping tests Signed-off-by: Steve Kriss --- pkg/restore/restore_new_test.go | 59 +++++++++++++++++++++++++++ pkg/restore/restore_test.go | 72 --------------------------------- 2 files changed, 59 insertions(+), 72 deletions(-) diff --git a/pkg/restore/restore_new_test.go b/pkg/restore/restore_new_test.go index d0541292c..9cf081cf5 100644 --- a/pkg/restore/restore_new_test.go +++ b/pkg/restore/restore_new_test.go @@ -508,6 +508,65 @@ func TestRestoreResourceFiltering(t *testing.T) { } } +// TestRestoreNamespaceMapping runs restores with namespace mappings specified, +// and verifies that the set of items created in the API are in the correct +// namespaces. Validation is done by looking at the namespaces/names of the items +// in the API; contents are not checked. +func TestRestoreNamespaceMapping(t *testing.T) { + tests := []struct { + name string + restore *velerov1api.Restore + backup *velerov1api.Backup + apiResources []*test.APIResource + tarball io.Reader + want map[*test.APIResource][]string + }{ + { + name: "namespace mappings are applied", + restore: defaultRestore().NamespaceMappings("ns-1", "mapped-ns-1", "ns-2", "mapped-ns-2").Restore(), + backup: defaultBackup().Backup(), + apiResources: []*test.APIResource{ + test.Pods(), + }, + tarball: newTarWriter(t). + addItems("pods", + test.NewPod("ns-1", "pod-1"), + test.NewPod("ns-2", "pod-2"), + test.NewPod("ns-3", "pod-3"), + ). + done(), + want: map[*test.APIResource][]string{ + test.Pods(): {"mapped-ns-1/pod-1", "mapped-ns-2/pod-2", "ns-3/pod-3"}, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + h := newHarness(t) + + for _, r := range tc.apiResources { + h.DiscoveryClient.WithAPIResource(r) + } + require.NoError(t, h.restorer.discoveryHelper.Refresh()) + + warnings, errs := h.restorer.Restore( + h.log, + tc.restore, + tc.backup, + nil, // volume snapshots + tc.tarball, + nil, // actions + nil, // snapshot location lister + nil, // volume snapshotter getter + ) + + assertEmptyResults(t, warnings, errs) + assertAPIContents(t, h, tc.want) + }) + } +} + func defaultRestore() *Builder { return NewNamedBuilder(velerov1api.DefaultNamespace, "restore-1").Backup("backup-1") } diff --git a/pkg/restore/restore_test.go b/pkg/restore/restore_test.go index 7d8158af0..f594a1a57 100644 --- a/pkg/restore/restore_test.go +++ b/pkg/restore/restore_test.go @@ -219,70 +219,6 @@ func TestRestorePriority(t *testing.T) { } } -func TestNamespaceRemapping(t *testing.T) { - var ( - baseDir = "bak" - restore = &api.Restore{Spec: api.RestoreSpec{IncludedNamespaces: []string{"*"}, NamespaceMapping: map[string]string{"ns-1": "ns-2"}}} - prioritizedResources = []schema.GroupResource{{Resource: "namespaces"}, {Resource: "configmaps"}} - labelSelector = labels.NewSelector() - fileSystem = velerotest.NewFakeFileSystem(). - WithFile("bak/resources/configmaps/namespaces/ns-1/cm-1.json", newTestConfigMap().WithNamespace("ns-1").ToJSON()). - WithFile("bak/resources/namespaces/cluster/ns-1.json", newTestNamespace("ns-1").ToJSON()) - expectedNS = "ns-2" - expectedObjs = toUnstructured(newTestConfigMap().WithNamespace("ns-2").ConfigMap) - ) - - resourceClient := &velerotest.FakeDynamicClient{} - for i := range expectedObjs { - addRestoreLabels(&expectedObjs[i], "", "") - resourceClient.On("Create", &expectedObjs[i]).Return(&expectedObjs[i], nil) - } - - dynamicFactory := &velerotest.FakeDynamicFactory{} - resource := metav1.APIResource{Name: "configmaps", Namespaced: true} - gv := schema.GroupVersion{Group: "", Version: "v1"} - dynamicFactory.On("ClientForGroupVersionResource", gv, resource, expectedNS).Return(resourceClient, nil) - - nsClient := &velerotest.FakeNamespaceClient{} - - ctx := &context{ - dynamicFactory: dynamicFactory, - fileSystem: fileSystem, - selector: labelSelector, - namespaceClient: nsClient, - prioritizedResources: prioritizedResources, - restore: restore, - backup: &api.Backup{}, - log: velerotest.NewLogger(), - applicableActions: make(map[schema.GroupResource][]resolvedAction), - resourceClients: make(map[resourceClientKey]pkgclient.Dynamic), - restoredItems: make(map[velero.ResourceIdentifier]struct{}), - restoreDir: baseDir, - } - - nsClient.On("Get", "ns-2", metav1.GetOptions{}).Return(&v1.Namespace{}, k8serrors.NewNotFound(schema.GroupResource{Resource: "namespaces"}, "ns-2")) - ns := newTestNamespace("ns-2").Namespace - nsClient.On("Create", ns).Return(ns, nil) - - warnings, errors := ctx.restoreFromDir() - - assert.Empty(t, warnings.Velero) - assert.Empty(t, warnings.Cluster) - assert.Empty(t, warnings.Namespaces) - assert.Empty(t, errors.Velero) - assert.Empty(t, errors.Cluster) - assert.Empty(t, errors.Namespaces) - - // ensure the remapped NS (only) was created via the namespaceClient - nsClient.AssertExpectations(t) - - // ensure that we did not try to create namespaces via dynamic client - dynamicFactory.AssertNotCalled(t, "ClientForGroupVersionResource", gv, metav1.APIResource{Name: "namespaces", Namespaced: true}, "") - - dynamicFactory.AssertExpectations(t) - resourceClient.AssertExpectations(t) -} - func TestRestoreResourceForNamespace(t *testing.T) { tests := []struct { name string @@ -327,14 +263,6 @@ func TestRestoreResourceForNamespace(t *testing.T) { }, expectedObjs: toUnstructured(newNamedTestConfigMap("cm-2").ConfigMap), }, - { - name: "namespace is remapped", - namespace: "ns-2", - resourcePath: "configmaps", - labelSelector: labels.NewSelector(), - fileSystem: velerotest.NewFakeFileSystem().WithFile("configmaps/cm-1.json", newTestConfigMap().WithNamespace("ns-1").ToJSON()), - expectedObjs: toUnstructured(newTestConfigMap().WithNamespace("ns-2").ConfigMap), - }, { name: "custom restorer is correctly used", namespace: "ns-1", From 0089fa4d9374bbebb10cac352cd5ec293ee13918 Mon Sep 17 00:00:00 2001 From: Steve Kriss Date: Mon, 24 Jun 2019 16:18:30 -0600 Subject: [PATCH 3/3] migrate restore resource priorities test Signed-off-by: Steve Kriss --- pkg/restore/restore_new_test.go | 148 +++++++++++++++++++++++++++++++- pkg/restore/restore_test.go | 36 -------- 2 files changed, 145 insertions(+), 39 deletions(-) diff --git a/pkg/restore/restore_new_test.go b/pkg/restore/restore_new_test.go index 9cf081cf5..6e172696e 100644 --- a/pkg/restore/restore_new_test.go +++ b/pkg/restore/restore_new_test.go @@ -30,9 +30,11 @@ import ( "github.com/stretchr/testify/assert" "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/runtime" "k8s.io/apimachinery/pkg/util/sets" + kubetesting "k8s.io/client-go/testing" velerov1api "github.com/heptio/velero/pkg/apis/velero/v1" "github.com/heptio/velero/pkg/client" @@ -567,6 +569,146 @@ func TestRestoreNamespaceMapping(t *testing.T) { } } +// TestRestoreResourcePriorities runs restores with resource priorities specified, +// and verifies that the set of items created in the API are created in the expected +// order. Validation is done by adding a Reactor to the fake dynamic client that records +// resource identifiers as they're created, and comparing that to the expected order. +func TestRestoreResourcePriorities(t *testing.T) { + tests := []struct { + name string + restore *velerov1api.Restore + backup *velerov1api.Backup + apiResources []*test.APIResource + tarball io.Reader + resourcePriorities []string + }{ + { + name: "resources are restored according to the specified resource priorities", + restore: defaultRestore().Restore(), + backup: defaultBackup().Backup(), + tarball: newTarWriter(t). + addItems("pods", + test.NewPod("ns-1", "pod-1"), + test.NewPod("ns-2", "pod-2"), + ). + addItems("persistentvolumes", + test.NewPV("pv-1"), + test.NewPV("pv-2"), + ). + addItems("deployments.apps", + test.NewDeployment("ns-1", "deploy-1"), + test.NewDeployment("ns-2", "deploy-2"), + ). + addItems("serviceaccounts", + test.NewServiceAccount("ns-1", "sa-1"), + test.NewServiceAccount("ns-2", "sa-2"), + ). + addItems("persistentvolumeclaims", + test.NewPVC("ns-1", "pvc-1"), + test.NewPVC("ns-2", "pvc-2"), + ). + done(), + apiResources: []*test.APIResource{ + test.Pods(), + test.PVs(), + test.Deployments(), + test.ServiceAccounts(), + }, + resourcePriorities: []string{"persistentvolumes", "serviceaccounts", "pods", "deployments.apps"}, + }, + } + + for _, tc := range tests { + h := newHarness(t) + h.restorer.resourcePriorities = tc.resourcePriorities + + recorder := &createRecorder{t: t} + h.DynamicClient.PrependReactor("create", "*", recorder.reactor()) + + for _, r := range tc.apiResources { + h.DiscoveryClient.WithAPIResource(r) + } + require.NoError(t, h.restorer.discoveryHelper.Refresh()) + + warnings, errs := h.restorer.Restore( + h.log, + tc.restore, + tc.backup, + nil, // volume snapshots + tc.tarball, + nil, // actions + nil, // snapshot location lister + nil, // volume snapshotter getter + ) + + assertEmptyResults(t, warnings, errs) + assertResourceCreationOrder(t, tc.resourcePriorities, recorder.resources) + } +} + +// assertResourceCreationOrder ensures that resources were created in the expected +// order. Any resources *not* in resourcePriorities are required to come *after* all +// resources in any order. +func assertResourceCreationOrder(t *testing.T, resourcePriorities []string, createdResources []resourceID) { + // lastSeen tracks the index in 'resourcePriorities' of the last resource type + // we saw created. Once we've seen a resource in 'resourcePriorities', we should + // never see another instance of a prior resource. + lastSeen := 0 + + // Find the index in 'resourcePriorities' of the resource type for + // the current item, if it exists. This index ('current') *must* + // be greater than or equal to 'lastSeen', which was the last resource + // we saw, since otherwise the current resource would be out of order. By + // initializing current to len(ordered), we're saying that if the resource + // is not explicitly in orderedResources, then it must come *after* + // all orderedResources. + for _, r := range createdResources { + current := len(resourcePriorities) + for i, item := range resourcePriorities { + if item == r.groupResource { + current = i + break + } + } + + // the index of the current resource must be the same as or greater than the index of + // the last resource we saw for the restored order to be correct. + assert.True(t, current >= lastSeen, "%s was restored out of order", r.groupResource) + lastSeen = current + } +} + +type resourceID struct { + groupResource string + nsAndName string +} + +// createRecorder provides a Reactor that can be used to capture +// resources created in a fake client. +type createRecorder struct { + t *testing.T + resources []resourceID +} + +func (cr *createRecorder) reactor() func(kubetesting.Action) (bool, runtime.Object, error) { + return func(action kubetesting.Action) (bool, runtime.Object, error) { + createAction, ok := action.(kubetesting.CreateAction) + if !ok { + return false, nil, nil + } + + accessor, err := meta.Accessor(createAction.GetObject()) + assert.NoError(cr.t, err) + + cr.resources = append(cr.resources, resourceID{ + groupResource: action.GetResource().GroupResource().String(), + nsAndName: fmt.Sprintf("%s/%s", action.GetNamespace(), accessor.GetName()), + }) + + return false, nil, nil + } +} + func defaultRestore() *Builder { return NewNamedBuilder(velerov1api.DefaultNamespace, "restore-1").Backup("backup-1") } @@ -618,16 +760,16 @@ func newTarWriter(t *testing.T) *tarWriter { return tw } -func (tw *tarWriter) addItems(groupVersion string, items ...metav1.Object) *tarWriter { +func (tw *tarWriter) addItems(groupResource string, items ...metav1.Object) *tarWriter { tw.t.Helper() for _, obj := range items { var path string if obj.GetNamespace() == "" { - path = fmt.Sprintf("resources/%s/cluster/%s.json", groupVersion, obj.GetName()) + path = fmt.Sprintf("resources/%s/cluster/%s.json", groupResource, obj.GetName()) } else { - path = fmt.Sprintf("resources/%s/namespaces/%s/%s.json", groupVersion, obj.GetNamespace(), obj.GetName()) + path = fmt.Sprintf("resources/%s/namespaces/%s/%s.json", groupResource, obj.GetNamespace(), obj.GetName()) } tw.add(path, obj) diff --git a/pkg/restore/restore_test.go b/pkg/restore/restore_test.go index f594a1a57..d0dfb25eb 100644 --- a/pkg/restore/restore_test.go +++ b/pkg/restore/restore_test.go @@ -133,42 +133,6 @@ func TestRestorePriority(t *testing.T) { expectedErrors Result expectedReadDirs []string }{ - { - name: "cluster test", - fileSystem: velerotest.NewFakeFileSystem().WithDirectory("bak/resources/a/cluster").WithDirectory("bak/resources/c/cluster"), - baseDir: "bak", - restore: &api.Restore{Spec: api.RestoreSpec{IncludedNamespaces: []string{"*"}}}, - prioritizedResources: []schema.GroupResource{ - {Resource: "a"}, - {Resource: "b"}, - {Resource: "c"}, - }, - expectedReadDirs: []string{"bak/resources", "bak/resources/a/cluster", "bak/resources/c/cluster"}, - }, - { - name: "resource priorities are applied", - fileSystem: velerotest.NewFakeFileSystem().WithDirectory("bak/resources/a/cluster").WithDirectory("bak/resources/c/cluster"), - restore: &api.Restore{Spec: api.RestoreSpec{IncludedNamespaces: []string{"*"}}}, - baseDir: "bak", - prioritizedResources: []schema.GroupResource{ - {Resource: "c"}, - {Resource: "b"}, - {Resource: "a"}, - }, - expectedReadDirs: []string{"bak/resources", "bak/resources/c/cluster", "bak/resources/a/cluster"}, - }, - { - name: "basic namespace", - fileSystem: velerotest.NewFakeFileSystem().WithDirectory("bak/resources/a/namespaces/ns-1").WithDirectory("bak/resources/c/namespaces/ns-1"), - restore: &api.Restore{Spec: api.RestoreSpec{IncludedNamespaces: []string{"*"}}}, - baseDir: "bak", - prioritizedResources: []schema.GroupResource{ - {Resource: "a"}, - {Resource: "b"}, - {Resource: "c"}, - }, - expectedReadDirs: []string{"bak/resources", "bak/resources/a/namespaces", "bak/resources/a/namespaces/ns-1", "bak/resources/c/namespaces", "bak/resources/c/namespaces/ns-1"}, - }, { name: "error in a single resource doesn't terminate restore immediately, but is returned", fileSystem: velerotest.NewFakeFileSystem().