Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Select namespace to include by its labelSelector #7492

Closed
kaovilai opened this issue Mar 4, 2024 · 8 comments
Closed

Select namespace to include by its labelSelector #7492

kaovilai opened this issue Mar 4, 2024 · 8 comments
Assignees
Labels
kind/requirement Needs triage We need discussion to understand problem and decide the priority

Comments

@kaovilai
Copy link
Contributor

kaovilai commented Mar 4, 2024

Describe the problem/challenge you have

includedNamespacesByLabelSelector Equivalent to includedNamespaces in Backup/Restore CR but not by name, but namespace's labelSelector.

User would not have to label each item inside the namespace, they can update which namespaces have this label, which on a future run manually or via schedule, could desirably result in different list of namespaces being included.

Not to be confused with #7105 which wants to exclude namespaces not containing backup items selected by label.

Describe the solution you'd like

Since backup can only be run once, we can reasonbly expect velero to annotate the backup to communicate what namespaces were in at backup time.

option 1: User who want to rerun a backup when list of namespaces with label changed will have to remove includedNamespaces field if any. velero update includedNamespaces to be result of namespaces in the backup
option 2: includedNamespacesByLabelSelector is additive to but will not modify includedNamespaces backup spec
option 3: includedNamespacesByLabelSelector is additive to and will update includedNamespaces backup spec
option 4: includedNamespacesByLabelSelector always overrides includedNamespaces backup spec

Anything else you would like to add:

Environment:

  • Velero version (use velero version):
  • Kubernetes version (use kubectl version):
  • Kubernetes installer & version:
  • Cloud provider or hardware configuration:
  • OS (e.g. from /etc/os-release):

Vote on this issue!

This is an invitation to the Velero community to vote on issues, you can see the project's top voted issues listed here.
Use the "reaction smiley face" up to the right of this comment to vote.

  • 👍 for "The project would be better with this feature added"
  • 👎 for "This feature will not enhance the project in a meaningful way"

Hackaround

# not tested, just quick notes
NSes=$(kubectl get ns --label -oname)
echo '
backup…
includedNamespaces: $NSes
' | kubectl create -f --

and if you need this to be a schedule, can probably also wrap this into a kubernetes cronjob

@kaovilai kaovilai changed the title Select namespace to include by it's labelSelector Select namespace to include by its labelSelector Mar 4, 2024
@blackpiglet
Copy link
Contributor

@kaovilai
Thanks for proposing this solution.
I suggest to put this on hold for a while. If there is a way to make it work in #7105, there is no need to introduce a new filter.
Although we can make the decision about which filter has higher priority, more filters will introduce confusion to the user.

@kaovilai
Copy link
Contributor Author

kaovilai commented Mar 8, 2024

Ok. I think we can make it work within 7105.

If user label a namespace to backup, I think they would want everything in it.. thus it should behave equivalently to includedNamespaces.

No need for new flag.

@reasonerjt reasonerjt added Needs triage We need discussion to understand problem and decide the priority kind/requirement labels Mar 11, 2024
@kaovilai kaovilai closed this as not planned Won't fix, can't repro, duplicate, stale Apr 17, 2024
@blackpiglet
Copy link
Contributor

blackpiglet commented Apr 19, 2024

The PR #7697 addresses this requirement.

@kaovilai
Copy link
Contributor Author

Re closing as completed per 7697.

@muellerfabi
Copy link

Re closing as completed per 7697.

tbh after checking that PR, I still don't get how to backup selected namespaces upon their label.

@blackpiglet
Copy link
Contributor

@muellerfabi
There are two rules:

  • When the backup's namespace filter is not set, or all namespaces are included, and the backup's label selectors are set, the namespaces, that do not have backup including resources, are not collected.
  • If the namespace I/E filters and the (Or)LabelSelectors selected namespaces are different. The tracker takes the union of them.

@kaovilai
Copy link
Contributor Author

kaovilai commented May 8, 2024

If the namespace I/E filters and the (Or)LabelSelectors selected namespaces are different. The tracker takes the union of them.

Which is equivalent to --includedNamespaces right?

@blackpiglet
Copy link
Contributor

Say there is a namespace ns1 labeled as demo=issue-7492, and a namespace ns2, then create a backup with parameters --selector demo=issue-7493 and --include-namespaces ns2.
Both namespace ns1 and ns2 are included in the backup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/requirement Needs triage We need discussion to understand problem and decide the priority
Projects
None yet
Development

No branches or pull requests

4 participants