From e21b21c19ec882df38111fc07c146385124bab19 Mon Sep 17 00:00:00 2001 From: 0xLeo258 Date: Thu, 18 Sep 2025 17:21:25 +0800 Subject: [PATCH 1/3] 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 +} From 25de1bb3b6bf646e233ad401ee498c7f6d92287a Mon Sep 17 00:00:00 2001 From: 0xLeo258 Date: Thu, 18 Sep 2025 17:36:07 +0800 Subject: [PATCH 2/3] add changelog Signed-off-by: 0xLeo258 --- changelogs/unreleased/9281-0xLeo258 | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelogs/unreleased/9281-0xLeo258 diff --git a/changelogs/unreleased/9281-0xLeo258 b/changelogs/unreleased/9281-0xLeo258 new file mode 100644 index 000000000..eb5bf3f5d --- /dev/null +++ b/changelogs/unreleased/9281-0xLeo258 @@ -0,0 +1 @@ +Implement concurrency control for cache of native VolumeSnapshotter plugin. From 4847eeaf62c9359038eb4edc26b6be5cd3c61f31 Mon Sep 17 00:00:00 2001 From: Xun Jiang Date: Thu, 11 Sep 2025 16:58:33 +0800 Subject: [PATCH 3/3] Use different go version check logic for main and other branches. main branch will read go version from go.mod's go primitive, and only keep major and minor version, because we want the actions to use the lastest patch version automatically, even the go.mod specify version like 1.24.0. release branch can read the go version from go.mod file by setup-go action's own logic. Refactor the get Go version to reusable workflow. Signed-off-by: Xun Jiang --- .github/workflows/e2e-test-kind.yaml | 19 +++++++++++---- .github/workflows/get-go-version.yaml | 33 +++++++++++++++++++++++++++ .github/workflows/pr-ci-check.yml | 12 ++++++++-- .github/workflows/pr-linter-check.yml | 12 ++++++++-- .github/workflows/push.yml | 11 +++++++-- 5 files changed, 77 insertions(+), 10 deletions(-) create mode 100644 .github/workflows/get-go-version.yaml diff --git a/.github/workflows/e2e-test-kind.yaml b/.github/workflows/e2e-test-kind.yaml index df092d5d3..47979a27c 100644 --- a/.github/workflows/e2e-test-kind.yaml +++ b/.github/workflows/e2e-test-kind.yaml @@ -8,18 +8,26 @@ on: - "design/**" - "**/*.md" jobs: + get-go-version: + uses: ./.github/workflows/get-go-version.yaml + with: + ref: ${{ github.event.pull_request.base.ref }} + # Build the Velero CLI and image once for all Kubernetes versions, and cache it so the fan-out workers can get it. build: runs-on: ubuntu-latest + needs: get-go-version outputs: minio-dockerfile-sha: ${{ steps.minio-version.outputs.dockerfile_sha }} steps: - name: Check out the code uses: actions/checkout@v5 - - name: Set up Go + + - name: Set up Go version uses: actions/setup-go@v6 with: - go-version-file: 'go.mod' + go-version: ${{ needs.get-go-version.outputs.version }} + # Look for a CLI that's made for this PR - name: Fetch built CLI id: cli-cache @@ -97,6 +105,7 @@ jobs: needs: - build - setup-test-matrix + - get-go-version runs-on: ubuntu-latest strategy: matrix: ${{fromJson(needs.setup-test-matrix.outputs.matrix)}} @@ -104,10 +113,12 @@ jobs: steps: - name: Check out the code uses: actions/checkout@v5 - - name: Set up Go + + - name: Set up Go version uses: actions/setup-go@v6 with: - go-version-file: 'go.mod' + go-version: ${{ needs.get-go-version.outputs.version }} + # Fetch the pre-built MinIO image from the build job - name: Fetch built MinIO Image uses: actions/cache@v4 diff --git a/.github/workflows/get-go-version.yaml b/.github/workflows/get-go-version.yaml new file mode 100644 index 000000000..3bc3f8e53 --- /dev/null +++ b/.github/workflows/get-go-version.yaml @@ -0,0 +1,33 @@ +on: + workflow_call: + inputs: + ref: + description: "The target branch's ref" + required: true + type: string + outputs: + version: + description: "The expected Go version" + value: ${{ jobs.extract.outputs.version }} + +jobs: + extract: + runs-on: ubuntu-latest + outputs: + version: ${{ steps.pick-version.outputs.version }} + steps: + - name: Check out the code + uses: actions/checkout@v5 + + - id: pick-version + run: | + if [ "${{ inputs.ref }}" == "main" ]; then + version=$(grep '^go ' go.mod | awk '{print $2}' | cut -d. -f1-2) + else + goDirectiveVersion=$(grep '^go ' go.mod | awk '{print $2}') + toolChainVersion=$(grep '^toolchain ' go.mod | awk '{print $2}') + version=$(printf "%s\n%s\n" "$goDirectiveVersion" "$toolChainVersion" | sort -V | tail -n1) + fi + + echo "version=$version" + echo "version=$version" >> $GITHUB_OUTPUT diff --git a/.github/workflows/pr-ci-check.yml b/.github/workflows/pr-ci-check.yml index 84c63047f..c97f216b4 100644 --- a/.github/workflows/pr-ci-check.yml +++ b/.github/workflows/pr-ci-check.yml @@ -1,18 +1,26 @@ name: Pull Request CI Check on: [pull_request] jobs: + get-go-version: + uses: ./.github/workflows/get-go-version.yaml + with: + ref: ${{ github.event.pull_request.base.ref }} + build: name: Run CI + needs: get-go-version runs-on: ubuntu-latest strategy: fail-fast: false steps: - name: Check out the code uses: actions/checkout@v5 - - name: Set up Go + + - name: Set up Go version uses: actions/setup-go@v6 with: - go-version-file: 'go.mod' + go-version: ${{ needs.get-go-version.outputs.version }} + - name: Make ci run: make ci - name: Upload test coverage diff --git a/.github/workflows/pr-linter-check.yml b/.github/workflows/pr-linter-check.yml index 586acc5fd..81a2bdd64 100644 --- a/.github/workflows/pr-linter-check.yml +++ b/.github/workflows/pr-linter-check.yml @@ -7,16 +7,24 @@ on: - "design/**" - "**/*.md" jobs: + get-go-version: + uses: ./.github/workflows/get-go-version.yaml + with: + ref: ${{ github.event.pull_request.base.ref }} + build: name: Run Linter Check runs-on: ubuntu-latest + needs: get-go-version steps: - name: Check out the code uses: actions/checkout@v5 - - name: Set up Go + + - name: Set up Go version uses: actions/setup-go@v6 with: - go-version-file: 'go.mod' + go-version: ${{ needs.get-go-version.outputs.version }} + - name: Linter check uses: golangci/golangci-lint-action@v8 with: diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml index 99f42a498..8e1bc1219 100644 --- a/.github/workflows/push.yml +++ b/.github/workflows/push.yml @@ -9,17 +9,24 @@ on: - '*' jobs: + get-go-version: + uses: ./.github/workflows/get-go-version.yaml + with: + ref: ${ github.ref } build: name: Build runs-on: ubuntu-latest + needs: get-go-version steps: - name: Check out the code uses: actions/checkout@v5 - - name: Set up Go + + - name: Set up Go version uses: actions/setup-go@v6 with: - go-version-file: 'go.mod' + go-version: ${{ needs.get-go-version.outputs.version }} + - name: Set up QEMU id: qemu uses: docker/setup-qemu-action@v3