resolve code review

pkg/drivers/none/none.go

@@ -229,10 +229,11 @@ func stopKubelet(cr command.Runner) error {
 		cmd = exec.Command("sudo", "systemctl", "show", "-p", "SubState", "kubelet")
 		cmd.Stdout = &out
 		cmd.Stderr = &out
-		if rr, err := cr.RunCmd(cmd); err != nil {
+		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") {
+		if !strings.Contains(rr.Stdout.String(), "dead") && !strings.Contains(rr.Stdout.String(), "failed") {
 			return fmt.Errorf("unexpected kubelet state: %q", out)
 		}
 		return nil
@@ -260,7 +261,7 @@ func checkKubelet(cr command.Runner) error {
 	glog.Infof("checking for running kubelet ...")
 	c := exec.Command("systemctl", "is-active", "--quiet", "service", "kubelet")
 	if _, err := cr.RunCmd(c); err != nil {
-		return errors.Wrap(err, "checkKubelet")
+		return errors.Wrap(err, "check kubelet")
 	}
 	return nil
 }

pkg/minikube/bootstrapper/certs.go

@@ -347,19 +347,19 @@ func configureCACerts(cr command.Runner, caCerts map[string]string) error {
 		_, 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, "error making symbol link for certificate %s", caCertFile)
+				return errors.Wrapf(err, "create symlink for %s", caCertFile)
 			}
 		}
 		if hasSSLBinary {
 			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))
 			_, 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 errors.Wrapf(err, "linking caCertFile %s", caCertFile)
 				}
 			}
 		}

pkg/minikube/bootstrapper/kubeadm/kubeadm.go

@@ -642,8 +642,8 @@ func (k *Bootstrapper) UpdateCluster(cfg config.KubernetesConfig) error {
 		}
 	}
 
-	if _, err := k.c.RunCmd(exec.Command("/bin/bash", "-c", "set -x;sudo systemctl daemon-reload && sudo systemctl start kubelet")); err != nil {
-		return errors.Wrap(err, "starting kubelet.")
+	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/command/command_runner.go

@@ -36,8 +36,8 @@ type RunResult struct {
 
 // Runner represents an interface to run commands.
 type Runner interface {
-	// RunCmd is a new expermintal way to run commands, takes Cmd interface and returns run result.
-	// if succesfull will cause a clean up to get rid of older methods.
+	// 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

pkg/minikube/command/exec_runner.go

@@ -18,7 +18,6 @@ package command
 
 import (
 	"bytes"
-	"fmt"
 	"io"
 	"os"
 	"os/exec"
@@ -73,7 +72,7 @@ func (*ExecRunner) RunCmd(cmd *exec.Cmd) (*RunResult, error) {
 			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, fmt.Sprintf("stderr: %s", rr.Stderr.String()))
+		err = errors.Wrapf(err, "command failed: %s\nstdout: %s\nstderr: %s", cmd, rr.Stdout.String(), rr.Stderr.String())
 	}
 	return rr, err
 }

pkg/minikube/cruntime/containerd.go

@@ -161,7 +161,7 @@ func (r *Containerd) Active() bool {
 
 // Available returns an error if it is not possible to use this runtime on a host
 func (r *Containerd) Available() error {
-	c := exec.Command("command", "-v", "containerd")
+	c := exec.Command("which", "containerd")
 	if _, err := r.Runner.RunCmd(c); err != nil {
 		return errors.Wrap(err, "check containerd availability.")
 	}
@@ -207,7 +207,7 @@ func (r *Containerd) Enable(disOthers bool) error {
 	// Otherwise, containerd will fail API requests with 'Unimplemented'
 	c := exec.Command("sudo", "systemctl", "restart", "containerd")
 	if _, err := r.Runner.RunCmd(c); err != nil {
-		return errors.Wrap(err, "enable containrd.")
+		return errors.Wrap(err, "restart containerd")
 	}
 	return nil
 }
@@ -216,7 +216,7 @@ func (r *Containerd) Enable(disOthers bool) error {
 func (r *Containerd) Disable() error {
 	c := exec.Command("sudo", "systemctl", "stop", "containerd")
 	if _, err := r.Runner.RunCmd(c); err != nil {
-		return errors.Wrapf(err, "disable containrd.")
+		return errors.Wrapf(err, "stop containerd")
 	}
 	return nil
 }
@@ -224,9 +224,9 @@ func (r *Containerd) Disable() error {
 // LoadImage loads an image into this runtime
 func (r *Containerd) LoadImage(path string) error {
 	glog.Infof("Loading image: %s", path)
-	c := exec.Command("ctr", "-n=k8s.io", "images", "import", path)
+	c := exec.Command("sudo", "ctr", "-n=k8s.io", "images", "import", path)
 	if _, err := r.Runner.RunCmd(c); err != nil {
-		return errors.Wrapf(err, "disable containrd.")
+		return errors.Wrapf(err, "ctr images import")
 	}
 	return nil
 }

pkg/minikube/cruntime/crio.go

@@ -73,7 +73,7 @@ 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 {
-	c := exec.Command("command", "-v", "crio")
+	c := exec.Command("which", "crio")
 	if _, err := r.Runner.RunCmd(c); err != nil {
 		return errors.Wrapf(err, "check crio available.")
 	}
@@ -124,7 +124,7 @@ func (r *CRIO) LoadImage(path string) error {
 	glog.Infof("Loading image: %s", path)
 	c := exec.Command("sudo", "podman", "load", "-i", path)
 	if _, err := r.Runner.RunCmd(c); err != nil {
-		return errors.Wrap(err, "LoadImage crio.")
+		return errors.Wrap(err, "crio load image")
 	}
 	return nil
 }

pkg/minikube/cruntime/cruntime.go

@@ -133,12 +133,12 @@ func disableOthers(me Manager, cr CommandRunner) error {
 func enableIPForwarding(cr CommandRunner) error {
 	c := exec.Command("sudo", "modprobe", "br_netfilter")
 	if _, err := cr.RunCmd(c); err != nil {
-		return errors.Wrap(err, "br_netfilter.")
+		return errors.Wrap(err, "br_netfilter")
 	}
 
 	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 errors.Wrapf(err, "ip_forward")
 	}
 	return nil
 }