veyron2/naming: prevent annoying last-step RPC for name resolution.
The client MountTable code resolves names iteratively until it either
encounters a terminal name or an error. This CL changes the behaviour
of Publish to always publish an address that will be considered terminal
when resolved. So: ep, _ := s.Listen(...); s.Publish("foo/bar") will
result a Mount on foo/bar with an address of ep.String() + "//". This
ensures that when foo/bar is resolved in the MountTable and
ep.String() + "//" is returned then the client knows to stop resolving
and thus can avoid an extra RPC call that is doomed to fail. This
is the desired behaviour for all servers except MountTables. MountTables
must therefore be handled specially. MountTable servers (note, only
servers) must be started on ipc.Server that is passed the option
veryon2.ServesMountTables(true) which instructs the ipc.Server
implementation to not add the "//" suffix.
naming.MountTable now exposes a 'SetRoots' method to allow the
root MountTable servers for the in-process MountTable client to be
configured dynamically.
Whilst working on this CL I cam across lots of test and example code
that is hand-manipulating the name space rather than using Publish
which is likely to be error prone. We should clean this all up at
some point.
Change-Id: I1d7aa3841b616af0917ff1299ff5deffc85fb34d
diff --git a/examples/rockpaperscissors/impl/impl_test.go b/examples/rockpaperscissors/impl/impl_test.go
index 63c67f5..883b6f8 100644
--- a/examples/rockpaperscissors/impl/impl_test.go
+++ b/examples/rockpaperscissors/impl/impl_test.go
@@ -16,7 +16,7 @@
)
func startMountTable(t *testing.T, runtime veyron2.Runtime) (string, func()) {
- server, err := runtime.NewServer()
+ server, err := runtime.NewServer(veyron2.ServesMountTableOpt(true))
if err != nil {
t.Fatalf("NewServer() failed: %v", err)
}
diff --git a/examples/unresolve/test_util.go b/examples/unresolve/test_util.go
index 7805910..09d877c 100644
--- a/examples/unresolve/test_util.go
+++ b/examples/unresolve/test_util.go
@@ -22,8 +22,8 @@
return rt.Init(opts...).Shutdown
}
-func newServer() ipc.Server {
- server, err := rt.R().NewServer()
+func newServer(opts ...ipc.ServerOpt) ipc.Server {
+ server, err := rt.R().NewServer(opts...)
if err != nil {
panic(fmt.Sprintf("r.NewServer failed with %v", err))
}
@@ -41,9 +41,9 @@
return naming.JoinAddressName(ep.String(), prefix)
}
-func serverMain(serviceCreator func(ipc.Server) string, args []string) {
+func serverMain(servesMT bool, serviceCreator func(ipc.Server) string, args []string) {
defer initRT()()
- server := newServer()
+ server := newServer(veyron2.ServesMountTableOpt(servesMT))
defer server.Stop()
service := serviceCreator(server)
vlog.Infof("created %v", service)
@@ -65,7 +65,7 @@
}
func childMT(args []string) {
- serverMain(createMT, args)
+ serverMain(true, createMT, args)
}
func createMTClient(name string) mtidl.MountTable {
@@ -93,7 +93,7 @@
}
func childFortune(args []string) {
- serverMain(createFortune, args)
+ serverMain(false, createFortune, args)
}
type fortuneCustomUnresolve struct {
@@ -124,14 +124,15 @@
func createFortuneCustomUnresolve(server ipc.Server) string {
oa := createServer(server, "tell/me/the/future", ipc.SoloDispatcher(fortuneidl.NewServerFortune(new(fortuneCustomUnresolve)), nil))
ep, _ := naming.SplitAddressName(oa)
- oa = naming.JoinAddressName(ep, "tell/me")
+ oa = naming.MakeTerminal(naming.JoinAddressName(ep, "tell/me"))
// Doesn't get unmounted. Fine for a test.
+ oa = naming.MakeTerminal(oa)
rt.R().MountTable().Mount("I/want/to/know", oa, 0)
return oa
}
func childFortuneCustomUnresolve(args []string) {
- serverMain(createFortuneCustomUnresolve, args)
+ serverMain(false, createFortuneCustomUnresolve, args)
}
func createFortuneClient(rt veyron2.Runtime, name string) fortuneidl.Fortune {
@@ -166,7 +167,7 @@
}
func childFortuneNoIDL(args []string) {
- serverMain(createFortuneNoIDL, args)
+ serverMain(false, createFortuneNoIDL, args)
}
func resolveStep(t *testing.T, name string) string {
diff --git a/examples/unresolve/unresolve_test.go b/examples/unresolve/unresolve_test.go
index ffb84e2..7a218ab 100644
--- a/examples/unresolve/unresolve_test.go
+++ b/examples/unresolve/unresolve_test.go
@@ -98,11 +98,11 @@
// TODO(ataly): Eventually we want to use the same identities the servers
// would have if they were running in production.
defer initRT()()
- server := newServer()
- defer server.Stop()
+ mtServer := newServer(veyron2.ServesMountTableOpt(true))
+ defer mtServer.Stop()
// Create mounttable A.
- aOA := createMT(server)
+ aOA := createMT(mtServer)
if len(aOA) == 0 {
t.Fatalf("aOA is empty")
}
diff --git a/runtimes/google/ipc/full_test.go b/runtimes/google/ipc/full_test.go
index 54e56ca..4b53246 100644
--- a/runtimes/google/ipc/full_test.go
+++ b/runtimes/google/ipc/full_test.go
@@ -194,6 +194,11 @@
return nil, nil
}
+func (mt *mountTable) SetRoots([]string) error {
+ panic("SetRoots not implemented")
+ return nil
+}
+
func startServer(t *testing.T, serverID security.PrivateID, sm stream.Manager, mt naming.MountTable, ts interface{}) ipc.Server {
vlog.VI(1).Info("InternalNewServer")
server, err := InternalNewServer(sm, mt, veyron2.LocalID(serverID))
diff --git a/runtimes/google/ipc/server.go b/runtimes/google/ipc/server.go
index 4844772..02a6924 100644
--- a/runtimes/google/ipc/server.go
+++ b/runtimes/google/ipc/server.go
@@ -32,15 +32,16 @@
type server struct {
sync.Mutex
- streamMgr stream.Manager // stream manager to listen for new flows.
- disptrie *disptrie // dispatch trie for method dispatching.
- publisher Publisher // publisher to publish mounttable mounts.
- listenerOpts []stream.ListenerOpt // listener opts passed to Listen.
- listeners []stream.Listener // listeners created by Listen.
- active sync.WaitGroup // active goroutines we've spawned.
- stopped bool // whether the server has been stopped.
- mt naming.MountTable
- publishOpt veyron2.ServerPublishOpt // which endpoints to publish
+ streamMgr stream.Manager // stream manager to listen for new flows.
+ disptrie *disptrie // dispatch trie for method dispatching.
+ publisher Publisher // publisher to publish mounttable mounts.
+ listenerOpts []stream.ListenerOpt // listener opts passed to Listen.
+ listeners []stream.Listener // listeners created by Listen.
+ active sync.WaitGroup // active goroutines we've spawned.
+ stopped bool // whether the server has been stopped.
+ mt naming.MountTable
+ publishOpt veyron2.ServerPublishOpt // which endpoints to publish
+ servesMountTable bool
}
func InternalNewServer(streamMgr stream.Manager, mt naming.MountTable, opts ...ipc.ServerOpt) (ipc.Server, error) {
@@ -57,6 +58,8 @@
s.listenerOpts = append(s.listenerOpts, opt)
case veyron2.ServerPublishOpt:
s.publishOpt = opt
+ case veyron2.ServesMountTableOpt:
+ s.servesMountTable = bool(opt)
}
}
return s, nil
@@ -161,6 +164,11 @@
}
s.Unlock()
if len(publishEP) > 0 {
+ if !s.servesMountTable {
+ // Make sure that client MountTable code doesn't try and
+ // ResolveStep past this final address.
+ publishEP += "//"
+ }
s.publisher.AddServer(publishEP)
}
return ep, nil
diff --git a/runtimes/google/naming/mounttable/all_test.go b/runtimes/google/naming/mounttable/all_test.go
index e771e7b..a3d67b8 100644
--- a/runtimes/google/naming/mounttable/all_test.go
+++ b/runtimes/google/naming/mounttable/all_test.go
@@ -256,15 +256,15 @@
}
// This mounts the service OA (ep/joke1) as joke1.
- if err := mt.Mount("joke1", naming.JoinAddressName(estr, "joke1"), ttl); err != nil {
+ if err := mt.Mount("joke1", naming.JoinAddressName(estr, "//joke1"), ttl); err != nil {
boom(t, "Failed to Mount joke1: %s", err)
}
// This mounts the raw server endpoint as joke2 -- like Publish would.
- if err := mt.Mount("joke2", naming.JoinAddressName(estr, ""), ttl); err != nil {
+ if err := mt.Mount("joke2", naming.JoinAddressName(estr, "")+"//", ttl); err != nil {
boom(t, "Failed to Mount joke2: %s", err)
}
// This mounts the raw server endpoint as joke3 in mt3 -- like Publish would.
- if err := mt.Mount("mt3/joke3", naming.JoinAddressName(estr, ""), ttl); err != nil {
+ if err := mt.Mount("mt3/joke3", naming.JoinAddressName(estr, "")+"//", ttl); err != nil {
boom(t, "Failed to Mount joke3: %s", err)
}
@@ -295,14 +295,16 @@
// And the MountTable that serves //mt4/mt5 is /<epstr>//mt1/mt4/mt5
testResolveToMountTable(t, mt, "//mt4/mt5", naming.JoinAddressName(estr, "//mt1//mt4/mt5"))
+ vlog.Infof("\n-------------------------------------------------")
jokeTests := []struct {
name, resolved, resolvedToMT string
}{
{"joke1", naming.JoinAddressName(estr, "//joke1"), naming.JoinAddressName(estr, "//mt1/joke1")},
- {"joke2", naming.JoinAddressName(estr, ""), naming.JoinAddressName(estr, "//mt1/joke2")},
- {"mt3/joke3", naming.JoinAddressName(estr, ""), naming.JoinAddressName(estr, "//mt3/joke3")},
+ {"joke2", naming.JoinAddressName(estr, "") + "//", naming.JoinAddressName(estr, "//mt1/joke2")},
+ {"mt3/joke3", naming.JoinAddressName(estr, "") + "//", naming.JoinAddressName(estr, "//mt3/joke3")},
}
for _, test := range jokeTests {
+
servers, err := mt.Resolve(test.name)
if err != nil {
boom(t, "Failed to Resolve %s: %s", test.name, err)
@@ -310,6 +312,7 @@
if len(servers) != 1 || servers[0] != test.resolved {
boom(t, "Resolve %s returned wrong servers: %v, expected: %s", test.name, servers, test.resolved)
}
+
servers, err = mt.ResolveToMountTable(test.name)
if err != nil {
boom(t, "Failed to ResolveToMountTable %s: %s", test.name, err)
@@ -329,6 +332,7 @@
expected []string
}{
{"*", []string{"mt2", "mt3", "joke1", "joke2"}},
+
{"*/...", []string{"mt2", "mt3", "mt2/mt4", "mt3/mt4", "mt2/mt4/mt5", "mt3/mt4/mt5", "joke1", "joke2", "mt3/joke3"}},
{"*/m?4/*5", []string{"mt2/mt4/mt5", "mt3/mt4/mt5"}},
{"*2*/*/*5", []string{"mt2/mt4/mt5"}},
@@ -339,4 +343,5 @@
out := doGlob(t, mt, test.pattern)
checkMatch(t, test.pattern, test.expected, out)
}
+
}
diff --git a/runtimes/google/naming/mounttable/namespace.go b/runtimes/google/naming/mounttable/namespace.go
index 7a8bb00..6de47ad 100644
--- a/runtimes/google/naming/mounttable/namespace.go
+++ b/runtimes/google/naming/mounttable/namespace.go
@@ -40,9 +40,8 @@
return &namespace{rt: rt, roots: roots}, nil
}
-// TODO(cnicolaou,caprita): make this a public interface.
-// SetLocalRoots points the local roots at a set of mount table servers.
-func (ns *namespace) SetLocalRoots(roots ...string) error {
+// SetRoots implements naming.MountTable.SetRoots
+func (ns *namespace) SetRoots(roots []string) error {
if !rooted(roots) {
return badRoots(roots)
}
diff --git a/runtimes/google/naming/mounttable/resolve.go b/runtimes/google/naming/mounttable/resolve.go
index d7272d3..43d9b59 100644
--- a/runtimes/google/naming/mounttable/resolve.go
+++ b/runtimes/google/naming/mounttable/resolve.go
@@ -118,6 +118,9 @@
vlog.VI(2).Infof("ResolveToMountTable loop %s", names)
var err error
curr := names
+ if terminal(curr) {
+ return makeTerminal(last), nil
+ }
if names, err = resolveAgainstMountTable(ns.rt, names); err != nil {
if verror.Equal(naming.ErrNoSuchNameRoot, err) {
return makeTerminal(last), nil
@@ -138,9 +141,7 @@
// mounttable rather than an error.
return nil, err
}
- if terminal(curr) {
- return curr, nil
- }
+
last = curr
}
return nil, naming.ErrResolutionDepthExceeded
diff --git a/services/mgmt/node/impl/impl_test.go b/services/mgmt/node/impl/impl_test.go
index ddebcc2..ad89b20 100644
--- a/services/mgmt/node/impl/impl_test.go
+++ b/services/mgmt/node/impl/impl_test.go
@@ -127,7 +127,7 @@
return child
}
-func startApplicationRepository(t *testing.T, runtime veyron2.Runtime, cmAddress string, envelope *application.Envelope) (string, func()) {
+func startApplicationRepository(t *testing.T, runtime veyron2.Runtime, cmAddress string, envelope *application.Envelope) (string, naming.Endpoint, func()) {
server, err := runtime.NewServer()
if err != nil {
t.Fatalf("NewServer() failed: %v", err)
@@ -141,16 +141,17 @@
if err != nil {
t.Fatalf("Listen(%v, %v) failed: %v", protocol, hostname, err)
}
- address := naming.JoinAddressName(endpoint.String(), suffix)
- vlog.VI(1).Infof("Application repository running at endpoint: %s", address)
- return address, func() {
+ // Method calls must be directed to suffix+"/"+suffix
+ server.Publish(suffix)
+ vlog.VI(1).Infof("Application repository running at endpoint: %s", endpoint)
+ return suffix + "/" + suffix, endpoint, func() {
if err := server.Stop(); err != nil {
t.Fatalf("Stop() failed: %v", err)
}
}
}
-func startContentManager(t *testing.T, runtime veyron2.Runtime) (string, func()) {
+func startContentManager(t *testing.T, runtime veyron2.Runtime) (string, naming.Endpoint, func()) {
server, err := runtime.NewServer()
if err != nil {
t.Fatalf("NewServer() failed: %v", err)
@@ -164,9 +165,10 @@
if err != nil {
t.Fatalf("Listen(%v, %v) failed: %v", protocol, hostname, err)
}
- address := naming.JoinAddressName(endpoint.String(), suffix)
- vlog.VI(1).Infof("Content manager running at endpoint: %s", address)
- return address, func() {
+ // Method calls must be directed to suffix+"/"+suffix
+ server.Publish(suffix)
+ vlog.VI(1).Infof("Content manager running at endpoint: %s", endpoint)
+ return suffix + "/" + suffix, endpoint, func() {
if err := server.Stop(); err != nil {
t.Fatalf("Stop() failed: %v", err)
}
@@ -174,7 +176,7 @@
}
func startMountTable(t *testing.T, runtime veyron2.Runtime) (string, func()) {
- server, err := runtime.NewServer()
+ server, err := runtime.NewServer(veyron2.ServesMountTableOpt(true))
if err != nil {
t.Fatalf("NewServer() failed: %v", err)
}
@@ -191,9 +193,9 @@
if err != nil {
t.Fatalf("Listen(%v, %v) failed: %v", protocol, hostname, err)
}
- address := naming.JoinAddressName(endpoint.String(), suffix)
- vlog.VI(1).Infof("Mount table running at endpoint: %s", address)
- return address, func() {
+ name := naming.JoinAddressName(endpoint.String(), suffix)
+ vlog.VI(1).Infof("Mount table running at endpoint: %s, name %q", endpoint, name)
+ return name, func() {
if err := server.Stop(); err != nil {
t.Fatalf("Stop() failed: %v", err)
}
@@ -236,15 +238,30 @@
// Set up a mount table, a content manager, and an application repository.
runtime := rt.Init()
defer runtime.Shutdown()
- mtAddress, mtCleanup := startMountTable(t, runtime)
+ mtName, mtCleanup := startMountTable(t, runtime)
defer mtCleanup()
mt := runtime.MountTable()
- cmAddress, cmCleanup := startContentManager(t, runtime)
+ // The local, client side MountTable is now relative the MountTable server
+ // started above.
+ mt.SetRoots([]string{mtName})
+
+ cmSuffix, cmEndpoint, cmCleanup := startContentManager(t, runtime)
+ cmName := naming.Join(mtName, cmSuffix)
defer cmCleanup()
envelope := application.Envelope{}
- arAddress, arCleanup := startApplicationRepository(t, runtime, cmAddress, &envelope)
+ arSuffix, arEndpoint, arCleanup := startApplicationRepository(t, runtime, cmSuffix, &envelope)
+ //arName := naming.Join(mtName, arSuffix)
defer arCleanup()
+ if s, err := mt.Resolve(arSuffix); err != nil || s[0] != "/"+arEndpoint.String()+"//ar" {
+ t.Errorf("failed to resolve %q", arSuffix)
+ t.Errorf("err: %v, got %v, want /%v//ar", err, s[0], arEndpoint)
+ }
+ if s, err := mt.Resolve(cmSuffix); err != nil || s[0] != "/"+cmEndpoint.String()+"//cm" {
+ t.Errorf("failed to resolve %q", cmSuffix)
+ t.Errorf("err: %v, got %v, want /%v//cm", err, s[0], cmEndpoint)
+ }
+
// Spawn a node manager with an identity blessed by the mounttable's identity.
// under the name "test", and obtain its endpoint.
// TODO(ataly): Eventually we want to use the same identity the node manager
@@ -252,14 +269,14 @@
idFile := testutil.SaveIdentityToFile(testutil.NewBlessedIdentity(runtime.Identity(), "test"))
defer os.Remove(idFile)
- child := spawnNodeManager(t, arAddress, mtAddress, idFile)
+ child := spawnNodeManager(t, arSuffix, mtName, idFile)
defer child.Cleanup()
_ = getProcessID(t, child) // sync with the child
envelope.Args = child.Cmd.Args[1:]
envelope.Env = child.Cmd.Env
- envelope.Binary = cmAddress
+ envelope.Binary = cmName
- name := naming.Join(mtAddress, "nm")
+ name := naming.Join(mtName, "nm")
results, err := mt.Resolve(name)
if err != nil {
t.Fatalf("Resolve(%v) failed: %v", name, err)
diff --git a/services/mounttable/lib/mounttable_test.go b/services/mounttable/lib/mounttable_test.go
index d9e2bc2..8bd0f82 100644
--- a/services/mounttable/lib/mounttable_test.go
+++ b/services/mounttable/lib/mounttable_test.go
@@ -101,6 +101,10 @@
return nil, errors.New("Glob is not implemented in this MountTable")
}
+func (s stupidMT) SetRoots([]string) error {
+ return nil
+}
+
func doMount(t *testing.T, name, service string, shouldSucceed bool, id ipc.ClientOpt) {
mtpt, err := mounttable.BindMountTable(name, quuxClient(id))
if err != nil {
diff --git a/services/mounttable/mounttabled/mounttable.go b/services/mounttable/mounttabled/mounttable.go
index ac44e45..5749146 100644
--- a/services/mounttable/mounttabled/mounttable.go
+++ b/services/mounttable/mounttabled/mounttable.go
@@ -6,6 +6,7 @@
"fmt"
"os"
+ "veyron2"
"veyron2/naming"
"veyron2/rt"
"veyron2/vlog"
@@ -46,7 +47,7 @@
r := rt.Init()
defer r.Shutdown()
- server, err := r.NewServer()
+ server, err := r.NewServer(veyron2.ServesMountTableOpt(true))
if err != nil {
vlog.Errorf("r.NewServer failed: %v", err)
return
diff --git a/services/wspr/wsprd/lib/wspr_test.go b/services/wspr/wsprd/lib/wspr_test.go
index 6e68a2d..1ed34a9 100644
--- a/services/wspr/wsprd/lib/wspr_test.go
+++ b/services/wspr/wsprd/lib/wspr_test.go
@@ -92,8 +92,16 @@
type registerFunc func(ipc.Server) error
func startServer(registerer registerFunc) (ipc.Server, naming.Endpoint, error) {
+ return startAnyServer(false, registerer)
+}
+
+func startMTServer(registerer registerFunc) (ipc.Server, naming.Endpoint, error) {
+ return startAnyServer(true, registerer)
+}
+
+func startAnyServer(servesMT bool, registerer registerFunc) (ipc.Server, naming.Endpoint, error) {
// Create a new server instance.
- s, err := r.NewServer()
+ s, err := r.NewServer(veyron2.ServesMountTableOpt(servesMT))
if err != nil {
return nil, nil, err
}
@@ -126,7 +134,7 @@
}
func startMountTableServer() (ipc.Server, naming.Endpoint, error) {
- return startServer(func(server ipc.Server) error {
+ return startMTServer(func(server ipc.Server) error {
mt, err := mounttable.NewMountTable("")
if err != nil {
return err
@@ -231,7 +239,7 @@
wspr.setup()
wsp := websocketPipe{ctx: wspr}
wsp.setup()
- jsSig, err := wsp.getSignature("/"+endpoint.String()+"/cache", "")
+ jsSig, err := wsp.getSignature("/"+endpoint.String()+"//cache", "")
if err != nil {
t.Errorf("Failed to get signature: %v", err)
}
@@ -280,7 +288,7 @@
}
request := veyronRPC{
- Name: "/" + endpoint.String() + "/cache",
+ Name: "/" + endpoint.String() + "//cache",
Method: test.method,
InArgs: test.inArgs,
NumOutArgs: test.numOutArgs,
@@ -453,7 +461,7 @@
proxyEndpoint := proxyServer.Endpoint().String()
- wspr := NewWSPR(0, "/"+proxyEndpoint, veyron2.MountTableRoots{"/" + endpoint.String() + "/mt"})
+ wspr := NewWSPR(0, "/"+proxyEndpoint, veyron2.MountTableRoots{"/" + endpoint.String() + "/mt//"})
wspr.setup()
wsp := websocketPipe{ctx: wspr}
writer := testWriter{
@@ -500,7 +508,7 @@
t.Errorf("unable to create client: %v", err)
}
- call, err := client.StartCall(wspr.rt.NewContext(), "/"+msg+"/adder", test.method, test.inArgs)
+ call, err := client.StartCall(wspr.rt.NewContext(), "/"+msg+"//adder", test.method, test.inArgs)
if err != nil {
t.Errorf("failed to start call: %v", err)
}
