rbd: resolve all volumes that are part of a volume group

Signed-off-by: Niels de Vos <ndevos@ibm.com>

internal/rbd/manager.go

@@ -138,7 +138,7 @@ func (mgr *rbdManager) GetVolumeGroupByID(ctx context.Context, id string) (types
 		return nil, err
 	}
 
-	vg, err := group.GetVolumeGroup(ctx, id, vgJournal, creds)
+	vg, err := group.GetVolumeGroup(ctx, id, vgJournal, creds, mgr)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get volume group with id %q: %w", id, err)
 	}
@@ -197,7 +197,7 @@ func (mgr *rbdManager) CreateVolumeGroup(ctx context.Context, name string) (type
 		return nil, fmt.Errorf("failed to generate a unique CSI volume group id for %q: %w", vgName, err)
 	}
 
-	vg, err := group.GetVolumeGroup(ctx, csiID, vgJournal, creds)
+	vg, err := group.GetVolumeGroup(ctx, csiID, vgJournal, creds, mgr)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get volume group %q at cluster %q: %w", name, clusterID, err)
 	}

internal/rbd/types/manager.go

@@ -20,16 +20,22 @@ import (
 	"context"
 )
 
+// VolumeResolver can be used to construct a Volume from a CSI VolumeId.
+type VolumeResolver interface {
+	// GetVolumeByID uses the CSI VolumeId to resolve the returned Volume.
+	GetVolumeByID(ctx context.Context, id string) (Volume, error)
+}
+
 // Manager provides a way for other packages to get Volumes and VolumeGroups.
 // It handles the operations on the backend, and makes sure the journal
 // reflects the expected state.
 type Manager interface {
+	// VolumeResolver is fully implemented by the Manager.
+	VolumeResolver
+
 	// Destroy frees all resources that the Manager allocated.
 	Destroy(ctx context.Context)
 
-	// GetVolumeByID uses the CSI VolumeId to resolve the returned Volume.
-	GetVolumeByID(ctx context.Context, id string) (Volume, error)
-
 	// GetVolumeGroupByID uses the CSI-Addons VolumeGroupId to resolve the
 	// returned VolumeGroup.
 	GetVolumeGroupByID(ctx context.Context, id string) (VolumeGroup, error)

internal/rbd_group/volume_group.go

@@ -56,11 +56,17 @@ type volumeGroup struct {
 	pool      string
 	namespace string
 
+	journal journal.VolumeGroupJournal
+
 	// volumes is a list of rbd-images that are part of the group. The ID
 	// of each volume is stored in the journal.
 	volumes []types.Volume
 
-	journal journal.VolumeGroupJournal
+	// volumeToFree contains Volumes that were resolved during
+	// GetVolumeGroup. The volumes slice can be updated independently of
+	// this by calling AddVolume (Volumes are allocated elsewhere), and
+	// RemoveVolume (need to keep track of the allocated Volume).
+	volumesToFree []types.Volume
 }
 
 // verify that volumeGroup implements the VolumeGroup and Stringer interfaces.
@@ -76,6 +82,7 @@ func GetVolumeGroup(
 	id string,
 	j journal.VolumeGroupJournal,
 	creds *util.Credentials,
+	volumeResolver types.VolumeResolver,
 ) (types.VolumeGroup, error) {
 	csiID := util.CSIIdentifier{}
 	err := csiID.DecomposeCSIID(id)
@@ -103,7 +110,15 @@ func GetVolumeGroup(
 		return nil, fmt.Errorf("failed to get attributes for volume group id %q: %w", id, err)
 	}
 
-	// TODO: get the volumes that are part of this volume group
+	var volumes []types.Volume
+	for volID := range attrs.VolumeMap {
+		vol, err := volumeResolver.GetVolumeByID(ctx, volID)
+		if err != nil {
+			return nil, fmt.Errorf("failed to get attributes for volume group id %q: %w", id, err)
+		}
+
+		volumes = append(volumes, vol)
+	}
 
 	return &volumeGroup{
 		journal:     j,
@@ -114,6 +129,9 @@ func GetVolumeGroup(
 		monitors:    mons,
 		pool:        pool,
 		namespace:   namespace,
+		volumes:     volumes,
+		// all allocated volumes need to be free'd at Destroy() time
+		volumesToFree: volumes,
 	}, nil
 }
 
@@ -229,6 +247,14 @@ func (vg *volumeGroup) GetIOContext(ctx context.Context) (*rados.IOContext, erro
 
 // Destroy frees the resources used by the volumeGroup.
 func (vg *volumeGroup) Destroy(ctx context.Context) {
+	// free the volumes that were allocated in GetVolumeGroup()
+	if len(vg.volumesToFree) > 0 {
+		for _, volume := range vg.volumesToFree {
+			volume.Destroy(ctx)
+		}
+		vg.volumesToFree = make([]types.Volume, 0)
+	}
+
 	if vg.ioctx != nil {
 		vg.ioctx.Destroy()
 		vg.ioctx = nil
@@ -244,7 +270,6 @@ func (vg *volumeGroup) Destroy(ctx context.Context) {
 		vg.credentials = nil
 	}
 
-	// FIXME: maybe need to .Destroy() all vg.volumes?
 	log.DebugLog(ctx, "destroyed volume group instance with id %q", vg.id)
 }