Skip to content

Commit

Permalink
cephfs: use string const for cli error comparison
Browse files Browse the repository at this point in the history
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 <madhupr007@gmail.com>
(cherry picked from commit 0bd45a5)
  • Loading branch information
Madhu-1 authored and mergify-bot committed Aug 19, 2020
1 parent b2a9ecf commit d6de84f
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 22 deletions.
39 changes: 27 additions & 12 deletions internal/cephfs/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")

// ErrSnapProtectionExist is returned when the snapshot is already protected
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 = fmt.Errorf("%v", "Error ENOENT")
ErrVolumeNotFound = errors.New("volume not found")

// 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")
ErrInvalidCommand = errors.New("invalid command")
)
10 changes: 5 additions & 5 deletions internal/cephfs/snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
10 changes: 5 additions & 5 deletions internal/cephfs/volume.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit d6de84f

Please sign in to comment.