From e21b21c19ec882df38111fc07c146385124bab19 Mon Sep 17 00:00:00 2001 From: 0xLeo258 Date: Thu, 18 Sep 2025 17:21:25 +0800 Subject: [PATCH] fix 9234: Add safe VolumeSnapshotterCache Signed-off-by: 0xLeo258 --- pkg/backup/backup.go | 2 +- pkg/backup/item_backupper.go | 36 +++------------------- pkg/backup/volume_snapshotter_cache.go | 42 ++++++++++++++++++++++++++ 3 files changed, 48 insertions(+), 32 deletions(-) create mode 100644 pkg/backup/volume_snapshotter_cache.go diff --git a/pkg/backup/backup.go b/pkg/backup/backup.go index a11acd52a..824c44875 100644 --- a/pkg/backup/backup.go +++ b/pkg/backup/backup.go @@ -366,7 +366,7 @@ func (kb *kubernetesBackupper) BackupWithResolvers( discoveryHelper: kb.discoveryHelper, podVolumeBackupper: podVolumeBackupper, podVolumeSnapshotTracker: podvolume.NewTracker(), - volumeSnapshotterGetter: volumeSnapshotterGetter, + volumeSnapshotterCache: NewVolumeSnapshotterCache(volumeSnapshotterGetter), itemHookHandler: &hook.DefaultItemHookHandler{ PodCommandExecutor: kb.podCommandExecutor, }, diff --git a/pkg/backup/item_backupper.go b/pkg/backup/item_backupper.go index b890b23c3..b0e5fadfa 100644 --- a/pkg/backup/item_backupper.go +++ b/pkg/backup/item_backupper.go @@ -70,13 +70,11 @@ type itemBackupper struct { discoveryHelper discovery.Helper podVolumeBackupper podvolume.Backupper podVolumeSnapshotTracker *podvolume.Tracker - volumeSnapshotterGetter VolumeSnapshotterGetter kubernetesBackupper *kubernetesBackupper - - itemHookHandler hook.ItemHookHandler - snapshotLocationVolumeSnapshotters map[string]vsv1.VolumeSnapshotter - hookTracker *hook.HookTracker - volumeHelperImpl volumehelper.VolumeHelper + volumeSnapshotterCache *VolumeSnapshotterCache + itemHookHandler hook.ItemHookHandler + hookTracker *hook.HookTracker + volumeHelperImpl volumehelper.VolumeHelper } type FileForArchive struct { @@ -502,30 +500,6 @@ func (ib *itemBackupper) executeActions( return obj, itemFiles, nil } -// volumeSnapshotter instantiates and initializes a VolumeSnapshotter given a VolumeSnapshotLocation, -// or returns an existing one if one's already been initialized for the location. -func (ib *itemBackupper) volumeSnapshotter(snapshotLocation *velerov1api.VolumeSnapshotLocation) (vsv1.VolumeSnapshotter, error) { - if bs, ok := ib.snapshotLocationVolumeSnapshotters[snapshotLocation.Name]; ok { - return bs, nil - } - - bs, err := ib.volumeSnapshotterGetter.GetVolumeSnapshotter(snapshotLocation.Spec.Provider) - if err != nil { - return nil, err - } - - if err := bs.Init(snapshotLocation.Spec.Config); err != nil { - return nil, err - } - - if ib.snapshotLocationVolumeSnapshotters == nil { - ib.snapshotLocationVolumeSnapshotters = make(map[string]vsv1.VolumeSnapshotter) - } - ib.snapshotLocationVolumeSnapshotters[snapshotLocation.Name] = bs - - return bs, nil -} - // zoneLabelDeprecated is the label that stores availability-zone info // on PVs this is deprecated on Kubernetes >= 1.17.0 // zoneLabel is the label that stores availability-zone info @@ -641,7 +615,7 @@ func (ib *itemBackupper) takePVSnapshot(obj runtime.Unstructured, log logrus.Fie for _, snapshotLocation := range ib.backupRequest.SnapshotLocations { log := log.WithField("volumeSnapshotLocation", snapshotLocation.Name) - bs, err := ib.volumeSnapshotter(snapshotLocation) + bs, err := ib.volumeSnapshotterCache.SetNX(snapshotLocation) if err != nil { log.WithError(err).Error("Error getting volume snapshotter for volume snapshot location") continue diff --git a/pkg/backup/volume_snapshotter_cache.go b/pkg/backup/volume_snapshotter_cache.go new file mode 100644 index 000000000..620ebc337 --- /dev/null +++ b/pkg/backup/volume_snapshotter_cache.go @@ -0,0 +1,42 @@ +package backup + +import ( + "sync" + + velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" + vsv1 "github.com/vmware-tanzu/velero/pkg/plugin/velero/volumesnapshotter/v1" +) + +type VolumeSnapshotterCache struct { + cache map[string]vsv1.VolumeSnapshotter + mutex sync.Mutex + getter VolumeSnapshotterGetter +} + +func NewVolumeSnapshotterCache(getter VolumeSnapshotterGetter) *VolumeSnapshotterCache { + return &VolumeSnapshotterCache{ + cache: make(map[string]vsv1.VolumeSnapshotter), + getter: getter, + } +} + +func (c *VolumeSnapshotterCache) SetNX(location *velerov1api.VolumeSnapshotLocation) (vsv1.VolumeSnapshotter, error) { + c.mutex.Lock() + defer c.mutex.Unlock() + + if snapshotter, exists := c.cache[location.Name]; exists { + return snapshotter, nil + } + + snapshotter, err := c.getter.GetVolumeSnapshotter(location.Spec.Provider) + if err != nil { + return nil, err + } + + if err := snapshotter.Init(location.Spec.Config); err != nil { + return nil, err + } + + c.cache[location.Name] = snapshotter + return snapshotter, nil +}