-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Design to add label selector as a criteria for volume policy #8503
base: main
Are you sure you want to change the base?
Design to add label selector as a criteria for volume policy #8503
Conversation
7da988d
to
a8b3363
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #8503 +/- ##
==========================================
+ Coverage 59.03% 59.38% +0.35%
==========================================
Files 369 370 +1
Lines 39068 39987 +919
==========================================
+ Hits 23064 23747 +683
- Misses 14543 14747 +204
- Partials 1461 1493 +32 ☔ View full report in Codecov by Sentry. |
@shubham-pampattiwar Since users are creating PVCs directly but not generally PVs (and may not have permission to label PVs), should the label selector match PVC labels rather than PV labels? |
+1 to PVC. We have heard in the past that orgs may grant user ability to label/annotate their resources for inclusion to backup. I believe the same use case would come up eventually and it would be handy for user to be able to configure volume policy via label. |
Signed-off-by: Shubham Pampattiwar <[email protected]> add changelog file Signed-off-by: Shubham Pampattiwar <[email protected]> use pvc labels for vp criteria Signed-off-by: Shubham Pampattiwar <[email protected]>
a8b3363
to
bb966c5
Compare
Done, updated the PR. PTAL! |
Just curious: Users can backup using label selector in Backup CR but the idea here is that for some reason, they don't want to specify labels there but still want to limit PVCs based on some labels? |
Yes, even though users can specify label selectors directly in the Backup CR, there are scenarios where they might prefer not to clutter that resource with detailed PVC selection logic. Instead, they can define granular filtering rules within the volume policies themselves. This decouples the backup specification from the operational policies applied to individual volumes. |
To add to @shubham-pampattiwar's reply -- note also that volume policies are not so much about what to include in the backup but how to handle various volumes that are included in the backup. Label selector on the backup CR filters everything not just volumes -- i.e. if you have a label selector on your backup, then you must have matching labels for your pods, PVCs, configmaps, basically everything. The problem that we're trying to solve here is to give users a new way to choose datamover vs. fs-backup or to skip volume data backup for specific volumes that are already included. This overall feature (i.e. "snapshot policy" vs "fs-backup policy" vs "skip policy") already exists, but currently only allows selection based on certain features -- size, provisioner, storage class, etc. A use case here is "I want to use datamover for my postgresql database volumes but fs-backup elsewhere". This is not possible with Velero 1.15 if your PG volume is the same size/type/etc. as your other volumes. So this isn't a new feature in terms of policy options -- this is just a new selection mechanism for an existing feature. |
For me the purpose of volume policies is not to exclude k8s PVC and PV resources (manifests) from being backed up as such, but to be able to say how to back up the content of their associated PVs (snapshot, FSB, ignore). Even if a volume policy ignores the content of a volume, the PVC/PV manifests will be present in the backup (and can be restored later, with empty content). |
Thank you for contributing to Velero!
Please add a summary of your change
Does your change fix a particular issue?
Design for #8256
Please indicate you've done the following:
make new-changelog
) or comment/kind changelog-not-required
on this PR.site/content/docs/main
.