diff --git a/internal/cephfs/store/volumegroup.go b/internal/cephfs/store/volumegroup.go index 3d302ffc5..ea1c66e4d 100644 --- a/internal/cephfs/store/volumegroup.go +++ b/internal/cephfs/store/volumegroup.go @@ -245,7 +245,7 @@ func ReserveVolumeGroup( defer j.Destroy() groupUUID, vgsi.FsVolumeGroupSnapshotName, err = j.ReserveName( - ctx, volOptions.MetadataPool, util.InvalidPoolID, volOptions.RequestName, volOptions.NamePrefix) + ctx, volOptions.MetadataPool, volOptions.RequestName, volOptions.NamePrefix) if err != nil { return nil, err } diff --git a/internal/csi-addons/rbd/reclaimspace.go b/internal/csi-addons/rbd/reclaimspace.go index 97a5ea025..9fb124a21 100644 --- a/internal/csi-addons/rbd/reclaimspace.go +++ b/internal/csi-addons/rbd/reclaimspace.go @@ -68,7 +68,7 @@ func (rscs *ReclaimSpaceControllerServer) ControllerReclaimSpace( if err != nil { return nil, status.Errorf(codes.Aborted, "failed to find volume with ID %q: %s", volumeID, err.Error()) } - defer rbdVol.Destroy() + defer rbdVol.Destroy(ctx) err = rbdVol.Sparsify() if errors.Is(err, rbdutil.ErrImageInUse) { diff --git a/internal/csi-addons/rbd/replication.go b/internal/csi-addons/rbd/replication.go index f263d3e2b..544907b4f 100644 --- a/internal/csi-addons/rbd/replication.go +++ b/internal/csi-addons/rbd/replication.go @@ -273,7 +273,7 @@ func (rs *ReplicationServer) EnableVolumeReplication(ctx context.Context, rbdVol, err := corerbd.GenVolFromVolID(ctx, volumeID, cr, req.GetSecrets()) defer func() { if rbdVol != nil { - rbdVol.Destroy() + rbdVol.Destroy(ctx) } }() if err != nil { @@ -350,7 +350,7 @@ func (rs *ReplicationServer) DisableVolumeReplication(ctx context.Context, rbdVol, err := corerbd.GenVolFromVolID(ctx, volumeID, cr, req.GetSecrets()) defer func() { if rbdVol != nil { - rbdVol.Destroy() + rbdVol.Destroy(ctx) } }() if err != nil { @@ -425,7 +425,7 @@ func (rs *ReplicationServer) PromoteVolume(ctx context.Context, rbdVol, err := corerbd.GenVolFromVolID(ctx, volumeID, cr, req.GetSecrets()) defer func() { if rbdVol != nil { - rbdVol.Destroy() + rbdVol.Destroy(ctx) } }() if err != nil { @@ -525,7 +525,7 @@ func (rs *ReplicationServer) DemoteVolume(ctx context.Context, rbdVol, err := corerbd.GenVolFromVolID(ctx, volumeID, cr, req.GetSecrets()) defer func() { if rbdVol != nil { - rbdVol.Destroy() + rbdVol.Destroy(ctx) } }() if err != nil { @@ -642,7 +642,7 @@ func (rs *ReplicationServer) ResyncVolume(ctx context.Context, rbdVol, err := corerbd.GenVolFromVolID(ctx, volumeID, cr, req.GetSecrets()) defer func() { if rbdVol != nil { - rbdVol.Destroy() + rbdVol.Destroy(ctx) } }() if err != nil { @@ -856,7 +856,7 @@ func (rs *ReplicationServer) GetVolumeReplicationInfo(ctx context.Context, rbdVol, err := corerbd.GenVolFromVolID(ctx, volumeID, cr, req.GetSecrets()) defer func() { if rbdVol != nil { - rbdVol.Destroy() + rbdVol.Destroy(ctx) } }() if err != nil { diff --git a/internal/journal/volumegroupjournal.go b/internal/journal/volumegroupjournal.go index 990c38d85..949cc9145 100644 --- a/internal/journal/volumegroupjournal.go +++ b/internal/journal/volumegroupjournal.go @@ -51,8 +51,7 @@ type VolumeGroupJournal interface { objectUUID string) (*VolumeGroupAttributes, error) ReserveName( ctx context.Context, - journalPool string, - journalPoolID int64, + journalPool, reqName, namePrefix string) (string, string, error) // AddVolumesMapping adds a volumeMap map which contains volumeID's and its @@ -296,7 +295,6 @@ held, to prevent parallel operations from modifying the state of the omaps for t Input arguments: - journalPool: Pool where the CSI journal is stored - - journalPoolID: pool ID of the journalPool - reqName: Name of the volumeGroupSnapshot request received - namePrefix: Prefix to use when generating the volumeGroupName name (suffix is an auto-generated UUID) @@ -306,8 +304,7 @@ Return values: - error: non-nil in case of any errors */ func (vgjc *VolumeGroupJournalConnection) ReserveName(ctx context.Context, - journalPool string, journalPoolID int64, - reqName, namePrefix string, + journalPool, reqName, namePrefix string, ) (string, string, error) { cj := vgjc.config diff --git a/internal/rbd/clone.go b/internal/rbd/clone.go index b2ce33ba5..25452921a 100644 --- a/internal/rbd/clone.go +++ b/internal/rbd/clone.go @@ -46,10 +46,10 @@ import ( func (rv *rbdVolume) checkCloneImage(ctx context.Context, parentVol *rbdVolume) (bool, error) { // generate temp cloned volume tempClone := rv.generateTempClone() - defer tempClone.Destroy() + defer tempClone.Destroy(ctx) snap := &rbdSnapshot{} - defer snap.Destroy() + defer snap.Destroy(ctx) snap.RbdSnapName = rv.RbdImageName snap.Pool = rv.Pool @@ -141,7 +141,7 @@ func (rv *rbdVolume) createCloneFromImage(ctx context.Context, parentVol *rbdVol defer func() { if err != nil { log.DebugLog(ctx, "Removing clone image %q", rv) - errDefer := rv.deleteImage(ctx) + errDefer := rv.Delete(ctx) if errDefer != nil { log.ErrorLog(ctx, "failed to delete clone image %q: %v", rv, errDefer) } @@ -183,7 +183,7 @@ func (rv *rbdVolume) doSnapClone(ctx context.Context, parentVol *rbdVolume) erro // generate temp cloned volume tempClone := rv.generateTempClone() - defer tempClone.Destroy() + defer tempClone.Destroy(ctx) // snapshot name is same as temporary cloned image, This helps to // flatten the temporary cloned images as we cannot have more than 510 diff --git a/internal/rbd/controllerserver.go b/internal/rbd/controllerserver.go index 71f0492c4..640827796 100644 --- a/internal/rbd/controllerserver.go +++ b/internal/rbd/controllerserver.go @@ -335,7 +335,7 @@ func (cs *ControllerServer) CreateVolume( if err != nil { return nil, err } - defer rbdVol.Destroy() + defer rbdVol.Destroy(ctx) // Existence and conflict checks if acquired := cs.VolumeLocks.TryAcquire(req.GetName()); !acquired { log.ErrorLog(ctx, util.VolumeOperationAlreadyExistsFmt, req.GetName()) @@ -349,10 +349,10 @@ func (cs *ControllerServer) CreateVolume( return nil, err } if parentVol != nil { - defer parentVol.Destroy() + defer parentVol.Destroy(ctx) } if rbdSnap != nil { - defer rbdSnap.Destroy() + defer rbdSnap.Destroy(ctx) } err = updateTopologyConstraints(rbdVol, rbdSnap) @@ -403,7 +403,7 @@ func (cs *ControllerServer) CreateVolume( metadata := k8s.GetVolumeMetadata(req.GetParameters()) err = rbdVol.setAllMetadata(metadata) if err != nil { - if deleteErr := rbdVol.deleteImage(ctx); deleteErr != nil { + if deleteErr := rbdVol.Delete(ctx); deleteErr != nil { log.ErrorLog(ctx, "failed to delete rbd image: %s with error: %v", rbdVol, deleteErr) } @@ -465,7 +465,7 @@ func flattenParentImage( // in case of any error call Destroy for cleanup. defer func() { if err != nil { - rbdSnap.Destroy() + rbdSnap.Destroy(ctx) } }() @@ -621,7 +621,7 @@ func checkFlatten(ctx context.Context, rbdVol *rbdVolume, cr *util.Credentials) if errors.Is(err, ErrFlattenInProgress) { return status.Error(codes.Aborted, err.Error()) } - if errDefer := rbdVol.deleteImage(ctx); errDefer != nil { + if errDefer := rbdVol.Delete(ctx); errDefer != nil { log.ErrorLog(ctx, "failed to delete rbd image: %s with error: %v", rbdVol, errDefer) return status.Error(codes.Internal, err.Error()) @@ -661,14 +661,14 @@ func (cs *ControllerServer) createVolumeFromSnapshot( return status.Error(codes.Internal, err.Error()) } - defer rbdSnap.Destroy() + defer rbdSnap.Destroy(ctx) // update parent name(rbd image name in snapshot) rbdSnap.RbdImageName = rbdSnap.RbdSnapName parentVol := rbdSnap.toVolume() // as we are operating on single cluster reuse the connection parentVol.conn = rbdVol.conn.Copy() - defer parentVol.Destroy() + defer parentVol.Destroy(ctx) // create clone image and delete snapshot err = rbdVol.cloneRbdImageFromSnapshot(ctx, rbdSnap, parentVol) @@ -681,7 +681,7 @@ func (cs *ControllerServer) createVolumeFromSnapshot( defer func() { if err != nil { log.DebugLog(ctx, "Removing clone image %q", rbdVol) - errDefer := rbdVol.deleteImage(ctx) + errDefer := rbdVol.Delete(ctx) if errDefer != nil { log.ErrorLog(ctx, "failed to delete clone image %q: %v", rbdVol, errDefer) } @@ -764,7 +764,7 @@ func (cs *ControllerServer) createBackingImage( defer func() { if err != nil { - if deleteErr := rbdVol.deleteImage(ctx); deleteErr != nil { + if deleteErr := rbdVol.Delete(ctx); deleteErr != nil { log.ErrorLog(ctx, "failed to delete rbd image: %s with error: %v", rbdVol, deleteErr) } } @@ -944,7 +944,7 @@ func (cs *ControllerServer) DeleteVolume( rbdVol, err := GenVolFromVolID(ctx, volumeID, cr, req.GetSecrets()) defer func() { if rbdVol != nil { - rbdVol.Destroy() + rbdVol.Destroy(ctx) } }() if err != nil { @@ -1029,7 +1029,7 @@ func cleanupRBDImage(ctx context.Context, // Deleting rbd image log.DebugLog(ctx, "deleting image %s", rbdVol.RbdImageName) - if err = rbdVol.deleteImage(ctx); err != nil { + if err = rbdVol.Delete(ctx); err != nil { log.ErrorLog(ctx, "failed to delete rbd image: %s with error: %v", rbdVol, err) @@ -1094,7 +1094,7 @@ func (cs *ControllerServer) CreateSnapshot( rbdVol, err := GenVolFromVolID(ctx, req.GetSourceVolumeId(), cr, req.GetSecrets()) defer func() { if rbdVol != nil { - rbdVol.Destroy() + rbdVol.Destroy(ctx) } }() if err != nil { @@ -1188,7 +1188,7 @@ func (cs *ControllerServer) CreateSnapshot( defer func() { if err != nil { log.DebugLog(ctx, "Removing clone image %q", rbdVol) - errDefer := rbdVol.deleteImage(ctx) + errDefer := rbdVol.Delete(ctx) if errDefer != nil { log.ErrorLog(ctx, "failed to delete clone image %q: %v", rbdVol, errDefer) } @@ -1237,7 +1237,7 @@ func cloneFromSnapshot( return nil, status.Errorf(codes.Internal, err.Error()) } - defer vol.Destroy() + defer vol.Destroy(ctx) err = rbdVol.copyEncryptionConfig(ctx, &vol.rbdImage, false) if err != nil { @@ -1313,7 +1313,7 @@ func (cs *ControllerServer) doSnapshotClone( ) (*rbdVolume, error) { // generate cloned volume details from snapshot cloneRbd := rbdSnap.toVolume() - defer cloneRbd.Destroy() + defer cloneRbd.Destroy(ctx) // add image feature for cloneRbd f := []string{librbd.FeatureNameLayering, librbd.FeatureNameDeepFlatten} cloneRbd.ImageFeatureSet = librbd.FeatureSetFromNames(f) @@ -1460,7 +1460,7 @@ func (cs *ControllerServer) DeleteSnapshot( return nil, status.Error(codes.Internal, err.Error()) } - defer rbdSnap.Destroy() + defer rbdSnap.Destroy(ctx) // safeguard against parallel create or delete requests against the same // name @@ -1480,7 +1480,7 @@ func (cs *ControllerServer) DeleteSnapshot( if err != nil { return nil, status.Error(codes.Internal, err.Error()) } - defer rbdVol.Destroy() + defer rbdVol.Destroy(ctx) rbdVol.ImageID = rbdSnap.ImageID // update parent name to delete the snapshot @@ -1510,7 +1510,7 @@ func cleanUpImageAndSnapReservation(ctx context.Context, rbdSnap *rbdSnapshot, c if err != nil { return status.Error(codes.Internal, err.Error()) } - defer rbdVol.Destroy() + defer rbdVol.Destroy(ctx) err = rbdVol.openIoctx() if err != nil { @@ -1584,7 +1584,7 @@ func (cs *ControllerServer) ControllerExpandVolume( return nil, err } - defer rbdVol.Destroy() + defer rbdVol.Destroy(ctx) // NodeExpansion is needed for PersistentVolumes with, // 1. Filesystem VolumeMode with & without Encryption and diff --git a/internal/rbd/migration.go b/internal/rbd/migration.go index e68437bad..b7629251b 100644 --- a/internal/rbd/migration.go +++ b/internal/rbd/migration.go @@ -81,8 +81,8 @@ func deleteMigratedVolume(ctx context.Context, parsedMigHandle *migrationVolID, if err != nil { return err } - defer rv.Destroy() - err = rv.deleteImage(ctx) + defer rv.Destroy(ctx) + err = rv.Delete(ctx) if err != nil { log.ErrorLog(ctx, "failed to delete rbd image: %s, err: %v", rv, err) } diff --git a/internal/rbd/nodeserver.go b/internal/rbd/nodeserver.go index c203c82db..5f4dd0007 100644 --- a/internal/rbd/nodeserver.go +++ b/internal/rbd/nodeserver.go @@ -198,7 +198,7 @@ func (ns *NodeServer) populateRbdVol( } else { rv, err = GenVolFromVolID(ctx, volID, cr, req.GetSecrets()) if err != nil { - rv.Destroy() + rv.Destroy(ctx) log.ErrorLog(ctx, "error generating volume %s: %v", volID, err) return nil, status.Errorf(codes.Internal, "error generating volume %s: %v", volID, err) @@ -221,7 +221,7 @@ func (ns *NodeServer) populateRbdVol( // in case of any error call Destroy for cleanup. defer func() { if err != nil { - rv.Destroy() + rv.Destroy(ctx) } }() // get the image details from the ceph cluster. @@ -345,7 +345,7 @@ func (ns *NodeServer) NodeStageVolume( if err != nil { return nil, err } - defer rv.Destroy() + defer rv.Destroy(ctx) rv.NetNamespaceFilePath, err = util.GetRBDNetNamespaceFilePath(util.CsiConfigFile, rv.ClusterID) if err != nil { diff --git a/internal/rbd/rbd_journal.go b/internal/rbd/rbd_journal.go index 69611fd08..78f5d5fdd 100644 --- a/internal/rbd/rbd_journal.go +++ b/internal/rbd/rbd_journal.go @@ -163,7 +163,7 @@ func checkSnapCloneExists( } vol := rbdSnap.toVolume() - defer vol.Destroy() + defer vol.Destroy(ctx) err = vol.Connect(cr) if err != nil { return false, err diff --git a/internal/rbd/rbd_util.go b/internal/rbd/rbd_util.go index b84f64344..48b445d07 100644 --- a/internal/rbd/rbd_util.go +++ b/internal/rbd/rbd_util.go @@ -28,6 +28,7 @@ import ( "strings" "time" + types "github.com/ceph/ceph-csi/internal/rbd_types" "github.com/ceph/ceph-csi/internal/util" "github.com/ceph/ceph-csi/internal/util/log" @@ -150,6 +151,9 @@ type rbdImage struct { ParentInTrash bool } +// check that rbdVolume implements the types.Volume interface. +var _ types.Volume = &rbdVolume{} + // rbdVolume represents a CSI volume and its RBD image specifics. type rbdVolume struct { rbdImage @@ -385,7 +389,7 @@ func (ri *rbdImage) Connect(cr *util.Credentials) error { // Destroy cleans up the rbdVolume and closes the connection to the Ceph // cluster in case one was setup. -func (ri *rbdImage) Destroy() { +func (ri *rbdImage) Destroy(ctx context.Context) { if ri.ioctx != nil { ri.ioctx.Destroy() } @@ -418,6 +422,11 @@ func (rs *rbdSnapshot) String() string { return fmt.Sprintf("%s/%s@%s", rs.Pool, rs.RbdImageName, rs.RbdSnapName) } +// GetID returns the CSI volume handle of the image. +func (ri *rbdImage) GetID(ctx context.Context) (string, error) { + return ri.VolID, nil +} + // createImage creates a new ceph image with provision and volume options. func createImage(ctx context.Context, pOpts *rbdVolume, cr *util.Credentials) error { volSzMiB := fmt.Sprintf("%dM", util.RoundOffVolSize(pOpts.VolSize)) @@ -627,8 +636,8 @@ func (ri *rbdImage) ensureImageCleanup(ctx context.Context) error { return nil } -// deleteImage deletes a ceph image with provision and volume options. -func (ri *rbdImage) deleteImage(ctx context.Context) error { +// Delete deletes a ceph image with provision and volume options. +func (ri *rbdImage) Delete(ctx context.Context) error { image := ri.RbdImageName log.DebugLog(ctx, "rbd: delete %s using mon %s, pool %s", image, ri.Monitors, ri.Pool) @@ -706,7 +715,7 @@ func (ri *rbdImage) trashRemoveImage(ctx context.Context) error { // DeleteTempImage deletes the temporary image created for volume datasource. func (rv *rbdVolume) DeleteTempImage(ctx context.Context) error { tempClone := rv.generateTempClone() - err := tempClone.deleteImage(ctx) + err := tempClone.Delete(ctx) if err != nil { if errors.Is(err, ErrImageNotFound) { return tempClone.ensureImageCleanup(ctx) @@ -777,7 +786,7 @@ func flattenClonedRbdImages( rv.Pool = pool rv.RbdImageName = rbdImageName - defer rv.Destroy() + defer rv.Destroy(ctx) err := rv.Connect(cr) if err != nil { log.ErrorLog(ctx, "failed to open connection %s; err %v", rv, err) @@ -1045,7 +1054,7 @@ func genSnapFromSnapID( } defer func() { if err != nil { - rbdSnap.Destroy() + rbdSnap.Destroy(ctx) } }() @@ -1064,7 +1073,7 @@ func genSnapFromSnapID( } } - err = updateSnapshotDetails(rbdSnap) + err = updateSnapshotDetails(ctx, rbdSnap) if err != nil { return rbdSnap, fmt.Errorf("failed to update snapshot details for %q: %w", rbdSnap, err) } @@ -1074,13 +1083,13 @@ func genSnapFromSnapID( // updateSnapshotDetails will copy the details from the rbdVolume to the // rbdSnapshot. example copying size from rbdVolume to rbdSnapshot. -func updateSnapshotDetails(rbdSnap *rbdSnapshot) error { +func updateSnapshotDetails(ctx context.Context, rbdSnap *rbdSnapshot) error { vol := rbdSnap.toVolume() err := vol.Connect(rbdSnap.conn.Creds) if err != nil { return err } - defer vol.Destroy() + defer vol.Destroy(ctx) err = vol.getImageInfo() if err != nil { @@ -1685,7 +1694,7 @@ func (ri *rbdImage) flattenParent(ctx context.Context, hardLimit, softLimit uint if parentImage == nil { return nil } - defer parentImage.Destroy() + defer parentImage.Destroy(ctx) return parentImage.flattenRbdImage(ctx, false, hardLimit, softLimit) } @@ -2128,7 +2137,7 @@ func genVolFromVolIDWithMigration( } rv, err := GenVolFromVolID(ctx, volID, cr, secrets) if err != nil { - rv.Destroy() + rv.Destroy(ctx) } return rv, err diff --git a/internal/rbd/snapshot.go b/internal/rbd/snapshot.go index d6dcd48c2..fc3fa2427 100644 --- a/internal/rbd/snapshot.go +++ b/internal/rbd/snapshot.go @@ -56,7 +56,7 @@ func createRBDClone( errSnap := parentVol.deleteSnapshot(ctx, snap) if errSnap != nil { log.ErrorLog(ctx, "failed to delete snapshot: %v", errSnap) - delErr := cloneRbdVol.deleteImage(ctx) + delErr := cloneRbdVol.Delete(ctx) if delErr != nil { log.ErrorLog(ctx, "failed to delete rbd image: %s with error: %v", cloneRbdVol, delErr) } @@ -85,7 +85,7 @@ func cleanUpSnapshot( } if rbdVol != nil { - err := rbdVol.deleteImage(ctx) + err := rbdVol.Delete(ctx) if err != nil { if !errors.Is(err, ErrImageNotFound) { log.ErrorLog(ctx, "failed to delete rbd image %q with error: %v", rbdVol, err) diff --git a/internal/rbd_types/volume.go b/internal/rbd_types/volume.go new file mode 100644 index 000000000..b2bb1e2bc --- /dev/null +++ b/internal/rbd_types/volume.go @@ -0,0 +1,30 @@ +/* +Copyright 2024 The Ceph-CSI Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package rbd_types + +import ( + "context" +) + +type Volume interface { + // Destroy frees the resources used by the Volume. + Destroy(ctx context.Context) + + Delete(ctx context.Context) error + + GetID(ctx context.Context) (string, error) +}