diff --git a/internal/cephfs/cephfs_util.go b/internal/cephfs/cephfs_util.go index 40e505e68..79be1f1b3 100644 --- a/internal/cephfs/cephfs_util.go +++ b/internal/cephfs/cephfs_util.go @@ -84,7 +84,7 @@ func getMetadataPool(ctx context.Context, monitors string, cr *util.Credentials, } } - return "", util.ErrPoolNotFound{Pool: fsName, Err: fmt.Errorf("fsName (%s) not found in Ceph cluster", fsName)} + return "", fmt.Errorf("%w: fsName (%s) not found in Ceph cluster", util.ErrPoolNotFound, fsName) } // CephFilesystemDump is a representation of the main json structure returned by 'ceph fs dump'. @@ -114,5 +114,5 @@ func getFsName(ctx context.Context, monitors string, cr *util.Credentials, fscID } } - return "", util.ErrPoolNotFound{Pool: string(fscID), Err: fmt.Errorf("fscID (%d) not found in Ceph cluster", fscID)} + return "", fmt.Errorf("%w: fscID (%d) not found in Ceph cluster", util.ErrPoolNotFound, fscID) } diff --git a/internal/cephfs/controllerserver.go b/internal/cephfs/controllerserver.go index 255e81219..b8a8945e5 100644 --- a/internal/cephfs/controllerserver.go +++ b/internal/cephfs/controllerserver.go @@ -172,16 +172,14 @@ func (cs *ControllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVol if err != nil { // if error is ErrPoolNotFound, the pool is already deleted we dont // need to worry about deleting subvolume or omap data, return success - var epnf util.ErrPoolNotFound - if errors.As(err, &epnf) { + if errors.Is(err, util.ErrPoolNotFound) { klog.Warningf(util.Log(ctx, "failed to get backend volume for %s: %v"), string(volID), err) return &csi.DeleteVolumeResponse{}, nil } // if error is ErrKeyNotFound, then a previous attempt at deletion was complete // or partially complete (subvolume and imageOMap are garbage collected already), hence // return success as deletion is complete - var eknf util.ErrKeyNotFound - if errors.As(err, &eknf) { + if errors.Is(err, util.ErrKeyNotFound) { return &csi.DeleteVolumeResponse{}, nil } diff --git a/internal/journal/omap.go b/internal/journal/omap.go index f975a25ec..dcfc1de03 100644 --- a/internal/journal/omap.go +++ b/internal/journal/omap.go @@ -37,7 +37,7 @@ func getOMapValues( // fetch and configure the rados ioctx ioctx, err := conn.conn.GetIoctx(poolName) if err != nil { - return nil, omapPoolError(poolName, err) + return nil, omapPoolError(err) } defer ioctx.Destroy() @@ -66,7 +66,7 @@ func getOMapValues( klog.Errorf( util.Log(ctx, "omap not found (pool=%q, namespace=%q, name=%q): %v"), poolName, namespace, oid, err) - return nil, util.NewErrKeyNotFound(oid, err) + return nil, util.JoinErrors(util.ErrKeyNotFound, err) } return nil, err } @@ -83,7 +83,7 @@ func removeMapKeys( // fetch and configure the rados ioctx ioctx, err := conn.conn.GetIoctx(poolName) if err != nil { - return omapPoolError(poolName, err) + return omapPoolError(err) } defer ioctx.Destroy() @@ -118,7 +118,7 @@ func setOMapKeys( // fetch and configure the rados ioctx ioctx, err := conn.conn.GetIoctx(poolName) if err != nil { - return omapPoolError(poolName, err) + return omapPoolError(err) } defer ioctx.Destroy() @@ -142,9 +142,9 @@ func setOMapKeys( return nil } -func omapPoolError(poolName string, err error) error { +func omapPoolError(err error) error { if errors.Is(err, rados.ErrNotFound) { - return util.NewErrPoolNotFound(poolName, err) + return util.JoinErrors(util.ErrPoolNotFound, err) } return err } diff --git a/internal/journal/voljournal.go b/internal/journal/voljournal.go index a65599fb8..8b83eda9b 100644 --- a/internal/journal/voljournal.go +++ b/internal/journal/voljournal.go @@ -282,9 +282,7 @@ func (conn *Connection) CheckReservation(ctx context.Context, ctx, conn, journalPool, cj.namespace, cj.csiDirectory, cj.commonPrefix, fetchKeys) if err != nil { - var eknf util.ErrKeyNotFound - var epnf util.ErrPoolNotFound - if errors.As(err, &eknf) || errors.As(err, &epnf) { + if errors.Is(err, util.ErrKeyNotFound) || errors.Is(err, util.ErrPoolNotFound) { // pool or omap (oid) was not present // stop processing but without an error for no reservation exists return nil, nil @@ -316,8 +314,7 @@ func (conn *Connection) CheckReservation(ctx context.Context, savedImagePool, err = util.GetPoolName(conn.monitors, conn.cr, savedImagePoolID) if err != nil { - var epnf util.ErrPoolNotFound - if errors.As(err, &epnf) { + if errors.Is(err, util.ErrPoolNotFound) { err = conn.UndoReservation(ctx, journalPool, "", "", reqName) } return nil, err @@ -329,8 +326,7 @@ func (conn *Connection) CheckReservation(ctx context.Context, if err != nil { // error should specifically be not found, for image to be absent, any other error // is not conclusive, and we should not proceed - var eknf util.ErrKeyNotFound - if errors.As(err, &eknf) { + if errors.Is(err, util.ErrKeyNotFound) { err = conn.UndoReservation(ctx, journalPool, savedImagePool, cj.GetNameForUUID(namePrefix, objUUID, snapSource), reqName) } @@ -363,10 +359,10 @@ func (conn *Connection) CheckReservation(ctx context.Context, if savedImageAttributes.SourceName != parentName { // NOTE: This can happen if there is a snapname conflict, and we already have a snapshot // with the same name pointing to a different UUID as the source - err = fmt.Errorf("snapname points to different volume, request name (%s)"+ - " source name (%s) saved source name (%s)", + err = fmt.Errorf("%w: snapname points to different volume, request name (%s)"+ + " source name (%s) saved source name (%s)", util.ErrSnapNameConflict, reqName, parentName, savedImageAttributes.SourceName) - return nil, util.NewErrSnapNameConflict(reqName, err) + return nil, err } } @@ -412,8 +408,7 @@ func (conn *Connection) UndoReservation(ctx context.Context, err := util.RemoveObject(ctx, conn.monitors, conn.cr, volJournalPool, cj.namespace, cj.cephUUIDDirectoryPrefix+imageUUID) if err != nil { - var eonf util.ErrObjectNotFound - if !errors.As(err, &eonf) { + if !errors.Is(err, util.ErrObjectNotFound) { klog.Errorf(util.Log(ctx, "failed removing oMap %s (%s)"), cj.cephUUIDDirectoryPrefix+imageUUID, err) return err } @@ -445,8 +440,7 @@ func reserveOMapName(ctx context.Context, monitors string, cr *util.Credentials, err := util.CreateObject(ctx, monitors, cr, pool, namespace, oMapNamePrefix+iterUUID) if err != nil { - var eoe util.ErrObjectExists - if errors.As(err, &eoe) { + if errors.Is(err, util.ErrObjectExists) { attempt++ // try again with a different uuid, for maxAttempts tries util.DebugLog(ctx, "uuid (%s) conflict detected, retrying (attempt %d of %d)", @@ -616,9 +610,7 @@ func (conn *Connection) GetImageAttributes(ctx context.Context, pool, objectUUID ctx, conn, pool, cj.namespace, cj.cephUUIDDirectoryPrefix+objectUUID, cj.commonPrefix, fetchKeys) if err != nil { - var eknf util.ErrKeyNotFound - var epnf util.ErrPoolNotFound - if !errors.As(err, &eknf) && !errors.As(err, &epnf) { + if !errors.Is(err, util.ErrKeyNotFound) && !errors.Is(err, util.ErrPoolNotFound) { return nil, err } klog.Warningf(util.Log(ctx, "unable to read omap keys: pool or key missing: %v"), err) @@ -656,9 +648,8 @@ func (conn *Connection) GetImageAttributes(ctx context.Context, pool, objectUUID if snapSource { imageAttributes.SourceName, found = values[cj.cephSnapSourceKey] if !found { - return nil, util.NewErrKeyNotFound( - cj.cephSnapSourceKey, - fmt.Errorf("no snap source in omap for %q", cj.cephUUIDDirectoryPrefix+objectUUID)) + return nil, fmt.Errorf("%w: no snap source in omap for %q", + util.ErrKeyNotFound, cj.cephUUIDDirectoryPrefix+objectUUID) } } diff --git a/internal/rbd/controllerserver.go b/internal/rbd/controllerserver.go index d59e7a93c..0e0279fbc 100644 --- a/internal/rbd/controllerserver.go +++ b/internal/rbd/controllerserver.go @@ -391,8 +391,7 @@ func (cs *ControllerServer) createVolumeFromSnapshot(ctx context.Context, cr *ut err := genSnapFromSnapID(ctx, rbdSnap, snapshotID, cr) if err != nil { - var epnf util.ErrPoolNotFound - if errors.As(err, &epnf) { + if errors.Is(err, util.ErrPoolNotFound) { klog.Errorf(util.Log(ctx, "failed to get backend snapshot for %s: %v"), snapshotID, err) return status.Error(codes.InvalidArgument, err.Error()) } @@ -581,8 +580,7 @@ func (cs *ControllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVol rbdVol, err = genVolFromVolID(ctx, volumeID, cr, req.GetSecrets()) if err != nil { - var epnf util.ErrPoolNotFound - if errors.As(err, &epnf) { + if errors.Is(err, util.ErrPoolNotFound) { klog.Warningf(util.Log(ctx, "failed to get backend volume for %s: %v"), volumeID, err) return &csi.DeleteVolumeResponse{}, nil } @@ -590,8 +588,7 @@ func (cs *ControllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVol // if error is ErrKeyNotFound, then a previous attempt at deletion was complete // or partially complete (image and imageOMap are garbage collected already), hence return // success as deletion is complete - var eknf util.ErrKeyNotFound - if errors.As(err, &eknf) { + if errors.Is(err, util.ErrKeyNotFound) { klog.Warningf(util.Log(ctx, "Failed to volume options for %s: %v"), volumeID, err) return &csi.DeleteVolumeResponse{}, nil } @@ -717,11 +714,10 @@ func (cs *ControllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateS rbdVol, err = genVolFromVolID(ctx, req.GetSourceVolumeId(), cr, req.GetSecrets()) if err != nil { var einf ErrImageNotFound - var epnf util.ErrPoolNotFound // nolint:gocritic // this ifElseChain can not be rewritten to a switch statement if errors.As(err, &einf) { err = status.Errorf(codes.NotFound, "source Volume ID %s not found", req.GetSourceVolumeId()) - } else if errors.As(err, &epnf) { + } else if errors.Is(err, util.ErrPoolNotFound) { klog.Errorf(util.Log(ctx, "failed to get backend volume for %s: %v"), req.GetSourceVolumeId(), err) err = status.Errorf(codes.NotFound, err.Error()) } else { @@ -765,8 +761,7 @@ func (cs *ControllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateS // check for the requested source volume id and already allocated source volume id found, err := checkSnapCloneExists(ctx, rbdVol, rbdSnap, cr) if err != nil { - var esnc util.ErrSnapNameConflict - if errors.As(err, &esnc) { + if errors.Is(err, util.ErrSnapNameConflict) { return nil, status.Error(codes.AlreadyExists, err.Error()) } return nil, status.Errorf(codes.Internal, err.Error()) @@ -983,8 +978,7 @@ func (cs *ControllerServer) DeleteSnapshot(ctx context.Context, req *csi.DeleteS if err = genSnapFromSnapID(ctx, rbdSnap, snapshotID, cr); err != nil { // if error is ErrPoolNotFound, the pool is already deleted we dont // need to worry about deleting snapshot or omap data, return success - var epnf util.ErrPoolNotFound - if errors.As(err, &epnf) { + if errors.Is(err, util.ErrPoolNotFound) { klog.Warningf(util.Log(ctx, "failed to get backend snapshot for %s: %v"), snapshotID, err) return &csi.DeleteSnapshotResponse{}, nil } @@ -992,8 +986,7 @@ func (cs *ControllerServer) DeleteSnapshot(ctx context.Context, req *csi.DeleteS // if error is ErrKeyNotFound, then a previous attempt at deletion was complete // or partially complete (snap and snapOMap are garbage collected already), hence return // success as deletion is complete - var eknf util.ErrKeyNotFound - if errors.As(err, &eknf) { + if errors.Is(err, util.ErrKeyNotFound) { return &csi.DeleteSnapshotResponse{}, nil } @@ -1089,11 +1082,10 @@ func (cs *ControllerServer) ControllerExpandVolume(ctx context.Context, req *csi rbdVol, err = genVolFromVolID(ctx, volID, cr, req.GetSecrets()) if err != nil { var einf ErrImageNotFound - var epnf util.ErrPoolNotFound // nolint:gocritic // this ifElseChain can not be rewritten to a switch statement if errors.As(err, &einf) { err = status.Errorf(codes.NotFound, "volume ID %s not found", volID) - } else if errors.As(err, &epnf) { + } else if errors.Is(err, util.ErrPoolNotFound) { klog.Errorf(util.Log(ctx, "failed to get backend volume for %s: %v"), volID, err) err = status.Errorf(codes.NotFound, err.Error()) } else { diff --git a/internal/util/cephcmds.go b/internal/util/cephcmds.go index e9d1b2be5..62b465e93 100644 --- a/internal/util/cephcmds.go +++ b/internal/util/cephcmds.go @@ -61,7 +61,8 @@ func GetPoolID(monitors string, cr *Credentials, poolName string) (int64, error) id, err := conn.GetPoolByName(poolName) if errors.Is(err, rados.ErrNotFound) { - return InvalidPoolID, ErrPoolNotFound{poolName, fmt.Errorf("pool (%s) not found in Ceph cluster", poolName)} + return InvalidPoolID, fmt.Errorf("%w: pool (%s) not found in Ceph cluster", + ErrPoolNotFound, poolName) } else if err != nil { return InvalidPoolID, err } @@ -80,7 +81,8 @@ func GetPoolName(monitors string, cr *Credentials, poolID int64) (string, error) name, err := conn.GetPoolByID(poolID) if err != nil { - return "", ErrPoolNotFound{string(poolID), fmt.Errorf("pool ID (%d) not found in Ceph cluster", poolID)} + return "", fmt.Errorf("%w: pool ID (%d) not found in Ceph cluster", + ErrPoolNotFound, poolID) } return name, nil } @@ -117,9 +119,8 @@ func CreateObject(ctx context.Context, monitors string, cr *Credentials, poolNam ioctx, err := conn.GetIoctx(poolName) if err != nil { - var epnf ErrPoolNotFound - if errors.As(err, &epnf) { - err = ErrObjectNotFound{poolName, err} + if errors.Is(err, ErrPoolNotFound) { + err = JoinErrors(ErrObjectNotFound, err) } return err } @@ -131,7 +132,7 @@ func CreateObject(ctx context.Context, monitors string, cr *Credentials, poolNam err = ioctx.Create(objectName, rados.CreateExclusive) if errors.Is(err, rados.ErrObjectExists) { - return ErrObjectExists{objectName, err} + return JoinErrors(ErrObjectExists, err) } else if err != nil { klog.Errorf(Log(ctx, "failed creating omap (%s) in pool (%s): (%v)"), objectName, poolName, err) return err @@ -152,9 +153,8 @@ func RemoveObject(ctx context.Context, monitors string, cr *Credentials, poolNam ioctx, err := conn.GetIoctx(poolName) if err != nil { - var epnf ErrPoolNotFound - if errors.As(err, &epnf) { - err = ErrObjectNotFound{poolName, err} + if errors.Is(err, ErrPoolNotFound) { + err = JoinErrors(ErrObjectNotFound, err) } return err } @@ -166,7 +166,7 @@ func RemoveObject(ctx context.Context, monitors string, cr *Credentials, poolNam err = ioctx.Delete(oMapName) if errors.Is(err, rados.ErrNotFound) { - return ErrObjectNotFound{oMapName, err} + return JoinErrors(ErrObjectNotFound, err) } else if err != nil { klog.Errorf(Log(ctx, "failed removing omap (%s) in pool (%s): (%v)"), oMapName, poolName, err) return err diff --git a/internal/util/connection.go b/internal/util/connection.go index 1426ddeb7..172faffe8 100644 --- a/internal/util/connection.go +++ b/internal/util/connection.go @@ -74,7 +74,7 @@ func (cc *ClusterConnection) GetIoctx(pool string) (*rados.IOContext, error) { if err != nil { // ErrNotFound indicates the Pool was not found if errors.Is(err, rados.ErrNotFound) { - err = ErrPoolNotFound{pool, err} + err = JoinErrors(ErrPoolNotFound, err) } else { err = fmt.Errorf("failed to open IOContext for pool %s: %w", pool, err) } diff --git a/internal/util/errors.go b/internal/util/errors.go index a794154cf..444d6fb3b 100644 --- a/internal/util/errors.go +++ b/internal/util/errors.go @@ -16,98 +16,46 @@ limitations under the License. package util -// ErrKeyNotFound is returned when requested key in omap is not found. -type ErrKeyNotFound struct { - keyName string - err error +import ( + "errors" + "fmt" +) + +var ( + // ErrKeyNotFound is returned when requested key in omap is not found. + ErrKeyNotFound = errors.New("key not found") + // ErrObjectExists is returned when named omap is already present in rados. + ErrObjectExists = errors.New("object exists") + // ErrObjectNotFound is returned when named omap is not found in rados. + ErrObjectNotFound = errors.New("object not found") + // ErrSnapNameConflict is generated when a requested CSI snap name already exists on RBD but with + // different properties, and hence is in conflict with the passed in CSI volume name. + ErrSnapNameConflict = errors.New("snapshot name conflict") + // ErrPoolNotFound is returned when pool is not found. + ErrPoolNotFound = errors.New("pool not found") +) + +type errorPair struct { + first error + second error } -// NewErrKeyNotFound returns a new ErrKeyNotFound error. -func NewErrKeyNotFound(keyName string, err error) ErrKeyNotFound { - return ErrKeyNotFound{keyName, err} +func (e errorPair) Error() string { + return fmt.Sprintf("%v: %v", e.first, e.second) } -// Error returns the error string for ErrKeyNotFound. -func (e ErrKeyNotFound) Error() string { - return e.err.Error() +// Is checks if target error is wrapped in the first error. +func (e errorPair) Is(target error) bool { + return errors.Is(e.first, target) } -// Unwrap returns the encapsulated error. -func (e ErrKeyNotFound) Unwrap() error { - return e.err +// Unwrap returns the second error. +func (e errorPair) Unwrap() error { + return e.second } -// ErrObjectExists is returned when named omap is already present in rados. -type ErrObjectExists struct { - objectName string - err error -} - -// Error returns the error string for ErrObjectExists. -func (e ErrObjectExists) Error() string { - return e.err.Error() -} - -// Unwrap returns the encapsulated error. -func (e ErrObjectExists) Unwrap() error { - return e.err -} - -// ErrObjectNotFound is returned when named omap is not found in rados. -type ErrObjectNotFound struct { - oMapName string - err error -} - -// Error returns the error string for ErrObjectNotFound. -func (e ErrObjectNotFound) Error() string { - return e.err.Error() -} - -// Unwrap returns the encapsulated error. -func (e ErrObjectNotFound) Unwrap() error { - return e.err -} - -// ErrSnapNameConflict is generated when a requested CSI snap name already exists on RBD but with -// different properties, and hence is in conflict with the passed in CSI volume name. -type ErrSnapNameConflict struct { - requestName string - err error -} - -// Error returns the error string for ErrSnapNameConflict. -func (e ErrSnapNameConflict) Error() string { - return e.err.Error() -} - -// Unwrap returns the encapsulated error. -func (e ErrSnapNameConflict) Unwrap() error { - return e.err -} - -// NewErrSnapNameConflict returns a ErrSnapNameConflict error when CSI snap name already exists. -func NewErrSnapNameConflict(name string, err error) ErrSnapNameConflict { - return ErrSnapNameConflict{name, err} -} - -// ErrPoolNotFound is returned when pool is not found. -type ErrPoolNotFound struct { - Pool string - Err error -} - -// Error returns the error string for ErrPoolNotFound. -func (e ErrPoolNotFound) Error() string { - return e.Err.Error() -} - -// Unwrap returns the encapsulated error. -func (e ErrPoolNotFound) Unwrap() error { - return e.Err -} - -// NewErrPoolNotFound returns a new ErrPoolNotFound error. -func NewErrPoolNotFound(pool string, err error) ErrPoolNotFound { - return ErrPoolNotFound{pool, err} +// JoinErrors combines two errors. Of the returned error, Is() follows the first +// branch, Unwrap() folllows the second branch. +func JoinErrors(e1, e2 error) error { + return errorPair{e1, e2} }