diff --git a/internal/rbd/controllerserver.go b/internal/rbd/controllerserver.go index 8283970f5..6bec4ef98 100644 --- a/internal/rbd/controllerserver.go +++ b/internal/rbd/controllerserver.go @@ -208,7 +208,7 @@ func (cs *ControllerServer) parseVolCreateRequest( return nil, status.Error(codes.InvalidArgument, err.Error()) } - rbdVol.RequestName = req.GetName() + rbdVol.requestName = req.GetName() // Volume Size - Default is 1 GiB volSizeBytes := int64(oneGB) @@ -772,7 +772,11 @@ func (cs *ControllerServer) createBackingImage( } } - log.DebugLog(ctx, "created image %s backed for request name %s", rbdVol, rbdVol.RequestName) + requestName, err := rbdVol.GetRequestName(ctx) + if err != nil { + requestName = "" + } + log.DebugLog(ctx, "created image %s backed for request name %s", rbdVol, requestName) defer func() { if err != nil { @@ -884,12 +888,12 @@ func (cs *ControllerServer) checkErrAndUndoReserve( // If error is ErrImageNotFound then we failed to find the image, but found the imageOMap // to lead us to the image, hence the imageOMap needs to be garbage collected, by calling // unreserve for the same - if acquired := cs.VolumeLocks.TryAcquire(rbdVol.RequestName); !acquired { - log.ErrorLog(ctx, util.VolumeOperationAlreadyExistsFmt, rbdVol.RequestName) + if acquired := cs.VolumeLocks.TryAcquire(rbdVol.requestName); !acquired { + log.ErrorLog(ctx, util.VolumeOperationAlreadyExistsFmt, rbdVol.requestName) - return nil, status.Errorf(codes.Aborted, util.VolumeOperationAlreadyExistsFmt, rbdVol.RequestName) + return nil, status.Errorf(codes.Aborted, util.VolumeOperationAlreadyExistsFmt, rbdVol.requestName) } - defer cs.VolumeLocks.Release(rbdVol.RequestName) + defer cs.VolumeLocks.Release(rbdVol.requestName) if err = undoVolReservation(ctx, rbdVol, cr); err != nil { return nil, status.Error(codes.Internal, err.Error()) @@ -965,12 +969,17 @@ func (cs *ControllerServer) DeleteVolume( // lock out parallel create requests against the same volume name as we // clean up the image and associated omaps for the same - if acquired := cs.VolumeLocks.TryAcquire(rbdVol.RequestName); !acquired { - log.ErrorLog(ctx, util.VolumeOperationAlreadyExistsFmt, rbdVol.RequestName) - - return nil, status.Errorf(codes.Aborted, util.VolumeOperationAlreadyExistsFmt, rbdVol.RequestName) + requestName, err := rbdVol.GetRequestName(ctx) + if err != nil { + return nil, status.Error(codes.Internal, err.Error()) } - defer cs.VolumeLocks.Release(rbdVol.RequestName) + + if acquired := cs.VolumeLocks.TryAcquire(requestName); !acquired { + log.ErrorLog(ctx, util.VolumeOperationAlreadyExistsFmt, requestName) + + return nil, status.Errorf(codes.Aborted, util.VolumeOperationAlreadyExistsFmt, requestName) + } + defer cs.VolumeLocks.Release(requestName) return cleanupRBDImage(ctx, rbdVol, cr) } @@ -1012,7 +1021,7 @@ func cleanupRBDImage(ctx context.Context, if localStatus.IsUP() && localStatus.GetState() == librbd.MirrorImageStatusStateReplaying.String() { if err = undoVolReservation(ctx, rbdVol, cr); err != nil { log.ErrorLog(ctx, "failed to remove reservation for volume (%s) with backing image (%s) (%s)", - rbdVol.RequestName, rbdVol.RbdImageName, err) + rbdVol.requestName, rbdVol.RbdImageName, err) return nil, status.Error(codes.Internal, err.Error()) } @@ -1057,7 +1066,7 @@ func cleanupRBDImage(ctx context.Context, if err = undoVolReservation(ctx, rbdVol, cr); err != nil { log.ErrorLog(ctx, "failed to remove reservation for volume (%s) with backing image (%s) (%s)", - rbdVol.RequestName, rbdVol.RbdImageName, err) + rbdVol.requestName, rbdVol.RbdImageName, err) return nil, status.Error(codes.Internal, err.Error()) } @@ -1146,7 +1155,7 @@ func (cs *ControllerServer) CreateSnapshot( rbdSnap.RbdImageName = rbdVol.RbdImageName rbdSnap.VolSize = rbdVol.VolSize rbdSnap.SourceVolumeID = req.GetSourceVolumeId() - rbdSnap.RequestName = req.GetName() + rbdSnap.requestName = req.GetName() if acquired := cs.SnapshotLocks.TryAcquire(req.GetName()); !acquired { log.ErrorLog(ctx, util.SnapshotOperationAlreadyExistsFmt, req.GetName()) @@ -1250,7 +1259,7 @@ func cloneFromSnapshot( if err != nil { uErr := undoSnapshotCloning(ctx, rbdVol, rbdSnap, vol, cr) if uErr != nil { - log.WarningLog(ctx, "failed undoing reservation of snapshot: %s %v", rbdSnap.RequestName, uErr) + log.WarningLog(ctx, "failed undoing reservation of snapshot: %s %v", rbdSnap.requestName, uErr) } return nil, status.Error(codes.Internal, err.Error()) @@ -1269,7 +1278,7 @@ func cloneFromSnapshot( } else if err != nil { uErr := undoSnapshotCloning(ctx, rbdVol, rbdSnap, vol, cr) if uErr != nil { - log.WarningLog(ctx, "failed undoing reservation of snapshot: %s %v", rbdSnap.RequestName, uErr) + log.WarningLog(ctx, "failed undoing reservation of snapshot: %s %v", rbdSnap.requestName, uErr) } return nil, status.Error(codes.Internal, err.Error()) @@ -1476,12 +1485,17 @@ func (cs *ControllerServer) DeleteSnapshot( // safeguard against parallel create or delete requests against the same // name - if acquired := cs.SnapshotLocks.TryAcquire(rbdSnap.RequestName); !acquired { - log.ErrorLog(ctx, util.SnapshotOperationAlreadyExistsFmt, rbdSnap.RequestName) - - return nil, status.Errorf(codes.Aborted, util.VolumeOperationAlreadyExistsFmt, rbdSnap.RequestName) + requestName, err := rbdSnap.GetRequestName(ctx) + if err != nil { + return nil, status.Error(codes.Internal, err.Error()) } - defer cs.SnapshotLocks.Release(rbdSnap.RequestName) + + if acquired := cs.SnapshotLocks.TryAcquire(requestName); !acquired { + log.ErrorLog(ctx, util.SnapshotOperationAlreadyExistsFmt, requestName) + + return nil, status.Errorf(codes.Aborted, util.VolumeOperationAlreadyExistsFmt, requestName) + } + defer cs.SnapshotLocks.Release(requestName) // Deleting snapshot and cloned volume log.DebugLog(ctx, "deleting cloned rbd volume %s", rbdSnap.RbdSnapName) @@ -1516,7 +1530,7 @@ func cleanUpImageAndSnapReservation(ctx context.Context, rbdSnap *rbdSnapshot, c err = undoSnapReservation(ctx, rbdSnap, cr) if err != nil { log.ErrorLog(ctx, "failed to remove reservation for snapname (%s) with backing snap %q", - rbdSnap.RequestName, rbdSnap, err) + rbdSnap.requestName, rbdSnap, err) return status.Error(codes.Internal, err.Error()) } diff --git a/internal/rbd/rbd_journal.go b/internal/rbd/rbd_journal.go index 67e13daba..2fc4fca0d 100644 --- a/internal/rbd/rbd_journal.go +++ b/internal/rbd/rbd_journal.go @@ -38,7 +38,7 @@ func validateNonEmptyField(field, fieldName, structName string) error { func validateRbdSnap(rbdSnap *rbdSnapshot) error { var err error - if err = validateNonEmptyField(rbdSnap.RequestName, "RequestName", "rbdSnapshot"); err != nil { + if err = validateNonEmptyField(rbdSnap.requestName, "RequestName", "rbdSnapshot"); err != nil { return err } @@ -64,7 +64,7 @@ func validateRbdSnap(rbdSnap *rbdSnapshot) error { func validateRbdVol(rbdVol *rbdVolume) error { var err error - if err = validateNonEmptyField(rbdVol.RequestName, "RequestName", "rbdVolume"); err != nil { + if err = validateNonEmptyField(rbdVol.requestName, "RequestName", "rbdVolume"); err != nil { return err } @@ -145,7 +145,7 @@ func checkSnapCloneExists( defer j.Destroy() snapData, err := j.CheckReservation(ctx, rbdSnap.JournalPool, - rbdSnap.RequestName, rbdSnap.NamePrefix, rbdSnap.RbdImageName, "", util.EncryptionTypeNone) + rbdSnap.requestName, rbdSnap.NamePrefix, rbdSnap.RbdImageName, "", util.EncryptionTypeNone) if err != nil { return false, err } @@ -235,7 +235,7 @@ func checkSnapCloneExists( } log.DebugLog(ctx, "found existing image (%s) with name (%s) for request (%s)", - rbdSnap.VolID, rbdSnap.RbdSnapName, rbdSnap.RequestName) + rbdSnap.VolID, rbdSnap.RbdSnapName, rbdSnap.requestName) return true, nil } @@ -269,7 +269,7 @@ func (rv *rbdVolume) Exists(ctx context.Context, parentVol *rbdVolume) (bool, er defer j.Destroy() imageData, err := j.CheckReservation( - ctx, rv.JournalPool, rv.RequestName, rv.NamePrefix, "", kmsID, encryptionType) + ctx, rv.JournalPool, rv.requestName, rv.NamePrefix, "", kmsID, encryptionType) if err != nil { return false, err } @@ -310,7 +310,7 @@ func (rv *rbdVolume) Exists(ctx context.Context, parentVol *rbdVolume) (bool, er } } err = j.UndoReservation(ctx, rv.JournalPool, rv.Pool, - rv.RbdImageName, rv.RequestName) + rv.RbdImageName, rv.requestName) return false, err } @@ -347,7 +347,7 @@ func (rv *rbdVolume) Exists(ctx context.Context, parentVol *rbdVolume) (bool, er } log.DebugLog(ctx, "found existing volume (%s) with image name (%s) for request (%s)", - rv.VolID, rv.RbdImageName, rv.RequestName) + rv.VolID, rv.RbdImageName, rv.requestName) return true, nil } @@ -415,7 +415,7 @@ func reserveSnap(ctx context.Context, rbdSnap *rbdSnapshot, rbdVol *rbdVolume, c rbdSnap.ReservedID, rbdSnap.RbdSnapName, err = j.ReserveName( ctx, rbdSnap.JournalPool, journalPoolID, rbdSnap.Pool, imagePoolID, - rbdSnap.RequestName, rbdSnap.NamePrefix, rbdVol.RbdImageName, kmsID, rbdSnap.ReservedID, rbdVol.Owner, + rbdSnap.requestName, rbdSnap.NamePrefix, rbdVol.RbdImageName, kmsID, rbdSnap.ReservedID, rbdVol.Owner, "", encryptionType) defer func() { // only undo the reservation when an error occurred @@ -439,7 +439,7 @@ func reserveSnap(ctx context.Context, rbdSnap *rbdSnapshot, rbdVol *rbdVolume, c } log.DebugLog(ctx, "generated Volume ID (%s) and image name (%s) for request name (%s)", - rbdSnap.VolID, rbdSnap.RbdSnapName, rbdSnap.RequestName) + rbdSnap.VolID, rbdSnap.RbdSnapName, rbdSnap.requestName) return nil } @@ -501,7 +501,7 @@ func reserveVol(ctx context.Context, rbdVol *rbdVolume, cr *util.Credentials) er rbdVol.ReservedID, rbdVol.RbdImageName, err = j.ReserveName( ctx, rbdVol.JournalPool, journalPoolID, rbdVol.Pool, imagePoolID, - rbdVol.RequestName, rbdVol.NamePrefix, "", kmsID, rbdVol.ReservedID, rbdVol.Owner, "", encryptionType) + rbdVol.requestName, rbdVol.NamePrefix, "", kmsID, rbdVol.ReservedID, rbdVol.Owner, "", encryptionType) if err != nil { return err } @@ -513,7 +513,7 @@ func reserveVol(ctx context.Context, rbdVol *rbdVolume, cr *util.Credentials) er } log.DebugLog(ctx, "generated Volume ID (%s) and image name (%s) for request name (%s)", - rbdVol.VolID, rbdVol.RbdImageName, rbdVol.RequestName) + rbdVol.VolID, rbdVol.RbdImageName, rbdVol.requestName) return nil } @@ -528,7 +528,7 @@ func undoSnapReservation(ctx context.Context, rbdSnap *rbdSnapshot, cr *util.Cre err = j.UndoReservation( ctx, rbdSnap.JournalPool, rbdSnap.Pool, rbdSnap.RbdSnapName, - rbdSnap.RequestName) + rbdSnap.requestName) return err } @@ -542,7 +542,7 @@ func undoVolReservation(ctx context.Context, rbdVol *rbdVolume, cr *util.Credent defer j.Destroy() err = j.UndoReservation(ctx, rbdVol.JournalPool, rbdVol.Pool, - rbdVol.RbdImageName, rbdVol.RequestName) + rbdVol.RbdImageName, rbdVol.requestName) return err } @@ -636,11 +636,11 @@ func RegenerateJournal( return "", err } - rbdVol.RequestName = requestName + rbdVol.requestName = requestName rbdVol.NamePrefix = volumeAttributes["volumeNamePrefix"] imageData, err := j.CheckReservation( - ctx, rbdVol.JournalPool, rbdVol.RequestName, rbdVol.NamePrefix, "", kmsID, encryptionType) + ctx, rbdVol.JournalPool, rbdVol.requestName, rbdVol.NamePrefix, "", kmsID, encryptionType) if err != nil { return "", err } @@ -680,7 +680,7 @@ func RegenerateJournal( rbdVol.ReservedID, rbdVol.RbdImageName, err = j.ReserveName( ctx, rbdVol.JournalPool, journalPoolID, rbdVol.Pool, imagePoolID, - rbdVol.RequestName, rbdVol.NamePrefix, "", kmsID, vi.ObjectUUID, rbdVol.Owner, "", encryptionType) + rbdVol.requestName, rbdVol.NamePrefix, "", kmsID, vi.ObjectUUID, rbdVol.Owner, "", encryptionType) if err != nil { return "", err } @@ -688,7 +688,7 @@ func RegenerateJournal( defer func() { if err != nil { undoErr := j.UndoReservation(ctx, rbdVol.JournalPool, rbdVol.Pool, - rbdVol.RbdImageName, rbdVol.RequestName) + rbdVol.RbdImageName, rbdVol.requestName) if undoErr != nil { log.ErrorLog(ctx, "failed to undo reservation %s: %v", rbdVol, undoErr) } @@ -701,7 +701,7 @@ func RegenerateJournal( } log.DebugLog(ctx, "re-generated Volume ID (%s) and image name (%s) for request name (%s)", - rbdVol.VolID, rbdVol.RbdImageName, rbdVol.RequestName) + rbdVol.VolID, rbdVol.RbdImageName, rbdVol.requestName) if rbdVol.ImageID == "" { err = rbdVol.storeImageID(ctx, j) if err != nil { diff --git a/internal/rbd/rbd_util.go b/internal/rbd/rbd_util.go index 274981bfe..e0d04cbf1 100644 --- a/internal/rbd/rbd_util.go +++ b/internal/rbd/rbd_util.go @@ -106,10 +106,10 @@ type rbdImage struct { Pool string RadosNamespace string ClusterID string `json:"clusterId"` - // RequestName is the CSI generated volume name for the rbdVolume. + // requestName is the CSI generated volume name for the rbdVolume. // This does not have a JSON tag as it is not stashed in JSON encoded // config maps in v1.0.0 - RequestName string + requestName string ReservedID string NamePrefix string // ParentName represents the parent image name of the image. @@ -417,6 +417,16 @@ func (ri *rbdImage) String() string { return fmt.Sprintf("%s/%s", ri.Pool, ri.RbdImageName) } +func (ri *rbdImage) GetRequestName(_ context.Context) (string, error) { + var err error + + if ri.requestName == "" { + err = fmt.Errorf("rbd image %q does not have a request name", ri) + } + + return ri.requestName, err +} + func (ri *rbdImage) GetPoolName() string { return ri.Pool } @@ -1026,7 +1036,7 @@ func genSnapFromSnapID( return rbdSnap, err } rbdSnap.ImageID = imageAttributes.ImageID - rbdSnap.RequestName = imageAttributes.RequestName + rbdSnap.requestName = imageAttributes.RequestName rbdSnap.RbdImageName = imageAttributes.SourceName rbdSnap.RbdSnapName = imageAttributes.ImageName rbdSnap.ReservedID = vi.ObjectUUID @@ -1156,7 +1166,7 @@ func generateVolumeFromVolumeID( if err != nil { return rbdVol, err } - rbdVol.RequestName = imageAttributes.RequestName + rbdVol.requestName = imageAttributes.RequestName rbdVol.RbdImageName = imageAttributes.ImageName rbdVol.ReservedID = vi.ObjectUUID rbdVol.ImageID = imageAttributes.ImageID diff --git a/internal/rbd/snapshot.go b/internal/rbd/snapshot.go index ff8887590..c13680db1 100644 --- a/internal/rbd/snapshot.go +++ b/internal/rbd/snapshot.go @@ -190,7 +190,7 @@ func (rbdSnap *rbdSnapshot) Delete(ctx context.Context) error { err = undoSnapReservation(ctx, rbdSnap, rbdSnap.conn.Creds) if err != nil { log.ErrorLog(ctx, "failed to remove reservation for snapname (%s) with backing snap (%s) on image (%s) (%s)", - rbdSnap.RequestName, rbdSnap.RbdSnapName, rbdSnap.RbdImageName, err) + rbdSnap.requestName, rbdSnap.RbdSnapName, rbdSnap.RbdImageName, err) return err } @@ -235,7 +235,7 @@ func (rv *rbdVolume) NewSnapshotByID( id uint64, ) (types.Snapshot, error) { snap := rv.toSnapshot() - snap.RequestName = name + snap.requestName = name srcVolID, err := rv.GetID(ctx) if err != nil { diff --git a/internal/rbd/types/group.go b/internal/rbd/types/group.go index 08183f9fb..247ebbb97 100644 --- a/internal/rbd/types/group.go +++ b/internal/rbd/types/group.go @@ -29,6 +29,9 @@ type journalledObject interface { // GetID returns the ID in the backend storage for the object. GetID(ctx context.Context) (string, error) + // GetRequestName returns the requestName of the VolumeGroup. + GetRequestName(ctx context.Context) (string, error) + // GetName returns the name of the object in the backend storage. GetName(ctx context.Context) (string, error)