From 267c70919457811c3e86684827a922b831300426 Mon Sep 17 00:00:00 2001 From: Niels de Vos Date: Thu, 25 Jun 2020 10:35:19 +0200 Subject: [PATCH] cleanup: use errors.As() in rbd/controllerserver methods See-also: https://github.com/golang/go/wiki/ErrorValueFAQ#how-should-i-change-my-error-handling-code-to-work-with-the-new-features Signed-off-by: Niels de Vos --- internal/rbd/controllerserver.go | 56 ++++++++++++++++++++------------ 1 file changed, 36 insertions(+), 20 deletions(-) diff --git a/internal/rbd/controllerserver.go b/internal/rbd/controllerserver.go index 1bc5d3f9c..564d72709 100644 --- a/internal/rbd/controllerserver.go +++ b/internal/rbd/controllerserver.go @@ -18,6 +18,7 @@ package rbd import ( "context" + "errors" csicommon "github.com/ceph/ceph-csi/internal/csi-common" "github.com/ceph/ceph-csi/internal/journal" @@ -199,7 +200,8 @@ func (cs *ControllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol found, err := rbdVol.Exists(ctx) if err != nil { - if _, ok := err.(ErrVolNameConflict); ok { + var evnc ErrVolNameConflict + if errors.As(err, &evnc) { return nil, status.Error(codes.AlreadyExists, err.Error()) } return nil, status.Error(codes.Internal, err.Error()) @@ -395,11 +397,13 @@ func (cs *ControllerServer) checkSnapshotSource(ctx context.Context, req *csi.Cr rbdSnap := &rbdSnapshot{} if err := genSnapFromSnapID(ctx, rbdSnap, snapshotID, cr); err != nil { - if _, ok := err.(ErrSnapNotFound); !ok { + var esnf ErrSnapNotFound + if !errors.As(err, &esnf) { return nil, status.Error(codes.Internal, err.Error()) } - if _, ok := err.(util.ErrPoolNotFound); ok { + var epnf util.ErrPoolNotFound + if errors.As(err, &epnf) { klog.Errorf(util.Log(ctx, "failed to get backend snapshot for %s: %v"), snapshotID, err) return nil, status.Error(codes.InvalidArgument, err.Error()) } @@ -496,14 +500,16 @@ func (cs *ControllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVol rbdVol, err = genVolFromVolID(ctx, volumeID, cr, req.GetSecrets()) if err != nil { - switch err.(type) { - case util.ErrPoolNotFound: + var epnf util.ErrPoolNotFound + if errors.As(err, &epnf) { klog.Warningf(util.Log(ctx, "failed to get backend volume for %s: %v"), volumeID, err) return &csi.DeleteVolumeResponse{}, nil + } // If error is ErrInvalidVolID it could be a version 1.0.0 or lower volume, attempt // to process it as such - case ErrInvalidVolID: + var eivi ErrInvalidVolID + if errors.As(err, &eivi) { if isLegacyVolumeID(volumeID) { klog.V(2).Infof(util.Log(ctx, "attempting deletion of potential legacy volume (%s)"), volumeID) return cs.DeleteLegacyVolume(ctx, req, cr) @@ -511,17 +517,20 @@ func (cs *ControllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVol // Consider unknown volumeID as a successfully deleted volume return &csi.DeleteVolumeResponse{}, nil + } // 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 - case util.ErrKeyNotFound: + var eknf util.ErrKeyNotFound + if errors.As(err, &eknf) { klog.Warningf(util.Log(ctx, "Failed to volume options for %s: %v"), volumeID, err) return &csi.DeleteVolumeResponse{}, nil + } // All errors other than ErrImageNotFound should return an error back to the caller - case ErrImageNotFound: - default: + var einf ErrImageNotFound + if !errors.As(err, &einf) { return nil, status.Error(codes.Internal, err.Error()) } @@ -624,13 +633,15 @@ func (cs *ControllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateS // Fetch source volume information rbdVol, err = genVolFromVolID(ctx, req.GetSourceVolumeId(), cr, req.GetSecrets()) if err != nil { - switch err.(type) { - case ErrImageNotFound: + 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()) - case util.ErrPoolNotFound: + } else if errors.As(err, &epnf) { klog.Errorf(util.Log(ctx, "failed to get backend volume for %s: %v"), req.GetSourceVolumeId(), err) err = status.Errorf(codes.NotFound, err.Error()) - default: + } else { err = status.Errorf(codes.Internal, err.Error()) } return nil, err @@ -664,7 +675,8 @@ 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 { - if _, ok := err.(util.ErrSnapNameConflict); ok { + var esnc util.ErrSnapNameConflict + if errors.As(err, &esnc) { return nil, status.Error(codes.AlreadyExists, err.Error()) } return nil, status.Errorf(codes.Internal, err.Error()) @@ -866,7 +878,8 @@ 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 - if _, ok := err.(util.ErrPoolNotFound); ok { + var epnf util.ErrPoolNotFound + if errors.As(err, &epnf) { klog.Warningf(util.Log(ctx, "failed to get backend snapshot for %s: %v"), snapshotID, err) return &csi.DeleteSnapshotResponse{}, nil } @@ -874,7 +887,8 @@ 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 - if _, ok := err.(util.ErrKeyNotFound); ok { + var eknf util.ErrKeyNotFound + if errors.As(err, &eknf) { return &csi.DeleteSnapshotResponse{}, nil } @@ -975,13 +989,15 @@ func (cs *ControllerServer) ControllerExpandVolume(ctx context.Context, req *csi rbdVol, err = genVolFromVolID(ctx, volID, cr, req.GetSecrets()) if err != nil { - switch err.(type) { - case ErrImageNotFound: + 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) - case util.ErrPoolNotFound: + } else if errors.As(err, &epnf) { klog.Errorf(util.Log(ctx, "failed to get backend volume for %s: %v"), volID, err) err = status.Errorf(codes.NotFound, err.Error()) - default: + } else { err = status.Errorf(codes.Internal, err.Error()) } return nil, err