veyron/services/mgmt/node/impl: fix issue with closing stdin for subcommand
Context: https://code.google.com/p/envyor/issues/detail?id=192
The root cause was a copy/paste bug: I was calling nm.CloseStdin() instead of
runNM.CloseStdin(); this was essentially a no-op. The test was still passing
since the logic inside execScript that was trying to capture a stdin closure
and pass it along to the subcommand was also broken -- the stdin of the command
was not set, and hence the subcommand's stdin started out as closed. Hence,
the call to blackbox.WaitForEOFOnStdin() was a no-op for subcommands started
via execScript.
After fixing the nm/runNM.CloseStdin() bug, and setting stdin correctly in the
subcommand started by execScript, a bunch of code can be removed and things
actually work as designed.
Change-Id: Ib6e22b73f94d3f8e8d1db84e7cd8ea3c21463864
diff --git a/services/mgmt/node/impl/impl_test.go b/services/mgmt/node/impl/impl_test.go
index 3a0c0fc..0ac00ee 100644
--- a/services/mgmt/node/impl/impl_test.go
+++ b/services/mgmt/node/impl/impl_test.go
@@ -9,7 +9,6 @@
goexec "os/exec"
"path/filepath"
"strings"
- "sync"
"testing"
"veyron/lib/signals"
@@ -55,23 +54,10 @@
cmd := goexec.Cmd{
Path: script,
Env: env,
+ Stdin: os.Stdin,
Stderr: os.Stderr,
Stdout: os.Stdout,
}
- // os.exec.Cmd is not thread safe
- var cmdLock sync.Mutex
- go func() {
- cmdLock.Lock()
- stdin, err := cmd.StdinPipe()
- cmdLock.Unlock()
- if err != nil {
- vlog.Fatalf("Failed to get stdin pipe: %v", err)
- }
- blackbox.WaitForEOFOnStdin()
- stdin.Close()
- }()
- cmdLock.Lock()
- defer cmdLock.Unlock()
if err := cmd.Run(); err != nil {
vlog.Fatalf("Run cmd %v failed: %v", cmd, err)
}
@@ -366,7 +352,7 @@
// Revert the node manager to its previous version (v2).
revert(t, "v3NM")
revertExpectError(t, "v3NM", verror.Exists) // Revert already in progress.
- nm.CloseStdin()
+ runNM.CloseStdin()
runNM.Expect("v3NM terminating")
if evalLink() != scriptPathV2 {
t.Fatalf("current link was not reverted correctly")
