From bb4f1c7c9d6f91316af3030e30de8a0e40f78249 Mon Sep 17 00:00:00 2001 From: Niels de Vos Date: Wed, 22 Jul 2020 11:16:08 +0200 Subject: [PATCH] rbd: use util.ExecCommand() instead of execCommand() Signed-off-by: Niels de Vos --- internal/rbd/rbd_attach.go | 22 ++++++++++------------ internal/rbd/rbd_util.go | 22 ++++++---------------- 2 files changed, 16 insertions(+), 28 deletions(-) diff --git a/internal/rbd/rbd_attach.go b/internal/rbd/rbd_attach.go index b1623d73f..16b7c256b 100644 --- a/internal/rbd/rbd_attach.go +++ b/internal/rbd/rbd_attach.go @@ -159,13 +159,13 @@ func checkRbdNbdTools() bool { _, err := os.Stat(fmt.Sprintf("/sys/module/%s", moduleNbd)) if os.IsNotExist(err) { // try to load the module - _, err = execCommand("modprobe", []string{moduleNbd}) + _, _, err = util.ExecCommand("modprobe", moduleNbd) if err != nil { util.ExtendedLogMsg("rbd-nbd: nbd modprobe failed with error %v", err) return false } } - if _, err := execCommand(rbdTonbd, []string{"--version"}); err != nil { + if _, _, err := util.ExecCommand(rbdTonbd, "--version"); err != nil { util.ExtendedLogMsg("rbd-nbd: running rbd-nbd --version failed with error %v", err) return false } @@ -229,9 +229,9 @@ func createPath(ctx context.Context, volOpt *rbdVolume, cr *util.Credentials) (s mapOptions = append(mapOptions, "--read-only") } // Execute map - output, err := execCommand(rbd, mapOptions) + stdout, stderr, err := util.ExecCommand(rbd, mapOptions...) if err != nil { - klog.Warningf(util.Log(ctx, "rbd: map error %v, rbd output: %s"), err, string(output)) + klog.Warningf(util.Log(ctx, "rbd: map error %v, rbd output: %s"), err, string(stderr)) // unmap rbd image if connection timeout if strings.Contains(err.Error(), rbdMapConnectionTimeout) { detErr := detachRBDImageOrDeviceSpec(ctx, imagePath, true, isNbd, volOpt.Encrypted, volOpt.VolID) @@ -239,9 +239,9 @@ func createPath(ctx context.Context, volOpt *rbdVolume, cr *util.Credentials) (s klog.Warningf(util.Log(ctx, "rbd: %s unmap error %v"), imagePath, detErr) } } - return "", fmt.Errorf("rbd: map failed %v, rbd output: %s", err, string(output)) + return "", fmt.Errorf("rbd: map failed %v, rbd output: %s", err, string(stderr)) } - devicePath := strings.TrimSuffix(string(output), "\n") + devicePath := strings.TrimSuffix(string(stdout), "\n") return devicePath, nil } @@ -280,8 +280,6 @@ func detachRBDDevice(ctx context.Context, devicePath, volumeID string, encrypted // detachRBDImageOrDeviceSpec detaches an rbd imageSpec or devicePath, with additional checking // when imageSpec is used to decide if image is already unmapped. func detachRBDImageOrDeviceSpec(ctx context.Context, imageOrDeviceSpec string, isImageSpec, ndbType, encrypted bool, volumeID string) error { - var output []byte - if encrypted { mapperFile, mapperPath := util.VolumeMapper(volumeID) mappedDevice, mapper, err := util.DeviceEncryptionStatus(ctx, mapperPath) @@ -308,18 +306,18 @@ func detachRBDImageOrDeviceSpec(ctx context.Context, imageOrDeviceSpec string, i } options := []string{"unmap", "--device-type", accessType, imageOrDeviceSpec} - output, err := execCommand(rbd, options) + _, stderr, err := util.ExecCommand(rbd, options...) if err != nil { // Messages for krbd and nbd differ, hence checking either of them for missing mapping // This is not applicable when a device path is passed in if isImageSpec && - (strings.Contains(string(output), fmt.Sprintf(rbdUnmapCmdkRbdMissingMap, imageOrDeviceSpec)) || - strings.Contains(string(output), fmt.Sprintf(rbdUnmapCmdNbdMissingMap, imageOrDeviceSpec))) { + (strings.Contains(string(stderr), fmt.Sprintf(rbdUnmapCmdkRbdMissingMap, imageOrDeviceSpec)) || + strings.Contains(string(stderr), fmt.Sprintf(rbdUnmapCmdNbdMissingMap, imageOrDeviceSpec))) { // Devices found not to be mapped are treated as a successful detach util.TraceLog(ctx, "image or device spec (%s) not mapped", imageOrDeviceSpec) return nil } - return fmt.Errorf("rbd: unmap for spec (%s) failed (%v): (%s)", imageOrDeviceSpec, err, string(output)) + return fmt.Errorf("rbd: unmap for spec (%s) failed (%v): (%s)", imageOrDeviceSpec, err, string(stderr)) } return nil diff --git a/internal/rbd/rbd_util.go b/internal/rbd/rbd_util.go index 0ff56c9a7..363c6325b 100644 --- a/internal/rbd/rbd_util.go +++ b/internal/rbd/rbd_util.go @@ -23,7 +23,6 @@ import ( "fmt" "io/ioutil" "os" - "os/exec" "path/filepath" "strconv" "strings" @@ -300,20 +299,19 @@ func (rv *rbdVolume) isInUse() (bool, error) { // asynchronously. If command is not found returns a bool set to false // example arg ["trash", "remove","pool/image"]. func addRbdManagerTask(ctx context.Context, pOpts *rbdVolume, arg []string) (bool, error) { - var output []byte args := []string{"rbd", "task", "add"} args = append(args, arg...) util.DebugLog(ctx, "executing %v for image (%s) using mon %s, pool %s", args, pOpts.RbdImageName, pOpts.Monitors, pOpts.Pool) supported := true - output, err := execCommand("ceph", args) + _, stderr, err := util.ExecCommand("ceph", args...) if err != nil { switch { - case strings.Contains(string(output), rbdTaskRemoveCmdInvalidString1) && - strings.Contains(string(output), rbdTaskRemoveCmdInvalidString2): + case strings.Contains(string(stderr), rbdTaskRemoveCmdInvalidString1) && + strings.Contains(string(stderr), rbdTaskRemoveCmdInvalidString2): klog.Warningf(util.Log(ctx, "cluster with cluster ID (%s) does not support Ceph manager based rbd commands (minimum ceph version required is v14.2.3)"), pOpts.ClusterID) supported = false - case strings.HasPrefix(string(output), rbdTaskRemoveCmdAccessDeniedMessage): + case strings.HasPrefix(string(stderr), rbdTaskRemoveCmdAccessDeniedMessage): klog.Warningf(util.Log(ctx, "access denied to Ceph MGR-based rbd commands on cluster ID (%s)"), pOpts.ClusterID) supported = false default: @@ -665,12 +663,6 @@ func genVolFromVolID(ctx context.Context, volumeID string, cr *util.Credentials, return rbdVol, err } -func execCommand(command string, args []string) ([]byte, error) { - // #nosec - cmd := exec.Command(command, args...) - return cmd.CombinedOutput() -} - func getMonsAndClusterID(ctx context.Context, options map[string]string) (monitors, clusterID string, err error) { var ok bool @@ -1050,16 +1042,14 @@ func cleanupRBDImageMetadataStash(path string) error { // resizeRBDImage resizes the given volume to new size. func resizeRBDImage(rbdVol *rbdVolume, cr *util.Credentials) error { - var output []byte - mon := rbdVol.Monitors volSzMiB := fmt.Sprintf("%dM", util.RoundOffVolSize(rbdVol.VolSize)) args := []string{"resize", rbdVol.String(), "--size", volSzMiB, "--id", cr.ID, "-m", mon, "--keyfile=" + cr.KeyFile} - output, err := execCommand("rbd", args) + _, stderr, err := util.ExecCommand("rbd", args...) if err != nil { - return fmt.Errorf("failed to resize rbd image (%w), command output: %s", err, string(output)) + return fmt.Errorf("failed to resize rbd image (%w), command output: %s", err, string(stderr)) } return nil