From f2bc1c674bad36e918d2fde5187f945ce152f0c8 Mon Sep 17 00:00:00 2001
From: Niels de Vos <ndevos@ibm.com>
Date: Tue, 17 Sep 2024 14:07:19 +0200
Subject: [PATCH] rbd: replace Manager.DeleteVolumeGroup() by
 VolumeGroup.Delete()

There is no need for the `Manager.DeleteVolumeGroup()` function as
`VolumeGroup.Delete()` should cover everything too.

By moving the `.Delete()` functionality of removing the group from the
journal to the shared `commonVolumeGroup` type, a volume group snaphot
can use it as well.

Signed-off-by: Niels de Vos <ndevos@ibm.com>
---
 internal/csi-addons/rbd/volumegroup.go |  2 +-
 internal/rbd/group/util.go             | 25 ++++++++++++++++
 internal/rbd/group/volume_group.go     |  2 +-
 internal/rbd/manager.go                | 41 --------------------------
 internal/rbd/types/manager.go          |  4 ---
 5 files changed, 27 insertions(+), 47 deletions(-)

diff --git a/internal/csi-addons/rbd/volumegroup.go b/internal/csi-addons/rbd/volumegroup.go
index 1b418a082..065a6c517 100644
--- a/internal/csi-addons/rbd/volumegroup.go
+++ b/internal/csi-addons/rbd/volumegroup.go
@@ -222,7 +222,7 @@ func (vs *VolumeGroupServer) DeleteVolumeGroup(
 	}
 
 	// delete the volume group
-	err = mgr.DeleteVolumeGroup(ctx, vg)
+	err = vg.Delete(ctx)
 	if err != nil {
 		return nil, status.Errorf(codes.Internal,
 			"failed to delete volume group %q: %s",
diff --git a/internal/rbd/group/util.go b/internal/rbd/group/util.go
index 8e7f76eb3..0271cfc81 100644
--- a/internal/rbd/group/util.go
+++ b/internal/rbd/group/util.go
@@ -233,6 +233,31 @@ func (cvg *commonVolumeGroup) GetIOContext(ctx context.Context) (*rados.IOContex
 	return ioctx, nil
 }
 
+// Delete removes the volume group from the journal.
+func (cvg *commonVolumeGroup) Delete(ctx context.Context) error {
+	name, err := cvg.GetName(ctx)
+	if err != nil {
+		return fmt.Errorf("failed to get name for volume group %q: %w", cvg, err)
+	}
+
+	csiID, err := cvg.GetID(ctx)
+	if err != nil {
+		return fmt.Errorf("failed to get id for volume group %q: %w", cvg, err)
+	}
+
+	pool, err := cvg.GetPool(ctx)
+	if err != nil {
+		return fmt.Errorf("failed to get pool for volume group %q: %w", cvg, err)
+	}
+
+	err = cvg.journal.UndoReservation(ctx, pool, name, csiID)
+	if err != nil /* TODO? !errors.Is(..., err) */ {
+		return fmt.Errorf("failed to undo the reservation for volume group %q: %w", cvg, err)
+	}
+
+	return nil
+}
+
 // GetCreationTime fetches the creation time of the volume group from the
 // journal and returns it.
 func (cvg *commonVolumeGroup) GetCreationTime(ctx context.Context) (*time.Time, error) {
diff --git a/internal/rbd/group/volume_group.go b/internal/rbd/group/volume_group.go
index 337c974ef..7de46131a 100644
--- a/internal/rbd/group/volume_group.go
+++ b/internal/rbd/group/volume_group.go
@@ -194,7 +194,7 @@ func (vg *volumeGroup) Delete(ctx context.Context) error {
 
 	log.DebugLog(ctx, "volume group %q has been removed", vg)
 
-	return nil
+	return vg.commonVolumeGroup.Delete(ctx)
 }
 
 func (vg *volumeGroup) AddVolume(ctx context.Context, vol types.Volume) error {
diff --git a/internal/rbd/manager.go b/internal/rbd/manager.go
index 3c41afdc6..b8c9c2f7d 100644
--- a/internal/rbd/manager.go
+++ b/internal/rbd/manager.go
@@ -21,8 +21,6 @@ import (
 	"errors"
 	"fmt"
 
-	"github.com/ceph/go-ceph/rados"
-
 	"github.com/ceph/ceph-csi/internal/journal"
 	rbd_group "github.com/ceph/ceph-csi/internal/rbd/group"
 	"github.com/ceph/ceph-csi/internal/rbd/types"
@@ -255,42 +253,3 @@ func (mgr *rbdManager) CreateVolumeGroup(ctx context.Context, name string) (type
 
 	return vg, nil
 }
-
-func (mgr *rbdManager) DeleteVolumeGroup(ctx context.Context, vg types.VolumeGroup) error {
-	err := vg.Delete(ctx)
-	if err != nil && !errors.Is(rados.ErrNotFound, err) {
-		return fmt.Errorf("failed to delete volume group %q: %w", vg, err)
-	}
-
-	clusterID, err := vg.GetClusterID(ctx)
-	if err != nil {
-		return fmt.Errorf("failed to get cluster id for volume group %q: %w", vg, err)
-	}
-
-	vgJournal, err := mgr.getVolumeGroupJournal(clusterID)
-	if err != nil {
-		return err
-	}
-
-	name, err := vg.GetName(ctx)
-	if err != nil {
-		return fmt.Errorf("failed to get name for volume group %q: %w", vg, err)
-	}
-
-	csiID, err := vg.GetID(ctx)
-	if err != nil {
-		return fmt.Errorf("failed to get id for volume group %q: %w", vg, err)
-	}
-
-	pool, err := vg.GetPool(ctx)
-	if err != nil {
-		return fmt.Errorf("failed to get pool for volume group %q: %w", vg, err)
-	}
-
-	err = vgJournal.UndoReservation(ctx, pool, name, csiID)
-	if err != nil /* TODO? !errors.Is(..., err) */ {
-		return fmt.Errorf("failed to undo the reservation for volume group %q: %w", vg, err)
-	}
-
-	return nil
-}
diff --git a/internal/rbd/types/manager.go b/internal/rbd/types/manager.go
index 334974932..2e9521570 100644
--- a/internal/rbd/types/manager.go
+++ b/internal/rbd/types/manager.go
@@ -43,8 +43,4 @@ type Manager interface {
 	// CreateVolumeGroup allocates a new VolumeGroup in the backend storage
 	// and records details about it in the journal.
 	CreateVolumeGroup(ctx context.Context, name string) (VolumeGroup, error)
-
-	// DeleteVolumeGroup removes VolumeGroup from the backend storage and
-	// any details from the journal.
-	DeleteVolumeGroup(ctx context.Context, vg VolumeGroup) error
 }