rbd: use util.ExecCommand() instead of execCommand()

Signed-off-by: Niels de Vos <ndevos@redhat.com>
This commit is contained in:
Niels de Vos 2020-07-22 11:16:08 +02:00 committed by mergify[bot]
parent 457d846241
commit bb4f1c7c9d
2 changed files with 16 additions and 28 deletions

View File

@ -159,13 +159,13 @@ func checkRbdNbdTools() bool {
_, err := os.Stat(fmt.Sprintf("/sys/module/%s", moduleNbd)) _, err := os.Stat(fmt.Sprintf("/sys/module/%s", moduleNbd))
if os.IsNotExist(err) { if os.IsNotExist(err) {
// try to load the module // try to load the module
_, err = execCommand("modprobe", []string{moduleNbd}) _, _, err = util.ExecCommand("modprobe", moduleNbd)
if err != nil { if err != nil {
util.ExtendedLogMsg("rbd-nbd: nbd modprobe failed with error %v", err) util.ExtendedLogMsg("rbd-nbd: nbd modprobe failed with error %v", err)
return false 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) util.ExtendedLogMsg("rbd-nbd: running rbd-nbd --version failed with error %v", err)
return false return false
} }
@ -229,9 +229,9 @@ func createPath(ctx context.Context, volOpt *rbdVolume, cr *util.Credentials) (s
mapOptions = append(mapOptions, "--read-only") mapOptions = append(mapOptions, "--read-only")
} }
// Execute map // Execute map
output, err := execCommand(rbd, mapOptions) stdout, stderr, err := util.ExecCommand(rbd, mapOptions...)
if err != nil { 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 // unmap rbd image if connection timeout
if strings.Contains(err.Error(), rbdMapConnectionTimeout) { if strings.Contains(err.Error(), rbdMapConnectionTimeout) {
detErr := detachRBDImageOrDeviceSpec(ctx, imagePath, true, isNbd, volOpt.Encrypted, volOpt.VolID) 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) 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 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 // detachRBDImageOrDeviceSpec detaches an rbd imageSpec or devicePath, with additional checking
// when imageSpec is used to decide if image is already unmapped. // 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 { func detachRBDImageOrDeviceSpec(ctx context.Context, imageOrDeviceSpec string, isImageSpec, ndbType, encrypted bool, volumeID string) error {
var output []byte
if encrypted { if encrypted {
mapperFile, mapperPath := util.VolumeMapper(volumeID) mapperFile, mapperPath := util.VolumeMapper(volumeID)
mappedDevice, mapper, err := util.DeviceEncryptionStatus(ctx, mapperPath) 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} options := []string{"unmap", "--device-type", accessType, imageOrDeviceSpec}
output, err := execCommand(rbd, options) _, stderr, err := util.ExecCommand(rbd, options...)
if err != nil { if err != nil {
// Messages for krbd and nbd differ, hence checking either of them for missing mapping // 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 // This is not applicable when a device path is passed in
if isImageSpec && if isImageSpec &&
(strings.Contains(string(output), fmt.Sprintf(rbdUnmapCmdkRbdMissingMap, imageOrDeviceSpec)) || (strings.Contains(string(stderr), fmt.Sprintf(rbdUnmapCmdkRbdMissingMap, imageOrDeviceSpec)) ||
strings.Contains(string(output), fmt.Sprintf(rbdUnmapCmdNbdMissingMap, imageOrDeviceSpec))) { strings.Contains(string(stderr), fmt.Sprintf(rbdUnmapCmdNbdMissingMap, imageOrDeviceSpec))) {
// Devices found not to be mapped are treated as a successful detach // Devices found not to be mapped are treated as a successful detach
util.TraceLog(ctx, "image or device spec (%s) not mapped", imageOrDeviceSpec) util.TraceLog(ctx, "image or device spec (%s) not mapped", imageOrDeviceSpec)
return nil 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 return nil

View File

@ -23,7 +23,6 @@ import (
"fmt" "fmt"
"io/ioutil" "io/ioutil"
"os" "os"
"os/exec"
"path/filepath" "path/filepath"
"strconv" "strconv"
"strings" "strings"
@ -300,20 +299,19 @@ func (rv *rbdVolume) isInUse() (bool, error) {
// asynchronously. If command is not found returns a bool set to false // asynchronously. If command is not found returns a bool set to false
// example arg ["trash", "remove","pool/image"]. // example arg ["trash", "remove","pool/image"].
func addRbdManagerTask(ctx context.Context, pOpts *rbdVolume, arg []string) (bool, error) { func addRbdManagerTask(ctx context.Context, pOpts *rbdVolume, arg []string) (bool, error) {
var output []byte
args := []string{"rbd", "task", "add"} args := []string{"rbd", "task", "add"}
args = append(args, arg...) args = append(args, arg...)
util.DebugLog(ctx, "executing %v for image (%s) using mon %s, pool %s", args, pOpts.RbdImageName, pOpts.Monitors, pOpts.Pool) util.DebugLog(ctx, "executing %v for image (%s) using mon %s, pool %s", args, pOpts.RbdImageName, pOpts.Monitors, pOpts.Pool)
supported := true supported := true
output, err := execCommand("ceph", args) _, stderr, err := util.ExecCommand("ceph", args...)
if err != nil { if err != nil {
switch { switch {
case strings.Contains(string(output), rbdTaskRemoveCmdInvalidString1) && case strings.Contains(string(stderr), rbdTaskRemoveCmdInvalidString1) &&
strings.Contains(string(output), rbdTaskRemoveCmdInvalidString2): 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) 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 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) klog.Warningf(util.Log(ctx, "access denied to Ceph MGR-based rbd commands on cluster ID (%s)"), pOpts.ClusterID)
supported = false supported = false
default: default:
@ -665,12 +663,6 @@ func genVolFromVolID(ctx context.Context, volumeID string, cr *util.Credentials,
return rbdVol, err 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) { func getMonsAndClusterID(ctx context.Context, options map[string]string) (monitors, clusterID string, err error) {
var ok bool var ok bool
@ -1050,16 +1042,14 @@ func cleanupRBDImageMetadataStash(path string) error {
// resizeRBDImage resizes the given volume to new size. // resizeRBDImage resizes the given volume to new size.
func resizeRBDImage(rbdVol *rbdVolume, cr *util.Credentials) error { func resizeRBDImage(rbdVol *rbdVolume, cr *util.Credentials) error {
var output []byte
mon := rbdVol.Monitors mon := rbdVol.Monitors
volSzMiB := fmt.Sprintf("%dM", util.RoundOffVolSize(rbdVol.VolSize)) volSzMiB := fmt.Sprintf("%dM", util.RoundOffVolSize(rbdVol.VolSize))
args := []string{"resize", rbdVol.String(), "--size", volSzMiB, "--id", cr.ID, "-m", mon, "--keyfile=" + cr.KeyFile} 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 { 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 return nil