From 59b4ec94ac1a0cae85f855a8e2d9263b3066acc5 Mon Sep 17 00:00:00 2001 From: Steve Kriss Date: Mon, 23 Oct 2017 10:37:30 -0700 Subject: [PATCH] make --include-cluster-resources optional/default true for restores Signed-off-by: Steve Kriss --- docs/cli-reference/ark_create_restore.md | 26 ++--- docs/cli-reference/ark_restore_create.md | 26 ++--- pkg/apis/ark/v1/restore.go | 5 +- pkg/cmd/cli/restore/create.go | 9 +- pkg/restore/restore.go | 2 +- pkg/restore/restore_test.go | 115 +++++++++++++++++++---- 6 files changed, 134 insertions(+), 49 deletions(-) diff --git a/docs/cli-reference/ark_create_restore.md b/docs/cli-reference/ark_create_restore.md index e73ce35d85..071757d20f 100644 --- a/docs/cli-reference/ark_create_restore.md +++ b/docs/cli-reference/ark_create_restore.md @@ -14,19 +14,19 @@ ark create restore BACKUP [flags] ### Options ``` - --exclude-namespaces stringArray namespaces to exclude from the restore - --exclude-resources stringArray resources to exclude from the restore, formatted as resource.group, such as storageclasses.storage.k8s.io - -h, --help help for restore - --include-cluster-resources include cluster-scoped resources in the restore (default true) - --include-namespaces stringArray namespaces to include in the restore (use '*' for all namespaces) (default *) - --include-resources stringArray resources to include in the restore, formatted as resource.group, such as storageclasses.storage.k8s.io (use '*' for all resources) - --label-columns stringArray a comma-separated list of labels to be displayed as columns - --labels mapStringString labels to apply to the restore - --namespace-mappings mapStringString namespace mappings from name in the backup to desired restored name in the form src1:dst1,src2:dst2,... - -o, --output string Output display format. For create commands, display the object but do not send it to the server. Valid formats are 'table', 'json', and 'yaml'. - --restore-volumes optionalBool[=true] whether to restore volumes from snapshots - -l, --selector labelSelector only restore resources matching this label selector (default ) - --show-labels show labels in the last column + --exclude-namespaces stringArray namespaces to exclude from the restore + --exclude-resources stringArray resources to exclude from the restore, formatted as resource.group, such as storageclasses.storage.k8s.io + -h, --help help for restore + --include-cluster-resources optionalBool[=true] include cluster-scoped resources in the restore + --include-namespaces stringArray namespaces to include in the restore (use '*' for all namespaces) (default *) + --include-resources stringArray resources to include in the restore, formatted as resource.group, such as storageclasses.storage.k8s.io (use '*' for all resources) + --label-columns stringArray a comma-separated list of labels to be displayed as columns + --labels mapStringString labels to apply to the restore + --namespace-mappings mapStringString namespace mappings from name in the backup to desired restored name in the form src1:dst1,src2:dst2,... + -o, --output string Output display format. For create commands, display the object but do not send it to the server. Valid formats are 'table', 'json', and 'yaml'. + --restore-volumes optionalBool[=true] whether to restore volumes from snapshots + -l, --selector labelSelector only restore resources matching this label selector (default ) + --show-labels show labels in the last column ``` ### Options inherited from parent commands diff --git a/docs/cli-reference/ark_restore_create.md b/docs/cli-reference/ark_restore_create.md index d3ee4d27ec..ec984dfab9 100644 --- a/docs/cli-reference/ark_restore_create.md +++ b/docs/cli-reference/ark_restore_create.md @@ -14,19 +14,19 @@ ark restore create BACKUP [flags] ### Options ``` - --exclude-namespaces stringArray namespaces to exclude from the restore - --exclude-resources stringArray resources to exclude from the restore, formatted as resource.group, such as storageclasses.storage.k8s.io - -h, --help help for create - --include-cluster-resources include cluster-scoped resources in the restore (default true) - --include-namespaces stringArray namespaces to include in the restore (use '*' for all namespaces) (default *) - --include-resources stringArray resources to include in the restore, formatted as resource.group, such as storageclasses.storage.k8s.io (use '*' for all resources) - --label-columns stringArray a comma-separated list of labels to be displayed as columns - --labels mapStringString labels to apply to the restore - --namespace-mappings mapStringString namespace mappings from name in the backup to desired restored name in the form src1:dst1,src2:dst2,... - -o, --output string Output display format. For create commands, display the object but do not send it to the server. Valid formats are 'table', 'json', and 'yaml'. - --restore-volumes optionalBool[=true] whether to restore volumes from snapshots - -l, --selector labelSelector only restore resources matching this label selector (default ) - --show-labels show labels in the last column + --exclude-namespaces stringArray namespaces to exclude from the restore + --exclude-resources stringArray resources to exclude from the restore, formatted as resource.group, such as storageclasses.storage.k8s.io + -h, --help help for create + --include-cluster-resources optionalBool[=true] include cluster-scoped resources in the restore + --include-namespaces stringArray namespaces to include in the restore (use '*' for all namespaces) (default *) + --include-resources stringArray resources to include in the restore, formatted as resource.group, such as storageclasses.storage.k8s.io (use '*' for all resources) + --label-columns stringArray a comma-separated list of labels to be displayed as columns + --labels mapStringString labels to apply to the restore + --namespace-mappings mapStringString namespace mappings from name in the backup to desired restored name in the form src1:dst1,src2:dst2,... + -o, --output string Output display format. For create commands, display the object but do not send it to the server. Valid formats are 'table', 'json', and 'yaml'. + --restore-volumes optionalBool[=true] whether to restore volumes from snapshots + -l, --selector labelSelector only restore resources matching this label selector (default ) + --show-labels show labels in the last column ``` ### Options inherited from parent commands diff --git a/pkg/apis/ark/v1/restore.go b/pkg/apis/ark/v1/restore.go index b3d6d9df35..196532cc1b 100644 --- a/pkg/apis/ark/v1/restore.go +++ b/pkg/apis/ark/v1/restore.go @@ -56,8 +56,9 @@ type RestoreSpec struct { RestorePVs *bool `json:"restorePVs"` // IncludeClusterResources specifies whether cluster-scoped resources - // should be included for consideration in the restore. - IncludeClusterResources bool `json:"includeClusterResources"` + // should be included for consideration in the restore. If null, defaults + // to true. + IncludeClusterResources *bool `json:"includeClusterResources"` } // RestorePhase is a string representation of the lifecycle phase diff --git a/pkg/cmd/cli/restore/create.go b/pkg/cmd/cli/restore/create.go index 2dcd8dcdae..edddabb8dd 100644 --- a/pkg/cmd/cli/restore/create.go +++ b/pkg/cmd/cli/restore/create.go @@ -63,7 +63,7 @@ type CreateOptions struct { ExcludeResources flag.StringArray NamespaceMappings flag.Map Selector flag.LabelSelector - IncludeClusterResources bool + IncludeClusterResources flag.OptionalBool } func NewCreateOptions() *CreateOptions { @@ -72,7 +72,7 @@ func NewCreateOptions() *CreateOptions { IncludeNamespaces: flag.NewStringArray("*"), NamespaceMappings: flag.NewMap().WithEntryDelimiter(",").WithKeyValueDelimiter(":"), RestoreVolumes: flag.NewOptionalBool(nil), - IncludeClusterResources: true, + IncludeClusterResources: flag.NewOptionalBool(nil), } } @@ -89,7 +89,8 @@ func (o *CreateOptions) BindFlags(flags *pflag.FlagSet) { // like a normal bool flag f.NoOptDefVal = "true" - flags.BoolVar(&o.IncludeClusterResources, "include-cluster-resources", o.IncludeClusterResources, "include cluster-scoped resources in the restore") + f = flags.VarPF(&o.IncludeClusterResources, "include-cluster-resources", "", "include cluster-scoped resources in the restore") + f.NoOptDefVal = "true" } func (o *CreateOptions) Validate(c *cobra.Command, args []string) error { @@ -130,7 +131,7 @@ func (o *CreateOptions) Run(c *cobra.Command, f client.Factory) error { NamespaceMapping: o.NamespaceMappings.Data(), LabelSelector: o.Selector.LabelSelector, RestorePVs: o.RestoreVolumes.Value, - IncludeClusterResources: o.IncludeClusterResources, + IncludeClusterResources: o.IncludeClusterResources.Value, }, } diff --git a/pkg/restore/restore.go b/pkg/restore/restore.go index c644911f75..cad0c2d897 100644 --- a/pkg/restore/restore.go +++ b/pkg/restore/restore.go @@ -397,7 +397,7 @@ func addToResult(r *api.RestoreResult, ns string, e error) { func (ctx *context) restoreResource(resource, namespace, resourcePath string) (api.RestoreResult, api.RestoreResult) { warnings, errs := api.RestoreResult{}, api.RestoreResult{} - if !ctx.restore.Spec.IncludeClusterResources && namespace == "" { + if ctx.restore.Spec.IncludeClusterResources != nil && !*ctx.restore.Spec.IncludeClusterResources && namespace == "" { ctx.infof("Skipping resource %s because it's cluster-scoped", resource) return warnings, errs } diff --git a/pkg/restore/restore_test.go b/pkg/restore/restore_test.go index 859a89bd6c..6596a99e26 100644 --- a/pkg/restore/restore_test.go +++ b/pkg/restore/restore_test.go @@ -129,7 +129,7 @@ func TestRestoreNamespaceFiltering(t *testing.T) { name: "namespacesToRestore having * restores all namespaces", fileSystem: 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{IncludeClusterResources: true, IncludedNamespaces: []string{"*"}}}, + 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{ schema.GroupResource{Resource: "nodes"}, @@ -140,7 +140,7 @@ func TestRestoreNamespaceFiltering(t *testing.T) { name: "namespacesToRestore properly filters", fileSystem: 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{IncludeClusterResources: true, IncludedNamespaces: []string{"b", "c"}}}, + 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{ schema.GroupResource{Resource: "nodes"}, @@ -151,7 +151,7 @@ func TestRestoreNamespaceFiltering(t *testing.T) { name: "namespacesToRestore properly filters with exclusion filter", fileSystem: 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{IncludeClusterResources: true, IncludedNamespaces: []string{"*"}, ExcludedNamespaces: []string{"a"}}}, + 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{ schema.GroupResource{Resource: "nodes"}, @@ -164,9 +164,8 @@ func TestRestoreNamespaceFiltering(t *testing.T) { baseDir: "bak", restore: &api.Restore{ Spec: api.RestoreSpec{ - IncludeClusterResources: true, - IncludedNamespaces: []string{"a", "b", "c"}, - ExcludedNamespaces: []string{"b"}, + 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"}, @@ -216,7 +215,7 @@ func TestRestorePriority(t *testing.T) { name: "cluster test", fileSystem: newFakeFileSystem().WithDirectory("bak/resources/a/cluster").WithDirectory("bak/resources/c/cluster"), baseDir: "bak", - restore: &api.Restore{Spec: api.RestoreSpec{IncludeClusterResources: true, IncludedNamespaces: []string{"*"}}}, + restore: &api.Restore{Spec: api.RestoreSpec{IncludedNamespaces: []string{"*"}}}, prioritizedResources: []schema.GroupResource{ schema.GroupResource{Resource: "a"}, schema.GroupResource{Resource: "b"}, @@ -227,7 +226,7 @@ func TestRestorePriority(t *testing.T) { { name: "resource priorities are applied", fileSystem: newFakeFileSystem().WithDirectory("bak/resources/a/cluster").WithDirectory("bak/resources/c/cluster"), - restore: &api.Restore{Spec: api.RestoreSpec{IncludeClusterResources: true, IncludedNamespaces: []string{"*"}}}, + restore: &api.Restore{Spec: api.RestoreSpec{IncludedNamespaces: []string{"*"}}}, baseDir: "bak", prioritizedResources: []schema.GroupResource{ schema.GroupResource{Resource: "c"}, @@ -239,7 +238,7 @@ func TestRestorePriority(t *testing.T) { { name: "basic namespace", fileSystem: newFakeFileSystem().WithDirectory("bak/resources/a/namespaces/ns-1").WithDirectory("bak/resources/c/namespaces/ns-1"), - restore: &api.Restore{Spec: api.RestoreSpec{IncludeClusterResources: true, IncludedNamespaces: []string{"*"}}}, + restore: &api.Restore{Spec: api.RestoreSpec{IncludedNamespaces: []string{"*"}}}, baseDir: "bak", prioritizedResources: []schema.GroupResource{ schema.GroupResource{Resource: "a"}, @@ -253,7 +252,7 @@ func TestRestorePriority(t *testing.T) { fileSystem: newFakeFileSystem(). WithFile("bak/resources/a/namespaces/ns-1/invalid-json.json", []byte("invalid json")). WithDirectory("bak/resources/c/namespaces/ns-1"), - restore: &api.Restore{Spec: api.RestoreSpec{IncludeClusterResources: true, IncludedNamespaces: []string{"*"}}}, + restore: &api.Restore{Spec: api.RestoreSpec{IncludedNamespaces: []string{"*"}}}, baseDir: "bak", prioritizedResources: []schema.GroupResource{ schema.GroupResource{Resource: "a"}, @@ -294,12 +293,19 @@ func TestRestorePriority(t *testing.T) { } func TestRestoreResourceForNamespace(t *testing.T) { + var ( + trueVal = true + falseVal = false + truePtr = &trueVal + falsePtr = &falseVal + ) + tests := []struct { name string namespace string resourcePath string labelSelector labels.Selector - excludeClusterResources bool + includeClusterResources *bool fileSystem *fakeFileSystem restorers map[schema.GroupResource]restorers.ResourceRestorer expectedErrors api.RestoreResult @@ -406,15 +412,51 @@ func TestRestoreResourceForNamespace(t *testing.T) { namespace: "", resourcePath: "persistentvolumes", labelSelector: labels.NewSelector(), - excludeClusterResources: true, - fileSystem: newFakeFileSystem().WithFile("persistentvolumes/pv-1.json", newTestConfigMap().ToJSON()), + includeClusterResources: falsePtr, + fileSystem: 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(), - excludeClusterResources: true, + includeClusterResources: falsePtr, + fileSystem: newFakeFileSystem().WithFile("configmaps/cm-1.json", newTestConfigMap().ToJSON()), + expectedObjs: toUnstructured(newTestConfigMap().WithArkLabel("my-restore").ConfigMap), + }, + { + name: "cluster-scoped resources are not skipped when IncludeClusterResources=true", + namespace: "", + resourcePath: "persistentvolumes", + labelSelector: labels.NewSelector(), + includeClusterResources: truePtr, + fileSystem: newFakeFileSystem().WithFile("persistentvolumes/pv-1.json", newTestPV().ToJSON()), + expectedObjs: toUnstructured(newTestPV().WithArkLabel("my-restore").PersistentVolume), + }, + { + name: "namespaced resources are not skipped when IncludeClusterResources=true", + namespace: "ns-1", + resourcePath: "configmaps", + labelSelector: labels.NewSelector(), + includeClusterResources: truePtr, + fileSystem: newFakeFileSystem().WithFile("configmaps/cm-1.json", newTestConfigMap().ToJSON()), + expectedObjs: toUnstructured(newTestConfigMap().WithArkLabel("my-restore").ConfigMap), + }, + { + name: "cluster-scoped resources are not skipped when IncludeClusterResources=nil", + namespace: "", + resourcePath: "persistentvolumes", + labelSelector: labels.NewSelector(), + includeClusterResources: nil, + fileSystem: newFakeFileSystem().WithFile("persistentvolumes/pv-1.json", newTestPV().ToJSON()), + expectedObjs: toUnstructured(newTestPV().WithArkLabel("my-restore").PersistentVolume), + }, + { + name: "namespaced resources are not skipped when IncludeClusterResources=nil", + namespace: "ns-1", + resourcePath: "configmaps", + labelSelector: labels.NewSelector(), + includeClusterResources: nil, fileSystem: newFakeFileSystem().WithFile("configmaps/cm-1.json", newTestConfigMap().ToJSON()), expectedObjs: toUnstructured(newTestConfigMap().WithArkLabel("my-restore").ConfigMap), }, @@ -432,6 +474,9 @@ func TestRestoreResourceForNamespace(t *testing.T) { gv := schema.GroupVersion{Group: "", Version: "v1"} dynamicFactory.On("ClientForGroupVersionResource", gv, resource, test.namespace).Return(resourceClient, nil) + pvResource := metav1.APIResource{Name: "persistentvolumes", Namespaced: false} + dynamicFactory.On("ClientForGroupVersionResource", gv, pvResource, test.namespace).Return(resourceClient, nil) + log, _ := testlogger.NewNullLogger() ctx := &context{ @@ -445,14 +490,14 @@ func TestRestoreResourceForNamespace(t *testing.T) { Name: "my-restore", }, Spec: api.RestoreSpec{ - IncludeClusterResources: !test.excludeClusterResources, + IncludeClusterResources: test.includeClusterResources, }, }, backup: &api.Backup{}, logger: log, } - warnings, errors := ctx.restoreResource("configmaps", test.namespace, test.resourcePath) + warnings, errors := ctx.restoreResource(test.resourcePath, test.namespace, test.resourcePath) assert.Empty(t, warnings.Ark) assert.Empty(t, warnings.Cluster) @@ -544,12 +589,50 @@ func toUnstructured(objs ...runtime.Object) []unstructured.Unstructured { delete(metadata, "creationTimestamp") + if _, exists := metadata["namespace"]; !exists { + metadata["namespace"] = "" + } + + delete(unstructuredObj.Object, "status") + res = append(res, unstructuredObj) } return res } +type testPersistentVolume struct { + *v1.PersistentVolume +} + +func newTestPV() *testPersistentVolume { + return &testPersistentVolume{ + PersistentVolume: &v1.PersistentVolume{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "PersistentVolume", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pv", + }, + Status: v1.PersistentVolumeStatus{}, + }, + } +} + +func (pv *testPersistentVolume) WithArkLabel(restoreName string) *testPersistentVolume { + if pv.Labels == nil { + pv.Labels = make(map[string]string) + } + pv.Labels[api.RestoreLabelKey] = restoreName + return pv +} + +func (pv *testPersistentVolume) ToJSON() []byte { + bytes, _ := json.Marshal(pv.PersistentVolume) + return bytes +} + type testConfigMap struct { *v1.ConfigMap }