Code review comment for lp:~axwalk/juju-core/lp1235716-sshstorage-sudo-prompt

Revision history for this message
Andrew Wilkins (axwalk) wrote :

Reviewers: mp+189526_code.launchpad.net,

Message:
Please take a look.

Description:
environs/sshstorage: don't hide sudo prompt

Connect sudo command's stdout to os.Stdout,
and capture stderr in a bytes.Buffer. This
allows sudo prompts to be presented, while
still capturing "install" errors.

Fixes #1235716

https://code.launchpad.net/~axwalk/juju-core/lp1235716-sshstorage-sudo-prompt/+merge/189526

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/14461044/

Affected files (+8, -3 lines):
   A [revision details]
   M environs/sshstorage/storage.go
   M environs/sshstorage/storage_test.go

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: tarmac-20131004160159-i9eoormyw24etfr4
+New revision: <email address hidden>

Index: environs/sshstorage/storage.go
=== modified file 'environs/sshstorage/storage.go'
--- environs/sshstorage/storage.go 2013-10-03 08:34:41 +0000
+++ environs/sshstorage/storage.go 2013-10-07 03:40:52 +0000
@@ -84,8 +84,11 @@

   cmd := sshCommand(host, true, fmt.Sprintf("sudo bash -c %s",
utils.ShQuote(script)))
   cmd.Stdin = os.Stdin
- if out, err := cmd.CombinedOutput(); err != nil {
- err = fmt.Errorf("failed to create storage dir: %v (%v)", err,
strings.TrimSpace(string(out)))
+ cmd.Stdout = os.Stdout // for sudo prompts/output
+ var stderr bytes.Buffer
+ cmd.Stderr = &stderr
+ if err := cmd.Run(); err != nil {
+ err = fmt.Errorf("failed to create storage dir: %v (%v)", err,
strings.TrimSpace(stderr.String()))
    return nil, err
   }

Index: environs/sshstorage/storage_test.go
=== modified file 'environs/sshstorage/storage_test.go'
--- environs/sshstorage/storage_test.go 2013-10-03 08:44:30 +0000
+++ environs/sshstorage/storage_test.go 2013-10-07 03:40:52 +0000
@@ -162,7 +162,7 @@
    invocations++
    switch invocations {
    case 1, 3:
- return exec.Command("bash", "-c", "echo alles gut")
+ return exec.Command("true")
    case 2:
     // Note: must close stdin before responding the first time, or
     // the second command will race with closing stdin, and may

« Back to merge proposal