diff --git a/changelogs/unreleased/8342-kaovilai b/changelogs/unreleased/8342-kaovilai new file mode 100644 index 0000000000..5b55b8366e --- /dev/null +++ b/changelogs/unreleased/8342-kaovilai @@ -0,0 +1 @@ +breaking: Namespaces included by labelSelector act as IncludedNamespaces, all items including unlabled under a namespace with labelSelector selected by backup selector will be included in the backup. diff --git a/pkg/backup/backup_test.go b/pkg/backup/backup_test.go index 78ba0b0cc9..96730837f2 100644 --- a/pkg/backup/backup_test.go +++ b/pkg/backup/backup_test.go @@ -267,6 +267,41 @@ func TestBackupOldResourceFiltering(t *testing.T) { "resources/deployments.apps/v1-preferredversion/namespaces/foo/bar.json", }, }, + { + name: "included namespaces via label filter backs up all resources in those namespaces and other includedNamespaces resources containing label", + backup: defaultBackup(). + IncludedNamespaces("moo"). + LabelSelector(&metav1.LabelSelector{MatchLabels: map[string]string{"backup-ns": "true"}}). + Result(), + apiResources: []*test.APIResource{ + test.Namespaces( + builder.ForNamespace("foo").ObjectMeta(builder.WithLabels("backup-ns", "true")).Result(), + builder.ForNamespace("moo").Result(), + ), + test.Pods( + builder.ForPod("foo", "bar").Result(), + builder.ForPod("moo", "mar").ObjectMeta(builder.WithLabels("backup-ns", "true")).Result(), + builder.ForPod("moo", "unlabeled-not-included").Result(), + builder.ForPod("zoo", "raz").Result(), + ), + test.Deployments( + builder.ForDeployment("foo", "bar").Result(), + builder.ForDeployment("zoo", "raz").Result(), + ), + }, + want: []string{ + "resources/deployments.apps/namespaces/foo/bar.json", + "resources/deployments.apps/v1-preferredversion/namespaces/foo/bar.json", + "resources/pods/v1-preferredversion/namespaces/foo/bar.json", + "resources/pods/v1-preferredversion/namespaces/moo/mar.json", + "resources/pods/namespaces/foo/bar.json", + "resources/pods/namespaces/moo/mar.json", + "resources/namespaces/cluster/foo.json", + "resources/namespaces/cluster/moo.json", + "resources/namespaces/v1-preferredversion/cluster/foo.json", + "resources/namespaces/v1-preferredversion/cluster/moo.json", + }, + }, { name: "excluded namespaces filter only backs up resources not in those namespaces", backup: defaultBackup(). diff --git a/pkg/backup/item_collector.go b/pkg/backup/item_collector.go index a1fce5ee74..a59e12e6e7 100644 --- a/pkg/backup/item_collector.go +++ b/pkg/backup/item_collector.go @@ -32,6 +32,7 @@ import ( "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/tools/pager" "github.com/vmware-tanzu/velero/pkg/client" @@ -44,14 +45,16 @@ import ( // itemCollector collects items from the Kubernetes API according to // the backup spec and writes them to files inside dir. type itemCollector struct { - log logrus.FieldLogger - backupRequest *Request - discoveryHelper discovery.Helper - dynamicFactory client.DynamicFactory - cohabitatingResources map[string]*cohabitatingResource - dir string - pageSize int - nsTracker nsTracker + log logrus.FieldLogger + backupRequest *Request + // Namespaces that are included by the backup's labelSelector will backup all resources in that namespace even if resources are not labeled. + namespacesIncludedByLabelSelector sets.Set[string] + discoveryHelper discovery.Helper + dynamicFactory client.DynamicFactory + cohabitatingResources map[string]*cohabitatingResource + dir string + pageSize int + nsTracker nsTracker } // nsTracker is used to integrate several namespace filters together. @@ -72,32 +75,40 @@ type nsTracker struct { orLabelSelector []labels.Selector namespaceFilter *collections.IncludesExcludes logger logrus.FieldLogger - - namespaceMap map[string]bool + namespaceMap sets.Set[string] // namespaceMap is a set of namespaces tracked to include in backup + labeledNamespaces sets.Set[string] // labeledNamespaces is a subset of namespaceMap added via label selectors. } // track add the namespace into the namespaceMap. func (nt *nsTracker) track(ns string) { if nt.namespaceMap == nil { - nt.namespaceMap = make(map[string]bool) + nt.namespaceMap = make(sets.Set[string]) } + nt.namespaceMap.Insert(ns) +} - if _, ok := nt.namespaceMap[ns]; !ok { - nt.namespaceMap[ns] = true +// trackViaLabel add the namespace into the labeledNamespaces and namespacesMap. +func (nt *nsTracker) trackViaLabel(ns string) { + if nt.labeledNamespaces == nil { + nt.labeledNamespaces = make(sets.Set[string]) } + nt.labeledNamespaces.Insert(ns) + nt.track(ns) } // isTracked check whether the namespace's name exists in // namespaceMap. func (nt *nsTracker) isTracked(ns string) bool { - if nt.namespaceMap != nil { - return nt.namespaceMap[ns] - } - return false + return nt.namespaceMap.Has(ns) +} + +func (nt *nsTracker) isTrackedViaLabel(ns string) bool { + return nt.labeledNamespaces.Has(ns) } -// init initialize the namespaceMap, and add elements according to +// init initialize the namespaceMap, and add (track) elements according to // namespace include/exclude filters and the backup label selectors. +// and return only namespaces that are added to tracker. func (nt *nsTracker) init( unstructuredNSs []unstructured.Unstructured, singleLabelSelector labels.Selector, @@ -105,9 +116,6 @@ func (nt *nsTracker) init( namespaceFilter *collections.IncludesExcludes, logger logrus.FieldLogger, ) { - if nt.namespaceMap == nil { - nt.namespaceMap = make(map[string]bool) - } nt.singleLabelSelector = singleLabelSelector nt.orLabelSelector = orLabelSelector nt.namespaceFilter = namespaceFilter @@ -120,7 +128,7 @@ func (nt *nsTracker) init( namespace.GetName(), ) - nt.track(namespace.GetName()) + nt.trackViaLabel(namespace.GetName()) continue } @@ -130,7 +138,7 @@ func (nt *nsTracker) init( nt.logger.Debugf("Track namespace %s, because its labels match the backup OrLabelSelector.", namespace.GetName(), ) - nt.track(namespace.GetName()) + nt.trackViaLabel(namespace.GetName()) continue } } @@ -194,6 +202,7 @@ func (r *itemCollector) getItemsFromResourceIdentifiers( // getAllItems gets all backup-relevant items from all API groups. func (r *itemCollector) getAllItems() []*kubernetesResource { + // getItems should return all namespaces and will be filtered by nsTracker.filterNamespaces resources := r.getItems(nil) return r.nsTracker.filterNamespaces(resources) @@ -436,13 +445,33 @@ func (r *itemCollector) getResourceItems( // Namespace are filtered by namespace include/exclude filters, // backup LabelSelectors and OrLabelSelectors are checked too. if gr == kuberesource.Namespaces { - return r.collectNamespaces( + // collectNamespaces initializes the nsTracker which should only contain namespaces to backup but returns all namespaces. + namespaces, err := r.collectNamespaces( resource, gv, gr, preferredGVR, log, ) + if err != nil { + return nil, err + } + // The namespaces collected contains namespaces selected by namespace filters like include/exclude and label selector. + // Per https://github.com/vmware-tanzu/velero/issues/7492#issuecomment-1986146411 we want labelSelector included namespace to act as IncludedNamespaces so add to NamespaceIncludesExcludes string set. + r.namespacesIncludedByLabelSelector = make(sets.Set[string]) + for i := range namespaces { + if namespaces[i] != nil { + if r.nsTracker.isTrackedViaLabel(namespaces[i].name) { + // this namespace is included by the backup's label selector, not the namespace include/exclude filter + // this is a special case where we want to include this namespace in the backup and all resources in it regardless of the resource's label selector + // in other cases, if label selector is set, we only want to include resources that match the label selector + r.namespacesIncludedByLabelSelector.Insert(namespaces[i].name) + r.backupRequest.NamespaceIncludesExcludes.Includes(namespaces[i].name) + r.log.Infof("including namespace %s by labelselector\n", namespaces[i].name) + } + } + } + return namespaces, err } clusterScoped := !resource.Namespaced @@ -509,9 +538,13 @@ func (r *itemCollector) listResourceByLabelsPerNamespace( logger.WithError(err).Error("Error getting dynamic client") return nil, err } - + labeledNsSoBackupUnlabeled := false + if r.namespacesIncludedByLabelSelector.Has(namespace) { + logger.Infof("Listing all items including unlabeled in namespace %s because ns was selected by the backup's label selector and acts as IncludedNamespaces", namespace) + labeledNsSoBackupUnlabeled = true + } var orLabelSelectors []string - if r.backupRequest.Spec.OrLabelSelectors != nil { + if r.backupRequest.Spec.OrLabelSelectors != nil && !labeledNsSoBackupUnlabeled { for _, s := range r.backupRequest.Spec.OrLabelSelectors { orLabelSelectors = append(orLabelSelectors, metav1.FormatLabelSelector(s)) } @@ -537,7 +570,7 @@ func (r *itemCollector) listResourceByLabelsPerNamespace( } var labelSelector string - if selector := r.backupRequest.Spec.LabelSelector; selector != nil { + if selector := r.backupRequest.Spec.LabelSelector; selector != nil && !labeledNsSoBackupUnlabeled { labelSelector = metav1.FormatLabelSelector(selector) } @@ -594,7 +627,8 @@ func sortCoreGroup(group *metav1.APIResourceList) { // pod is backed up, we can perform a pre hook, then process pvcs and pvs (including taking a // snapshot), then perform a post hook on the pod. const ( - pod = iota + namespaces = iota + pod pvc pv other @@ -604,6 +638,8 @@ const ( // pods, pvcs, pvs, everything else. func coreGroupResourcePriority(resource string) int { switch strings.ToLower(resource) { + case "namespaces": + return namespaces case "pods": return pod case "persistentvolumeclaims": @@ -722,6 +758,7 @@ func (r *itemCollector) listItemsForLabel( } // collectNamespaces process namespace resource according to namespace filters. +// returns a list of ALL namespaces. Filtering will happen outside of this function. func (r *itemCollector) collectNamespaces( resource metav1.APIResource, gv schema.GroupVersion, @@ -780,7 +817,8 @@ func (r *itemCollector) collectNamespaces( orSelectors = append(orSelectors, orSelector) } } - + // init initialize the namespaceMap, and add elements according to + // namespace include/exclude filters and the backup label selectors. r.nsTracker.init( unstructuredList.Items, singleSelector, @@ -806,6 +844,5 @@ func (r *itemCollector) collectNamespaces( path: path, }) } - return items, nil }