cleanup: move ErrImageNotFound from rbd/errors to util/errors

Signed-off-by: Praveen M <m.praveen@ibm.com>
This commit is contained in:
Praveen M 2025-02-06 11:45:42 +05:30 committed by mergify[bot]
parent 75c70e158f
commit 6414e94401
12 changed files with 33 additions and 32 deletions

View File

@ -68,7 +68,7 @@ func (ekrs *EncryptionKeyRotationServer) EncryptionKeyRotate(
rbdVol, err := mgr.GetVolumeByID(ctx, volID) rbdVol, err := mgr.GetVolumeByID(ctx, volID)
if err != nil { if err != nil {
switch { switch {
case errors.Is(err, rbd.ErrImageNotFound): case errors.Is(err, util.ErrImageNotFound):
err = status.Errorf(codes.NotFound, "volume ID %s not found", volID) err = status.Errorf(codes.NotFound, "volume ID %s not found", volID)
case errors.Is(err, util.ErrPoolNotFound): case errors.Is(err, util.ErrPoolNotFound):
log.ErrorLog(ctx, "failed to get backend volume for %s: %v", volID, err) log.ErrorLog(ctx, "failed to get backend volume for %s: %v", volID, err)

View File

@ -651,7 +651,7 @@ func (rs *ReplicationServer) ResyncVolume(ctx context.Context,
sts, err := mirror.GetGlobalMirroringStatus(ctx) sts, err := mirror.GetGlobalMirroringStatus(ctx)
if err != nil { if err != nil {
// the image gets recreated after issuing resync // the image gets recreated after issuing resync
if errors.Is(err, corerbd.ErrImageNotFound) { if errors.Is(err, util.ErrImageNotFound) {
// caller retries till RBD syncs an initial version of the image to // caller retries till RBD syncs an initial version of the image to
// report its status in the resync call. Ideally, this line will not // report its status in the resync call. Ideally, this line will not
// be executed as the error would get returned due to getMirroringInfo // be executed as the error would get returned due to getMirroringInfo
@ -785,7 +785,7 @@ func getGRPCError(err error) error {
} }
errorStatusMap := map[error]codes.Code{ errorStatusMap := map[error]codes.Code{
corerbd.ErrImageNotFound: codes.NotFound, util.ErrImageNotFound: codes.NotFound,
util.ErrPoolNotFound: codes.NotFound, util.ErrPoolNotFound: codes.NotFound,
corerbd.ErrInvalidArgument: codes.InvalidArgument, corerbd.ErrInvalidArgument: codes.InvalidArgument,
corerbd.ErrFlattenInProgress: codes.Aborted, corerbd.ErrFlattenInProgress: codes.Aborted,
@ -835,7 +835,7 @@ func (rs *ReplicationServer) GetVolumeReplicationInfo(ctx context.Context,
log.ErrorLog(ctx, "failed to get volume with id %q: %v", volumeID, err) log.ErrorLog(ctx, "failed to get volume with id %q: %v", volumeID, err)
switch { switch {
case errors.Is(err, corerbd.ErrImageNotFound): case errors.Is(err, util.ErrImageNotFound):
err = status.Error(codes.NotFound, err.Error()) err = status.Error(codes.NotFound, err.Error())
case errors.Is(err, util.ErrPoolNotFound): case errors.Is(err, util.ErrPoolNotFound):
err = status.Error(codes.NotFound, err.Error()) err = status.Error(codes.NotFound, err.Error())
@ -872,7 +872,7 @@ func (rs *ReplicationServer) GetVolumeReplicationInfo(ctx context.Context,
if err != nil { if err != nil {
log.ErrorLog(ctx, "failed to get status for mirror %q: %v", mirror, err) log.ErrorLog(ctx, "failed to get status for mirror %q: %v", mirror, err)
if errors.Is(err, corerbd.ErrImageNotFound) { if errors.Is(err, util.ErrImageNotFound) {
return nil, status.Error(codes.Aborted, err.Error()) return nil, status.Error(codes.Aborted, err.Error())
} }

View File

@ -597,8 +597,8 @@ func TestGetGRPCError(t *testing.T) {
}, },
{ {
name: "ErrImageNotFound", name: "ErrImageNotFound",
err: corerbd.ErrImageNotFound, err: util.ErrImageNotFound,
expectedErr: status.Error(codes.NotFound, corerbd.ErrImageNotFound.Error()), expectedErr: status.Error(codes.NotFound, util.ErrImageNotFound.Error()),
}, },
{ {
name: "ErrPoolNotFound", name: "ErrPoolNotFound",

View File

@ -21,6 +21,7 @@ import (
"errors" "errors"
"fmt" "fmt"
"github.com/ceph/ceph-csi/internal/util"
"github.com/ceph/ceph-csi/internal/util/k8s" "github.com/ceph/ceph-csi/internal/util/k8s"
"github.com/ceph/ceph-csi/internal/util/log" "github.com/ceph/ceph-csi/internal/util/log"
@ -66,7 +67,7 @@ func (rv *rbdVolume) checkCloneImage(ctx context.Context, parentVol *rbdVolume)
return true, nil return true, nil
case errors.Is(err, ErrImageNotFound): case errors.Is(err, util.ErrImageNotFound):
// as the temp clone does not exist,check snapshot exists on parent volume // as the temp clone does not exist,check snapshot exists on parent volume
// snapshot name is same as temporary clone image // snapshot name is same as temporary clone image
snap.RbdImageName = tempClone.RbdImageName snap.RbdImageName = tempClone.RbdImageName

View File

@ -575,7 +575,7 @@ func (cs *ControllerServer) repairExistingVolume(ctx context.Context, req *csi.C
func flattenTemporaryClonedImages(ctx context.Context, rbdVol *rbdVolume, cr *util.Credentials) error { func flattenTemporaryClonedImages(ctx context.Context, rbdVol *rbdVolume, cr *util.Credentials) error {
snaps, children, err := rbdVol.listSnapAndChildren() snaps, children, err := rbdVol.listSnapAndChildren()
if err != nil { if err != nil {
if errors.Is(err, ErrImageNotFound) { if errors.Is(err, util.ErrImageNotFound) {
return status.Error(codes.InvalidArgument, err.Error()) return status.Error(codes.InvalidArgument, err.Error())
} }
@ -831,7 +831,7 @@ func checkContentSource(
rbdvol, err := GenVolFromVolID(ctx, volID, cr, req.GetSecrets()) rbdvol, err := GenVolFromVolID(ctx, volID, cr, req.GetSecrets())
if err != nil { if err != nil {
log.ErrorLog(ctx, "failed to get backend image for %s: %v", volID, err) log.ErrorLog(ctx, "failed to get backend image for %s: %v", volID, err)
if !errors.Is(err, ErrImageNotFound) { if !errors.Is(err, util.ErrImageNotFound) {
return nil, nil, status.Error(codes.Internal, err.Error()) return nil, nil, status.Error(codes.Internal, err.Error())
} }
@ -871,7 +871,7 @@ func (cs *ControllerServer) checkErrAndUndoReserve(
return &csi.DeleteVolumeResponse{}, nil return &csi.DeleteVolumeResponse{}, nil
} }
if errors.Is(err, ErrImageNotFound) { if errors.Is(err, util.ErrImageNotFound) {
notFoundErr := rbdVol.ensureImageCleanup(ctx) notFoundErr := rbdVol.ensureImageCleanup(ctx)
if notFoundErr != nil { if notFoundErr != nil {
return nil, status.Errorf(codes.Internal, "failed to cleanup image %q: %v", rbdVol, notFoundErr) return nil, status.Errorf(codes.Internal, "failed to cleanup image %q: %v", rbdVol, notFoundErr)
@ -946,7 +946,7 @@ func (cs *ControllerServer) DeleteVolume(
return nil, status.Error(codes.InvalidArgument, pErr.Error()) return nil, status.Error(codes.InvalidArgument, pErr.Error())
} }
pErr = deleteMigratedVolume(ctx, pmVolID, cr) pErr = deleteMigratedVolume(ctx, pmVolID, cr)
if pErr != nil && !errors.Is(pErr, ErrImageNotFound) { if pErr != nil && !errors.Is(pErr, util.ErrImageNotFound) {
return nil, status.Error(codes.Internal, pErr.Error()) return nil, status.Error(codes.Internal, pErr.Error())
} }
@ -1118,7 +1118,7 @@ func (cs *ControllerServer) CreateSnapshot(
}() }()
if err != nil { if err != nil {
switch { switch {
case errors.Is(err, ErrImageNotFound): case errors.Is(err, util.ErrImageNotFound):
err = status.Errorf(codes.NotFound, "source Volume ID %s not found", req.GetSourceVolumeId()) err = status.Errorf(codes.NotFound, "source Volume ID %s not found", req.GetSourceVolumeId())
case errors.Is(err, util.ErrPoolNotFound): case errors.Is(err, util.ErrPoolNotFound):
log.ErrorLog(ctx, "failed to get backend volume for %s: %v", req.GetSourceVolumeId(), err) log.ErrorLog(ctx, "failed to get backend volume for %s: %v", req.GetSourceVolumeId(), err)
@ -1459,7 +1459,7 @@ func (cs *ControllerServer) DeleteSnapshot(
// if the error is ErrImageNotFound, We need to cleanup the image from // if the error is ErrImageNotFound, We need to cleanup the image from
// trash and remove the metadata in OMAP. // trash and remove the metadata in OMAP.
if errors.Is(err, ErrImageNotFound) { if errors.Is(err, util.ErrImageNotFound) {
log.UsefulLog(ctx, "cleaning up leftovers of snapshot %s: %v", snapshotID, err) log.UsefulLog(ctx, "cleaning up leftovers of snapshot %s: %v", snapshotID, err)
err = cleanUpImageAndSnapReservation(ctx, rbdSnap, cr) err = cleanUpImageAndSnapReservation(ctx, rbdSnap, cr)
@ -1562,7 +1562,7 @@ func (cs *ControllerServer) ControllerExpandVolume(
rbdVol, err := genVolFromVolIDWithMigration(ctx, volID, cr, req.GetSecrets()) rbdVol, err := genVolFromVolIDWithMigration(ctx, volID, cr, req.GetSecrets())
if err != nil { if err != nil {
switch { switch {
case errors.Is(err, ErrImageNotFound): case errors.Is(err, util.ErrImageNotFound):
err = status.Errorf(codes.NotFound, "volume ID %s not found", volID) err = status.Errorf(codes.NotFound, "volume ID %s not found", volID)
case errors.Is(err, util.ErrPoolNotFound): case errors.Is(err, util.ErrPoolNotFound):
log.ErrorLog(ctx, "failed to get backend volume for %s: %v", volID, err) log.ErrorLog(ctx, "failed to get backend volume for %s: %v", volID, err)

View File

@ -19,8 +19,6 @@ package rbd
import "errors" import "errors"
var ( var (
// ErrImageNotFound is returned when image name is not found in the cluster on the given pool and/or namespace.
ErrImageNotFound = errors.New("image not found")
// ErrSnapNotFound is returned when snap name passed is not found in the list of snapshots for the // ErrSnapNotFound is returned when snap name passed is not found in the list of snapshots for the
// given image. // given image.
ErrSnapNotFound = errors.New("snapshot not found") ErrSnapNotFound = errors.New("snapshot not found")

View File

@ -174,7 +174,7 @@ func (mgr *rbdManager) GetVolumeByID(ctx context.Context, id string) (types.Volu
volume, err := GenVolFromVolID(ctx, id, creds, mgr.secrets) volume, err := GenVolFromVolID(ctx, id, creds, mgr.secrets)
if err != nil { if err != nil {
switch { switch {
case errors.Is(err, ErrImageNotFound): case errors.Is(err, util.ErrImageNotFound):
err = fmt.Errorf("volume %s not found: %w", id, err) err = fmt.Errorf("volume %s not found: %w", id, err)
return nil, err return nil, err
@ -199,7 +199,7 @@ func (mgr *rbdManager) GetSnapshotByID(ctx context.Context, id string) (types.Sn
snapshot, err := genSnapFromSnapID(ctx, id, creds, mgr.secrets) snapshot, err := genSnapFromSnapID(ctx, id, creds, mgr.secrets)
if err != nil { if err != nil {
switch { switch {
case errors.Is(err, ErrImageNotFound): case errors.Is(err, util.ErrImageNotFound):
err = fmt.Errorf("volume %s not found: %w", id, err) err = fmt.Errorf("volume %s not found: %w", id, err)
return nil, err return nil, err
@ -467,7 +467,7 @@ func (mgr *rbdManager) CreateVolumeGroupSnapshot(
return vgs, nil return vgs, nil
} }
} else if err != nil && !errors.Is(ErrImageNotFound, err) { } else if err != nil && !errors.Is(err, util.ErrImageNotFound) {
// ErrImageNotFound can be returned if the VolumeGroupSnapshot // ErrImageNotFound can be returned if the VolumeGroupSnapshot
// could not be found. It is expected that it does not exist // could not be found. It is expected that it does not exist
// yet, in which case it will be created below. // yet, in which case it will be created below.

View File

@ -172,7 +172,7 @@ func checkSnapCloneExists(
// Fetch on-disk image attributes // Fetch on-disk image attributes
err = vol.getImageInfo() err = vol.getImageInfo()
if err != nil { if err != nil {
if errors.Is(err, ErrImageNotFound) { if errors.Is(err, util.ErrImageNotFound) {
err = parentVol.deleteSnapshot(ctx, rbdSnap) err = parentVol.deleteSnapshot(ctx, rbdSnap)
if err != nil { if err != nil {
if !errors.Is(err, ErrSnapNotFound) { if !errors.Is(err, ErrSnapNotFound) {
@ -298,7 +298,7 @@ func (rv *rbdVolume) Exists(ctx context.Context, parentVol *rbdVolume) (bool, er
// Fetch on-disk image attributes and compare against request // Fetch on-disk image attributes and compare against request
err = rv.getImageInfo() err = rv.getImageInfo()
if err != nil { if err != nil {
if errors.Is(err, ErrImageNotFound) { if errors.Is(err, util.ErrImageNotFound) {
// Need to check cloned info here not on createvolume, // Need to check cloned info here not on createvolume,
if parentVol != nil { if parentVol != nil {
found, cErr := rv.checkCloneImage(ctx, parentVol) found, cErr := rv.checkCloneImage(ctx, parentVol)

View File

@ -524,7 +524,7 @@ func (ri *rbdImage) open() (*librbd.Image, error) {
image, err := librbd.OpenImage(ri.ioctx, ri.RbdImageName, librbd.NoSnapshot) image, err := librbd.OpenImage(ri.ioctx, ri.RbdImageName, librbd.NoSnapshot)
if err != nil { if err != nil {
if errors.Is(err, librbd.ErrNotFound) { if errors.Is(err, librbd.ErrNotFound) {
err = fmt.Errorf("Failed as %w (internal %w)", ErrImageNotFound, err) err = fmt.Errorf("Failed as %w (internal %w)", util.ErrImageNotFound, err)
} }
return nil, err return nil, err
@ -542,7 +542,7 @@ func (ri *rbdImage) open() (*librbd.Image, error) {
func (ri *rbdImage) isInUse() (bool, error) { func (ri *rbdImage) isInUse() (bool, error) {
image, err := ri.open() image, err := ri.open()
if err != nil { if err != nil {
if errors.Is(err, ErrImageNotFound) || errors.Is(err, util.ErrPoolNotFound) { if errors.Is(err, util.ErrImageNotFound) || errors.Is(err, util.ErrPoolNotFound) {
return false, err return false, err
} }
// any error should assume something else is using the image // any error should assume something else is using the image
@ -681,7 +681,7 @@ func (ri *rbdImage) Delete(ctx context.Context) error {
err = rbdImage.Trash(0) err = rbdImage.Trash(0)
if err != nil { if err != nil {
if errors.Is(err, librbd.ErrNotFound) { if errors.Is(err, librbd.ErrNotFound) {
return fmt.Errorf("Failed as %w (internal %w)", ErrImageNotFound, err) return fmt.Errorf("Failed as %w (internal %w)", util.ErrImageNotFound, err)
} }
log.ErrorLog(ctx, "failed to delete rbd image: %s, error: %v", ri, err) log.ErrorLog(ctx, "failed to delete rbd image: %s, error: %v", ri, err)
@ -731,7 +731,7 @@ func (rv *rbdVolume) DeleteTempImage(ctx context.Context) error {
tempClone := rv.generateTempClone() tempClone := rv.generateTempClone()
err := tempClone.Delete(ctx) err := tempClone.Delete(ctx)
if err != nil { if err != nil {
if errors.Is(err, ErrImageNotFound) { if errors.Is(err, util.ErrImageNotFound) {
return tempClone.ensureImageCleanup(ctx) return tempClone.ensureImageCleanup(ctx)
} else { } else {
// return error if it is not ErrImageNotFound // return error if it is not ErrImageNotFound
@ -770,7 +770,7 @@ func (ri *rbdImage) getCloneDepth(ctx context.Context) (uint, error) {
// if the parent image is moved to trash the name will be present // if the parent image is moved to trash the name will be present
// in rbd image info but the image will be in trash, in that case // in rbd image info but the image will be in trash, in that case
// return the found depth // return the found depth
if errors.Is(err, ErrImageNotFound) { if errors.Is(err, util.ErrImageNotFound) {
return depth, nil return depth, nil
} }
log.ErrorLog(ctx, "failed to check depth on image %s: %s", &vol, err) log.ErrorLog(ctx, "failed to check depth on image %s: %s", &vol, err)
@ -956,7 +956,7 @@ func (ri *rbdImage) checkImageChainHasFeature(ctx context.Context, feature uint6
// is in the trash, when we try to open the parent image to get its // is in the trash, when we try to open the parent image to get its
// information it fails because it is already in trash. We should // information it fails because it is already in trash. We should
// treat error as nil if the parent is not found. // treat error as nil if the parent is not found.
if errors.Is(err, ErrImageNotFound) { if errors.Is(err, util.ErrImageNotFound) {
return false, nil return false, nil
} }
log.ErrorLog(ctx, "failed to get image info for %s: %s", rbdImg.String(), err) log.ErrorLog(ctx, "failed to get image info for %s: %s", rbdImg.String(), err)
@ -1312,7 +1312,7 @@ func shouldRetryVolumeGeneration(err error) bool {
// Continue searching for specific known errors // Continue searching for specific known errors
return (errors.Is(err, util.ErrKeyNotFound) || return (errors.Is(err, util.ErrKeyNotFound) ||
errors.Is(err, util.ErrPoolNotFound) || errors.Is(err, util.ErrPoolNotFound) ||
errors.Is(err, ErrImageNotFound) || errors.Is(err, util.ErrImageNotFound) ||
errors.Is(err, rados.ErrPermissionDenied)) errors.Is(err, rados.ErrPermissionDenied))
} }

View File

@ -418,7 +418,7 @@ func Test_shouldRetryVolumeGeneration(t *testing.T) {
}, },
{ {
name: "ErrImageNotFound (continue searching)", name: "ErrImageNotFound (continue searching)",
args: args{err: ErrImageNotFound}, args: args{err: util.ErrImageNotFound},
want: true, // Known error, continue searching want: true, // Known error, continue searching
}, },
{ {

View File

@ -82,7 +82,7 @@ func cleanUpSnapshot(
) error { ) error {
err := parentVol.deleteSnapshot(ctx, rbdSnap) err := parentVol.deleteSnapshot(ctx, rbdSnap)
if err != nil { if err != nil {
if !errors.Is(err, ErrImageNotFound) && !errors.Is(err, ErrSnapNotFound) { if !errors.Is(err, util.ErrImageNotFound) && !errors.Is(err, ErrSnapNotFound) {
log.ErrorLog(ctx, "failed to delete snapshot %q: %v", rbdSnap, err) log.ErrorLog(ctx, "failed to delete snapshot %q: %v", rbdSnap, err)
return err return err
@ -92,7 +92,7 @@ func cleanUpSnapshot(
if rbdVol != nil { if rbdVol != nil {
err := rbdVol.Delete(ctx) err := rbdVol.Delete(ctx)
if err != nil { if err != nil {
if !errors.Is(err, ErrImageNotFound) { if !errors.Is(err, util.ErrImageNotFound) {
log.ErrorLog(ctx, "failed to delete rbd image %q with error: %v", rbdVol, err) log.ErrorLog(ctx, "failed to delete rbd image %q with error: %v", rbdVol, err)
return err return err

View File

@ -21,6 +21,8 @@ import (
) )
var ( var (
// ErrImageNotFound is returned when image name is not found in the cluster on the given pool and/or namespace.
ErrImageNotFound = errors.New("image not found")
// ErrKeyNotFound is returned when requested key in omap is not found. // ErrKeyNotFound is returned when requested key in omap is not found.
ErrKeyNotFound = errors.New("key not found") ErrKeyNotFound = errors.New("key not found")
// ErrObjectExists is returned when named omap is already present in rados. // ErrObjectExists is returned when named omap is already present in rados.