Skip to content

Commit ef39833

Browse files
tstrombergtstromberg
authored
Merge pull request #5530 from medyagh/NewCmdRunner
Refactor command runner interface, allow stdin writes
2 parents 646180b + 77ccd46 commit ef39833

26 files changed

+637
-447
lines changed

‎Makefile

+1-1
Original file line numberDiff line numberDiff line change
@@ -572,4 +572,4 @@ site: site/themes/docsy/assets/vendor/bootstrap/package.js out/hugo/hugo
572572

573573
.PHONY: out/mkcmp
574574
out/mkcmp:
575-
GOOS=$(GOOS) GOARCH=$(GOARCH) go build -o $@ cmd/performance/main.go
575+
GOOS=$(GOOS) GOARCH=$(GOARCH) go build -o $@ cmd/performance/main.go

‎cmd/minikube/cmd/start.go

+7-5
Original file line numberDiff line numberDiff line change
@@ -1029,27 +1029,29 @@ Suggested workarounds:
10291029
defer conn.Close()
10301030
}
10311031

1032-
if err := r.Run("nslookup kubernetes.io"); err != nil {
1032+
if _, err := r.RunCmd(exec.Command("nslookup", "kubernetes.io")); err != nil {
10331033
out.WarningT("VM is unable to resolve DNS hosts: {[.error}}", out.V{"error": err})
10341034
}
10351035

10361036
// Try both UDP and ICMP to assert basic external connectivity
1037-
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 {
1037+
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 {
10381038
out.WarningT("VM is unable to directly connect to the internet: {{.error}}", out.V{"error": err})
10391039
}
10401040

10411041
// Try an HTTPS connection to the
10421042
proxy := os.Getenv("HTTPS_PROXY")
1043-
opts := "-sS"
1043+
opts := []string{"-sS"}
10441044
if proxy != "" && !strings.HasPrefix(proxy, "localhost") && !strings.HasPrefix(proxy, "127.0") {
1045-
opts = fmt.Sprintf("-x %s %s", proxy, opts)
1045+
opts = append([]string{"-x", proxy}, opts...)
10461046
}
10471047

10481048
repo := viper.GetString(imageRepository)
10491049
if repo == "" {
10501050
repo = images.DefaultImageRepo
10511051
}
1052-
if err := r.Run(fmt.Sprintf("curl %s https://%s/", opts, repo)); err != nil {
1052+
1053+
opts = append(opts, fmt.Sprintf("https://%s/", repo))
1054+
if _, err := r.RunCmd(exec.Command("curl", opts...)); err != nil {
10531055
out.WarningT("VM is unable to connect to the selected image repository: {{.error}}", out.V{"error": err})
10541056
}
10551057
return ip

‎go.mod

+1
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ require (
4343
github.com/juju/testing v0.0.0-20190723135506-ce30eb24acd2 // indirect
4444
github.com/juju/utils v0.0.0-20180820210520-bf9cc5bdd62d // indirect
4545
github.com/juju/version v0.0.0-20180108022336-b64dbd566305 // indirect
46+
github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51
4647
github.com/libvirt/libvirt-go v3.4.0+incompatible
4748
github.com/machine-drivers/docker-machine-driver-vmware v0.1.1
4849
github.com/mattn/go-isatty v0.0.8

‎go.sum

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

‎pkg/drivers/none/none.go

+25-19
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ limitations under the License.
1717
package none
1818

1919
import (
20-
"bytes"
2120
"fmt"
21+
"os/exec"
2222
"strings"
2323
"time"
2424

@@ -168,8 +168,8 @@ func (d *Driver) Remove() error {
168168
return errors.Wrap(err, "kill")
169169
}
170170
glog.Infof("Removing: %s", cleanupPaths)
171-
cmd := fmt.Sprintf("sudo rm -rf %s", strings.Join(cleanupPaths, " "))
172-
if err := d.exec.Run(cmd); err != nil {
171+
args := append([]string{"rm", "-rf"}, cleanupPaths...)
172+
if _, err := d.exec.RunCmd(exec.Command("sudo", args...)); err != nil {
173173
glog.Errorf("cleanup incomplete: %v", err)
174174
}
175175
return nil
@@ -217,22 +217,20 @@ func (d *Driver) RunSSHCommandFromDriver() error {
217217
}
218218

219219
// stopKubelet idempotently stops the kubelet
220-
func stopKubelet(exec command.Runner) error {
220+
func stopKubelet(cr command.Runner) error {
221221
glog.Infof("stopping kubelet.service ...")
222222
stop := func() error {
223-
cmdStop := "sudo systemctl stop kubelet.service"
224-
cmdCheck := "sudo systemctl show -p SubState kubelet"
225-
err := exec.Run(cmdStop)
226-
if err != nil {
227-
glog.Errorf("temporary error for %q : %v", cmdStop, err)
223+
cmd := exec.Command("sudo", "systemctl", "stop", "kubelet.service")
224+
if rr, err := cr.RunCmd(cmd); err != nil {
225+
glog.Errorf("temporary error for %q : %v", rr.Command(), err)
228226
}
229-
var out bytes.Buffer
230-
errStatus := exec.CombinedOutputTo(cmdCheck, &out)
231-
if errStatus != nil {
232-
glog.Errorf("temporary error: for %q : %v", cmdCheck, errStatus)
227+
cmd = exec.Command("sudo", "systemctl", "show", "-p", "SubState", "kubelet")
228+
rr, err := cr.RunCmd(cmd)
229+
if err != nil {
230+
glog.Errorf("temporary error: for %q : %v", rr.Command(), err)
233231
}
234-
if !strings.Contains(out.String(), "dead") && !strings.Contains(out.String(), "failed") {
235-
return fmt.Errorf("unexpected kubelet state: %q", out)
232+
if !strings.Contains(rr.Stdout.String(), "dead") && !strings.Contains(rr.Stdout.String(), "failed") {
233+
return fmt.Errorf("unexpected kubelet state: %q", rr.Stdout.String())
236234
}
237235
return nil
238236
}
@@ -245,13 +243,21 @@ func stopKubelet(exec command.Runner) error {
245243
}
246244

247245
// restartKubelet restarts the kubelet
248-
func restartKubelet(exec command.Runner) error {
246+
func restartKubelet(cr command.Runner) error {
249247
glog.Infof("restarting kubelet.service ...")
250-
return exec.Run("sudo systemctl restart kubelet.service")
248+
c := exec.Command("sudo", "systemctl", "restart", "kubelet.service")
249+
if _, err := cr.RunCmd(c); err != nil {
250+
return err
251+
}
252+
return nil
251253
}
252254

253255
// checkKubelet returns an error if the kubelet is not running.
254-
func checkKubelet(exec command.Runner) error {
256+
func checkKubelet(cr command.Runner) error {
255257
glog.Infof("checking for running kubelet ...")
256-
return exec.Run("systemctl is-active --quiet service kubelet")
258+
c := exec.Command("systemctl", "is-active", "--quiet", "service", "kubelet")
259+
if _, err := cr.RunCmd(c); err != nil {
260+
return errors.Wrap(err, "check kubelet")
261+
}
262+
return nil
257263
}

‎pkg/minikube/bootstrapper/certs.go

+19-17
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"io/ioutil"
2323
"net"
2424
"os"
25+
"os/exec"
2526
"path"
2627
"path/filepath"
2728
"strings"
@@ -141,7 +142,7 @@ func SetupCerts(cmd command.Runner, k8s config.KubernetesConfig) error {
141142

142143
// configure CA certificates
143144
if err := configureCACerts(cmd, caCerts); err != nil {
144-
return errors.Wrapf(err, "error configuring CA certificates during provisioning %v", err)
145+
return errors.Wrapf(err, "Configuring CA certs")
145146
}
146147
return nil
147148
}
@@ -318,21 +319,21 @@ func collectCACerts() (map[string]string, error) {
318319
}
319320

320321
// getSubjectHash calculates Certificate Subject Hash for creating certificate symlinks
321-
func getSubjectHash(cmd command.Runner, filePath string) (string, error) {
322-
out, err := cmd.CombinedOutput(fmt.Sprintf("openssl x509 -hash -noout -in '%s'", filePath))
322+
func getSubjectHash(cr command.Runner, filePath string) (string, error) {
323+
rr, err := cr.RunCmd(exec.Command("openssl", "x509", "-hash", "-noout", "-in", filePath))
323324
if err != nil {
324-
return "", err
325+
return "", errors.Wrapf(err, rr.Command())
325326
}
326-
327-
stringHash := strings.TrimSpace(out)
327+
stringHash := strings.TrimSpace(rr.Stdout.String())
328328
return stringHash, nil
329329
}
330330

331331
// configureCACerts looks up and installs all uploaded PEM certificates in /usr/share/ca-certificates to system-wide certificate store (/etc/ssl/certs).
332332
// OpenSSL binary required in minikube ISO
333-
func configureCACerts(cmd command.Runner, caCerts map[string]string) error {
333+
func configureCACerts(cr command.Runner, caCerts map[string]string) error {
334334
hasSSLBinary := true
335-
if err := cmd.Run("which openssl"); err != nil {
335+
_, err := cr.RunCmd(exec.Command("openssl", "version"))
336+
if err != nil {
336337
hasSSLBinary = false
337338
}
338339

@@ -343,24 +344,25 @@ func configureCACerts(cmd command.Runner, caCerts map[string]string) error {
343344
for _, caCertFile := range caCerts {
344345
dstFilename := path.Base(caCertFile)
345346
certStorePath := path.Join(SSLCertStoreDir, dstFilename)
346-
if err := cmd.Run(fmt.Sprintf("sudo test -f '%s'", certStorePath)); err != nil {
347-
if err := cmd.Run(fmt.Sprintf("sudo ln -s '%s' '%s'", caCertFile, certStorePath)); err != nil {
348-
return errors.Wrapf(err, "error making symbol link for certificate %s", caCertFile)
347+
_, err := cr.RunCmd(exec.Command("sudo", "test", "-f", certStorePath))
348+
if err != nil {
349+
if _, err := cr.RunCmd(exec.Command("sudo", "ln", "-s", caCertFile, certStorePath)); err != nil {
350+
return errors.Wrapf(err, "create symlink for %s", caCertFile)
349351
}
350352
}
351353
if hasSSLBinary {
352-
subjectHash, err := getSubjectHash(cmd, caCertFile)
354+
subjectHash, err := getSubjectHash(cr, caCertFile)
353355
if err != nil {
354-
return errors.Wrapf(err, "error calculating subject hash for certificate %s", caCertFile)
356+
return errors.Wrapf(err, "calculate hash for cacert %s", caCertFile)
355357
}
356358
subjectHashLink := path.Join(SSLCertStoreDir, fmt.Sprintf("%s.0", subjectHash))
357-
if err := cmd.Run(fmt.Sprintf("sudo test -f '%s'", subjectHashLink)); err != nil {
358-
if err := cmd.Run(fmt.Sprintf("sudo ln -s '%s' '%s'", certStorePath, subjectHashLink)); err != nil {
359-
return errors.Wrapf(err, "error making subject hash symbol link for certificate %s", caCertFile)
359+
_, err = cr.RunCmd(exec.Command("sudo", "test", "-f", subjectHashLink))
360+
if err != nil {
361+
if _, err := cr.RunCmd(exec.Command("sudo", "ln", "-s", certStorePath, subjectHashLink)); err != nil {
362+
return errors.Wrapf(err, "linking caCertFile %s", caCertFile)
360363
}
361364
}
362365
}
363366
}
364-
365367
return nil
366368
}

‎pkg/minikube/bootstrapper/certs_test.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,9 @@ func TestSetupCerts(t *testing.T) {
6262
certStorePath := path.Join(SSLCertStoreDir, dst)
6363
certNameHash := "abcdef"
6464
remoteCertHashLink := path.Join(SSLCertStoreDir, fmt.Sprintf("%s.0", certNameHash))
65-
cmdMap[fmt.Sprintf("sudo ln -s '%s' '%s'", certFile, certStorePath)] = "1"
66-
cmdMap[fmt.Sprintf("openssl x509 -hash -noout -in '%s'", certFile)] = certNameHash
67-
cmdMap[fmt.Sprintf("sudo ln -s '%s' '%s'", certStorePath, remoteCertHashLink)] = "1"
65+
cmdMap[fmt.Sprintf("sudo ln -s %s %s", certFile, certStorePath)] = "1"
66+
cmdMap[fmt.Sprintf("openssl x509 -hash -noout -in %s", certFile)] = certNameHash
67+
cmdMap[fmt.Sprintf("sudo ln -s %s %s", certStorePath, remoteCertHashLink)] = "1"
6868
}
6969
f := command.NewFakeCommandRunner()
7070
f.SetCommandToOutput(cmdMap)

0 commit comments

Comments
 (0)