veyron/services/mgmt/node/impl: wait for SetNamespaceRootsCommand to finish.
This fixes a race between running the modules function setting the local
namespace's roots (which runs in a goroutine), and using the local namespace to
mount services like the mock app repository. If the phase of the moon was right,
the app repository would try to publish itself before the roots were set, and
was therefore attempting to use the roots that had been set by a previous test
case's shell. Exhacerbating the issue was the fact that the attempt at resetting
of the roots to "" that was supposedly happening at the end of the deferred
cleanup function was actually a no-op: Namespace.SetRoots() rejects empty
strings as invalid roots, so sh.Start(core.SetNamespaceRootsCommand, nil, "")
would be failing silently.
This CL makes three changes:
(1) wait for the SetNamespaceRootsCommand command to complete before proceeding
(2) instead of sh.Start(core.SetNamespaceRootsCommand, nil, ""), try to reset
the roots to what they were before
(3) no longer expect at least one arg in SetNamespaceRootsCommand (so passing no
args should result in the roots being set to empty set).
Should fix go/vissues/408.
Change-Id: I3f5ef93bd62ea062d02074863ea0df6fa1768398
diff --git a/lib/modules/core/mounttable.go b/lib/modules/core/mounttable.go
index 51373a6..b314fed 100644
--- a/lib/modules/core/mounttable.go
+++ b/lib/modules/core/mounttable.go
@@ -148,9 +148,5 @@
}
func setNamespaceRoots(stdin io.Reader, stdout, stderr io.Writer, env map[string]string, args ...string) error {
- ns := rt.R().Namespace()
- if err := checkArgs(args, -1, "<name>..."); err != nil {
- return err
- }
- return ns.SetRoots(args[1:]...)
+ return rt.R().Namespace().SetRoots(args[1:]...)
}
diff --git a/services/mgmt/node/impl/util_test.go b/services/mgmt/node/impl/util_test.go
index 0db0633..74f8871 100644
--- a/services/mgmt/node/impl/util_test.go
+++ b/services/mgmt/node/impl/util_test.go
@@ -59,6 +59,18 @@
return creds, []string{consts.VeyronCredentials + "=" + creds}
}
+// setNSRoots sets the roots for the local runtime's namespace using the modules
+// function SetNamesaceRootCommand.
+func setNSRoots(t *testing.T, sh *modules.Shell, roots ...string) {
+ setRootsHandle, err := sh.Start(core.SetNamespaceRootsCommand, nil, roots...)
+ if err != nil {
+ t.Fatalf("%s: unexpected error: %s", loc(2), err)
+ }
+ if err := setRootsHandle.Shutdown(nil, os.Stderr); err != nil {
+ t.Fatalf("%s: SetNamespaceRootsCommand failed with %v", loc(2), err)
+ }
+}
+
func createShellAndMountTable(t *testing.T) (*modules.Shell, func()) {
sh := modules.NewShell()
// The shell, will, by default share credentials with its children.
@@ -70,17 +82,18 @@
// their shutdown process
sh.Forget(mtHandle)
+ // TODO(caprita): Define a GetNamespaceRootsCommand in modules/core and
+ // use that?
+ oldNamespaceRoots := rt.R().Namespace().Roots()
fn := func() {
vlog.VI(1).Info("------------ CLEANUP ------------")
vlog.VI(1).Info("---------------------------------")
sh.Cleanup(nil, os.Stderr)
mtHandle.Shutdown(nil, os.Stderr)
- sh.Start(core.SetNamespaceRootsCommand, nil, "")
+ setNSRoots(t, sh, oldNamespaceRoots...)
}
- if _, err := sh.Start(core.SetNamespaceRootsCommand, nil, mtName); err != nil {
- t.Fatalf("%s: unexpected error: %s", loc(1), err)
- }
+ setNSRoots(t, sh, mtName)
sh.SetVar(consts.NamespaceRootPrefix, mtName)
return sh, fn
}
