From 75d190f430c8def973465751cd3f9c2caf8d3eb5 Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Fri, 6 Nov 2020 08:27:37 +0530 Subject: [PATCH] cephfs: check stderror for CLI commands currently in someplaces we are checking the stderror for CLI errors and insome places we are checking for just error. This commit fixes the issue by checking the stderror for actual CLI errors. Co-authored-by: Yug Signed-off-by: Madhu Rajanna --- internal/cephfs/cephfs_util.go | 9 ++++++--- internal/cephfs/clone.go | 4 ++-- internal/cephfs/snapshot.go | 32 ++++++++++++++++---------------- internal/cephfs/util.go | 16 +++++++++++----- internal/cephfs/volume.go | 15 +++++++-------- 5 files changed, 42 insertions(+), 34 deletions(-) diff --git a/internal/cephfs/cephfs_util.go b/internal/cephfs/cephfs_util.go index 79be1f1b3..2acad0e34 100644 --- a/internal/cephfs/cephfs_util.go +++ b/internal/cephfs/cephfs_util.go @@ -38,7 +38,7 @@ func getFscID(ctx context.Context, monitors string, cr *util.Credentials, fsName // ceph fs get myfs --format=json // {"mdsmap":{...},"id":2} var fsDetails CephFilesystemDetails - err := execCommandJSON(ctx, &fsDetails, + stdErr, err := execCommandJSON(ctx, &fsDetails, "ceph", "-m", monitors, "--id", cr.ID, @@ -47,6 +47,7 @@ func getFscID(ctx context.Context, monitors string, cr *util.Credentials, fsName "fs", "get", fsName, "--format=json", ) if err != nil { + util.ErrorLog(ctx, "failed to get filesystem details %s with Error: %v. stdError: %s", fsName, err, stdErr) return 0, err } @@ -66,7 +67,7 @@ func getMetadataPool(ctx context.Context, monitors string, cr *util.Credentials, // ./tbox ceph fs ls --format=json // [{"name":"myfs","metadata_pool":"myfs-metadata","metadata_pool_id":4,...},...] var filesystems []CephFilesystem - err := execCommandJSON(ctx, &filesystems, + stdErr, err := execCommandJSON(ctx, &filesystems, "ceph", "-m", monitors, "--id", cr.ID, @@ -75,6 +76,7 @@ func getMetadataPool(ctx context.Context, monitors string, cr *util.Credentials, "fs", "ls", "--format=json", ) if err != nil { + util.ErrorLog(ctx, "failed to list filesystem with Error: %v. stdError: %s", err, stdErr) return "", err } @@ -96,7 +98,7 @@ func getFsName(ctx context.Context, monitors string, cr *util.Credentials, fscID // ./tbox ceph fs dump --format=json // JSON: {...,"filesystems":[{"mdsmap":{},"id":},...],...} var fsDump CephFilesystemDump - err := execCommandJSON(ctx, &fsDump, + stdErr, err := execCommandJSON(ctx, &fsDump, "ceph", "-m", monitors, "--id", cr.ID, @@ -105,6 +107,7 @@ func getFsName(ctx context.Context, monitors string, cr *util.Credentials, fscID "fs", "dump", "--format=json", ) if err != nil { + util.ErrorLog(ctx, "failed to dump filesystem details with Error: %v. stdError: %s", err, stdErr) return "", err } diff --git a/internal/cephfs/clone.go b/internal/cephfs/clone.go index 48873bd62..283aacfb6 100644 --- a/internal/cephfs/clone.go +++ b/internal/cephfs/clone.go @@ -211,13 +211,13 @@ func getCloneInfo(ctx context.Context, volOptions *volumeOptions, cr *util.Crede "--keyfile=" + cr.KeyFile, "--format=json", } - err := execCommandJSON( + stdErr, err := execCommandJSON( ctx, &clone, "ceph", args[:]...) if err != nil { - util.ErrorLog(ctx, "failed to get subvolume clone info %s(%s) in fs %s", string(volID), err, volOptions.FsName) + util.ErrorLog(ctx, "failed to get subvolume clone info %s in fs %s with Error: %v. stdError: %s", string(volID), volOptions.FsName, err, stdErr) return clone, err } return clone, nil diff --git a/internal/cephfs/snapshot.go b/internal/cephfs/snapshot.go index de860587d..e0e3c4fef 100644 --- a/internal/cephfs/snapshot.go +++ b/internal/cephfs/snapshot.go @@ -60,12 +60,12 @@ func createSnapshot(ctx context.Context, volOptions *volumeOptions, cr *util.Cre "--keyfile=" + cr.KeyFile, } - err := execCommandErr( + stdErr, err := execCommandWithStdErr( ctx, "ceph", args[:]...) if err != nil { - util.ErrorLog(ctx, "failed to create subvolume snapshot %s %s(%s) in fs %s", string(snapID), string(volID), err, volOptions.FsName) + util.ErrorLog(ctx, "failed to create subvolume snapshot %s %s in fs %s with Error: %v. stdError %s", string(snapID), string(volID), volOptions.FsName, err, stdErr) return err } return nil @@ -89,12 +89,12 @@ func deleteSnapshot(ctx context.Context, volOptions *volumeOptions, cr *util.Cre "--force", } - err := execCommandErr( + stdErr, err := execCommandWithStdErr( ctx, "ceph", args[:]...) if err != nil { - util.ErrorLog(ctx, "failed to delete subvolume snapshot %s %s(%s) in fs %s", string(snapID), string(volID), err, volOptions.FsName) + util.ErrorLog(ctx, "failed to delete subvolume snapshot %s %s in fs %s with Error: %v. stdError: %s", string(snapID), string(volID), volOptions.FsName, err, stdErr) return err } return nil @@ -127,16 +127,16 @@ func getSnapshotInfo(ctx context.Context, volOptions *volumeOptions, cr *util.Cr "--keyfile=" + cr.KeyFile, "--format=json", } - err := execCommandJSON( + stdErr, err := execCommandJSON( ctx, &snap, "ceph", args[:]...) if err != nil { - if strings.Contains(err.Error(), snapNotFound) { + util.ErrorLog(ctx, "failed to get subvolume snapshot info %s %s in fs %s with Error: %v. stdError: %s", string(snapID), string(volID), volOptions.FsName, err, stdErr) + if strings.Contains(stdErr, 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 } return snap, nil @@ -164,15 +164,15 @@ func protectSnapshot(ctx context.Context, volOptions *volumeOptions, cr *util.Cr "--keyfile=" + cr.KeyFile, } - err := execCommandErr( + stdErr, err := execCommandWithStdErr( ctx, "ceph", args[:]...) if err != nil { - if strings.Contains(err.Error(), snapProtectionExist) { + util.ErrorLog(ctx, "failed to protect subvolume snapshot %s %s in fs %s with Error: %v. stdError: %s", string(snapID), string(volID), volOptions.FsName, err, stdErr) + if strings.Contains(stdErr, snapProtectionExist) { return nil } - util.ErrorLog(ctx, "failed to protect subvolume snapshot %s %s(%s) in fs %s", string(snapID), string(volID), err, volOptions.FsName) return err } return nil @@ -200,17 +200,17 @@ func unprotectSnapshot(ctx context.Context, volOptions *volumeOptions, cr *util. "--keyfile=" + cr.KeyFile, } - err := execCommandErr( + stdErr, err := execCommandWithStdErr( ctx, "ceph", args[:]...) if err != nil { + util.ErrorLog(ctx, "failed to unprotect subvolume snapshot %s %s in fs %s with Error: %vv stdError: %s", string(snapID), string(volID), volOptions.FsName, err, stdErr) // 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(), snapProtectionExist) { + if strings.Contains(stdErr, snapProtectionExist) { return nil } - util.ErrorLog(ctx, "failed to unprotect subvolume snapshot %s %s(%s) in fs %s", string(snapID), string(volID), err, volOptions.FsName) return err } return nil @@ -239,14 +239,14 @@ func cloneSnapshot(ctx context.Context, parentVolOptions *volumeOptions, cr *uti args = append(args, "--pool_layout", cloneVolOptions.Pool) } - err := execCommandErr( + stdErr, err := execCommandWithStdErr( ctx, "ceph", args[:]...) 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(), volumeNotFound) { + util.ErrorLog(ctx, "failed to clone subvolume snapshot %s %s in fs %s with Error: %v. stdError: %s", string(cloneID), string(volID), parentVolOptions.FsName, err, stdErr) + if strings.HasPrefix(stdErr, volumeNotFound) { return ErrVolumeNotFound } return err diff --git a/internal/cephfs/util.go b/internal/cephfs/util.go index 8aa8a875d..729b9e054 100644 --- a/internal/cephfs/util.go +++ b/internal/cephfs/util.go @@ -39,17 +39,23 @@ func execCommandErr(ctx context.Context, program string, args ...string) error { } // nolint:unparam // todo:program values has to be revisited later -func execCommandJSON(ctx context.Context, v interface{}, program string, args ...string) error { - stdout, _, err := util.ExecCommand(ctx, program, args...) +func execCommandWithStdErr(ctx context.Context, program string, args ...string) (string, error) { + _, stdErr, err := util.ExecCommand(ctx, program, args...) + return stdErr, err +} + +// nolint:unparam // todo:program values has to be revisited later +func execCommandJSON(ctx context.Context, v interface{}, program string, args ...string) (string, error) { + stdout, stderr, err := util.ExecCommand(ctx, program, args...) if err != nil { - return err + return stderr, err } if err = json.Unmarshal([]byte(stdout), v); err != nil { - return fmt.Errorf("failed to unmarshal JSON for %s %v: %s: %w", program, util.StripSecretInArgs(args), stdout, err) + return "", fmt.Errorf("failed to unmarshal JSON for %s %v: %s: %w", program, util.StripSecretInArgs(args), stdout, err) } - return nil + return "", nil } // Controller service request validation. diff --git a/internal/cephfs/volume.go b/internal/cephfs/volume.go index 61617302e..d99de0742 100644 --- a/internal/cephfs/volume.go +++ b/internal/cephfs/volume.go @@ -90,7 +90,7 @@ func getVolumeRootPathCeph(ctx context.Context, volOptions *volumeOptions, cr *u func getSubVolumeInfo(ctx context.Context, volOptions *volumeOptions, cr *util.Credentials, volID volumeID) (Subvolume, error) { info := Subvolume{} - err := execCommandJSON( + stdErr, err := execCommandJSON( ctx, &info, "ceph", @@ -106,12 +106,11 @@ func getSubVolumeInfo(ctx context.Context, volOptions *volumeOptions, cr *util.C "-n", cephEntityClientPrefix+cr.ID, "--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(), volumeNotFound) { + util.ErrorLog(ctx, "failed to get subvolume info %s in fs %s with Error: %v. stdError: %s", string(volID), volOptions.FsName, err, stdErr) + if strings.HasPrefix(stdErr, volumeNotFound) || 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(), invalidCommand) { + if strings.Contains(stdErr, invalidCommand) || strings.Contains(err.Error(), invalidCommand) { return info, ErrInvalidCommand } @@ -218,7 +217,7 @@ func resizeVolume(ctx context.Context, volOptions *volumeOptions, cr *util.Crede "--keyfile=" + cr.KeyFile, } - err := execCommandErr( + stdErr, err := execCommandWithStdErr( ctx, "ceph", args[:]...) @@ -228,8 +227,8 @@ 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(), invalidCommand) { - util.ErrorLog(ctx, "failed to resize subvolume %s(%s) in fs %s", string(volID), err, volOptions.FsName) + if !strings.Contains(err.Error(), invalidCommand) && !strings.Contains(stdErr, invalidCommand) { + util.ErrorLog(ctx, "failed to resize subvolume %s in fs %s with Error: %v. stdError: %s", string(volID), volOptions.FsName, err, stdErr) return err } }