From 823b1d333facda7774d3c69ccdfea7dd3488315f Mon Sep 17 00:00:00 2001
From: Niels de Vos <ndevos@ibm.com>
Date: Thu, 22 Aug 2024 12:47:09 +0200
Subject: [PATCH] group controller: better cleanup when group snapshot fails

---
 internal/rbd/group_controller.go | 35 ++++++++++++++++++++++----------
 1 file changed, 24 insertions(+), 11 deletions(-)

diff --git a/internal/rbd/group_controller.go b/internal/rbd/group_controller.go
index 61ee94ea2d36..f06c077178ef 100644
--- a/internal/rbd/group_controller.go
+++ b/internal/rbd/group_controller.go
@@ -47,6 +47,10 @@ func (cs *ControllerServer) CreateVolumeGroupSnapshot(ctx context.Context, req *
 	// 4. create a GroupSnapshot
 	// 5. remove all rbd-images from the VolumeGroup
 	// 6. return the VolumeGroupSnapshot-name and list of snapshots
+	var (
+		vg            types.VolumeGroup
+		groupSnapshot types.VolumeGroupSnapshot
+	)
 
 	mgr := NewManager(cs.Driver.GetInstanceID(), req.GetParameters(), req.GetSecrets())
 	defer mgr.Destroy(ctx)
@@ -54,8 +58,25 @@ func (cs *ControllerServer) CreateVolumeGroupSnapshot(ctx context.Context, req *
 	// resolve all volumes, free them later on
 	volumes := make([]types.Volume, len(req.GetSourceVolumeIds()))
 	defer func() {
-		for _, vol := range volumes {
-			vol.Destroy(ctx)
+		for _, volume := range volumes {
+			if vg != nil {
+				// 'normal' cleanup, remove all images from the group
+				err := vg.RemoveVolume(ctx, volume)
+				if err != nil {
+					log.ErrorLog(ctx, "failed to remove volume %q from volume group %q: %v", volume, vg, err)
+				}
+			}
+
+			// free all allocated volumes
+			volume.Destroy(ctx)
+		}
+
+		if groupSnapshot == nil && vg != nil {
+			// group snapshot creation failed, delete the volume group
+			err := vg.Delete(ctx)
+			if err != nil {
+				log.ErrorLog(ctx, "failed to delete volume group %q: %v", vg, err)
+			}
 		}
 	}()
 	for i, id := range req.GetSourceVolumeIds() {
@@ -93,20 +114,12 @@ func (cs *ControllerServer) CreateVolumeGroupSnapshot(ctx context.Context, req *
 		}
 	}
 
-	groupSnapshot, err := vg.CreateSnapshot(ctx, req.GetName())
+	groupSnapshot, err = vg.CreateSnapshot(ctx, req.GetName())
 	if err != nil {
 		return nil, status.Error(codes.Aborted, err.Error())
 	}
 	defer groupSnapshot.Destroy(ctx)
 
-	// TODO: remove images from the group
-	for _, volume := range volumes {
-		err = vg.RemoveVolume(ctx, volume)
-		if err != nil {
-			return nil, status.Error(codes.Aborted, err.Error())
-		}
-	}
-
 	csiVGS, err := groupSnapshot.ToCSI(ctx)
 	if err != nil {
 		return nil, status.Error(codes.Aborted, err.Error())