Merge lp:~rogpeppe/juju-core/097-state-unit-id into lp:~juju/juju-core/trunk

[Roger Peppe]

state: rename Unit.Name to Unit.Id

This is a preparatory step to renaming PathKey to Name.

Also changed a couple of tests to use PathKey rather than Id.

https://codereview.appspot.com/6585053/

[Roger Peppe (rogpeppe)]

Reviewers: mp+127341_code.launchpad.net,

Message:
Please take a look.

Description:
state: rename Unit.Name to Unit.Id

This is a preparatory step to renaming PathKey to Name.

Also changed a couple of tests to use PathKey rather than Id.

https://code.launchpad.net/~rogpeppe/juju-core/097-state-unit-id/+merge/127341

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/6585053/

Affected files:
   A [revision details]
   M cmd/juju/deploy_test.go
   M cmd/juju/status.go
   M cmd/jujuc/server/context.go
   M cmd/jujuc/server/context_test.go
   M cmd/jujuc/server/juju-log.go
   M cmd/jujuc/server/relation-get.go
   M cmd/jujuc/server/util_test.go
   M cmd/jujud/unit.go
   M cmd/jujud/unit_test.go
   M container/container.go
   M state/assign_test.go
   M state/life_test.go
   M state/machine.go
   M state/machine_test.go
   M state/relation.go
   M state/relation_test.go
   M state/service.go
   M state/service_test.go
   M state/unit.go
   M state/unit_test.go
   M state/watcher.go
   M worker/firewaller/firewaller.go
   M worker/machiner/machiner.go
   M worker/machiner/machiner_test.go
   M worker/uniter/relationer_test.go
   M worker/uniter/uniter.go

[Gustavo Niemeyer (niemeyer)]

I've reviewed about 80% of this with trivial comments only, and then
when I got to reviewing the service bits, I got deadlocked.

You wisely decided not to switch service.Name to mean
"service-wordpress", because that'd mean the service name, which is a
sane concept, wouldn't be a service name. That, in turn, means the unit
Name means something totally unlike the service Name, so we get
unit.Name() => "unit-wordpress-0", but service.Name() => "wordpress".

Also, there are public parts of the interface, such as the
JUJU_UNIT_NAME environment variable, that we can't change, so we're
introducing gratuitous inconsistencies that we can't avoid with our
renaming.

I suggest dropping our idea of replacing what the Name methods mean, and
instead doing a simpler change that introduces a Key method that returns
the equivalent of PathKey today, and dropping PathKey:

machine.Key() => "machine-0"
unit.Key() => "unit-wordpress-0"
service.Key() => "service-wordpress"

We can also have PrincipalKey in state.Info.

https://codereview.appspot.com/6585053/

Unmerged revisions

608. By Roger Peppe on 2012-10-01

state: finish rename Unit.Name -> Unit.Id

607. By Roger Peppe on 2012-10-01

all: rename state.Unit.Name to state.Unit.Id

606. By Roger Peppe on 2012-10-01

state: rename Unit.Name to Unit.Id

605. By Roger Peppe on 2012-10-01

cmd/juju: implement upgrade-juju --bump-version

R=niemeyer
CC=
https://codereview.appspot.com/6593053

604. By Dave Cheney on 2012-10-01

cmd/juju: add scp subcommand

R=niemeyer
CC=
https://codereview.appspot.com/6565055

603. By William Reade on 2012-09-28

state: add CharmURL() to Service

R=niemeyer
CC=
https://codereview.appspot.com/6570064

602. By William Reade on 2012-09-28

uniter: faster tests

In each case, without regular StartSyncing, the Uniter just hangs around
waiting for events that... well, they come sooner or later, and the timeouts
are long so the tests are reliable. But I think that, in the interests of
sanity, it's better to make the environment unrealistically eager.

R=niemeyer
CC=
https://codereview.appspot.com/6574061

601. By Gustavo Niemeyer on 2012-09-28

state,worker: full lifecycle machines watcher

MachinesWatcher now returns a list of ids that have had their
lifecycle changed in any way. It's up to the consumer to work
out details.

R=TheMue, aram
CC=
https://codereview.appspot.com/6566066

600. By Roger Peppe on 2012-09-28

version: add Build number

This will be used to implement upgrade --bump-version.

R=niemeyer
CC=
https://codereview.appspot.com/6560066

599. By Roger Peppe on 2012-09-28

machiner: eat provisioner and firewaller for lunch

Three in one.

R=fwereade, niemeyer
CC=
https://codereview.appspot.com/6570063

=== modified file 'cmd/juju/deploy_test.go'
--- cmd/juju/deploy_test.go	2012-09-22 23:33:31 +0000
+++ cmd/juju/deploy_test.go	2012-10-01 17:54:22 +0000
@@ -81,24 +81,24 @@
 }
 
 func (s *repoSuite) assertUnitMachines(c *C, units []*state.Unit) {
-	expectUnitNames := []string{}
+	expectUnitIds := []string{}
 	for _, u := range units {
-		expectUnitNames = append(expectUnitNames, u.Name())
+		expectUnitIds = append(expectUnitIds, u.Id())
 	}
-	sort.Strings(expectUnitNames)
+	sort.Strings(expectUnitIds)
 
 	machines, err := s.State.AllMachines()
 	c.Assert(err, IsNil)
 	c.Assert(machines, HasLen, len(units))
-	unitNames := []string{}
+	unitIds := []string{}
 	for _, m := range machines {
 		mUnits, err := m.Units()
 		c.Assert(err, IsNil)
 		c.Assert(mUnits, HasLen, 1)
-		unitNames = append(unitNames, mUnits[0].Name())
+		unitIds = append(unitIds, mUnits[0].Id())
 	}
-	sort.Strings(unitNames)
-	c.Assert(unitNames, DeepEquals, expectUnitNames)
+	sort.Strings(unitIds)
+	c.Assert(unitIds, DeepEquals, expectUnitIds)
 }
 
 type DeploySuite struct {

=== modified file 'cmd/juju/status.go'
--- cmd/juju/status.go	2012-09-21 18:41:24 +0000
+++ cmd/juju/status.go	2012-10-01 17:54:22 +0000
@@ -194,7 +194,7 @@
 func processUnits(units []*state.Unit) (map[string]interface{}, error) {
 	r := m()
 	for _, unit := range units {
-		r[unit.Name()] = checkError(processUnit(unit))
+		r[unit.Id()] = checkError(processUnit(unit))
 	}
 	return r, nil
 }

=== modified file 'cmd/jujuc/server/context.go'
--- cmd/jujuc/server/context.go	2012-09-24 11:59:53 +0000
+++ cmd/jujuc/server/context.go	2012-10-01 17:54:22 +0000
@@ -90,7 +90,7 @@
 		"CHARM_DIR=" + charmDir,
 		"JUJU_CONTEXT_ID=" + ctx.Id,
 		"JUJU_AGENT_SOCKET=" + socketPath,
-		"JUJU_UNIT_NAME=" + ctx.Unit.Name(),
+		"JUJU_UNIT_NAME=" + ctx.Unit.Id(),
 	}
 	if ctx.RelationId != -1 {
 		vars = append(vars, "JUJU_RELATION="+ctx.envRelation())

=== modified file 'cmd/jujuc/server/context_test.go'
--- cmd/jujuc/server/context_test.go	2012-09-24 21:19:46 +0000
+++ cmd/jujuc/server/context_test.go	2012-10-01 17:54:22 +0000
@@ -320,12 +320,12 @@
 	node0, err = s.relctxs[0].Settings()
 	c.Assert(err, IsNil)
 	c.Assert(node0.Map(), DeepEquals, map[string]interface{}{
-		"private-address": "u-0.example.com",
+		"private-address": "unit-u-0.example.com",
 	})
 	node1, err = s.relctxs[1].Settings()
 	c.Assert(err, IsNil)
 	c.Assert(node1.Map(), DeepEquals, map[string]interface{}{
-		"private-address": "u-0.example.com",
+		"private-address": "unit-u-0.example.com",
 	})
 
 	// Check that the changes have been written to state.
@@ -352,13 +352,13 @@
 	node0, err = s.relctxs[0].Settings()
 	c.Assert(err, IsNil)
 	c.Assert(node0.Map(), DeepEquals, map[string]interface{}{
-		"private-address": "u-0.example.com",
+		"private-address": "unit-u-0.example.com",
 		"baz":             3,
 	})
 	node1, err = s.relctxs[1].Settings()
 	c.Assert(err, IsNil)
 	c.Assert(node1.Map(), DeepEquals, map[string]interface{}{
-		"private-address": "u-0.example.com",
+		"private-address": "unit-u-0.example.com",
 		"qux":             4,
 	})
 
@@ -392,7 +392,7 @@
 	c.Assert(err, IsNil)
 	unit, err := s.svc.AddUnit()
 	c.Assert(err, IsNil)
-	unit.SetPrivateAddress("u-0.example.com")
+	unit.SetPrivateAddress("unit-u-0.example.com")
 	c.Assert(err, IsNil)
 	s.ru, err = s.rel.Unit(unit)
 	c.Assert(err, IsNil)

=== modified file 'cmd/jujuc/server/juju-log.go'
--- cmd/jujuc/server/juju-log.go	2012-08-06 16:41:17 +0000
+++ cmd/jujuc/server/juju-log.go	2012-10-01 17:54:22 +0000
@@ -37,7 +37,7 @@
 }
 
 func (c *JujuLogCommand) Run(_ *cmd.Context) error {
-	s := []string{c.Unit.Name()}
+	s := []string{c.Unit.Id()}
 	if c.RelationId != -1 {
 		s = append(s, c.envRelationId())
 	}

=== modified file 'cmd/jujuc/server/relation-get.go'
--- cmd/jujuc/server/relation-get.go	2012-08-07 14:22:28 +0000
+++ cmd/jujuc/server/relation-get.go	2012-10-01 17:54:22 +0000
@@ -65,7 +65,7 @@
 
 func (c *RelationGetCommand) Run(ctx *cmd.Context) error {
 	var settings map[string]interface{}
-	if c.UnitName == c.Unit.Name() {
+	if c.UnitName == c.Unit.Id() {
 		node, err := c.Relations[c.RelationId].Settings()
 		if err != nil {
 			return err

=== modified file 'cmd/jujuc/server/util_test.go'
--- cmd/jujuc/server/util_test.go	2012-09-21 09:32:11 +0000
+++ cmd/jujuc/server/util_test.go	2012-10-01 17:54:22 +0000
@@ -10,7 +10,6 @@
 	"launchpad.net/juju-core/juju/testing"
 	"launchpad.net/juju-core/state"
 	coretesting "launchpad.net/juju-core/testing"
-	"strings"
 	stdtesting "testing"
 )
 
@@ -51,8 +50,7 @@
 func (s *HookContextSuite) AddUnit(c *C) *state.Unit {
 	unit, err := s.service.AddUnit()
 	c.Assert(err, IsNil)
-	name := strings.Replace(unit.Name(), "/", "-", 1)
-	err = unit.SetPrivateAddress(name + ".example.com")
+	err = unit.SetPrivateAddress(unit.PathKey() + ".example.com")
 	c.Assert(err, IsNil)
 	return unit
 }

=== modified file 'cmd/jujud/unit.go'
--- cmd/jujud/unit.go	2012-09-28 16:10:09 +0000
+++ cmd/jujud/unit.go	2012-10-01 17:54:22 +0000
@@ -92,7 +92,7 @@
 		return err
 	}
 	return runTasks(a.tomb.Dying(),
-		uniter.NewUniter(st, unit.Name(), a.Conf.DataDir),
+		uniter.NewUniter(st, unit.Id(), a.Conf.DataDir),
 		NewUpgrader(st, unit, a.Conf.DataDir),
 	)
 }

=== modified file 'cmd/jujud/unit_test.go'
--- cmd/jujud/unit_test.go	2012-09-28 15:33:46 +0000
+++ cmd/jujud/unit_test.go	2012-10-01 17:54:22 +0000
@@ -102,7 +102,7 @@
 
 	return &UnitAgent{
 		Conf:     AgentConf{dataDir, *s.StateInfo(c)},
-		UnitName: unit.Name(),
+		UnitName: unit.Id(),
 	}, unit, tools
 }
 
@@ -132,7 +132,7 @@
 	dataDir := c.MkDir()
 	a := &UnitAgent{
 		Conf:     AgentConf{dataDir, *s.StateInfo(c)},
-		UnitName: unit.Name(),
+		UnitName: unit.Id(),
 	}
 	err = runWithTimeout(a)
 	c.Assert(err, IsNil)
@@ -142,7 +142,7 @@
 	c.Assert(err, IsNil)
 	a = &UnitAgent{
 		Conf:     AgentConf{dataDir, *s.StateInfo(c)},
-		UnitName: unit.Name(),
+		UnitName: unit.Id(),
 	}
 	err = runWithTimeout(a)
 	c.Assert(err, IsNil)

=== modified file 'container/container.go'
--- container/container.go	2012-09-21 11:53:49 +0000
+++ container/container.go	2012-10-01 17:54:22 +0000
@@ -67,11 +67,11 @@
 		filepath.Join(toolsDir, "jujud"),
 		strings.Join(info.Addrs, ","),
 		filepath.Join("/var/log/juju", unit.PathKey()+".log"),
-		unit.Name())
+		unit.Id())
 
 	conf := &upstart.Conf{
 		Service: *c.service(unit),
-		Desc:    "juju unit agent for " + unit.Name(),
+		Desc:    "juju unit agent for " + unit.Id(),
 		Cmd:     cmd,
 	}
 	dir := c.dirName(unit)

=== modified file 'state/assign_test.go'
--- state/assign_test.go	2012-09-28 07:21:43 +0000
+++ state/assign_test.go	2012-10-01 17:54:22 +0000
@@ -471,7 +471,7 @@
 		units, err := m.Units()
 		c.Assert(err, IsNil)
 		c.Assert(units, HasLen, 1)
-		c.Assert(units[0].Name(), Equals, unit.Name())
+		c.Assert(units[0].Id(), Equals, unit.Id())
 	}
 
 	// Remove units from alternate machines.

=== modified file 'state/life_test.go'
--- state/life_test.go	2012-09-28 07:21:43 +0000
+++ state/life_test.go	2012-10-01 17:54:22 +0000
@@ -108,7 +108,7 @@
 }
 
 func (l *unitLife) id() (coll string, id interface{}) {
-	return "units", l.unit.Name()
+	return "units", l.unit.Id()
 }
 
 func (l *unitLife) setup(s *LifeSuite, c *C) state.Living {

=== modified file 'state/machine.go'
--- state/machine.go	2012-09-28 15:43:25 +0000
+++ state/machine.go	2012-10-01 17:54:22 +0000
@@ -186,7 +186,7 @@
 	for _, pudoc := range pudocs {
 		units = append(units, newUnit(m.st, &pudoc))
 		docs := []unitDoc{}
-		err = m.st.units.Find(D{{"principal", pudoc.Name}}).All(&docs)
+		err = m.st.units.Find(D{{"principal", pudoc.Id}}).All(&docs)
 		if err != nil {
 			return nil, err
 		}

=== modified file 'state/machine_test.go'
--- state/machine_test.go	2012-09-28 16:38:16 +0000
+++ state/machine_test.go	2012-10-01 17:54:22 +0000
@@ -235,18 +235,18 @@
 		c.Logf("test %d", i)
 		got, err := a.machine.Units()
 		c.Assert(err, IsNil)
-		expect := sortedUnitNames(append(a.units, a.subordinates...))
-		c.Assert(sortedUnitNames(got), DeepEquals, expect)
+		expect := sortedUnitIds(append(a.units, a.subordinates...))
+		c.Assert(sortedUnitIds(got), DeepEquals, expect)
 	}
 }
 
-func sortedUnitNames(units []*state.Unit) []string {
-	names := make([]string, len(units))
+func sortedUnitIds(units []*state.Unit) []string {
+	ids := make([]string, len(units))
 	for i, u := range units {
-		names[i] = u.Name()
+		ids[i] = u.Id()
 	}
-	sort.Strings(names)
-	return names
+	sort.Strings(ids)
+	return ids
 }
 
 type machineInfo struct {
@@ -553,12 +553,12 @@
 	sort.Sort(unitSlice(change.Removed))
 	var got []string
 	for _, g := range change.Added {
-		got = append(got, g.Name())
+		got = append(got, g.Id())
 	}
 	c.Assert(got, DeepEquals, added)
 	got = nil
 	for _, g := range change.Removed {
-		got = append(got, g.Name())
+		got = append(got, g.Id())
 	}
 	c.Assert(got, DeepEquals, removed)
 }

=== modified file 'state/relation.go'
--- state/relation.go	2012-09-26 13:43:28 +0000
+++ state/relation.go	2012-10-01 17:54:22 +0000
@@ -192,7 +192,7 @@
 	if ep.RelationScope == charm.ScopeContainer {
 		container := u.doc.Principal
 		if container == "" {
-			container = u.doc.Name
+			container = u.doc.Id
 		}
 		scope = append(scope, container)
 	}
@@ -235,7 +235,7 @@
 	if err != nil {
 		return err
 	}
-	key, err := ru.key(ru.unit.Name())
+	key, err := ru.key(ru.unit.Id())
 	if err != nil {
 		return err
 	}
@@ -257,7 +257,7 @@
 
 // LeaveScope signals that the unit has left its scope in the relation.
 func (ru *RelationUnit) LeaveScope() error {
-	key, err := ru.key(ru.unit.Name())
+	key, err := ru.key(ru.unit.Id())
 	if err != nil {
 		return err
 	}
@@ -274,13 +274,13 @@
 func (ru *RelationUnit) WatchScope() *RelationScopeWatcher {
 	role := ru.endpoint.RelationRole.counterpartRole()
 	scope := ru.scope + "#" + string(role)
-	return newRelationScopeWatcher(ru.st, scope, ru.unit.Name())
+	return newRelationScopeWatcher(ru.st, scope, ru.unit.Id())
 }
 
 // Settings returns a ConfigNode which allows access to the unit's settings
 // within the relation.
 func (ru *RelationUnit) Settings() (*ConfigNode, error) {
-	key, err := ru.key(ru.unit.Name())
+	key, err := ru.key(ru.unit.Id())
 	if err != nil {
 		return nil, err
 	}

=== modified file 'state/relation_test.go'
--- state/relation_test.go	2012-09-26 14:18:25 +0000
+++ state/relation_test.go	2012-10-01 17:54:22 +0000
@@ -6,7 +6,6 @@
 	"launchpad.net/juju-core/charm"
 	"launchpad.net/juju-core/state"
 	"sort"
-	"strings"
 	"time"
 )
 
@@ -316,7 +315,7 @@
 	s.assertNoScopeChange(c, w0)
 	node, err := pr.ru0.Settings()
 	c.Assert(err, IsNil)
-	c.Assert(node.Map(), DeepEquals, map[string]interface{}{"private-address": "peer-0.example.com"})
+	c.Assert(node.Map(), DeepEquals, map[string]interface{}{"private-address": "unit-peer-0.example.com"})
 
 	// ru1 enters; check change is observed.
 	err = pr.ru1.EnterScope()
@@ -577,7 +576,7 @@
 		u, err = svc.AddUnitSubordinateTo(principal)
 	}
 	c.Assert(err, IsNil)
-	addr := fmt.Sprintf("%s.example.com", strings.Replace(u.Name(), "/", "-", 1))
+	addr := fmt.Sprintf("%s.example.com", u.PathKey())
 	err = u.SetPrivateAddress(addr)
 	c.Assert(err, IsNil)
 	ru, err := rel.Unit(u)

=== modified file 'state/service.go'
--- state/service.go	2012-09-28 22:42:17 +0000
+++ state/service.go	2012-10-01 17:54:22 +0000
@@ -170,7 +170,7 @@
 // addUnit adds the named unit.
 func (s *Service) addUnit(name string, principal *Unit) (*Unit, error) {
 	udoc := &unitDoc{
-		Name:    name,
+		Id:      name,
 		Service: s.doc.Name,
 		Life:    Alive,
 	}
@@ -186,10 +186,10 @@
 		Update: D{{"$inc", D{{"unitcount", 1}}}},
 	}}
 	if principal != nil {
-		udoc.Principal = principal.Name()
+		udoc.Principal = principal.Id()
 		ops = append(ops, txn.Op{
 			C:      s.st.units.Name,
-			Id:     principal.Name(),
+			Id:     principal.Id(),
 			Assert: isAlive,
 			Update: D{{"$addToSet", D{{"subordinates", name}}}},
 		})
@@ -269,7 +269,7 @@
 	}
 	ops := []txn.Op{{
 		C:      s.st.units.Name,
-		Id:     u.doc.Name,
+		Id:     u.doc.Id,
 		Assert: D{{"life", Dead}},
 		Remove: true,
 	}, {
@@ -283,14 +283,14 @@
 			C:      s.st.units.Name,
 			Id:     u.doc.Principal,
 			Assert: txn.DocExists,
-			Update: D{{"$pull", D{{"subordinates", u.doc.Name}}}},
+			Update: D{{"$pull", D{{"subordinates", u.Id()}}}},
 		})
 	}
 	// TODO assert that subordinates are empty before deleting
 	// a principal
 	if err = s.st.runner.Run(ops, "", nil); err != nil {
 		// TODO Remove this once we know the logic is right:
-		if c, err := s.st.units.FindId(u.doc.Name).Count(); err != nil {
+		if c, err := s.st.units.FindId(u.Id()).Count(); err != nil {
 			return err
 		} else if c > 0 {
 			return fmt.Errorf("cannot remove unit; something smells bad")

=== modified file 'state/service_test.go'
--- state/service_test.go	2012-09-28 16:53:40 +0000
+++ state/service_test.go	2012-10-01 17:54:22 +0000
@@ -140,12 +140,12 @@
 	// Check that principal units can be added on their own.
 	unitZero, err := s.service.AddUnit()
 	c.Assert(err, IsNil)
-	c.Assert(unitZero.Name(), Equals, "mysql/0")
+	c.Assert(unitZero.Id(), Equals, "mysql/0")
 	principal := unitZero.IsPrincipal()
 	c.Assert(principal, Equals, true)
 	unitOne, err := s.service.AddUnit()
 	c.Assert(err, IsNil)
-	c.Assert(unitOne.Name(), Equals, "mysql/1")
+	c.Assert(unitOne.Id(), Equals, "mysql/1")
 	principal = unitOne.IsPrincipal()
 	c.Assert(principal, Equals, true)
 
@@ -167,7 +167,7 @@
 	// Check that subordinate units can be added to principal units
 	subZero, err := logging.AddUnitSubordinateTo(unitZero)
 	c.Assert(err, IsNil)
-	c.Assert(subZero.Name(), Equals, "logging/0")
+	c.Assert(subZero.Id(), Equals, "logging/0")
 	principal = subZero.IsPrincipal()
 	c.Assert(principal, Equals, false)
 
@@ -201,12 +201,12 @@
 	// Check that retrieving a unit from the service works correctly.
 	unit, err := s.service.Unit("mysql/0")
 	c.Assert(err, IsNil)
-	c.Assert(unit.Name(), Equals, "mysql/0")
+	c.Assert(unit.Id(), Equals, "mysql/0")
 
 	// Check that retrieving a unit from state works correctly.
 	unit, err = s.State.Unit("mysql/0")
 	c.Assert(err, IsNil)
-	c.Assert(unit.Name(), Equals, "mysql/0")
+	c.Assert(unit.Id(), Equals, "mysql/0")
 
 	// Check that retrieving a non-existent or an invalidly
 	// named unit fail nicely.
@@ -237,7 +237,7 @@
 
 	units, err := s.service.AllUnits()
 	c.Assert(err, IsNil)
-	c.Assert(sortedUnitNames(units), DeepEquals, []string{"mysql/0", "mysql/1"})
+	c.Assert(sortedUnitIds(units), DeepEquals, []string{"mysql/0", "mysql/1"})
 }
 
 func (s *ServiceSuite) TestReadUnitWhenDying(c *C) {
@@ -287,7 +287,7 @@
 	units, err := s.service.AllUnits()
 	c.Assert(err, IsNil)
 	c.Assert(units, HasLen, 1)
-	c.Assert(units[0].Name(), Equals, "mysql/1")
+	c.Assert(units[0].Id(), Equals, "mysql/1")
 
 	err = s.service.RemoveUnit(unit)
 	c.Assert(err, IsNil)
@@ -518,7 +518,7 @@
 
 func (m unitSlice) Len() int           { return len(m) }
 func (m unitSlice) Swap(i, j int)      { m[i], m[j] = m[j], m[i] }
-func (m unitSlice) Less(i, j int) bool { return m[i].Name() < m[j].Name() }
+func (m unitSlice) Less(i, j int) bool { return m[i].Id() < m[j].Id() }
 
 func assertSameServiceUnits(c *C, change *state.ServiceUnitsChange, added, removed []string) {
 	c.Assert(change, NotNil)
@@ -528,16 +528,17 @@
 	if len(removed) == 0 {
 		removed = nil
 	}
+	// TODO use sortedUnitIds
 	sort.Sort(unitSlice(change.Added))
 	sort.Sort(unitSlice(change.Removed))
 	var got []string
 	for _, g := range change.Added {
-		got = append(got, g.Name())
+		got = append(got, g.Id())
 	}
 	c.Assert(got, DeepEquals, added)
 	got = nil
 	for _, g := range change.Removed {
-		got = append(got, g.Name())
+		got = append(got, g.Id())
 	}
 	c.Assert(got, DeepEquals, removed)
 }

=== modified file 'state/unit.go'
--- state/unit.go	2012-09-28 15:43:25 +0000
+++ state/unit.go	2012-10-01 17:54:22 +0000
@@ -67,7 +67,7 @@
 
 // unitDoc represents the internal state of a unit in MongoDB.
 type unitDoc struct {
-	Name           string `bson:"_id"`
+	Id             string `bson:"_id"`
 	Service        string
 	CharmURL       *charm.URL
 	Principal      string
@@ -102,17 +102,17 @@
 
 // String returns the unit as string.
 func (u *Unit) String() string {
-	return u.doc.Name
+	return u.doc.Id
 }
 
-// Name returns the unit name.
-func (u *Unit) Name() string {
-	return u.doc.Name
+// Id returns the unit name.
+func (u *Unit) Id() string {
+	return u.doc.Id
 }
 
 // globalKey returns the global database key for the unit.
 func (u *Unit) globalKey() string {
-	return "u#" + u.doc.Name
+	return "u#" + u.doc.Id
 }
 
 // Life returns whether the unit is Alive, Dying or Dead.
@@ -138,7 +138,7 @@
 	}
 	ops := []txn.Op{{
 		C:      u.st.units.Name,
-		Id:     u.doc.Name,
+		Id:     u.doc.Id,
 		Assert: notDead,
 		Update: D{{"$set", D{{"tools", t}}}},
 	}}
@@ -153,7 +153,7 @@
 // EnsureDying sets the unit lifecycle to Dying if it is Alive.
 // It does nothing otherwise.
 func (u *Unit) EnsureDying() error {
-	err := ensureDying(u.st, u.st.units, u.doc.Name, "unit")
+	err := ensureDying(u.st, u.st.units, u.doc.Id, "unit")
 	if err != nil {
 		return err
 	}
@@ -164,7 +164,7 @@
 // EnsureDead sets the unit lifecycle to Dead if it is Alive or Dying.
 // It does nothing otherwise.
 func (u *Unit) EnsureDead() error {
-	err := ensureDead(u.st, u.st.units, u.doc.Name, "unit", nil, "")
+	err := ensureDead(u.st, u.st.units, u.doc.Id, "unit", nil, "")
 	if err != nil {
 		return err
 	}
@@ -202,7 +202,7 @@
 // Refresh refreshes the contents of the Unit from the underlying
 // state. It returns a NotFoundError if the unit has been removed.
 func (u *Unit) Refresh() error {
-	err := u.st.units.FindId(u.doc.Name).One(&u.doc)
+	err := u.st.units.FindId(u.doc.Id).One(&u.doc)
 	if err == mgo.ErrNotFound {
 		return notFound("unit %q", u)
 	}
@@ -268,7 +268,7 @@
 	defer trivial.ErrorContextf(&err, "cannot open port %v for unit %q", port, u)
 	ops := []txn.Op{{
 		C:      u.st.units.Name,
-		Id:     u.doc.Name,
+		Id:     u.doc.Id,
 		Assert: notDead,
 		Update: D{{"$addToSet", D{{"ports", port}}}},
 	}}
@@ -294,7 +294,7 @@
 	defer trivial.ErrorContextf(&err, "cannot close port %v for unit %q", port, u)
 	ops := []txn.Op{{
 		C:      u.st.units.Name,
-		Id:     u.doc.Name,
+		Id:     u.doc.Id,
 		Assert: notDead,
 		Update: D{{"$pull", D{{"ports", port}}}},
 	}}
@@ -331,7 +331,7 @@
 func (u *Unit) SetCharm(ch *Charm) (err error) {
 	ops := []txn.Op{{
 		C:      u.st.units.Name,
-		Id:     u.doc.Name,
+		Id:     u.doc.Id,
 		Assert: notDead,
 		Update: D{{"$set", D{{"charmurl", ch.URL()}}}},
 	}}
@@ -351,7 +351,7 @@
 // file name.  The returned key will be different from other
 // PathKey values returned by any other entities from the same state.
 func (u *Unit) PathKey() string {
-	return "unit-" + strings.Replace(u.Name(), "/", "-", -1)
+	return "unit-" + strings.Replace(u.Id(), "/", "-", -1)
 }
 
 // WaitAgentAlive blocks until the respective agent is alive.
@@ -442,14 +442,14 @@
 	}
 	ops := []txn.Op{{
 		C:      u.st.units.Name,
-		Id:     u.doc.Name,
+		Id:     u.doc.Id,
 		Assert: assert,
 		Update: D{{"$set", D{{"machineid", m.doc.Id}}}},
 	}, {
 		C:      u.st.machines.Name,
 		Id:     m.doc.Id,
 		Assert: massert,
-		Update: D{{"$addToSet", D{{"principals", u.doc.Name}}}},
+		Update: D{{"$addToSet", D{{"principals", u.doc.Id}}}},
 	}}
 	err = u.st.runner.Run(ops, "", nil)
 	if err == nil {
@@ -459,7 +459,7 @@
 	if err != txn.ErrAborted {
 		return err
 	}
-	u0, err := u.st.Unit(u.Name())
+	u0, err := u.st.Unit(u.Id())
 	if err != nil {
 		return err
 	}
@@ -518,7 +518,7 @@
 	// machine id is as expected.
 	ops := []txn.Op{{
 		C:      u.st.units.Name,
-		Id:     u.doc.Name,
+		Id:     u.doc.Id,
 		Assert: txn.DocExists,
 		Update: D{{"$set", D{{"machineid", nil}}}},
 	}}
@@ -527,7 +527,7 @@
 			C:      u.st.machines.Name,
 			Id:     u.doc.MachineId,
 			Assert: txn.DocExists,
-			Update: D{{"$pull", D{{"principals", u.doc.Name}}}},
+			Update: D{{"$pull", D{{"principals", u.doc.Id}}}},
 		})
 	}
 	err = u.st.runner.Run(ops, "", nil)
@@ -542,7 +542,7 @@
 func (u *Unit) SetPublicAddress(address string) (err error) {
 	ops := []txn.Op{{
 		C:      u.st.units.Name,
-		Id:     u.doc.Name,
+		Id:     u.doc.Id,
 		Assert: txn.DocExists,
 		Update: D{{"$set", D{{"publicaddress", address}}}},
 	}}
@@ -557,7 +557,7 @@
 func (u *Unit) SetPrivateAddress(address string) error {
 	ops := []txn.Op{{
 		C:      u.st.units.Name,
-		Id:     u.doc.Name,
+		Id:     u.doc.Id,
 		Assert: txn.DocExists,
 		Update: D{{"$set", D{{"privateaddress", address}}}},
 	}}
@@ -584,7 +584,7 @@
 	assert := append(isAlive, D{{"resolved", ResolvedNone}}...)
 	ops := []txn.Op{{
 		C:      u.st.units.Name,
-		Id:     u.doc.Name,
+		Id:     u.doc.Id,
 		Assert: assert,
 		Update: D{{"$set", D{{"resolved", mode}}}},
 	}}
@@ -592,7 +592,7 @@
 		if err == txn.ErrAborted {
 			// Find which assertion failed so we can give a
 			// more specific error.
-			u1, err := u.st.Unit(u.Name())
+			u1, err := u.st.Unit(u.Id())
 			if err != nil {
 				return err
 			}
@@ -611,7 +611,7 @@
 func (u *Unit) ClearResolved() error {
 	ops := []txn.Op{{
 		C:      u.st.units.Name,
-		Id:     u.doc.Name,
+		Id:     u.doc.Id,
 		Assert: txn.DocExists,
 		Update: D{{"$set", D{{"resolved", ResolvedNone}}}},
 	}}

=== modified file 'state/unit_test.go'
--- state/unit_test.go	2012-09-27 14:22:57 +0000
+++ state/unit_test.go	2012-10-01 17:54:22 +0000
@@ -50,7 +50,7 @@
 }
 
 func (s *UnitSuite) TestRefresh(c *C) {
-	unit1, err := s.State.Unit(s.unit.Name())
+	unit1, err := s.State.Unit(s.unit.Id())
 	c.Assert(err, IsNil)
 
 	err = s.unit.SetPrivateAddress("example.local")
@@ -308,7 +308,7 @@
 	c.Assert(err, IsNil)
 
 	doc := make(map[string][]string)
-	s.ConnSuite.units.FindId(s.unit.Name()).One(&doc)
+	s.ConnSuite.units.FindId(s.unit.Id()).One(&doc)
 	subordinates, ok := doc["subordinates"]
 	if !ok {
 		c.Errorf(`unit document does not have a "subordinates" field`)
@@ -320,7 +320,7 @@
 	err = logService.RemoveUnit(su1)
 	c.Assert(err, IsNil)
 	doc = make(map[string][]string)
-	s.ConnSuite.units.FindId(s.unit.Name()).One(&doc)
+	s.ConnSuite.units.FindId(s.unit.Id()).One(&doc)
 	subordinates, ok = doc["subordinates"]
 	if !ok {
 		c.Errorf(`unit document does not have a "subordinates" field`)
@@ -364,7 +364,7 @@
 }
 
 func (s *UnitSuite) TestWatchUnit(c *C) {
-	altunit, err := s.State.Unit(s.unit.Name())
+	altunit, err := s.State.Unit(s.unit.Id())
 	c.Assert(err, IsNil)
 	err = altunit.SetPublicAddress("newer-address")
 	c.Assert(err, IsNil)
@@ -379,7 +379,7 @@
 	select {
 	case u, ok := <-w.Changes():
 		c.Assert(ok, Equals, true)
-		c.Assert(u.Name(), Equals, s.unit.Name())
+		c.Assert(u.Id(), Equals, s.unit.Id())
 		addr, err := u.PublicAddress()
 		c.Assert(err, IsNil)
 		c.Assert(addr, Equals, "newer-address")
@@ -395,7 +395,7 @@
 		select {
 		case unit, ok := <-w.Changes():
 			c.Assert(ok, Equals, true)
-			c.Assert(unit.Name(), Equals, s.unit.Name())
+			c.Assert(unit.Id(), Equals, s.unit.Id())
 			var info unitInfo
 			info.Life = unit.Life()
 			c.Assert(err, IsNil)

=== modified file 'state/watcher.go'
--- state/watcher.go	2012-09-28 17:20:29 +0000
+++ state/watcher.go	2012-10-01 17:54:22 +0000
@@ -455,7 +455,7 @@
 	}
 	for _, doc := range docs {
 		unit := newUnit(w.st, &doc)
-		w.knownUnits[doc.Name] = unit
+		w.knownUnits[doc.Id] = unit
 		changes.Added = append(changes.Added, unit)
 	}
 	return changes, nil
@@ -704,7 +704,7 @@
 	}
 	for _, doc := range docs {
 		unit := newUnit(w.st, &doc)
-		w.knownUnits[doc.Name] = unit
+		w.knownUnits[doc.Id] = unit
 		changes.Added = append(changes.Added, unit)
 	}
 	return changes, nil
@@ -1175,13 +1175,13 @@
 }
 
 func (w *UnitWatcher) loop(unit *Unit) (err error) {
-	name := unit.doc.Name
-	if unit, err = w.st.Unit(name); err != nil {
+	id := unit.doc.Id
+	if unit, err = w.st.Unit(id); err != nil {
 		return err
 	}
 	ch := make(chan watcher.Change)
-	w.st.watcher.Watch(w.st.units.Name, name, unit.doc.TxnRevno, ch)
-	defer w.st.watcher.Unwatch(w.st.units.Name, name, ch)
+	w.st.watcher.Watch(w.st.units.Name, id, unit.doc.TxnRevno, ch)
+	defer w.st.watcher.Unwatch(w.st.units.Name, id, ch)
 	for {
 		for unit != nil {
 			select {
@@ -1190,7 +1190,7 @@
 			case <-w.tomb.Dying():
 				return tomb.ErrDying
 			case <-ch:
-				if unit, err = w.st.Unit(name); err != nil {
+				if unit, err = w.st.Unit(id); err != nil {
 					return err
 				}
 			case w.changeChan <- unit:
@@ -1203,7 +1203,7 @@
 		case <-w.tomb.Dying():
 			return tomb.ErrDying
 		case <-ch:
-			if unit, err = w.st.Unit(name); err != nil {
+			if unit, err = w.st.Unit(id); err != nil {
 				return err
 			}
 		}

=== modified file 'worker/firewaller/firewaller.go'
--- worker/firewaller/firewaller.go	2012-09-27 23:26:16 +0000
+++ worker/firewaller/firewaller.go	2012-10-01 17:54:22 +0000
@@ -75,17 +75,17 @@
 		case change := <-fw.unitsChange:
 			changed := []*unitData{}
 			for _, unit := range change.Removed {
-				unitd, ok := fw.unitds[unit.Name()]
+				unitd, ok := fw.unitds[unit.Id()]
 				if !ok {
 					panic("trying to remove unit that was not added")
 				}
 				fw.forgetUnit(unitd)
 				changed = append(changed, unitd)
-				log.Debugf("firewaller: stopped watching unit %s", unit.Name())
+				log.Debugf("firewaller: stopped watching unit %s", unit.Id())
 			}
 			for _, unit := range change.Added {
 				unitd := newUnitData(unit, fw)
-				fw.unitds[unit.Name()] = unitd
+				fw.unitds[unit.Id()] = unitd
 				machineId, err := unit.AssignedMachineId()
 				if err != nil {
 					fw.tomb.Kill(err)
@@ -94,7 +94,7 @@
 					panic("machine of added unit is not watched")
 				}
 				unitd.machined = fw.machineds[machineId]
-				unitd.machined.unitds[unit.Name()] = unitd
+				unitd.machined.unitds[unit.Id()] = unitd
 				if fw.serviceds[unit.ServiceName()] == nil {
 					service, err := fw.st.Service(unit.ServiceName())
 					if err != nil {
@@ -103,9 +103,9 @@
 					fw.serviceds[unit.ServiceName()] = newServiceData(service, fw)
 				}
 				unitd.serviced = fw.serviceds[unit.ServiceName()]
-				unitd.serviced.unitds[unit.Name()] = unitd
+				unitd.serviced.unitds[unit.Id()] = unitd
 				changed = append(changed, unitd)
-				log.Debugf("firewaller: started watching unit %s", unit.Name())
+				log.Debugf("firewaller: started watching unit %s", unit.Id())
 			}
 			if err := fw.flushUnits(changed); err != nil {
 				return fmt.Errorf("cannot change firewall ports: %v", err)
@@ -247,7 +247,7 @@
 
 // forgetUnit cleans the unit data after the unit is removed.
 func (fw *Firewaller) forgetUnit(unitd *unitData) {
-	name := unitd.unit.Name()
+	name := unitd.unit.Id()
 	serviced := unitd.serviced
 	machined := unitd.machined
 	if err := unitd.Stop(); err != nil {

=== modified file 'worker/machiner/machiner.go'
--- worker/machiner/machiner.go	2012-09-28 15:27:47 +0000
+++ worker/machiner/machiner.go	2012-10-01 17:54:22 +0000
@@ -63,7 +63,7 @@
 			for _, u := range change.Removed {
 				if u.IsPrincipal() {
 					if err := m.localContainer.Destroy(u); err != nil {
-						log.Printf("cannot destroy unit %s: %v", u.Name(), err)
+						log.Printf("cannot destroy unit %s: %v", u.Id(), err)
 					}
 				}
 			}
@@ -71,7 +71,7 @@
 				if u.IsPrincipal() {
 					if err := m.localContainer.Deploy(u, m.stateInfo, m.tools); err != nil {
 						// TODO put unit into a queue to retry the deploy.
-						log.Printf("cannot deploy unit %s: %v", u.Name(), err)
+						log.Printf("cannot deploy unit %s: %v", u.Id(), err)
 					}
 				}
 			}

=== modified file 'worker/machiner/machiner_test.go'
--- worker/machiner/machiner_test.go	2012-09-28 07:21:43 +0000
+++ worker/machiner/machiner_test.go	2012-10-01 17:54:22 +0000
@@ -131,12 +131,12 @@
 	d.c.Check(info, Equals, d.expectedInfo)
 	d.c.Check(tools, DeepEquals, d.expectedTools)
 
-	d.action <- "+" + u.Name()
+	d.action <- "+" + u.Id()
 	return nil
 }
 
 func (d *dummyContainer) Destroy(u *state.Unit) error {
-	d.action <- "-" + u.Name()
+	d.action <- "-" + u.Id()
 	return nil
 }
 

=== modified file 'worker/uniter/relationer_test.go'
--- worker/uniter/relationer_test.go	2012-09-25 16:21:48 +0000
+++ worker/uniter/relationer_test.go	2012-10-01 17:54:22 +0000
@@ -8,7 +8,6 @@
 	"launchpad.net/juju-core/worker/uniter"
 	"launchpad.net/juju-core/worker/uniter/hook"
 	"launchpad.net/juju-core/worker/uniter/relation"
-	"strings"
 	"time"
 )
 
@@ -39,11 +38,11 @@
 	s.hooks = make(chan hook.Info)
 }
 
-func (s *RelationerSuite) AddRelationUnit(c *C, name string) *state.RelationUnit {
+func (s *RelationerSuite) AddRelationUnit(c *C, id string) *state.RelationUnit {
 	u, err := s.svc.AddUnit()
 	c.Assert(err, IsNil)
-	c.Assert(u.Name(), Equals, name)
-	err = u.SetPrivateAddress(strings.Replace(name, "/", "-", 1) + ".example.com")
+	c.Assert(u.Id(), Equals, id)
+	err = u.SetPrivateAddress(u.PathKey() + ".example.com")
 	c.Assert(err, IsNil)
 	ru, err := s.rel.Unit(u)
 	c.Assert(err, IsNil)
@@ -134,14 +133,14 @@
 		Kind:       hook.RelationJoined,
 		RemoteUnit: "u/1",
 		Members: map[string]map[string]interface{}{
-			"u/1": {"private-address": "u-1.example.com"},
+			"u/1": {"private-address": "unit-u-1.example.com"},
 		},
 	})
 	s.assertHook(c, hook.Info{
 		Kind:       hook.RelationChanged,
 		RemoteUnit: "u/1",
 		Members: map[string]map[string]interface{}{
-			"u/1": {"private-address": "u-1.example.com"},
+			"u/1": {"private-address": "unit-u-1.example.com"},
 		},
 	})
 	s.assertNoHook(c)
@@ -224,7 +223,7 @@
 		Kind:       hook.RelationJoined,
 		RemoteUnit: "u/1",
 		Members: map[string]map[string]interface{}{
-			"u/1": {"private-address": "u-1.example.com"},
+			"u/1": {"private-address": "unit-u-1.example.com"},
 		},
 	}
 	name, err := r.PrepareHook(joined)
@@ -266,7 +265,7 @@
 	c.Assert(ctx.Units(), DeepEquals, []string{"u/1"})
 	s1, err = ctx.ReadSettings("u/1")
 	c.Assert(err, IsNil)
-	c.Assert(s1, DeepEquals, map[string]interface{}{"private-address": "u-1.example.com"})
+	c.Assert(s1, DeepEquals, map[string]interface{}{"private-address": "unit-u-1.example.com"})
 
 	// ...and commit it.
 	err = r.CommitHook(changed)
@@ -306,7 +305,7 @@
 		Kind:       hook.RelationJoined,
 		RemoteUnit: "u/1",
 		Members: map[string]map[string]interface{}{
-			"u/1": {"private-address": "u-1.example.com"},
+			"u/1": {"private-address": "unit-u-1.example.com"},
 		},
 	})
 

=== modified file 'worker/uniter/uniter.go'
--- worker/uniter/uniter.go	2012-09-28 15:27:47 +0000
+++ worker/uniter/uniter.go	2012-10-01 17:54:22 +0000
@@ -108,7 +108,7 @@
 }
 
 func (u *Uniter) String() string {
-	return "uniter for " + u.unit.Name()
+	return "uniter for " + u.unit.Id()
 }
 
 func (u *Uniter) Dying() <-chan struct{} {
@@ -193,7 +193,7 @@
 		panic("relation hooks are not yet supported")
 		// TODO: update relation context; get hook name.
 	}
-	hctxId := fmt.Sprintf("%s:%s:%d", u.unit.Name(), hookName, u.rand.Int63())
+	hctxId := fmt.Sprintf("%s:%s:%d", u.unit.Id(), hookName, u.rand.Int63())
 	hctx := server.HookContext{
 		Service:    u.service,
 		Unit:       u.unit,