From a8e6bdee8edab8fb79f752b0159e2a2c1061536a Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Mon, 17 Aug 2020 11:28:59 +0530 Subject: [PATCH] cephfs: use string const for cli error comparison we should not return the CLI errors in GRPC errors we need to return proper readable error messages to the user for better understanding and better debugging. updates #1242 Signed-off-by: Madhu Rajanna (cherry picked from commit 0bd45a5eb172477ea29e3b9c97eb79b654587044) --- internal/cephfs/errors.go | 41 +++++++++++++++++++++++++------------ internal/cephfs/snapshot.go | 10 ++++----- internal/cephfs/volume.go | 10 ++++----- 3 files changed, 38 insertions(+), 23 deletions(-) diff --git a/internal/cephfs/errors.go b/internal/cephfs/errors.go index 02fdaf8ba..5b9f68867 100644 --- a/internal/cephfs/errors.go +++ b/internal/cephfs/errors.go @@ -18,27 +18,42 @@ package cephfs import ( "errors" - "fmt" +) + +// Error strings for comparison with CLI errors. +const ( + // volumeNotFound is returned when a subvolume is not found in CephFS. + volumeNotFound = "Error ENOENT" + // invalidCommand is returned when a command is not known to the cluster + invalidCommand = "Error EINVAL" + // snapNotFound is returned when snap name passed is not found in the list + // of snapshots for the given image. + snapNotFound = "Error ENOENT" + // snapProtectionExist is returned when the snapshot is already protected + snapProtectionExist = "Error EEXIST" ) var ( + // ErrCloneInProgress is returned when snapshot clone state is `in progress` + ErrCloneInProgress = errors.New("in progress") + // ErrInvalidVolID is returned when a CSI passed VolumeID is not conformant to any known volume ID // formats. ErrInvalidVolID = errors.New("invalid VolumeID") // ErrNonStaticVolume is returned when a volume is detected as not being // statically provisioned. ErrNonStaticVolume = errors.New("volume not static") - // ErrVolNotEmpty is returned when a subvolume has snapshots in it. - ErrVolNotEmpty = fmt.Errorf("%v", "Error ENOTEMPTY") - // ErrVolumeNotFound is returned when a subvolume is not found in CephFS. - ErrVolumeNotFound = fmt.Errorf("%v", "Error ENOENT") - // ErrInvalidCommand is returned when a command is not known to the cluster - ErrInvalidCommand = fmt.Errorf("%v", "Error EINVAL") - // ErrSnapNotFound is returned when snap name passed is not found in the list of snapshots for the - // given image. - ErrSnapNotFound = fmt.Errorf("%v", "Error ENOENT") + // ErrSnapProtectionExist is returned when the snapshot is already protected - ErrSnapProtectionExist = fmt.Errorf("%v", "Error EEXIST") - // ErrCloneInProgress is returned when snapshot clone state is `in progress` - ErrCloneInProgress = fmt.Errorf("in progress") + ErrSnapProtectionExist = errors.New("snapshot protection already exists") + + // ErrSnapNotFound is returned when snap name passed is not found in the list + // of snapshots for the given image. + ErrSnapNotFound = errors.New("snapshot not found") + + // ErrVolumeNotFound is returned when a subvolume is not found in CephFS. + ErrVolumeNotFound = errors.New("volume not found") + + // ErrInvalidCommand is returned when a command is not known to the cluster + ErrInvalidCommand = errors.New("invalid command") ) diff --git a/internal/cephfs/snapshot.go b/internal/cephfs/snapshot.go index c24113c17..8c124b91b 100644 --- a/internal/cephfs/snapshot.go +++ b/internal/cephfs/snapshot.go @@ -127,8 +127,8 @@ func getSnapshotInfo(ctx context.Context, volOptions *volumeOptions, cr *util.Cr "ceph", args[:]...) if err != nil { - if strings.Contains(err.Error(), ErrSnapNotFound.Error()) { - return snapshotInfo{}, err + if strings.Contains(err.Error(), snapNotFound) { + return snapshotInfo{}, ErrSnapNotFound } util.ErrorLog(ctx, "failed to get subvolume snapshot info %s %s(%s) in fs %s", string(snapID), string(volID), err, volOptions.FsName) return snapshotInfo{}, err @@ -158,7 +158,7 @@ func protectSnapshot(ctx context.Context, volOptions *volumeOptions, cr *util.Cr "ceph", args[:]...) if err != nil { - if strings.Contains(err.Error(), ErrSnapProtectionExist.Error()) { + if strings.Contains(err.Error(), snapProtectionExist) { return nil } util.ErrorLog(ctx, "failed to protect subvolume snapshot %s %s(%s) in fs %s", string(snapID), string(volID), err, volOptions.FsName) @@ -191,7 +191,7 @@ func unprotectSnapshot(ctx context.Context, volOptions *volumeOptions, cr *util. if err != nil { // Incase the snap is already unprotected we get ErrSnapProtectionExist error code // in that case we are safe and we could discard this error. - if strings.Contains(err.Error(), ErrSnapProtectionExist.Error()) { + if strings.Contains(err.Error(), snapProtectionExist) { return nil } util.ErrorLog(ctx, "failed to unprotect subvolume snapshot %s %s(%s) in fs %s", string(snapID), string(volID), err, volOptions.FsName) @@ -230,7 +230,7 @@ func cloneSnapshot(ctx context.Context, parentVolOptions *volumeOptions, cr *uti if err != nil { util.ErrorLog(ctx, "failed to clone subvolume snapshot %s %s(%s) in fs %s", string(cloneID), string(volID), err, parentVolOptions.FsName) - if strings.HasPrefix(err.Error(), ErrVolumeNotFound.Error()) { + if strings.HasPrefix(err.Error(), volumeNotFound) { return ErrVolumeNotFound } return err diff --git a/internal/cephfs/volume.go b/internal/cephfs/volume.go index efc2a557b..45c85c5a5 100644 --- a/internal/cephfs/volume.go +++ b/internal/cephfs/volume.go @@ -73,7 +73,7 @@ func getVolumeRootPathCeph(ctx context.Context, volOptions *volumeOptions, cr *u if err != nil { util.ErrorLog(ctx, "failed to get the rootpath for the vol %s(%s) stdError %s", string(volID), err, stderr) - if strings.Contains(stderr, ErrVolumeNotFound.Error()) { + if strings.Contains(stderr, volumeNotFound) { return "", util.JoinErrors(ErrVolumeNotFound, err) } @@ -101,11 +101,11 @@ func getSubVolumeInfo(ctx context.Context, volOptions *volumeOptions, cr *util.C "--keyfile="+cr.KeyFile) if err != nil { util.ErrorLog(ctx, "failed to get subvolume info for the vol %s(%s)", string(volID), err) - if strings.HasPrefix(err.Error(), ErrVolumeNotFound.Error()) { + if strings.HasPrefix(err.Error(), volumeNotFound) { return info, ErrVolumeNotFound } // Incase the error is other than invalid command return error to the caller. - if !strings.Contains(err.Error(), ErrInvalidCommand.Error()) { + if !strings.Contains(err.Error(), invalidCommand) { return info, ErrInvalidCommand } @@ -222,7 +222,7 @@ func resizeVolume(ctx context.Context, volOptions *volumeOptions, cr *util.Crede return nil } // Incase the error is other than invalid command return error to the caller. - if !strings.Contains(err.Error(), ErrInvalidCommand.Error()) { + if !strings.Contains(err.Error(), invalidCommand) { util.ErrorLog(ctx, "failed to resize subvolume %s(%s) in fs %s", string(volID), err, volOptions.FsName) return err } @@ -252,7 +252,7 @@ func purgeVolume(ctx context.Context, volID volumeID, cr *util.Credentials, volO err := execCommandErr(ctx, "ceph", arg...) if err != nil { util.ErrorLog(ctx, "failed to purge subvolume %s(%s) in fs %s", string(volID), err, volOptions.FsName) - if strings.Contains(err.Error(), ErrVolumeNotFound.Error()) { + if strings.Contains(err.Error(), volumeNotFound) { return util.JoinErrors(ErrVolumeNotFound, err) } return err