Merge lp:~themue/pyjuju/go-state-topology-relation-endpoints into lp:pyjuju/go

[Frank Mueller]

Added methods for relation endpoints to topology.

Relation endpoints needed two methods for testing and
key retrieving in topology.

https://codereview.appspot.com/6198055/

[William Reade (fwereade)]

https://codereview.appspot.com/6198055/diff/1/state/topology.go
File state/topology.go (right):

https://codereview.appspot.com/6198055/diff/1/state/topology.go#newcode550
state/topology.go:550: if service == nil || service.Name !=
e.RelationName || scope != e.RelationScope {
I'm pretty sure that "service.Name != e.RelationName" is either insane,
or it betrays a serious misnaming issue somewhere.

That said, per prereq review, I think we should drop RelationName
anyway.

https://codereview.appspot.com/6198055/diff/1/state/topology.go#newcode563
state/topology.go:563: // between "endpoints" or "found" will be false.
I'm -1 on the ... here, I think it's a wart in the python version and
I'd prefer not to duplicate it in Go. Also, it feels like "found" is
redundant -- can't we just return a key of ""? How about:

func (t *topology) RelationKey(ep1, ep2 RelationEndpoint) (string,
error)

func (t *topology) PeerRelationKey(ep RelationEndpoint) (string, error)

...and, given that, I think that HasRelationBetweenEndpoints is actually
completely redundant.

https://codereview.appspot.com/6198055/diff/1/state/topology.go#newcode576
state/topology.go:576: for key, relation := range t.topology.Relations {
Please double-check why the implementation here is different to the one
in HasRelationBetweenEndpoints... by gut says that this is evidence of a
bug somewhere.

https://codereview.appspot.com/6198055/

[Gustavo Niemeyer (niemeyer)]

https://codereview.appspot.com/6198055/diff/5001/state/topology.go
File state/topology.go (right):

https://codereview.appspot.com/6198055/diff/5001/state/topology.go#newcode545
state/topology.go:545: return "", nil
If the developer asks for a RelationKey, and it doesn't exist, an error
should be returned. Silently returning nothing and hoping that the
developer remembers to check for the case where it doesn't exist every
time is extremely error prone.

Imagine if you had to do this:

     f, err := os.Open(filename)
     if err != nil {
          ...
     }
     if f != nil {
          ...
     }
     ...

Every time!

The documentation should reflect the change too.

https://codereview.appspot.com/6198055/diff/5001/state/topology.go#newcode571
state/topology.go:571: return "", nil
Ditto.

https://codereview.appspot.com/6198055/

[Gustavo Niemeyer (niemeyer)]

https://codereview.appspot.com/6198055/diff/8002/state/topology.go
File state/topology.go (right):

https://codereview.appspot.com/6198055/diff/8002/state/topology.go#newcode20
state/topology.go:20: errRelationDoesNotExist = errors.New("relation
does not exist")
If we define that error more nicely, we can allow it to pop out to of
the state package when using it. For example:

type noRelationError struct {
     endpoint1, endpoint2 RelationEndpoint
}

func (e *noRelationError) Error() string {
     if e.endpoint2 != nil {
         return fmt.Errorf("state: no peer relation for %s", (what
here?))
     } else {
         return fmt.Errorf("state: no relation between %s and %s", (what
here?))
     }
}

Not sure about the details of the error messages, but it should be
something nice that defines the endpoints.

What do you think?

https://codereview.appspot.com/6198055/

[Gustavo Niemeyer (niemeyer)]

https://codereview.appspot.com/6198055/

[Gustavo Niemeyer (niemeyer)]

Also in a good direction, thanks Frank.

https://codereview.appspot.com/6198055/diff/9002/state/relation.go
File state/relation.go (right):

https://codereview.appspot.com/6198055/diff/9002/state/relation.go#newcode55
state/relation.go:55: func (e *RelationEndpoint) String() string {
That's somewhat unexpected. What's the goal here?

I think I'd prefer to leave this out. %#v and %v are simpler and produce
better results than this as it is.

https://codereview.appspot.com/6198055/diff/9002/state/topology.go
File state/topology.go (right):

https://codereview.appspot.com/6198055/diff/9002/state/topology.go#newcode17
state/topology.go:17: // consumer endpoints of for one peer endpoint
does not exist.
// NoRelationError represents a relation not found for one or more
endpoints.

https://codereview.appspot.com/6198055/diff/9002/state/topology.go#newcode19
state/topology.go:19: Endpoint1 *RelationEndpoint
I suggest using this instead:

     Endpoints []RelationEndpoint

https://codereview.appspot.com/6198055/diff/9002/state/topology.go#newcode474
state/topology.go:474: // interface between two endpoints. If it doesn't
exist an error is returned.
// RelationKey returns the key for the relation established between the
// provided endpoints. If no matching relation is found, error will be
// of type *NoRelationError.

https://codereview.appspot.com/6198055/diff/9002/state/topology.go#newcode475
state/topology.go:475: func (t *topology) RelationKey(ep1, ep2
RelationEndpoint) (string, error) {
I think the design we had is better one here:

RelationKey(endpoints ...RelationEndpoint)

https://codereview.appspot.com/6198055/diff/9002/state/topology.go#newcode494
state/topology.go:494: if ep1.Interface != relation.Interface ||
ep2.Interface != relation.Interface {
If ep1 and ep2 have diverging interfaces, we can return an error before
even starting to search.

https://codereview.appspot.com/6198055/diff/9002/state/topology.go#newcode499
state/topology.go:499: }
Relation names matter as well.

https://codereview.appspot.com/6198055/diff/9002/state/topology.go#newcode506
state/topology.go:506: func (t *topology) PeerRelationKey(ep
RelationEndpoint) (string, error) {
We can subsume that into RelationKey as well.

https://codereview.appspot.com/6198055/

[Gustavo Niemeyer (niemeyer)]

Moving it back onto WIP.

https://codereview.appspot.com/6198055/

[Gustavo Niemeyer (niemeyer)]

https://codereview.appspot.com/6198055/diff/27001/state/topology.go
File state/topology.go (right):

https://codereview.appspot.com/6198055/diff/27001/state/topology.go#newcode471
state/topology.go:471: return "", fmt.Errorf("state: differing
interfaces %q and %q", endpoints[0].Interface, endpoints[1].Interface)
What about just returning NoRelationError?

https://codereview.appspot.com/6198055/diff/27001/state/topology.go#newcode474
state/topology.go:474: return "", fmt.Errorf("state: illegal number of
endpoints passed")
s/passed/provided/

https://codereview.appspot.com/6198055/diff/27001/state/topology.go#newcode491
state/topology.go:491: if serviceName != endpoint.RelationName {
Huh? That's quite wrong.

https://codereview.appspot.com/6198055/

[Gustavo Niemeyer (niemeyer)]

LGTM, with a few trivials:

https://codereview.appspot.com/6198055/diff/25003/state/topology.go
File state/topology.go (right):

https://codereview.appspot.com/6198055/diff/25003/state/topology.go#newcode29
state/topology.go:29: panic("state: illegal relation error")
s/ error//

https://codereview.appspot.com/6198055/diff/25003/state/topology.go#newcode73
state/topology.go:73: ServiceKey string
s/ServiceKey/Service/; see the convention in the other types.

https://codereview.appspot.com/6198055/diff/25003/state/topology.go#newcode74
state/topology.go:74: RelationName string
Please add a tag "relation-name" so we follow the naming convention too.

https://codereview.appspot.com/6198055/diff/25003/state/topology.go#newcode430
state/topology.go:430: return fmt.Errorf("provider and consumer keys
must not be the same")
s/consumer/requirer/

https://codereview.appspot.com/6198055/diff/25003/state/topology.go#newcode474
state/topology.go:474: if t.topology.Relations == nil {
Shouldn't the logic below work fine if this is dropped?

https://codereview.appspot.com/6198055/diff/25003/state/topology.go#newcode485
state/topology.go:485: return "", fmt.Errorf("state: illegal number of
endpoints provided")
s/endpoints/relation endpoints/

https://codereview.appspot.com/6198055/diff/25003/state/topology.go#newcode498
state/topology.go:498: if service.RelationName != endpoint.RelationName
{
Both of these if blocks can be made into one:

if !ok || ... {

https://codereview.appspot.com/6198055/

[Gustavo Niemeyer (niemeyer)]

LGTM

https://codereview.appspot.com/6198055/

[lp:~themue/pyjuju/go-state-topology-relation-endpoints]

163. By Frank Mueller on 2012-05-29

Merged trunk.

=== modified file 'state/internal_test.go'
--- state/internal_test.go	2012-05-25 14:10:49 +0000
+++ state/internal_test.go	2012-05-29 18:02:21 +0000
@@ -1,6 +1,3 @@
-// launchpad.net/juju/go/state
-//
-// Copyright (c) 2011-2012 Canonical Ltd.
 package state
 
 import (
@@ -461,12 +458,12 @@
 	s.t.AddRelation("r-1", &zkRelation{
 		Interface: "ifce",
 		Scope:     ScopeGlobal,
-		Services:  map[RelationRole]string{RolePeer: "s-p"},
+		Services:  map[RelationRole]*zkRelationService{RolePeer: &zkRelationService{"s-p", "cache"}},
 	})
 	relation, err = s.t.Relation("r-1")
 	c.Assert(err, IsNil)
 	c.Assert(relation, NotNil)
-	c.Assert(relation.Services[RolePeer], Equals, "s-p")
+	c.Assert(relation.Services[RolePeer].Service, Equals, "s-p")
 }
 
 func (s *TopologySuite) TestAddRelation(c *C) {
@@ -480,61 +477,82 @@
 	err = s.t.AddRelation("r-1", &zkRelation{
 		Interface: "ifce",
 		Scope:     ScopeGlobal,
-		Services:  map[RelationRole]string{RoleProvider: "s-p", RoleRequirer: "s-r"},
+		Services: map[RelationRole]*zkRelationService{
+			RoleProvider: &zkRelationService{"s-p", "db"},
+			RoleRequirer: &zkRelationService{"s-r", "db"},
+		},
 	})
 	c.Assert(err, IsNil)
 	relation, err = s.t.Relation("r-1")
 	c.Assert(err, IsNil)
 	c.Assert(relation, NotNil)
-	c.Assert(relation.Services[RoleProvider], Equals, "s-p")
-	c.Assert(relation.Services[RoleRequirer], Equals, "s-r")
+	c.Assert(relation.Services[RoleProvider].Service, Equals, "s-p")
+	c.Assert(relation.Services[RoleRequirer].Service, Equals, "s-r")
 
 	err = s.t.AddRelation("r-2", &zkRelation{
 		Interface: "",
 		Scope:     ScopeGlobal,
-		Services:  map[RelationRole]string{RoleProvider: "s-p", RoleRequirer: "s-r"},
+		Services: map[RelationRole]*zkRelationService{
+			RoleProvider: &zkRelationService{"s-p", "db"},
+			RoleRequirer: &zkRelationService{"s-r", "db"},
+		},
 	})
 	c.Assert(err, ErrorMatches, `relation interface is empty`)
 
 	err = s.t.AddRelation("r-3", &zkRelation{
 		Interface: "ifce",
 		Scope:     ScopeGlobal,
-		Services:  map[RelationRole]string{},
+		Services:  map[RelationRole]*zkRelationService{},
 	})
 	c.Assert(err, ErrorMatches, `relation has no services`)
 
 	err = s.t.AddRelation("r-4", &zkRelation{
 		Interface: "ifce",
 		Scope:     ScopeGlobal,
-		Services:  map[RelationRole]string{RoleProvider: "s-p"},
+		Services: map[RelationRole]*zkRelationService{
+			RoleProvider: &zkRelationService{"s-p", "db"},
+		},
 	})
 	c.Assert(err, ErrorMatches, `relation has provider but no requirer`)
 
 	err = s.t.AddRelation("r-5", &zkRelation{
 		Interface: "ifce",
 		Scope:     ScopeGlobal,
-		Services:  map[RelationRole]string{RoleProvider: "s-p", RolePeer: "s-r"},
+		Services: map[RelationRole]*zkRelationService{
+			RoleProvider: &zkRelationService{"s-p", "db"},
+			RolePeer:     &zkRelationService{"s-r", "db"},
+		},
 	})
 	c.Assert(err, ErrorMatches, `relation has provider but no requirer`)
 
 	err = s.t.AddRelation("r-6", &zkRelation{
 		Interface: "ifce",
 		Scope:     ScopeGlobal,
-		Services:  map[RelationRole]string{RoleProvider: "s-p", RoleRequirer: "s-r", RolePeer: "s-r"},
+		Services: map[RelationRole]*zkRelationService{
+			RoleProvider: &zkRelationService{"s-p", "db"},
+			RoleRequirer: &zkRelationService{"s-r", "db"},
+			RolePeer:     &zkRelationService{"s-r", "db"},
+		},
 	})
 	c.Assert(err, ErrorMatches, `relation with mixed peer, provider, and requirer roles`)
 
 	err = s.t.AddRelation("r-7", &zkRelation{
 		Interface: "ifce",
 		Scope:     ScopeGlobal,
-		Services:  map[RelationRole]string{RoleProvider: "s-p", RoleRequirer: "illegal"},
+		Services: map[RelationRole]*zkRelationService{
+			RoleProvider: &zkRelationService{"s-p", "db"},
+			RoleRequirer: &zkRelationService{"illegal", "db"},
+		},
 	})
 	c.Assert(err, ErrorMatches, `service with key "illegal" not found`)
 
 	err = s.t.AddRelation("r-1", &zkRelation{
 		Interface: "ifce",
 		Scope:     ScopeGlobal,
-		Services:  map[RelationRole]string{RoleProvider: "s-p", RoleRequirer: "s-r"},
+		Services: map[RelationRole]*zkRelationService{
+			RoleProvider: &zkRelationService{"s-p", "db"},
+			RoleRequirer: &zkRelationService{"s-r", "db"},
+		},
 	})
 	c.Assert(err, ErrorMatches, `relation key "r-1" already in use`)
 }
@@ -548,7 +566,9 @@
 	s.t.AddRelation("r-1", &zkRelation{
 		Interface: "ifce",
 		Scope:     ScopeGlobal,
-		Services:  map[RelationRole]string{RolePeer: "s-p"},
+		Services: map[RelationRole]*zkRelationService{
+			RolePeer: &zkRelationService{"s-p", "cache"},
+		},
 	})
 	keys = s.t.RelationKeys()
 	c.Assert(keys, DeepEquals, []string{"r-1"})
@@ -556,7 +576,9 @@
 	s.t.AddRelation("r-2", &zkRelation{
 		Interface: "ifce",
 		Scope:     ScopeGlobal,
-		Services:  map[RelationRole]string{RolePeer: "s-p"},
+		Services: map[RelationRole]*zkRelationService{
+			RolePeer: &zkRelationService{"s-p", "cache"},
+		},
 	})
 	keys = s.t.RelationKeys()
 	c.Assert(keys, DeepEquals, []string{"r-1", "r-2"})
@@ -572,12 +594,16 @@
 	s.t.AddRelation("r-0", &zkRelation{
 		Interface: "ifce0",
 		Scope:     ScopeGlobal,
-		Services:  map[RelationRole]string{RolePeer: "s-p"},
+		Services: map[RelationRole]*zkRelationService{
+			RolePeer: &zkRelationService{"s-p", "cache"},
+		},
 	})
 	s.t.AddRelation("r-1", &zkRelation{
 		Interface: "ifce1",
 		Scope:     ScopeGlobal,
-		Services:  map[RelationRole]string{RolePeer: "s-p"},
+		Services: map[RelationRole]*zkRelationService{
+			RolePeer: &zkRelationService{"s-p", "cache"},
+		},
 	})
 	relations, err = s.t.RelationsForService("s-p")
 	c.Assert(err, IsNil)
@@ -594,21 +620,24 @@
 
 func (s *TopologySuite) TestRemoveRelation(c *C) {
 	// Check that removing of a relation works.
-	s.t.AddService("s-c", "wordpress")
+	s.t.AddService("s-r", "wordpress")
 	s.t.AddService("s-p", "mysql")
 
 	err := s.t.AddRelation("r-1", &zkRelation{
 		Interface: "ifce",
 		Scope:     ScopeGlobal,
-		Services:  map[RelationRole]string{RoleProvider: "s-p", RoleRequirer: "s-c"},
+		Services: map[RelationRole]*zkRelationService{
+			RoleProvider: &zkRelationService{"s-p", "db"},
+			RoleRequirer: &zkRelationService{"s-r", "db"},
+		},
 	})
 	c.Assert(err, IsNil)
 
 	relation, err := s.t.Relation("r-1")
 	c.Assert(err, IsNil)
 	c.Assert(relation, NotNil)
-	c.Assert(relation.Services[RoleProvider], Equals, "s-p")
-	c.Assert(relation.Services[RoleRequirer], Equals, "s-c")
+	c.Assert(relation.Services[RoleProvider].Service, Equals, "s-p")
+	c.Assert(relation.Services[RoleRequirer].Service, Equals, "s-r")
 
 	s.t.RemoveRelation("r-1")
 
@@ -624,13 +653,148 @@
 	s.t.AddRelation("r-1", &zkRelation{
 		Interface: "ifce",
 		Scope:     ScopeGlobal,
-		Services:  map[RelationRole]string{RolePeer: "s-p"},
+		Services: map[RelationRole]*zkRelationService{
+			RolePeer: &zkRelationService{"s-p", "cache"},
+		},
 	})
 
 	err := s.t.RemoveService("s-p")
 	c.Assert(err, ErrorMatches, `cannot remove service "s-p" with active relations`)
 }
 
+func (s *TopologySuite) TestRelationKeyEndpoints(c *C) {
+	mysqlep1 := RelationEndpoint{"mysql", "ifce1", "db", RoleProvider, ScopeGlobal}
+	blogep1 := RelationEndpoint{"wordpress", "ifce1", "db", RoleRequirer, ScopeGlobal}
+	mysqlep2 := RelationEndpoint{"mysql", "ifce2", "db", RoleProvider, ScopeGlobal}
+	blogep2 := RelationEndpoint{"wordpress", "ifce2", "db", RoleRequirer, ScopeGlobal}
+	mysqlep3 := RelationEndpoint{"mysql", "ifce3", "db", RoleProvider, ScopeGlobal}
+	blogep3 := RelationEndpoint{"wordpress", "ifce3", "db", RoleRequirer, ScopeGlobal}
+	s.t.AddService("s-r", "wordpress")
+	s.t.AddService("s-p", "mysql")
+	s.t.AddRelation("r-0", &zkRelation{
+		Interface: "ifce1",
+		Scope:     ScopeGlobal,
+		Services: map[RelationRole]*zkRelationService{
+			RoleProvider: &zkRelationService{"s-p", "db"},
+			RoleRequirer: &zkRelationService{"s-r", "db"},
+		},
+	})
+	s.t.AddRelation("r-1", &zkRelation{
+		Interface: "ifce2",
+		Scope:     ScopeGlobal,
+		Services: map[RelationRole]*zkRelationService{
+			RoleProvider: &zkRelationService{"s-p", "db"},
+			RoleRequirer: &zkRelationService{"s-r", "db"},
+		},
+	})
+
+	// Valid relations.
+	key, err := s.t.RelationKey(mysqlep1, blogep1)
+	c.Assert(err, IsNil)
+	c.Assert(key, Equals, "r-0")
+	key, err = s.t.RelationKey(blogep1, mysqlep1)
+	c.Assert(err, IsNil)
+	c.Assert(key, Equals, "r-0")
+	key, err = s.t.RelationKey(mysqlep2, blogep2)
+	c.Assert(err, IsNil)
+	c.Assert(key, Equals, "r-1")
+	key, err = s.t.RelationKey(blogep2, mysqlep2)
+	c.Assert(err, IsNil)
+	c.Assert(key, Equals, "r-1")
+
+	// Endpoints without relation.
+	_, err = s.t.RelationKey(mysqlep3, blogep3)
+	c.Assert(err, ErrorMatches, `state: no relation between "mysql:db" and "wordpress:db"`)
+
+	// Mix of endpoints of two relations.
+	_, err = s.t.RelationKey(mysqlep1, blogep2)
+	c.Assert(err, ErrorMatches, `state: no relation between "mysql:db" and "wordpress:db"`)
+
+	// Illegal number of endpoints.
+	_, err = s.t.RelationKey()
+	c.Assert(err, ErrorMatches, `state: illegal number of relation endpoints provided`)
+	_, err = s.t.RelationKey(mysqlep1, mysqlep2, blogep1)
+	c.Assert(err, ErrorMatches, `state: illegal number of relation endpoints provided`)
+}
+
+func (s *TopologySuite) TestRelationKeyIllegalEndpoints(c *C) {
+	mysqlep1 := RelationEndpoint{"mysql", "ifce", "db", RoleProvider, ScopeGlobal}
+	blogep1 := RelationEndpoint{"wordpress", "ifce", "db", RoleRequirer, ScopeGlobal}
+	mysqlep2 := RelationEndpoint{"illegal-mysql", "ifce", "db", RoleProvider, ScopeGlobal}
+	blogep2 := RelationEndpoint{"illegal-wordpress", "ifce", "db", RoleRequirer, ScopeGlobal}
+	riakep3 := RelationEndpoint{"riak", "ifce", "ring", RolePeer, ScopeGlobal}
+	s.t.AddService("s-r", "wordpress")
+	s.t.AddService("s-p1", "mysql")
+	s.t.AddService("s-p2", "riak")
+	s.t.AddRelation("r-0", &zkRelation{
+		Interface: "ifce1",
+		Scope:     ScopeGlobal,
+		Services: map[RelationRole]*zkRelationService{
+			RoleProvider: &zkRelationService{"s-p", "db"},
+			RoleRequirer: &zkRelationService{"s-r", "db"},
+		},
+	})
+
+	key, err := s.t.RelationKey(mysqlep1, blogep2)
+	c.Assert(key, Equals, "")
+	c.Assert(err, ErrorMatches, `state: no relation between "mysql:db" and "illegal-wordpress:db"`)
+	key, err = s.t.RelationKey(mysqlep2, blogep1)
+	c.Assert(key, Equals, "")
+	c.Assert(err, ErrorMatches, `state: no relation between "illegal-mysql:db" and "wordpress:db"`)
+	key, err = s.t.RelationKey(mysqlep1, riakep3)
+	c.Assert(key, Equals, "")
+	c.Assert(err, ErrorMatches, `state: no relation between "mysql:db" and "riak:ring"`)
+}
+
+func (s *TopologySuite) TestPeerRelationKeyEndpoints(c *C) {
+	riakep1 := RelationEndpoint{"riak", "ifce1", "ring", RolePeer, ScopeGlobal}
+	riakep2 := RelationEndpoint{"riak", "ifce2", "ring", RolePeer, ScopeGlobal}
+	riakep3 := RelationEndpoint{"riak", "ifce3", "ring", RolePeer, ScopeGlobal}
+	s.t.AddService("s-p", "ring")
+	s.t.AddRelation("r-0", &zkRelation{
+		Interface: "ifce1",
+		Scope:     ScopeGlobal,
+		Services: map[RelationRole]*zkRelationService{
+			RolePeer: &zkRelationService{"s-p", "ring"},
+		},
+	})
+	s.t.AddRelation("r-1", &zkRelation{
+		Interface: "ifce2",
+		Scope:     ScopeGlobal,
+		Services: map[RelationRole]*zkRelationService{
+			RolePeer: &zkRelationService{"s-p", "ring"},
+		},
+	})
+
+	// Valid relations.
+	key, err := s.t.RelationKey(riakep1)
+	c.Assert(err, IsNil)
+	c.Assert(key, Equals, "r-0")
+	key, err = s.t.RelationKey(riakep2)
+	c.Assert(err, IsNil)
+	c.Assert(key, Equals, "r-1")
+
+	// Endpoint without relation.
+	key, err = s.t.RelationKey(riakep3)
+	c.Assert(err, ErrorMatches, `state: no peer relation for "riak:ring"`)
+}
+
+func (s *TopologySuite) TestPeerRelationKeyIllegalEndpoints(c *C) {
+	riakep1 := RelationEndpoint{"riak", "ifce", "illegal-ring", RolePeer, ScopeGlobal}
+	s.t.AddService("s-p", "riak")
+	s.t.AddRelation("r-0", &zkRelation{
+		Interface: "ifce",
+		Scope:     ScopeGlobal,
+		Services: map[RelationRole]*zkRelationService{
+			RolePeer: &zkRelationService{"s-p", "ring"},
+		},
+	})
+
+	key, err := s.t.RelationKey(riakep1)
+	c.Assert(key, Equals, "")
+	c.Assert(err, ErrorMatches, `state: no peer relation for "riak:illegal-ring"`)
+}
+
 type ConfigNodeSuite struct {
 	zkServer   *zookeeper.Server
 	zkTestRoot string

=== modified file 'state/relation.go'
--- state/relation.go	2012-05-24 18:57:02 +0000
+++ state/relation.go	2012-05-29 18:02:21 +0000
@@ -41,3 +41,8 @@
 	}
 	panic("endpoint role is undefined")
 }
+
+// String returns the unique identifier of the relation endpoint.
+func (e RelationEndpoint) String() string {
+	return e.ServiceName + ":" + e.RelationName
+}

=== modified file 'state/topology.go'
--- state/topology.go	2012-05-24 18:57:02 +0000
+++ state/topology.go	2012-05-29 18:02:21 +0000
@@ -13,6 +13,22 @@
 // when we know that a version is in fact actually incompatible.
 const topologyVersion = 1
 
+// NoRelationError represents a relation not found for one or more endpoints.
+type NoRelationError struct {
+	Endpoints []RelationEndpoint
+}
+
+// Error returns the string representation of the error.
+func (e NoRelationError) Error() string {
+	switch len(e.Endpoints) {
+	case 1:
+		return fmt.Sprintf("state: no peer relation for %q", e.Endpoints[0])
+	case 2:
+		return fmt.Sprintf("state: no relation between %q and %q", e.Endpoints[0], e.Endpoints[1])
+	}
+	panic("state: illegal relation")
+}
+
 // zkTopology is used to marshal and unmarshal the content
 // of the /topology node in ZooKeeper.
 type zkTopology struct {
@@ -47,7 +63,15 @@
 type zkRelation struct {
 	Interface string
 	Scope     RelationScope
-	Services  map[RelationRole]string
+	Services  map[RelationRole]*zkRelationService
+}
+
+// zkRelationService represents the data of one
+// service of a relation within the /topology
+// node in ZooKeeper.
+type zkRelationService struct {
+	Service      string
+	RelationName string "relation-name"
 }
 
 // check verifies that r is a proper relation.
@@ -63,9 +87,12 @@
 		RoleProvider: RoleRequirer,
 		RolePeer:     RolePeer,
 	}
-	for serviceRole, serviceKey := range r.Services {
-		if serviceKey == "" {
-			return fmt.Errorf("relation has %s service with empty key", serviceRole)
+	for serviceRole, service := range r.Services {
+		if service.Service == "" {
+			return fmt.Errorf("relation has %s service with empty service key", serviceRole)
+		}
+		if service.RelationName == "" {
+			return fmt.Errorf("relation has %s service with empty relation name", serviceRole)
 		}
 		counterRole, ok := counterpart[serviceRole]
 		if !ok {
@@ -391,16 +418,16 @@
 	if err := relation.check(); err != nil {
 		return err
 	}
-	for _, serviceKey := range relation.Services {
-		if err := t.assertService(serviceKey); err != nil {
+	for _, service := range relation.Services {
+		if err := t.assertService(service.Service); err != nil {
 			return err
 		}
 	}
-	if relation.Services[RolePeer] == "" {
-		providerKey := relation.Services[RoleProvider]
-		consumerKey := relation.Services[RoleRequirer]
-		if providerKey == consumerKey {
-			return fmt.Errorf("provider and consumer keys must not be the same")
+	if relation.Services[RolePeer] == nil {
+		providerKey := relation.Services[RoleProvider].Service
+		requirerKey := relation.Services[RoleRequirer].Service
+		if providerKey == requirerKey {
+			return fmt.Errorf("provider and requirer keys must not be the same")
 		}
 	}
 	t.topology.Relations[relationKey] = relation
@@ -430,8 +457,8 @@
 	}
 	relations := make(map[string]*zkRelation)
 	for relationKey, relation := range t.topology.Relations {
-		for _, roleServiceKey := range relation.Services {
-			if roleServiceKey == serviceKey {
+		for _, service := range relation.Services {
+			if service.Service == serviceKey {
 				relations[relationKey] = relation
 				break
 			}
@@ -440,6 +467,40 @@
 	return relations, nil
 }
 
+// RelationKey returns the key for the relation established between the
+// provided endpoints. If no matching relation is found, error will be
+// of type *NoRelationError.
+func (t *topology) RelationKey(endpoints ...RelationEndpoint) (string, error) {
+	switch len(endpoints) {
+	case 1:
+		// Just pass.
+	case 2:
+		if endpoints[0].Interface != endpoints[1].Interface {
+			return "", &NoRelationError{endpoints}
+		}
+	default:
+		return "", fmt.Errorf("state: illegal number of relation endpoints provided")
+	}
+	for relationKey, relation := range t.topology.Relations {
+		if relation.Interface != endpoints[0].Interface {
+			continue
+		}
+		found := true
+		for _, endpoint := range endpoints {
+			service, ok := relation.Services[endpoint.RelationRole]
+			if !ok || service.RelationName != endpoint.RelationName {
+				found = false
+				break
+			}
+		}
+		if found {
+			// All endpoints tested positive.
+			return relationKey, nil
+		}
+	}
+	return "", &NoRelationError{endpoints}
+}
+
 // assertMachine checks if a machine exists.
 func (t *topology) assertMachine(machineKey string) error {
 	if _, ok := t.topology.Machines[machineKey]; !ok {