Merge pull request #5530 from medyagh/NewCmdRunner

Refactor command runner interface, allow stdin writes

Makefile

@@ -572,4 +572,4 @@ site: site/themes/docsy/assets/vendor/bootstrap/package.js out/hugo/hugo
 
 .PHONY: out/mkcmp
 out/mkcmp:
-	GOOS=$(GOOS) GOARCH=$(GOARCH) go build -o $@ cmd/performance/main.go
+	GOOS=$(GOOS) GOARCH=$(GOARCH) go build -o $@ cmd/performance/main.go
\ No newline at end of file

cmd/minikube/cmd/start.go

@@ -1029,27 +1029,29 @@ Suggested workarounds:
 		defer conn.Close()
 	}
 
-	if err := r.Run("nslookup kubernetes.io"); err != nil {
+	if _, err := r.RunCmd(exec.Command("nslookup", "kubernetes.io")); err != nil {
 		out.WarningT("VM is unable to resolve DNS hosts: {[.error}}", out.V{"error": err})
 	}
 
 	// Try both UDP and ICMP to assert basic external connectivity
-	if err := r.Run("nslookup k8s.io 8.8.8.8 || nslookup k8s.io 1.1.1.1 || ping -c1 8.8.8.8"); err != nil {
+	if _, err := r.RunCmd(exec.Command("/bin/bash", "-c", "nslookup k8s.io 8.8.8.8 || nslookup k8s.io 1.1.1.1 || ping -c1 8.8.8.8")); err != nil {
 		out.WarningT("VM is unable to directly connect to the internet: {{.error}}", out.V{"error": err})
 	}
 
 	// Try an HTTPS connection to the
 	proxy := os.Getenv("HTTPS_PROXY")
-	opts := "-sS"
+	opts := []string{"-sS"}
 	if proxy != "" && !strings.HasPrefix(proxy, "localhost") && !strings.HasPrefix(proxy, "127.0") {
-		opts = fmt.Sprintf("-x %s %s", proxy, opts)
+		opts = append([]string{"-x", proxy}, opts...)
 	}
 
 	repo := viper.GetString(imageRepository)
 	if repo == "" {
 		repo = images.DefaultImageRepo
 	}
-	if err := r.Run(fmt.Sprintf("curl %s https://%s/", opts, repo)); err != nil {
+
+	opts = append(opts, fmt.Sprintf("https://%s/", repo))
+	if _, err := r.RunCmd(exec.Command("curl", opts...)); err != nil {
 		out.WarningT("VM is unable to connect to the selected image repository: {{.error}}", out.V{"error": err})
 	}
 	return ip

go.mod

@@ -43,6 +43,7 @@ require (
 	github.com/juju/testing v0.0.0-20190723135506-ce30eb24acd2 // indirect
 	github.com/juju/utils v0.0.0-20180820210520-bf9cc5bdd62d // indirect
 	github.com/juju/version v0.0.0-20180108022336-b64dbd566305 // indirect
+	github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51
 	github.com/libvirt/libvirt-go v3.4.0+incompatible
 	github.com/machine-drivers/docker-machine-driver-vmware v0.1.1
 	github.com/mattn/go-isatty v0.0.8

go.sum

@@ -294,6 +294,8 @@ github.com/juju/version v0.0.0-20180108022336-b64dbd566305 h1:lQxPJ1URr2fjsKnJRt
 github.com/juju/version v0.0.0-20180108022336-b64dbd566305/go.mod h1:kE8gK5X0CImdr7qpSKl3xB2PmpySSmfj7zVbkZFs81U=
 github.com/kardianos/osext v0.0.0-20150410034420-8fef92e41e22/go.mod h1:1NbS8ALrpOvjt0rHPNLyCIeMtbizbir8U//inJ+zuB8=
 github.com/karrick/godirwalk v1.7.5/go.mod h1:2c9FRhkDxdIbgkOnCEvnSWs71Bhugbl46shStcFDJ34=
+github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 h1:Z9n2FFNUXsshfwJMBgNA0RU6/i7WVaAegv3PtuIHPMs=
+github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51/go.mod h1:CzGEWj7cYgsdH8dAjBGEr58BoE7ScuLd+fwFZ44+/x8=
 github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck=
 github.com/konsorten/go-windows-terminal-sequences v1.0.1 h1:mweAR1A6xJ3oS2pRaGiHgQ4OO8tzTaLawm8vnODuwDk=
 github.com/konsorten/go-windows-terminal-sequences v1.0.1/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ=

pkg/drivers/none/none.go

@@ -17,8 +17,8 @@ limitations under the License.
 package none
 
 import (
-	"bytes"
 	"fmt"
+	"os/exec"
 	"strings"
 	"time"
 
@@ -168,8 +168,8 @@ func (d *Driver) Remove() error {
 		return errors.Wrap(err, "kill")
 	}
 	glog.Infof("Removing: %s", cleanupPaths)
-	cmd := fmt.Sprintf("sudo rm -rf %s", strings.Join(cleanupPaths, " "))
-	if err := d.exec.Run(cmd); err != nil {
+	args := append([]string{"rm", "-rf"}, cleanupPaths...)
+	if _, err := d.exec.RunCmd(exec.Command("sudo", args...)); err != nil {
 		glog.Errorf("cleanup incomplete: %v", err)
 	}
 	return nil
@@ -217,22 +217,20 @@ func (d *Driver) RunSSHCommandFromDriver() error {
 }
 
 // stopKubelet idempotently stops the kubelet
-func stopKubelet(exec command.Runner) error {
+func stopKubelet(cr command.Runner) error {
 	glog.Infof("stopping kubelet.service ...")
 	stop := func() error {
-		cmdStop := "sudo systemctl stop kubelet.service"
-		cmdCheck := "sudo systemctl show -p SubState kubelet"
-		err := exec.Run(cmdStop)
-		if err != nil {
-			glog.Errorf("temporary error for %q : %v", cmdStop, err)
+		cmd := exec.Command("sudo", "systemctl", "stop", "kubelet.service")
+		if rr, err := cr.RunCmd(cmd); err != nil {
+			glog.Errorf("temporary error for %q : %v", rr.Command(), err)
 		}
-		var out bytes.Buffer
-		errStatus := exec.CombinedOutputTo(cmdCheck, &out)
-		if errStatus != nil {
-			glog.Errorf("temporary error: for %q : %v", cmdCheck, errStatus)
+		cmd = exec.Command("sudo", "systemctl", "show", "-p", "SubState", "kubelet")
+		rr, err := cr.RunCmd(cmd)
+		if err != nil {
+			glog.Errorf("temporary error: for %q : %v", rr.Command(), err)
 		}
-		if !strings.Contains(out.String(), "dead") && !strings.Contains(out.String(), "failed") {
-			return fmt.Errorf("unexpected kubelet state: %q", out)
+		if !strings.Contains(rr.Stdout.String(), "dead") && !strings.Contains(rr.Stdout.String(), "failed") {
+			return fmt.Errorf("unexpected kubelet state: %q", rr.Stdout.String())
 		}
 		return nil
 	}
@@ -245,13 +243,21 @@ func stopKubelet(exec command.Runner) error {
 }
 
 // restartKubelet restarts the kubelet
-func restartKubelet(exec command.Runner) error {
+func restartKubelet(cr command.Runner) error {
 	glog.Infof("restarting kubelet.service ...")
-	return exec.Run("sudo systemctl restart kubelet.service")
+	c := exec.Command("sudo", "systemctl", "restart", "kubelet.service")
+	if _, err := cr.RunCmd(c); err != nil {
+		return err
+	}
+	return nil
 }
 
 // checkKubelet returns an error if the kubelet is not running.
-func checkKubelet(exec command.Runner) error {
+func checkKubelet(cr command.Runner) error {
 	glog.Infof("checking for running kubelet ...")
-	return exec.Run("systemctl is-active --quiet service kubelet")
+	c := exec.Command("systemctl", "is-active", "--quiet", "service", "kubelet")
+	if _, err := cr.RunCmd(c); err != nil {
+		return errors.Wrap(err, "check kubelet")
+	}
+	return nil
 }

pkg/minikube/bootstrapper/certs.go

@@ -22,6 +22,7 @@ import (
 	"io/ioutil"
 	"net"
 	"os"
+	"os/exec"
 	"path"
 	"path/filepath"
 	"strings"
@@ -141,7 +142,7 @@ func SetupCerts(cmd command.Runner, k8s config.KubernetesConfig) error {
 
 	// configure CA certificates
 	if err := configureCACerts(cmd, caCerts); err != nil {
-		return errors.Wrapf(err, "error configuring CA certificates during provisioning %v", err)
+		return errors.Wrapf(err, "Configuring CA certs")
 	}
 	return nil
 }
@@ -318,21 +319,21 @@ func collectCACerts() (map[string]string, error) {
 }
 
 // getSubjectHash calculates Certificate Subject Hash for creating certificate symlinks
-func getSubjectHash(cmd command.Runner, filePath string) (string, error) {
-	out, err := cmd.CombinedOutput(fmt.Sprintf("openssl x509 -hash -noout -in '%s'", filePath))
+func getSubjectHash(cr command.Runner, filePath string) (string, error) {
+	rr, err := cr.RunCmd(exec.Command("openssl", "x509", "-hash", "-noout", "-in", filePath))
 	if err != nil {
-		return "", err
+		return "", errors.Wrapf(err, rr.Command())
 	}
-
-	stringHash := strings.TrimSpace(out)
+	stringHash := strings.TrimSpace(rr.Stdout.String())
 	return stringHash, nil
 }
 
 // configureCACerts looks up and installs all uploaded PEM certificates in /usr/share/ca-certificates to system-wide certificate store (/etc/ssl/certs).
 // OpenSSL binary required in minikube ISO
-func configureCACerts(cmd command.Runner, caCerts map[string]string) error {
+func configureCACerts(cr command.Runner, caCerts map[string]string) error {
 	hasSSLBinary := true
-	if err := cmd.Run("which openssl"); err != nil {
+	_, err := cr.RunCmd(exec.Command("openssl", "version"))
+	if err != nil {
 		hasSSLBinary = false
 	}
 
@@ -343,24 +344,25 @@ func configureCACerts(cmd command.Runner, caCerts map[string]string) error {
 	for _, caCertFile := range caCerts {
 		dstFilename := path.Base(caCertFile)
 		certStorePath := path.Join(SSLCertStoreDir, dstFilename)
-		if err := cmd.Run(fmt.Sprintf("sudo test -f '%s'", certStorePath)); err != nil {
-			if err := cmd.Run(fmt.Sprintf("sudo ln -s '%s' '%s'", caCertFile, certStorePath)); err != nil {
-				return errors.Wrapf(err, "error making symbol link for certificate %s", caCertFile)
+		_, err := cr.RunCmd(exec.Command("sudo", "test", "-f", certStorePath))
+		if err != nil {
+			if _, err := cr.RunCmd(exec.Command("sudo", "ln", "-s", caCertFile, certStorePath)); err != nil {
+				return errors.Wrapf(err, "create symlink for %s", caCertFile)
 			}
 		}
 		if hasSSLBinary {
-			subjectHash, err := getSubjectHash(cmd, caCertFile)
+			subjectHash, err := getSubjectHash(cr, caCertFile)
 			if err != nil {
-				return errors.Wrapf(err, "error calculating subject hash for certificate %s", caCertFile)
+				return errors.Wrapf(err, "calculate hash for cacert %s", caCertFile)
 			}
 			subjectHashLink := path.Join(SSLCertStoreDir, fmt.Sprintf("%s.0", subjectHash))
-			if err := cmd.Run(fmt.Sprintf("sudo test -f '%s'", subjectHashLink)); err != nil {
-				if err := cmd.Run(fmt.Sprintf("sudo ln -s '%s' '%s'", certStorePath, subjectHashLink)); err != nil {
-					return errors.Wrapf(err, "error making subject hash symbol link for certificate %s", caCertFile)
+			_, err = cr.RunCmd(exec.Command("sudo", "test", "-f", subjectHashLink))
+			if err != nil {
+				if _, err := cr.RunCmd(exec.Command("sudo", "ln", "-s", certStorePath, subjectHashLink)); err != nil {
+					return errors.Wrapf(err, "linking caCertFile %s", caCertFile)
 				}
 			}
 		}
 	}
-
 	return nil
 }

pkg/minikube/bootstrapper/certs_test.go

@@ -62,9 +62,9 @@ func TestSetupCerts(t *testing.T) {
 		certStorePath := path.Join(SSLCertStoreDir, dst)
 		certNameHash := "abcdef"
 		remoteCertHashLink := path.Join(SSLCertStoreDir, fmt.Sprintf("%s.0", certNameHash))
-		cmdMap[fmt.Sprintf("sudo ln -s '%s' '%s'", certFile, certStorePath)] = "1"
-		cmdMap[fmt.Sprintf("openssl x509 -hash -noout -in '%s'", certFile)] = certNameHash
-		cmdMap[fmt.Sprintf("sudo ln -s '%s' '%s'", certStorePath, remoteCertHashLink)] = "1"
+		cmdMap[fmt.Sprintf("sudo ln -s %s %s", certFile, certStorePath)] = "1"
+		cmdMap[fmt.Sprintf("openssl x509 -hash -noout -in %s", certFile)] = certNameHash
+		cmdMap[fmt.Sprintf("sudo ln -s %s %s", certStorePath, remoteCertHashLink)] = "1"
 	}
 	f := command.NewFakeCommandRunner()
 	f.SetCommandToOutput(cmdMap)

pkg/minikube/bootstrapper/kubeadm/kubeadm.go

@@ -19,6 +19,7 @@ package kubeadm
 import (
 	"bytes"
 	"crypto/tls"
+	"os/exec"
 
 	"fmt"
 	"net"
@@ -138,12 +139,11 @@ func NewKubeadmBootstrapper(api libmachine.API) (*Bootstrapper, error) {
 
 // GetKubeletStatus returns the kubelet status
 func (k *Bootstrapper) GetKubeletStatus() (string, error) {
-	statusCmd := `sudo systemctl is-active kubelet`
-	status, err := k.c.CombinedOutput(statusCmd)
+	rr, err := k.c.RunCmd(exec.Command("sudo", "systemctl", "is-active", "kubelet"))
 	if err != nil {
-		return "", errors.Wrap(err, "getting status")
+		return "", errors.Wrapf(err, "getting kublet status. command: %q", rr.Command())
 	}
-	s := strings.TrimSpace(status)
+	s := strings.TrimSpace(rr.Stdout.String())
 	switch s {
 	case "active":
 		return state.Running.String(), nil
@@ -224,16 +224,16 @@ func etcdDataDir() string {
 // createCompatSymlinks creates compatibility symlinks to transition running services to new directory structures
 func (k *Bootstrapper) createCompatSymlinks() error {
 	legacyEtcd := "/data/minikube"
-	if err := k.c.Run(fmt.Sprintf("sudo test -d %s", legacyEtcd)); err != nil {
-		glog.Infof("%s check failed, skipping compat symlinks: %v", legacyEtcd, err)
+
+	if _, err := k.c.RunCmd(exec.Command("sudo", "test", "-d", legacyEtcd)); err != nil {
+		glog.Infof("%s skipping compat symlinks: %v", legacyEtcd, err)
 		return nil
 	}
-
 	glog.Infof("Found %s, creating compatibility symlinks ...", legacyEtcd)
-	cmd := fmt.Sprintf("sudo ln -s %s %s", legacyEtcd, etcdDataDir())
-	out, err := k.c.CombinedOutput(cmd)
-	if err != nil {
-		return errors.Wrapf(err, "cmd failed: %s\n%s\n", cmd, out)
+
+	c := exec.Command("sudo", "ln", "-s", legacyEtcd, etcdDataDir())
+	if rr, err := k.c.RunCmd(c); err != nil {
+		return errors.Wrapf(err, "create symlink failed: %s", rr.Command())
 	}
 	return nil
 }
@@ -275,11 +275,9 @@ func (k *Bootstrapper) StartCluster(k8s config.KubernetesConfig) error {
 		ignore = append(ignore, "SystemVerification")
 	}
 
-	cmd := fmt.Sprintf("%s init --config %s %s --ignore-preflight-errors=%s",
-		invokeKubeadm(k8s.KubernetesVersion), yamlConfigPath, extraFlags, strings.Join(ignore, ","))
-	out, err := k.c.CombinedOutput(cmd)
-	if err != nil {
-		return errors.Wrapf(err, "cmd failed: %s\n%s\n", cmd, out)
+	c := exec.Command("/bin/bash", "-c", fmt.Sprintf("%s init --config %s %s --ignore-preflight-errors=%s", invokeKubeadm(k8s.KubernetesVersion), yamlConfigPath, extraFlags, strings.Join(ignore, ",")))
+	if rr, err := k.c.RunCmd(c); err != nil {
+		return errors.Wrapf(err, "init failed. cmd: %q", rr.Command())
 	}
 
 	glog.Infof("Configuring cluster permissions ...")
@@ -304,22 +302,23 @@ func (k *Bootstrapper) StartCluster(k8s config.KubernetesConfig) error {
 
 // adjustResourceLimits makes fine adjustments to pod resources that aren't possible via kubeadm config.
 func (k *Bootstrapper) adjustResourceLimits() error {
-	score, err := k.c.CombinedOutput("cat /proc/$(pgrep kube-apiserver)/oom_adj")
+	rr, err := k.c.RunCmd(exec.Command("/bin/bash", "-c", "cat /proc/$(pgrep kube-apiserver)/oom_adj"))
 	if err != nil {
-		return errors.Wrap(err, "oom_adj check")
+		return errors.Wrapf(err, "oom_adj check cmd %s. ", rr.Command())
 	}
-	glog.Infof("apiserver oom_adj: %s", score)
+	glog.Infof("apiserver oom_adj: %s", rr.Stdout.String())
 	// oom_adj is already a negative number
-	if strings.HasPrefix(score, "-") {
+	if strings.HasPrefix(rr.Stdout.String(), "-") {
 		return nil
 	}
 	glog.Infof("adjusting apiserver oom_adj to -10")
 
 	// Prevent the apiserver from OOM'ing before other pods, as it is our gateway into the cluster.
 	// It'd be preferable to do this via Kubernetes, but kubeadm doesn't have a way to set pod QoS.
-	if err := k.c.Run("echo -10 | sudo tee /proc/$(pgrep kube-apiserver)/oom_adj"); err != nil {
-		return errors.Wrap(err, "oom_adj adjust")
+	if _, err = k.c.RunCmd(exec.Command("/bin/bash", "-c", "echo -10 | sudo tee /proc/$(pgrep kube-apiserver)/oom_adj")); err != nil {
+		return errors.Wrap(err, fmt.Sprintf("oom_adj adjust"))
 	}
+
 	return nil
 }
 
@@ -465,18 +464,20 @@ func (k *Bootstrapper) RestartCluster(k8s config.KubernetesConfig) error {
 	}
 
 	// Run commands one at a time so that it is easier to root cause failures.
-	for _, cmd := range cmds {
-		if err := k.c.Run(cmd); err != nil {
-			return errors.Wrapf(err, "running cmd: %s", cmd)
+	for _, c := range cmds {
+		rr, err := k.c.RunCmd(exec.Command("/bin/bash", "-c", c))
+		if err != nil {
+			return errors.Wrapf(err, "running cmd: %s", rr.Command())
 		}
 	}
 
 	if err := k.waitForAPIServer(k8s); err != nil {
 		return errors.Wrap(err, "waiting for apiserver")
 	}
+
 	// restart the proxy and coredns
-	if err := k.c.Run(fmt.Sprintf("%s phase addon all --config %s", baseCmd, yamlConfigPath)); err != nil {
-		return errors.Wrapf(err, "addon phase")
+	if rr, err := k.c.RunCmd(exec.Command("/bin/bash", "-c", fmt.Sprintf("%s phase addon all --config %s", baseCmd, yamlConfigPath))); err != nil {
+		return errors.Wrapf(err, fmt.Sprintf("addon phase cmd:%q", rr.Command()))
 	}
 
 	if err := k.adjustResourceLimits(); err != nil {
@@ -496,9 +497,9 @@ func (k *Bootstrapper) waitForAPIServer(k8s config.KubernetesConfig) error {
 	// To give a better error message, first check for process existence via ssh
 	// Needs minutes in case the image isn't cached (such as with v1.10.x)
 	err := wait.PollImmediate(time.Millisecond*300, time.Minute*3, func() (bool, error) {
-		ierr := k.c.Run(`sudo pgrep kube-apiserver`)
+		rr, ierr := k.c.RunCmd(exec.Command("sudo", "pgrep", "kube-apiserver"))
 		if ierr != nil {
-			glog.Warningf("pgrep apiserver: %v", ierr)
+			glog.Warningf("pgrep apiserver: %v cmd: %s", ierr, rr.Command())
 			return false, nil
 		}
 		return true, nil
@@ -549,9 +550,9 @@ func (k *Bootstrapper) DeleteCluster(k8s config.KubernetesConfig) error {
 	if version.LT(semver.MustParse("1.11.0")) {
 		cmd = fmt.Sprintf("%s reset", invokeKubeadm(k8s.KubernetesVersion))
 	}
-	out, err := k.c.CombinedOutput(cmd)
-	if err != nil {
-		return errors.Wrapf(err, "kubeadm reset: %s\n%s\n", cmd, out)
+
+	if rr, err := k.c.RunCmd(exec.Command("/bin/bash", "-c", cmd)); err != nil {
+		return errors.Wrapf(err, "kubeadm reset: cmd: %q", rr.Command())
 	}
 
 	return nil
@@ -567,9 +568,9 @@ func (k *Bootstrapper) PullImages(k8s config.KubernetesConfig) error {
 		return fmt.Errorf("pull command is not supported by kubeadm v%s", version)
 	}
 
-	cmd := fmt.Sprintf("%s config images pull --config %s", invokeKubeadm(k8s.KubernetesVersion), yamlConfigPath)
-	if err := k.c.Run(cmd); err != nil {
-		return errors.Wrapf(err, "running cmd: %s", cmd)
+	rr, err := k.c.RunCmd(exec.Command("/bin/bash", "-c", fmt.Sprintf("%s config images pull --config %s", invokeKubeadm(k8s.KubernetesVersion), yamlConfigPath)))
+	if err != nil {
+		return errors.Wrapf(err, "running cmd: %q", rr.Command())
 	}
 	return nil
 }
@@ -663,11 +664,12 @@ func (k *Bootstrapper) UpdateCluster(cfg config.KubernetesConfig) error {
 
 	glog.Infof("kubelet %s config:\n%s", cfg.KubernetesVersion, kubeletCfg)
 
+	stopCmd := exec.Command("/bin/bash", "-c", "pgrep kubelet && sudo systemctl stop kubelet")
 	// stop kubelet to avoid "Text File Busy" error
-	err = k.c.Run(`pgrep kubelet && sudo systemctl stop kubelet`)
-	if err != nil {
-		glog.Warningf("unable to stop kubelet: %s", err)
+	if rr, err := k.c.RunCmd(stopCmd); err != nil {
+		glog.Warningf("unable to stop kubelet: %s command: %q output: %q", err, rr.Command(), rr.Output())
 	}
+
 	if err := transferBinaries(cfg, k.c); err != nil {
 		return errors.Wrap(err, "downloading binaries")
 	}
@@ -681,7 +683,7 @@ func (k *Bootstrapper) UpdateCluster(cfg config.KubernetesConfig) error {
 		}
 	}
 
-	if err := k.c.Run(`sudo systemctl daemon-reload && sudo systemctl start kubelet`); err != nil {
+	if _, err := k.c.RunCmd(exec.Command("/bin/bash", "-c", "sudo systemctl daemon-reload && sudo systemctl start kubelet")); err != nil {
 		return errors.Wrap(err, "starting kubelet")
 	}
 	return nil

pkg/minikube/cluster/mount.go

@@ -19,12 +19,14 @@ package cluster
 import (
 	"fmt"
 	"os"
+	"os/exec"
 	"sort"
 	"strconv"
 	"strings"
 
 	"github.com/golang/glog"
 	"github.com/pkg/errors"
+	"k8s.io/minikube/pkg/minikube/command"
 )
 
 // MountConfig defines the options available to the Mount command
@@ -49,7 +51,7 @@ type MountConfig struct {
 
 // mountRunner is the subset of CommandRunner used for mounting
 type mountRunner interface {
-	CombinedOutput(string) (string, error)
+	RunCmd(*exec.Cmd) (*command.RunResult, error)
 }
 
 // Mount runs the mount command from the 9p client on the VM to the 9p server on the host
@@ -58,14 +60,16 @@ func Mount(r mountRunner, source string, target string, c *MountConfig) error {
 		return errors.Wrap(err, "umount")
 	}
 
-	cmd := fmt.Sprintf("sudo mkdir -m %o -p %s && %s", c.Mode, target, mntCmd(source, target, c))
-	glog.Infof("Will run: %s", cmd)
-	out, err := r.CombinedOutput(cmd)
+	if _, err := r.RunCmd(exec.Command("/bin/bash", "-c", fmt.Sprintf("sudo mkdir -m %o -p %s && %s", c.Mode, target, mntCmd(source, target, c)))); err != nil {
+		return errors.Wrap(err, "create folder pre-mount")
+	}
+
+	rr, err := r.RunCmd(exec.Command("/bin/bash", "-c", mntCmd(source, target, c)))
 	if err != nil {
-		glog.Infof("%s failed: err=%s, output: %q", cmd, err, out)
-		return errors.Wrap(err, out)
+		return errors.Wrapf(err, "mount with cmd %s ", rr.Command())
 	}
-	glog.Infof("%s output: %q", cmd, out)
+
+	glog.Infof("mount successful: %q", rr.Output())
 	return nil
 }
 
@@ -131,20 +135,13 @@ func mntCmd(source string, target string, c *MountConfig) string {
 	return fmt.Sprintf("sudo mount -t %s -o %s %s %s", c.Type, strings.Join(opts, ","), source, target)
 }
 
-// umountCmd returns a command for unmounting
-func umountCmd(target string) string {
-	// grep because findmnt will also display the parent!
-	return fmt.Sprintf("[ \"x$(findmnt -T %s | grep %s)\" != \"x\" ] && sudo umount -f %s || echo ", target, target, target)
-}
-
 // Unmount unmounts a path
 func Unmount(r mountRunner, target string) error {
-	cmd := umountCmd(target)
-	glog.Infof("Will run: %s", cmd)
-	out, err := r.CombinedOutput(cmd)
-	glog.Infof("unmount force err=%v, out=%s", err, out)
-	if err != nil {
-		return errors.Wrap(err, out)
+	// grep because findmnt will also display the parent!
+	c := exec.Command("/bin/bash", "-c", fmt.Sprintf("[ \"x$(findmnt -T %s | grep %s)\" != \"x\" ] && sudo umount -f %s || echo ", target, target, target))
+	if _, err := r.RunCmd(c); err != nil {
+		return errors.Wrap(err, "unmount")
 	}
+	glog.Infof("unmount for %s ran successfully", target)
 	return nil
 }

pkg/minikube/cluster/mount_test.go

@@ -23,50 +23,27 @@ import (
 	"github.com/google/go-cmp/cmp"
 )
 
-type mockMountRunner struct {
-	cmds []string
-	T    *testing.T
-}
-
-func newMockMountRunner(t *testing.T) *mockMountRunner {
-	return &mockMountRunner{
-		T:    t,
-		cmds: []string{},
-	}
-}
-
-func (m *mockMountRunner) CombinedOutput(cmd string) (string, error) {
-	m.cmds = append(m.cmds, cmd)
-	return "", nil
-}
-
-func TestMount(t *testing.T) {
+func TestMntCmd(t *testing.T) {
 	var tests = []struct {
 		name   string
 		source string
 		target string
 		cfg    *MountConfig
-		want   []string
+		want   string
 	}{
 		{
 			name:   "simple",
 			source: "src",
 			target: "target",
 			cfg:    &MountConfig{Type: "9p", Mode: os.FileMode(0700)},
-			want: []string{
-				"[ \"x$(findmnt -T target | grep target)\" != \"x\" ] && sudo umount -f target || echo ",
-				"sudo mkdir -m 700 -p target && sudo mount -t 9p -o dfltgid=0,dfltuid=0 src target",
-			},
+			want:   "sudo mount -t 9p -o dfltgid=0,dfltuid=0 src target",
 		},
 		{
 			name:   "named uid",
 			source: "src",
 			target: "target",
 			cfg:    &MountConfig{Type: "9p", Mode: os.FileMode(0700), UID: "docker", GID: "docker"},
-			want: []string{
-				"[ \"x$(findmnt -T target | grep target)\" != \"x\" ] && sudo umount -f target || echo ",
-				"sudo mkdir -m 700 -p target && sudo mount -t 9p -o dfltgid=$(grep ^docker: /etc/group | cut -d: -f3),dfltuid=$(id -u docker) src target",
-			},
+			want:   "sudo mount -t 9p -o dfltgid=$(grep ^docker: /etc/group | cut -d: -f3),dfltuid=$(id -u docker) src target",
 		},
 		{
 			name:   "everything",
@@ -76,10 +53,7 @@ func TestMount(t *testing.T) {
 				"noextend": "",
 				"cache":    "fscache",
 			}},
-			want: []string{
-				"[ \"x$(findmnt -T /target | grep /target)\" != \"x\" ] && sudo umount -f /target || echo ",
-				"sudo mkdir -m 777 -p /target && sudo mount -t 9p -o cache=fscache,dfltgid=72,dfltuid=82,noextend,version=9p2000.u 10.0.0.1 /target",
-			},
+			want: "sudo mount -t 9p -o cache=fscache,dfltgid=72,dfltuid=82,noextend,version=9p2000.u 10.0.0.1 /target",
 		},
 		{
 			name:   "version-conflict",
@@ -88,35 +62,17 @@ func TestMount(t *testing.T) {
 			cfg: &MountConfig{Type: "9p", Mode: os.FileMode(0700), Version: "9p2000.u", Options: map[string]string{
 				"version": "9p2000.L",
 			}},
-			want: []string{
-				"[ \"x$(findmnt -T tgt | grep tgt)\" != \"x\" ] && sudo umount -f tgt || echo ",
-				"sudo mkdir -m 700 -p tgt && sudo mount -t 9p -o dfltgid=0,dfltuid=0,version=9p2000.L src tgt",
-			},
+			want: "sudo mount -t 9p -o dfltgid=0,dfltuid=0,version=9p2000.L src tgt",
 		},
 	}
+
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
-			r := newMockMountRunner(t)
-			err := Mount(r, tc.source, tc.target, tc.cfg)
-			if err != nil {
-				t.Fatalf("Mount(%s, %s, %+v): %v", tc.source, tc.target, tc.cfg, err)
-			}
-			if diff := cmp.Diff(r.cmds, tc.want); diff != "" {
+			got := mntCmd(tc.source, tc.target, tc.cfg)
+			want := tc.want
+			if diff := cmp.Diff(got, want); diff != "" {
 				t.Errorf("command diff (-want +got): %s", diff)
 			}
 		})
 	}
 }
-
-func TestUnmount(t *testing.T) {
-	r := newMockMountRunner(t)
-	err := Unmount(r, "/mnt")
-	if err != nil {
-		t.Fatalf("Unmount(/mnt): %v", err)
-	}
-
-	want := []string{"[ \"x$(findmnt -T /mnt | grep /mnt)\" != \"x\" ] && sudo umount -f /mnt || echo "}
-	if diff := cmp.Diff(r.cmds, want); diff != "" {
-		t.Errorf("command diff (-want +got): %s", diff)
-	}
-}

pkg/minikube/command/command_runner.go

@@ -17,34 +17,28 @@ limitations under the License.
 package command
 
 import (
+	"bytes"
 	"fmt"
-	"io"
+	"os/exec"
 	"path"
+	"strings"
 
 	"k8s.io/minikube/pkg/minikube/assets"
 )
 
+// RunResult holds the results of a Runner
+type RunResult struct {
+	Stdout   bytes.Buffer
+	Stderr   bytes.Buffer
+	ExitCode int
+	Args     []string // the args that was passed to Runner
+}
+
 // Runner represents an interface to run commands.
 type Runner interface {
-	// Run starts the specified command and waits for it to complete.
-	Run(cmd string) error
-
-	// CombinedOutputTo runs the command and stores both command
-	// output and error to out. A typical usage is:
-	//
-	//          var b bytes.Buffer
-	//          CombinedOutput(cmd, &b)
-	//          fmt.Println(b.Bytes())
-	//
-	// Or, you can set out to os.Stdout, the command output and
-	// error would show on your terminal immediately before you
-	// cmd exit. This is useful for a long run command such as
-	// continuously print running logs.
-	CombinedOutputTo(cmd string, out io.Writer) error
-
-	// CombinedOutput runs the command and returns its combined standard
-	// output and standard error.
-	CombinedOutput(cmd string) (string, error)
+	// RunCmd runs a cmd of exec.Cmd type. allowing user to set cmd.Stdin, cmd.Stdout,...
+	// not all implementors are guaranteed to handle all the properties of cmd.
+	RunCmd(cmd *exec.Cmd) (*RunResult, error)
 
 	// Copy is a convenience method that runs a command to copy a file
 	Copy(assets.CopyableFile) error
@@ -56,3 +50,29 @@ type Runner interface {
 func getDeleteFileCommand(f assets.CopyableFile) string {
 	return fmt.Sprintf("sudo rm %s", path.Join(f.GetTargetDir(), f.GetTargetName()))
 }
+
+// Command returns a human readable command string that does not induce eye fatigue
+func (rr RunResult) Command() string {
+	var sb strings.Builder
+	sb.WriteString(rr.Args[0])
+	for _, a := range rr.Args[1:] {
+		if strings.Contains(a, " ") {
+			sb.WriteString(fmt.Sprintf(` "%s"`, a))
+			continue
+		}
+		sb.WriteString(fmt.Sprintf(" %s", a))
+	}
+	return sb.String()
+}
+
+// Output returns human-readable output for an execution result
+func (rr RunResult) Output() string {
+	var sb strings.Builder
+	if rr.Stdout.Len() > 0 {
+		sb.WriteString(fmt.Sprintf("-- stdout --\n%s\n-- /stdout --", rr.Stdout.Bytes()))
+	}
+	if rr.Stderr.Len() > 0 {
+		sb.WriteString(fmt.Sprintf("\n** stderr ** \n%s\n** /stderr **", rr.Stderr.Bytes()))
+	}
+	return sb.String()
+}

pkg/minikube/command/exec_runner.go

@@ -24,6 +24,7 @@ import (
 	"path"
 	"path/filepath"
 	"strconv"
+	"time"
 
 	"github.com/golang/glog"
 	"github.com/pkg/errors"
@@ -35,41 +36,45 @@ import (
 // It implements the CommandRunner interface.
 type ExecRunner struct{}
 
-// Run starts the specified command in a bash shell and waits for it to complete.
-func (*ExecRunner) Run(cmd string) error {
-	glog.Infoln("Run:", cmd)
-	c := exec.Command("/bin/bash", "-c", cmd)
-	if err := c.Run(); err != nil {
-		return errors.Wrapf(err, "running command: %s", cmd)
+// RunCmd implements the Command Runner interface to run a exec.Cmd object
+func (*ExecRunner) RunCmd(cmd *exec.Cmd) (*RunResult, error) {
+	rr := &RunResult{Args: cmd.Args}
+	glog.Infof("(ExecRunner) Run:  %v", rr.Command())
+
+	var outb, errb io.Writer
+	if cmd.Stdout == nil {
+		var so bytes.Buffer
+		outb = io.MultiWriter(&so, &rr.Stdout)
+	} else {
+		outb = io.MultiWriter(cmd.Stdout, &rr.Stdout)
 	}
-	return nil
-}
 
-// CombinedOutputTo runs the command and stores both command
-// output and error to out.
-func (*ExecRunner) CombinedOutputTo(cmd string, out io.Writer) error {
-	glog.Infoln("Run with output:", cmd)
-	c := exec.Command("/bin/bash", "-c", cmd)
-	c.Stdout = out
-	c.Stderr = out
-	err := c.Run()
-	if err != nil {
-		return errors.Wrapf(err, "running command: %s\n.", cmd)
+	if cmd.Stderr == nil {
+		var se bytes.Buffer
+		errb = io.MultiWriter(&se, &rr.Stderr)
+	} else {
+		errb = io.MultiWriter(cmd.Stderr, &rr.Stderr)
 	}
 
-	return nil
-}
+	cmd.Stdout = outb
+	cmd.Stderr = errb
 
-// CombinedOutput runs the command  in a bash shell and returns its
-// combined standard output and standard error.
-func (e *ExecRunner) CombinedOutput(cmd string) (string, error) {
-	var b bytes.Buffer
-	err := e.CombinedOutputTo(cmd, &b)
-	if err != nil {
-		return "", errors.Wrapf(err, "running command: %s\n output: %s", cmd, b.Bytes())
+	start := time.Now()
+	err := cmd.Run()
+	elapsed := time.Since(start)
+	if err == nil {
+		// Reduce log spam
+		if elapsed > (1 * time.Second) {
+			glog.Infof("(ExecRunner) Done: %v: (%s)", rr.Command(), elapsed)
+		}
+	} else {
+		if exitError, ok := err.(*exec.ExitError); ok {
+			rr.ExitCode = exitError.ExitCode()
+		}
+		glog.Infof("(ExecRunner) Non-zero exit: %v: %v (%s)\n%s", rr.Command(), err, elapsed, rr.Output())
+		err = errors.Wrapf(err, "command failed: %s\nstdout: %s\nstderr: %s", rr.Command(), rr.Stdout.String(), rr.Stderr.String())
 	}
-	return b.String(), nil
-
+	return rr, err
 }
 
 // Copy copies a file and its permissions

pkg/minikube/command/fake_runner.go

@@ -20,9 +20,13 @@ import (
 	"bytes"
 	"fmt"
 	"io"
+	"os/exec"
+	"strings"
+	"time"
 
 	"golang.org/x/sync/syncmap"
 
+	"github.com/golang/glog"
 	"github.com/pkg/errors"
 
 	"k8s.io/minikube/pkg/minikube/assets"
@@ -43,34 +47,38 @@ func NewFakeCommandRunner() *FakeCommandRunner {
 	return &FakeCommandRunner{}
 }
 
-// Run returns nil if output has been set for the given command text.
-func (f *FakeCommandRunner) Run(cmd string) error {
-	_, err := f.CombinedOutput(cmd)
-	return err
-}
+// RunCmd implements the Command Runner interface to run a exec.Cmd object
+func (f *FakeCommandRunner) RunCmd(cmd *exec.Cmd) (*RunResult, error) {
+	rr := &RunResult{Args: cmd.Args}
+	glog.Infof("(FakeCommandRunner) Run:  %v", rr.Command())
 
-// CombinedOutputTo runs the command and stores both command
-// output and error to out.
-func (f *FakeCommandRunner) CombinedOutputTo(cmd string, out io.Writer) error {
-	value, ok := f.cmdMap.Load(cmd)
-	if !ok {
-		return fmt.Errorf("unavailable command: %s", cmd)
+	start := time.Now()
+
+	out, ok := f.cmdMap.Load(strings.Join(rr.Args, " "))
+	var buf bytes.Buffer
+	outStr := ""
+	if out != nil {
+		outStr = out.(string)
 	}
-	_, err := fmt.Fprint(out, value)
+	_, err := buf.WriteString(outStr)
 	if err != nil {
-		return err
+		return rr, errors.Wrap(err, "Writing outStr to FakeCommandRunner's buffer")
 	}
-
-	return nil
-}
-
-// CombinedOutput returns the set output for a given command text.
-func (f *FakeCommandRunner) CombinedOutput(cmd string) (string, error) {
-	out, ok := f.cmdMap.Load(cmd)
-	if !ok {
-		return "", fmt.Errorf("unavailable command: %s", cmd)
+	rr.Stdout = buf
+	rr.Stderr = buf
+
+	elapsed := time.Since(start)
+
+	if ok {
+		// Reduce log spam
+		if elapsed > (1 * time.Second) {
+			glog.Infof("(FakeCommandRunner) Done: %v: (%s)", rr.Command(), elapsed)
+		}
+	} else {
+		glog.Infof("(FakeCommandRunner) Non-zero exit: %v: (%s)\n%s", rr.Command(), elapsed, out)
+		return rr, fmt.Errorf("unavailable command: %s", rr.Command())
 	}
-	return out.(string), nil
+	return rr, nil
 }
 
 // Copy adds the filename, file contents key value pair to the stored map.

pkg/minikube/command/ssh_runner.go

@@ -17,13 +17,17 @@ limitations under the License.
 package command
 
 import (
+	"bufio"
 	"bytes"
 	"fmt"
 	"io"
+	"os/exec"
 	"path"
 	"sync"
+	"time"
 
 	"github.com/golang/glog"
+	"github.com/kballard/go-shellquote"
 	"github.com/pkg/errors"
 	"golang.org/x/crypto/ssh"
 	"golang.org/x/sync/errgroup"
@@ -55,17 +59,6 @@ func (s *SSHRunner) Remove(f assets.CopyableFile) error {
 	return sess.Run(cmd)
 }
 
-type singleWriter struct {
-	b  bytes.Buffer
-	mu sync.Mutex
-}
-
-func (w *singleWriter) Write(p []byte) (int, error) {
-	w.mu.Lock()
-	defer w.mu.Unlock()
-	return w.b.Write(p)
-}
-
 // teeSSH runs an SSH command, streaming stdout, stderr to logs
 func teeSSH(s *ssh.Session, cmd string, outB io.Writer, errB io.Writer) error {
 	outPipe, err := s.StdoutPipe()
@@ -81,13 +74,13 @@ func teeSSH(s *ssh.Session, cmd string, outB io.Writer, errB io.Writer) error {
 	wg.Add(2)
 
 	go func() {
-		if err := util.TeePrefix(util.ErrPrefix, errPipe, errB, glog.V(8).Infof); err != nil {
+		if err := teePrefix(util.ErrPrefix, errPipe, errB, glog.V(8).Infof); err != nil {
 			glog.Errorf("tee stderr: %v", err)
 		}
 		wg.Done()
 	}()
 	go func() {
-		if err := util.TeePrefix(util.OutPrefix, outPipe, outB, glog.V(8).Infof); err != nil {
+		if err := teePrefix(util.OutPrefix, outPipe, outB, glog.V(8).Infof); err != nil {
 			glog.Errorf("tee stdout: %v", err)
 		}
 		wg.Done()
@@ -97,12 +90,31 @@ func teeSSH(s *ssh.Session, cmd string, outB io.Writer, errB io.Writer) error {
 	return err
 }
 
-// Run starts a command on the remote and waits for it to return.
-func (s *SSHRunner) Run(cmd string) error {
-	glog.Infof("SSH: %s", cmd)
+// RunCmd implements the Command Runner interface to run a exec.Cmd object
+func (s *SSHRunner) RunCmd(cmd *exec.Cmd) (*RunResult, error) {
+	rr := &RunResult{Args: cmd.Args}
+	glog.Infof("(SSHRunner) Run:  %v", rr.Command())
+
+	var outb, errb io.Writer
+	start := time.Now()
+
+	if cmd.Stdout == nil {
+		var so bytes.Buffer
+		outb = io.MultiWriter(&so, &rr.Stdout)
+	} else {
+		outb = io.MultiWriter(cmd.Stdout, &rr.Stdout)
+	}
+
+	if cmd.Stderr == nil {
+		var se bytes.Buffer
+		errb = io.MultiWriter(&se, &rr.Stderr)
+	} else {
+		errb = io.MultiWriter(cmd.Stderr, &rr.Stderr)
+	}
+
 	sess, err := s.c.NewSession()
 	if err != nil {
-		return errors.Wrap(err, "NewSession")
+		return rr, errors.Wrap(err, "NewSession")
 	}
 
 	defer func() {
@@ -112,43 +124,21 @@ func (s *SSHRunner) Run(cmd string) error {
 			}
 		}
 	}()
-	var outB bytes.Buffer
-	var errB bytes.Buffer
-	err = teeSSH(sess, cmd, &outB, &errB)
-	if err != nil {
-		return errors.Wrapf(err, "command failed: %s\nstdout: %s\nstderr: %s", cmd, outB.String(), errB.String())
-	}
-	return nil
-}
 
-// CombinedOutputTo runs the command and stores both command
-// output and error to out.
-func (s *SSHRunner) CombinedOutputTo(cmd string, w io.Writer) error {
-	out, err := s.CombinedOutput(cmd)
-	if err != nil {
-		return err
-	}
-	_, err = w.Write([]byte(out))
-	return err
-}
-
-// CombinedOutput runs the command on the remote and returns its combined
-// standard output and standard error.
-func (s *SSHRunner) CombinedOutput(cmd string) (string, error) {
-	glog.Infoln("Run with output:", cmd)
-	sess, err := s.c.NewSession()
-	if err != nil {
-		return "", errors.Wrap(err, "NewSession")
-	}
-	defer sess.Close()
-
-	var combined singleWriter
-	err = teeSSH(sess, cmd, &combined, &combined)
-	out := combined.b.String()
-	if err != nil {
-		return out, err
+	elapsed := time.Since(start)
+	err = teeSSH(sess, shellquote.Join(cmd.Args...), outb, errb)
+	if err == nil {
+		// Reduce log spam
+		if elapsed > (1 * time.Second) {
+			glog.Infof("(SSHRunner) Done: %v: (%s)", rr.Command(), elapsed)
+		}
+	} else {
+		if exitError, ok := err.(*exec.ExitError); ok {
+			rr.ExitCode = exitError.ExitCode()
+		}
+		glog.Infof("(SSHRunner) Non-zero exit: %v: %v (%s)\n%s", rr.Command(), err, elapsed, rr.Output())
 	}
-	return out, nil
+	return rr, err
 }
 
 // Copy copies a file to the remote over SSH.
@@ -198,3 +188,30 @@ func (s *SSHRunner) Copy(f assets.CopyableFile) error {
 	}
 	return g.Wait()
 }
+
+// teePrefix copies bytes from a reader to writer, logging each new line.
+func teePrefix(prefix string, r io.Reader, w io.Writer, logger func(format string, args ...interface{})) error {
+	scanner := bufio.NewScanner(r)
+	scanner.Split(bufio.ScanBytes)
+	var line bytes.Buffer
+
+	for scanner.Scan() {
+		b := scanner.Bytes()
+		if _, err := w.Write(b); err != nil {
+			return err
+		}
+		if bytes.IndexAny(b, "\r\n") == 0 {
+			if line.Len() > 0 {
+				logger("%s%s", prefix, line.String())
+				line.Reset()
+			}
+			continue
+		}
+		line.Write(b)
+	}
+	// Catch trailing output in case stream does not end with a newline
+	if line.Len() > 0 {
+		logger("%s%s", prefix, line.String())
+	}
+	return nil
+}

pkg/minikube/command/ssh_runner_test.go

+/*
+Copyright 2019 The Kubernetes Authors All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package command
+
+import (
+	"bytes"
+	"fmt"
+	"strings"
+	"sync"
+	"testing"
+)
+
+func TestTeePrefix(t *testing.T) {
+	var in bytes.Buffer
+	var out bytes.Buffer
+	var logged strings.Builder
+
+	logSink := func(format string, args ...interface{}) {
+		logged.WriteString("(" + fmt.Sprintf(format, args...) + ")")
+	}
+
+	// Simulate the primary use case: tee in the background. This also helps avoid I/O races.
+	var wg sync.WaitGroup
+	wg.Add(1)
+	go func() {
+		if err := teePrefix(":", &in, &out, logSink); err != nil {
+			t.Errorf("teePrefix: %v", err)
+		}
+		wg.Done()
+	}()
+
+	in.Write([]byte("goo"))
+	in.Write([]byte("\n"))
+	in.Write([]byte("g\r\n\r\n"))
+	in.Write([]byte("le"))
+	wg.Wait()
+
+	gotBytes := out.Bytes()
+	wantBytes := []byte("goo\ng\r\n\r\nle")
+	if !bytes.Equal(gotBytes, wantBytes) {
+		t.Errorf("output=%q, want: %q", gotBytes, wantBytes)
+	}
+
+	gotLog := logged.String()
+	wantLog := "(:goo)(:g)(:le)"
+	if gotLog != wantLog {
+		t.Errorf("log=%q, want: %q", gotLog, wantLog)
+	}
+}

pkg/minikube/cruntime/containerd.go

@@ -20,11 +20,13 @@ import (
 	"bytes"
 	"encoding/base64"
 	"fmt"
+	"os/exec"
 	"path"
 	"strings"
 	"text/template"
 
 	"github.com/golang/glog"
+	"github.com/pkg/errors"
 	"k8s.io/minikube/pkg/minikube/bootstrapper/images"
 	"k8s.io/minikube/pkg/minikube/out"
 )
@@ -124,17 +126,17 @@ func (r *Containerd) Style() out.StyleEnum {
 
 // Version retrieves the current version of this runtime
 func (r *Containerd) Version() (string, error) {
-	ver, err := r.Runner.CombinedOutput("containerd --version")
+	c := exec.Command("containerd", "--version")
+	rr, err := r.Runner.RunCmd(c)
 	if err != nil {
-		return "", err
+		return "", errors.Wrapf(err, "containerd check version.")
 	}
-
 	// containerd github.com/containerd/containerd v1.2.0 c4446665cb9c30056f4998ed953e6d4ff22c7c39
-	words := strings.Split(ver, " ")
+	words := strings.Split(rr.Stdout.String(), " ")
 	if len(words) >= 4 && words[0] == "containerd" {
 		return strings.Replace(words[2], "v", "", 1), nil
 	}
-	return "", fmt.Errorf("unknown version: %q", ver)
+	return "", fmt.Errorf("unknown version: %q", rr.Stdout.String())
 }
 
 // SocketPath returns the path to the socket file for containerd
@@ -152,13 +154,18 @@ func (r *Containerd) DefaultCNI() bool {
 
 // Active returns if containerd is active on the host
 func (r *Containerd) Active() bool {
-	err := r.Runner.Run("systemctl is-active --quiet service containerd")
+	c := exec.Command("systemctl", "is-active", "--quiet", "service", "containerd")
+	_, err := r.Runner.RunCmd(c)
 	return err == nil
 }
 
 // Available returns an error if it is not possible to use this runtime on a host
 func (r *Containerd) Available() error {
-	return r.Runner.Run("command -v containerd")
+	c := exec.Command("which", "containerd")
+	if _, err := r.Runner.RunCmd(c); err != nil {
+		return errors.Wrap(err, "check containerd availability.")
+	}
+	return nil
 }
 
 // generateContainerdConfig sets up /etc/containerd/config.toml
@@ -174,7 +181,11 @@ func generateContainerdConfig(cr CommandRunner, imageRepository string, k8sVersi
 	if err := t.Execute(&b, opts); err != nil {
 		return err
 	}
-	return cr.Run(fmt.Sprintf("sudo mkdir -p %s && printf %%s \"%s\" | base64 -d | sudo tee %s", path.Dir(cPath), base64.StdEncoding.EncodeToString(b.Bytes()), cPath))
+	c := exec.Command("/bin/bash", "-c", fmt.Sprintf("sudo mkdir -p %s && printf %%s \"%s\" | base64 -d | sudo tee %s", path.Dir(cPath), base64.StdEncoding.EncodeToString(b.Bytes()), cPath))
+	if _, err := cr.RunCmd(c); err != nil {
+		return errors.Wrap(err, "generate containerd cfg.")
+	}
+	return nil
 }
 
 // Enable idempotently enables containerd on a host
@@ -194,18 +205,30 @@ func (r *Containerd) Enable(disOthers bool) error {
 		return err
 	}
 	// Otherwise, containerd will fail API requests with 'Unimplemented'
-	return r.Runner.Run("sudo systemctl restart containerd")
+	c := exec.Command("sudo", "systemctl", "restart", "containerd")
+	if _, err := r.Runner.RunCmd(c); err != nil {
+		return errors.Wrap(err, "restart containerd")
+	}
+	return nil
 }
 
 // Disable idempotently disables containerd on a host
 func (r *Containerd) Disable() error {
-	return r.Runner.Run("sudo systemctl stop containerd")
+	c := exec.Command("sudo", "systemctl", "stop", "containerd")
+	if _, err := r.Runner.RunCmd(c); err != nil {
+		return errors.Wrapf(err, "stop containerd")
+	}
+	return nil
 }
 
 // LoadImage loads an image into this runtime
 func (r *Containerd) LoadImage(path string) error {
 	glog.Infof("Loading image: %s", path)
-	return r.Runner.Run(fmt.Sprintf("sudo ctr -n=k8s.io images import %s", path))
+	c := exec.Command("sudo", "ctr", "-n=k8s.io", "images", "import", path)
+	if _, err := r.Runner.RunCmd(c); err != nil {
+		return errors.Wrapf(err, "ctr images import")
+	}
+	return nil
 }
 
 // KubeletOptions returns kubelet options for a containerd

pkg/minikube/cruntime/cri.go

@@ -21,11 +21,14 @@ import (
 	"encoding/base64"
 	"fmt"
 	"html/template"
+	"os/exec"
 	"path"
 	"strings"
 
 	"github.com/golang/glog"
+	"github.com/pkg/errors"
 	"k8s.io/minikube/pkg/minikube/bootstrapper/images"
+	"k8s.io/minikube/pkg/minikube/command"
 )
 
 const (
@@ -330,19 +333,20 @@ plugin_dirs = [
 
 // listCRIContainers returns a list of containers using crictl
 func listCRIContainers(cr CommandRunner, filter string) ([]string, error) {
-	var content string
 	var err error
+	var rr *command.RunResult
 	state := "Running"
 	if filter != "" {
-		content, err = cr.CombinedOutput(fmt.Sprintf(`sudo crictl ps -a --name=%s --state=%s --quiet`, filter, state))
+		c := exec.Command("sudo", "crictl", "ps", "-a", fmt.Sprintf("--name=%s", filter), fmt.Sprintf("--state=%s", state), "--quiet")
+		rr, err = cr.RunCmd(c)
 	} else {
-		content, err = cr.CombinedOutput(fmt.Sprintf(`sudo crictl ps -a --state=%s --quiet`, state))
+		rr, err = cr.RunCmd(exec.Command("sudo", "crictl", "ps", "-a", fmt.Sprintf("--state=%s", state), "--quiet"))
 	}
 	if err != nil {
 		return nil, err
 	}
 	var ids []string
-	for _, line := range strings.Split(content, "\n") {
+	for _, line := range strings.Split(rr.Stderr.String(), "\n") {
 		if line != "" {
 			ids = append(ids, line)
 		}
@@ -356,7 +360,13 @@ func killCRIContainers(cr CommandRunner, ids []string) error {
 		return nil
 	}
 	glog.Infof("Killing containers: %s", ids)
-	return cr.Run(fmt.Sprintf("sudo crictl rm %s", strings.Join(ids, " ")))
+
+	args := append([]string{"crictl", "rm"}, ids...)
+	c := exec.Command("sudo", args...)
+	if _, err := cr.RunCmd(c); err != nil {
+		return errors.Wrap(err, "kill cri containers.")
+	}
+	return nil
 }
 
 // stopCRIContainers stops containers using crictl
@@ -365,7 +375,13 @@ func stopCRIContainers(cr CommandRunner, ids []string) error {
 		return nil
 	}
 	glog.Infof("Stopping containers: %s", ids)
-	return cr.Run(fmt.Sprintf("sudo crictl stop %s", strings.Join(ids, " ")))
+	args := append([]string{"crictl", "rm"}, ids...)
+	c := exec.Command("sudo", args...)
+	if _, err := cr.RunCmd(c); err != nil {
+		return errors.Wrap(err, "stop cri containers")
+	}
+	return nil
+
 }
 
 // populateCRIConfig sets up /etc/crictl.yaml
@@ -383,7 +399,11 @@ image-endpoint: unix://{{.Socket}}
 	if err := t.Execute(&b, opts); err != nil {
 		return err
 	}
-	return cr.Run(fmt.Sprintf("sudo mkdir -p %s && printf %%s \"%s\" | sudo tee %s", path.Dir(cPath), b.String(), cPath))
+	c := exec.Command("/bin/bash", "-c", fmt.Sprintf("sudo mkdir -p %s && printf %%s \"%s\" | sudo tee %s", path.Dir(cPath), b.String(), cPath))
+	if rr, err := cr.RunCmd(c); err != nil {
+		return errors.Wrapf(err, "Run: %q", rr.Command())
+	}
+	return nil
 }
 
 // generateCRIOConfig sets up /etc/crio/crio.conf
@@ -399,7 +419,12 @@ func generateCRIOConfig(cr CommandRunner, imageRepository string, k8sVersion str
 	if err := t.Execute(&b, opts); err != nil {
 		return err
 	}
-	return cr.Run(fmt.Sprintf("sudo mkdir -p %s && printf %%s \"%s\" | base64 -d | sudo tee %s", path.Dir(cPath), base64.StdEncoding.EncodeToString(b.Bytes()), cPath))
+
+	c := exec.Command("/bin/bash", "-c", fmt.Sprintf("sudo mkdir -p %s && printf %%s \"%s\" | base64 -d | sudo tee %s", path.Dir(cPath), base64.StdEncoding.EncodeToString(b.Bytes()), cPath))
+	if _, err := cr.RunCmd(c); err != nil {
+		return errors.Wrap(err, "generateCRIOConfig.")
+	}
+	return nil
 }
 
 // criContainerLogCmd returns the command to retrieve the log for a container based on ID

pkg/minikube/cruntime/crio.go

@@ -18,9 +18,11 @@ package cruntime
 
 import (
 	"fmt"
+	"os/exec"
 	"strings"
 
 	"github.com/golang/glog"
+	"github.com/pkg/errors"
 	"k8s.io/minikube/pkg/minikube/out"
 )
 
@@ -44,14 +46,15 @@ func (r *CRIO) Style() out.StyleEnum {
 
 // Version retrieves the current version of this runtime
 func (r *CRIO) Version() (string, error) {
-	ver, err := r.Runner.CombinedOutput("crio --version")
+	c := exec.Command("crio", "--version")
+	rr, err := r.Runner.RunCmd(c)
 	if err != nil {
-		return "", err
+		return "", errors.Wrap(err, "crio version.")
 	}
 
 	// crio version 1.13.0
 	// commit: ""
-	line := strings.Split(ver, "\n")[0]
+	line := strings.Split(rr.Stdout.String(), "\n")[0]
 	return strings.Replace(line, "crio version ", "", 1), nil
 }
 
@@ -70,12 +73,18 @@ func (r *CRIO) DefaultCNI() bool {
 
 // Available returns an error if it is not possible to use this runtime on a host
 func (r *CRIO) Available() error {
-	return r.Runner.Run("command -v crio")
+	c := exec.Command("which", "crio")
+	if _, err := r.Runner.RunCmd(c); err != nil {
+		return errors.Wrapf(err, "check crio available.")
+	}
+	return nil
+
 }
 
 // Active returns if CRIO is active on the host
 func (r *CRIO) Active() bool {
-	err := r.Runner.Run("systemctl is-active --quiet service crio")
+	c := exec.Command("systemctl", "is-active", "--quiet", "service", "crio")
+	_, err := r.Runner.RunCmd(c)
 	return err == nil
 }
 
@@ -95,18 +104,29 @@ func (r *CRIO) Enable(disOthers bool) error {
 	if err := enableIPForwarding(r.Runner); err != nil {
 		return err
 	}
-	return r.Runner.Run("sudo systemctl restart crio")
+
+	if _, err := r.Runner.RunCmd(exec.Command("sudo", "systemctl", "restart", "crio")); err != nil {
+		return errors.Wrapf(err, "enable crio.")
+	}
+	return nil
 }
 
 // Disable idempotently disables CRIO on a host
 func (r *CRIO) Disable() error {
-	return r.Runner.Run("sudo systemctl stop crio")
+	if _, err := r.Runner.RunCmd(exec.Command("sudo", "systemctl", "stop", "crio")); err != nil {
+		return errors.Wrapf(err, "disable crio.")
+	}
+	return nil
 }
 
 // LoadImage loads an image into this runtime
 func (r *CRIO) LoadImage(path string) error {
 	glog.Infof("Loading image: %s", path)
-	return r.Runner.Run(fmt.Sprintf("sudo podman load -i %s", path))
+	c := exec.Command("sudo", "podman", "load", "-i", path)
+	if _, err := r.Runner.RunCmd(c); err != nil {
+		return errors.Wrap(err, "crio load image")
+	}
+	return nil
 }
 
 // KubeletOptions returns kubelet options for a runtime.

pkg/minikube/cruntime/cruntime.go

@@ -19,16 +19,17 @@ package cruntime
 
 import (
 	"fmt"
+	"os/exec"
 
 	"github.com/golang/glog"
 	"github.com/pkg/errors"
+	"k8s.io/minikube/pkg/minikube/command"
 	"k8s.io/minikube/pkg/minikube/out"
 )
 
 // CommandRunner is the subset of command.Runner this package consumes
 type CommandRunner interface {
-	Run(string) error
-	CombinedOutput(string) (string, error)
+	RunCmd(cmd *exec.Cmd) (*command.RunResult, error)
 }
 
 // Manager is a common interface for container runtimes
@@ -130,11 +131,14 @@ func disableOthers(me Manager, cr CommandRunner) error {
 // enableIPForwarding configures IP forwarding, which is handled normally by Docker
 // Context: https://github.com/kubernetes/kubeadm/issues/1062
 func enableIPForwarding(cr CommandRunner) error {
-	if err := cr.Run("sudo modprobe br_netfilter"); err != nil {
+	c := exec.Command("sudo", "modprobe", "br_netfilter")
+	if _, err := cr.RunCmd(c); err != nil {
 		return errors.Wrap(err, "br_netfilter")
 	}
-	if err := cr.Run("sudo sh -c \"echo 1 > /proc/sys/net/ipv4/ip_forward\""); err != nil {
-		return errors.Wrap(err, "ip_forward")
+
+	c = exec.Command("sudo", "sh", "-c", "echo 1 > /proc/sys/net/ipv4/ip_forward")
+	if _, err := cr.RunCmd(c); err != nil {
+		return errors.Wrapf(err, "ip_forward")
 	}
 	return nil
 }

pkg/minikube/cruntime/cruntime_test.go

@@ -17,12 +17,16 @@ limitations under the License.
 package cruntime
 
 import (
+	"bytes"
 	"fmt"
+	"os/exec"
 	"strings"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
+	"github.com/pkg/errors"
+	"k8s.io/minikube/pkg/minikube/command"
 )
 
 func TestName(t *testing.T) {
@@ -111,46 +115,102 @@ func NewFakeRunner(t *testing.T) *FakeRunner {
 }
 
 // Run a fake command!
-func (f *FakeRunner) CombinedOutput(cmd string) (string, error) {
-	f.cmds = append(f.cmds, cmd)
-
+func (f *FakeRunner) RunCmd(cmd *exec.Cmd) (*command.RunResult, error) {
+	xargs := cmd.Args
+	f.cmds = append(f.cmds, xargs...)
 	root := false
-	args := strings.Split(cmd, " ")
-	bin, args := args[0], args[1:]
+	bin, args := xargs[0], xargs[1:]
 	f.t.Logf("bin=%s args=%v", bin, args)
 	if bin == "sudo" {
 		root = true
-		bin, args = args[0], args[1:]
+		bin, args = xargs[1], xargs[2:]
 	}
 	switch bin {
 	case "systemctl":
-		return f.systemctl(args, root)
+		s, err := f.systemctl(args, root)
+		rr := &command.RunResult{}
+		if err != nil {
+			return rr, err
+		}
+		var buf bytes.Buffer
+		_, err = buf.WriteString(s)
+		if err != nil {
+			return rr, errors.Wrap(err, "Writing outStr to FakeRunner's buffer")
+		}
+		rr.Stdout = buf
+		rr.Stderr = buf
+		return rr, err
 	case "docker":
-		return f.docker(args, root)
+		s, err := f.docker(args, root)
+		rr := &command.RunResult{}
+		if err != nil {
+			return rr, err
+		}
+		var buf bytes.Buffer
+		_, err = buf.WriteString(s)
+		if err != nil {
+			return rr, errors.Wrap(err, "Writing FakeRunner's buffer")
+		}
+		rr.Stdout = buf
+		rr.Stderr = buf
+		return rr, err
+
 	case "crictl":
-		return f.crictl(args, root)
+		s, err := f.crictl(args, root)
+		rr := &command.RunResult{}
+		if err != nil {
+			return rr, err
+		}
+		var buf bytes.Buffer
+		_, err = buf.WriteString(s)
+		if err != nil {
+			return rr, errors.Wrap(err, "Writing to FakeRunner's buffer")
+		}
+		rr.Stdout = buf
+		rr.Stderr = buf
+		return rr, err
 	case "crio":
-		return f.crio(args, root)
+		s, err := f.crio(args, root)
+		rr := &command.RunResult{}
+		if err != nil {
+			return rr, err
+		}
+		var buf bytes.Buffer
+		_, err = buf.WriteString(s)
+		if err != nil {
+			return rr, errors.Wrap(err, "Writing to FakeRunner's buffer")
+		}
+		rr.Stdout = buf
+		rr.Stderr = buf
+		return rr, err
 	case "containerd":
-		return f.containerd(args, root)
+		s, err := f.containerd(args, root)
+		rr := &command.RunResult{}
+		if err != nil {
+			return rr, err
+		}
+
+		var buf bytes.Buffer
+		_, err = buf.WriteString(s)
+		if err != nil {
+			return rr, errors.Wrap(err, "Writing to FakeRunner's buffer")
+		}
+		rr.Stdout = buf
+		rr.Stderr = buf
+		return rr, err
 	default:
-		return "", nil
+		rr := &command.RunResult{}
+		return rr, nil
 	}
 }
 
-// Run a fake command!
-func (f *FakeRunner) Run(cmd string) error {
-	_, err := f.CombinedOutput(cmd)
-	return err
-}
-
 // docker is a fake implementation of docker
 func (f *FakeRunner) docker(args []string, _ bool) (string, error) {
 	switch cmd := args[0]; cmd {
 	case "ps":
 		// ps -a --filter="name=apiserver" --format="{{.ID}}"
 		if args[1] == "-a" && strings.HasPrefix(args[2], "--filter") {
-			filter := strings.Split(args[2], `"`)[1]
+			filter := strings.Split(args[2], `r=`)[1]
 			fname := strings.Split(filter, "=")[1]
 			ids := []string{}
 			f.t.Logf("fake docker: Looking for containers matching %q", fname)
@@ -163,7 +223,8 @@ func (f *FakeRunner) docker(args []string, _ bool) (string, error) {
 			return strings.Join(ids, "\n"), nil
 		}
 	case "stop":
-		for _, id := range args[1:] {
+		ids := strings.Split(args[1], " ")
+		for _, id := range ids {
 			f.t.Logf("fake docker: Stopping id %q", id)
 			if f.containers[id] == "" {
 				return "", fmt.Errorf("no such container")
@@ -181,16 +242,16 @@ func (f *FakeRunner) docker(args []string, _ bool) (string, error) {
 
 		}
 	case "version":
+
 		if args[1] == "--format" && args[2] == "'{{.Server.Version}}'" {
 			return "18.06.2-ce", nil
 		}
-
 	}
 	return "", nil
 }
 
 // crio is a fake implementation of crio
-func (f *FakeRunner) crio(args []string, _ bool) (string, error) {
+func (f *FakeRunner) crio(args []string, _ bool) (string, error) { //nolint (result 1 (error) is always nil)
 	if args[0] == "--version" {
 		return "crio version 1.13.0", nil
 	}
@@ -202,6 +263,9 @@ func (f *FakeRunner) containerd(args []string, _ bool) (string, error) {
 	if args[0] == "--version" {
 		return "containerd github.com/containerd/containerd v1.2.0 c4446665cb9c30056f4998ed953e6d4ff22c7c39", nil
 	}
+	if args[0] != "--version" { // doing this to suppress lint "result 1 (error) is always nil"
+		return "", fmt.Errorf("unknown args[0]")
+	}
 	return "", nil
 }
 
@@ -253,7 +317,7 @@ func (f *FakeRunner) crictl(args []string, _ bool) (string, error) {
 }
 
 // systemctl is a fake implementation of systemctl
-func (f *FakeRunner) systemctl(args []string, root bool) (string, error) {
+func (f *FakeRunner) systemctl(args []string, root bool) (string, error) { // nolint result 0 (string) is always ""
 	action := args[0]
 	svcs := args[1:]
 	out := ""
@@ -263,6 +327,7 @@ func (f *FakeRunner) systemctl(args []string, root bool) (string, error) {
 		if arg == "service" {
 			svcs = args[i+1:]
 		}
+
 	}
 
 	for _, svc := range svcs {
@@ -314,8 +379,7 @@ func TestVersion(t *testing.T) {
 	}
 	for _, tc := range tests {
 		t.Run(tc.runtime, func(t *testing.T) {
-			runner := NewFakeRunner(t)
-			r, err := New(Config{Type: tc.runtime, Runner: runner})
+			r, err := New(Config{Type: tc.runtime, Runner: NewFakeRunner(t)})
 			if err != nil {
 				t.Fatalf("New(%s): %v", tc.runtime, err)
 			}
@@ -344,9 +408,9 @@ func TestDisable(t *testing.T) {
 		runtime string
 		want    []string
 	}{
-		{"docker", []string{"sudo systemctl stop docker docker.socket"}},
-		{"crio", []string{"sudo systemctl stop crio"}},
-		{"containerd", []string{"sudo systemctl stop containerd"}},
+		{"docker", []string{"sudo", "systemctl", "stop", "docker", "docker.socket"}},
+		{"crio", []string{"sudo", "systemctl", "stop", "crio"}},
+		{"containerd", []string{"sudo", "systemctl", "stop", "containerd"}},
 	}
 	for _, tc := range tests {
 		t.Run(tc.runtime, func(t *testing.T) {

pkg/minikube/cruntime/docker.go

@@ -22,6 +22,7 @@ import (
 	"strings"
 
 	"github.com/golang/glog"
+	"github.com/pkg/errors"
 	"k8s.io/minikube/pkg/minikube/out"
 )
 
@@ -47,12 +48,12 @@ func (r *Docker) Style() out.StyleEnum {
 // Version retrieves the current version of this runtime
 func (r *Docker) Version() (string, error) {
 	// Note: the server daemon has to be running, for this call to return successfully
-	ver, err := r.Runner.CombinedOutput("docker version --format '{{.Server.Version}}'")
+	c := exec.Command("docker", "version", "--format", "'{{.Server.Version}}'")
+	rr, err := r.Runner.RunCmd(c)
 	if err != nil {
 		return "", err
 	}
-
-	return strings.Split(ver, "\n")[0], nil
+	return strings.Split(rr.Stdout.String(), "\n")[0], nil
 }
 
 // SocketPath returns the path to the socket file for Docker
@@ -73,7 +74,8 @@ func (r *Docker) Available() error {
 
 // Active returns if docker is active on the host
 func (r *Docker) Active() bool {
-	err := r.Runner.Run("systemctl is-active --quiet service docker")
+	c := exec.Command("systemctl", "is-active", "--quiet", "service", "docker")
+	_, err := r.Runner.RunCmd(c)
 	return err == nil
 }
 
@@ -84,18 +86,31 @@ func (r *Docker) Enable(disOthers bool) error {
 			glog.Warningf("disableOthers: %v", err)
 		}
 	}
-	return r.Runner.Run("sudo systemctl start docker")
+	c := exec.Command("sudo", "systemctl", "start", "docker")
+	if _, err := r.Runner.RunCmd(c); err != nil {
+		return errors.Wrap(err, "enable docker.")
+	}
+	return nil
 }
 
 // Disable idempotently disables Docker on a host
 func (r *Docker) Disable() error {
-	return r.Runner.Run("sudo systemctl stop docker docker.socket")
+	c := exec.Command("sudo", "systemctl", "stop", "docker", "docker.socket")
+	if _, err := r.Runner.RunCmd(c); err != nil {
+		return errors.Wrap(err, "disable docker")
+	}
+	return nil
 }
 
 // LoadImage loads an image into this runtime
 func (r *Docker) LoadImage(path string) error {
 	glog.Infof("Loading image: %s", path)
-	return r.Runner.Run(fmt.Sprintf("docker load -i %s", path))
+	c := exec.Command("docker", "load", "-i", path)
+	if _, err := r.Runner.RunCmd(c); err != nil {
+		return errors.Wrap(err, "loadimage docker.")
+	}
+	return nil
+
 }
 
 // KubeletOptions returns kubelet options for a runtime.
@@ -108,12 +123,12 @@ func (r *Docker) KubeletOptions() map[string]string {
 // ListContainers returns a list of containers
 func (r *Docker) ListContainers(filter string) ([]string, error) {
 	filter = KubernetesContainerPrefix + filter
-	content, err := r.Runner.CombinedOutput(fmt.Sprintf(`docker ps -a --filter="name=%s" --format="{{.ID}}"`, filter))
+	rr, err := r.Runner.RunCmd(exec.Command("docker", "ps", "-a", fmt.Sprintf("--filter=name=%s", filter), "--format=\"{{.ID}}\""))
 	if err != nil {
-		return nil, err
+		return nil, errors.Wrapf(err, "docker ListContainers. ")
 	}
 	var ids []string
-	for _, line := range strings.Split(content, "\n") {
+	for _, line := range strings.Split(rr.Stdout.String(), "\n") {
 		if line != "" {
 			ids = append(ids, line)
 		}
@@ -127,7 +142,12 @@ func (r *Docker) KillContainers(ids []string) error {
 		return nil
 	}
 	glog.Infof("Killing containers: %s", ids)
-	return r.Runner.Run(fmt.Sprintf("docker rm -f %s", strings.Join(ids, " ")))
+	args := append([]string{"rm", "-f"}, ids...)
+	c := exec.Command("docker", args...)
+	if _, err := r.Runner.RunCmd(c); err != nil {
+		return errors.Wrap(err, "Killing containers docker.")
+	}
+	return nil
 }
 
 // StopContainers stops a running container based on ID
@@ -136,7 +156,12 @@ func (r *Docker) StopContainers(ids []string) error {
 		return nil
 	}
 	glog.Infof("Stopping containers: %s", ids)
-	return r.Runner.Run(fmt.Sprintf("docker stop %s", strings.Join(ids, " ")))
+	args := append([]string{"stop"}, ids...)
+	c := exec.Command("docker", args...)
+	if _, err := r.Runner.RunCmd(c); err != nil {
+		return errors.Wrap(err, "stopping containers docker.")
+	}
+	return nil
 }
 
 // ContainerLogCmd returns the command to retrieve the log for a container based on ID

pkg/minikube/logs/logs.go

@@ -22,11 +22,13 @@ import (
 	"bytes"
 	"fmt"
 	"os"
+	"os/exec"
 	"regexp"
 	"sort"
 	"strings"
 
 	"github.com/golang/glog"
+	"github.com/pkg/errors"
 	"k8s.io/minikube/pkg/minikube/bootstrapper"
 	"k8s.io/minikube/pkg/minikube/command"
 	"k8s.io/minikube/pkg/minikube/cruntime"
@@ -51,18 +53,30 @@ var importantPods = []string{
 	"kube-controller-manager",
 }
 
+// logRunner is the subset of CommandRunner used for logging
+type logRunner interface {
+	RunCmd(*exec.Cmd) (*command.RunResult, error)
+}
+
 // lookbackwardsCount is how far back to look in a log for problems. This should be large enough to
 // include usage messages from a failed binary, but small enough to not include irrelevant problems.
 const lookBackwardsCount = 200
 
 // Follow follows logs from multiple files in tail(1) format
-func Follow(r cruntime.Manager, bs bootstrapper.Bootstrapper, runner command.Runner) error {
+func Follow(r cruntime.Manager, bs bootstrapper.Bootstrapper, cr logRunner) error {
 	cs := []string{}
 	for _, v := range logCommands(r, bs, 0, true) {
 		cs = append(cs, v+" &")
 	}
 	cs = append(cs, "wait")
-	return runner.CombinedOutputTo(strings.Join(cs, " "), os.Stdout)
+
+	cmd := exec.Command("/bin/bash", "-c", strings.Join(cs, " "))
+	cmd.Stdout = os.Stdout
+	cmd.Stderr = os.Stdout
+	if _, err := cr.RunCmd(cmd); err != nil {
+		return errors.Wrapf(err, "log follow")
+	}
+	return nil
 }
 
 // IsProblem returns whether this line matches a known problem
@@ -71,15 +85,18 @@ func IsProblem(line string) bool {
 }
 
 // FindProblems finds possible root causes among the logs
-func FindProblems(r cruntime.Manager, bs bootstrapper.Bootstrapper, runner command.Runner) map[string][]string {
+func FindProblems(r cruntime.Manager, bs bootstrapper.Bootstrapper, cr logRunner) map[string][]string {
 	pMap := map[string][]string{}
 	cmds := logCommands(r, bs, lookBackwardsCount, false)
-	for name, cmd := range cmds {
+	for name := range cmds {
 		glog.Infof("Gathering logs for %s ...", name)
 		var b bytes.Buffer
-		err := runner.CombinedOutputTo(cmds[name], &b)
-		if err != nil {
-			glog.Warningf("failed %s: %s: %v", name, cmd, err)
+		c := exec.Command("/bin/bash", "-c", cmds[name])
+		c.Stderr = &b
+		c.Stdout = &b
+
+		if rr, err := cr.RunCmd(c); err != nil {
+			glog.Warningf("failed %s: command: %s %v output: %s", name, rr.Command(), err, rr.Output())
 			continue
 		}
 		scanner := bufio.NewScanner(&b)
@@ -129,10 +146,11 @@ func Output(r cruntime.Manager, bs bootstrapper.Bootstrapper, runner command.Run
 		}
 		out.T(out.Empty, "==> {{.name}} <==", out.V{"name": name})
 		var b bytes.Buffer
-
-		err := runner.CombinedOutputTo(cmds[name], &b)
-		if err != nil {
-			glog.Errorf("failed: %v", err)
+		c := exec.Command("/bin/bash", "-c", cmds[name])
+		c.Stdout = &b
+		c.Stderr = &b
+		if rr, err := runner.RunCmd(c); err != nil {
+			glog.Errorf("command %s failed with error: %v output: %q", rr.Command(), err, rr.Output())
 			failed = append(failed, name)
 			continue
 		}

pkg/minikube/problem/err_map.go

@@ -56,7 +56,7 @@ var vmProblems = map[string]match{
 		Issues: []int{1926, 4206},
 	},
 	"HYPERKIT_NOT_FOUND": {
-		Regexp:         re(`Driver "hyperkit" not found. Do you have the plugin binary .* accessible in your PATH?`),
+		Regexp:         re(`Driver "hyperkit" not found.`),
 		Advice:         "Please install the minikube hyperkit VM driver, or select an alternative --vm-driver",
 		URL:            "https://minikube.sigs.k8s.io/docs/reference/drivers/hyperkit/",
 		HideCreateLink: true,

pkg/minikube/tunnel/route_darwin.go

@@ -156,21 +156,21 @@ func (router *osRouter) Cleanup(route *Route) error {
 	if !exists {
 		return nil
 	}
-	command := exec.Command("sudo", "route", "-n", "delete", route.DestCIDR.String())
-	stdInAndOut, err := command.CombinedOutput()
+	cmd := exec.Command("sudo", "route", "-n", "delete", route.DestCIDR.String())
+	stdInAndOut, err := cmd.CombinedOutput()
 	if err != nil {
 		return err
 	}
-	message := fmt.Sprintf("%s", stdInAndOut)
-	glog.V(4).Infof("%s", message)
+	msg := fmt.Sprintf("%s", stdInAndOut)
+	glog.V(4).Infof("%s", msg)
 	re := regexp.MustCompile("^delete net ([^:]*)$")
-	if !re.MatchString(message) {
-		return fmt.Errorf("error deleting route: %s, %d", message, len(strings.Split(message, "\n")))
+	if !re.MatchString(msg) {
+		return fmt.Errorf("error deleting route: %s, %d", msg, len(strings.Split(msg, "\n")))
 	}
 	// idempotent removal of cluster domain dns
 	resolverFile := fmt.Sprintf("/etc/resolver/%s", route.ClusterDomain)
-	command = exec.Command("sudo", "rm", "-f", resolverFile)
-	if err := command.Run(); err != nil {
+	cmd = exec.Command("sudo", "rm", "-f", resolverFile)
+	if err := cmd.Run(); err != nil {
 		return fmt.Errorf("could not remove %s: %s", resolverFile, err)
 	}
 	return nil
@@ -191,12 +191,12 @@ func writeResolverFile(route *Route) error {
 	if err = tmpFile.Close(); err != nil {
 		return err
 	}
-	command := exec.Command("sudo", "mkdir", "-p", "/etc/resolver")
-	if err := command.Run(); err != nil {
+	cmd := exec.Command("sudo", "mkdir", "-p", "/etc/resolver")
+	if err := cmd.Run(); err != nil {
 		return err
 	}
-	command = exec.Command("sudo", "cp", "-f", tmpFile.Name(), resolverFile)
-	if err := command.Run(); err != nil {
+	cmd = exec.Command("sudo", "cp", "-f", tmpFile.Name(), resolverFile)
+	if err := cmd.Run(); err != nil {
 		return err
 	}
 	return nil

pkg/util/utils.go

@@ -17,7 +17,6 @@ limitations under the License.
 package util
 
 import (
-	"bufio"
 	"bytes"
 	"fmt"
 	"io"
@@ -150,34 +149,6 @@ func MaybeChownDirRecursiveToMinikubeUser(dir string) error {
 	return nil
 }
 
-// TeePrefix copies bytes from a reader to writer, logging each new line.
-func TeePrefix(prefix string, r io.Reader, w io.Writer, logger func(format string, args ...interface{})) error {
-	scanner := bufio.NewScanner(r)
-	scanner.Split(bufio.ScanBytes)
-	var line bytes.Buffer
-
-	for scanner.Scan() {
-		b := scanner.Bytes()
-		if _, err := w.Write(b); err != nil {
-			return err
-		}
-
-		if bytes.IndexAny(b, "\r\n") == 0 {
-			if line.Len() > 0 {
-				logger("%s%s", prefix, line.String())
-				line.Reset()
-			}
-			continue
-		}
-		line.Write(b)
-	}
-	// Catch trailing output in case stream does not end with a newline
-	if line.Len() > 0 {
-		logger("%s%s", prefix, line.String())
-	}
-	return nil
-}
-
 // ReplaceChars returns a copy of the src slice with each string modified by the replacer
 func ReplaceChars(src []string, replacer *strings.Replacer) []string {
 	ret := make([]string, len(src))

pkg/util/utils_test.go

@@ -17,10 +17,7 @@ limitations under the License.
 package util
 
 import (
-	"bytes"
-	"fmt"
 	"strings"
-	"sync"
 	"testing"
 )
 
@@ -44,44 +41,6 @@ func TestGetBinaryDownloadURL(t *testing.T) {
 
 }
 
-func TestTeePrefix(t *testing.T) {
-	var in bytes.Buffer
-	var out bytes.Buffer
-	var logged strings.Builder
-
-	logSink := func(format string, args ...interface{}) {
-		logged.WriteString("(" + fmt.Sprintf(format, args...) + ")")
-	}
-
-	// Simulate the primary use case: tee in the background. This also helps avoid I/O races.
-	var wg sync.WaitGroup
-	wg.Add(1)
-	go func() {
-		if err := TeePrefix(":", &in, &out, logSink); err != nil {
-			t.Errorf("TeePrefix: %v", err)
-		}
-		wg.Done()
-	}()
-
-	in.Write([]byte("goo"))
-	in.Write([]byte("\n"))
-	in.Write([]byte("g\r\n\r\n"))
-	in.Write([]byte("le"))
-	wg.Wait()
-
-	gotBytes := out.Bytes()
-	wantBytes := []byte("goo\ng\r\n\r\nle")
-	if !bytes.Equal(gotBytes, wantBytes) {
-		t.Errorf("output=%q, want: %q", gotBytes, wantBytes)
-	}
-
-	gotLog := logged.String()
-	wantLog := "(:goo)(:g)(:le)"
-	if gotLog != wantLog {
-		t.Errorf("log=%q, want: %q", gotLog, wantLog)
-	}
-}
-
 func TestReplaceChars(t *testing.T) {
 	testData := []struct {
 		src         []string