From 2d71430e807bf6881f749374c48381820c28a01b Mon Sep 17 00:00:00 2001 From: Xun Jiang Date: Sun, 27 Apr 2025 18:47:58 +0800 Subject: [PATCH] Skip namespace in terminating state in backup resource collection. To make sure resources in terminating namespaces are not included. Signed-off-by: Xun Jiang --- changelogs/unreleased/8890-blackpiglet | 1 + pkg/backup/backup_test.go | 36 ++++++++++---------- pkg/backup/item_collector.go | 11 ++++++ pkg/backup/item_collector_test.go | 47 +++++++++++++++++--------- 4 files changed, 61 insertions(+), 34 deletions(-) create mode 100644 changelogs/unreleased/8890-blackpiglet diff --git a/changelogs/unreleased/8890-blackpiglet b/changelogs/unreleased/8890-blackpiglet new file mode 100644 index 000000000..e670b9635 --- /dev/null +++ b/changelogs/unreleased/8890-blackpiglet @@ -0,0 +1 @@ +Skip namespace in terminating state in backup resource collection. diff --git a/pkg/backup/backup_test.go b/pkg/backup/backup_test.go index e2926d471..184b05310 100644 --- a/pkg/backup/backup_test.go +++ b/pkg/backup/backup_test.go @@ -5341,9 +5341,9 @@ func TestBackupNamespaces(t *testing.T) { Result(), apiResources: []*test.APIResource{ test.Namespaces( - builder.ForNamespace("ns-1").Result(), - builder.ForNamespace("ns-2").Result(), - builder.ForNamespace("ns-3").Result(), + builder.ForNamespace("ns-1").Phase(corev1api.NamespaceActive).Result(), + builder.ForNamespace("ns-2").Phase(corev1api.NamespaceActive).Result(), + builder.ForNamespace("ns-3").Phase(corev1api.NamespaceActive).Result(), ), test.Deployments( builder.ForDeployment("ns-1", "deploy-1").ObjectMeta(builder.WithLabels("a", "b")).Result(), @@ -5364,9 +5364,9 @@ func TestBackupNamespaces(t *testing.T) { }).Result(), apiResources: []*test.APIResource{ test.Namespaces( - builder.ForNamespace("ns-1").Result(), - builder.ForNamespace("ns-2").Result(), - builder.ForNamespace("ns-3").Result(), + builder.ForNamespace("ns-1").Phase(corev1api.NamespaceActive).Result(), + builder.ForNamespace("ns-2").Phase(corev1api.NamespaceActive).Result(), + builder.ForNamespace("ns-3").Phase(corev1api.NamespaceActive).Result(), ), test.Deployments( builder.ForDeployment("ns-1", "deploy-1").ObjectMeta(builder.WithLabels("a", "b")).Result(), @@ -5390,9 +5390,9 @@ func TestBackupNamespaces(t *testing.T) { Result(), apiResources: []*test.APIResource{ test.Namespaces( - builder.ForNamespace("ns-1").Result(), - builder.ForNamespace("ns-2").Result(), - builder.ForNamespace("ns-3").Result(), + builder.ForNamespace("ns-1").Phase(corev1api.NamespaceActive).Result(), + builder.ForNamespace("ns-2").Phase(corev1api.NamespaceActive).Result(), + builder.ForNamespace("ns-3").Phase(corev1api.NamespaceActive).Result(), ), test.Deployments( builder.ForDeployment("ns-1", "deploy-1").ObjectMeta(builder.WithLabels("a", "b")).Result(), @@ -5411,9 +5411,9 @@ func TestBackupNamespaces(t *testing.T) { Result(), apiResources: []*test.APIResource{ test.Namespaces( - builder.ForNamespace("ns-1").ObjectMeta(builder.WithLabels("a", "b")).Result(), - builder.ForNamespace("ns-2").Result(), - builder.ForNamespace("ns-3").Result(), + builder.ForNamespace("ns-1").ObjectMeta(builder.WithLabels("a", "b")).Phase(corev1api.NamespaceActive).Result(), + builder.ForNamespace("ns-2").Phase(corev1api.NamespaceActive).Result(), + builder.ForNamespace("ns-3").Phase(corev1api.NamespaceActive).Result(), ), test.Deployments( builder.ForDeployment("ns-1", "deploy-1").ObjectMeta(builder.WithLabels("a", "b")).Result(), @@ -5431,9 +5431,9 @@ func TestBackupNamespaces(t *testing.T) { backup: defaultBackup().IncludedNamespaces("invalid*").Result(), apiResources: []*test.APIResource{ test.Namespaces( - builder.ForNamespace("ns-1").Result(), - builder.ForNamespace("ns-2").Result(), - builder.ForNamespace("ns-3").Result(), + builder.ForNamespace("ns-1").Phase(corev1api.NamespaceActive).Result(), + builder.ForNamespace("ns-2").Phase(corev1api.NamespaceActive).Result(), + builder.ForNamespace("ns-3").Phase(corev1api.NamespaceActive).Result(), ), test.Deployments( builder.ForDeployment("ns-1", "deploy-1").ObjectMeta(builder.WithLabels("a", "b")).Result(), @@ -5446,9 +5446,9 @@ func TestBackupNamespaces(t *testing.T) { backup: defaultBackup().Result(), apiResources: []*test.APIResource{ test.Namespaces( - builder.ForNamespace("ns-1").Result(), - builder.ForNamespace("ns-2").Result(), - builder.ForNamespace("ns-3").Result(), + builder.ForNamespace("ns-1").Phase(corev1api.NamespaceActive).Result(), + builder.ForNamespace("ns-2").Phase(corev1api.NamespaceActive).Result(), + builder.ForNamespace("ns-3").Phase(corev1api.NamespaceActive).Result(), ), test.Deployments( builder.ForDeployment("ns-1", "deploy-1").ObjectMeta(builder.WithLabels("a", "b")).Result(), diff --git a/pkg/backup/item_collector.go b/pkg/backup/item_collector.go index 2eb9079f5..2338dd77b 100644 --- a/pkg/backup/item_collector.go +++ b/pkg/backup/item_collector.go @@ -26,6 +26,7 @@ import ( "github.com/pkg/errors" "github.com/sirupsen/logrus" + corev1api "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -114,6 +115,16 @@ func (nt *nsTracker) init( nt.logger = logger for _, namespace := range unstructuredNSs { + ns := new(corev1api.Namespace) + if err := runtime.DefaultUnstructuredConverter.FromUnstructured(namespace.UnstructuredContent(), ns); err != nil { + nt.logger.WithError(err).Errorf("Fail to convert unstructured into namespace %s", namespace.GetName()) + continue + } + if ns.Status.Phase != corev1api.NamespaceActive { + nt.logger.Infof("Skip namespace %s because it's not in Active phase.", namespace.GetName()) + continue + } + if nt.singleLabelSelector != nil && nt.singleLabelSelector.Matches(labels.Set(namespace.GetLabels())) { nt.logger.Debugf("Track namespace %s, because its labels match backup LabelSelector.", diff --git a/pkg/backup/item_collector_test.go b/pkg/backup/item_collector_test.go index 992b5ec7f..bbdad7e43 100644 --- a/pkg/backup/item_collector_test.go +++ b/pkg/backup/item_collector_test.go @@ -158,14 +158,15 @@ func TestItemCollectorBackupNamespaces(t *testing.T) { namespaces []*corev1api.Namespace backup *velerov1api.Backup expectedTrackedNS []string + converter runtime.UnstructuredConverter }{ { name: "ns filter by namespace IE filter", backup: builder.ForBackup("velero", "backup").Result(), ie: collections.NewIncludesExcludes().Includes("ns1"), namespaces: []*corev1api.Namespace{ - builder.ForNamespace("ns1").Result(), - builder.ForNamespace("ns2").Result(), + builder.ForNamespace("ns1").Phase(corev1api.NamespaceActive).Result(), + builder.ForNamespace("ns2").Phase(corev1api.NamespaceActive).Result(), }, expectedTrackedNS: []string{"ns1"}, }, @@ -176,8 +177,8 @@ func TestItemCollectorBackupNamespaces(t *testing.T) { }).Result(), ie: collections.NewIncludesExcludes().Includes("*"), namespaces: []*corev1api.Namespace{ - builder.ForNamespace("ns1").ObjectMeta(builder.WithLabels("name", "ns1")).Result(), - builder.ForNamespace("ns2").Result(), + builder.ForNamespace("ns1").ObjectMeta(builder.WithLabels("name", "ns1")).Phase(corev1api.NamespaceActive).Result(), + builder.ForNamespace("ns2").Phase(corev1api.NamespaceActive).Result(), }, expectedTrackedNS: []string{"ns1"}, }, @@ -188,8 +189,8 @@ func TestItemCollectorBackupNamespaces(t *testing.T) { }).Result(), ie: collections.NewIncludesExcludes().Includes("*"), namespaces: []*corev1api.Namespace{ - builder.ForNamespace("ns1").ObjectMeta(builder.WithLabels("name", "ns1")).Result(), - builder.ForNamespace("ns2").Result(), + builder.ForNamespace("ns1").ObjectMeta(builder.WithLabels("name", "ns1")).Phase(corev1api.NamespaceActive).Result(), + builder.ForNamespace("ns2").Phase(corev1api.NamespaceActive).Result(), }, expectedTrackedNS: []string{"ns1"}, }, @@ -200,8 +201,8 @@ func TestItemCollectorBackupNamespaces(t *testing.T) { }).Result(), ie: collections.NewIncludesExcludes().Excludes("ns1"), namespaces: []*corev1api.Namespace{ - builder.ForNamespace("ns1").ObjectMeta(builder.WithLabels("name", "ns1")).Result(), - builder.ForNamespace("ns2").Result(), + builder.ForNamespace("ns1").ObjectMeta(builder.WithLabels("name", "ns1")).Phase(corev1api.NamespaceActive).Result(), + builder.ForNamespace("ns2").Phase(corev1api.NamespaceActive).Result(), }, expectedTrackedNS: []string{"ns1"}, }, @@ -212,9 +213,9 @@ func TestItemCollectorBackupNamespaces(t *testing.T) { }).Result(), ie: collections.NewIncludesExcludes().Excludes("ns1", "ns2"), namespaces: []*corev1api.Namespace{ - builder.ForNamespace("ns1").ObjectMeta(builder.WithLabels("name", "ns1")).Result(), - builder.ForNamespace("ns2").Result(), - builder.ForNamespace("ns3").Result(), + builder.ForNamespace("ns1").ObjectMeta(builder.WithLabels("name", "ns1")).Phase(corev1api.NamespaceActive).Result(), + builder.ForNamespace("ns2").Phase(corev1api.NamespaceActive).Result(), + builder.ForNamespace("ns3").Phase(corev1api.NamespaceActive).Result(), }, expectedTrackedNS: []string{"ns1", "ns3"}, }, @@ -223,8 +224,8 @@ func TestItemCollectorBackupNamespaces(t *testing.T) { backup: builder.ForBackup("velero", "backup").Result(), ie: collections.NewIncludesExcludes().Includes("*"), namespaces: []*corev1api.Namespace{ - builder.ForNamespace("ns1").ObjectMeta(builder.WithLabels("name", "ns1")).Result(), - builder.ForNamespace("ns2").Result(), + builder.ForNamespace("ns1").ObjectMeta(builder.WithLabels("name", "ns1")).Phase(corev1api.NamespaceActive).Result(), + builder.ForNamespace("ns2").Phase(corev1api.NamespaceActive).Result(), }, expectedTrackedNS: []string{"ns1", "ns2"}, }, @@ -233,12 +234,22 @@ func TestItemCollectorBackupNamespaces(t *testing.T) { backup: builder.ForBackup("velero", "backup").IncludedNamespaces("ns1", "invalid", "*").Result(), ie: collections.NewIncludesExcludes().Includes("ns1", "invalid", "*"), namespaces: []*corev1api.Namespace{ - builder.ForNamespace("ns1").ObjectMeta(builder.WithLabels("name", "ns1")).Result(), - builder.ForNamespace("ns2").Result(), - builder.ForNamespace("ns3").Result(), + builder.ForNamespace("ns1").ObjectMeta(builder.WithLabels("name", "ns1")).Phase(corev1api.NamespaceActive).Result(), + builder.ForNamespace("ns2").Phase(corev1api.NamespaceActive).Result(), + builder.ForNamespace("ns3").Phase(corev1api.NamespaceActive).Result(), }, expectedTrackedNS: []string{"ns1"}, }, + { + name: "terminating ns should not tracked", + backup: builder.ForBackup("velero", "backup").Result(), + ie: collections.NewIncludesExcludes().Includes("ns1", "ns2"), + namespaces: []*corev1api.Namespace{ + builder.ForNamespace("ns1").Phase(corev1api.NamespaceTerminating).Result(), + builder.ForNamespace("ns2").Phase(corev1api.NamespaceActive).Result(), + }, + expectedTrackedNS: []string{"ns2"}, + }, } for _, tc := range tests { @@ -274,6 +285,10 @@ func TestItemCollectorBackupNamespaces(t *testing.T) { dir: tempDir, } + if tc.converter == nil { + tc.converter = runtime.DefaultUnstructuredConverter + } + r.collectNamespaces( metav1.APIResource{ Name: "Namespace",