Skip to content

Commit 5dd9d52

Browse files
sseagokaovilai
sseago
and
kaovilai
committed
Add custom action type to volume policies (velero-io#9540)
* Add custom action type to volume policies Signed-off-by: Scott Seago <sseago@redhat.com> * Update internal/resourcepolicies/resource_policies.go Co-authored-by: Tiger Kaovilai <passawit.kaovilai@gmail.com> Signed-off-by: Scott Seago <sseago@redhat.com> * added "custom" to validation list Signed-off-by: Scott Seago <sseago@redhat.com> * responding to review comments Signed-off-by: Scott Seago <sseago@redhat.com> --------- Signed-off-by: Scott Seago <sseago@redhat.com> Co-authored-by: Tiger Kaovilai <passawit.kaovilai@gmail.com> Signed-off-by: Scott Seago <sseago@redhat.com>
1 parent bf9e1f8 commit 5dd9d52

9 files changed

Lines changed: 215 additions & 32 deletions

File tree

‎changelogs/unreleased/9678-sseago‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Add custom action type to volume policies

‎internal/resourcepolicies/resource_policies.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ const (
4242
FSBackup VolumeActionType = "fs-backup"
4343
// snapshot action can have 3 different meaning based on velero configuration and backup spec - cloud provider based snapshots, local csi snapshots and datamover snapshots
4444
Snapshot VolumeActionType = "snapshot"
45+
// custom action is used to identify a volume that will be handled by an external plugin. Velero will not snapshot or use fs-backup if action=="custom"
46+
Custom VolumeActionType = "custom"
4547
)
4648

4749
// Action defined as one action for a specific way of backup

‎internal/resourcepolicies/volume_resources_validator.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ func decodeStruct(r io.Reader, s any) error {
9090
func (a *Action) validate() error {
9191
// validate Type
9292
valid := false
93-
if a.Type == Skip || a.Type == Snapshot || a.Type == FSBackup {
93+
if a.Type == Skip || a.Type == Snapshot || a.Type == FSBackup || a.Type == Custom {
9494
valid = true
9595
}
9696
if !valid {

‎internal/volumehelper/volume_policy_helper.go‎

Lines changed: 119 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,9 @@ import (
1818
"github.com/vmware-tanzu/velero/pkg/util/boolptr"
1919
kubeutil "github.com/vmware-tanzu/velero/pkg/util/kube"
2020
podvolumeutil "github.com/vmware-tanzu/velero/pkg/util/podvolume"
21+
vhutil "github.com/vmware-tanzu/velero/pkg/util/volumehelper"
2122
)
2223

23-
type VolumeHelper interface {
24-
ShouldPerformSnapshot(obj runtime.Unstructured, groupResource schema.GroupResource) (bool, error)
25-
ShouldPerformFSBackup(volume corev1api.Volume, pod corev1api.Pod) (bool, error)
26-
}
27-
2824
type volumeHelperImpl struct {
2925
volumePolicy *resourcepolicies.Policies
3026
snapshotVolumes *bool
@@ -53,7 +49,7 @@ func NewVolumeHelperImpl(
5349
client crclient.Client,
5450
defaultVolumesToFSBackup bool,
5551
backupExcludePVC bool,
56-
) VolumeHelper {
52+
) vhutil.VolumeHelper {
5753
// Pass nil namespaces - no cache will be built, so this never fails.
5854
// This is used by plugins that don't need the cache optimization.
5955
vh, _ := NewVolumeHelperImplWithNamespaces(
@@ -81,7 +77,7 @@ func NewVolumeHelperImplWithNamespaces(
8177
defaultVolumesToFSBackup bool,
8278
backupExcludePVC bool,
8379
namespaces []string,
84-
) (VolumeHelper, error) {
80+
) (vhutil.VolumeHelper, error) {
8581
var pvcPodCache *podvolumeutil.PVCPodCache
8682
if len(namespaces) > 0 {
8783
pvcPodCache = podvolumeutil.NewPVCPodCache()
@@ -110,7 +106,7 @@ func NewVolumeHelperImplWithCache(
110106
client crclient.Client,
111107
logger logrus.FieldLogger,
112108
pvcPodCache *podvolumeutil.PVCPodCache,
113-
) (VolumeHelper, error) {
109+
) (vhutil.VolumeHelper, error) {
114110
resourcePolicies, err := resourcepolicies.GetResourcePoliciesFromBackup(backup, client, logger)
115111
if err != nil {
116112
return nil, errors.Wrap(err, "failed to get volume policies from backup")
@@ -319,6 +315,121 @@ func (v volumeHelperImpl) shouldPerformFSBackupLegacy(
319315
}
320316
}
321317

318+
func (v *volumeHelperImpl) ShouldPerformCustomAction(obj runtime.Unstructured, groupResource schema.GroupResource, matchParams map[string]any) (bool, error) {
319+
// check if volume policy exists and also check if the object(pv/pvc) fits a volume policy criteria and see if the associated action is custom with the provided param values
320+
pvc := new(corev1api.PersistentVolumeClaim)
321+
pv := new(corev1api.PersistentVolume)
322+
var err error
323+
324+
var pvNotFoundErr error
325+
if groupResource == kuberesource.PersistentVolumeClaims {
326+
if err = runtime.DefaultUnstructuredConverter.FromUnstructured(obj.UnstructuredContent(), &pvc); err != nil {
327+
v.logger.WithError(err).Error("fail to convert unstructured into PVC")
328+
return false, err
329+
}
330+
331+
pv, err = kubeutil.GetPVForPVC(pvc, v.client)
332+
if err != nil {
333+
// Any error means PV not available - save to return later if no policy matches
334+
v.logger.Debugf("PV not found for PVC %s: %v", pvc.Namespace+"/"+pvc.Name, err)
335+
pvNotFoundErr = err
336+
pv = nil
337+
}
338+
}
339+
340+
if groupResource == kuberesource.PersistentVolumes {
341+
if err = runtime.DefaultUnstructuredConverter.FromUnstructured(obj.UnstructuredContent(), &pv); err != nil {
342+
v.logger.WithError(err).Error("fail to convert unstructured into PV")
343+
return false, err
344+
}
345+
}
346+
347+
if v.volumePolicy != nil {
348+
vfd := resourcepolicies.NewVolumeFilterData(pv, nil, pvc)
349+
action, err := v.volumePolicy.GetMatchAction(vfd)
350+
if err != nil {
351+
v.logger.WithError(err).Errorf("fail to get VolumePolicy match action for %+v", vfd)
352+
return false, err
353+
}
354+
355+
// If there is a match action, and the action type is custom, return true
356+
// if the provided parameters match as well, else return false.
357+
// If there is no match action, also return false
358+
if action != nil {
359+
if action.Type == resourcepolicies.Custom {
360+
for k, requiredValue := range matchParams {
361+
if actionValue, ok := action.Parameters[k]; !ok || actionValue != requiredValue {
362+
v.logger.Infof("Skipping custom action for %+v as value for parameter %s is %s rather than the required %s", vfd, k, actionValue, requiredValue)
363+
return false, nil
364+
}
365+
}
366+
v.logger.Infof("performing custom action for %+v", vfd)
367+
return true, nil
368+
} else {
369+
v.logger.Infof("Skipping custom action for %+v as the action type is %s", vfd, action.Type)
370+
return false, nil
371+
}
372+
}
373+
}
374+
// If resource is PVC, and PV is nil (e.g., Pending/Lost PVC with no matching policy), return the original error
375+
// Don't error out on no PV, just return false
376+
if groupResource == kuberesource.PersistentVolumeClaims && pv == nil && pvNotFoundErr != nil {
377+
v.logger.WithError(pvNotFoundErr).Warnf("fail to get PV for PVC %s", pvc.Namespace+"/"+pvc.Name)
378+
return false, nil
379+
}
380+
381+
v.logger.Infof("skipping custom action for pv %s due to no matching volume policy", pv.Name)
382+
return false, nil
383+
}
384+
385+
// returns false if no matching action found. Returns true with the action name and Parameters map if there is a matching policy
386+
func (v *volumeHelperImpl) GetActionParameters(obj runtime.Unstructured, groupResource schema.GroupResource) (bool, string, map[string]any, error) {
387+
// if volume policy exists, return action parameters.
388+
pvc := new(corev1api.PersistentVolumeClaim)
389+
pv := new(corev1api.PersistentVolume)
390+
var err error
391+
392+
if groupResource == kuberesource.PersistentVolumeClaims {
393+
if err = runtime.DefaultUnstructuredConverter.FromUnstructured(obj.UnstructuredContent(), &pvc); err != nil {
394+
v.logger.WithError(err).Error("fail to convert unstructured into PVC")
395+
return false, "", nil, err
396+
}
397+
398+
pv, err = kubeutil.GetPVForPVC(pvc, v.client)
399+
if err != nil {
400+
v.logger.WithError(err).Warnf("failed to get PV for PVC %s", pvc.Namespace+"/"+pvc.Name)
401+
return false, "", nil, nil
402+
}
403+
}
404+
405+
if groupResource == kuberesource.PersistentVolumes {
406+
if err = runtime.DefaultUnstructuredConverter.FromUnstructured(obj.UnstructuredContent(), &pv); err != nil {
407+
v.logger.WithError(err).Error("fail to convert unstructured into PV")
408+
return false, "", nil, err
409+
}
410+
}
411+
412+
if v.volumePolicy != nil {
413+
vfd := resourcepolicies.NewVolumeFilterData(pv, nil, pvc)
414+
action, err := v.volumePolicy.GetMatchAction(vfd)
415+
if err != nil {
416+
v.logger.WithError(err).Errorf("fail to get VolumePolicy match action for PV %s", pv.Name)
417+
return false, "", nil, err
418+
}
419+
420+
// If there is a match action, and the action type is custom, return true
421+
// if the provided parameters match as well, else return false.
422+
// If there is no match action, also return false
423+
if action != nil {
424+
v.logger.Infof("found matching action for pv %s, returning parameters", pv.Name)
425+
return true, string(action.Type), action.Parameters, nil
426+
}
427+
}
428+
429+
v.logger.Infof("no matching volume policy found for pv %s, no parameters to return", pv.Name)
430+
return false, "", nil, nil
431+
}
432+
322433
func (v *volumeHelperImpl) shouldIncludeVolumeInBackup(vol corev1api.Volume) bool {
323434
includeVolumeInBackup := true
324435
// cannot backup hostpath volumes as they are not mounted into /var/lib/kubelet/pods

‎pkg/backup/actions/csi/pvc_action.go‎

Lines changed: 8 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@ import (
4444

4545
"k8s.io/apimachinery/pkg/api/resource"
4646

47-
internalvolumehelper "github.com/vmware-tanzu/velero/internal/volumehelper"
4847
velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
4948
velerov2alpha1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v2alpha1"
5049
veleroclient "github.com/vmware-tanzu/velero/pkg/client"
@@ -59,6 +58,7 @@ import (
5958
"github.com/vmware-tanzu/velero/pkg/util/csi"
6059
kubeutil "github.com/vmware-tanzu/velero/pkg/util/kube"
6160
podvolumeutil "github.com/vmware-tanzu/velero/pkg/util/podvolume"
61+
vhutil "github.com/vmware-tanzu/velero/pkg/util/volumehelper"
6262
)
6363

6464
// TODO: Replace hardcoded VolumeSnapshot finalizer strings with constants from
@@ -128,9 +128,9 @@ func (p *pvcBackupItemAction) ensurePVCPodCacheForNamespace(ctx context.Context,
128128

129129
// getVolumeHelperWithCache creates a VolumeHelper using the pre-built PVC-to-Pod cache.
130130
// The cache should be ensured for the relevant namespace(s) before calling this.
131-
func (p *pvcBackupItemAction) getVolumeHelperWithCache(backup *velerov1api.Backup) (internalvolumehelper.VolumeHelper, error) {
131+
func (p *pvcBackupItemAction) getVolumeHelperWithCache(backup *velerov1api.Backup) (vhutil.VolumeHelper, error) {
132132
// Create VolumeHelper with our lazy-built cache
133-
vh, err := internalvolumehelper.NewVolumeHelperImplWithCache(
133+
vh, err := volumehelper.NewVolumeHelperWithCache(
134134
*backup,
135135
p.crClient,
136136
p.log,
@@ -149,7 +149,7 @@ func (p *pvcBackupItemAction) getVolumeHelperWithCache(backup *velerov1api.Backu
149149
// Since plugin instances are unique per backup (created via newPluginManager and
150150
// cleaned up via CleanupClients at backup completion), we can safely cache this.
151151
// See issue #9179 and PR #9226 for details.
152-
func (p *pvcBackupItemAction) getOrCreateVolumeHelper(backup *velerov1api.Backup) (internalvolumehelper.VolumeHelper, error) {
152+
func (p *pvcBackupItemAction) getOrCreateVolumeHelper(backup *velerov1api.Backup) (vhutil.VolumeHelper, error) {
153153
// Initialize the PVC-to-Pod cache if needed
154154
if p.pvcPodCache == nil {
155155
p.pvcPodCache = podvolumeutil.NewPVCPodCache()
@@ -322,13 +322,9 @@ func (p *pvcBackupItemAction) Execute(
322322
return nil, nil, "", nil, err
323323
}
324324

325-
shouldSnapshot, err := volumehelper.ShouldPerformSnapshotWithVolumeHelper(
325+
shouldSnapshot, err := vh.ShouldPerformSnapshot(
326326
item,
327327
kuberesource.PersistentVolumeClaims,
328-
*backup,
329-
p.crClient,
330-
p.log,
331-
vh,
332328
)
333329
if err != nil {
334330
return nil, nil, "", nil, err
@@ -708,7 +704,7 @@ func (p *pvcBackupItemAction) getVolumeSnapshotReference(
708704
}
709705

710706
// Filter PVCs by volume policy
711-
filteredPVCs, err := p.filterPVCsByVolumePolicy(groupedPVCs, backup, vh)
707+
filteredPVCs, err := p.filterPVCsByVolumePolicy(groupedPVCs, vh)
712708
if err != nil {
713709
return nil, errors.Wrapf(err, "failed to filter PVCs by volume policy for VolumeGroupSnapshot group %q", group)
714710
}
@@ -844,8 +840,7 @@ func (p *pvcBackupItemAction) listGroupedPVCs(ctx context.Context, namespace, la
844840

845841
func (p *pvcBackupItemAction) filterPVCsByVolumePolicy(
846842
pvcs []corev1api.PersistentVolumeClaim,
847-
backup *velerov1api.Backup,
848-
vh internalvolumehelper.VolumeHelper,
843+
vh vhutil.VolumeHelper,
849844
) ([]corev1api.PersistentVolumeClaim, error) {
850845
var filteredPVCs []corev1api.PersistentVolumeClaim
851846

@@ -859,13 +854,9 @@ func (p *pvcBackupItemAction) filterPVCsByVolumePolicy(
859854

860855
// Check if this PVC should be snapshotted according to volume policies
861856
// Uses the cached VolumeHelper for better performance with many PVCs/pods
862-
shouldSnapshot, err := volumehelper.ShouldPerformSnapshotWithVolumeHelper(
857+
shouldSnapshot, err := vh.ShouldPerformSnapshot(
863858
unstructuredPVC,
864859
kuberesource.PersistentVolumeClaims,
865-
*backup,
866-
p.crClient,
867-
p.log,
868-
vh,
869860
)
870861
if err != nil {
871862
return nil, errors.Wrapf(err, "failed to check volume policy for PVC %s/%s", pvc.Namespace, pvc.Name)

‎pkg/backup/actions/csi/pvc_action_test.go‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -842,9 +842,13 @@ volumePolicies:
842842
crClient: client,
843843
}
844844

845-
// Pass nil for VolumeHelper in tests - it will fall back to creating a new one per call
846-
// This is the expected behavior for testing and third-party plugins
847-
result, err := action.filterPVCsByVolumePolicy(tt.pvcs, backup, nil)
845+
// Create a VolumeHelper using the same method the plugin would use
846+
vh, err := action.getOrCreateVolumeHelper(backup)
847+
require.NoError(t, err)
848+
require.NotNil(t, vh)
849+
850+
// Test with the pre-created VolumeHelper
851+
result, err := action.filterPVCsByVolumePolicy(tt.pvcs, vh)
848852
if tt.expectError {
849853
require.Error(t, err)
850854
} else {
@@ -959,7 +963,7 @@ volumePolicies:
959963
require.NotNil(t, vh)
960964

961965
// Test with the pre-created VolumeHelper (non-nil path)
962-
result, err := action.filterPVCsByVolumePolicy(pvcs, backup, vh)
966+
result, err := action.filterPVCsByVolumePolicy(pvcs, vh)
963967
require.NoError(t, err)
964968

965969
// Should filter out the NFS PVC, leaving only the CSI PVC

‎pkg/backup/item_backupper.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ import (
4040
"github.com/vmware-tanzu/velero/internal/hook"
4141
"github.com/vmware-tanzu/velero/internal/resourcepolicies"
4242
"github.com/vmware-tanzu/velero/internal/volume"
43-
"github.com/vmware-tanzu/velero/internal/volumehelper"
4443
velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
4544
"github.com/vmware-tanzu/velero/pkg/archive"
4645
"github.com/vmware-tanzu/velero/pkg/client"
@@ -54,6 +53,7 @@ import (
5453
"github.com/vmware-tanzu/velero/pkg/podvolume"
5554
"github.com/vmware-tanzu/velero/pkg/util/boolptr"
5655
csiutil "github.com/vmware-tanzu/velero/pkg/util/csi"
56+
"github.com/vmware-tanzu/velero/pkg/util/volumehelper"
5757
)
5858

5959
const (

‎pkg/plugin/utils/volumehelper/volume_policy_helper.go‎

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ import (
2626
"github.com/vmware-tanzu/velero/internal/volumehelper"
2727
velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
2828
"github.com/vmware-tanzu/velero/pkg/util/boolptr"
29+
podvolumeutil "github.com/vmware-tanzu/velero/pkg/util/podvolume"
30+
vhutil "github.com/vmware-tanzu/velero/pkg/util/volumehelper"
2931
)
3032

3133
// ShouldPerformSnapshotWithBackup is used for third-party plugins.
@@ -66,7 +68,7 @@ func ShouldPerformSnapshotWithVolumeHelper(
6668
backup velerov1api.Backup,
6769
crClient crclient.Client,
6870
logger logrus.FieldLogger,
69-
vh volumehelper.VolumeHelper,
71+
vh vhutil.VolumeHelper,
7072
) (bool, error) {
7173
// If a VolumeHelper is provided, use it directly
7274
if vh != nil {
@@ -95,3 +97,45 @@ func ShouldPerformSnapshotWithVolumeHelper(
9597

9698
return volumeHelperImpl.ShouldPerformSnapshot(unstructured, groupResource)
9799
}
100+
101+
// NewVolumeHelperWithNamespaces creates a VolumeHelper with a PVC-to-Pod cache for improved performance.
102+
// The cache is built internally from the provided namespaces list.
103+
// This avoids O(N*M) complexity when there are many PVCs and pods.
104+
// See issue #9179 for details.
105+
// Returns an error if cache building fails - callers should not proceed with backup in this case.
106+
func NewVolumeHelperWithNamespaces(
107+
volumePolicy *resourcepolicies.Policies,
108+
snapshotVolumes *bool,
109+
logger logrus.FieldLogger,
110+
client crclient.Client,
111+
defaultVolumesToFSBackup bool,
112+
backupExcludePVC bool,
113+
namespaces []string,
114+
) (vhutil.VolumeHelper, error) {
115+
return volumehelper.NewVolumeHelperImplWithNamespaces(
116+
volumePolicy,
117+
snapshotVolumes,
118+
logger,
119+
client,
120+
defaultVolumesToFSBackup,
121+
backupExcludePVC,
122+
namespaces,
123+
)
124+
}
125+
126+
// NewVolumeHelperWithCache creates a VolumeHelper using an externally managed PVC-to-Pod cache.
127+
// This is used by plugins that build the cache lazily per-namespace (following the pattern from PR #9226).
128+
// The cache can be nil, in which case PVC-to-Pod lookups will fall back to direct API calls.
129+
func NewVolumeHelperWithCache(
130+
backup velerov1api.Backup,
131+
client crclient.Client,
132+
logger logrus.FieldLogger,
133+
pvcPodCache *podvolumeutil.PVCPodCache,
134+
) (vhutil.VolumeHelper, error) {
135+
return volumehelper.NewVolumeHelperImplWithCache(
136+
backup,
137+
client,
138+
logger,
139+
pvcPodCache,
140+
)
141+
}

0 commit comments

Comments
 (0)