From b25e12419c73b00d29e5771e736b9fd01c0d21e8 Mon Sep 17 00:00:00 2001 From: Mathieu Ouellet Date: Thu, 8 Feb 2024 15:48:48 -0500 Subject: [PATCH] Add support for namespaces RestoreItemAction plugin Signed-off-by: Mathieu Ouellet --- changelogs/unreleased/7409-mouellet | 1 + pkg/restore/restore.go | 167 ++++++++------------- pkg/restore/restore_test.go | 222 +++++++++++++++++++++++++--- pkg/util/kube/utils.go | 64 -------- pkg/util/kube/utils_test.go | 99 ------------- 5 files changed, 264 insertions(+), 289 deletions(-) create mode 100644 changelogs/unreleased/7409-mouellet diff --git a/changelogs/unreleased/7409-mouellet b/changelogs/unreleased/7409-mouellet new file mode 100644 index 0000000000..796ea33139 --- /dev/null +++ b/changelogs/unreleased/7409-mouellet @@ -0,0 +1 @@ +Add support for namespaces RestoreItemAction plugin \ No newline at end of file diff --git a/pkg/restore/restore.go b/pkg/restore/restore.go index 3c30e4cd2f..a8cf5c53c4 100644 --- a/pkg/restore/restore.go +++ b/pkg/restore/restore.go @@ -18,7 +18,6 @@ package restore import ( go_context "context" - "encoding/json" "fmt" "io" "os" @@ -718,55 +717,22 @@ func (ctx *restoreContext) processSelectedResource( for namespace, selectedItems := range selectedResource.selectedItemsByNamespace { for _, selectedItem := range selectedItems { - targetNS := selectedItem.targetNamespace - if groupResource == kuberesource.Namespaces { - // namespace is a cluster-scoped resource and doesn't have "targetNamespace" attribute in the restoreableItem instance - namespace = selectedItem.name - if n, ok := ctx.restore.Spec.NamespaceMapping[namespace]; ok { - targetNS = n - } else { - targetNS = namespace - } - } - // If we don't know whether this namespace exists yet, attempt to create - // it in order to ensure it exists. Try to get it from the backup tarball - // (in order to get any backed-up metadata), but if we don't find it there, - // create a blank one. - if namespace != "" && !existingNamespaces.Has(targetNS) { - logger := ctx.log.WithField("namespace", namespace) - - ns := getNamespace( - logger, + // For namespaced resource where the namespace hasn't been restored yet, attempt + // to create it in order to ensure it does before restoring the resource. Try to + // get it from the backup tarball (in order to get any backed-up metadata), remap + // it if needed, but if we don't find it there, create a blank one. + if namespace != "" && !existingNamespaces.Has(selectedItem.targetNamespace) { + ns := ctx.getNamespace( archive.GetItemFilePath(ctx.restoreDir, "namespaces", "", namespace), - targetNS, - ) - _, nsCreated, err := kube.EnsureNamespaceExistsAndIsReady( - ns, - ctx.namespaceClient, - ctx.resourceTerminatingTimeout, - ) - if err != nil { - errs.AddVeleroError(err) - continue - } - - // Add the newly created namespace to the list of restored items. - if nsCreated { - itemKey := itemKey{ - resource: resourceKey(ns), - namespace: ns.Namespace, - name: ns.Name, - } - ctx.restoredItems[itemKey] = restoredItemStatus{action: itemRestoreResultCreated, itemExists: true} - } + selectedItem.targetNamespace) + w, e, _ := ctx.restoreItem(ns, kuberesource.Namespaces, "") + warnings.Merge(&w) + errs.Merge(&e) + processedItems++ // Keep track of namespaces that we know exist so we don't // have to try to create them multiple times. - existingNamespaces.Insert(targetNS) - } - // For namespaces resources we don't need to following steps - if groupResource == kuberesource.Namespaces { - continue + existingNamespaces.Insert(ns.GetName()) } obj, err := archive.Unmarshal(ctx.fileSystem, selectedItem.path) @@ -782,11 +748,23 @@ func (ctx *restoreContext) processSelectedResource( continue } - w, e, _ := ctx.restoreItem(obj, groupResource, targetNS) + // For namespaces resources, override the name if a mapping exits + if groupResource == kuberesource.Namespaces { + if n, ok := ctx.restore.Spec.NamespaceMapping[selectedItem.name]; ok { + obj.SetName(n) + } + } + + w, e, _ := ctx.restoreItem(obj, groupResource, selectedItem.targetNamespace) warnings.Merge(&w) errs.Merge(&e) processedItems++ + // For namespaces resources, keep track of those we know exits + if groupResource == kuberesource.Namespaces { + existingNamespaces.Insert(obj.GetName()) + } + // totalItems keeps the count of items previously known. There // may be additional items restored by plugins. We want to include // the additional items by looking at restoredItems at the same @@ -811,58 +789,38 @@ func (ctx *restoreContext) processSelectedResource( return processedItems, warnings, errs } -// getNamespace returns a namespace API object that we should attempt to +// getNamespace returns a namespace Unstructured object that we should attempt to // create before restoring anything into it. It will come from the backup // tarball if it exists, else will be a new one. If from the tarball, it // will retain its labels, annotations, and spec. -func getNamespace(logger logrus.FieldLogger, path, remappedName string) *v1.Namespace { - var nsBytes []byte - var err error - - if nsBytes, err = os.ReadFile(path); err != nil { - return &v1.Namespace{ - TypeMeta: metav1.TypeMeta{ - Kind: "Namespace", - APIVersion: "v1", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: remappedName, - }, - } - } - - var backupNS v1.Namespace - if err := json.Unmarshal(nsBytes, &backupNS); err != nil { - logger.Warnf("Error unmarshaling namespace from backup, creating new one.") - return &v1.Namespace{ +func (ctx *restoreContext) getNamespace(path, targetNamespace string) *unstructured.Unstructured { + obj, err := archive.Unmarshal(ctx.fileSystem, path) + if err != nil { + ctx.log.WithField("namespace", targetNamespace).Warnf("Error unmarshaling namespace from backup, creating new one.") + ns, _ := runtime.DefaultUnstructuredConverter.ToUnstructured(&v1.Namespace{ TypeMeta: metav1.TypeMeta{ Kind: "Namespace", - APIVersion: "v1", + APIVersion: v1.SchemeGroupVersion.String(), }, ObjectMeta: metav1.ObjectMeta{ - Name: remappedName, + Name: targetNamespace, }, - } - } - - return &v1.Namespace{ - TypeMeta: metav1.TypeMeta{ - Kind: backupNS.Kind, - APIVersion: backupNS.APIVersion, - }, - ObjectMeta: metav1.ObjectMeta{ - Name: remappedName, - Labels: backupNS.Labels, - Annotations: backupNS.Annotations, - }, - Spec: backupNS.Spec, + }) + return &unstructured.Unstructured{Object: ns} } + obj.SetName(targetNamespace) + return obj } -func (ctx *restoreContext) getApplicableActions(groupResource schema.GroupResource, namespace string) []framework.RestoreItemResolvedActionV2 { +func (ctx *restoreContext) getApplicableActions(groupResource schema.GroupResource, namespace string, metadata metav1.Object) []framework.RestoreItemResolvedActionV2 { var actions []framework.RestoreItemResolvedActionV2 for _, action := range ctx.restoreItemActions { - if action.ShouldUse(groupResource, namespace, nil, ctx.log) { + logger := ctx.log.WithFields(logrus.Fields{ + "action": action.Name(), + "namespace": namespace, + "groupResource": groupResource.String(), + }) + if action.ShouldUse(groupResource, namespace, metadata, logger) { actions = append(actions, action) } } @@ -1123,7 +1081,7 @@ func (ctx *restoreContext) restoreItem(obj *unstructured.Unstructured, groupReso // Check if group/resource should be restored. We need to do this here since // this method may be getting called for an additional item which is a group/resource // that's excluded. - if !ctx.resourceIncludesExcludes.ShouldInclude(groupResource.String()) { + if groupResource != kuberesource.Namespaces && !ctx.resourceIncludesExcludes.ShouldInclude(groupResource.String()) { restoreLogger.Info("Not restoring item because resource is excluded") return warnings, errs, itemExists } @@ -1134,29 +1092,22 @@ func (ctx *restoreContext) restoreItem(obj *unstructured.Unstructured, groupReso // and should be excluded. Note that we're checking the object's namespace ( // via obj.GetNamespace()) instead of the namespace parameter, because we want // to check the *original* namespace, not the remapped one if it's been remapped. - if namespace != "" { - if !ctx.namespaceIncludesExcludes.ShouldInclude(obj.GetNamespace()) && !ctx.resourceMustHave.Has(groupResource.String()) { - restoreLogger.Info("Not restoring item because namespace is excluded") - return warnings, errs, itemExists + if groupResource == kuberesource.Namespaces { + targetNamespace := obj.GetName() + for i, n := range ctx.restore.Spec.NamespaceMapping { + if n == targetNamespace { + targetNamespace = i + break + } } - - // If the namespace scoped resource should be restored, ensure that the - // namespace into which the resource is being restored into exists. - // This is the *remapped* namespace that we are ensuring exists. - nsToEnsure := getNamespace(ctx.log, archive.GetItemFilePath(ctx.restoreDir, "namespaces", "", obj.GetNamespace()), namespace) - _, nsCreated, err := kube.EnsureNamespaceExistsAndIsReady(nsToEnsure, ctx.namespaceClient, ctx.resourceTerminatingTimeout) - if err != nil { - errs.AddVeleroError(err) + if !ctx.namespaceIncludesExcludes.ShouldInclude(targetNamespace) && !ctx.resourceMustHave.Has(groupResource.String()) { + restoreLogger.Info("Not restoring namespace because it's excluded") return warnings, errs, itemExists } - // Add the newly created namespace to the list of restored items. - if nsCreated { - itemKey := itemKey{ - resource: resourceKey(nsToEnsure), - namespace: nsToEnsure.Namespace, - name: nsToEnsure.Name, - } - ctx.restoredItems[itemKey] = restoredItemStatus{action: itemRestoreResultCreated, itemExists: true} + } else if namespace != "" { + if !ctx.namespaceIncludesExcludes.ShouldInclude(obj.GetNamespace()) && !ctx.resourceMustHave.Has(groupResource.String()) { + restoreLogger.Info("Not restoring item because namespace is excluded") + return warnings, errs, itemExists } } else { if boolptr.IsSetToFalse(ctx.restore.Spec.IncludeClusterResources) { @@ -1341,7 +1292,7 @@ func (ctx *restoreContext) restoreItem(obj *unstructured.Unstructured, groupReso ctx.log.Infof("restore status includes excludes: %+v", ctx.resourceStatusIncludesExcludes) - for _, action := range ctx.getApplicableActions(groupResource, namespace) { + for _, action := range ctx.getApplicableActions(groupResource, namespace, obj) { if !action.Selector.Matches(labels.Set(obj.GetLabels())) { continue } diff --git a/pkg/restore/restore_test.go b/pkg/restore/restore_test.go index efca338703..18ecda1945 100644 --- a/pkg/restore/restore_test.go +++ b/pkg/restore/restore_test.go @@ -276,6 +276,10 @@ func TestRestoreResourceFiltering(t *testing.T) { restore: defaultRestore().Result(), backup: defaultBackup().Result(), tarball: test.NewTarWriter(t). + AddItems("namespaces", + builder.ForNamespace("ns-1").Result(), + builder.ForNamespace("ns-2").Result(), + ). AddItems("pods", builder.ForPod("ns-1", "pod-1").Result(), builder.ForPod("ns-2", "pod-2").Result(), @@ -286,12 +290,14 @@ func TestRestoreResourceFiltering(t *testing.T) { ). Done(), apiResources: []*test.APIResource{ + test.Namespaces(), test.Pods(), test.PVs(), }, want: map[*test.APIResource][]string{ - test.Pods(): {"ns-1/pod-1", "ns-2/pod-2"}, - test.PVs(): {"/pv-1", "/pv-2"}, + test.Namespaces(): {"/ns-1", "/ns-2"}, + test.Pods(): {"ns-1/pod-1", "ns-2/pod-2"}, + test.PVs(): {"/pv-1", "/pv-2"}, }, }, { @@ -299,6 +305,10 @@ func TestRestoreResourceFiltering(t *testing.T) { restore: defaultRestore().IncludedResources("pods").Result(), backup: defaultBackup().Result(), tarball: test.NewTarWriter(t). + AddItems("namespaces", + builder.ForNamespace("ns-1").Result(), + builder.ForNamespace("ns-2").Result(), + ). AddItems("pods", builder.ForPod("ns-1", "pod-1").Result(), builder.ForPod("ns-2", "pod-2").Result(), @@ -309,18 +319,25 @@ func TestRestoreResourceFiltering(t *testing.T) { ). Done(), apiResources: []*test.APIResource{ + test.Namespaces(), test.Pods(), test.PVs(), }, want: map[*test.APIResource][]string{ - test.Pods(): {"ns-1/pod-1", "ns-2/pod-2"}, + test.Namespaces(): {"/ns-1", "/ns-2"}, + test.Pods(): {"ns-1/pod-1", "ns-2/pod-2"}, + test.PVs(): {}, }, }, { name: "excluded resources filter only restores resources not of those types", - restore: defaultRestore().ExcludedResources("pvs").Result(), + restore: defaultRestore().ExcludedResources("persistentvolumes").Result(), backup: defaultBackup().Result(), tarball: test.NewTarWriter(t). + AddItems("namespaces", + builder.ForNamespace("ns-1").Result(), + builder.ForNamespace("ns-2").Result(), + ). AddItems("pods", builder.ForPod("ns-1", "pod-1").Result(), builder.ForPod("ns-2", "pod-2").Result(), @@ -331,11 +348,14 @@ func TestRestoreResourceFiltering(t *testing.T) { ). Done(), apiResources: []*test.APIResource{ + test.Namespaces(), test.Pods(), test.PVs(), }, want: map[*test.APIResource][]string{ - test.Pods(): {"ns-1/pod-1", "ns-2/pod-2"}, + test.Namespaces(): {"/ns-1", "/ns-2"}, + test.Pods(): {"ns-1/pod-1", "ns-2/pod-2"}, + test.PVs(): {}, }, }, { @@ -343,6 +363,10 @@ func TestRestoreResourceFiltering(t *testing.T) { restore: defaultRestore().IncludedNamespaces("ns-1").Result(), backup: defaultBackup().Result(), tarball: test.NewTarWriter(t). + AddItems("namespaces", + builder.ForNamespace("ns-1").Result(), + builder.ForNamespace("ns-2").Result(), + ). AddItems("pods", builder.ForPod("ns-1", "pod-1").Result(), builder.ForPod("ns-2", "pod-2").Result(), @@ -357,13 +381,16 @@ func TestRestoreResourceFiltering(t *testing.T) { ). Done(), apiResources: []*test.APIResource{ + test.Namespaces(), test.Pods(), test.Deployments(), test.PVs(), }, want: map[*test.APIResource][]string{ + test.Namespaces(): {"/ns-1"}, test.Pods(): {"ns-1/pod-1"}, test.Deployments(): {"ns-1/deploy-1"}, + test.PVs(): {}, }, }, { @@ -371,6 +398,10 @@ func TestRestoreResourceFiltering(t *testing.T) { restore: defaultRestore().ExcludedNamespaces("ns-2").Result(), backup: defaultBackup().Result(), tarball: test.NewTarWriter(t). + AddItems("namespaces", + builder.ForNamespace("ns-1").Result(), + builder.ForNamespace("ns-2").Result(), + ). AddItems("pods", builder.ForPod("ns-1", "pod-1").Result(), builder.ForPod("ns-2", "pod-2").Result(), @@ -385,13 +416,16 @@ func TestRestoreResourceFiltering(t *testing.T) { ). Done(), apiResources: []*test.APIResource{ + test.Namespaces(), test.Pods(), test.Deployments(), test.PVs(), }, want: map[*test.APIResource][]string{ + test.Namespaces(): {"/ns-1"}, test.Pods(): {"ns-1/pod-1"}, test.Deployments(): {"ns-1/deploy-1"}, + test.PVs(): {}, }, }, { @@ -399,6 +433,10 @@ func TestRestoreResourceFiltering(t *testing.T) { restore: defaultRestore().IncludeClusterResources(false).Result(), backup: defaultBackup().Result(), tarball: test.NewTarWriter(t). + AddItems("namespaces", + builder.ForNamespace("ns-1").Result(), + builder.ForNamespace("ns-2").Result(), + ). AddItems("pods", builder.ForPod("ns-1", "pod-1").Result(), builder.ForPod("ns-2", "pod-2").Result(), @@ -413,13 +451,16 @@ func TestRestoreResourceFiltering(t *testing.T) { ). Done(), apiResources: []*test.APIResource{ + test.Namespaces(), test.Pods(), test.Deployments(), test.PVs(), }, want: map[*test.APIResource][]string{ + test.Namespaces(): {"/ns-1", "/ns-2"}, test.Pods(): {"ns-1/pod-1", "ns-2/pod-2"}, test.Deployments(): {"ns-1/deploy-1", "ns-2/deploy-2"}, + test.PVs(): {}, }, }, { @@ -427,6 +468,10 @@ func TestRestoreResourceFiltering(t *testing.T) { restore: defaultRestore().LabelSelector(&metav1.LabelSelector{MatchLabels: map[string]string{"a": "b"}}).Result(), backup: defaultBackup().Result(), tarball: test.NewTarWriter(t). + AddItems("namespaces", + builder.ForNamespace("ns-1").Result(), + builder.ForNamespace("ns-2").Result(), + ). AddItems("pods", builder.ForPod("ns-1", "pod-1").ObjectMeta(builder.WithLabels("a", "b")).Result(), builder.ForPod("ns-2", "pod-2").Result(), @@ -441,11 +486,13 @@ func TestRestoreResourceFiltering(t *testing.T) { ). Done(), apiResources: []*test.APIResource{ + test.Namespaces(), test.Pods(), test.Deployments(), test.PVs(), }, want: map[*test.APIResource][]string{ + test.Namespaces(): {"/ns-1", "/ns-2"}, test.Pods(): {"ns-1/pod-1"}, test.Deployments(): {"ns-2/deploy-2"}, test.PVs(): {"/pv-1"}, @@ -457,6 +504,10 @@ func TestRestoreResourceFiltering(t *testing.T) { {MatchLabels: map[string]string{"a3": "b3"}}, {MatchLabels: map[string]string{"a4": "b4"}}}).Result(), backup: defaultBackup().Result(), tarball: test.NewTarWriter(t). + AddItems("namespaces", + builder.ForNamespace("ns-1").Result(), + builder.ForNamespace("ns-2").Result(), + ). AddItems("pods", builder.ForPod("ns-1", "pod-1").ObjectMeta(builder.WithLabels("a1", "b1")).Result(), builder.ForPod("ns-2", "pod-2").Result(), @@ -471,11 +522,13 @@ func TestRestoreResourceFiltering(t *testing.T) { ). Done(), apiResources: []*test.APIResource{ + test.Namespaces(), test.Pods(), test.Deployments(), test.PVs(), }, want: map[*test.APIResource][]string{ + test.Namespaces(): {"/ns-1", "/ns-2"}, test.Pods(): {"ns-1/pod-1"}, test.Deployments(): {"ns-2/deploy-2"}, test.PVs(): {"/pv-2"}, @@ -486,6 +539,10 @@ func TestRestoreResourceFiltering(t *testing.T) { restore: defaultRestore().IncludedNamespaces("ns-1").IncludeClusterResources(true).Result(), backup: defaultBackup().Result(), tarball: test.NewTarWriter(t). + AddItems("namespaces", + builder.ForNamespace("ns-1").Result(), + builder.ForNamespace("ns-2").Result(), + ). AddItems("pods", builder.ForPod("ns-1", "pod-1").Result(), builder.ForPod("ns-2", "pod-2").Result(), @@ -500,11 +557,13 @@ func TestRestoreResourceFiltering(t *testing.T) { ). Done(), apiResources: []*test.APIResource{ + test.Namespaces(), test.Pods(), test.Deployments(), test.PVs(), }, want: map[*test.APIResource][]string{ + test.Namespaces(): {"/ns-1"}, test.Pods(): {"ns-1/pod-1"}, test.Deployments(): {"ns-1/deploy-1"}, test.PVs(): {"/pv-1", "/pv-2"}, @@ -515,6 +574,10 @@ func TestRestoreResourceFiltering(t *testing.T) { restore: defaultRestore().IncludedNamespaces("ns-1").IncludeClusterResources(false).Result(), backup: defaultBackup().Result(), tarball: test.NewTarWriter(t). + AddItems("namespaces", + builder.ForNamespace("ns-1").Result(), + builder.ForNamespace("ns-2").Result(), + ). AddItems("pods", builder.ForPod("ns-1", "pod-1").Result(), builder.ForPod("ns-2", "pod-2").Result(), @@ -529,11 +592,13 @@ func TestRestoreResourceFiltering(t *testing.T) { ). Done(), apiResources: []*test.APIResource{ + test.Namespaces(), test.Pods(), test.Deployments(), test.PVs(), }, want: map[*test.APIResource][]string{ + test.Namespaces(): {"/ns-1"}, test.Pods(): {"ns-1/pod-1"}, test.Deployments(): {"ns-1/deploy-1"}, test.PVs(): {}, @@ -544,6 +609,10 @@ func TestRestoreResourceFiltering(t *testing.T) { restore: defaultRestore().IncludedNamespaces("ns-1").Result(), backup: defaultBackup().Result(), tarball: test.NewTarWriter(t). + AddItems("namespaces", + builder.ForNamespace("ns-1").Result(), + builder.ForNamespace("ns-2").Result(), + ). AddItems("pods", builder.ForPod("ns-1", "pod-1").Result(), builder.ForPod("ns-2", "pod-2").Result(), @@ -558,11 +627,13 @@ func TestRestoreResourceFiltering(t *testing.T) { ). Done(), apiResources: []*test.APIResource{ + test.Namespaces(), test.Pods(), test.Deployments(), test.PVs(), }, want: map[*test.APIResource][]string{ + test.Namespaces(): {"/ns-1"}, test.Pods(): {"ns-1/pod-1"}, test.Deployments(): {"ns-1/deploy-1"}, test.PVs(): {}, @@ -573,6 +644,10 @@ func TestRestoreResourceFiltering(t *testing.T) { restore: defaultRestore().IncludeClusterResources(true).Result(), backup: defaultBackup().Result(), tarball: test.NewTarWriter(t). + AddItems("namespaces", + builder.ForNamespace("ns-1").Result(), + builder.ForNamespace("ns-2").Result(), + ). AddItems("pods", builder.ForPod("ns-1", "pod-1").Result(), builder.ForPod("ns-2", "pod-2").Result(), @@ -587,11 +662,13 @@ func TestRestoreResourceFiltering(t *testing.T) { ). Done(), apiResources: []*test.APIResource{ + test.Namespaces(), test.Pods(), test.Deployments(), test.PVs(), }, want: map[*test.APIResource][]string{ + test.Namespaces(): {"/ns-1", "/ns-2"}, 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"}, @@ -602,6 +679,10 @@ func TestRestoreResourceFiltering(t *testing.T) { restore: defaultRestore().IncludeClusterResources(false).Result(), backup: defaultBackup().Result(), tarball: test.NewTarWriter(t). + AddItems("namespaces", + builder.ForNamespace("ns-1").Result(), + builder.ForNamespace("ns-2").Result(), + ). AddItems("pods", builder.ForPod("ns-1", "pod-1").Result(), builder.ForPod("ns-2", "pod-2").Result(), @@ -616,13 +697,16 @@ func TestRestoreResourceFiltering(t *testing.T) { ). Done(), apiResources: []*test.APIResource{ + test.Namespaces(), test.Pods(), test.Deployments(), test.PVs(), }, want: map[*test.APIResource][]string{ + test.Namespaces(): {"/ns-1", "/ns-2"}, test.Pods(): {"ns-1/pod-1", "ns-2/pod-2"}, test.Deployments(): {"ns-1/deploy-1", "ns-2/deploy-2"}, + test.PVs(): {}, }, }, { @@ -630,6 +714,10 @@ func TestRestoreResourceFiltering(t *testing.T) { restore: defaultRestore().IncludedResources("*", "pods").Result(), backup: defaultBackup().Result(), tarball: test.NewTarWriter(t). + AddItems("namespaces", + builder.ForNamespace("ns-1").Result(), + builder.ForNamespace("ns-2").Result(), + ). AddItems("pods", builder.ForPod("ns-1", "pod-1").Result(), builder.ForPod("ns-2", "pod-2").Result(), @@ -644,11 +732,13 @@ func TestRestoreResourceFiltering(t *testing.T) { ). Done(), apiResources: []*test.APIResource{ + test.Namespaces(), test.Pods(), test.Deployments(), test.PVs(), }, want: map[*test.APIResource][]string{ + test.Namespaces(): {"/ns-1", "/ns-2"}, 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"}, @@ -659,6 +749,10 @@ func TestRestoreResourceFiltering(t *testing.T) { restore: defaultRestore().ExcludedResources("*").Result(), backup: defaultBackup().Result(), tarball: test.NewTarWriter(t). + AddItems("namespaces", + builder.ForNamespace("ns-1").Result(), + builder.ForNamespace("ns-2").Result(), + ). AddItems("pods", builder.ForPod("ns-1", "pod-1").Result(), builder.ForPod("ns-2", "pod-2").Result(), @@ -673,11 +767,13 @@ func TestRestoreResourceFiltering(t *testing.T) { ). Done(), apiResources: []*test.APIResource{ + test.Namespaces(), test.Pods(), test.Deployments(), test.PVs(), }, want: map[*test.APIResource][]string{ + test.Namespaces(): {"/ns-1", "/ns-2"}, 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"}, @@ -688,6 +784,10 @@ func TestRestoreResourceFiltering(t *testing.T) { restore: defaultRestore().IncludedResources("pods", "unresolvable").Result(), backup: defaultBackup().Result(), tarball: test.NewTarWriter(t). + AddItems("namespaces", + builder.ForNamespace("ns-1").Result(), + builder.ForNamespace("ns-2").Result(), + ). AddItems("pods", builder.ForPod("ns-1", "pod-1").Result(), builder.ForPod("ns-2", "pod-2").Result(), @@ -702,12 +802,16 @@ func TestRestoreResourceFiltering(t *testing.T) { ). Done(), apiResources: []*test.APIResource{ + test.Namespaces(), test.Pods(), test.Deployments(), test.PVs(), }, want: map[*test.APIResource][]string{ - test.Pods(): {"ns-1/pod-1", "ns-2/pod-2"}, + test.Namespaces(): {"/ns-1", "/ns-2"}, + test.Pods(): {"ns-1/pod-1", "ns-2/pod-2"}, + test.Deployments(): {}, + test.PVs(): {}, }, }, { @@ -715,6 +819,10 @@ func TestRestoreResourceFiltering(t *testing.T) { restore: defaultRestore().ExcludedResources("deployments", "unresolvable").Result(), backup: defaultBackup().Result(), tarball: test.NewTarWriter(t). + AddItems("namespaces", + builder.ForNamespace("ns-1").Result(), + builder.ForNamespace("ns-2").Result(), + ). AddItems("pods", builder.ForPod("ns-1", "pod-1").Result(), builder.ForPod("ns-2", "pod-2").Result(), @@ -729,13 +837,16 @@ func TestRestoreResourceFiltering(t *testing.T) { ). Done(), apiResources: []*test.APIResource{ + test.Namespaces(), 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"}, + test.Namespaces(): {"/ns-1", "/ns-2"}, + test.Pods(): {"ns-1/pod-1", "ns-2/pod-2"}, + test.Deployments(): {}, + test.PVs(): {"/pv-1", "/pv-2"}, }, }, { @@ -803,9 +914,15 @@ func TestRestoreNamespaceMapping(t *testing.T) { restore: defaultRestore().NamespaceMappings("ns-1", "mapped-ns-1", "ns-2", "mapped-ns-2").Result(), backup: defaultBackup().Result(), apiResources: []*test.APIResource{ + test.Namespaces(), test.Pods(), }, tarball: test.NewTarWriter(t). + AddItems("namespaces", + builder.ForNamespace("ns-1").Result(), + builder.ForNamespace("ns-2").Result(), + builder.ForNamespace("ns-3").Result(), + ). AddItems("pods", builder.ForPod("ns-1", "pod-1").Result(), builder.ForPod("ns-2", "pod-2").Result(), @@ -813,7 +930,8 @@ func TestRestoreNamespaceMapping(t *testing.T) { ). Done(), want: map[*test.APIResource][]string{ - test.Pods(): {"mapped-ns-1/pod-1", "mapped-ns-2/pod-2", "ns-3/pod-3"}, + test.Namespaces(): {"/mapped-ns-1", "/mapped-ns-2", "/ns-3"}, + test.Pods(): {"mapped-ns-1/pod-1", "mapped-ns-2/pod-2", "ns-3/pod-3"}, }, }, { @@ -821,9 +939,15 @@ func TestRestoreNamespaceMapping(t *testing.T) { restore: defaultRestore().IncludedNamespaces("ns-1", "ns-2").NamespaceMappings("ns-1", "mapped-ns-1", "ns-2", "mapped-ns-2").Result(), backup: defaultBackup().Result(), apiResources: []*test.APIResource{ + test.Namespaces(), test.Pods(), }, tarball: test.NewTarWriter(t). + AddItems("namespaces", + builder.ForNamespace("ns-1").Result(), + builder.ForNamespace("ns-2").Result(), + builder.ForNamespace("ns-3").Result(), + ). AddItems("pods", builder.ForPod("ns-1", "pod-1").Result(), builder.ForPod("ns-2", "pod-2").Result(), @@ -831,7 +955,31 @@ func TestRestoreNamespaceMapping(t *testing.T) { ). Done(), want: map[*test.APIResource][]string{ - test.Pods(): {"mapped-ns-1/pod-1", "mapped-ns-2/pod-2"}, + test.Namespaces(): {"/mapped-ns-1", "/mapped-ns-2"}, + test.Pods(): {"mapped-ns-1/pod-1", "mapped-ns-2/pod-2"}, + }, + }, + { + name: "namespace mappings are applied when namespace resource is not in backup tarball", + restore: defaultRestore().NamespaceMappings("ns-1", "mapped-ns-1", "ns-2", "mapped-ns-2").Result(), + backup: defaultBackup().Result(), + apiResources: []*test.APIResource{ + test.Namespaces(), + test.Pods(), + }, + tarball: test.NewTarWriter(t). + AddItems("namespaces", + builder.ForNamespace("ns-3").Result(), + ). + AddItems("pods", + builder.ForPod("ns-1", "pod-1").Result(), + builder.ForPod("ns-2", "pod-2").Result(), + builder.ForPod("ns-3", "pod-3").Result(), + ). + Done(), + want: map[*test.APIResource][]string{ + test.Namespaces(): {"/mapped-ns-1", "/mapped-ns-2", "/ns-3"}, + test.Pods(): {"mapped-ns-1/pod-1", "mapped-ns-2/pod-2", "ns-3/pod-3"}, }, }, } @@ -883,6 +1031,10 @@ func TestRestoreResourcePriorities(t *testing.T) { restore: defaultRestore().Result(), backup: defaultBackup().Result(), tarball: test.NewTarWriter(t). + AddItems("namespaces", + builder.ForNamespace("ns-1").Result(), + builder.ForNamespace("ns-2").Result(), + ). AddItems("pods", builder.ForPod("ns-1", "pod-1").Result(), builder.ForPod("ns-2", "pod-2").Result(), @@ -905,13 +1057,14 @@ func TestRestoreResourcePriorities(t *testing.T) { ). Done(), apiResources: []*test.APIResource{ + test.Namespaces(), test.Pods(), test.PVs(), test.Deployments(), test.ServiceAccounts(), }, resourcePriorities: Priorities{ - HighPriorities: []string{"persistentvolumes", "persistentvolumeclaims", "serviceaccounts"}, + HighPriorities: []string{"namespaces", "persistentvolumes", "persistentvolumeclaims", "serviceaccounts"}, LowPriorities: []string{"deployments.apps"}, }, }, @@ -944,7 +1097,7 @@ func TestRestoreResourcePriorities(t *testing.T) { ) assertEmptyResults(t, warnings, errs) - assertResourceCreationOrder(t, []string{"persistentvolumes", "persistentvolumeclaims", "serviceaccounts", "pods", "deployments.apps"}, recorder.resources) + assertResourceCreationOrder(t, []string{"namespaces", "persistentvolumes", "persistentvolumeclaims", "serviceaccounts", "pods", "deployments.apps"}, recorder.resources) } } @@ -1510,12 +1663,13 @@ func TestRestoreActionsRunForCorrectItems(t *testing.T) { restore: defaultRestore().Result(), backup: defaultBackup().Result(), tarball: test.NewTarWriter(t). + AddItems("namespaces", builder.ForNamespace("ns-1").Result(), builder.ForNamespace("ns-2").Result()). AddItems("pods", builder.ForPod("ns-1", "pod-1").Result(), builder.ForPod("ns-2", "pod-2").Result()). AddItems("persistentvolumes", builder.ForPersistentVolume("pv-1").Result(), builder.ForPersistentVolume("pv-2").Result()). Done(), - apiResources: []*test.APIResource{test.Pods(), test.PVs()}, + apiResources: []*test.APIResource{test.Namespaces(), test.Pods(), test.PVs()}, actions: map[*recordResourcesAction][]string{ - new(recordResourcesAction): {"ns-1/pod-1", "ns-2/pod-2", "pv-1", "pv-2"}, + new(recordResourcesAction): {"ns-1", "ns-2", "ns-1/pod-1", "ns-2/pod-2", "pv-1", "pv-2"}, }, }, { @@ -1920,12 +2074,28 @@ func TestRestoreWithAsyncOperations(t *testing.T) { name: "action that starts a short-running process records operation", restore: defaultRestore().Result(), backup: defaultBackup().Result(), - apiResources: []*test.APIResource{test.Pods()}, - tarball: test.NewTarWriter(t).AddItems("pods", builder.ForPod("ns-1", "pod-1").Result()).Done(), + apiResources: []*test.APIResource{test.Namespaces(), test.Pods()}, + tarball: test.NewTarWriter(t). + AddItems("namespaces", builder.ForNamespace("ns-1").Result()). + AddItems("pods", builder.ForPod("ns-1", "pod-1").Result()). + Done(), actions: []riav2.RestoreItemAction{ completedOperationAction, }, want: []*itemoperation.RestoreOperation{ + { + Spec: itemoperation.RestoreOperationSpec{ + RestoreName: "restore-1", + ResourceIdentifier: velero.ResourceIdentifier{ + GroupResource: kuberesource.Namespaces, + Namespace: "", + Name: "ns-1"}, + OperationID: "ns-1-1", + }, + Status: itemoperation.OperationStatus{ + Phase: "New", + }, + }, { Spec: itemoperation.RestoreOperationSpec{ RestoreName: "restore-1", @@ -1945,12 +2115,28 @@ func TestRestoreWithAsyncOperations(t *testing.T) { name: "action that starts a long-running process records operation", restore: defaultRestore().Result(), backup: defaultBackup().Result(), - apiResources: []*test.APIResource{test.Pods()}, - tarball: test.NewTarWriter(t).AddItems("pods", builder.ForPod("ns-1", "pod-2").Result()).Done(), + apiResources: []*test.APIResource{test.Namespaces(), test.Pods()}, + tarball: test.NewTarWriter(t). + AddItems("namespaces", builder.ForNamespace("ns-1").Result()). + AddItems("pods", builder.ForPod("ns-1", "pod-2").Result()). + Done(), actions: []riav2.RestoreItemAction{ incompleteOperationAction, }, want: []*itemoperation.RestoreOperation{ + { + Spec: itemoperation.RestoreOperationSpec{ + RestoreName: "restore-1", + ResourceIdentifier: velero.ResourceIdentifier{ + GroupResource: kuberesource.Namespaces, + Namespace: "", + Name: "ns-1"}, + OperationID: "ns-1-1", + }, + Status: itemoperation.OperationStatus{ + Phase: "New", + }, + }, { Spec: itemoperation.RestoreOperationSpec{ RestoreName: "restore-1", diff --git a/pkg/util/kube/utils.go b/pkg/util/kube/utils.go index bda40d35ce..db4da8c514 100644 --- a/pkg/util/kube/utils.go +++ b/pkg/util/kube/utils.go @@ -19,7 +19,6 @@ package kube import ( "context" "fmt" - "time" "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -27,12 +26,9 @@ import ( storagev1api "k8s.io/api/storage/v1" apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apiextv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" - apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/util/wait" - corev1client "k8s.io/client-go/kubernetes/typed/core/v1" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/vmware-tanzu/velero/pkg/uploader" @@ -61,66 +57,6 @@ func NamespaceAndName(objMeta metav1.Object) string { return fmt.Sprintf("%s/%s", objMeta.GetNamespace(), objMeta.GetName()) } -// EnsureNamespaceExistsAndIsReady attempts to create the provided Kubernetes namespace. -// It returns three values: a bool indicating whether or not the namespace is ready, -// a bool indicating whether or not the namespace was created and an error if the creation failed -// for a reason other than that the namespace already exists. Note that in the case where the -// namespace already exists and is not ready, this function will return (false, false, nil). -// If the namespace exists and is marked for deletion, this function will wait up to the timeout for it to fully delete. -func EnsureNamespaceExistsAndIsReady(namespace *corev1api.Namespace, client corev1client.NamespaceInterface, timeout time.Duration) (bool, bool, error) { - // nsCreated tells whether the namespace was created by this method - // required for keeping track of number of restored items - var nsCreated bool - var ready bool - err := wait.PollUntilContextTimeout(context.Background(), time.Second, timeout, true, func(ctx context.Context) (bool, error) { - clusterNS, err := client.Get(ctx, namespace.Name, metav1.GetOptions{}) - - if apierrors.IsNotFound(err) { - // Namespace isn't in cluster, we're good to create. - return true, nil - } - - if err != nil { - // Return the err and exit the loop. - return true, err - } - - if clusterNS != nil && (clusterNS.GetDeletionTimestamp() != nil || clusterNS.Status.Phase == corev1api.NamespaceTerminating) { - // Marked for deletion, keep waiting - return false, nil - } - - // clusterNS found, is not nil, and not marked for deletion, therefore we shouldn't create it. - ready = true - return true, nil - }) - - // err will be set if we timed out or encountered issues retrieving the namespace, - if err != nil { - return false, nsCreated, errors.Wrapf(err, "error getting namespace %s", namespace.Name) - } - - // In the case the namespace already exists and isn't marked for deletion, assume it's ready for use. - if ready { - return true, nsCreated, nil - } - - clusterNS, err := client.Create(context.TODO(), namespace, metav1.CreateOptions{}) - if apierrors.IsAlreadyExists(err) { - if clusterNS != nil && (clusterNS.GetDeletionTimestamp() != nil || clusterNS.Status.Phase == corev1api.NamespaceTerminating) { - // Somehow created after all our polling and marked for deletion, return an error - return false, nsCreated, errors.Errorf("namespace %s created and marked for termination after timeout", namespace.Name) - } - } else if err != nil { - return false, nsCreated, errors.Wrapf(err, "error creating namespace %s", namespace.Name) - } else { - nsCreated = true - } - - // The namespace created successfully - return true, nsCreated, nil -} - // GetVolumeDirectory gets the name of the directory on the host, under /var/lib/kubelet/pods//volumes/, // where the specified volume lives. // For volumes with a CSIVolumeSource, append "/mount" to the directory name. diff --git a/pkg/util/kube/utils_test.go b/pkg/util/kube/utils_test.go index 31110dc8c0..15ddd01795 100644 --- a/pkg/util/kube/utils_test.go +++ b/pkg/util/kube/utils_test.go @@ -20,7 +20,6 @@ import ( "context" "encoding/json" "testing" - "time" "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" @@ -29,11 +28,9 @@ import ( storagev1api "k8s.io/api/storage/v1" apiextv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apiextv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" - k8serrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" "sigs.k8s.io/controller-runtime/pkg/client/fake" "github.com/vmware-tanzu/velero/pkg/builder" @@ -45,102 +42,6 @@ func TestNamespaceAndName(t *testing.T) { //TODO } -func TestEnsureNamespaceExistsAndIsReady(t *testing.T) { - tests := []struct { - name string - expectNSFound bool - nsPhase corev1.NamespacePhase - nsDeleting bool - expectCreate bool - alreadyExists bool - expectedResult bool - expectedCreatedResult bool - }{ - { - name: "namespace found, not deleting", - expectNSFound: true, - expectedResult: true, - expectedCreatedResult: false, - }, - { - name: "namespace found, terminating phase", - expectNSFound: true, - nsPhase: corev1.NamespaceTerminating, - expectedResult: false, - expectedCreatedResult: false, - }, - { - name: "namespace found, deletiontimestamp set", - expectNSFound: true, - nsDeleting: true, - expectedResult: false, - expectedCreatedResult: false, - }, - { - name: "namespace not found, successfully created", - expectCreate: true, - expectedResult: true, - expectedCreatedResult: true, - }, - { - name: "namespace not found initially, create returns already exists error, returned namespace is ready", - alreadyExists: true, - expectedResult: true, - expectedCreatedResult: false, - }, - { - name: "namespace not found initially, create returns already exists error, returned namespace is terminating", - alreadyExists: true, - nsPhase: corev1.NamespaceTerminating, - expectedResult: false, - expectedCreatedResult: false, - }, - } - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - namespace := &corev1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test", - }, - } - - if test.nsPhase != "" { - namespace.Status.Phase = test.nsPhase - } - - if test.nsDeleting { - namespace.SetDeletionTimestamp(&metav1.Time{Time: time.Now()}) - } - - timeout := time.Millisecond - - nsClient := &velerotest.FakeNamespaceClient{} - defer nsClient.AssertExpectations(t) - - if test.expectNSFound { - nsClient.On("Get", "test", metav1.GetOptions{}).Return(namespace, nil) - } else { - nsClient.On("Get", "test", metav1.GetOptions{}).Return(&corev1.Namespace{}, k8serrors.NewNotFound(schema.GroupResource{Resource: "namespaces"}, "test")) - } - - if test.alreadyExists { - nsClient.On("Create", namespace).Return(namespace, k8serrors.NewAlreadyExists(schema.GroupResource{Resource: "namespaces"}, "test")) - } - - if test.expectCreate { - nsClient.On("Create", namespace).Return(namespace, nil) - } - - result, nsCreated, _ := EnsureNamespaceExistsAndIsReady(namespace, nsClient, timeout) - - assert.Equal(t, test.expectedResult, result) - assert.Equal(t, test.expectedCreatedResult, nsCreated) - }) - } - -} - // TestGetVolumeDirectorySuccess tests that the GetVolumeDirectory function // returns a volume's name or a volume's name plus '/mount' when a PVC is present. func TestGetVolumeDirectorySuccess(t *testing.T) {