feat: support Volume Group Snapshot #3028

umagnus · 2025-04-16T07:19:17Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

implement volume group snapshots
KEP-3476: Volume Group Snapshot

Which issue(s) this PR fixes:

Fixes #

Requirements:

uses conventional commit messages
includes documentation
adds unit tests
tested upgrade from previous version

Special notes for your reviewer:

Release note:

none

k8s-ci-robot · 2025-04-16T07:19:25Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: umagnus
Once this PR has been reviewed and has the lgtm label, please assign feiskyer for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot · 2025-04-16T07:19:27Z

Hi @umagnus. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

deploy/csi-azuredisk-controller.yaml

pkg/azuredisk/controllerserver.go

andyzhangx · 2025-04-21T02:59:13Z

/ok-to-test

andyzhangx · 2025-04-21T08:06:58Z

pls also fix the sanity test failure:

Summarizing 1 Failure:
  [FAIL] GroupController Service [GroupController VolumeGroupSnapshots] GetVolumeGroupSnapshot [It] should fail when an invalid volume id is used
  /home/prow/go/src/github.com/kubernetes-csi/csi-test/pkg/sanity/util.go:73
Ran 66 of 92 Specs in 167.108 seconds
FAIL! -- 65 Passed | 1 Failed | 1 Pending | 25 Skipped
pkill -f azurediskplugin

andyzhangx

set enableVolumeGroupSnapshot: true in helm chart values

andyzhangx · 2025-04-24T02:46:18Z

charts/latest/azuredisk-csi-driver/templates/csi-snapshot-controller.yaml

            - "--leader-election-namespace={{ .Release.Namespace }}"
            - "--worker-threads=250"
            - "--retry-interval-max=30m"
+            - "--feature-gates=CSIVolumeGroupSnapshot=true"


enableVolumeGroupSnapshot

andyzhangx · 2025-04-24T02:46:40Z

charts/latest/azuredisk-csi-driver/templates/csi-azuredisk-controller.yaml

            - "--extra-create-metadata=true"
            - "--retry-interval-max=30m"
            - "--worker-threads=250"
+            - "--feature-gates=CSIVolumeGroupSnapshot=true"


enableVolumeGroupSnapshot

"--feature-gates=CSIVolumeGroupSnapshot={{ .Values.snapshot.enableVolumeGroupSnapshot }}"

charts/latest/azuredisk-csi-driver/templates/crd-csi-volumegroupsnapshot.yaml

umagnus · 2025-04-24T06:35:07Z

/retest

umagnus · 2025-04-24T08:51:41Z

pkg/azuredisk/controllerserver.go

+	snapshotNames := []string{}
+	for _, sourceVolumeID := range sourceVolumeIDs {
+		groupSnapshotUUID := strings.TrimPrefix(volumeGroupSnapshotName, "groupsnapshot-")
+		snapshotName := fmt.Sprintf("snapshot-%x", sha256.Sum256([]byte(groupSnapshotUUID+sourceVolumeID)))


Here I use groupsnapshotuuid+sourceVolumeID to hash, since in snapshot-controller it use groupsnapshotcontentuuid+sourceVolumeID. But in CSI we can't get groupsnapshotcontent uuid

@xing-yang we are generating snapshotName inside the CreateVolumeGroupSnapshot, do you think this is the right way now? should this snapshotName be provided by CreateVolumeGroupSnapshot request?

I think we should be able to use groupSnapshotUUID in snapshot-controller as well.

kubernetes-csi/external-snapshotter#1294 have a pr for this issue

umagnus · 2025-04-25T06:41:44Z

/retest

pkg/azuredisk/controllerserver.go

andyzhangx · 2025-04-27T01:56:56Z

/hold
hold on until kubernetes-csi/external-snapshotter#1294 is fixed and released with new version

umagnus · 2025-04-27T03:00:01Z

/retest

k8s-ci-robot · 2025-05-07T01:31:19Z

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

k8s-ci-robot · 2025-06-05T01:17:51Z

@umagnus: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-azuredisk-csi-driver-e2e-capz-vmssflex	`36a1a09`	link	true	`/test pull-azuredisk-csi-driver-e2e-capz-vmssflex`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

k8s-triage-robot · 2025-09-03T01:52:34Z

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

Mark this PR as fresh with /remove-lifecycle stale
Close this PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Apr 16, 2025

k8s-ci-robot requested a review from andyzhangx April 16, 2025 07:19

k8s-ci-robot requested a review from ZeroMagic April 16, 2025 07:19

k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 16, 2025

k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Apr 16, 2025

umagnus force-pushed the add_volume_group_snapshots branch from 52b2ef9 to aa48853 Compare April 18, 2025 10:29

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 18, 2025

andyzhangx reviewed Apr 21, 2025

View reviewed changes

deploy/csi-azuredisk-controller.yaml Show resolved Hide resolved

pkg/azuredisk/controllerserver.go Outdated Show resolved Hide resolved

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 21, 2025

umagnus changed the title ~~[draft] implement volume group snapshots~~ [feat] implement volume group snapshots Apr 22, 2025

umagnus force-pushed the add_volume_group_snapshots branch 4 times, most recently from e313841 to ec34393 Compare April 23, 2025 10:25

andyzhangx reviewed Apr 24, 2025

View reviewed changes

andyzhangx changed the title ~~[feat] implement volume group snapshots~~ feat: support Volume Group Snapshot Apr 24, 2025

umagnus force-pushed the add_volume_group_snapshots branch 2 times, most recently from 7a2658a to d549219 Compare April 24, 2025 08:23

umagnus commented Apr 24, 2025

View reviewed changes

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 24, 2025

umagnus force-pushed the add_volume_group_snapshots branch from d549219 to 5323ee8 Compare April 25, 2025 02:06

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 25, 2025

andyzhangx reviewed Apr 25, 2025

View reviewed changes

pkg/azuredisk/controllerserver.go Outdated Show resolved Hide resolved

pkg/azuredisk/controllerserver.go Outdated Show resolved Hide resolved

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 27, 2025

umagnus force-pushed the add_volume_group_snapshots branch from 9ea4642 to 6cd10dc Compare April 27, 2025 02:48

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 27, 2025

add volume group snapshot

36a1a09

umagnus force-pushed the add_volume_group_snapshots branch from 6cd10dc to 36a1a09 Compare May 2, 2025 04:04

k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels May 2, 2025

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 3, 2025

feat: support Volume Group Snapshot #3028

Are you sure you want to change the base?

feat: support Volume Group Snapshot #3028

Uh oh!

Conversation

umagnus commented Apr 16, 2025

Uh oh!

k8s-ci-robot commented Apr 16, 2025

Uh oh!

k8s-ci-robot commented Apr 16, 2025

Uh oh!

Uh oh!

Uh oh!

andyzhangx commented Apr 21, 2025

Uh oh!

andyzhangx commented Apr 21, 2025

Uh oh!

andyzhangx left a comment

Choose a reason for hiding this comment

Uh oh!

andyzhangx Apr 24, 2025

Choose a reason for hiding this comment

Uh oh!

andyzhangx Apr 24, 2025

Choose a reason for hiding this comment

Uh oh!

andyzhangx Apr 24, 2025

Choose a reason for hiding this comment

Uh oh!

umagnus Apr 24, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

umagnus commented Apr 24, 2025

Uh oh!

umagnus Apr 24, 2025

Choose a reason for hiding this comment

Uh oh!

andyzhangx Apr 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xing-yang Apr 25, 2025

Choose a reason for hiding this comment

Uh oh!

umagnus Apr 26, 2025

Choose a reason for hiding this comment

Uh oh!

umagnus commented Apr 25, 2025

Uh oh!

Uh oh!

Uh oh!

andyzhangx commented Apr 27, 2025

Uh oh!

umagnus commented Apr 27, 2025

Uh oh!

k8s-ci-robot commented May 7, 2025

Uh oh!

k8s-ci-robot commented Jun 5, 2025

Uh oh!

k8s-triage-robot commented Sep 3, 2025

Uh oh!

Uh oh!

andyzhangx Apr 25, 2025 •

edited

Loading