Merge pull request #971 from RaunakShah/deletegroupsnapshot-part-3

Update API for pre provisioned group snapshots

client/apis/volumegroupsnapshot/v1alpha1/types.go

@@ -348,16 +348,33 @@ type VolumeGroupSnapshotContentStatus struct {
 // Exactly one of its members must be set.
 // Members in VolumeGroupSnapshotContentSource are immutable.
 type VolumeGroupSnapshotContentSource struct {
-	// PersistentVolumeNames is a list of names of PersistentVolumes to be snapshotted
+	// VolumeHandles is a list of volume handles on the backend to be snapshotted
 	// together. It is specified for dynamic provisioning of the VolumeGroupSnapshot.
 	// This field is immutable.
 	// +optional
-	PersistentVolumeNames []string `json:"persistentVolumeNames,omitempty" protobuf:"bytes,1,opt,name=persistentVolumeNames"`
+	VolumeHandles []string `json:"volumeHandles,omitempty" protobuf:"bytes,1,opt,name=volumeHandles"`
 
+	// GroupSnapshotHandles specifies the CSI "group_snapshot_id" of a pre-existing
+	// group snapshot and a list of CSI "snapshot_id" of pre-existing snapshots
+	// on the underlying storage system for which a Kubernetes object
+	// representation was (or should be) created.
+	// This field is immutable.
+	// +optional
+	GroupSnapshotHandles *GroupSnapshotHandles `json:"groupSnapshotHandles,omitempty" protobuf:"bytes,2,opt,name=groupSnapshotHandles"`
+}
+
+type GroupSnapshotHandles struct {
 	// VolumeGroupSnapshotHandle specifies the CSI "group_snapshot_id" of a pre-existing
 	// group snapshot on the underlying storage system for which a Kubernetes object
 	// representation was (or should be) created.
 	// This field is immutable.
-	// +optional
+	// Required.
-	VolumeGroupSnapshotHandle *string `json:"volumeGroupSnapshotHandle,omitempty" protobuf:"bytes,2,opt,name=volumeGroupSnapshotHandle"`
+	VolumeGroupSnapshotHandle string `json:"volumeGroupSnapshotHandle" protobuf:"bytes,1,opt,name=volumeGroupSnapshotHandle"`
+
+	// VolumeSnapshotHandles is a list of CSI "snapshot_id" of pre-existing
+	// snapshots on the underlying storage system for which Kubernetes objects
+	// representation were (or should be) created.
+	// This field is immutable.
+	// Required.
+	VolumeSnapshotHandles []string `json:"volumeSnapshotHandles" protobuf:"bytes,2,opt,name=volumeSnapshotHandles"`
 }

client/apis/volumegroupsnapshot/v1alpha1/zz_generated.deepcopy.go

@@ -2,7 +2,7 @@
 // +build !ignore_autogenerated
 
 /*
-Copyright 2023 The Kubernetes Authors.
+Copyright 2024 The Kubernetes Authors.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -27,6 +27,27 @@ import (
 	runtime "k8s.io/apimachinery/pkg/runtime"
 )
 
+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *GroupSnapshotHandles) DeepCopyInto(out *GroupSnapshotHandles) {
+	*out = *in
+	if in.VolumeSnapshotHandles != nil {
+		in, out := &in.VolumeSnapshotHandles, &out.VolumeSnapshotHandles
+		*out = make([]string, len(*in))
+		copy(*out, *in)
+	}
+	return
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GroupSnapshotHandles.
+func (in *GroupSnapshotHandles) DeepCopy() *GroupSnapshotHandles {
+	if in == nil {
+		return nil
+	}
+	out := new(GroupSnapshotHandles)
+	in.DeepCopyInto(out)
+	return out
+}
+
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *VolumeGroupSnapshot) DeepCopyInto(out *VolumeGroupSnapshot) {
 	*out = *in
@@ -193,15 +214,15 @@ func (in *VolumeGroupSnapshotContentList) DeepCopyObject() runtime.Object {
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *VolumeGroupSnapshotContentSource) DeepCopyInto(out *VolumeGroupSnapshotContentSource) {
 	*out = *in
-	if in.PersistentVolumeNames != nil {
+	if in.VolumeHandles != nil {
-		in, out := &in.PersistentVolumeNames, &out.PersistentVolumeNames
+		in, out := &in.VolumeHandles, &out.VolumeHandles
 		*out = make([]string, len(*in))
 		copy(*out, *in)
 	}
-	if in.VolumeGroupSnapshotHandle != nil {
+	if in.GroupSnapshotHandles != nil {
-		in, out := &in.VolumeGroupSnapshotHandle, &out.VolumeGroupSnapshotHandle
+		in, out := &in.GroupSnapshotHandles, &out.GroupSnapshotHandles
-		*out = new(string)
+		*out = new(GroupSnapshotHandles)
-		**out = **in
+		(*in).DeepCopyInto(*out)
 	}
 	return
 }

client/apis/volumesnapshot/v1/zz_generated.deepcopy.go

@@ -2,7 +2,7 @@
 // +build !ignore_autogenerated
 
 /*
-Copyright 2023 The Kubernetes Authors.
+Copyright 2024 The Kubernetes Authors.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.

client/config/crd/groupsnapshot.storage.k8s.io_volumegroupsnapshotcontents.yaml

@@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    api-approved.kubernetes.io: "https://github.com/kubernetes-csi/external-snapshotter/pull/814"
+    api-approved.kubernetes.io: "https://github.com/kubernetes-csi/external-snapshotter/pull/971"
     controller-gen.kubebuilder.io/version: v0.12.0
   creationTimestamp: null
   name: volumegroupsnapshotcontents.groupsnapshot.storage.k8s.io
@@ -104,23 +104,42 @@ spec:
                   dynamically provisioned or already exists, and just requires a Kubernetes
                   object representation. This field is immutable after creation. Required.
                 properties:
-                  persistentVolumeNames:
+                  groupSnapshotHandles:
-                    description: PersistentVolumeNames is a list of names of PersistentVolumes
+                    description: GroupSnapshotHandles specifies the CSI "group_snapshot_id"
-                      to be snapshotted together. It is specified for dynamic provisioning
+                      of a pre-existing group snapshot and a list of CSI "snapshot_id"
-                      of the VolumeGroupSnapshot. This field is immutable.
+                      of pre-existing snapshots on the underlying storage system for
+                      which a Kubernetes object representation was (or should be)
+                      created. This field is immutable.
+                    properties:
+                      volumeGroupSnapshotHandle:
+                        description: VolumeGroupSnapshotHandle specifies the CSI "group_snapshot_id"
+                          of a pre-existing group snapshot on the underlying storage
+                          system for which a Kubernetes object representation was
+                          (or should be) created. This field is immutable. Required.
+                        type: string
+                      volumeSnapshotHandles:
+                        description: VolumeSnapshotHandles is a list of CSI "snapshot_id"
+                          of pre-existing snapshots on the underlying storage system
+                          for which Kubernetes objects representation were (or should
+                          be) created. This field is immutable. Required.
                         items:
                           type: string
                         type: array
-                  volumeGroupSnapshotHandle:
+                    required:
-                    description: VolumeGroupSnapshotHandle specifies the CSI "group_snapshot_id"
+                    - volumeGroupSnapshotHandle
-                      of a pre-existing group snapshot on the underlying storage system
+                    - volumeSnapshotHandles
-                      for which a Kubernetes object representation was (or should
+                    type: object
-                      be) created. This field is immutable.
+                  volumeHandles:
+                    description: VolumeHandles is a list of volume handles on the
+                      backend to be snapshotted together. It is specified for dynamic
+                      provisioning of the VolumeGroupSnapshot. This field is immutable.
+                    items:
                       type: string
+                    type: array
                 type: object
                 oneOf:
-                - required: ["persistentVolumeNames"]
+                - required: ["volumeHandles"]
-                - required: ["volumeGroupSnapshotHandle"]
+                - required: ["groupSnapshotHandles"]
               volumeGroupSnapshotClassName:
                 description: VolumeGroupSnapshotClassName is the name of the VolumeGroupSnapshotClass
                   from which this group snapshot was (or will be) created. Note that

client/hack/README.md

@@ -144,7 +144,7 @@ Update the restoreSize property to use type string only:
 
 * Add the VolumeSnapshot namespace to the `additionalPrinterColumns` section. Refer https://github.com/kubernetes-csi/external-snapshotter/pull/535 for more details.
 
-* In `client/config/crd/groupsnapshot.storage.k8s.io_volumegroupsnapshotcontents.yaml `, we need to add the `oneOf` constraint to make sure only one of `persistentVolumeNames` and `volumeGroupSnapshotHandle` is specified in the `source` field of the `spec` of `VolumeGroupSnapshotContent`.
+* In `client/config/crd/groupsnapshot.storage.k8s.io_volumegroupsnapshotcontents.yaml `, we need to add the `oneOf` constraint to make sure only one of `volumeHandles` and `groupSnapshotHandles` is specified in the `source` field of the `spec` of `VolumeGroupSnapshotContent`.
 
 ```bash
               source:
@@ -152,22 +152,41 @@ Update the restoreSize property to use type string only:
                   dynamically provisioned or already exists, and just requires a Kubernetes
                   object representation. This field is immutable after creation. Required.
                 properties:
-                  persistentVolumeNames:
+                  groupSnapshotHandles:
-                    description: PersistentVolumeNames is a list of names of PersistentVolumes
+                    description: GroupSnapshotHandles specifies the CSI "group_snapshot_id"
-                      to be snapshotted together. It is specified for dynamic provisioning
+                      of a pre-existing group snapshot and a list of CSI "snapshot_id"
-                      of the VolumeGroupSnapshot. This field is immutable.
+                      of pre-existing snapshots on the underlying storage system for
+                      which a Kubernetes object representation was (or should be)
+                      created. This field is immutable.
+                    properties:
+                      volumeGroupSnapshotHandle:
+                        description: VolumeGroupSnapshotHandle specifies the CSI "group_snapshot_id"
+                          of a pre-existing group snapshot on the underlying storage
+                          system for which a Kubernetes object representation was
+                          (or should be) created. This field is immutable. Required.
+                        type: string
+                      volumeSnapshotHandles:
+                        description: VolumeSnapshotHandles is a list of CSI "snapshot_id"
+                          of pre-existing snapshots on the underlying storage system
+                          for which Kubernetes objects representation were (or should
+                          be) created. This field is immutable. Required.
                         items:
                           type: string
                         type: array
-                  volumeGroupSnapshotHandle:
+                    required:
-                    description: VolumeGroupSnapshotHandle specifies the CSI "group_snapshot_id"
+                    - volumeGroupSnapshotHandle
-                      of a pre-existing group snapshot on the underlying storage system
+                    - volumeSnapshotHandles
-                      for which a Kubernetes object representation was (or should
+                    type: object
-                      be) created. This field is immutable.
+                  volumeHandles:
+                    description: VolumeHandles is a list of volume handles on the
+                      backend to be snapshotted together. It is specified for dynamic
+                      provisioning of the VolumeGroupSnapshot. This field is immutable.
+                    items:
                       type: string
+                    type: array
                 type: object
                 oneOf:
-                - required: ["persistentVolumeNames"]
+                - required: ["volumeHandles"]
-                - required: ["volumeGroupSnapshotHandle"]
+                - required: ["groupSnapshotHandles"]
               volumeGroupSnapshotClassName:
 ```

pkg/common-controller/groupsnapshot_controller_helper.go

@@ -426,7 +426,7 @@ func (ctrl *csiSnapshotCommonController) syncUnreadyGroupSnapshot(groupSnapshot
 
 	if contentObj != nil {
 		klog.V(5).Infof("Found VolumeGroupSnapshotContent object %s for group snapshot %s", contentObj.Name, uniqueGroupSnapshotName)
-		if contentObj.Spec.Source.VolumeGroupSnapshotHandle != nil {
+		if contentObj.Spec.Source.GroupSnapshotHandles != nil {
 			ctrl.updateGroupSnapshotErrorStatusWithEvent(groupSnapshot, true, v1.EventTypeWarning, "GroupSnapshotHandleSet", fmt.Sprintf("GroupSnapshot handle should not be set in group snapshot content %s for dynamic provisioning", uniqueGroupSnapshotName))
 			return fmt.Errorf("VolumeGroupSnapshotHandle should not be set in the group snapshot content for dynamic provisioning for group snapshot %s", uniqueGroupSnapshotName)
 		}
@@ -483,7 +483,7 @@ func (ctrl *csiSnapshotCommonController) getPreprovisionedGroupSnapshotContentFr
 		return nil, nil
 	}
 	// check whether the content is a pre-provisioned VolumeGroupSnapshotContent
-	if groupSnapshotContent.Spec.Source.VolumeGroupSnapshotHandle == nil {
+	if groupSnapshotContent.Spec.Source.GroupSnapshotHandles == nil {
 		// found a group snapshot content which represents a dynamically provisioned group snapshot
 		// update the group snapshot and return an error
 		ctrl.updateGroupSnapshotErrorStatusWithEvent(groupSnapshot, true, v1.EventTypeWarning, "GroupSnapshotContentMismatch", "VolumeGroupSnapshotContent is dynamically provisioned while expecting a pre-provisioned one")
@@ -682,7 +682,7 @@ func (ctrl *csiSnapshotCommonController) getDynamicallyProvisionedGroupContentFr
 		return nil, nil
 	}
 	// check whether the group snapshot content represents a dynamically provisioned snapshot
-	if groupSnapshotContent.Spec.Source.VolumeGroupSnapshotHandle != nil {
+	if groupSnapshotContent.Spec.Source.GroupSnapshotHandles != nil {
 		ctrl.updateGroupSnapshotErrorStatusWithEvent(groupSnapshot, true, v1.EventTypeWarning, "GroupSnapshotContentMismatch", "VolumeGroupSnapshotContent "+contentName+" is pre-provisioned while expecting a dynamically provisioned one")
 		klog.V(4).Infof("sync group snapshot[%s]: group snapshot content %s is pre-provisioned while expecting a dynamically provisioned one", utils.GroupSnapshotKey(groupSnapshot), contentName)
 		return nil, fmt.Errorf("group snapshot %s expects a dynamically provisioned VolumeGroupSnapshotContent %s but gets a pre-provisioned one", utils.GroupSnapshotKey(groupSnapshot), contentName)
@@ -752,9 +752,9 @@ func (ctrl *csiSnapshotCommonController) createGroupSnapshotContent(groupSnapsho
 	if err != nil {
 		return nil, err
 	}
-	var pvNames []string
+	var volumeHandles []string
 	for _, pv := range volumes {
-		pvNames = append(pvNames, pv.Name)
+		volumeHandles = append(volumeHandles, pv.Spec.CSI.VolumeHandle)
 	}
 
 	groupSnapshotContent := &crdv1alpha1.VolumeGroupSnapshotContent{
@@ -764,7 +764,7 @@ func (ctrl *csiSnapshotCommonController) createGroupSnapshotContent(groupSnapsho
 		Spec: crdv1alpha1.VolumeGroupSnapshotContentSpec{
 			VolumeGroupSnapshotRef: *snapshotRef,
 			Source: crdv1alpha1.VolumeGroupSnapshotContentSource{
-				PersistentVolumeNames: pvNames,
+				VolumeHandles: volumeHandles,
 			},
 			VolumeGroupSnapshotClassName: &(groupSnapshotClass.Name),
 			DeletionPolicy:               groupSnapshotClass.DeletionPolicy,
@@ -846,9 +846,9 @@ func (ctrl *csiSnapshotCommonController) syncGroupSnapshotContent(groupSnapshotC
 	klog.V(5).Infof("syncGroupSnapshotContent[%s]: check if we should add invalid label on group snapshot content", groupSnapshotContent.Name)
 
 	// Keep this check in the controller since the validation webhook may not have been deployed.
-	if (groupSnapshotContent.Spec.Source.VolumeGroupSnapshotHandle == nil && len(groupSnapshotContent.Spec.Source.PersistentVolumeNames) == 0) ||
+	if (groupSnapshotContent.Spec.Source.GroupSnapshotHandles == nil && len(groupSnapshotContent.Spec.Source.VolumeHandles) == 0) ||
-		(groupSnapshotContent.Spec.Source.VolumeGroupSnapshotHandle != nil && len(groupSnapshotContent.Spec.Source.PersistentVolumeNames) > 0) {
+		(groupSnapshotContent.Spec.Source.GroupSnapshotHandles != nil && len(groupSnapshotContent.Spec.Source.VolumeHandles) > 0) {
-		err := fmt.Errorf("Exactly one of VolumeGroupSnapshotHandle and PersistentVolumeNames should be specified")
+		err := fmt.Errorf("Exactly one of GroupSnapshotHandles and VolumeHandles should be specified")
 		klog.Errorf("syncGroupSnapshotContent[%s]: validation error, %s", groupSnapshotContent.Name, err.Error())
 		ctrl.eventRecorder.Event(groupSnapshotContent, v1.EventTypeWarning, "GroupContentValidationError", err.Error())
 		return err
@@ -1161,7 +1161,7 @@ func (ctrl *csiSnapshotCommonController) processGroupSnapshotWithDeletionTimesta
 	// Delete the individual snapshots part of the group snapshot
 	for _, snapshot := range groupSnapshot.Status.VolumeSnapshotRefList {
 		err := ctrl.clientset.SnapshotV1().VolumeSnapshots(snapshot.Namespace).Delete(context.TODO(), snapshot.Name, metav1.DeleteOptions{})
-		if err != nil {
+		if err != nil && !apierrs.IsNotFound(err) {
 			msg := fmt.Sprintf("failed to delete snapshot API object %s/%s part of group snapshot %s: %v", snapshot.Namespace, snapshot.Name, utils.GroupSnapshotKey(groupSnapshot), err)
 			klog.Error(msg)
 			ctrl.eventRecorder.Event(groupSnapshot, v1.EventTypeWarning, "SnapshotDeleteError", msg)

pkg/sidecar-controller/csi_handler.go

@@ -35,7 +35,7 @@ type Handler interface {
 	CreateSnapshot(content *crdv1.VolumeSnapshotContent, parameters map[string]string, snapshotterCredentials map[string]string) (string, string, time.Time, int64, bool, error)
 	DeleteSnapshot(content *crdv1.VolumeSnapshotContent, snapshotterCredentials map[string]string) error
 	GetSnapshotStatus(content *crdv1.VolumeSnapshotContent, snapshotterListCredentials map[string]string) (bool, time.Time, int64, string, error)
-	CreateGroupSnapshot(content *crdv1alpha1.VolumeGroupSnapshotContent, volumeIDs []string, parameters map[string]string, snapshotterCredentials map[string]string) (string, string, []*csi.Snapshot, time.Time, bool, error)
+	CreateGroupSnapshot(content *crdv1alpha1.VolumeGroupSnapshotContent, parameters map[string]string, snapshotterCredentials map[string]string) (string, string, []*csi.Snapshot, time.Time, bool, error)
 	GetGroupSnapshotStatus(content *crdv1alpha1.VolumeGroupSnapshotContent, snapshotterListCredentials map[string]string) (bool, time.Time, error)
 	DeleteGroupSnapshot(content *crdv1alpha1.VolumeGroupSnapshotContent, SnapshotID []string, snapshotterCredentials map[string]string) error
 }
@@ -148,7 +148,7 @@ func makeSnapshotName(prefix, snapshotUID string, snapshotNameUUIDLength int) (s
 	return fmt.Sprintf("%s-%s", prefix, strings.Replace(snapshotUID, "-", "", -1)[0:snapshotNameUUIDLength]), nil
 }
 
-func (handler *csiHandler) CreateGroupSnapshot(content *crdv1alpha1.VolumeGroupSnapshotContent, volumeIDs []string, parameters map[string]string, snapshotterCredentials map[string]string) (string, string, []*csi.Snapshot, time.Time, bool, error) {
+func (handler *csiHandler) CreateGroupSnapshot(content *crdv1alpha1.VolumeGroupSnapshotContent, parameters map[string]string, snapshotterCredentials map[string]string) (string, string, []*csi.Snapshot, time.Time, bool, error) {
 	ctx, cancel := context.WithTimeout(context.Background(), handler.timeout)
 	defer cancel()
 
@@ -156,7 +156,7 @@ func (handler *csiHandler) CreateGroupSnapshot(content *crdv1alpha1.VolumeGroupS
 		return "", "", nil, time.Time{}, false, fmt.Errorf("cannot create group snapshot. Group snapshot content %s not bound to a group snapshot", content.Name)
 	}
 
-	if len(volumeIDs) == 0 {
+	if len(content.Spec.Source.VolumeHandles) == 0 {
 		return "", "", nil, time.Time{}, false, fmt.Errorf("cannot create group snapshot. PVCs to be snapshotted not found in group snapshot content %s", content.Name)
 	}
 
@@ -164,7 +164,7 @@ func (handler *csiHandler) CreateGroupSnapshot(content *crdv1alpha1.VolumeGroupS
 	if err != nil {
 		return "", "", nil, time.Time{}, false, err
 	}
-	return handler.groupSnapshotter.CreateGroupSnapshot(ctx, groupSnapshotName, volumeIDs, parameters, snapshotterCredentials)
+	return handler.groupSnapshotter.CreateGroupSnapshot(ctx, groupSnapshotName, content.Spec.Source.VolumeHandles, parameters, snapshotterCredentials)
 }
 
 func (handler *csiHandler) DeleteGroupSnapshot(content *crdv1alpha1.VolumeGroupSnapshotContent, snapshotIDs []string, snapshotterCredentials map[string]string) error {
@@ -179,8 +179,8 @@ func (handler *csiHandler) DeleteGroupSnapshot(content *crdv1alpha1.VolumeGroupS
 
 	if content.Status != nil && content.Status.VolumeGroupSnapshotHandle != nil {
 		groupSnapshotHandle = *content.Status.VolumeGroupSnapshotHandle
-	} else if content.Spec.Source.VolumeGroupSnapshotHandle != nil {
+	} else if content.Spec.Source.GroupSnapshotHandles != nil {
-		groupSnapshotHandle = *content.Spec.Source.VolumeGroupSnapshotHandle
+		groupSnapshotHandle = content.Spec.Source.GroupSnapshotHandles.VolumeGroupSnapshotHandle
 	} else {
 		return fmt.Errorf("failed to delete group snapshot content %s: groupsnapshotHandle is missing", content.Name)
 	}
@@ -196,8 +196,8 @@ func (handler *csiHandler) GetGroupSnapshotStatus(groupSnapshotContent *crdv1alp
 	var err error
 	if groupSnapshotContent.Status != nil && groupSnapshotContent.Status.VolumeGroupSnapshotHandle != nil {
 		groupSnapshotHandle = *groupSnapshotContent.Status.VolumeGroupSnapshotHandle
-	} else if groupSnapshotContent.Spec.Source.VolumeGroupSnapshotHandle != nil {
+	} else if groupSnapshotContent.Spec.Source.GroupSnapshotHandles != nil {
-		groupSnapshotHandle = *groupSnapshotContent.Spec.Source.VolumeGroupSnapshotHandle
+		groupSnapshotHandle = groupSnapshotContent.Spec.Source.GroupSnapshotHandles.VolumeGroupSnapshotHandle
 	} else {
 		return false, time.Time{}, fmt.Errorf("failed to list group snapshot for group snapshot content %s: groupSnapshotHandle is missing", groupSnapshotContent.Name)
 	}

pkg/sidecar-controller/groupsnapshot_helper.go

@@ -182,7 +182,7 @@ func (ctrl *csiSnapshotSideCarController) syncGroupSnapshotContent(groupSnapshot
 		return ctrl.removeGroupSnapshotContentFinalizer(groupSnapshotContent)
 	}
 
-	if len(groupSnapshotContent.Spec.Source.PersistentVolumeNames) != 0 && groupSnapshotContent.Status == nil {
+	if len(groupSnapshotContent.Spec.Source.VolumeHandles) != 0 && groupSnapshotContent.Status == nil {
 		klog.V(5).Infof("syncGroupSnapshotContent: Call CreateGroupSnapshot for group snapshot content %s", groupSnapshotContent.Name)
 		return ctrl.createGroupSnapshot(groupSnapshotContent)
 	}
@@ -331,7 +331,7 @@ func (ctrl *csiSnapshotSideCarController) shouldDeleteGroupSnapshotContent(group
 	}
 	// 1) shouldDeleteGroupSnapshot returns true if a content is not bound
 	// (VolumeGroupSnapshotRef == "") for pre-provisioned snapshot
-	if groupSnapshotContent.Spec.Source.VolumeGroupSnapshotHandle != nil && groupSnapshotContent.Spec.VolumeGroupSnapshotRef.UID == "" {
+	if groupSnapshotContent.Spec.Source.GroupSnapshotHandles != nil && groupSnapshotContent.Spec.VolumeGroupSnapshotRef.UID == "" {
 		return true
 	}
 
@@ -401,8 +401,7 @@ func (ctrl *csiSnapshotSideCarController) createGroupSnapshotWrapper(groupSnapsh
 		parameters[utils.PrefixedVolumeGroupSnapshotContentNameKey] = groupSnapshotContent.Name
 	}
 
-	volumeIDs, uuidMap, err := ctrl.getGroupSnapshotVolumeIDs(groupSnapshotContent)
+	driverName, groupSnapshotID, snapshots, creationTime, readyToUse, err := ctrl.handler.CreateGroupSnapshot(groupSnapshotContent, parameters, snapshotterCredentials)
-	driverName, groupSnapshotID, snapshots, creationTime, readyToUse, err := ctrl.handler.CreateGroupSnapshot(groupSnapshotContent, volumeIDs, parameters, snapshotterCredentials)
 	if err != nil {
 		// NOTE(xyang): handle create timeout
 		// If it is a final error, remove annotation to indicate
@@ -415,7 +414,7 @@ func (ctrl *csiSnapshotSideCarController) createGroupSnapshotWrapper(groupSnapsh
 			}
 		}
 
-		return groupSnapshotContent, fmt.Errorf("failed to take group snapshot of the volumes %s: %q", groupSnapshotContent.Spec.Source.PersistentVolumeNames, err)
+		return groupSnapshotContent, fmt.Errorf("failed to take group snapshot of the volumes %s: %q", groupSnapshotContent.Spec.Source.VolumeHandles, err)
 	}
 
 	klog.V(5).Infof("Created group snapshot: driver %s, groupSnapshotId %s, creationTime %v, readyToUse %t", driverName, groupSnapshotID, creationTime, readyToUse)
@@ -427,12 +426,8 @@ func (ctrl *csiSnapshotSideCarController) createGroupSnapshotWrapper(groupSnapsh
 	// Create individual snapshots and snapshot contents
 	var snapshotContentNames []string
 	for _, snapshot := range snapshots {
-		uuid, ok := uuidMap[snapshot.SourceVolumeId]
+		volumeSnapshotContentName := GetSnapshotContentNameForVolumeGroupSnapshotContent(string(groupSnapshotContent.UID), snapshot.SourceVolumeId)
-		if !ok {
+		volumeSnapshotName := GetSnapshotNameForVolumeGroupSnapshotContent(string(groupSnapshotContent.UID), snapshot.SourceVolumeId)
-			continue
-		}
-		volumeSnapshotContentName := GetSnapshotContentNameForVolumeGroupSnapshotContent(string(groupSnapshotContent.UID), uuid)
-		volumeSnapshotName := GetSnapshotNameForVolumeGroupSnapshotContent(string(groupSnapshotContent.UID), uuid)
 		volumeSnapshotNamespace := groupSnapshotContent.Spec.VolumeGroupSnapshotRef.Namespace
 		volumeSnapshotContent := &crdv1.VolumeSnapshotContent{
 			ObjectMeta: metav1.ObjectMeta{
@@ -514,7 +509,7 @@ func (ctrl *csiSnapshotSideCarController) getCSIGroupSnapshotInput(groupSnapshot
 		}
 	} else {
 		// If dynamic provisioning, return failure if no group snapshot class
-		if len(groupSnapshotContent.Spec.Source.PersistentVolumeNames) != 0 {
+		if len(groupSnapshotContent.Spec.Source.VolumeHandles) != 0 {
 			klog.Errorf("failed to getCSISnapshotInput %s without a group snapshot class", groupSnapshotContent.Name)
 			return nil, nil, fmt.Errorf("failed to take group snapshot %s without a group snapshot class", groupSnapshotContent.Name)
 		}
@@ -582,25 +577,6 @@ func (ctrl *csiSnapshotSideCarController) setAnnVolumeGroupSnapshotBeingCreated(
 	return groupSnapshotContent, nil
 }
 
-func (ctrl *csiSnapshotSideCarController) getGroupSnapshotVolumeIDs(groupSnapshotContent *crdv1alpha1.VolumeGroupSnapshotContent) ([]string, map[string]string, error) {
-	// TODO: Get add PV lister
-	var volumeIDs []string
-	uuidMap := make(map[string]string)
-	for _, pvName := range groupSnapshotContent.Spec.Source.PersistentVolumeNames {
-		pv, err := ctrl.client.CoreV1().PersistentVolumes().Get(context.TODO(), pvName, metav1.GetOptions{})
-		if err != nil {
-			return nil, nil, err
-		}
-		if pv.Spec.CSI != nil && pv.Spec.CSI.VolumeHandle != "" {
-			volumeIDs = append(volumeIDs, pv.Spec.CSI.VolumeHandle)
-			if pv.Spec.ClaimRef != nil {
-				uuidMap[pv.Spec.CSI.VolumeHandle] = string(pv.Spec.ClaimRef.UID)
-			}
-		}
-	}
-	return volumeIDs, uuidMap, nil
-}
-
 // removeAnnVolumeGroupSnapshotBeingCreated removes the VolumeGroupSnapshotBeingCreated
 // annotation from a groupSnapshotContent if there exists one.
 func (ctrl csiSnapshotSideCarController) removeAnnVolumeGroupSnapshotBeingCreated(groupSnapshotContent *crdv1alpha1.VolumeGroupSnapshotContent) (*crdv1alpha1.VolumeGroupSnapshotContent, error) {
@@ -802,7 +778,7 @@ func (ctrl *csiSnapshotSideCarController) checkandUpdateGroupSnapshotContentStat
 	var groupSnapshotID string
 	var snapshotterListCredentials map[string]string
 
-	if groupSnapshotContent.Spec.Source.VolumeGroupSnapshotHandle != nil {
+	if groupSnapshotContent.Spec.Source.GroupSnapshotHandles != nil {
 		klog.V(5).Infof("checkandUpdateGroupSnapshotContentStatusOperation: call GetGroupSnapshotStatus for group snapshot which is pre-bound to group snapshot content [%s]", groupSnapshotContent.Name)
 
 		if groupSnapshotContent.Spec.VolumeGroupSnapshotClassName != nil {
@@ -832,7 +808,7 @@ func (ctrl *csiSnapshotSideCarController) checkandUpdateGroupSnapshotContentStat
 			return groupSnapshotContent, err
 		}
 		driverName = groupSnapshotContent.Spec.Driver
-		groupSnapshotID = *groupSnapshotContent.Spec.Source.VolumeGroupSnapshotHandle
+		groupSnapshotID = groupSnapshotContent.Spec.Source.GroupSnapshotHandles.VolumeGroupSnapshotHandle
 
 		klog.V(5).Infof("checkandUpdateGroupSnapshotContentStatusOperation: driver %s, groupSnapshotId %s, creationTime %v, readyToUse %t", driverName, groupSnapshotID, creationTime, readyToUse)
 

pkg/sidecar-controller/snapshot_controller_base.go

@@ -316,7 +316,7 @@ func (ctrl *csiSnapshotSideCarController) isDriverMatch(object interface{}) bool
 		return true
 	}
 	if content, ok := object.(*crdv1alpha1.VolumeGroupSnapshotContent); ok {
-		if content.Spec.Source.VolumeGroupSnapshotHandle == nil && len(content.Spec.Source.PersistentVolumeNames) == 0 {
+		if content.Spec.Source.GroupSnapshotHandles == nil && len(content.Spec.Source.VolumeHandles) == 0 {
 			// Skip this group snapshot content if it does not have a valid source
 			return false
 		}

pkg/validation-webhook/groupsnapshot.go

@@ -233,11 +233,11 @@ func checkGroupSnapshotContentImmutableFieldsV1Alpha1(groupSnapcontent, oldGroup
 	source := groupSnapcontent.Spec.Source
 	oldSource := oldGroupSnapcontent.Spec.Source
 
-	if !reflect.DeepEqual(source.VolumeGroupSnapshotHandle, oldSource.VolumeGroupSnapshotHandle) {
+	if !reflect.DeepEqual(source.GroupSnapshotHandles, oldSource.GroupSnapshotHandles) {
-		return fmt.Errorf("Spec.Source.VolumeGroupSnapshotHandle is immutable but was changed from %s to %s", strPtrDereference(oldSource.VolumeGroupSnapshotHandle), strPtrDereference(source.VolumeGroupSnapshotHandle))
+		return fmt.Errorf("Spec.Source.GroupSnapshotHandles is immutable but was changed from %s to %s", oldSource.GroupSnapshotHandles, source.GroupSnapshotHandles)
 	}
-	if !reflect.DeepEqual(source.PersistentVolumeNames, oldSource.PersistentVolumeNames) {
+	if !reflect.DeepEqual(source.VolumeHandles, oldSource.VolumeHandles) {
-		return fmt.Errorf("Spec.Source.PersistentVolumeNames is immutable but was changed from %v to %v", oldSource.PersistentVolumeNames, source.PersistentVolumeNames)
+		return fmt.Errorf("Spec.Source.VolumeHandles is immutable but was changed from %v to %v", oldSource.VolumeHandles, source.VolumeHandles)
 	}
 
 	ref := groupSnapcontent.Spec.VolumeGroupSnapshotRef

pkg/validation-webhook/groupsnapshot_test.go

@@ -248,13 +248,22 @@ func TestAdmitVolumeGroupSnapshotV1Alpha1(t *testing.T) {
 
 func TestAdmitVolumeGroupSnapshotContentV1Alpha1(t *testing.T) {
 	volumeHandle := "volumeHandle1"
-	modifiedField := "modified-field"
+	modifiedRefName := "modified-ref-name"
 	groupSnapshotHandle := "groupsnapshotHandle1"
+	volumeSnapshotHandles := []string{"volumeSnapshotHandle1"}
+	groupSnapshotHandles := &volumegroupsnapshotv1alpha1.GroupSnapshotHandles{
+		VolumeGroupSnapshotHandle: groupSnapshotHandle,
+		VolumeSnapshotHandles:     volumeSnapshotHandles,
+	}
+	modifiedGroupSnapshotHandles := &volumegroupsnapshotv1alpha1.GroupSnapshotHandles{
+		VolumeGroupSnapshotHandle: groupSnapshotHandle,
+		VolumeSnapshotHandles:     append(volumeSnapshotHandles, "volumeSnapshotHandle2"),
+	}
 	volumeGroupSnapshotClassName := "volume-snapshot-class-1"
 	validContent := &volumegroupsnapshotv1alpha1.VolumeGroupSnapshotContent{
 		Spec: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotContentSpec{
 			Source: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotContentSource{
-				VolumeGroupSnapshotHandle: &groupSnapshotHandle,
+				GroupSnapshotHandles: groupSnapshotHandles,
 			},
 			VolumeGroupSnapshotRef: core_v1.ObjectReference{
 				Name:      "group-snapshot-ref",
@@ -266,8 +275,8 @@ func TestAdmitVolumeGroupSnapshotContentV1Alpha1(t *testing.T) {
 	invalidContent := &volumegroupsnapshotv1alpha1.VolumeGroupSnapshotContent{
 		Spec: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotContentSpec{
 			Source: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotContentSource{
-				VolumeGroupSnapshotHandle: &groupSnapshotHandle,
+				GroupSnapshotHandles: groupSnapshotHandles,
-				PersistentVolumeNames:     []string{volumeHandle},
+				VolumeHandles:        []string{volumeHandle},
 			},
 			VolumeGroupSnapshotRef: core_v1.ObjectReference{
 				Name:      "",
@@ -303,7 +312,7 @@ func TestAdmitVolumeGroupSnapshotContentV1Alpha1(t *testing.T) {
 			oldGroupSnapContent: validContent,
 			shouldAdmit:         false,
 			operation:           v1.Update,
-			msg:                 fmt.Sprintf("Spec.Source.PersistentVolumeNames is immutable but was changed from %s to %s", []string{}, []string{volumeHandle}),
+			msg:                 fmt.Sprintf("Spec.Source.VolumeHandles is immutable but was changed from %s to %s", []string{}, []string{volumeHandle}),
 		},
 		{
 			name:                "Update: old is valid and new is valid",
@@ -317,7 +326,7 @@ func TestAdmitVolumeGroupSnapshotContentV1Alpha1(t *testing.T) {
 			groupSnapContent: &volumegroupsnapshotv1alpha1.VolumeGroupSnapshotContent{
 				Spec: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotContentSpec{
 					Source: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotContentSource{
-						VolumeGroupSnapshotHandle: &modifiedField,
+						GroupSnapshotHandles: modifiedGroupSnapshotHandles,
 					},
 					VolumeGroupSnapshotRef: core_v1.ObjectReference{
 						Name:      "snapshot-ref",
@@ -328,17 +337,20 @@ func TestAdmitVolumeGroupSnapshotContentV1Alpha1(t *testing.T) {
 			oldGroupSnapContent: validContent,
 			shouldAdmit:         false,
 			operation:           v1.Update,
-			msg:                 fmt.Sprintf("Spec.Source.VolumeGroupSnapshotHandle is immutable but was changed from %s to %s", groupSnapshotHandle, modifiedField),
+			msg:                 fmt.Sprintf("Spec.Source.GroupSnapshotHandles is immutable but was changed from %s to %s", groupSnapshotHandles, modifiedGroupSnapshotHandles),
 		},
 		{
 			name: "Update: old is valid and new is valid but modifies immutable ref",
 			groupSnapContent: &volumegroupsnapshotv1alpha1.VolumeGroupSnapshotContent{
 				Spec: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotContentSpec{
 					Source: volumegroupsnapshotv1alpha1.VolumeGroupSnapshotContentSource{
-						VolumeGroupSnapshotHandle: &groupSnapshotHandle,
+						GroupSnapshotHandles: &volumegroupsnapshotv1alpha1.GroupSnapshotHandles{
+							VolumeGroupSnapshotHandle: groupSnapshotHandle,
+							VolumeSnapshotHandles:     volumeSnapshotHandles,
+						},
 					},
 					VolumeGroupSnapshotRef: core_v1.ObjectReference{
-						Name:      modifiedField,
+						Name:      modifiedRefName,
 						Namespace: "default-ns",
 					},
 				},
@@ -347,7 +359,7 @@ func TestAdmitVolumeGroupSnapshotContentV1Alpha1(t *testing.T) {
 			shouldAdmit:         false,
 			operation:           v1.Update,
 			msg: fmt.Sprintf("Spec.VolumeGroupSnapshotRef.Name is immutable but was changed from %s to %s",
-				validContent.Spec.VolumeGroupSnapshotRef.Name, modifiedField),
+				validContent.Spec.VolumeGroupSnapshotRef.Name, modifiedRefName),
 		},
 		{
 			name:                "Update: old is invalid and new is valid",
@@ -355,7 +367,7 @@ func TestAdmitVolumeGroupSnapshotContentV1Alpha1(t *testing.T) {
 			oldGroupSnapContent: invalidContent,
 			shouldAdmit:         false,
 			operation:           v1.Update,
-			msg:                 fmt.Sprintf("Spec.Source.PersistentVolumeNames is immutable but was changed from %s to %s", []string{volumeHandle}, []string{}),
+			msg:                 fmt.Sprintf("Spec.Source.VolumeHandles is immutable but was changed from %s to %s", []string{volumeHandle}, []string{}),
 		},
 		{
 			name:                "Update: old is invalid and new is invalid",

vendor/github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumegroupsnapshot/v1alpha1/types.go

@@ -348,16 +348,33 @@ type VolumeGroupSnapshotContentStatus struct {
 // Exactly one of its members must be set.
 // Members in VolumeGroupSnapshotContentSource are immutable.
 type VolumeGroupSnapshotContentSource struct {
-	// PersistentVolumeNames is a list of names of PersistentVolumes to be snapshotted
+	// VolumeHandles is a list of volume handles on the backend to be snapshotted
 	// together. It is specified for dynamic provisioning of the VolumeGroupSnapshot.
 	// This field is immutable.
 	// +optional
-	PersistentVolumeNames []string `json:"persistentVolumeNames,omitempty" protobuf:"bytes,1,opt,name=persistentVolumeNames"`
+	VolumeHandles []string `json:"volumeHandles,omitempty" protobuf:"bytes,1,opt,name=volumeHandles"`
 
+	// GroupSnapshotHandles specifies the CSI "group_snapshot_id" of a pre-existing
+	// group snapshot and a list of CSI "snapshot_id" of pre-existing snapshots
+	// on the underlying storage system for which a Kubernetes object
+	// representation was (or should be) created.
+	// This field is immutable.
+	// +optional
+	GroupSnapshotHandles *GroupSnapshotHandles `json:"groupSnapshotHandles,omitempty" protobuf:"bytes,2,opt,name=groupSnapshotHandles"`
+}
+
+type GroupSnapshotHandles struct {
 	// VolumeGroupSnapshotHandle specifies the CSI "group_snapshot_id" of a pre-existing
 	// group snapshot on the underlying storage system for which a Kubernetes object
 	// representation was (or should be) created.
 	// This field is immutable.
-	// +optional
+	// Required.
-	VolumeGroupSnapshotHandle *string `json:"volumeGroupSnapshotHandle,omitempty" protobuf:"bytes,2,opt,name=volumeGroupSnapshotHandle"`
+	VolumeGroupSnapshotHandle string `json:"volumeGroupSnapshotHandle" protobuf:"bytes,1,opt,name=volumeGroupSnapshotHandle"`
+
+	// VolumeSnapshotHandles is a list of CSI "snapshot_id" of pre-existing
+	// snapshots on the underlying storage system for which Kubernetes objects
+	// representation were (or should be) created.
+	// This field is immutable.
+	// Required.
+	VolumeSnapshotHandles []string `json:"volumeSnapshotHandles" protobuf:"bytes,2,opt,name=volumeSnapshotHandles"`
 }

vendor/github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumegroupsnapshot/v1alpha1/zz_generated.deepcopy.go

@@ -2,7 +2,7 @@
 // +build !ignore_autogenerated
 
 /*
-Copyright 2023 The Kubernetes Authors.
+Copyright 2024 The Kubernetes Authors.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -27,6 +27,27 @@ import (
 	runtime "k8s.io/apimachinery/pkg/runtime"
 )
 
+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *GroupSnapshotHandles) DeepCopyInto(out *GroupSnapshotHandles) {
+	*out = *in
+	if in.VolumeSnapshotHandles != nil {
+		in, out := &in.VolumeSnapshotHandles, &out.VolumeSnapshotHandles
+		*out = make([]string, len(*in))
+		copy(*out, *in)
+	}
+	return
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GroupSnapshotHandles.
+func (in *GroupSnapshotHandles) DeepCopy() *GroupSnapshotHandles {
+	if in == nil {
+		return nil
+	}
+	out := new(GroupSnapshotHandles)
+	in.DeepCopyInto(out)
+	return out
+}
+
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *VolumeGroupSnapshot) DeepCopyInto(out *VolumeGroupSnapshot) {
 	*out = *in
@@ -193,15 +214,15 @@ func (in *VolumeGroupSnapshotContentList) DeepCopyObject() runtime.Object {
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *VolumeGroupSnapshotContentSource) DeepCopyInto(out *VolumeGroupSnapshotContentSource) {
 	*out = *in
-	if in.PersistentVolumeNames != nil {
+	if in.VolumeHandles != nil {
-		in, out := &in.PersistentVolumeNames, &out.PersistentVolumeNames
+		in, out := &in.VolumeHandles, &out.VolumeHandles
 		*out = make([]string, len(*in))
 		copy(*out, *in)
 	}
-	if in.VolumeGroupSnapshotHandle != nil {
+	if in.GroupSnapshotHandles != nil {
-		in, out := &in.VolumeGroupSnapshotHandle, &out.VolumeGroupSnapshotHandle
+		in, out := &in.GroupSnapshotHandles, &out.GroupSnapshotHandles
-		*out = new(string)
+		*out = new(GroupSnapshotHandles)
-		**out = **in
+		(*in).DeepCopyInto(*out)
 	}
 	return
 }

vendor/github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1/zz_generated.deepcopy.go

@@ -2,7 +2,7 @@
 // +build !ignore_autogenerated
 
 /*
-Copyright 2023 The Kubernetes Authors.
+Copyright 2024 The Kubernetes Authors.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.