[WIP] add DeleteVolumeGroupSnapshot API to delete group snapshot

pkg/group_snapshotter/group_snapshotter.go

@@ -18,11 +18,12 @@ package group_snapshotter
 
 import (
 	"context"
+	"time"
+
 	"github.com/container-storage-interface/spec/lib/go/csi"
 	csirpc "github.com/kubernetes-csi/csi-lib-utils/rpc"
 	"google.golang.org/grpc"
 	klog "k8s.io/klog/v2"
-	"time"
 )
 
 // GroupSnapshotter implements CreateGroupSnapshot/DeleteGroupSnapshot operations against a CSI driver.
@@ -31,7 +32,7 @@ type GroupSnapshotter interface {
 	CreateGroupSnapshot(ctx context.Context, groupSnapshotName string, volumeIDs []string, parameters map[string]string, snapshotterCredentials map[string]string) (driverName string, groupSnapshotId string, snapshots []*csi.Snapshot, timestamp time.Time, readyToUse bool, err error)
 
 	// DeleteGroupSnapshot deletes a group snapshot of multiple volumes
-	DeleteGroupSnapshot(ctx context.Context, groupSnapshotID string, snapshotterCredentials map[string]string) (err error)
+	DeleteGroupSnapshot(ctx context.Context, groupSnapshotID string, snapshotIDs []string, snapshotterCredentials map[string]string) (err error)
 
 	// GetGroupSnapshotStatus returns if a group snapshot is ready to use, its creation time, etc
 	GetGroupSnapshotStatus(ctx context.Context, groupSnapshotID string, snapshotterListCredentials map[string]string) (bool, time.Time, error)
@@ -74,8 +75,20 @@ func (gs *groupSnapshot) CreateGroupSnapshot(ctx context.Context, groupSnapshotN
 
 }
 
-func (gs *groupSnapshot) DeleteGroupSnapshot(ctx context.Context, groupSnapshotID string, snapshotterCredentials map[string]string) error {
-	// TODO: Implement DeleteGroupSnapshot
+func (gs *groupSnapshot) DeleteGroupSnapshot(ctx context.Context, groupSnapshotID string, snapshotIds []string, snapshotterCredentials map[string]string) error {
+	client := csi.NewGroupControllerClient(gs.conn)
+
+	req := csi.DeleteVolumeGroupSnapshotRequest{
+		Secrets:         snapshotterCredentials,
+		GroupSnapshotId: groupSnapshotID,
+		SnapshotIds:     snapshotIds,
+	}
+
+	_, err := client.DeleteVolumeGroupSnapshot(ctx, &req)
+	if err != nil {
+		return err
+	}
+
 	return nil
 }
 

pkg/sidecar-controller/csi_handler.go

@@ -19,11 +19,12 @@ package sidecar_controller
 import (
 	"context"
 	"fmt"
+	"strings"
+	"time"
+
 	"github.com/container-storage-interface/spec/lib/go/csi"
 	crdv1alpha1 "github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumegroupsnapshot/v1alpha1"
 	"github.com/kubernetes-csi/external-snapshotter/v6/pkg/group_snapshotter"
-	"strings"
-	"time"
 
 	crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1"
 	"github.com/kubernetes-csi/external-snapshotter/v6/pkg/snapshotter"
@@ -36,6 +37,7 @@ type Handler interface {
 	GetSnapshotStatus(content *crdv1.VolumeSnapshotContent, snapshotterListCredentials map[string]string) (bool, time.Time, int64, error)
 	CreateGroupSnapshot(content *crdv1alpha1.VolumeGroupSnapshotContent, volumeIDs []string, 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, SanpshotID []string, snapshotterCredentials map[string]string) error
 }
 
 // csiHandler is a handler that calls CSI to create/delete volume snapshot.
@@ -165,6 +167,30 @@ func (handler *csiHandler) CreateGroupSnapshot(content *crdv1alpha1.VolumeGroupS
 	return handler.groupSnapshotter.CreateGroupSnapshot(ctx, groupSnapshotName, volumeIDs, parameters, snapshotterCredentials)
 }
 
+func (handler *csiHandler) DeleteGroupSnapshot(content *crdv1alpha1.VolumeGroupSnapshotContent, snapshotIDs []string, snapshotterCredentials map[string]string) error {
+	ctx, cancel := context.WithTimeout(context.Background(), handler.timeout)
+	defer cancel()
+
+	if content.Spec.VolumeGroupSnapshotRef.UID == "" {
+		return fmt.Errorf("cannot delete group snapshot. Group snapshot content %s not bound to a group snapshot", content.Name)
+	}
+
+	if len(snapshotIDs) == 0 {
+		return fmt.Errorf("cannot delete group snapshot. Snapshots found in the group snapshot%s", content.Name)
+	}
+	var groupSnapshotHandle string
+
+	if content.Status != nil && content.Status.VolumeGroupSnapshotHandle != nil {
+		groupSnapshotHandle = *content.Status.VolumeGroupSnapshotHandle
+	} else if content.Spec.Source.VolumeGroupSnapshotHandle != nil {
+		groupSnapshotHandle = *content.Spec.Source.VolumeGroupSnapshotHandle
+	} else {
+		return fmt.Errorf("failed to delete group snapshot content %s: groupsnapshotHandle is missing", content.Name)
+	}
+
+	return handler.groupSnapshotter.DeleteGroupSnapshot(ctx, groupSnapshotHandle, snapshotIDs, snapshotterCredentials)
+}
+
 func (handler *csiHandler) GetGroupSnapshotStatus(groupSnapshotContent *crdv1alpha1.VolumeGroupSnapshotContent, snapshotterListCredentials map[string]string) (bool, time.Time, error) {
 	ctx, cancel := context.WithTimeout(context.Background(), handler.timeout)
 	defer cancel()

pkg/sidecar-controller/groupsnapshot_helper.go

@@ -163,10 +163,23 @@ func (ctrl *csiSnapshotSideCarController) deleteGroupSnapshotContentInCacheStore
 func (ctrl *csiSnapshotSideCarController) syncGroupSnapshotContent(groupSnapshotContent *crdv1alpha1.VolumeGroupSnapshotContent) error {
 	klog.V(5).Infof("synchronizing VolumeGroupSnapshotContent[%s]", groupSnapshotContent.Name)
 
-	/*
-		TODO: Check if the group snapshot content should be deleted
-	*/
+	if ctrl.shouldDeleteGroupSnapshot(groupSnapshotContent) {
+		klog.V(4).Infof("VolumeGroupSnapshotContent[%s]: the policy is %s", groupSnapshotContent.Name, groupSnapshotContent.Spec.DeletionPolicy)
+		if groupSnapshotContent.Spec.DeletionPolicy == crdv1.VolumeSnapshotContentDelete &&
+			groupSnapshotContent.Status != nil && groupSnapshotContent.Status.VolumeGroupSnapshotHandle != nil {
+			// issue a CSI deletion call if the snapshot has not been deleted yet from
+			// underlying storage system. Note that the deletion snapshot operation will
+			// update content SnapshotHandle to nil upon a successful deletion. At this
+			// point, the finalizer on content should NOT be removed to avoid leaking.
+			return ctrl.deleteCSIGroupSnapshotOperation(groupSnapshotContent)
+		}
+		// otherwise, either the snapshot has been deleted from the underlying
+		// storage system, or the deletion policy is Retain, remove the finalizer
+		// if there is one so that API server could delete the object if there is
+		// no other finalizer.
+		return ctrl.removeGroupSnapshotContentFinalizer(groupSnapshotContent)
 
+	}
 	if len(groupSnapshotContent.Spec.Source.PersistentVolumeNames) != 0 && groupSnapshotContent.Status == nil {
 		klog.V(5).Infof("syncGroupSnapshotContent: Call CreateGroupSnapshot for group snapshot content %s", groupSnapshotContent.Name)
 		return ctrl.createGroupSnapshot(groupSnapshotContent)
@@ -184,6 +197,152 @@ func (ctrl *csiSnapshotSideCarController) syncGroupSnapshotContent(groupSnapshot
 	return ctrl.checkandUpdateGroupSnapshotContentStatus(groupSnapshotContent)
 }
 
+// removeContentFinalizer removes the VolumeSnapshotContentFinalizer from a
+// content if there exists one.
+func (ctrl csiSnapshotSideCarController) removeGroupSnapshotContentFinalizer(content *crdv1alpha1.VolumeGroupSnapshotContent) error {
+	if !utils.ContainsString(content.ObjectMeta.Finalizers, utils.VolumeSnapshotContentFinalizer) {
+		// the finalizer does not exit, return directly
+		return nil
+	}
+	contentClone := content.DeepCopy()
+	contentClone.ObjectMeta.Finalizers = utils.RemoveString(contentClone.ObjectMeta.Finalizers, utils.VolumeSnapshotContentFinalizer)
+
+	updatedContent, err := ctrl.clientset.GroupsnapshotV1alpha1().VolumeGroupSnapshotContents().Update(context.TODO(), contentClone, metav1.UpdateOptions{})
+	if err != nil {
+		return newControllerUpdateError(content.Name, err.Error())
+	}
+
+	klog.V(5).Infof("Removed protection finalizer from volume group snapshot content %s", updatedContent.Name)
+	_, err = ctrl.storeContentUpdate(updatedContent)
+	if err != nil {
+		klog.Errorf("failed to update content store %v", err)
+	}
+	return nil
+}
+
+// Delete a groupsnapshot: Ask the backend to remove the groupsnapshot device
+func (ctrl *csiSnapshotSideCarController) deleteCSIGroupSnapshotOperation(groupSnapshotContent *crdv1alpha1.VolumeGroupSnapshotContent) error {
+	klog.V(5).Infof("deleteCSISnapshotOperation [%s] started", groupSnapshotContent.Name)
+
+	snapshotterCredentials, err := ctrl.GetCredentialsFromAnnotationForGroupSnapshot(groupSnapshotContent)
+	if err != nil {
+		ctrl.eventRecorder.Event(groupSnapshotContent, v1.EventTypeWarning, "SnapshotDeleteError", "Failed to get snapshot credentials")
+		return fmt.Errorf("failed to get input parameters to delete snapshot for content %s: %q", groupSnapshotContent.Name, err)
+	}
+
+	var snapshotIDs []string
+	if groupSnapshotContent.Status != nil && len(groupSnapshotContent.Status.VolumeSnapshotContentRefList) != 0 {
+		for _, contentRef := range groupSnapshotContent.Status.VolumeSnapshotContentRefList {
+			snapshotContent, err := ctrl.contentLister.Get(contentRef.Name)
+			if err != nil {
+				return fmt.Errorf("failed to get group snapshot content %s from group snapshot content store: %v", contentRef.Name, err)
+			}
+			snapshotIDs = append(snapshotIDs, *snapshotContent.Status.SnapshotHandle)
+		}
+	}
+
+	err = ctrl.handler.DeleteGroupSnapshot(groupSnapshotContent, snapshotIDs, snapshotterCredentials)
+	if err != nil {
+		ctrl.eventRecorder.Event(groupSnapshotContent, v1.EventTypeWarning, "GroupSnapshotDeleteError", "Failed to delete group snapshot")
+		return fmt.Errorf("failed to delete group snapshot %#v, err: %v", groupSnapshotContent.Name, err)
+	}
+	// the snapshot has been deleted from the underlying storage system, update
+	// content status to remove snapshot handle etc.
+	newContent, err := ctrl.clearVolumeGroupContentStatus(groupSnapshotContent.Name)
+	if err != nil {
+		ctrl.eventRecorder.Event(groupSnapshotContent, v1.EventTypeWarning, "GroupSnapshotDeleteError", "Failed to clear content status")
+		return err
+	}
+	// trigger syncContent
+	ctrl.updateGroupSnapshotContentInInformerCache(newContent)
+	return nil
+}
+
+// clearVolumeContentStatus resets all fields to nil related to a snapshot in
+// content.Status. On success, the latest version of the content object will be
+// returned.
+func (ctrl *csiSnapshotSideCarController) clearVolumeGroupContentStatus(
+	contentName string) (*crdv1alpha1.VolumeGroupSnapshotContent, error) {
+	klog.V(5).Infof("cleanVolumeGroupSnapshotStatus content [%s]", contentName)
+	// get the latest version from API server
+	content, err := ctrl.clientset.GroupsnapshotV1alpha1().VolumeGroupSnapshotContents().Get(context.TODO(), contentName, metav1.GetOptions{})
+	if err != nil {
+		return nil, fmt.Errorf("error get snapshot content %s from api server: %v", contentName, err)
+	}
+	if content.Status != nil {
+		content.Status.VolumeGroupSnapshotHandle = nil
+		content.Status.ReadyToUse = nil
+		content.Status.CreationTime = nil
+		content.Status.Error = nil
+		content.Status.VolumeSnapshotContentRefList = nil
+	}
+	newContent, err := ctrl.clientset.GroupsnapshotV1alpha1().VolumeGroupSnapshotContents().UpdateStatus(context.TODO(), content, metav1.UpdateOptions{})
+	if err != nil {
+		return content, newControllerUpdateError(contentName, err.Error())
+	}
+	return newContent, nil
+}
+
+func (ctrl *csiSnapshotSideCarController) GetCredentialsFromAnnotationForGroupSnapshot(content *crdv1alpha1.VolumeGroupSnapshotContent) (map[string]string, error) {
+	// get secrets if VolumeSnapshotClass specifies it
+	var snapshotterCredentials map[string]string
+	var err error
+
+	// Check if annotation exists
+	if metav1.HasAnnotation(content.ObjectMeta, utils.AnnDeletionSecretRefName) && metav1.HasAnnotation(content.ObjectMeta, utils.AnnDeletionSecretRefNamespace) {
+		annDeletionSecretName := content.Annotations[utils.AnnDeletionSecretRefName]
+		annDeletionSecretNamespace := content.Annotations[utils.AnnDeletionSecretRefNamespace]
+
+		snapshotterSecretRef := &v1.SecretReference{}
+
+		if annDeletionSecretName == "" || annDeletionSecretNamespace == "" {
+			return nil, fmt.Errorf("cannot retrieve secrets for snapshot content %#v, err: secret name or namespace not specified", content.Name)
+		}
+
+		snapshotterSecretRef.Name = annDeletionSecretName
+		snapshotterSecretRef.Namespace = annDeletionSecretNamespace
+
+		snapshotterCredentials, err = utils.GetCredentials(ctrl.client, snapshotterSecretRef)
+		if err != nil {
+			// Continue with deletion, as the secret may have already been deleted.
+			klog.Errorf("Failed to get credentials for snapshot %s: %s", content.Name, err.Error())
+			return nil, fmt.Errorf("cannot get credentials for snapshot content %#v", content.Name)
+		}
+	}
+
+	return snapshotterCredentials, nil
+}
+
+// shouldDeleteGroupSnapshot checks if groupSnapshotContent object should be deleted
+// if DeletionTimestamp is set on the content
+func (ctrl *csiSnapshotSideCarController) shouldDeleteGroupSnapshot(groupSnapshotContent *crdv1alpha1.VolumeGroupSnapshotContent) bool {
+	klog.V(5).Infof("Check if VolumeGroupSnapshotContent[%s] should be deleted.", groupSnapshotContent.Name)
+
+	if groupSnapshotContent.ObjectMeta.DeletionTimestamp == nil {
+		return false
+	}
+	// 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 == "" {
+		return true
+	}
+
+	// NOTE(xyang): Handle create snapshot timeout
+	// 2) shouldDeleteGroupSnapshot returns false if AnnVolumeGroupSnapshotBeingCreated
+	// annotation is set. This indicates a CreateGroupSnapshot CSI RPC has
+	// not responded with success or failure.
+	// We need to keep waiting for a response from the CSI driver.
+	if metav1.HasAnnotation(groupSnapshotContent.ObjectMeta, utils.AnnVolumeGroupSnapshotBeingCreated) {
+		return false
+	}
+
+	// 3) shouldDeleteGroupSnapshot returns true if AnnVolumeSnapshotBeingDeleted annotation is set
+	if metav1.HasAnnotation(groupSnapshotContent.ObjectMeta, utils.AnnVolumeGroupSnapshotBeingDeleted) {
+		return true
+	}
+	return false
+}
+
 // createGroupSnapshot starts new asynchronous operation to create group snapshot
 func (ctrl *csiSnapshotSideCarController) createGroupSnapshot(groupSnapshotContent *crdv1alpha1.VolumeGroupSnapshotContent) error {
 	klog.V(5).Infof("createGroupSnapshot for group snapshot content [%s]: started", groupSnapshotContent.Name)

pkg/sidecar-controller/snapshot_controller.go

@@ -58,7 +58,7 @@ func (ctrl *csiSnapshotSideCarController) syncContent(content *crdv1.VolumeSnaps
 	if ctrl.shouldDelete(content) {
 		klog.V(4).Infof("VolumeSnapshotContent[%s]: the policy is %s", content.Name, content.Spec.DeletionPolicy)
 		if content.Spec.DeletionPolicy == crdv1.VolumeSnapshotContentDelete &&
-			content.Status != nil && content.Status.SnapshotHandle != nil {
+			content.Status != nil && content.Status.SnapshotHandle != nil && content.Status.VolumeGroupSnapshotContentName == nil {
 			// issue a CSI deletion call if the snapshot has not been deleted yet from
 			// underlying storage system. Note that the deletion snapshot operation will
 			// update content SnapshotHandle to nil upon a successful deletion. At this

pkg/utils/util.go

@@ -114,6 +114,14 @@ const (
 	// group snapshots.
 	AnnVolumeGroupSnapshotBeingCreated = "groupsnapshot.storage.kubernetes.io/volumegroupsnapshot-being-created"
 
+	// AnnVolumeGroupSnapshotBeingDeleted annotation applies to VolumeGroupSnapshotContents.
+	// It indicates that the common snapshot controller has verified that volume
+	// group snapshot has a deletion timestamp and is being deleted.
+	// Sidecar controller needs to check the deletion policy on the
+	// VolumeGroupSnapshotContent and decide whether to delete the volume group snapshot
+	// backing the group snapshot content.
+	AnnVolumeGroupSnapshotBeingDeleted = "groupsnapshot.storage.kubernetes.io/volumegroupsnapshot-being-deleted"
+
 	// Annotation for secret name and namespace will be added to the content
 	// and used at snapshot content deletion time.
 	AnnDeletionSecretRefName      = "snapshot.storage.kubernetes.io/deletion-secret-name"