fix etcd keys for services (#1922)

registry/etcd/etcd.go

@@ -143,9 +143,10 @@ func configure(e *etcdRegistry, opts ...registry.Option) error {
 	return nil
 }
 
-// hasName checks if the key has the name we expect
+// getName returns the domain and name
+// it returns false if there's an issue
 // the key is a path of /prefix/domain/name/id e.g /micro/registry/domain/service/uuid
-func hasName(key, prefix, name string) bool {
+func getName(key, prefix string) (string, string, bool) {
 	// strip the prefix from keys
 	key = strings.TrimPrefix(key, prefix)
 
@@ -153,7 +154,7 @@ func hasName(key, prefix, name string) bool {
 	parts := strings.Split(key, "/")
 
 	if len(parts) == 0 {
-		return false
+		return "", "", false
 	}
 
 	if len(parts[0]) == 0 {
@@ -162,11 +163,11 @@ func hasName(key, prefix, name string) bool {
 
 	// we expect a domain and then name domain/service
 	if len(parts) < 2 {
-		return false
+		return "", "", false
 	}
 
-	// ensure the name matches what we expect
+	// return name, domain
-	return parts[1] == name
+	return parts[0], parts[1], true
 }
 
 func encode(s *registry.Service) string {
@@ -220,12 +221,16 @@ func (e *etcdRegistry) registerNode(s *registry.Service, node *registry.Node, op
 		options.Domain = defaultDomain
 	}
 
-	// set the domain in metadata so it can be retrieved by wildcard queries
 	if s.Metadata == nil {
-		s.Metadata = map[string]string{"domain": options.Domain}
+		s.Metadata = map[string]string{}
-	} else {
-		s.Metadata["domain"] = options.Domain
 	}
+	if node.Metadata == nil {
+		node.Metadata = map[string]string{}
+	}
+
+	// set the domain in metadata so it can be retrieved by wildcard queries
+	s.Metadata["domain"] = options.Domain
+	node.Metadata["domain"] = options.Domain
 
 	e.Lock()
 	// ensure the leases and registers are setup for this domain
@@ -465,7 +470,8 @@ func (e *etcdRegistry) GetService(name string, opts ...registry.GetOption) ([]*r
 		// filter the results for the key we care about
 		for _, kv := range rsp.Kvs {
 			// if the key does not contain the name then pass
-			if !hasName(string(kv.Key), prefix, name) {
+			_, service, ok := getName(string(kv.Key), prefix)
+			if !ok || service != name {
 				continue
 			}
 
@@ -489,12 +495,16 @@ func (e *etcdRegistry) GetService(name string, opts ...registry.GetOption) ([]*r
 
 	for _, n := range results {
 		// only process the things we care about
-		if !hasName(string(n.Key), prefix, name) {
+		domain, service, ok := getName(string(n.Key), prefix)
+		if !ok || service != name {
 			continue
 		}
 
 		if sn := decode(n.Value); sn != nil {
-			s, ok := versions[sn.Version]
+			// compose a key of name/version/domain
+			key := sn.Name + sn.Version + domain
+
+			s, ok := versions[key]
 			if !ok {
 				s = &registry.Service{
 					Name:      sn.Name,
@@ -502,7 +512,7 @@ func (e *etcdRegistry) GetService(name string, opts ...registry.GetOption) ([]*r
 					Metadata:  sn.Metadata,
 					Endpoints: sn.Endpoints,
 				}
-				versions[s.Version] = s
+				versions[key] = s
 			}
 			s.Nodes = append(s.Nodes, sn.Nodes...)
 		}
@@ -514,8 +524,6 @@ func (e *etcdRegistry) GetService(name string, opts ...registry.GetOption) ([]*r
 		services = append(services, service)
 	}
 
-	logger.Tracef("[etcd] registry get service %s returned %v", name, services)
-
 	return services, nil
 }
 
@@ -550,15 +558,22 @@ func (e *etcdRegistry) ListServices(opts ...registry.ListOption) ([]*registry.Se
 
 	versions := make(map[string]*registry.Service)
 	for _, n := range rsp.Kvs {
-		sn := decode(n.Value)
+		domain, service, ok := getName(string(n.Key), prefix)
-
+		if !ok {
-		if sn == nil {
 			continue
 		}
 
-		v, ok := versions[sn.Name+sn.Version]
+		sn := decode(n.Value)
+		if sn == nil || sn.Name != service {
+			continue
+		}
+
+		// key based on name/version/domain
+		key := sn.Name + sn.Version + domain
+
+		v, ok := versions[key]
 		if !ok {
-			versions[sn.Name+sn.Version] = sn
+			versions[key] = sn
 			continue
 		}
 

registry/etcd/etcd_test.go

@@ -10,44 +10,59 @@ func TestEtcdHasName(t *testing.T) {
 		key    string
 		prefix string
 		name   string
+		domain string
 		expect bool
 	}{
 		{
 			"/micro/registry/micro/registry",
 			"/micro/registry",
 			"registry",
+			"micro",
 			true,
 		},
 		{
-			"/micro/registry/micro/store",
+			"/micro/registry/micro",
 			"/micro/registry",
-			"registry",
+			"store",
+			"micro",
 			false,
 		},
 		{
 			"/prefix/baz/*/registry",
 			"/prefix/baz",
 			"registry",
+			"*",
 			true,
 		},
 		{
-			"/prefix/baz/micro/registry",
+			"/prefix/baz",
 			"/prefix/baz",
 			"store",
+			"micro",
 			false,
 		},
 		{
-			"/prefix/baz/micro/registry",
+			"/prefix/baz/foobar/registry",
 			"/prefix/baz",
 			"registry",
+			"foobar",
 			true,
 		},
 	}
 
 	for _, c := range testCases {
-		v := hasName(c.key, c.prefix, c.name)
+		domain, service, ok := getName(c.key, c.prefix)
-		if v != c.expect {
+		if ok != c.expect {
-			t.Fatalf("Expected %t for %v got: %t", c.expect, c, v)
+			t.Fatalf("Expected %t for %v got: %t", c.expect, c, ok)
+		}
+		if !ok {
+			continue
+		}
+		if service != c.name {
+			t.Fatalf("Expected service %s got %s", c.name, service)
+		}
+		if domain != c.domain {
+			t.Fatalf("Expected domain %s got %s", c.domain, domain)
 		}
 	}
 }

registry/mdns/mdns.go

@@ -202,7 +202,8 @@ func createServiceMDNSEntry(name, domain string) (*mdnsEntry, error) {
 	return &mdnsEntry{id: "*", node: srv}, nil
 }
 
-func (m *mdnsRegistry) getMdnsEntries(domain, serviceName string) ([]*mdnsEntry, error) {
+func (m *mdnsRegistry) createMDNSEntries(domain, serviceName string) ([]*mdnsEntry, error) {
+	// if it already exists don't reegister it again
 	entries, ok := m.domains[domain][serviceName]
 	if ok {
 		return entries, nil
@@ -320,7 +321,7 @@ func (m *mdnsRegistry) Register(service *registry.Service, opts ...registry.Regi
 		m.domains[options.Domain] = make(services)
 	}
 
-	entries, err := m.getMdnsEntries(options.Domain, service.Name)
+	entries, err := m.createMDNSEntries(options.Domain, service.Name)
 	if err != nil {
 		m.Unlock()
 		return err

registry/memory/memory.go

@@ -199,24 +199,36 @@ func (m *Registry) Register(s *registry.Service, opts ...registry.RegisterOption
 		go m.sendEvent(&registry.Result{Action: "create", Service: s})
 	}
 
-	addedNodes := false
+	var addedNodes bool
+
 	for _, n := range s.Nodes {
-		if _, ok := srvs[s.Name][s.Version].Nodes[n.Id]; !ok {
+		// check if already exists
-			addedNodes = true
+		if _, ok := srvs[s.Name][s.Version].Nodes[n.Id]; ok {
-			metadata := make(map[string]string)
+			continue
-			for k, v := range n.Metadata {
-				metadata[k] = v
-				srvs[s.Name][s.Version].Nodes[n.Id] = &node{
-					Node: &registry.Node{
-						Id:       n.Id,
-						Address:  n.Address,
-						Metadata: metadata,
-					},
-					TTL:      options.TTL,
-					LastSeen: time.Now(),
-				}
-			}
 		}
+
+		metadata := make(map[string]string)
+
+		// make copy of metadata
+		for k, v := range n.Metadata {
+			metadata[k] = v
+		}
+
+		// set the domain
+		metadata["domain"] = options.Domain
+
+		// add the node
+		srvs[s.Name][s.Version].Nodes[n.Id] = &node{
+			Node: &registry.Node{
+				Id:       n.Id,
+				Address:  n.Address,
+				Metadata: metadata,
+			},
+			TTL:      options.TTL,
+			LastSeen: time.Now(),
+		}
+
+		addedNodes = true
 	}
 
 	if addedNodes {
@@ -270,6 +282,7 @@ func (m *Registry) Deregister(s *registry.Service, opts ...registry.DeregisterOp
 	if !ok {
 		return nil
 	}
+
 	version, ok := versions[s.Version]
 	if !ok {
 		return nil
@@ -332,6 +345,7 @@ func (m *Registry) GetService(name string, opts ...registry.GetOption) ([]*regis
 		m.RUnlock()
 
 		var services []*registry.Service
+
 		for domain := range recs {
 			srvs, err := m.GetService(name, append(opts, registry.GetDomain(domain))...)
 			if err == registry.ErrNotFound {
@@ -365,11 +379,14 @@ func (m *Registry) GetService(name string, opts ...registry.GetOption) ([]*regis
 
 	// serialize the response
 	result := make([]*registry.Service, len(versions))
-	i := 0
+
+	var i int
+
 	for _, r := range versions {
 		result[i] = recordToService(r, options.Domain)
 		i++
 	}
+
 	return result, nil
 }
 
@@ -390,6 +407,7 @@ func (m *Registry) ListServices(opts ...registry.ListOption) ([]*registry.Servic
 		m.RUnlock()
 
 		var services []*registry.Service
+
 		for domain := range recs {
 			srvs, err := m.ListServices(append(opts, registry.ListDomain(domain))...)
 			if err != nil {
@@ -412,11 +430,13 @@ func (m *Registry) ListServices(opts ...registry.ListOption) ([]*registry.Servic
 
 	// serialize the result, each version counts as an individual service
 	var result []*registry.Service
+
 	for domain, service := range services {
 		for _, version := range service {
 			result = append(result, recordToService(version, domain))
 		}
 	}
+
 	return result, nil
 }