diff --git a/internal/rbd/manager.go b/internal/rbd/manager.go index ca120b3fca80..8c52db6bd608 100644 --- a/internal/rbd/manager.go +++ b/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) } diff --git a/internal/rbd/types/manager.go b/internal/rbd/types/manager.go index 7744eb18a6f9..334974932263 100644 --- a/internal/rbd/types/manager.go +++ b/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) diff --git a/internal/rbd_group/volume_group.go b/internal/rbd_group/volume_group.go index d0a75c9a82b6..141bbf384369 100644 --- a/internal/rbd_group/volume_group.go +++ b/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) }