Merge lp:~thumper/juju-core/provisioner-reget-state-info into lp:~juju/juju-core/trunk

[Tim Penhey]

Provisioner gets addresses for each new machine.

The addresses are retrieved each time we attempt to start a machine. This
allows the provisioner to take advantage of any new api or state servers that
are started as part of a block of machines being provisioned at one time.

There is some saving of addresses at this stage to make the existing tests
pass, even though this goes a little against what William has mentioned
before. If we have invalid config, the tests assume that we continue with old
info, whereas William has suggested that we pause provisioning while the
config is broken.

One behavioural tweak in this branch. If we do fail to setup authentication,
we no longer kill the task, but instead log and continue. We will attempt to
start it again next time through the loop.

https://codereview.appspot.com/9824047/

[Tim Penhey (thumper)]

Reviewers: mp+168562_code.launchpad.net,

Message:
Please take a look.

Description:
Provisioner gets addresses for each new machine.

The addresses are retrieved each time we attempt to start a machine.
This
allows the provisioner to take advantage of any new api or state servers
that
are started as part of a block of machines being provisioned at one
time.

There is some saving of addresses at this stage to make the existing
tests
pass, even though this goes a little against what William has mentioned
before. If we have invalid config, the tests assume that we continue
with old
info, whereas William has suggested that we pause provisioning while the
config is broken.

One behavioural tweak in this branch. If we do fail to setup
authentication,
we no longer kill the task, but instead log and continue. We will
attempt to
start it again next time through the loop.

https://code.launchpad.net/~thumper/juju-core/provisioner-reget-state-info/+merge/168562

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M worker/provisioner/export_test.go
   M worker/provisioner/provisioner.go
   M worker/provisioner/provisioner_task.go

[William Reade (fwereade)]

Strongly -1 on the clever bits here. Just barf on failure and I'll be
happy :).

https://codereview.appspot.com/9824047/diff/1/worker/provisioner/provisioner_task.go
File worker/provisioner/provisioner_task.go (right):

https://codereview.appspot.com/9824047/diff/1/worker/provisioner/provisioner_task.go#newcode304
worker/provisioner/provisioner_task.go:304: // Don't return a real
error, just try again next time.
This is problematic, because the watcher won't report that machine again
until it changes -- which it won't. For this to work, we need the
provisioner to be restarted regularly... but I'm hoping for a
provisioner that can run for weeks without breaking sweat.

https://codereview.appspot.com/9824047/diff/1/worker/provisioner/provisioner_task.go#newcode360
worker/provisioner/provisioner_task.go:360: // info if config becomes
invalid.
Honestly, I think this is sufficient justification to nuke the whole
provisioner; it'll be restarted in a few seconds, and for now I'd rather
not even try to provision a machine (which can be expected to cost our
users actual money) unless I'm as sure as I can be that I'll be able to
start a functioning machine agent...

https://codereview.appspot.com/9824047/

[William Reade (fwereade)]

Re what we do without a valid environment... hmm. Thinking aloud:

* we certainly start off with an invalid environ config, and only get a
valid one once whoever started the environment has connected for the
first time.

* today, on first startup, there will be one machine present; but it'll
be marked as provisioned already, so it won't cause env access directly;
and if we're tolerant of list-instances failures, we'll be able to
complete the first processMachines without difficulty. But I don't think
we can necessarily depend on that forever...

* in theory, it is impossible to set an invalid environ config in state,
except via the special mechanism at bootstrap time.

* in practice, an environ config can most certainly become invalid --
you can (1) replace the config with *any* valid config, which might ofc
be invalid in this context (changed environ type, for example, will
screw us most comprehensively) and (2) config update is racy anyway,
such that two users making valid changes concurrently can end up with an
environment config that reflects neither of their desires... and I bet
there's a way for that to cause invalidity. (oh, and, on inspection,
(3): it looks like values can't be dependably unset anyway. we probably
need that too. grar.)

* BUT the consequences of an invalid environ affect many more things
than just the provisioner.

* AND SO it would be folly to spend time and effort hardening the
provisioner against catastrophic environ failure when we could be fixing
the root cause.

* AND THUS you should (1) wait for a valid config at startup before
processing machines and (2) proceed otherwise as though the environment
configuration were always guaranteed consistent and valid from a juju
perspective and (3) use the time thereby saved to fix SetEnvironConfig.

* BUT ALSO remember that a valid *environ config* does not necessarily
contain valid *provider credentials*. So issues can always arise at
runtime anyway, and should I think be dealt with as follows:

   * StartInstance: record on machine via SetInstanceError.
   * AllInstances: log and ignore -- I don't think there are negative
consequences (out-of-date
     instance list is always possible anyway).
   * StopInstances: log and ignore, they'll be tried again next time we
processMachines. (We should
     add a periodic processMachines call so that we do at least handle
them in something resembling
     a timely fashion. Not too hard, I think?)

https://codereview.appspot.com/9824047/

[Tim Penhey (thumper)]

Please take a look.

https://codereview.appspot.com/9824047/diff/1/worker/provisioner/provisioner_task.go
File worker/provisioner/provisioner_task.go (right):

https://codereview.appspot.com/9824047/diff/1/worker/provisioner/provisioner_task.go#newcode304
worker/provisioner/provisioner_task.go:304: // Don't return a real
error, just try again next time.
On 2013/06/11 09:08:21, fwereade wrote:
> This is problematic, because the watcher won't report that machine
again until
> it changes -- which it won't. For this to work, we need the
provisioner to be
> restarted regularly... but I'm hoping for a provisioner that can run
for weeks
> without breaking sweat.

OK, reverted.

https://codereview.appspot.com/9824047/diff/1/worker/provisioner/provisioner_task.go#newcode360
worker/provisioner/provisioner_task.go:360: // info if config becomes
invalid.
On 2013/06/11 09:08:21, fwereade wrote:
> Honestly, I think this is sufficient justification to nuke the whole
> provisioner; it'll be restarted in a few seconds, and for now I'd
rather not
> even try to provision a machine (which can be expected to cost our
users actual
> money) unless I'm as sure as I can be that I'll be able to start a
functioning
> machine agent...

OK, nuking in progress.

https://codereview.appspot.com/9824047/

[William Reade (fwereade)]

LGTM

https://codereview.appspot.com/9824047/

[Ian Booth (wallyworld)]

LGTM

https://codereview.appspot.com/9824047/

=== modified file '.lbox.check'
--- .lbox.check	2013-04-08 13:46:30 +0000
+++ .lbox.check	2013-06-17 04:13:30 +0000
@@ -2,7 +2,7 @@
 
 set -e
 
-BADFMT=`find * -name '*.go' | xargs gofmt -l`
+BADFMT=`find * -name '*.go' -not -name '.#*' | xargs gofmt -l`
 if [ -n "$BADFMT" ]; then
 	BADFMT=`echo "$BADFMT" | sed "s/^/  /"`
 	echo -e "gofmt is sad:\n\n$BADFMT"

=== modified file 'cmd/juju/plugin_test.go'
--- cmd/juju/plugin_test.go	2013-06-13 10:56:08 +0000
+++ cmd/juju/plugin_test.go	2013-06-17 04:13:30 +0000
@@ -17,6 +17,7 @@
 )
 
 type PluginSuite struct {
+	testing.LoggingSuite
 	oldPath string
 	home    *testing.FakeHome
 }
@@ -24,6 +25,7 @@
 var _ = Suite(&PluginSuite{})
 
 func (suite *PluginSuite) SetUpTest(c *C) {
+	suite.LoggingSuite.SetUpTest(c)
 	suite.oldPath = os.Getenv("PATH")
 	suite.home = testing.MakeSampleHome(c)
 	os.Setenv("PATH", "/bin:"+testing.HomePath())
@@ -32,6 +34,7 @@
 func (suite *PluginSuite) TearDownTest(c *C) {
 	suite.home.Restore()
 	os.Setenv("PATH", suite.oldPath)
+	suite.LoggingSuite.TearDownTest(c)
 }
 
 func (*PluginSuite) TestFindPlugins(c *C) {
@@ -195,8 +198,6 @@
   fi
   echo "{{.Name}} description"
   exit {{.ExitStatus}}
-else
-  echo "No --description" >2
 fi
 
 if [ "$1" = "--help" ]; then

=== modified file 'cmd/juju/status.go'
--- cmd/juju/status.go	2013-06-04 11:50:12 +0000
+++ cmd/juju/status.go	2013-06-17 04:13:30 +0000
@@ -10,6 +10,7 @@
 	"launchpad.net/juju-core/charm"
 	"launchpad.net/juju-core/cmd"
 	"launchpad.net/juju-core/environs"
+	"launchpad.net/juju-core/instance"
 	"launchpad.net/juju-core/juju"
 	"launchpad.net/juju-core/state"
 	"launchpad.net/juju-core/state/api/params"
@@ -42,8 +43,8 @@
 }
 
 type statusContext struct {
-	instances map[state.InstanceId]environs.Instance
-	machines  map[string]*state.Machine
+	instances map[state.InstanceId]instance.Instance
+	machines  map[string][]*state.Machine
 	services  map[string]*state.Service
 	units     map[string]map[string]*state.Unit
 }
@@ -55,14 +56,14 @@
 	}
 	defer conn.Close()
 
-	var ctxt statusContext
-	if ctxt.machines, err = fetchAllMachines(conn.State); err != nil {
-		return err
-	}
-	if ctxt.services, ctxt.units, err = fetchAllServicesAndUnits(conn.State); err != nil {
-		return err
-	}
-	ctxt.instances, err = fetchAllInstances(conn.Environ)
+	var context statusContext
+	if context.machines, err = fetchAllMachines(conn.State); err != nil {
+		return err
+	}
+	if context.services, context.units, err = fetchAllServicesAndUnits(conn.State); err != nil {
+		return err
+	}
+	context.instances, err = fetchAllInstances(conn.Environ)
 	if err != nil {
 		// We cannot see instances from the environment, but
 		// there's still lots of potentially useful info to print.
@@ -72,15 +73,15 @@
 		Machines map[string]machineStatus `json:"machines"`
 		Services map[string]serviceStatus `json:"services"`
 	}{
-		Machines: ctxt.processMachines(),
-		Services: ctxt.processServices(),
+		Machines: context.processMachines(),
+		Services: context.processServices(),
 	}
 	return c.out.Write(ctx, result)
 }
 
 // fetchAllInstances returns a map from instance id to instance.
-func fetchAllInstances(env environs.Environ) (map[state.InstanceId]environs.Instance, error) {
-	m := make(map[state.InstanceId]environs.Instance)
+func fetchAllInstances(env environs.Environ) (map[state.InstanceId]instance.Instance, error) {
+	m := make(map[state.InstanceId]instance.Instance)
 	insts, err := env.AllInstances()
 	if err != nil {
 		return nil, err
@@ -91,15 +92,29 @@
 	return m, nil
 }
 
-// fetchAllMachines returns a map from machine id to machine.
-func fetchAllMachines(st *state.State) (map[string]*state.Machine, error) {
-	v := make(map[string]*state.Machine)
+// fetchAllMachines returns a map from top level machine id to machines, where machines[0] is the host
+// machine and machines[1..n] are any containers (including nested ones).
+func fetchAllMachines(st *state.State) (map[string][]*state.Machine, error) {
+	v := make(map[string][]*state.Machine)
 	machines, err := st.AllMachines()
 	if err != nil {
 		return nil, err
 	}
+	// AllMachines gives us machines sorted by id.
 	for _, m := range machines {
-		v[m.Id()] = m
+		parentId, ok := m.ParentId()
+		if !ok {
+			// Only top level host machines go directly into the machine map.
+			v[m.Id()] = []*state.Machine{m}
+		} else {
+			topParentId := state.TopParentId(m.Id())
+			machines, ok := v[topParentId]
+			if !ok {
+				panic(fmt.Errorf("unexpected machine id %q", parentId))
+			}
+			machines = append(machines, m)
+			v[topParentId] = machines
+		}
 	}
 	return v, nil
 }
@@ -128,15 +143,40 @@
 	return svcMap, unitMap, nil
 }
 
-func (ctxt *statusContext) processMachines() map[string]machineStatus {
+func (context *statusContext) processMachines() map[string]machineStatus {
 	machinesMap := make(map[string]machineStatus)
-	for _, m := range ctxt.machines {
-		machinesMap[m.Id()] = ctxt.processMachine(m)
+	for id, machines := range context.machines {
+		hostStatus := context.makeMachineStatus(machines[0])
+		context.processMachine(machines, &hostStatus, 0)
+		machinesMap[id] = hostStatus
 	}
 	return machinesMap
 }
 
-func (ctxt *statusContext) processMachine(machine *state.Machine) (status machineStatus) {
+func (context *statusContext) processMachine(machines []*state.Machine, host *machineStatus, startIndex int) (nextIndex int) {
+	nextIndex = startIndex + 1
+	currentHost := host
+	var previousContainer *machineStatus
+	for nextIndex < len(machines) {
+		machine := machines[nextIndex]
+		container := context.makeMachineStatus(machine)
+		if currentHost.Id == state.ParentId(machine.Id()) {
+			currentHost.Containers[machine.Id()] = container
+			previousContainer = &container
+			nextIndex++
+		} else {
+			if state.NestingLevel(machine.Id()) > state.NestingLevel(previousContainer.Id) {
+				nextIndex = context.processMachine(machines, previousContainer, nextIndex-1)
+			} else {
+				break
+			}
+		}
+	}
+	return
+}
+
+func (context *statusContext) makeMachineStatus(machine *state.Machine) (status machineStatus) {
+	status.Id = machine.Id()
 	status.Life,
 		status.AgentVersion,
 		status.AgentState,
@@ -146,7 +186,7 @@
 	instid, ok := machine.InstanceId()
 	if ok {
 		status.InstanceId = instid
-		instance, ok := ctxt.instances[instid]
+		instance, ok := context.instances[instid]
 		if ok {
 			status.DNSName, _ = instance.DNSName()
 		} else {
@@ -163,43 +203,44 @@
 		// in the output.
 		status.AgentState = ""
 	}
+	status.Containers = make(map[string]machineStatus)
 	return
 }
 
-func (ctxt *statusContext) processServices() map[string]serviceStatus {
+func (context *statusContext) processServices() map[string]serviceStatus {
 	servicesMap := make(map[string]serviceStatus)
-	for _, s := range ctxt.services {
-		servicesMap[s.Name()] = ctxt.processService(s)
+	for _, s := range context.services {
+		servicesMap[s.Name()] = context.processService(s)
 	}
 	return servicesMap
 }
 
-func (ctxt *statusContext) processService(service *state.Service) (status serviceStatus) {
+func (context *statusContext) processService(service *state.Service) (status serviceStatus) {
 	url, _ := service.CharmURL()
 	status.Charm = url.String()
 	status.Exposed = service.IsExposed()
 	status.Life = processLife(service)
 	var err error
-	status.Relations, status.SubordinateTo, err = ctxt.processRelations(service)
+	status.Relations, status.SubordinateTo, err = context.processRelations(service)
 	if err != nil {
 		status.Err = err
 		return
 	}
 	if service.IsPrincipal() {
-		status.Units = ctxt.processUnits(ctxt.units[service.Name()])
+		status.Units = context.processUnits(context.units[service.Name()])
 	}
 	return status
 }
 
-func (ctxt *statusContext) processUnits(units map[string]*state.Unit) map[string]unitStatus {
+func (context *statusContext) processUnits(units map[string]*state.Unit) map[string]unitStatus {
 	unitsMap := make(map[string]unitStatus)
 	for _, unit := range units {
-		unitsMap[unit.Name()] = ctxt.processUnit(unit)
+		unitsMap[unit.Name()] = context.processUnit(unit)
 	}
 	return unitsMap
 }
 
-func (ctxt *statusContext) processUnit(unit *state.Unit) (status unitStatus) {
+func (context *statusContext) processUnit(unit *state.Unit) (status unitStatus) {
 	status.PublicAddress, _ = unit.PublicAddress()
 	if unit.IsPrincipal() {
 		status.Machine, _ = unit.AssignedMachineId()
@@ -212,16 +253,16 @@
 	if subUnits := unit.SubordinateNames(); len(subUnits) > 0 {
 		status.Subordinates = make(map[string]unitStatus)
 		for _, name := range subUnits {
-			subUnit := ctxt.unitByName(name)
-			status.Subordinates[name] = ctxt.processUnit(subUnit)
+			subUnit := context.unitByName(name)
+			status.Subordinates[name] = context.processUnit(subUnit)
 		}
 	}
 	return
 }
 
-func (ctxt *statusContext) unitByName(name string) *state.Unit {
+func (context *statusContext) unitByName(name string) *state.Unit {
 	serviceName := strings.Split(name, "/")[0]
-	return ctxt.units[serviceName][name]
+	return context.units[serviceName][name]
 }
 
 func (*statusContext) processRelations(service *state.Service) (related map[string][]string, subord []string, err error) {
@@ -311,15 +352,17 @@
 }
 
 type machineStatus struct {
-	Err            error            `json:"-" yaml:",omitempty"`
-	AgentState     params.Status    `json:"agent-state,omitempty" yaml:"agent-state,omitempty"`
-	AgentStateInfo string           `json:"agent-state-info,omitempty" yaml:"agent-state-info,omitempty"`
-	AgentVersion   string           `json:"agent-version,omitempty" yaml:"agent-version,omitempty"`
-	DNSName        string           `json:"dns-name,omitempty" yaml:"dns-name,omitempty"`
-	InstanceId     state.InstanceId `json:"instance-id,omitempty" yaml:"instance-id,omitempty"`
-	InstanceState  string           `json:"instance-state,omitempty" yaml:"instance-state,omitempty"`
-	Life           string           `json:"life,omitempty" yaml:"life,omitempty"`
-	Series         string           `json:"series,omitempty" yaml:"series,omitempty"`
+	Err            error                    `json:"-" yaml:",omitempty"`
+	AgentState     params.Status            `json:"agent-state,omitempty" yaml:"agent-state,omitempty"`
+	AgentStateInfo string                   `json:"agent-state-info,omitempty" yaml:"agent-state-info,omitempty"`
+	AgentVersion   string                   `json:"agent-version,omitempty" yaml:"agent-version,omitempty"`
+	DNSName        string                   `json:"dns-name,omitempty" yaml:"dns-name,omitempty"`
+	InstanceId     state.InstanceId         `json:"instance-id,omitempty" yaml:"instance-id,omitempty"`
+	InstanceState  string                   `json:"instance-state,omitempty" yaml:"instance-state,omitempty"`
+	Life           string                   `json:"life,omitempty" yaml:"life,omitempty"`
+	Series         string                   `json:"series,omitempty" yaml:"series,omitempty"`
+	Id             string                   `json:"-" yaml:"-"`
+	Containers     map[string]machineStatus `json:"containers,omitempty" yaml:"containers,omitempty"`
 }
 
 // A goyaml bug means we can't declare these types

=== modified file 'cmd/juju/status_test.go'
--- cmd/juju/status_test.go	2013-05-27 03:00:31 +0000
+++ cmd/juju/status_test.go	2013-06-17 04:13:30 +0000
@@ -117,6 +117,32 @@
 		"instance-id": "dummyenv-4",
 		"series":      "series",
 	}
+	machine1WithContainers = M{
+		"agent-state": "started",
+		"containers": M{
+			"1/lxc/0": M{
+				"agent-state": "started",
+				"containers": M{
+					"1/lxc/0/lxc/0": M{
+						"agent-state": "started",
+						"dns-name":    "dummyenv-3.dns",
+						"instance-id": "dummyenv-3",
+						"series":      "series",
+					},
+				},
+				"dns-name":    "dummyenv-2.dns",
+				"instance-id": "dummyenv-2",
+				"series":      "series",
+			},
+			"1/lxc/1": M{
+				"instance-id": "pending",
+				"series":      "series",
+			},
+		},
+		"dns-name":    "dummyenv-1.dns",
+		"instance-id": "dummyenv-1",
+		"series":      "series",
+	}
 	unexposedService = M{
 		"charm":   "local:series/dummy-1",
 		"exposed": false,
@@ -716,6 +742,59 @@
 				},
 			},
 		},
+	), test(
+		"machines with containers",
+		addMachine{"0", state.JobManageEnviron},
+		startAliveMachine{"0"},
+		setMachineStatus{"0", params.StatusStarted, ""},
+		addCharm{"mysql"},
+		addService{"mysql", "mysql"},
+		setServiceExposed{"mysql", true},
+
+		addMachine{"1", state.JobHostUnits},
+		startAliveMachine{"1"},
+		setMachineStatus{"1", params.StatusStarted, ""},
+		addAliveUnit{"mysql", "1"},
+		setUnitStatus{"mysql/0", params.StatusStarted, ""},
+
+		// A container on machine 1.
+		addContainer{"1", "1/lxc/0", state.JobHostUnits},
+		startAliveMachine{"1/lxc/0"},
+		setMachineStatus{"1/lxc/0", params.StatusStarted, ""},
+		addAliveUnit{"mysql", "1/lxc/0"},
+		setUnitStatus{"mysql/1", params.StatusStarted, ""},
+		addContainer{"1", "1/lxc/1", state.JobHostUnits},
+
+		// A nested container.
+		addContainer{"1/lxc/0", "1/lxc/0/lxc/0", state.JobHostUnits},
+		startAliveMachine{"1/lxc/0/lxc/0"},
+		setMachineStatus{"1/lxc/0/lxc/0", params.StatusStarted, ""},
+
+		expect{
+			"machines with nested containers",
+			M{
+				"machines": M{
+					"0": machine0,
+					"1": machine1WithContainers,
+				},
+				"services": M{
+					"mysql": M{
+						"charm":   "local:series/mysql-1",
+						"exposed": true,
+						"units": M{
+							"mysql/0": M{
+								"machine":     "1",
+								"agent-state": "started",
+							},
+							"mysql/1": M{
+								"machine":     "1/lxc/0",
+								"agent-state": "started",
+							},
+						},
+					},
+				},
+			},
+		},
 	),
 }
 
@@ -732,6 +811,24 @@
 	c.Assert(m.Id(), Equals, am.machineId)
 }
 
+type addContainer struct {
+	parentId  string
+	machineId string
+	job       state.MachineJob
+}
+
+func (ac addContainer) step(c *C, ctx *context) {
+	params := &state.AddMachineParams{
+		ParentId:      ac.parentId,
+		ContainerType: state.LXC,
+		Series:        "series",
+		Jobs:          []state.MachineJob{ac.job},
+	}
+	m, err := ctx.st.AddMachineWithConstraints(params)
+	c.Assert(err, IsNil)
+	c.Assert(m.Id(), Equals, ac.machineId)
+}
+
 type startMachine struct {
 	machineId string
 }

=== modified file 'environs/azure/environ.go'
--- environs/azure/environ.go	2013-06-11 12:41:10 +0000
+++ environs/azure/environ.go	2013-06-17 04:13:30 +0000
@@ -7,6 +7,7 @@
 	"launchpad.net/juju-core/constraints"
 	"launchpad.net/juju-core/environs"
 	"launchpad.net/juju-core/environs/config"
+	"launchpad.net/juju-core/instance"
 	"launchpad.net/juju-core/state"
 	"launchpad.net/juju-core/state/api"
 	"launchpad.net/juju-core/state/api/params"
@@ -43,22 +44,22 @@
 }
 
 // StartInstance is specified in the Environ interface.
-func (env *azureEnviron) StartInstance(machineId, machineNonce string, series string, cons constraints.Value, info *state.Info, apiInfo *api.Info) (environs.Instance, error) {
+func (env *azureEnviron) StartInstance(machineId, machineNonce string, series string, cons constraints.Value, info *state.Info, apiInfo *api.Info) (instance.Instance, error) {
 	panic("unimplemented")
 }
 
 // StopInstances is specified in the Environ interface.
-func (env *azureEnviron) StopInstances([]environs.Instance) error {
+func (env *azureEnviron) StopInstances([]instance.Instance) error {
 	panic("unimplemented")
 }
 
 // Instances is specified in the Environ interface.
-func (env *azureEnviron) Instances(ids []state.InstanceId) ([]environs.Instance, error) {
+func (env *azureEnviron) Instances(ids []state.InstanceId) ([]instance.Instance, error) {
 	panic("unimplemented")
 }
 
 // AllInstances is specified in the Environ interface.
-func (env *azureEnviron) AllInstances() ([]environs.Instance, error) {
+func (env *azureEnviron) AllInstances() ([]instance.Instance, error) {
 	panic("unimplemented")
 }
 
@@ -73,7 +74,7 @@
 }
 
 // Destroy is specified in the Environ interface.
-func (env *azureEnviron) Destroy(insts []environs.Instance) error {
+func (env *azureEnviron) Destroy(insts []instance.Instance) error {
 	panic("unimplemented")
 }
 

=== modified file 'environs/azure/instance.go'
--- environs/azure/instance.go	2013-06-11 12:41:10 +0000
+++ environs/azure/instance.go	2013-06-17 04:13:30 +0000
@@ -4,7 +4,7 @@
 package azure
 
 import (
-	"launchpad.net/juju-core/environs"
+	"launchpad.net/juju-core/instance"
 	"launchpad.net/juju-core/state"
 	"launchpad.net/juju-core/state/api/params"
 )
@@ -12,7 +12,7 @@
 type azureInstance struct{}
 
 // azureInstance implements Instance.
-var _ environs.Instance = (*azureInstance)(nil)
+var _ instance.Instance = (*azureInstance)(nil)
 
 // Id is specified in the Instance interface.
 func (instance *azureInstance) Id() state.InstanceId {

=== modified file 'environs/dummy/environs.go'
--- environs/dummy/environs.go	2013-05-28 08:05:49 +0000
+++ environs/dummy/environs.go	2013-06-17 04:13:30 +0000
@@ -29,6 +29,7 @@
 	"launchpad.net/juju-core/environs"
 	"launchpad.net/juju-core/environs/config"
 	envtesting "launchpad.net/juju-core/environs/testing"
+	"launchpad.net/juju-core/instance"
 	"launchpad.net/juju-core/log"
 	"launchpad.net/juju-core/schema"
 	"launchpad.net/juju-core/state"
@@ -75,7 +76,7 @@
 	Env          string
 	MachineId    string
 	MachineNonce string
-	Instance     environs.Instance
+	Instance     instance.Instance
 	Constraints  constraints.Value
 	Info         *state.Info
 	APIInfo      *api.Info
@@ -84,7 +85,7 @@
 
 type OpStopInstances struct {
 	Env       string
-	Instances []environs.Instance
+	Instances []instance.Instance
 }
 
 type OpOpenPorts struct {
@@ -122,7 +123,7 @@
 	ops           chan<- Operation
 	mu            sync.Mutex
 	maxId         int // maximum instance id allocated so far.
-	insts         map[state.InstanceId]*instance
+	insts         map[state.InstanceId]*dummyInstance
 	globalPorts   map[params.Port]bool
 	firewallMode  config.FirewallMode
 	bootstrapped  bool
@@ -224,7 +225,7 @@
 	s := &environState{
 		name:         name,
 		ops:          ops,
-		insts:        make(map[state.InstanceId]*instance),
+		insts:        make(map[state.InstanceId]*dummyInstance),
 		globalPorts:  make(map[params.Port]bool),
 		firewallMode: fwmode,
 	}
@@ -515,7 +516,7 @@
 	return nil
 }
 
-func (e *environ) Destroy([]environs.Instance) error {
+func (e *environ) Destroy([]instance.Instance) error {
 	defer delay()
 	if err := e.checkBroken("Destroy"); err != nil {
 		return err
@@ -527,7 +528,7 @@
 	return nil
 }
 
-func (e *environ) StartInstance(machineId, machineNonce string, series string, cons constraints.Value, info *state.Info, apiInfo *api.Info) (environs.Instance, error) {
+func (e *environ) StartInstance(machineId, machineNonce string, series string, cons constraints.Value, info *state.Info, apiInfo *api.Info) (instance.Instance, error) {
 	defer delay()
 	log.Infof("environs/dummy: dummy startinstance, machine %s", machineId)
 	if err := e.checkBroken("StartInstance"); err != nil {
@@ -552,7 +553,7 @@
 	if apiInfo.Tag != state.MachineTag(machineId) {
 		return nil, fmt.Errorf("entity tag must match started machine")
 	}
-	i := &instance{
+	i := &dummyInstance{
 		state:     e.state,
 		id:        state.InstanceId(fmt.Sprintf("%s-%d", e.state.name, e.state.maxId)),
 		ports:     make(map[params.Port]bool),
@@ -574,7 +575,7 @@
 	return i, nil
 }
 
-func (e *environ) StopInstances(is []environs.Instance) error {
+func (e *environ) StopInstances(is []instance.Instance) error {
 	defer delay()
 	if err := e.checkBroken("StopInstance"); err != nil {
 		return err
@@ -582,7 +583,7 @@
 	e.state.mu.Lock()
 	defer e.state.mu.Unlock()
 	for _, i := range is {
-		delete(e.state.insts, i.(*instance).id)
+		delete(e.state.insts, i.(*dummyInstance).id)
 	}
 	e.state.ops <- OpStopInstances{
 		Env:       e.state.name,
@@ -591,7 +592,7 @@
 	return nil
 }
 
-func (e *environ) Instances(ids []state.InstanceId) (insts []environs.Instance, err error) {
+func (e *environ) Instances(ids []state.InstanceId) (insts []instance.Instance, err error) {
 	defer delay()
 	if err := e.checkBroken("Instances"); err != nil {
 		return nil, err
@@ -616,12 +617,12 @@
 	return
 }
 
-func (e *environ) AllInstances() ([]environs.Instance, error) {
+func (e *environ) AllInstances() ([]instance.Instance, error) {
 	defer delay()
 	if err := e.checkBroken("AllInstances"); err != nil {
 		return nil, err
 	}
-	var insts []environs.Instance
+	var insts []instance.Instance
 	e.state.mu.Lock()
 	defer e.state.mu.Unlock()
 	for _, v := range e.state.insts {
@@ -674,7 +675,7 @@
 	return &providerInstance
 }
 
-type instance struct {
+type dummyInstance struct {
 	state     *environState
 	ports     map[params.Port]bool
 	id        state.InstanceId
@@ -682,20 +683,20 @@
 	series    string
 }
 
-func (inst *instance) Id() state.InstanceId {
+func (inst *dummyInstance) Id() state.InstanceId {
 	return inst.id
 }
 
-func (inst *instance) DNSName() (string, error) {
+func (inst *dummyInstance) DNSName() (string, error) {
 	defer delay()
 	return string(inst.id) + ".dns", nil
 }
 
-func (inst *instance) WaitDNSName() (string, error) {
+func (inst *dummyInstance) WaitDNSName() (string, error) {
 	return inst.DNSName()
 }
 
-func (inst *instance) OpenPorts(machineId string, ports []params.Port) error {
+func (inst *dummyInstance) OpenPorts(machineId string, ports []params.Port) error {
 	defer delay()
 	log.Infof("environs/dummy: openPorts %s, %#v", machineId, ports)
 	if inst.state.firewallMode != config.FwInstance {
@@ -719,7 +720,7 @@
 	return nil
 }
 
-func (inst *instance) ClosePorts(machineId string, ports []params.Port) error {
+func (inst *dummyInstance) ClosePorts(machineId string, ports []params.Port) error {
 	defer delay()
 	if inst.state.firewallMode != config.FwInstance {
 		return fmt.Errorf("invalid firewall mode for closing ports on instance: %q",
@@ -742,7 +743,7 @@
 	return nil
 }
 
-func (inst *instance) Ports(machineId string) (ports []params.Port, err error) {
+func (inst *dummyInstance) Ports(machineId string) (ports []params.Port, err error) {
 	defer delay()
 	if inst.state.firewallMode != config.FwInstance {
 		return nil, fmt.Errorf("invalid firewall mode for retrieving ports from instance: %q",

=== modified file 'environs/ec2/ec2.go'
--- environs/ec2/ec2.go	2013-06-10 10:30:13 +0000
+++ environs/ec2/ec2.go	2013-06-17 04:13:30 +0000
@@ -17,6 +17,7 @@
 	"launchpad.net/juju-core/environs/instances"
 	"launchpad.net/juju-core/environs/tools"
 	"launchpad.net/juju-core/errors"
+	"launchpad.net/juju-core/instance"
 	"launchpad.net/juju-core/log"
 	"launchpad.net/juju-core/state"
 	"launchpad.net/juju-core/state/api"
@@ -65,22 +66,22 @@
 
 var _ environs.Environ = (*environ)(nil)
 
-type instance struct {
+type ec2Instance struct {
 	e *environ
 	*ec2.Instance
 }
 
-func (inst *instance) String() string {
+func (inst *ec2Instance) String() string {
 	return inst.InstanceId
 }
 
-var _ environs.Instance = (*instance)(nil)
+var _ instance.Instance = (*ec2Instance)(nil)
 
-func (inst *instance) Id() state.InstanceId {
+func (inst *ec2Instance) Id() state.InstanceId {
 	return state.InstanceId(inst.InstanceId)
 }
 
-func (inst *instance) DNSName() (string, error) {
+func (inst *ec2Instance) DNSName() (string, error) {
 	if inst.Instance.DNSName != "" {
 		return inst.Instance.DNSName, nil
 	}
@@ -90,18 +91,18 @@
 	if err != nil {
 		return "", err
 	}
-	freshInst := insts[0].(*instance).Instance
+	freshInst := insts[0].(*ec2Instance).Instance
 	if freshInst.DNSName == "" {
-		return "", environs.ErrNoDNSName
+		return "", instance.ErrNoDNSName
 	}
 	inst.Instance.DNSName = freshInst.DNSName
 	return freshInst.DNSName, nil
 }
 
-func (inst *instance) WaitDNSName() (string, error) {
+func (inst *ec2Instance) WaitDNSName() (string, error) {
 	for a := longAttempt.Start(); a.Next(); {
 		name, err := inst.DNSName()
-		if err == nil || err != environs.ErrNoDNSName {
+		if err == nil || err != instance.ErrNoDNSName {
 			return name, err
 		}
 	}
@@ -278,7 +279,7 @@
 	if err != nil {
 		// ignore error on StopInstance because the previous error is
 		// more important.
-		e.StopInstances([]environs.Instance{inst})
+		e.StopInstances([]instance.Instance{inst})
 		return fmt.Errorf("cannot save state: %v", err)
 	}
 	// TODO make safe in the case of racing Bootstraps
@@ -313,7 +314,7 @@
 			if inst == nil {
 				continue
 			}
-			name := inst.(*instance).Instance.DNSName
+			name := inst.(*ec2Instance).Instance.DNSName
 			if name != "" {
 				statePortSuffix := fmt.Sprintf(":%d", config.StatePort())
 				apiPortSuffix := fmt.Sprintf(":%d", config.APIPort())
@@ -340,7 +341,7 @@
 	return []string{imagemetadata.DefaultBaseURL}, nil
 }
 
-func (e *environ) StartInstance(machineId, machineNonce string, series string, cons constraints.Value, info *state.Info, apiInfo *api.Info) (environs.Instance, error) {
+func (e *environ) StartInstance(machineId, machineNonce string, series string, cons constraints.Value, info *state.Info, apiInfo *api.Info) (instance.Instance, error) {
 	possibleTools, err := environs.FindInstanceTools(e, series, cons)
 	if err != nil {
 		return nil, err
@@ -397,7 +398,7 @@
 
 // startInstance is the internal version of StartInstance, used by Bootstrap
 // as well as via StartInstance itself.
-func (e *environ) startInstance(scfg *startInstanceParams) (environs.Instance, error) {
+func (e *environ) startInstance(scfg *startInstanceParams) (instance.Instance, error) {
 	series := scfg.possibleTools.Series()
 	if len(series) != 1 {
 		return nil, fmt.Errorf("expected single series, got %v", series)
@@ -455,15 +456,15 @@
 	if len(instances.Instances) != 1 {
 		return nil, fmt.Errorf("expected 1 started instance, got %d", len(instances.Instances))
 	}
-	inst := &instance{e, &instances.Instances[0]}
+	inst := &ec2Instance{e, &instances.Instances[0]}
 	log.Infof("environs/ec2: started instance %q", inst.Id())
 	return inst, nil
 }
 
-func (e *environ) StopInstances(insts []environs.Instance) error {
+func (e *environ) StopInstances(insts []instance.Instance) error {
 	ids := make([]state.InstanceId, len(insts))
 	for i, inst := range insts {
-		ids[i] = inst.(*instance).Id()
+		ids[i] = inst.(*ec2Instance).Id()
 	}
 	return e.terminateInstances(ids)
 }
@@ -472,7 +473,7 @@
 // id whose corresponding insts slot is nil.
 // It returns environs.ErrPartialInstances if the insts
 // slice has not been completely filled.
-func (e *environ) gatherInstances(ids []state.InstanceId, insts []environs.Instance) error {
+func (e *environ) gatherInstances(ids []state.InstanceId, insts []instance.Instance) error {
 	var need []string
 	for i, inst := range insts {
 		if inst == nil {
@@ -502,7 +503,7 @@
 			for k := range r.Instances {
 				if r.Instances[k].InstanceId == string(id) {
 					inst := r.Instances[k]
-					insts[i] = &instance{e, &inst}
+					insts[i] = &ec2Instance{e, &inst}
 					n++
 				}
 			}
@@ -514,11 +515,11 @@
 	return nil
 }
 
-func (e *environ) Instances(ids []state.InstanceId) ([]environs.Instance, error) {
+func (e *environ) Instances(ids []state.InstanceId) ([]instance.Instance, error) {
 	if len(ids) == 0 {
 		return nil, nil
 	}
-	insts := make([]environs.Instance, len(ids))
+	insts := make([]instance.Instance, len(ids))
 	// Make a series of requests to cope with eventual consistency.
 	// Each request will attempt to add more instances to the requested
 	// set.
@@ -543,7 +544,7 @@
 	return insts, nil
 }
 
-func (e *environ) AllInstances() ([]environs.Instance, error) {
+func (e *environ) AllInstances() ([]instance.Instance, error) {
 	filter := ec2.NewFilter()
 	filter.Add("instance-state-name", "pending", "running")
 	filter.Add("group-name", e.jujuGroupName())
@@ -551,17 +552,17 @@
 	if err != nil {
 		return nil, err
 	}
-	var insts []environs.Instance
+	var insts []instance.Instance
 	for _, r := range resp.Reservations {
 		for i := range r.Instances {
 			inst := r.Instances[i]
-			insts = append(insts, &instance{e, &inst})
+			insts = append(insts, &ec2Instance{e, &inst})
 		}
 	}
 	return insts, nil
 }
 
-func (e *environ) Destroy(ensureInsts []environs.Instance) error {
+func (e *environ) Destroy(ensureInsts []instance.Instance) error {
 	log.Infof("environs/ec2: destroying environment %q", e.name)
 	insts, err := e.AllInstances()
 	if err != nil {
@@ -577,7 +578,7 @@
 	// Add any instances we've been told about but haven't yet shown
 	// up in the instance list.
 	for _, inst := range ensureInsts {
-		id := state.InstanceId(inst.(*instance).InstanceId)
+		id := state.InstanceId(inst.(*ec2Instance).InstanceId)
 		if !found[id] {
 			ids = append(ids, id)
 			found[id] = true
@@ -761,7 +762,7 @@
 	return "juju-" + e.name
 }
 
-func (inst *instance) OpenPorts(machineId string, ports []params.Port) error {
+func (inst *ec2Instance) OpenPorts(machineId string, ports []params.Port) error {
 	if inst.e.Config().FirewallMode() != config.FwInstance {
 		return fmt.Errorf("invalid firewall mode for opening ports on instance: %q",
 			inst.e.Config().FirewallMode())
@@ -774,7 +775,7 @@
 	return nil
 }
 
-func (inst *instance) ClosePorts(machineId string, ports []params.Port) error {
+func (inst *ec2Instance) ClosePorts(machineId string, ports []params.Port) error {
 	if inst.e.Config().FirewallMode() != config.FwInstance {
 		return fmt.Errorf("invalid firewall mode for closing ports on instance: %q",
 			inst.e.Config().FirewallMode())
@@ -787,7 +788,7 @@
 	return nil
 }
 
-func (inst *instance) Ports(machineId string) ([]params.Port, error) {
+func (inst *ec2Instance) Ports(machineId string) ([]params.Port, error) {
 	if inst.e.Config().FirewallMode() != config.FwInstance {
 		return nil, fmt.Errorf("invalid firewall mode for retrieving ports from instance: %q",
 			inst.e.Config().FirewallMode())

=== modified file 'environs/ec2/export_test.go'
--- environs/ec2/export_test.go	2013-05-30 05:39:33 +0000
+++ environs/ec2/export_test.go	2013-06-17 04:13:30 +0000
@@ -11,6 +11,7 @@
 	"launchpad.net/juju-core/environs"
 	"launchpad.net/juju-core/environs/imagemetadata"
 	"launchpad.net/juju-core/environs/jujutest"
+	"launchpad.net/juju-core/instance"
 	"launchpad.net/juju-core/state"
 	"launchpad.net/juju-core/utils"
 	"net/http"
@@ -48,8 +49,8 @@
 	return s.(*storage).deleteAll()
 }
 
-func InstanceEC2(inst environs.Instance) *ec2.Instance {
-	return inst.(*instance).Instance
+func InstanceEC2(inst instance.Instance) *ec2.Instance {
+	return inst.(*ec2Instance).Instance
 }
 
 // BucketStorage returns a storage instance addressing
@@ -147,9 +148,9 @@
 
 // FabricateInstance creates a new fictitious instance
 // given an existing instance and a new id.
-func FabricateInstance(inst environs.Instance, newId string) environs.Instance {
-	oldi := inst.(*instance)
-	newi := &instance{oldi.e, &ec2.Instance{}}
+func FabricateInstance(inst instance.Instance, newId string) instance.Instance {
+	oldi := inst.(*ec2Instance)
+	newi := &ec2Instance{oldi.e, &ec2.Instance{}}
 	*newi.Instance = *oldi.Instance
 	newi.InstanceId = newId
 	return newi

=== modified file 'environs/ec2/live_test.go'
--- environs/ec2/live_test.go	2013-05-30 00:47:30 +0000
+++ environs/ec2/live_test.go	2013-06-17 04:13:30 +0000
@@ -17,6 +17,7 @@
 	"launchpad.net/juju-core/environs/jujutest"
 	envtesting "launchpad.net/juju-core/environs/testing"
 	"launchpad.net/juju-core/errors"
+	"launchpad.net/juju-core/instance"
 	"launchpad.net/juju-core/juju/testing"
 	"launchpad.net/juju-core/state"
 	coretesting "launchpad.net/juju-core/testing"
@@ -113,7 +114,7 @@
 
 func (t *LiveTests) TestInstanceAttributes(c *C) {
 	inst := testing.StartInstance(c, t.Env, "30")
-	defer t.Env.StopInstances([]environs.Instance{inst})
+	defer t.Env.StopInstances([]instance.Instance{inst})
 	dns, err := inst.WaitDNSName()
 	// TODO(niemeyer): This assert sometimes fails with "no instances found"
 	c.Assert(err, IsNil)
@@ -132,7 +133,7 @@
 	cons := constraints.MustParse("mem=2G")
 	inst, err := t.Env.StartInstance("31", "fake_nonce", config.DefaultSeries, cons, testing.InvalidStateInfo("31"), testing.InvalidAPIInfo("31"))
 	c.Assert(err, IsNil)
-	defer t.Env.StopInstances([]environs.Instance{inst})
+	defer t.Env.StopInstances([]instance.Instance{inst})
 	ec2inst := ec2.InstanceEC2(inst)
 	c.Assert(ec2inst.InstanceType, Equals, "m1.medium")
 }
@@ -173,14 +174,14 @@
 	c.Assert(err, IsNil)
 
 	inst0 := testing.StartInstance(c, t.Env, "98")
-	defer t.Env.StopInstances([]environs.Instance{inst0})
+	defer t.Env.StopInstances([]instance.Instance{inst0})
 
 	// Create a same-named group for the second instance
 	// before starting it, to check that it's reused correctly.
 	oldMachineGroup := createGroup(c, ec2conn, groups[2].Name, "old machine group")
 
 	inst1 := testing.StartInstance(c, t.Env, "99")
-	defer t.Env.StopInstances([]environs.Instance{inst1})
+	defer t.Env.StopInstances([]instance.Instance{inst1})
 
 	groupsResp, err := ec2conn.SecurityGroups(groups, nil)
 	c.Assert(err, IsNil)
@@ -314,10 +315,10 @@
 	inst1 := ec2.FabricateInstance(inst0, "i-aaaaaaaa")
 	inst2 := testing.StartInstance(c, t.Env, "41")
 
-	err := t.Env.StopInstances([]environs.Instance{inst0, inst1, inst2})
+	err := t.Env.StopInstances([]instance.Instance{inst0, inst1, inst2})
 	c.Check(err, IsNil)
 
-	var insts []environs.Instance
+	var insts []instance.Instance
 
 	// We need the retry logic here because we are waiting
 	// for Instances to return an error, and it will not retry

=== modified file 'environs/interface.go'
--- environs/interface.go	2013-06-10 10:30:13 +0000
+++ environs/interface.go	2013-06-17 04:13:30 +0000
@@ -8,6 +8,7 @@
 	"io"
 	"launchpad.net/juju-core/constraints"
 	"launchpad.net/juju-core/environs/config"
+	"launchpad.net/juju-core/instance"
 	"launchpad.net/juju-core/state"
 	"launchpad.net/juju-core/state/api"
 	"launchpad.net/juju-core/state/api/params"
@@ -49,36 +50,6 @@
 	InstanceId() (state.InstanceId, error)
 }
 
-var ErrNoDNSName = errors.New("DNS name not allocated")
-
-// Instance represents the provider-specific notion of a machine.
-type Instance interface {
-	// Id returns a provider-generated identifier for the Instance.
-	Id() state.InstanceId
-
-	// DNSName returns the DNS name for the instance.
-	// If the name is not yet allocated, it will return
-	// an ErrNoDNSName error.
-	DNSName() (string, error)
-
-	// WaitDNSName returns the DNS name for the instance,
-	// waiting until it is allocated if necessary.
-	WaitDNSName() (string, error)
-
-	// OpenPorts opens the given ports on the instance, which
-	// should have been started with the given machine id.
-	OpenPorts(machineId string, ports []params.Port) error
-
-	// ClosePorts closes the given ports on the instance, which
-	// should have been started with the given machine id.
-	ClosePorts(machineId string, ports []params.Port) error
-
-	// Ports returns the set of ports open on the instance, which
-	// should have been started with the given machine id.
-	// The ports are returned as sorted by state.SortPorts.
-	Ports(machineId string) ([]params.Port, error)
-}
-
 var ErrNoInstances = errors.New("no instances found")
 var ErrPartialInstances = errors.New("only some instances were found")
 
@@ -164,10 +135,10 @@
 	// which must be unique within an environment, is used by juju to
 	// protect against the consequences of multiple instances being
 	// started with the same machine id.
-	StartInstance(machineId, machineNonce string, series string, cons constraints.Value, info *state.Info, apiInfo *api.Info) (Instance, error)
+	StartInstance(machineId, machineNonce string, series string, cons constraints.Value, info *state.Info, apiInfo *api.Info) (instance.Instance, error)
 
 	// StopInstances shuts down the given instances.
-	StopInstances([]Instance) error
+	StopInstances([]instance.Instance) error
 
 	// Instances returns a slice of instances corresponding to the
 	// given instance ids.  If no instances were found, but there
@@ -175,11 +146,11 @@
 	// some but not all the instances were found, the returned slice
 	// will have some nil slots, and an ErrPartialInstances error
 	// will be returned.
-	Instances(ids []state.InstanceId) ([]Instance, error)
+	Instances(ids []state.InstanceId) ([]instance.Instance, error)
 
 	// AllInstances returns all instances currently known to the
 	// environment.
-	AllInstances() ([]Instance, error)
+	AllInstances() ([]instance.Instance, error)
 
 	// Storage returns storage specific to the environment.
 	Storage() Storage
@@ -196,7 +167,7 @@
 	//
 	// When Destroy has been called, any Environ referring to the
 	// same remote environment may become invalid
-	Destroy(insts []Instance) error
+	Destroy(insts []instance.Instance) error
 
 	// OpenPorts opens the given ports for the whole environment.
 	// Must only be used if the environment was setup with the

=== modified file 'environs/jujutest/livetests.go'
--- environs/jujutest/livetests.go	2013-05-30 00:47:30 +0000
+++ environs/jujutest/livetests.go	2013-06-17 04:13:30 +0000
@@ -14,6 +14,7 @@
 	"launchpad.net/juju-core/environs/config"
 	"launchpad.net/juju-core/environs/tools"
 	"launchpad.net/juju-core/errors"
+	"launchpad.net/juju-core/instance"
 	"launchpad.net/juju-core/juju"
 	"launchpad.net/juju-core/juju/testing"
 	"launchpad.net/juju-core/state"
@@ -142,7 +143,7 @@
 	c.Check(insts[0].Id(), Equals, id0)
 	c.Check(insts[1], IsNil)
 
-	err = t.Env.StopInstances([]environs.Instance{inst})
+	err = t.Env.StopInstances([]instance.Instance{inst})
 	c.Assert(err, IsNil)
 
 	// The machine may not be marked as shutting down
@@ -160,7 +161,7 @@
 func (t *LiveTests) TestPorts(c *C) {
 	inst1 := testing.StartInstance(c, t.Env, "1")
 	c.Assert(inst1, NotNil)
-	defer t.Env.StopInstances([]environs.Instance{inst1})
+	defer t.Env.StopInstances([]instance.Instance{inst1})
 	ports, err := inst1.Ports("1")
 	c.Assert(err, IsNil)
 	c.Assert(ports, HasLen, 0)
@@ -170,7 +171,7 @@
 	ports, err = inst2.Ports("2")
 	c.Assert(err, IsNil)
 	c.Assert(ports, HasLen, 0)
-	defer t.Env.StopInstances([]environs.Instance{inst2})
+	defer t.Env.StopInstances([]instance.Instance{inst2})
 
 	// Open some ports and check they're there.
 	err = inst1.OpenPorts("1", []params.Port{{"udp", 67}, {"tcp", 45}})
@@ -260,7 +261,7 @@
 
 	// Create instances and check open ports on both instances.
 	inst1 := testing.StartInstance(c, t.Env, "1")
-	defer t.Env.StopInstances([]environs.Instance{inst1})
+	defer t.Env.StopInstances([]instance.Instance{inst1})
 	ports, err := t.Env.Ports()
 	c.Assert(err, IsNil)
 	c.Assert(ports, HasLen, 0)
@@ -269,7 +270,7 @@
 	ports, err = t.Env.Ports()
 	c.Assert(err, IsNil)
 	c.Assert(ports, HasLen, 0)
-	defer t.Env.StopInstances([]environs.Instance{inst2})
+	defer t.Env.StopInstances([]instance.Instance{inst2})
 
 	err = t.Env.OpenPorts([]params.Port{{"udp", 67}, {"tcp", 45}, {"tcp", 89}, {"tcp", 99}})
 	c.Assert(err, IsNil)
@@ -630,7 +631,7 @@
 // assertInstanceId asserts that the machine has an instance id
 // that matches that of the given instance. If the instance is nil,
 // It asserts that the instance id is unset.
-func assertInstanceId(c *C, m *state.Machine, inst environs.Instance) {
+func assertInstanceId(c *C, m *state.Machine, inst instance.Instance) {
 	var wantId, gotId state.InstanceId
 	var err error
 	if inst != nil {
@@ -700,7 +701,7 @@
 func (t *LiveTests) TestStartInstanceOnUnknownPlatform(c *C) {
 	inst, err := t.Env.StartInstance("4", "fake_nonce", "unknownseries", constraints.Value{}, testing.InvalidStateInfo("4"), testing.InvalidAPIInfo("4"))
 	if inst != nil {
-		err := t.Env.StopInstances([]environs.Instance{inst})
+		err := t.Env.StopInstances([]instance.Instance{inst})
 		c.Check(err, IsNil)
 	}
 	c.Assert(inst, IsNil)
@@ -713,7 +714,7 @@
 func (t *LiveTests) TestStartInstanceWithEmptyNonceFails(c *C) {
 	inst, err := t.Env.StartInstance("4", "", config.DefaultSeries, constraints.Value{}, testing.InvalidStateInfo("4"), testing.InvalidAPIInfo("4"))
 	if inst != nil {
-		err := t.Env.StopInstances([]environs.Instance{inst})
+		err := t.Env.StopInstances([]instance.Instance{inst})
 		c.Check(err, IsNil)
 	}
 	c.Assert(inst, IsNil)

=== modified file 'environs/jujutest/tests.go'
--- environs/jujutest/tests.go	2013-05-30 00:47:30 +0000
+++ environs/jujutest/tests.go	2013-06-17 04:13:30 +0000
@@ -12,6 +12,7 @@
 	"launchpad.net/juju-core/environs"
 	envtesting "launchpad.net/juju-core/environs/testing"
 	"launchpad.net/juju-core/errors"
+	"launchpad.net/juju-core/instance"
 	"launchpad.net/juju-core/juju/testing"
 	"launchpad.net/juju-core/state"
 	coretesting "launchpad.net/juju-core/testing"
@@ -107,7 +108,7 @@
 	c.Assert(insts, HasLen, 2)
 	c.Assert(insts[0].Id(), Not(Equals), insts[1].Id())
 
-	err = e.StopInstances([]environs.Instance{inst0})
+	err = e.StopInstances([]instance.Instance{inst0})
 	c.Assert(err, IsNil)
 
 	insts, err = e.Instances([]state.InstanceId{id0, id1})

=== modified file 'environs/maas/environ.go'
--- environs/maas/environ.go	2013-06-10 10:30:13 +0000
+++ environs/maas/environ.go	2013-06-17 04:13:30 +0000
@@ -13,6 +13,7 @@
 	"launchpad.net/juju-core/environs/cloudinit"
 	"launchpad.net/juju-core/environs/config"
 	"launchpad.net/juju-core/environs/tools"
+	"launchpad.net/juju-core/instance"
 	"launchpad.net/juju-core/log"
 	"launchpad.net/juju-core/state"
 	"launchpad.net/juju-core/state/api"
@@ -84,7 +85,7 @@
 }
 
 // startBootstrapNode starts the juju bootstrap node for this environment.
-func (env *maasEnviron) startBootstrapNode(cons constraints.Value) (environs.Instance, error) {
+func (env *maasEnviron) startBootstrapNode(cons constraints.Value) (instance.Instance, error) {
 	// The bootstrap instance gets machine id "0".  This is not related to
 	// instance ids or MAAS system ids.  Juju assigns the machine ID.
 	const machineID = "0"
@@ -352,7 +353,7 @@
 }
 
 // StartInstance is specified in the Environ interface.
-func (environ *maasEnviron) StartInstance(machineID, machineNonce string, series string, cons constraints.Value, stateInfo *state.Info, apiInfo *api.Info) (environs.Instance, error) {
+func (environ *maasEnviron) StartInstance(machineID, machineNonce string, series string, cons constraints.Value, stateInfo *state.Info, apiInfo *api.Info) (instance.Instance, error) {
 	possibleTools, err := environs.FindInstanceTools(environ, series, cons)
 	if err != nil {
 		return nil, err
@@ -362,7 +363,7 @@
 }
 
 // StopInstances is specified in the Environ interface.
-func (environ *maasEnviron) StopInstances(instances []environs.Instance) error {
+func (environ *maasEnviron) StopInstances(instances []instance.Instance) error {
 	// Shortcut to exit quickly if 'instances' is an empty slice or nil.
 	if len(instances) == 0 {
 		return nil
@@ -381,7 +382,7 @@
 }
 
 // releaseInstance releases a single instance.
-func (environ *maasEnviron) releaseInstance(inst environs.Instance) error {
+func (environ *maasEnviron) releaseInstance(inst instance.Instance) error {
 	maasInst := inst.(*maasInstance)
 	maasObj := maasInst.maasObject
 	_, err := maasObj.CallPost("release", nil)
@@ -391,10 +392,10 @@
 	return err
 }
 
-// Instances returns the environs.Instance objects corresponding to the given
+// Instances returns the instance.Instance objects corresponding to the given
 // slice of state.InstanceId.  Similar to what the ec2 provider does,
 // Instances returns nil if the given slice is empty or nil.
-func (environ *maasEnviron) Instances(ids []state.InstanceId) ([]environs.Instance, error) {
+func (environ *maasEnviron) Instances(ids []state.InstanceId) ([]instance.Instance, error) {
 	if len(ids) == 0 {
 		return nil, nil
 	}
@@ -406,7 +407,7 @@
 // If the some of the intances could not be found, it returns the instance
 // that could be found plus the error environs.ErrPartialInstances in the error
 // return.
-func (environ *maasEnviron) instances(ids []state.InstanceId) ([]environs.Instance, error) {
+func (environ *maasEnviron) instances(ids []state.InstanceId) ([]instance.Instance, error) {
 	nodeListing := environ.getMAASClient().GetSubObject("nodes")
 	filter := getSystemIdValues(ids)
 	listNodeObjects, err := nodeListing.CallGet("list", filter)
@@ -417,7 +418,7 @@
 	if err != nil {
 		return nil, err
 	}
-	instances := make([]environs.Instance, len(listNodes))
+	instances := make([]instance.Instance, len(listNodes))
 	for index, nodeObj := range listNodes {
 		node, err := nodeObj.GetMAASObject()
 		if err != nil {
@@ -434,8 +435,8 @@
 	return instances, nil
 }
 
-// AllInstances returns all the environs.Instance in this provider.
-func (environ *maasEnviron) AllInstances() ([]environs.Instance, error) {
+// AllInstances returns all the instance.Instance in this provider.
+func (environ *maasEnviron) AllInstances() ([]instance.Instance, error) {
 	return environ.instances(nil)
 }
 
@@ -452,7 +453,7 @@
 	return environs.EmptyStorage
 }
 
-func (environ *maasEnviron) Destroy(ensureInsts []environs.Instance) error {
+func (environ *maasEnviron) Destroy(ensureInsts []instance.Instance) error {
 	log.Debugf("environs/maas: destroying environment %q", environ.name)
 	insts, err := environ.AllInstances()
 	if err != nil {

=== modified file 'environs/maas/environ_test.go'
--- environs/maas/environ_test.go	2013-06-10 10:30:13 +0000
+++ environs/maas/environ_test.go	2013-06-17 04:13:30 +0000
@@ -15,6 +15,7 @@
 	envtesting "launchpad.net/juju-core/environs/testing"
 	"launchpad.net/juju-core/environs/tools"
 	"launchpad.net/juju-core/errors"
+	"launchpad.net/juju-core/instance"
 	"launchpad.net/juju-core/state"
 	"launchpad.net/juju-core/testing"
 	"launchpad.net/juju-core/utils"
@@ -344,7 +345,7 @@
 func (suite *EnvironSuite) TestStopInstancesReturnsIfParameterEmpty(c *C) {
 	suite.getInstance("test1")
 
-	err := suite.environ.StopInstances([]environs.Instance{})
+	err := suite.environ.StopInstances([]instance.Instance{})
 	c.Check(err, IsNil)
 	operations := suite.testMAASObject.TestServer.NodeOperations()
 	c.Check(operations, DeepEquals, map[string][]string{})
@@ -354,7 +355,7 @@
 	instance1 := suite.getInstance("test1")
 	instance2 := suite.getInstance("test2")
 	suite.getInstance("test3")
-	instances := []environs.Instance{instance1, instance2}
+	instances := []instance.Instance{instance1, instance2}
 
 	err := suite.environ.StopInstances(instances)
 
@@ -394,12 +395,12 @@
 func (suite *EnvironSuite) TestDestroy(c *C) {
 	env := suite.makeEnviron()
 	suite.getInstance("test1")
-	instance := suite.getInstance("test2")
+	testInstance := suite.getInstance("test2")
 	data := makeRandomBytes(10)
 	suite.testMAASObject.TestServer.NewFile("filename", data)
 	storage := env.Storage()
 
-	err := env.Destroy([]environs.Instance{instance})
+	err := env.Destroy([]instance.Instance{testInstance})
 
 	c.Check(err, IsNil)
 	// Instances have been stopped.

=== modified file 'environs/maas/instance.go'
--- environs/maas/instance.go	2013-05-02 15:55:42 +0000
+++ environs/maas/instance.go	2013-06-17 04:13:30 +0000
@@ -5,7 +5,7 @@
 
 import (
 	"launchpad.net/gomaasapi"
-	"launchpad.net/juju-core/environs"
+	"launchpad.net/juju-core/instance"
 	"launchpad.net/juju-core/log"
 	"launchpad.net/juju-core/state"
 	"launchpad.net/juju-core/state/api/params"
@@ -16,7 +16,7 @@
 	environ    *maasEnviron
 }
 
-var _ environs.Instance = (*maasInstance)(nil)
+var _ instance.Instance = (*maasInstance)(nil)
 
 func (instance *maasInstance) Id() state.InstanceId {
 	// Use the node's 'resource_uri' value.

=== modified file 'environs/openstack/local_test.go'
--- environs/openstack/local_test.go	2013-05-28 08:05:49 +0000
+++ environs/openstack/local_test.go	2013-06-17 04:13:30 +0000
@@ -17,6 +17,7 @@
 	"launchpad.net/juju-core/environs/jujutest"
 	"launchpad.net/juju-core/environs/openstack"
 	envtesting "launchpad.net/juju-core/environs/testing"
+	"launchpad.net/juju-core/instance"
 	"launchpad.net/juju-core/juju/testing"
 	"launchpad.net/juju-core/state"
 	coretesting "launchpad.net/juju-core/testing"
@@ -281,7 +282,7 @@
 	err = environs.Bootstrap(env, constraints.Value{})
 	c.Assert(err, IsNil)
 	inst := testing.StartInstance(c, env, "100")
-	err = s.Env.StopInstances([]environs.Instance{inst})
+	err = s.Env.StopInstances([]instance.Instance{inst})
 	c.Assert(err, IsNil)
 }
 
@@ -338,7 +339,7 @@
 	inst1 := testing.StartInstance(c, s.Env, "101")
 	id1 := inst1.Id()
 	defer func() {
-		err := s.Env.StopInstances([]environs.Instance{inst0, inst1})
+		err := s.Env.StopInstances([]instance.Instance{inst0, inst1})
 		c.Assert(err, IsNil)
 	}()
 

=== modified file 'environs/openstack/provider.go'
--- environs/openstack/provider.go	2013-06-10 10:30:13 +0000
+++ environs/openstack/provider.go	2013-06-17 04:13:30 +0000
@@ -23,6 +23,7 @@
 	"launchpad.net/juju-core/environs/instances"
 	"launchpad.net/juju-core/environs/tools"
 	coreerrors "launchpad.net/juju-core/errors"
+	"launchpad.net/juju-core/instance"
 	"launchpad.net/juju-core/log"
 	"launchpad.net/juju-core/state"
 	"launchpad.net/juju-core/state/api"
@@ -270,19 +271,19 @@
 
 var _ environs.Environ = (*environ)(nil)
 
-type instance struct {
+type openstackInstance struct {
 	e *environ
 	*nova.ServerDetail
 	address string
 }
 
-func (inst *instance) String() string {
+func (inst *openstackInstance) String() string {
 	return inst.ServerDetail.Id
 }
 
-var _ environs.Instance = (*instance)(nil)
+var _ instance.Instance = (*openstackInstance)(nil)
 
-func (inst *instance) Id() state.InstanceId {
+func (inst *openstackInstance) Id() state.InstanceId {
 	return state.InstanceId(inst.ServerDetail.Id)
 }
 
@@ -319,12 +320,12 @@
 		public = private
 	}
 	if public == "" {
-		return "", environs.ErrNoDNSName
+		return "", instance.ErrNoDNSName
 	}
 	return public, nil
 }
 
-func (inst *instance) DNSName() (string, error) {
+func (inst *openstackInstance) DNSName() (string, error) {
 	if inst.address != "" {
 		return inst.address, nil
 	}
@@ -341,10 +342,10 @@
 	return inst.address, nil
 }
 
-func (inst *instance) WaitDNSName() (string, error) {
+func (inst *openstackInstance) WaitDNSName() (string, error) {
 	for a := longAttempt.Start(); a.Next(); {
 		addr, err := inst.DNSName()
-		if err == nil || err != environs.ErrNoDNSName {
+		if err == nil || err != instance.ErrNoDNSName {
 			return addr, err
 		}
 	}
@@ -353,7 +354,7 @@
 
 // TODO: following 30 lines nearly verbatim from environs/ec2
 
-func (inst *instance) OpenPorts(machineId string, ports []params.Port) error {
+func (inst *openstackInstance) OpenPorts(machineId string, ports []params.Port) error {
 	if inst.e.Config().FirewallMode() != config.FwInstance {
 		return fmt.Errorf("invalid firewall mode for opening ports on instance: %q",
 			inst.e.Config().FirewallMode())
@@ -366,7 +367,7 @@
 	return nil
 }
 
-func (inst *instance) ClosePorts(machineId string, ports []params.Port) error {
+func (inst *openstackInstance) ClosePorts(machineId string, ports []params.Port) error {
 	if inst.e.Config().FirewallMode() != config.FwInstance {
 		return fmt.Errorf("invalid firewall mode for closing ports on instance: %q",
 			inst.e.Config().FirewallMode())
@@ -379,7 +380,7 @@
 	return nil
 }
 
-func (inst *instance) Ports(machineId string) ([]params.Port, error) {
+func (inst *openstackInstance) Ports(machineId string) ([]params.Port, error) {
 	if inst.e.Config().FirewallMode() != config.FwInstance {
 		return nil, fmt.Errorf("invalid firewall mode for retrieving ports from instance: %q",
 			inst.e.Config().FirewallMode())
@@ -507,7 +508,7 @@
 	if err != nil {
 		// ignore error on StopInstance because the previous error is
 		// more important.
-		e.StopInstances([]environs.Instance{inst})
+		e.StopInstances([]instance.Instance{inst})
 		return fmt.Errorf("cannot save state: %v", err)
 	}
 	// TODO make safe in the case of racing Bootstraps
@@ -545,7 +546,7 @@
 			if inst == nil {
 				continue
 			}
-			name, err := inst.(*instance).DNSName()
+			name, err := inst.(*openstackInstance).DNSName()
 			if err != nil {
 				continue
 			}
@@ -657,7 +658,7 @@
 	return e.imageBaseURLs, nil
 }
 
-func (e *environ) StartInstance(machineId, machineNonce string, series string, cons constraints.Value, info *state.Info, apiInfo *api.Info) (environs.Instance, error) {
+func (e *environ) StartInstance(machineId, machineNonce string, series string, cons constraints.Value, info *state.Info, apiInfo *api.Info) (instance.Instance, error) {
 	possibleTools, err := environs.FindInstanceTools(e, series, cons)
 	if err != nil {
 		return nil, err
@@ -767,7 +768,7 @@
 
 // startInstance is the internal version of StartInstance, used by Bootstrap
 // as well as via StartInstance itself.
-func (e *environ) startInstance(scfg *startInstanceParams) (environs.Instance, error) {
+func (e *environ) startInstance(scfg *startInstanceParams) (instance.Instance, error) {
 	series := scfg.possibleTools.Series()
 	if len(series) != 1 {
 		return nil, fmt.Errorf("expected single series, got %v", series)
@@ -832,7 +833,7 @@
 	if err != nil {
 		return nil, fmt.Errorf("cannot get started instance: %v", err)
 	}
-	inst := &instance{e, detail, ""}
+	inst := &openstackInstance{e, detail, ""}
 	log.Infof("environs/openstack: started instance %q", inst.Id())
 	if scfg.withPublicIP {
 		if err := e.assignPublicIP(publicIP, string(inst.Id())); err != nil {
@@ -847,12 +848,12 @@
 	return inst, nil
 }
 
-func (e *environ) StopInstances(insts []environs.Instance) error {
+func (e *environ) StopInstances(insts []instance.Instance) error {
 	ids := make([]state.InstanceId, len(insts))
 	for i, inst := range insts {
-		instanceValue, ok := inst.(*instance)
+		instanceValue, ok := inst.(*openstackInstance)
 		if !ok {
-			return errors.New("Incompatible environs.Instance supplied")
+			return errors.New("Incompatible instance.Instance supplied")
 		}
 		ids[i] = instanceValue.Id()
 	}
@@ -863,7 +864,7 @@
 // collectInstances tries to get information on each instance id in ids.
 // It fills the slots in the given map for known servers with status
 // either ACTIVE or BUILD. Returns a list of missing ids.
-func (e *environ) collectInstances(ids []state.InstanceId, out map[state.InstanceId]environs.Instance) []state.InstanceId {
+func (e *environ) collectInstances(ids []state.InstanceId, out map[state.InstanceId]instance.Instance) []state.InstanceId {
 	var err error
 	serversById := make(map[string]nova.ServerDetail)
 	if len(ids) == 1 {
@@ -887,7 +888,7 @@
 	for _, id := range ids {
 		if server, found := serversById[string(id)]; found {
 			if server.Status == nova.StatusActive || server.Status == nova.StatusBuild {
-				out[id] = &instance{e, &server, ""}
+				out[id] = &openstackInstance{e, &server, ""}
 			}
 			continue
 		}
@@ -896,12 +897,12 @@
 	return missing
 }
 
-func (e *environ) Instances(ids []state.InstanceId) ([]environs.Instance, error) {
+func (e *environ) Instances(ids []state.InstanceId) ([]instance.Instance, error) {
 	if len(ids) == 0 {
 		return nil, nil
 	}
 	missing := ids
-	found := make(map[state.InstanceId]environs.Instance)
+	found := make(map[state.InstanceId]instance.Instance)
 	// Make a series of requests to cope with eventual consistency.
 	// Each request will attempt to add more instances to the requested
 	// set.
@@ -913,7 +914,7 @@
 	if len(found) == 0 {
 		return nil, environs.ErrNoInstances
 	}
-	insts := make([]environs.Instance, len(ids))
+	insts := make([]instance.Instance, len(ids))
 	var err error
 	for i, id := range ids {
 		if inst := found[id]; inst != nil {
@@ -925,7 +926,7 @@
 	return insts, err
 }
 
-func (e *environ) AllInstances() (insts []environs.Instance, err error) {
+func (e *environ) AllInstances() (insts []instance.Instance, err error) {
 	servers, err := e.nova().ListServersDetail(e.machinesFilter())
 	if err != nil {
 		return nil, err
@@ -933,13 +934,13 @@
 	for _, server := range servers {
 		if server.Status == nova.StatusActive || server.Status == nova.StatusBuild {
 			var s = server
-			insts = append(insts, &instance{e, &s, ""})
+			insts = append(insts, &openstackInstance{e, &s, ""})
 		}
 	}
 	return insts, err
 }
 
-func (e *environ) Destroy(ensureInsts []environs.Instance) error {
+func (e *environ) Destroy(ensureInsts []instance.Instance) error {
 	log.Infof("environs/openstack: destroying environment %q", e.name)
 	insts, err := e.AllInstances()
 	if err != nil {
@@ -955,7 +956,7 @@
 	// Add any instances we've been told about but haven't yet shown
 	// up in the instance list.
 	for _, inst := range ensureInsts {
-		id := state.InstanceId(inst.(*instance).Id())
+		id := state.InstanceId(inst.(*openstackInstance).Id())
 		if !found[id] {
 			ids = append(ids, id)
 			found[id] = true

=== modified file 'environs/openstack/provider_test.go'
--- environs/openstack/provider_test.go	2013-05-20 00:10:30 +0000
+++ environs/openstack/provider_test.go	2013-06-17 04:13:30 +0000
@@ -8,8 +8,8 @@
 	. "launchpad.net/gocheck"
 	"launchpad.net/goose/identity"
 	"launchpad.net/goose/nova"
-	"launchpad.net/juju-core/environs"
 	"launchpad.net/juju-core/environs/openstack"
+	"launchpad.net/juju-core/instance"
 	"testing"
 )
 
@@ -44,14 +44,14 @@
 	{
 		summary:  "missing",
 		expected: "",
-		failure:  environs.ErrNoDNSName,
+		failure:  instance.ErrNoDNSName,
 	},
 	{
 		summary:  "empty",
 		private:  []nova.IPAddress{},
 		networks: []string{"private"},
 		expected: "",
-		failure:  environs.ErrNoDNSName,
+		failure:  instance.ErrNoDNSName,
 	},
 	{
 		summary:  "private only",
@@ -110,7 +110,7 @@
 		private:  []nova.IPAddress{{6, "::dead:beef:f00d"}},
 		networks: []string{"private"},
 		expected: "",
-		failure:  environs.ErrNoDNSName,
+		failure:  instance.ErrNoDNSName,
 	},
 }
 

=== added directory 'instance'
=== added file 'instance/instance.go'
--- instance/instance.go	1970-01-01 00:00:00 +0000
+++ instance/instance.go	2013-06-17 04:13:30 +0000
@@ -0,0 +1,41 @@
+// Copyright 2013 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package instance
+
+import (
+	"errors"
+
+	"launchpad.net/juju-core/state"
+	"launchpad.net/juju-core/state/api/params"
+)
+
+var ErrNoDNSName = errors.New("DNS name not allocated")
+
+// Instance represents the the realization of a machine in state.
+type Instance interface {
+	// Id returns a provider-generated identifier for the Instance.
+	Id() state.InstanceId
+
+	// DNSName returns the DNS name for the instance.
+	// If the name is not yet allocated, it will return
+	// an ErrNoDNSName error.
+	DNSName() (string, error)
+
+	// WaitDNSName returns the DNS name for the instance,
+	// waiting until it is allocated if necessary.
+	WaitDNSName() (string, error)
+
+	// OpenPorts opens the given ports on the instance, which
+	// should have been started with the given machine id.
+	OpenPorts(machineId string, ports []params.Port) error
+
+	// ClosePorts closes the given ports on the instance, which
+	// should have been started with the given machine id.
+	ClosePorts(machineId string, ports []params.Port) error
+
+	// Ports returns the set of ports open on the instance, which
+	// should have been started with the given machine id.
+	// The ports are returned as sorted by state.SortPorts.
+	Ports(machineId string) ([]params.Port, error)
+}

=== modified file 'juju/testing/conn.go'
--- juju/testing/conn.go	2013-05-02 15:55:42 +0000
+++ juju/testing/conn.go	2013-06-17 04:13:30 +0000
@@ -12,6 +12,7 @@
 	"launchpad.net/juju-core/environs"
 	"launchpad.net/juju-core/environs/config"
 	"launchpad.net/juju-core/environs/dummy"
+	"launchpad.net/juju-core/instance"
 	"launchpad.net/juju-core/juju"
 	"launchpad.net/juju-core/state"
 	"launchpad.net/juju-core/state/api"
@@ -77,7 +78,7 @@
 
 // StartInstance is a test helper function that starts an instance on the
 // environment using the current series and invalid info states.
-func StartInstance(c *C, env environs.Environ, machineId string) environs.Instance {
+func StartInstance(c *C, env environs.Environ, machineId string) instance.Instance {
 	series := config.DefaultSeries
 	inst, err := env.StartInstance(
 		machineId,

=== modified file 'state/container.go'
--- state/container.go	2013-06-11 22:40:34 +0000
+++ state/container.go	2013-06-17 04:13:30 +0000
@@ -88,9 +88,9 @@
 	return ops
 }
 
-// parentId returns the id of the host machine if machineId a container id, or ""
+// ParentId returns the id of the host machine if machineId a container id, or ""
 // if machineId is not for a container.
-func parentId(machineId string) string {
+func ParentId(machineId string) string {
 	idParts := strings.Split(machineId, "/")
 	if len(idParts) < 3 {
 		return ""
@@ -98,6 +98,18 @@
 	return strings.Join(idParts[:len(idParts)-2], "/")
 }
 
+// NestingLevel returns how many levels of nesting exist for a machine id.
+func NestingLevel(machineId string) int {
+	idParts := strings.Split(machineId, "/")
+	return (len(idParts) - 1) / 2
+}
+
+// TopParentId returns the id of the top level host machine for a container id.
+func TopParentId(machineId string) string {
+	idParts := strings.Split(machineId, "/")
+	return idParts[0]
+}
+
 // removeContainerRefOps returns the txn.Op's necessary to remove a machine container record.
 // These include removing the record itself and updating the host machine's children property.
 func removeContainerRefOps(st *State, machineId string) []txn.Op {
@@ -108,7 +120,7 @@
 		Remove: true,
 	}
 	// If the machine is a container, figure out it's parent host.
-	parentId := parentId(machineId)
+	parentId := ParentId(machineId)
 	if parentId == "" {
 		return []txn.Op{removeRefOp}
 	}

=== modified file 'state/export_test.go'
--- state/export_test.go	2013-05-02 15:55:42 +0000
+++ state/export_test.go	2013-06-17 04:13:30 +0000
@@ -93,6 +93,10 @@
 	return sch
 }
 
+func MachineIdLessThan(id1, id2 string) bool {
+	return machineIdLessThan(id1, id2)
+}
+
 func init() {
 	logSize = logSizeTests
 }

=== modified file 'state/machine.go'
--- state/machine.go	2013-06-13 14:39:31 +0000
+++ state/machine.go	2013-06-17 04:13:30 +0000
@@ -107,15 +107,26 @@
 	return machineGlobalKey(m.doc.Id)
 }
 
+const machineTagPrefix = "machine-"
+
 // MachineTag returns the tag for the
 // machine with the given id.
 func MachineTag(id string) string {
-	tag := fmt.Sprintf("machine-%s", id)
+	tag := fmt.Sprintf("%s%s", machineTagPrefix, id)
 	// Containers require "/" to be replaced by "-".
 	tag = strings.Replace(tag, "/", "-", -1)
 	return tag
 }
 
+// MachineIdFromTag returns the machine id that was used to create the tag.
+func MachineIdFromTag(tag string) string {
+	// Strip off the "machine-" prefix.
+	id := tag[len(machineTagPrefix):]
+	// Put the slashes back.
+	id = strings.Replace(id, "-", "/", -1)
+	return id
+}
+
 // Tag returns a name identifying the machine that is safe to use
 // as a file name.  The returned name will be different from other
 // Tag values returned by any other entities from the same state.
@@ -240,7 +251,7 @@
 
 // ParentId returns the Id of the host machine if this machine is a container.
 func (m *Machine) ParentId() (string, bool) {
-	parentId := parentId(m.Id())
+	parentId := ParentId(m.Id())
 	return parentId, parentId != ""
 }
 

=== modified file 'state/machine_test.go'
--- state/machine_test.go	2013-06-12 00:01:40 +0000
+++ state/machine_test.go	2013-06-17 04:13:30 +0000
@@ -192,6 +192,15 @@
 	c.Assert(state.MachineTag("10/lxc/1"), Equals, "machine-10-lxc-1")
 }
 
+func (s *MachineSuite) TestMachineIdFromTag(c *C) {
+	c.Assert(state.MachineIdFromTag("machine-10"), Equals, "10")
+	// Check a container id.
+	c.Assert(state.MachineIdFromTag("machine-10-lxc-1"), Equals, "10/lxc/1")
+	// Check reversability.
+	nested := "2/kvm/0/lxc/3"
+	c.Assert(state.MachineIdFromTag(state.MachineTag(nested)), Equals, nested)
+}
+
 func (s *MachineSuite) TestSetMongoPassword(c *C) {
 	testSetMongoPassword(c, func(st *state.State) (entity, error) {
 		return st.Machine(s.machine.Id())

=== modified file 'state/state.go'
--- state/state.go	2013-06-13 14:39:31 +0000
+++ state/state.go	2013-06-17 04:13:30 +0000
@@ -378,10 +378,49 @@
 func (ms machineDocSlice) Len() int      { return len(ms) }
 func (ms machineDocSlice) Swap(i, j int) { ms[i], ms[j] = ms[j], ms[i] }
 func (ms machineDocSlice) Less(i, j int) bool {
-	// There's nothing we can do with errors at this point.
-	m1, _ := strconv.Atoi(ms[i].Id)
-	m2, _ := strconv.Atoi(ms[j].Id)
-	return m1 < m2
+	return machineIdLessThan(ms[i].Id, ms[j].Id)
+}
+
+// machineIdLessThan returns true if id1 < id2, false otherwise.
+// Machine ids may include "/" separators if they are for a container so
+// the comparison is done by comparing the id component values from
+// left to right (most significant part to least significant). Ids for
+// host machines are always less than ids for their containers.
+func machineIdLessThan(id1, id2 string) bool {
+	// Most times, we are dealing with host machines and not containers, so we will
+	// try interpreting the ids as ints - this will be faster than dealing with the
+	// container ids below.
+	mint1, err1 := strconv.Atoi(id1)
+	mint2, err2 := strconv.Atoi(id2)
+	if err1 == nil && err2 == nil {
+		return mint1 < mint2
+	}
+	// We have at least one container id so it gets complicated.
+	idParts1 := strings.Split(id1, "/")
+	idParts2 := strings.Split(id2, "/")
+	nrParts1 := len(idParts1)
+	nrParts2 := len(idParts2)
+	minLen := nrParts1
+	if nrParts2 < minLen {
+		minLen = nrParts2
+	}
+	for x := 0; x < minLen; x++ {
+		m1 := idParts1[x]
+		m2 := idParts2[x]
+		if m1 == m2 {
+			continue
+		}
+		// See if the id part is a container type, and if so compare directly.
+		if x%2 == 1 {
+			return m1 < m2
+		}
+		// Compare the integer ids.
+		// There's nothing we can do with errors at this point.
+		mint1, _ := strconv.Atoi(m1)
+		mint2, _ := strconv.Atoi(m2)
+		return mint1 < mint2
+	}
+	return nrParts1 < nrParts2
 }
 
 // Machine returns the machine with the given id.

=== modified file 'state/state_test.go'
--- state/state_test.go	2013-06-13 14:39:31 +0000
+++ state/state_test.go	2013-06-17 04:13:30 +0000
@@ -331,6 +331,39 @@
 	c.Assert(m.CheckProvisioned(state.BootstrapNonce), Equals, true)
 }
 
+func (s *StateSuite) TestAddContainerToInjectedMachine(c *C) {
+	oneJob := []state.MachineJob{state.JobHostUnits}
+	m0, err := s.State.InjectMachine("series", emptyCons, state.InstanceId("i-mindustrious"), state.JobHostUnits, state.JobManageEnviron)
+	c.Assert(err, IsNil)
+
+	// Add first container.
+	params := state.AddMachineParams{
+		ParentId:      "0",
+		ContainerType: state.LXC,
+		Series:        "series",
+		Jobs:          []state.MachineJob{state.JobHostUnits},
+	}
+	m, err := s.State.AddMachineWithConstraints(&params)
+	c.Assert(err, IsNil)
+	c.Assert(m.Id(), Equals, "0/lxc/0")
+	c.Assert(m.Series(), Equals, "series")
+	c.Assert(m.ContainerType(), Equals, state.LXC)
+	mcons, err := m.Constraints()
+	c.Assert(err, IsNil)
+	c.Assert(mcons, DeepEquals, emptyCons)
+	c.Assert(m.Jobs(), DeepEquals, oneJob)
+	s.assertMachineContainers(c, m0, []string{"0/lxc/0"})
+
+	// Add second container.
+	m, err = s.State.AddMachineWithConstraints(&params)
+	c.Assert(err, IsNil)
+	c.Assert(m.Id(), Equals, "0/lxc/1")
+	c.Assert(m.Series(), Equals, "series")
+	c.Assert(m.ContainerType(), Equals, state.LXC)
+	c.Assert(m.Jobs(), DeepEquals, oneJob)
+	s.assertMachineContainers(c, m0, []string{"0/lxc/0", "0/lxc/1"})
+}
+
 func (s *StateSuite) TestReadMachine(c *C) {
 	machine, err := s.State.AddMachine("series", state.JobHostUnits)
 	c.Assert(err, IsNil)
@@ -346,6 +379,19 @@
 	c.Assert(errors.IsNotFoundError(err), Equals, true)
 }
 
+func (s *StateSuite) TestMachineIdLessThan(c *C) {
+	c.Assert(state.MachineIdLessThan("0", "0"), Equals, false)
+	c.Assert(state.MachineIdLessThan("0", "1"), Equals, true)
+	c.Assert(state.MachineIdLessThan("1", "0"), Equals, false)
+	c.Assert(state.MachineIdLessThan("10", "2"), Equals, false)
+	c.Assert(state.MachineIdLessThan("0", "0/lxc/0"), Equals, true)
+	c.Assert(state.MachineIdLessThan("0/lxc/0", "0"), Equals, false)
+	c.Assert(state.MachineIdLessThan("1", "0/lxc/0"), Equals, false)
+	c.Assert(state.MachineIdLessThan("0/lxc/0", "1"), Equals, true)
+	c.Assert(state.MachineIdLessThan("0/lxc/0/lxc/1", "0/lxc/0"), Equals, false)
+	c.Assert(state.MachineIdLessThan("0/kvm/0", "0/lxc/0"), Equals, true)
+}
+
 func (s *StateSuite) TestAllMachines(c *C) {
 	numInserts := 42
 	for i := 0; i < numInserts; i++ {
@@ -1559,3 +1605,21 @@
 	}
 	assertNoCleanupChange(c, st, cw)
 }
+
+func (s *StateSuite) TestNestingLevel(c *C) {
+	c.Assert(state.NestingLevel("0"), Equals, 0)
+	c.Assert(state.NestingLevel("0/lxc/1"), Equals, 1)
+	c.Assert(state.NestingLevel("0/lxc/1/kvm/0"), Equals, 2)
+}
+
+func (s *StateSuite) TestTopParentId(c *C) {
+	c.Assert(state.TopParentId("0"), Equals, "0")
+	c.Assert(state.TopParentId("0/lxc/1"), Equals, "0")
+	c.Assert(state.TopParentId("0/lxc/1/kvm/2"), Equals, "0")
+}
+
+func (s *StateSuite) TestParentId(c *C) {
+	c.Assert(state.ParentId("0"), Equals, "")
+	c.Assert(state.ParentId("0/lxc/1"), Equals, "0")
+	c.Assert(state.ParentId("0/lxc/1/kvm/0"), Equals, "0/lxc/1")
+}

=== modified file 'worker/firewaller/firewaller_test.go'
--- worker/firewaller/firewaller_test.go	2013-05-27 03:00:31 +0000
+++ worker/firewaller/firewaller_test.go	2013-06-17 04:13:30 +0000
@@ -5,9 +5,9 @@
 
 import (
 	. "launchpad.net/gocheck"
-	"launchpad.net/juju-core/environs"
 	"launchpad.net/juju-core/environs/config"
 	"launchpad.net/juju-core/environs/dummy"
+	"launchpad.net/juju-core/instance"
 	"launchpad.net/juju-core/juju/testing"
 	"launchpad.net/juju-core/state"
 	"launchpad.net/juju-core/state/api/params"
@@ -33,7 +33,7 @@
 
 // assertPorts retrieves the open ports of the instance and compares them
 // to the expected.
-func (s *FirewallerSuite) assertPorts(c *C, inst environs.Instance, machineId string, expected []params.Port) {
+func (s *FirewallerSuite) assertPorts(c *C, inst instance.Instance, machineId string, expected []params.Port) {
 	s.State.StartSync()
 	start := time.Now()
 	for {
@@ -133,7 +133,7 @@
 }
 
 // startInstance starts a new instance for the given machine.
-func (s *FirewallerSuite) startInstance(c *C, m *state.Machine) environs.Instance {
+func (s *FirewallerSuite) startInstance(c *C, m *state.Machine) instance.Instance {
 	inst := testing.StartInstance(c, s.Conn.Environ, m.Id())
 	err := m.SetProvisioned(inst.Id(), "fake_nonce")
 	c.Assert(err, IsNil)

=== modified file 'worker/provisioner/broker.go'
--- worker/provisioner/broker.go	2013-06-07 01:36:48 +0000
+++ worker/provisioner/broker.go	2013-06-17 04:13:30 +0000
@@ -5,7 +5,7 @@
 
 import (
 	"launchpad.net/juju-core/constraints"
-	"launchpad.net/juju-core/environs"
+	"launchpad.net/juju-core/instance"
 	"launchpad.net/juju-core/state"
 	"launchpad.net/juju-core/state/api"
 )
@@ -17,11 +17,11 @@
 	// unique within an environment, is used by juju to protect against the
 	// consequences of multiple instances being started with the same machine
 	// id.
-	StartInstance(machineId, machineNonce string, series string, cons constraints.Value, info *state.Info, apiInfo *api.Info) (environs.Instance, error)
+	StartInstance(machineId, machineNonce string, series string, cons constraints.Value, info *state.Info, apiInfo *api.Info) (instance.Instance, error)
 
 	// StopInstances shuts down the given instances.
-	StopInstances([]environs.Instance) error
+	StopInstances([]instance.Instance) error
 
 	// AllInstances returns all instances currently known to the broker.
-	AllInstances() ([]environs.Instance, error)
+	AllInstances() ([]instance.Instance, error)
 }

=== modified file 'worker/provisioner/export_test.go'
--- worker/provisioner/export_test.go	2013-05-02 15:55:42 +0000
+++ worker/provisioner/export_test.go	2013-06-17 04:13:30 +0000
@@ -11,13 +11,13 @@
 // exported so we can manually close the Provisioners underlying
 // state connection.
 func (p *Provisioner) CloseState() error {
-	return p.st.Close()
+	return p.state.Close()
 }
 
 // exported so we can discover all machines visible to the
 // Provisioners state connection.
 func (p *Provisioner) AllMachines() ([]*state.Machine, error) {
-	return p.st.AllMachines()
+	return p.state.AllMachines()
 }
 
 func (o *configObserver) SetObserver(observer chan<- *config.Config) {

=== modified file 'worker/provisioner/provisioner.go'
--- worker/provisioner/provisioner.go	2013-06-11 02:15:31 +0000
+++ worker/provisioner/provisioner.go	2013-06-17 04:13:30 +0000
@@ -19,7 +19,7 @@
 
 // Provisioner represents a running provisioning worker.
 type Provisioner struct {
-	st        *state.State
+	state     *state.State
 	machineId string // Which machine runs the provisioner.
 	environ   environs.Environ
 	tomb      tomb.Tomb
@@ -46,7 +46,7 @@
 // and allocates them to the new machines.
 func NewProvisioner(st *state.State, machineId string) *Provisioner {
 	p := &Provisioner{
-		st:        st,
+		state:     st,
 		machineId: machineId,
 	}
 	go func() {
@@ -57,7 +57,7 @@
 }
 
 func (p *Provisioner) loop() error {
-	environWatcher := p.st.WatchEnvironConfig()
+	environWatcher := p.state.WatchEnvironConfig()
 	defer watcher.Stop(environWatcher, &p.tomb)
 
 	var err error
@@ -66,27 +66,18 @@
 		return err
 	}
 
-	// Get a new StateInfo from the environment: the one used to
-	// launch the agent may refer to localhost, which will be
-	// unhelpful when attempting to run an agent on a new machine.
-	stateInfo, apiInfo, err := p.environ.StateInfo()
-	if err != nil {
-		return err
-	}
-
 	// Start a new worker for the environment provider.
 
 	// Start responding to changes in machines, and to any further updates
 	// to the environment config.
-	machinesWatcher := p.st.WatchEnvironMachines()
+	machinesWatcher := p.state.WatchEnvironMachines()
 	environmentBroker := newEnvironBroker(p.environ)
 	environmentProvisioner := newProvisionerTask(
 		p.machineId,
-		p.st,
+		p.state,
 		machinesWatcher,
 		environmentBroker,
-		stateInfo,
-		apiInfo)
+	)
 	defer watcher.Stop(environmentProvisioner, &p.tomb)
 
 	for {
@@ -103,6 +94,11 @@
 			}
 			if err := p.setConfig(cfg); err != nil {
 				logger.Errorf("loaded invalid environment configuration: %v", err)
+				// We *shouldn't* ever get into a state here where we have an
+				// invalid config.  If we do, stop the task and let jujud
+				// restart the provisioner, which will then wait for valid
+				// config.
+				return err
 			}
 		}
 	}

=== modified file 'worker/provisioner/provisioner_task.go'
--- worker/provisioner/provisioner_task.go	2013-06-10 04:25:51 +0000
+++ worker/provisioner/provisioner_task.go	2013-06-17 04:13:30 +0000
@@ -6,8 +6,8 @@
 import (
 	"fmt"
 
-	"launchpad.net/juju-core/environs"
 	"launchpad.net/juju-core/errors"
+	"launchpad.net/juju-core/instance"
 	"launchpad.net/juju-core/state"
 	"launchpad.net/juju-core/state/api"
 	"launchpad.net/juju-core/state/api/params"
@@ -30,25 +30,17 @@
 	Changes() <-chan []string
 }
 
-type MachineGetter interface {
-	Machine(id string) (*state.Machine, error)
-}
-
 func newProvisionerTask(
 	machineId string,
-	machineGetter MachineGetter,
+	st *state.State,
 	watcher Watcher,
 	broker Broker,
-	stateInfo *state.Info,
-	apiInfo *api.Info,
 ) ProvisionerTask {
 	task := &provisionerTask{
 		machineId:      machineId,
-		machineGetter:  machineGetter,
+		state:          st,
 		machineWatcher: watcher,
 		broker:         broker,
-		stateInfo:      stateInfo,
-		apiInfo:        apiInfo,
 		machines:       make(map[string]*state.Machine),
 	}
 	go func() {
@@ -60,15 +52,13 @@
 
 type provisionerTask struct {
 	machineId      string
-	machineGetter  MachineGetter
+	state          *state.State
 	machineWatcher Watcher
 	broker         Broker
 	tomb           tomb.Tomb
-	stateInfo      *state.Info
-	apiInfo        *api.Info
 
 	// instance id -> instance
-	instances map[state.InstanceId]environs.Instance
+	instances map[state.InstanceId]instance.Instance
 	// machine id -> machine
 	machines map[string]*state.Machine
 }
@@ -159,7 +149,7 @@
 }
 
 func (task *provisionerTask) populateMachineMaps(ids []string) error {
-	task.instances = make(map[state.InstanceId]environs.Instance)
+	task.instances = make(map[state.InstanceId]instance.Instance)
 
 	instances, err := task.broker.AllInstances()
 	if err != nil {
@@ -174,7 +164,7 @@
 	// change list.
 	// TODO(thumper): update for API server later to get all machines in one go.
 	for _, id := range ids {
-		machine, err := task.machineGetter.Machine(id)
+		machine, err := task.state.Machine(id)
 		switch {
 		case errors.IsNotFoundError(err):
 			logger.Debugf("machine %q not found in state", id)
@@ -238,9 +228,9 @@
 }
 
 // findUnknownInstances finds instances which are not associated with a machine.
-func (task *provisionerTask) findUnknownInstances() ([]environs.Instance, error) {
+func (task *provisionerTask) findUnknownInstances() ([]instance.Instance, error) {
 	// Make a copy of the instances we know about.
-	instances := make(map[state.InstanceId]environs.Instance)
+	instances := make(map[state.InstanceId]instance.Instance)
 	for k, v := range task.instances {
 		instances[k] = v
 	}
@@ -250,7 +240,7 @@
 			delete(instances, instId)
 		}
 	}
-	var unknown []environs.Instance
+	var unknown []instance.Instance
 	for _, i := range instances {
 		unknown = append(unknown, i)
 	}
@@ -258,11 +248,11 @@
 	return unknown, nil
 }
 
-// instancesForMachines returns a list of environs.Instance that represent
+// instancesForMachines returns a list of instance.Instance that represent
 // the list of machines running in the provider. Missing machines are
 // omitted from the list.
-func (task *provisionerTask) instancesForMachines(machines []*state.Machine) []environs.Instance {
-	var instances []environs.Instance
+func (task *provisionerTask) instancesForMachines(machines []*state.Machine) []instance.Instance {
+	var instances []instance.Instance
 	for _, machine := range machines {
 		instId, ok := machine.InstanceId()
 		if ok {
@@ -276,7 +266,7 @@
 	return instances
 }
 
-func (task *provisionerTask) stopInstances(instances []environs.Instance) error {
+func (task *provisionerTask) stopInstances(instances []instance.Instance) error {
 	// Although calling StopInstance with an empty slice should produce no change in the
 	// provider, environs like dummy do not consider this a noop.
 	if len(instances) == 0 {
@@ -306,7 +296,7 @@
 	// state.Info as the PA.
 	stateInfo, apiInfo, err := task.setupAuthentication(machine)
 	if err != nil {
-		logger.Errorf("failed to setup authentication: %v", err)
+		logger.Warningf("failed to setup authentication for machine %q: %v", machine.Id(), err)
 		return err
 	}
 	cons, err := machine.Constraints()
@@ -355,6 +345,22 @@
 }
 
 func (task *provisionerTask) setupAuthentication(machine *state.Machine) (*state.Info, *api.Info, error) {
+	// Grab a new list of state and api addresses each time along with the
+	// cert, as these can change during a processing loop.  If for example the
+	// provisioner was starting 100 machines in one go, some of those may be
+	// new api servers.  We should take advantage of those ASAP.
+	stateAddresses, err := task.state.Addresses()
+	if err != nil {
+		// This will only return an error if the config becomes invalid.  In
+		// those situations, the provisioner bombs out and is restarted.
+		return nil, nil, fmt.Errorf("cannot get addresses from state: %v", err)
+	}
+	apiAddresses, err := task.state.APIAddresses()
+	if err != nil {
+		// Same for the api addresses.  We technically shouldn't get here if
+		// we didn't fail before, but best to check.
+		return nil, nil, fmt.Errorf("cannot get api addresses from state: %v", err)
+	}
 	password, err := utils.RandomPassword()
 	if err != nil {
 		return nil, nil, fmt.Errorf("cannot make password for machine %v: %v", machine, err)
@@ -362,11 +368,16 @@
 	if err := machine.SetMongoPassword(password); err != nil {
 		return nil, nil, fmt.Errorf("cannot set password for machine %v: %v", machine, err)
 	}
-	stateInfo := *task.stateInfo
-	stateInfo.Tag = machine.Tag()
-	stateInfo.Password = password
-	apiInfo := *task.apiInfo
-	apiInfo.Tag = machine.Tag()
-	apiInfo.Password = password
-	return &stateInfo, &apiInfo, nil
+	cert := task.state.CACert()
+	return &state.Info{
+			Addrs:    stateAddresses,
+			CACert:   cert,
+			Tag:      machine.Tag(),
+			Password: password,
+		}, &api.Info{
+			Addrs:    apiAddresses,
+			CACert:   cert,
+			Tag:      machine.Tag(),
+			Password: password,
+		}, nil
 }

=== modified file 'worker/provisioner/provisioner_test.go'
--- worker/provisioner/provisioner_test.go	2013-06-11 02:15:31 +0000
+++ worker/provisioner/provisioner_test.go	2013-06-17 04:13:30 +0000
@@ -12,10 +12,10 @@
 	"labix.org/v2/mgo/bson"
 	. "launchpad.net/gocheck"
 	"launchpad.net/juju-core/constraints"
-	"launchpad.net/juju-core/environs"
 	"launchpad.net/juju-core/environs/config"
 	"launchpad.net/juju-core/environs/dummy"
 	"launchpad.net/juju-core/errors"
+	"launchpad.net/juju-core/instance"
 	"launchpad.net/juju-core/juju/testing"
 	"launchpad.net/juju-core/state"
 	"launchpad.net/juju-core/state/api/params"
@@ -100,11 +100,11 @@
 	c.Assert(s.Stop(), IsNil)
 }
 
-func (s *ProvisionerSuite) checkStartInstance(c *C, m *state.Machine) environs.Instance {
+func (s *ProvisionerSuite) checkStartInstance(c *C, m *state.Machine) instance.Instance {
 	return s.checkStartInstanceCustom(c, m, "pork", constraints.Value{})
 }
 
-func (s *ProvisionerSuite) checkStartInstanceCustom(c *C, m *state.Machine, secret string, cons constraints.Value) (instance environs.Instance) {
+func (s *ProvisionerSuite) checkStartInstanceCustom(c *C, m *state.Machine, secret string, cons constraints.Value) (instance instance.Instance) {
 	s.State.StartSync()
 	for {
 		select {
@@ -160,7 +160,7 @@
 }
 
 // checkStopInstances checks that an instance has been stopped.
-func (s *ProvisionerSuite) checkStopInstances(c *C, instances ...environs.Instance) {
+func (s *ProvisionerSuite) checkStopInstances(c *C, instances ...instance.Instance) {
 	s.State.StartSync()
 	instanceIds := set.NewStrings()
 	for _, instance := range instances {
@@ -361,27 +361,6 @@
 	s.checkStartInstance(c, m)
 }
 
-func (s *ProvisionerSuite) TestProvisioningDoesOccurAfterInvalidEnvironmentPublished(c *C) {
-	p := provisioner.NewProvisioner(s.State, "0")
-	defer stop(c, p)
-
-	// place a new machine into the state
-	m, err := s.State.AddMachine(config.DefaultSeries, state.JobHostUnits)
-	c.Assert(err, IsNil)
-
-	s.checkStartInstance(c, m)
-
-	err = s.invalidateEnvironment(c)
-	c.Assert(err, IsNil)
-
-	// create a second machine
-	m, err = s.State.AddMachine(config.DefaultSeries, state.JobHostUnits)
-	c.Assert(err, IsNil)
-
-	// the PA should create it using the old environment
-	s.checkStartInstance(c, m)
-}
-
 func (s *ProvisionerSuite) TestProvisioningDoesNotProvisionTheSameMachineAfterRestart(c *C) {
 	p := provisioner.NewProvisioner(s.State, "0")
 	defer stop(c, p)
@@ -466,55 +445,3 @@
 	c.Assert(err, IsNil)
 	c.Assert(m0.Life(), Equals, state.Dying)
 }
-
-func (s *ProvisionerSuite) TestProvisioningRecoversAfterInvalidEnvironmentPublished(c *C) {
-	p := provisioner.NewProvisioner(s.State, "0")
-	defer stop(c, p)
-
-	// place a new machine into the state
-	m, err := s.State.AddMachine(config.DefaultSeries, state.JobHostUnits)
-	c.Assert(err, IsNil)
-	s.checkStartInstance(c, m)
-
-	err = s.invalidateEnvironment(c)
-	c.Assert(err, IsNil)
-	s.State.StartSync()
-
-	// create a second machine
-	m, err = s.State.AddMachine(config.DefaultSeries, state.JobHostUnits)
-	c.Assert(err, IsNil)
-
-	// the PA should create it using the old environment
-	s.checkStartInstance(c, m)
-
-	err = s.fixEnvironment()
-	c.Assert(err, IsNil)
-
-	// insert our observer
-	cfgObserver := make(chan *config.Config, 1)
-	p.SetObserver(cfgObserver)
-
-	cfg, err := s.State.EnvironConfig()
-	c.Assert(err, IsNil)
-	attrs := cfg.AllAttrs()
-	attrs["secret"] = "beef"
-	cfg, err = config.New(attrs)
-	c.Assert(err, IsNil)
-	err = s.State.SetEnvironConfig(cfg)
-
-	s.State.StartSync()
-
-	// wait for the PA to load the new configuration
-	select {
-	case <-cfgObserver:
-	case <-time.After(200 * time.Millisecond):
-		c.Fatalf("PA did not action config change")
-	}
-
-	// create a third machine
-	m, err = s.State.AddMachine(config.DefaultSeries, state.JobHostUnits)
-	c.Assert(err, IsNil)
-
-	// the PA should create it using the new environment
-	s.checkStartInstanceCustom(c, m, "beef", constraints.Value{})
-}