diff --git a/Makefile b/Makefile index 6f8b80da71f572fff38a81e3af36b9e9b4ce3b2c..15e42c3e66fcbaaea44f84277ef6b9f1f6190bdd 100755 --- a/Makefile +++ b/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 diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go index 977803d8ab5b9b4a88aa656b045cf7289172568c..3e67510e1787a81c2f61474b3b1d3690396da265 100644 --- a/cmd/minikube/cmd/start.go +++ b/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 diff --git a/go.mod b/go.mod index ebb2c83a497045f913b1443ea6f72796d83335ac..7012e91cb32d7a07290997485a55fb94186ff106 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index 6ebce314fd7fa69b3334cda5e1a4ccec1d05d212..2567a47dfa072ee9f9b887a75a0aefca2440166c 100644 --- a/go.sum +++ b/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= diff --git a/pkg/drivers/none/none.go b/pkg/drivers/none/none.go index 1a8687fcd84d6cbbdb9a8d5ba8c7dc28c1d97406..4d99fd8b8ed1537531b0a14cf602b02c52747181 100644 --- a/pkg/drivers/none/none.go +++ b/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 } diff --git a/pkg/minikube/bootstrapper/certs.go b/pkg/minikube/bootstrapper/certs.go index e5d2edd18ca6570bfc2d0f65863d75896e32151c..6702a6b1220feac629e2c4341763e58952361b61 100644 --- a/pkg/minikube/bootstrapper/certs.go +++ b/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 } diff --git a/pkg/minikube/bootstrapper/certs_test.go b/pkg/minikube/bootstrapper/certs_test.go index e6a25f484e4da529b93658c77cf8076746f9581b..7d65e6c3af382ae93493f63c412833afa6829cec 100644 --- a/pkg/minikube/bootstrapper/certs_test.go +++ b/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) diff --git a/pkg/minikube/bootstrapper/kubeadm/kubeadm.go b/pkg/minikube/bootstrapper/kubeadm/kubeadm.go index a81f9aa21abc4000454b9dfe3a1668144d628a82..94b4c3ddda71fe34a99c7281b89f8c8969736e21 100644 --- a/pkg/minikube/bootstrapper/kubeadm/kubeadm.go +++ b/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 diff --git a/pkg/minikube/cluster/mount.go b/pkg/minikube/cluster/mount.go index 59e3af10558ed3b5adc9e740ebed047843b627f1..eeba0a7452547c7d8c05ad071679db6b319afc4a 100644 --- a/pkg/minikube/cluster/mount.go +++ b/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 } diff --git a/pkg/minikube/cluster/mount_test.go b/pkg/minikube/cluster/mount_test.go index db56f96faa6f7f37baf06263688200c6a0e8abdc..bb1890069c248a18fcb17f28493a4db6a27e2a78 100644 --- a/pkg/minikube/cluster/mount_test.go +++ b/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) - } -} diff --git a/pkg/minikube/command/command_runner.go b/pkg/minikube/command/command_runner.go index 57f250b0cab356b0dc189df788f49e52f54c91cf..309c8f6133a2982990f39b81eed06b8f38faf572 100644 --- a/pkg/minikube/command/command_runner.go +++ b/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() +} diff --git a/pkg/minikube/command/exec_runner.go b/pkg/minikube/command/exec_runner.go index 84891597e74cd87cb5bd6725ceb983c5c819fec5..a10023b3a82e50469b58459f241cbee37b1488d0 100644 --- a/pkg/minikube/command/exec_runner.go +++ b/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 diff --git a/pkg/minikube/command/fake_runner.go b/pkg/minikube/command/fake_runner.go index 029000cdc80bbb0d627d81fd63c4f13eff6e787f..fbdd40ca4007221fd287759d9f8329248e6d52ee 100644 --- a/pkg/minikube/command/fake_runner.go +++ b/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. diff --git a/pkg/minikube/command/ssh_runner.go b/pkg/minikube/command/ssh_runner.go index 01fd780e970a514ce6da59f2c33d45548fec5f31..a341afb0498c63560b40f86c95cbd4a52466d28f 100644 --- a/pkg/minikube/command/ssh_runner.go +++ b/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 +} diff --git a/pkg/minikube/command/ssh_runner_test.go b/pkg/minikube/command/ssh_runner_test.go new file mode 100644 index 0000000000000000000000000000000000000000..df9e7d509e60ec49cfb63c7bac2d958bf0ab5545 --- /dev/null +++ b/pkg/minikube/command/ssh_runner_test.go @@ -0,0 +1,63 @@ +/* +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) + } +} diff --git a/pkg/minikube/cruntime/containerd.go b/pkg/minikube/cruntime/containerd.go index a39298edf526d70f50f4d52c589adc257ac84406..934f65c416bac91d56653fbc0bd40b6487b2c8ca 100644 --- a/pkg/minikube/cruntime/containerd.go +++ b/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 diff --git a/pkg/minikube/cruntime/cri.go b/pkg/minikube/cruntime/cri.go index d3c8fd6e1302f836205df7ec17930317c49edde2..173e6523011f3e4d8341cb0605866c3104f073c7 100644 --- a/pkg/minikube/cruntime/cri.go +++ b/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 diff --git a/pkg/minikube/cruntime/crio.go b/pkg/minikube/cruntime/crio.go index cecefa3c8f1a2b0f978235b92a4b5cbc0df3a35e..763e52ac7395edd93f2d940d87ecec2fb87cf6e1 100644 --- a/pkg/minikube/cruntime/crio.go +++ b/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. diff --git a/pkg/minikube/cruntime/cruntime.go b/pkg/minikube/cruntime/cruntime.go index a7a17e9ffd460ea888eea617e3dd8c9e868c4295..6566a8d8cc023b8ad43347aff8013b45c2ed6ef6 100644 --- a/pkg/minikube/cruntime/cruntime.go +++ b/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 } diff --git a/pkg/minikube/cruntime/cruntime_test.go b/pkg/minikube/cruntime/cruntime_test.go index a43926912118efcad61d2042aa9aafa88f63099c..aa28c8a9dde2546035d5d7802c60130671ad04d7 100644 --- a/pkg/minikube/cruntime/cruntime_test.go +++ b/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) { diff --git a/pkg/minikube/cruntime/docker.go b/pkg/minikube/cruntime/docker.go index 817049aff132d172c093feb80d688eae823fe7e2..3d80fca4595bf94f0b89e076706a4efa369ee850 100644 --- a/pkg/minikube/cruntime/docker.go +++ b/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 diff --git a/pkg/minikube/logs/logs.go b/pkg/minikube/logs/logs.go index 0c6625f8b4c709e8670961ecbd9d3441d4112bf9..f56230261900c8002e987ff1fffad2a6046e0a89 100644 --- a/pkg/minikube/logs/logs.go +++ b/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 } diff --git a/pkg/minikube/problem/err_map.go b/pkg/minikube/problem/err_map.go index f2bb49c5e9416f6839e686648e28b4b7db24bcbd..bad986cdcf0d827ff85b057c8dcff0b315c3f7b2 100644 --- a/pkg/minikube/problem/err_map.go +++ b/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, diff --git a/pkg/minikube/tunnel/route_darwin.go b/pkg/minikube/tunnel/route_darwin.go index 815eff6bfe4a2754dc350452c8cbccdeae088108..0e2f5064ff28ce5ec54f41d4b4464f6f0a24c729 100644 --- a/pkg/minikube/tunnel/route_darwin.go +++ b/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 diff --git a/pkg/util/utils.go b/pkg/util/utils.go index ec2d8f6faf464f08a8cbd21e2a252d42f141cd99..54ec5d410139d9bf87d2bd41e0e9f12c7d5ff654 100644 --- a/pkg/util/utils.go +++ b/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)) diff --git a/pkg/util/utils_test.go b/pkg/util/utils_test.go index 4fc94ce7bc59459b06033884abef9e91b3939edb..26a10724a482906160a2260c37da4d45a5738b1b 100644 --- a/pkg/util/utils_test.go +++ b/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