Merge lp:~thumper/juju-core/new-agent-format into lp:~go-bot/juju-core/trunk

[Tim Penhey]

Introduce the format 1.16 for agent confg.

With adding the new format, a migrate method was added
to the formatter interface to provide a defined place
to do additional upgrade data munging as needed.

When an old format config is read, it is immediately
migrated and written out using the current format.

The new format allows storing of key/value pairs. These
are now used instead of OS environment variables for
passing on the provider type, lxc bridge, and local
storage information.

Cloudinit no longer writes upstart scripts with extra
environment variables for the machine agents, but instead
writes the extra variables to the agent config. Due to
this change, the MachineEnvironment param was changed
to AgentEnvironment.

Also the StatePort was never read, so it has been removed
from information that is saved.

The cloudinit tests were modified to be agnostic to the
actual format of the agent.conf files.

https://codereview.appspot.com/13481043/

Introduce the format 1.16 for agent confg.

With adding the new format, a migrate method was added
to the formatter interface to provide a defined place
to do additional upgrade data munging as needed.

When an old format config is read, it is immediately
migrated and written out using the current format.

The new format allows storing of key/value pairs. These
are now used instead of OS environment variables for
passing on the provider type, lxc bridge, and local
storage information.

Cloudinit no longer writes upstart scripts with extra
environment variables for the machine agents, but instead
writes the extra variables to the agent config. Due to
this change, the MachineEnvironment param was changed
to AgentEnvironment.

Also the StatePort was never read, so it has been removed
from information that is saved.

The cloudinit tests were modified to be agnostic to the
actual format of the agent.conf files.

Previous review found here:
https://codereview.appspot.com/13421043/

https://codereview.appspot.com/13481043/

[Tim Penhey (thumper)]

Reviewers: mp+183558_code.launchpad.net,

Message:
Please take a look.

Description:
Introduce the format 1.16 for agent confg.

With adding the new format, a migrate method was added
to the formatter interface to provide a defined place
to do additional upgrade data munging as needed.

When an old format config is read, it is immediately
migrated and written out using the current format.

The new format allows storing of key/value pairs. These
are now used instead of OS environment variables for
passing on the provider type, lxc bridge, and local
storage information.

Cloudinit no longer writes upstart scripts with extra
environment variables for the machine agents, but instead
writes the extra variables to the agent config. Due to
this change, the MachineEnvironment param was changed
to AgentEnvironment.

Also the StatePort was never read, so it has been removed
from information that is saved.

The cloudinit tests were modified to be agnostic to the
actual format of the agent.conf files.

Previous review found here:
https://codereview.appspot.com/13421043/

https://code.launchpad.net/~thumper/juju-core/new-agent-format/+merge/183558

Requires:
https://code.launchpad.net/~thumper/juju-core/pass-through-agent-config/+merge/183543

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M agent/agent.go
   M agent/format-1.12.go
   M agent/format-1.12_whitebox_test.go
   A agent/format-1.16.go
   A agent/format-1.16_whitebox_test.go
   M agent/format.go
   M agent/format_whitebox_test.go
   M cmd/jujud/machine.go
   M container/lxc/lxc.go
   M environs/cloudinit.go
   M environs/cloudinit/cloudinit.go
   M environs/cloudinit/cloudinit_test.go
   M environs/cloudinit_test.go
   M environs/manual/agent.go
   M juju/osenv/vars.go
   M juju/testing/conn.go
   M provider/local/environ.go
   M provider/local/storage/worker.go
   M provider/maas/environ.go
   M worker/deployer/simple.go
   M worker/deployer/simple_test.go
   M worker/provisioner/lxc-broker.go
   M worker/provisioner/lxc-broker_test.go
   M worker/provisioner/provisioner.go
   M worker/provisioner/provisioner_test.go

[Ian Booth (wallyworld)]

Typo:
50 +// most recent state to disk. Since we different agent configs for each

64 + values map[string]string
I'm a little -1 on the name "values". "values" is kinda meaningless. Perhaps "params"? Or "configParams"?

154 +func (c *configInternal) Value(key string) string {
Similarly, "SetParam" perhaps? etc

605 + currentFormat = format_1_16
606 + previousFormat = format_1_12
I'm not sure about this - what happens if we have another new format? Then we have more than one previous format. Shouldn't we iterate over the previous formats till we find one that suits? Now all we do is assume it's the one previous format if loading with the new format fails.

There are white box tests for the config migration etc, but it may be good to have tests for the provisioner etc where an old format is present initially and after starting the format migration is validated? Or perhaps there are such tests and I missed them?

[William Reade (fwereade)]

ISTM that Configs are passed around a bit casually, and certainly not
goroutine-safely, and that this will be a timebomb even if the usage
patterns today (we only write on startup) make it theoretically safe.
Discuss...

https://codereview.appspot.com/13481043/diff/1/agent/agent.go
File agent/agent.go (right):

https://codereview.appspot.com/13481043/diff/1/agent/agent.go#newcode85
agent/agent.go:85: // Set the value for the specified key.
It's nice to do complete sentences starting with the name of the thing
being documented.

And, man, the lack of Get prefixes is starting to bug me. It just feels
like wilful character-saving at the cost of consistency and clarity...

https://codereview.appspot.com/13481043/diff/1/agent/agent.go#newcode97
agent/agent.go:97: // written to disk, the mutex should be locked prior
to generating any disk
prior to generating disk state, or prior to reading the source state
from which the future disk state will be generated?

https://codereview.appspot.com/13481043/diff/1/agent/agent.go#newcode217
agent/agent.go:217: // that this should not be called more than once by
an agent, I feel that
Hmm... how far do we expect these to propagate? Will we be cloning them?
If so, surely writes will never be safe... and if not, we'll need to
mutex everything we do with them. Right?

https://codereview.appspot.com/13481043/

[William Reade (fwereade)]

WIP due to inactivity

[Tim Penhey (thumper)]

Please take a look.

https://codereview.appspot.com/13481043/diff/1/agent/agent.go
File agent/agent.go (right):

https://codereview.appspot.com/13481043/diff/1/agent/agent.go#newcode85
agent/agent.go:85: // Set the value for the specified key.
On 2013/09/05 07:05:54, fwereade wrote:
> It's nice to do complete sentences starting with the name of the thing
being
> documented.

> And, man, the lack of Get prefixes is starting to bug me. It just
feels like
> wilful character-saving at the cost of consistency and clarity...

Done.

https://codereview.appspot.com/13481043/diff/1/agent/agent.go#newcode97
agent/agent.go:97: // written to disk, the mutex should be locked prior
to generating any disk
On 2013/09/05 07:05:54, fwereade wrote:
> prior to generating disk state, or prior to reading the source state
from which
> the future disk state will be generated?

Yes, this is what I mean. Effectively noting that it should be locked
at the start of the method, not just prior to writing.

https://codereview.appspot.com/13481043/diff/1/agent/agent.go#newcode217
agent/agent.go:217: // that this should not be called more than once by
an agent, I feel that
On 2013/09/05 07:05:54, fwereade wrote:
> Hmm... how far do we expect these to propagate? Will we be cloning
them? If so,
> surely writes will never be safe... and if not, we'll need to mutex
everything
> we do with them. Right?

We only need to mutex the mutable values, which is the values map. This
is now done. There is only one agent config, and it isn't regenerated,
only read once, and now passed around. With the addition of the mutex
around the setting and getting of the arbitrary values, this is now
thread (go routine) safe.

https://codereview.appspot.com/13481043/

[William Reade (fwereade)]

On 2013/09/12 02:21:06, thumper wrote:
> Please take a look.

> https://codereview.appspot.com/13481043/diff/1/agent/agent.go
> File agent/agent.go (right):

https://codereview.appspot.com/13481043/diff/1/agent/agent.go#newcode85
> agent/agent.go:85: // Set the value for the specified key.
> On 2013/09/05 07:05:54, fwereade wrote:
> > It's nice to do complete sentences starting with the name of the
thing being
> > documented.
> >
> > And, man, the lack of Get prefixes is starting to bug me. It just
feels like
> > wilful character-saving at the cost of consistency and clarity...

> Done.

https://codereview.appspot.com/13481043/diff/1/agent/agent.go#newcode97
> agent/agent.go:97: // written to disk, the mutex should be locked
prior to
> generating any disk
> On 2013/09/05 07:05:54, fwereade wrote:
> > prior to generating disk state, or prior to reading the source state
from
> which
> > the future disk state will be generated?

> Yes, this is what I mean. Effectively noting that it should be locked
at the
> start of the method, not just prior to writing.

OK, cool. Thanks.

https://codereview.appspot.com/13481043/diff/1/agent/agent.go#newcode217
> agent/agent.go:217: // that this should not be called more than once
by an
> agent, I feel that
> On 2013/09/05 07:05:54, fwereade wrote:
> > Hmm... how far do we expect these to propagate? Will we be cloning
them? If
> so,
> > surely writes will never be safe... and if not, we'll need to mutex
everything
> > we do with them. Right?

> We only need to mutex the mutable values, which is the values map.
This is now
> done. There is only one agent config, and it isn't regenerated, only
read once,
> and now passed around. With the addition of the mutex around the
setting and
> getting of the arbitrary values, this is now thread (go routine) safe.

Ah, ok. I'm a little concerned about likely future changes in which we
need, eg, an api worker to write out the state info for a machine that's
become a manager node. I guess it's not something we need to worry about
yet, but I'm still worried that it will not be obvious when the time
comes that we *do* need to worry. Will review properly imminently.

https://codereview.appspot.com/13481043/

[William Reade (fwereade)]

LGTM; comments but no blockers.

https://codereview.appspot.com/13481043/diff/6001/agent/agent.go
File agent/agent.go (right):

https://codereview.appspot.com/13481043/diff/6001/agent/agent.go#newcode103
agent/agent.go:103: // when mutliple go-routines may be adding things to
the agent config, and
mutliple

https://codereview.appspot.com/13481043/diff/6001/agent/agent.go#newcode111
agent/agent.go:111: // map. Retrieving and setting values here are
protected by the mutex.
To get the read/write thing right across the board, I think we have to
go with a model whereby you tell the config to persist by passing in a
mutate func... but even then, I remain nervous about the N cached copies
of mutually diverging state.

I'm starting to get the impression that this is maybe a case for the
Forbidden Design Pattern... but I can also appreciate that what we have
here is progress even if it's not perfection. Would you just rewrite the
comment above to make the limits of the current model clear and
explicit?

https://codereview.appspot.com/13481043/diff/6001/agent/format-1.16.go
File agent/format-1.16.go (right):

https://codereview.appspot.com/13481043/diff/6001/agent/format-1.16.go#newcode147
agent/format-1.16.go:147: // directory to ".old".
I agree that we should; or possibly that we should be atomically
swapping a symlink. That's surely out of scope, because of backward
compatibility if nothing else, but it deserves its own bug.

https://codereview.appspot.com/13481043/diff/6001/worker/provisioner/provisioner.go
File worker/provisioner/provisioner.go (right):

https://codereview.appspot.com/13481043/diff/6001/worker/provisioner/provisioner.go#newcode71
worker/provisioner/provisioner.go:71: agentConfig: agentConfig,
I feel like we ought to be able to get the id (tag better perhaps?) out
of the agent config.

https://codereview.appspot.com/13481043/diff/6001/worker/provisioner/provisioner.go#newcode177
worker/provisioner/provisioner.go:177: tools, err :=
agenttools.ReadTools(dataDir, version.Current)
I feel like most of agenttools should really be methods on agentConfig.
YMMV, not one for this CL, etc.

https://codereview.appspot.com/13481043/

[Roger Peppe (rogpeppe)]

A couple of suggestions based on a very quick skim.

https://codereview.appspot.com/13481043/diff/6001/agent/format-1.16.go
File agent/format-1.16.go (right):

https://codereview.appspot.com/13481043/diff/6001/agent/format-1.16.go#newcode119
agent/format-1.16.go:119: CACert:
base64.StdEncoding.EncodeToString(config.caCert),
can't we just use string here and below?
the certificates and keys are defined to be in PEM format (it is, or was
at least, even documented in this packages as such), which is ASCII
(and mostly base64 itself).

https://codereview.appspot.com/13481043/diff/6001/agent/format.go
File agent/format.go (right):

https://codereview.appspot.com/13481043/diff/6001/agent/format.go#newcode33
agent/format.go:33: formatFilename = "format"
Why do we need the file format to be in a separate file?
Can't we just store the format inside the file itself
(some care would obviously need to be taken when first introducing
the format specifier, and that the file format is sufficiently
general to cater for different formats).

Then we don't need to worry about the config file getting
out of step with its purported format.

Something like:

type agentConfig struct {
     Format string
     Contents string
}

should do the job (it's a pity that goyaml doesn't have the equivalent
of json.RawMessage).

https://codereview.appspot.com/13481043/

[Tim Penhey (thumper)]

https://codereview.appspot.com/13481043/diff/6001/agent/agent.go
File agent/agent.go (right):

https://codereview.appspot.com/13481043/diff/6001/agent/agent.go#newcode103
agent/agent.go:103: // when mutliple go-routines may be adding things to
the agent config, and
On 2013/09/12 13:21:44, fwereade wrote:
> mutliple

Done.

https://codereview.appspot.com/13481043/diff/6001/agent/agent.go#newcode111
agent/agent.go:111: // map. Retrieving and setting values here are
protected by the mutex.
On 2013/09/12 13:21:44, fwereade wrote:
> To get the read/write thing right across the board, I think we have to
go with a
> model whereby you tell the config to persist by passing in a mutate
func... but
> even then, I remain nervous about the N cached copies of mutually
diverging
> state.

> I'm starting to get the impression that this is maybe a case for the
Forbidden
> Design Pattern... but I can also appreciate that what we have here is
progress
> even if it's not perfection. Would you just rewrite the comment above
to make
> the limits of the current model clear and explicit?

I'm not sure I agree here. The agent configuration is defined at
creation time, and the only mutable values are in the values map.
Synchronisation around these and the write methods are sufficient.
Clearly if we are adding other mutating functions to the interface,
these should also be protected. We don't have N cached copies mutually
diverging. We have one instance used in multiple places.

I'll add a bit more to the comment here, but I'm not clear on where you
feel the issue is.

https://codereview.appspot.com/13481043/diff/6001/agent/format-1.16.go
File agent/format-1.16.go (right):

https://codereview.appspot.com/13481043/diff/6001/agent/format-1.16.go#newcode119
agent/format-1.16.go:119: CACert:
base64.StdEncoding.EncodeToString(config.caCert),
On 2013/09/12 14:50:58, rog wrote:
> can't we just use string here and below?
> the certificates and keys are defined to be in PEM format (it is, or
was at
> least, even documented in this packages as such), which is ASCII
> (and mostly base64 itself).

This was done to just take the []byte safely into a string.

If it was a string before, why are we passing it around as a byte slice?

We can always change future formats to do this if we decide we want to
change.

https://codereview.appspot.com/13481043/diff/6001/agent/format-1.16.go#newcode147
agent/format-1.16.go:147: // directory to ".old".
On 2013/09/12 13:21:44, fwereade wrote:
> I agree that we should; or possibly that we should be atomically
swapping a
> symlink. That's surely out of scope, because of backward compatibility
if
> nothing else, but it deserves its own bug.

Agree that it is out of scope for this branch, filed a bug and linked
it.

https://codereview.appspot.com/13481043/diff/6001/agent/format.go
File agent/format.go (right):

https://codereview.appspot.com/13481043/diff/6001/agent/format.go#newcode33
agent/format.go:33: formatFilename = "format"
On 2013/09/12 14:50:58, rog wrote:
> Why do we need the file format to be in a separate file?
> Can't we just store the format inside the file itself
> (some care would obviously need to be taken when first introducing...

=== modified file 'agent/agent.go'
--- agent/agent.go	2013-09-03 02:23:44 +0000
+++ agent/agent.go	2013-09-12 22:52:57 +0000
@@ -7,6 +7,7 @@
 	"fmt"
 	"path"
 	"regexp"
+	"sync"
 
 	"launchpad.net/loggo"
 
@@ -20,6 +21,26 @@
 
 var logger = loggo.GetLogger("juju.agent")
 
+const (
+	LxcBridge         = "LXC_BRIDGE"
+	ProviderType      = "PROVIDER_TYPE"
+	ContainerType     = "CONTAINER_TYPE"
+	StorageDir        = "STORAGE_DIR"
+	StorageAddr       = "STORAGE_ADDR"
+	SharedStorageDir  = "SHARED_STORAGE_DIR"
+	SharedStorageAddr = "SHARED_STORAGE_ADDR"
+)
+
+// The Config interface is the sole way that the agent gets access to the
+// configuration information for the machine and unit agents.  There should
+// only be one instance of a config object for any given agent, and this
+// interface is passed between multiple go routines.  The mutable methods are
+// protected by a mutex, and it is expected that the caller doesn't modify any
+// slice that may be returned.
+//
+// NOTE: should new mutating methods be added to this interface, consideration
+// is needed around the synchronisation as a single instance is used in
+// multiple go routines.
 type Config interface {
 	// DataDir returns the data directory. Each agent has a subdirectory
 	// containing the configuration files.
@@ -68,11 +89,33 @@
 
 	// APIServerDetails returns the details needed to run an API server.
 	APIServerDetails() (port int, cert, key []byte)
+
+	// Value returns the value associated with the key, or an empty string if
+	// the key is not found.
+	Value(key string) string
+
+	// SetValue updates the value for the specified key.
+	SetValue(key, value string)
 }
 
 // Ensure that the configInternal struct implements the Config interface.
 var _ Config = (*configInternal)(nil)
 
+// The configMutex should be locked before any writing to disk during the
+// write commands, and unlocked when the writing is complete.  This process
+// wide lock should stop any unintended concurrent writes.  This may happen
+// when multiple go-routines may be adding things to the agent config, and
+// wanting to persist them to disk. To ensure that the correct data is written
+// to disk, the mutex should be locked prior to generating any disk state.
+// This way calls that might get interleaved would always write the most
+// recent state to disk.  Since we have different agent configs for each
+// agent, and there is only one process for each agent, a simple mutex is
+// enough for concurrency.  The mutex should also be locked around any access
+// to mutable values, either setting or getting.  The only mutable value is
+// the values map.  Retrieving and setting values here are protected by the
+// mutex.  New mutating methods should also be synchronized using this mutex.
+var configMutex sync.Mutex
+
 type connectionDetails struct {
 	addresses []string
 	password  string
@@ -88,8 +131,8 @@
 	oldPassword     string
 	stateServerCert []byte
 	stateServerKey  []byte
-	statePort       int
 	apiPort         int
+	values          map[string]string
 }
 
 type AgentConfigParams struct {
@@ -100,6 +143,7 @@
 	StateAddresses []string
 	APIAddresses   []string
 	CACert         []byte
+	Values         map[string]string
 }
 
 func newConfig(params AgentConfigParams) (*configInternal, error) {
@@ -123,6 +167,7 @@
 		nonce:       params.Nonce,
 		caCert:      params.CACert,
 		oldPassword: params.Password,
+		values:      params.Values,
 	}
 	if len(params.StateAddresses) > 0 {
 		config.stateDetails = &connectionDetails{
@@ -137,6 +182,9 @@
 	if err := config.check(); err != nil {
 		return nil, err
 	}
+	if config.values == nil {
+		config.values = make(map[string]string)
+	}
 	return config, nil
 }
 
@@ -168,7 +216,6 @@
 	}
 	config.stateServerCert = params.StateServerCert
 	config.stateServerKey = params.StateServerKey
-	config.statePort = params.StatePort
 	config.apiPort = params.APIPort
 	return config, nil
 }
@@ -181,8 +228,22 @@
 // ReadConf reads configuration data for the given
 // entity from the given data directory.
 func ReadConf(dataDir, tag string) (Config, error) {
+	// Even though the ReadConf is done at the start of the agent loading, and
+	// that this should not be called more than once by an agent, I feel that
+	// not locking the mutex that is used to protect writes is wrong.
+	configMutex.Lock()
+	defer configMutex.Unlock()
 	dir := Dir(dataDir, tag)
-	config, err := currentFormatter.read(dir)
+	format, err := readFormat(dir)
+	if err != nil {
+		return nil, err
+	}
+	logger.Debugf("Reading agent config, format: %s", format)
+	formatter, err := newFormatter(format)
+	if err != nil {
+		return nil, err
+	}
+	config, err := formatter.read(dir)
 	if err != nil {
 		return nil, err
 	}
@@ -190,12 +251,18 @@
 	if err := config.check(); err != nil {
 		return nil, err
 	}
+
+	if format != currentFormat {
+		// Migrate the config to the new format.
+		currentFormatter.migrate(config)
+		// Write the content out in the new format.
+		if err := currentFormatter.write(config); err != nil {
+			logger.Errorf("Problem writing the agent config out in format: %s, %v", currentFormat, err)
+			return nil, err
+		}
+	}
+
 	return config, nil
-	// Read the format for the dir.
-	// Create a formatter for the format
-	// Read in the config
-	// If the format isn't the current format, call the upgrade function, and
-	// write out using the new formatter.
 }
 
 func requiredError(what string) error {
@@ -222,6 +289,22 @@
 	return result
 }
 
+func (c *configInternal) Value(key string) string {
+	configMutex.Lock()
+	defer configMutex.Unlock()
+	return c.values[key]
+}
+
+func (c *configInternal) SetValue(key, value string) {
+	configMutex.Lock()
+	defer configMutex.Unlock()
+	if value == "" {
+		delete(c.values, key)
+	} else {
+		c.values[key] = value
+	}
+}
+
 func (c *configInternal) APIServerDetails() (port int, cert, key []byte) {
 	return c.apiPort, c.stateServerCert, c.stateServerKey
 }
@@ -308,6 +391,9 @@
 
 // Write writes the agent configuration.
 func (c *configInternal) Write() error {
+	// Lock is taken prior to generating any content to write.
+	configMutex.Lock()
+	defer configMutex.Unlock()
 	return currentFormatter.write(c)
 }
 

=== modified file 'agent/format-1.12.go'
--- agent/format-1.12.go	2013-09-02 03:54:20 +0000
+++ agent/format-1.12.go	2013-09-12 22:52:57 +0000
@@ -102,8 +102,8 @@
 		oldPassword:     conf.OldPassword,
 		stateServerCert: conf.StateServerCert,
 		stateServerKey:  conf.StateServerKey,
-		statePort:       conf.StatePort,
 		apiPort:         conf.APIPort,
+		values:          map[string]string{},
 	}, nil
 }
 
@@ -111,7 +111,6 @@
 	format := &format_1_12Serialization{
 		StateServerCert: config.stateServerCert,
 		StateServerKey:  config.stateServerKey,
-		StatePort:       config.statePort,
 		APIPort:         config.apiPort,
 		OldPassword:     config.oldPassword,
 		MachineNonce:    config.nonce,
@@ -173,3 +172,6 @@
 	addCommand(`printf '%%s\n' %s > %s`, utils.ShQuote(string(data)), filename)
 	return commands, nil
 }
+
+func (*formatter_1_12) migrate(config *configInternal) {
+}

=== modified file 'agent/format-1.12_whitebox_test.go'
--- agent/format-1.12_whitebox_test.go	2013-09-02 03:57:53 +0000
+++ agent/format-1.12_whitebox_test.go	2013-09-12 22:52:57 +0000
@@ -20,15 +20,6 @@
 
 var _ = gc.Suite(&format_1_12Suite{})
 
-var agentParams = AgentConfigParams{
-	Tag:            "omg",
-	Password:       "sekrit",
-	CACert:         []byte("ca cert"),
-	StateAddresses: []string{"localhost:1234"},
-	APIAddresses:   []string{"localhost:1235"},
-	Nonce:          "a nonce",
-}
-
 func (s *format_1_12Suite) newConfig(c *gc.C) *configInternal {
 	params := agentParams
 	params.DataDir = c.MkDir()

=== added file 'agent/format-1.16.go'
--- agent/format-1.16.go	1970-01-01 00:00:00 +0000
+++ agent/format-1.16.go	2013-09-12 22:52:57 +0000
@@ -0,0 +1,207 @@
+// Copyright 2013 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package agent
+
+import (
+	"encoding/base64"
+	"io/ioutil"
+	"os"
+	"path"
+
+	"launchpad.net/goyaml"
+
+	"launchpad.net/juju-core/juju/osenv"
+)
+
+const (
+	format_1_16 = "format 1.16"
+	// Old environment variables that are now stored in agent config.
+	JujuLxcBridge         = "JUJU_LXC_BRIDGE"
+	JujuProviderType      = "JUJU_PROVIDER_TYPE"
+	JujuStorageDir        = "JUJU_STORAGE_DIR"
+	JujuStorageAddr       = "JUJU_STORAGE_ADDR"
+	JujuSharedStorageDir  = "JUJU_SHARED_STORAGE_DIR"
+	JujuSharedStorageAddr = "JUJU_SHARED_STORAGE_ADDR"
+)
+
+// formatter_1_16 is the formatter for the 1.16 format.
+type formatter_1_16 struct {
+}
+
+// format_1_16Serialization holds information for a given agent.
+type format_1_16Serialization struct {
+	Tag   string
+	Nonce string
+	// CACert is base64 encoded
+	CACert         string
+	StateAddresses []string `yaml:",omitempty"`
+	StatePassword  string   `yaml:",omitempty"`
+
+	APIAddresses []string `yaml:",omitempty"`
+	APIPassword  string   `yaml:",omitempty"`
+
+	OldPassword string
+	Values      map[string]string
+
+	// Only state server machines have these next three items
+	StateServerCert string `yaml:",omitempty"`
+	StateServerKey  string `yaml:",omitempty"`
+	APIPort         int    `yaml:",omitempty"`
+}
+
+// Ensure that the formatter_1_16 struct implements the formatter interface.
+var _ formatter = (*formatter_1_16)(nil)
+
+func (*formatter_1_16) configFile(dirName string) string {
+	return path.Join(dirName, "agent.conf")
+}
+
+// decode64 makes sure that for an empty string we have a nil slice, not an
+// empty slice, which is what the base64 DecodeString function returns.
+func (*formatter_1_16) decode64(value string) (result []byte, err error) {
+	if value != "" {
+		result, err = base64.StdEncoding.DecodeString(value)
+	}
+	return
+}
+
+func (formatter *formatter_1_16) read(dirName string) (*configInternal, error) {
+	data, err := ioutil.ReadFile(formatter.configFile(dirName))
+	if err != nil {
+		return nil, err
+	}
+	var format format_1_16Serialization
+	if err := goyaml.Unmarshal(data, &format); err != nil {
+		return nil, err
+	}
+	caCert, err := formatter.decode64(format.CACert)
+	if err != nil {
+		return nil, err
+	}
+	stateServerCert, err := formatter.decode64(format.StateServerCert)
+	if err != nil {
+		return nil, err
+	}
+	stateServerKey, err := formatter.decode64(format.StateServerKey)
+	if err != nil {
+		return nil, err
+	}
+	config := &configInternal{
+		tag:             format.Tag,
+		nonce:           format.Nonce,
+		caCert:          caCert,
+		oldPassword:     format.OldPassword,
+		stateServerCert: stateServerCert,
+		stateServerKey:  stateServerKey,
+		apiPort:         format.APIPort,
+		values:          format.Values,
+	}
+	if len(format.StateAddresses) > 0 {
+		config.stateDetails = &connectionDetails{
+			format.StateAddresses,
+			format.StatePassword,
+		}
+	}
+	if len(format.APIAddresses) > 0 {
+		config.apiDetails = &connectionDetails{
+			format.APIAddresses,
+			format.APIPassword,
+		}
+	}
+	return config, nil
+}
+
+func (formatter *formatter_1_16) makeFormat(config *configInternal) *format_1_16Serialization {
+	format := &format_1_16Serialization{
+		Tag:             config.tag,
+		Nonce:           config.nonce,
+		CACert:          base64.StdEncoding.EncodeToString(config.caCert),
+		OldPassword:     config.oldPassword,
+		StateServerCert: base64.StdEncoding.EncodeToString(config.stateServerCert),
+		StateServerKey:  base64.StdEncoding.EncodeToString(config.stateServerKey),
+		APIPort:         config.apiPort,
+		Values:          config.values,
+	}
+	if config.stateDetails != nil {
+		format.StateAddresses = config.stateDetails.addresses
+		format.StatePassword = config.stateDetails.password
+	}
+	if config.apiDetails != nil {
+		format.APIAddresses = config.apiDetails.addresses
+		format.APIPassword = config.apiDetails.password
+	}
+	return format
+}
+
+func (formatter *formatter_1_16) write(config *configInternal) error {
+	dirName := config.Dir()
+	conf := formatter.makeFormat(config)
+	data, err := goyaml.Marshal(conf)
+	if err != nil {
+		return err
+	}
+	// TODO(thumper): 2013-09-13 bug 1224725
+	// We should be writing to a temp dir first then rename.
+	// Writing the format file makes sure that dirName exists.  We should
+	// really be writing the format and new config files into a separate
+	// directory, and renaming the directory, and moving the old agend
+	// directory to ".old".
+	if err := writeFormatFile(dirName, format_1_16); err != nil {
+		return err
+	}
+	newFile := path.Join(dirName, "agent.conf-new")
+	if err := ioutil.WriteFile(newFile, data, 0600); err != nil {
+		return err
+	}
+	if err := os.Rename(newFile, formatter.configFile(dirName)); err != nil {
+		return err
+	}
+	return nil
+}
+
+func (formatter *formatter_1_16) writeCommands(config *configInternal) ([]string, error) {
+	dirName := config.Dir()
+	conf := formatter.makeFormat(config)
+	data, err := goyaml.Marshal(conf)
+	if err != nil {
+		return nil, err
+	}
+	commands := writeCommandsForFormat(dirName, format_1_16)
+	commands = append(commands,
+		writeFileCommands(formatter.configFile(dirName), string(data), 0600)...)
+	return commands, nil
+}
+
+func (*formatter_1_16) migrate(config *configInternal) {
+	for _, name := range []struct {
+		environment string
+		config      string
+	}{{
+		JujuProviderType,
+		ProviderType,
+	}, {
+		osenv.JujuContainerType,
+		ContainerType,
+	}, {
+		JujuLxcBridge,
+		LxcBridge,
+	}, {
+		JujuStorageDir,
+		StorageDir,
+	}, {
+		JujuStorageAddr,
+		StorageAddr,
+	}, {
+		JujuSharedStorageDir,
+		SharedStorageDir,
+	}, {
+		JujuSharedStorageAddr,
+		SharedStorageAddr,
+	}} {
+		value := os.Getenv(name.environment)
+		if value != "" {
+			config.values[name.config] = value
+		}
+	}
+}

=== added file 'agent/format-1.16_whitebox_test.go'
--- agent/format-1.16_whitebox_test.go	1970-01-01 00:00:00 +0000
+++ agent/format-1.16_whitebox_test.go	2013-09-12 22:52:57 +0000
@@ -0,0 +1,143 @@
+// Copyright 2013 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+// The format tests are white box tests, meaning that the tests are in the
+// same package as the code, as all the format details are internal to the
+// package.
+
+package agent
+
+import (
+	"os"
+	"path"
+
+	gc "launchpad.net/gocheck"
+
+	"launchpad.net/juju-core/juju/osenv"
+	"launchpad.net/juju-core/testing"
+	jc "launchpad.net/juju-core/testing/checkers"
+)
+
+type format_1_16Suite struct {
+	testing.LoggingSuite
+	formatter formatter_1_16
+}
+
+var _ = gc.Suite(&format_1_16Suite{})
+
+func (s *format_1_16Suite) newConfig(c *gc.C) *configInternal {
+	params := agentParams
+	params.DataDir = c.MkDir()
+	config, err := newConfig(params)
+	c.Assert(err, gc.IsNil)
+	return config
+}
+
+func (s *format_1_16Suite) TestWriteAgentConfig(c *gc.C) {
+	config := s.newConfig(c)
+	err := s.formatter.write(config)
+	c.Assert(err, gc.IsNil)
+
+	expectedLocation := path.Join(config.Dir(), "agent.conf")
+	fileInfo, err := os.Stat(expectedLocation)
+	c.Assert(err, gc.IsNil)
+	c.Assert(fileInfo.Mode().IsRegular(), jc.IsTrue)
+	c.Assert(fileInfo.Mode().Perm(), gc.Equals, os.FileMode(0600))
+	c.Assert(fileInfo.Size(), jc.GreaterThan, 0)
+
+	formatLocation := path.Join(config.Dir(), formatFilename)
+	fileInfo, err = os.Stat(formatLocation)
+	c.Assert(err, gc.IsNil)
+	c.Assert(fileInfo.Mode().IsRegular(), jc.IsTrue)
+	c.Assert(fileInfo.Mode().Perm(), gc.Equals, os.FileMode(0644))
+	c.Assert(fileInfo.Size(), jc.GreaterThan, 0)
+
+	formatContent, err := readFormat(config.Dir())
+	c.Assert(formatContent, gc.Equals, format_1_16)
+}
+
+func (s *format_1_16Suite) assertWriteAndRead(c *gc.C, config *configInternal) {
+	err := s.formatter.write(config)
+	c.Assert(err, gc.IsNil)
+	// The readConfig is missing the dataDir initially.
+	readConfig, err := s.formatter.read(config.Dir())
+	c.Assert(err, gc.IsNil)
+	c.Assert(readConfig.dataDir, gc.Equals, "")
+	// This is put in by the ReadConf method that we are avoiding using
+	// becuase it will have side-effects soon around migrating configs.
+	readConfig.dataDir = config.dataDir
+	c.Assert(readConfig, gc.DeepEquals, config)
+}
+
+func (s *format_1_16Suite) TestRead(c *gc.C) {
+	config := s.newConfig(c)
+	s.assertWriteAndRead(c, config)
+}
+
+func (s *format_1_16Suite) TestWriteCommands(c *gc.C) {
+	config := s.newConfig(c)
+	commands, err := s.formatter.writeCommands(config)
+	c.Assert(err, gc.IsNil)
+	c.Assert(commands, gc.HasLen, 5)
+	c.Assert(commands[0], gc.Matches, `mkdir -p '\S+/agents/omg'`)
+	c.Assert(commands[1], gc.Matches, `install -m 644 /dev/null '\S+/agents/omg/format'`)
+	c.Assert(commands[2], gc.Matches, `printf '%s\\n' '.*' > '\S+/agents/omg/format'`)
+	c.Assert(commands[3], gc.Matches, `install -m 600 /dev/null '\S+/agents/omg/agent.conf'`)
+	c.Assert(commands[4], gc.Matches, `printf '%s\\n' '(.|\n)*' > '\S+/agents/omg/agent.conf'`)
+}
+
+func (s *format_1_16Suite) TestReadWriteStateConfig(c *gc.C) {
+	stateParams := StateMachineConfigParams{
+		AgentConfigParams: agentParams,
+		StateServerCert:   []byte("some special cert"),
+		StateServerKey:    []byte("a special key"),
+		StatePort:         12345,
+		APIPort:           23456,
+	}
+	stateParams.DataDir = c.MkDir()
+	stateParams.Values = map[string]string{"foo": "bar", "wibble": "wobble"}
+	configInterface, err := NewStateMachineConfig(stateParams)
+	c.Assert(err, gc.IsNil)
+	config, ok := configInterface.(*configInternal)
+	c.Assert(ok, jc.IsTrue)
+
+	s.assertWriteAndRead(c, config)
+}
+
+func (s *format_1_16Suite) TestMigrate(c *gc.C) {
+	defer testing.PatchEnvironment(JujuLxcBridge, "lxc bridge")()
+	defer testing.PatchEnvironment(JujuProviderType, "provider type")()
+	defer testing.PatchEnvironment(osenv.JujuContainerType, "container type")()
+	defer testing.PatchEnvironment(JujuStorageDir, "storage dir")()
+	defer testing.PatchEnvironment(JujuStorageAddr, "storage addr")()
+	defer testing.PatchEnvironment(JujuSharedStorageDir, "shared storage dir")()
+	defer testing.PatchEnvironment(JujuSharedStorageAddr, "shared storage addr")()
+
+	config := s.newConfig(c)
+	s.formatter.migrate(config)
+
+	expected := map[string]string{
+		LxcBridge:         "lxc bridge",
+		ProviderType:      "provider type",
+		ContainerType:     "container type",
+		StorageDir:        "storage dir",
+		StorageAddr:       "storage addr",
+		SharedStorageDir:  "shared storage dir",
+		SharedStorageAddr: "shared storage addr",
+	}
+
+	c.Assert(config.values, gc.DeepEquals, expected)
+}
+
+func (s *format_1_16Suite) TestMigrateOnlySetsExisting(c *gc.C) {
+	defer testing.PatchEnvironment(JujuProviderType, "provider type")()
+
+	config := s.newConfig(c)
+	s.formatter.migrate(config)
+
+	expected := map[string]string{
+		ProviderType: "provider type",
+	}
+
+	c.Assert(config.values, gc.DeepEquals, expected)
+}

=== modified file 'agent/format.go'
--- agent/format.go	2013-09-02 03:54:20 +0000
+++ agent/format.go	2013-09-12 22:52:57 +0000
@@ -5,6 +5,12 @@
 
 import (
 	"fmt"
+	"io/ioutil"
+	"os"
+	"path"
+	"strings"
+
+	"launchpad.net/juju-core/utils"
 )
 
 // The format file in the agent config directory is used to identify the
@@ -25,10 +31,14 @@
 
 const (
 	formatFilename = "format"
-	currentFormat  = format_1_12
+	currentFormat  = format_1_16
+	previousFormat = format_1_12
 )
 
-var currentFormatter = &formatter_1_12{}
+var (
+	currentFormatter  = &formatter_1_16{}
+	previousFormatter = &formatter_1_12{}
+)
 
 // The formatter defines the two methods needed by the formatters for
 // translating to and from the internal, format agnostic, structure.
@@ -36,16 +46,55 @@
 	read(dirName string) (*configInternal, error)
 	write(config *configInternal) error
 	writeCommands(config *configInternal) ([]string, error)
+	// Migrate is called when upgrading from the previous format to the current format.
+	migrate(config *configInternal)
+}
+
+func formatFile(dirName string) string {
+	return path.Join(dirName, formatFilename)
 }
 
 func readFormat(dirName string) (string, error) {
-	return currentFormat, nil
+	contents, err := ioutil.ReadFile(formatFile(dirName))
+	// Once the previousFormat is defined to have a format file (1.16 or
+	// above), not finding a format file should be a real error.
+	if err != nil {
+		return previousFormat, nil
+	}
+	return strings.TrimSpace(string(contents)), nil
 }
 
 func newFormatter(format string) (formatter, error) {
 	switch format {
 	case currentFormat:
 		return currentFormatter, nil
+	case previousFormat:
+		return previousFormatter, nil
 	}
 	return nil, fmt.Errorf("unknown agent config format")
 }
+
+func writeFormatFile(dirName string, format string) error {
+	if err := os.MkdirAll(dirName, 0755); err != nil {
+		return err
+	}
+	newFile := path.Join(dirName, formatFilename+"-new")
+	if err := ioutil.WriteFile(newFile, []byte(format+"\n"), 0644); err != nil {
+		return err
+	}
+	return os.Rename(newFile, formatFile(dirName))
+}
+
+func writeFileCommands(filename, contents string, permission int) []string {
+	quotedFilename := utils.ShQuote(filename)
+	return []string{
+		fmt.Sprintf("install -m %o /dev/null %s", permission, quotedFilename),
+		fmt.Sprintf(`printf '%%s\n' %s > %s`, utils.ShQuote(contents), quotedFilename),
+	}
+}
+
+func writeCommandsForFormat(dirName, format string) []string {
+	commands := []string{"mkdir -p " + utils.ShQuote(dirName)}
+	commands = append(commands, writeFileCommands(formatFile(dirName), format, 0644)...)
+	return commands
+}

=== modified file 'agent/format_whitebox_test.go'
--- agent/format_whitebox_test.go	2013-09-02 03:57:53 +0000
+++ agent/format_whitebox_test.go	2013-09-12 22:52:57 +0000
@@ -4,6 +4,9 @@
 package agent
 
 import (
+	"io/ioutil"
+	"path"
+
 	gc "launchpad.net/gocheck"
 
 	"launchpad.net/juju-core/testing"
@@ -15,9 +18,35 @@
 
 var _ = gc.Suite(&formatSuite{})
 
+// The agentParams are used by the specific formatter whitebox tests, and is
+// located here for easy reuse.
+var agentParams = AgentConfigParams{
+	Tag:            "omg",
+	Password:       "sekrit",
+	CACert:         []byte("ca cert"),
+	StateAddresses: []string{"localhost:1234"},
+	APIAddresses:   []string{"localhost:1235"},
+	Nonce:          "a nonce",
+}
+
+func (*formatSuite) TestReadFormatEmptyDir(c *gc.C) {
+	// Since the previous format didn't have a format file, a missing format
+	// should return the previous format.  Once we are over the hump of
+	// missing format files, a missing format file should generate an error.
+	dir := c.MkDir()
+	format, err := readFormat(dir)
+	c.Assert(format, gc.Equals, previousFormat)
+	c.Assert(err, gc.IsNil)
+}
+
 func (*formatSuite) TestReadFormat(c *gc.C) {
-	format, err := readFormat("ignored")
-	c.Assert(format, gc.Equals, currentFormat)
+	dir := c.MkDir()
+	// Make sure that the write adds the carriage return to show that
+	// this is stripped off for the returned format.
+	err := ioutil.WriteFile(path.Join(dir, formatFilename), []byte("some format\n"), 0644)
+	c.Assert(err, gc.IsNil)
+	format, err := readFormat(dir)
+	c.Assert(format, gc.Equals, "some format")
 	c.Assert(err, gc.IsNil)
 }
 
@@ -26,7 +55,51 @@
 	c.Assert(formatter, gc.NotNil)
 	c.Assert(err, gc.IsNil)
 
+	formatter, err = newFormatter(previousFormat)
+	c.Assert(formatter, gc.NotNil)
+	c.Assert(err, gc.IsNil)
+
 	formatter, err = newFormatter("other")
 	c.Assert(formatter, gc.IsNil)
 	c.Assert(err, gc.ErrorMatches, "unknown agent config format")
 }
+
+func (*formatSuite) TestWriteFormat(c *gc.C) {
+	dir := c.MkDir()
+	testDir := path.Join(dir, "test")
+	err := writeFormatFile(testDir, "some format")
+	c.Assert(err, gc.IsNil)
+	format, err := readFormat(testDir)
+	c.Assert(format, gc.Equals, "some format")
+	c.Assert(err, gc.IsNil)
+	// Make sure the carriage return is there as it makes catting the file nicer.
+	content, err := ioutil.ReadFile(path.Join(testDir, formatFilename))
+	c.Assert(err, gc.IsNil)
+	c.Assert(string(content), gc.Equals, "some format\n")
+}
+
+func (*formatSuite) TestWriteCommandsForFormat(c *gc.C) {
+	dir := c.MkDir()
+	testDir := path.Join(dir, "test")
+	commands := writeCommandsForFormat(testDir, "some format")
+	c.Assert(commands, gc.HasLen, 3)
+	c.Assert(commands[0], gc.Matches, `mkdir -p \S+`)
+	c.Assert(commands[1], gc.Matches, `install -m 644 /dev/null '\S+/format'`)
+	c.Assert(commands[2], gc.Matches, `printf '%s\\n' '.*' > '\S+/format'`)
+}
+
+func (*formatSuite) TestReadPreviousFormatWritesNew(c *gc.C) {
+	params := agentParams
+	params.DataDir = c.MkDir()
+	config, err := newConfig(params)
+	c.Assert(err, gc.IsNil)
+
+	err = previousFormatter.write(config)
+	c.Assert(err, gc.IsNil)
+
+	_, err = ReadConf(params.DataDir, params.Tag)
+	c.Assert(err, gc.IsNil)
+	format, err := readFormat(config.Dir())
+	c.Assert(err, gc.IsNil)
+	c.Assert(format, gc.Equals, currentFormat)
+}

=== modified file 'cmd/jujud/machine.go'
--- cmd/jujud/machine.go	2013-09-12 14:43:51 +0000
+++ cmd/jujud/machine.go	2013-09-12 22:52:57 +0000
@@ -12,10 +12,10 @@
 	"launchpad.net/gnuflag"
 	"launchpad.net/tomb"
 
+	"launchpad.net/juju-core/agent"
 	"launchpad.net/juju-core/charm"
 	"launchpad.net/juju-core/cmd"
 	"launchpad.net/juju-core/instance"
-	"launchpad.net/juju-core/juju/osenv"
 	"launchpad.net/juju-core/log"
 	"launchpad.net/juju-core/names"
 	"launchpad.net/juju-core/provider"
@@ -204,15 +204,14 @@
 // StateJobs returns a worker running all the workers that require
 // a *state.State cofnnection.
 func (a *MachineAgent) StateWorker() (worker.Worker, error) {
-	st, entity, err := openState(a.Conf.config, a)
+	agentConfig := a.Conf.config
+	st, entity, err := openState(agentConfig, a)
 	if err != nil {
 		return nil, err
 	}
 	reportOpenedState(st)
 	m := entity.(*state.Machine)
-	// TODO(rog) use more discriminating test for errors
-	// rather than taking everything down indiscriminately.
-	dataDir := a.Conf.dataDir
+
 	runner := newRunner(connectionIsFatal(st), moreImportant)
 	// At this stage, since we don't embed lxc containers, just start an lxc
 	// provisioner task for non-lxc containers.  Since we have only LXC
@@ -222,18 +221,18 @@
 	// containers, it is likely that we will want an LXC provisioner on a KVM
 	// machine, and once we get nested LXC containers, we can remove this
 	// check.
-	providerType := os.Getenv(osenv.JujuProviderType)
+	providerType := agentConfig.Value(agent.ProviderType)
 	if providerType != provider.Local && m.ContainerType() != instance.LXC {
 		workerName := fmt.Sprintf("%s-provisioner", provisioner.LXC)
 		runner.StartWorker(workerName, func() (worker.Worker, error) {
-			return provisioner.NewProvisioner(provisioner.LXC, st, a.MachineId, dataDir), nil
+			return provisioner.NewProvisioner(provisioner.LXC, st, a.MachineId, agentConfig), nil
 		})
 	}
 	// Take advantage of special knowledge here in that we will only ever want
 	// the storage provider on one machine, and that is the "bootstrap" node.
 	if providerType == provider.Local && m.Id() == bootstrapMachineId {
 		runner.StartWorker("local-storage", func() (worker.Worker, error) {
-			return localstorage.NewWorker(), nil
+			return localstorage.NewWorker(agentConfig), nil
 		})
 	}
 	for _, job := range m.Jobs() {
@@ -242,7 +241,7 @@
 			// Implemented in APIWorker.
 		case state.JobManageEnviron:
 			runner.StartWorker("environ-provisioner", func() (worker.Worker, error) {
-				return provisioner.NewProvisioner(provisioner.ENVIRON, st, a.MachineId, dataDir), nil
+				return provisioner.NewProvisioner(provisioner.ENVIRON, st, a.MachineId, agentConfig), nil
 			})
 			runner.StartWorker("firewaller", func() (worker.Worker, error) {
 				return firewaller.NewFirewaller(st), nil

=== modified file 'container/lxc/lxc.go'
--- container/lxc/lxc.go	2013-08-27 23:07:55 +0000
+++ container/lxc/lxc.go	2013-09-12 22:52:57 +0000
@@ -19,7 +19,6 @@
 	"launchpad.net/juju-core/environs/cloudinit"
 	"launchpad.net/juju-core/environs/config"
 	"launchpad.net/juju-core/instance"
-	"launchpad.net/juju-core/juju/osenv"
 	"launchpad.net/juju-core/names"
 	"launchpad.net/juju-core/state"
 	"launchpad.net/juju-core/state/api"
@@ -340,9 +339,6 @@
 	if err := environs.FinishMachineConfig(machineConfig, environConfig, constraints.Value{}); err != nil {
 		return nil, err
 	}
-	// TODO(thumper): 2013-08-28 bug 1217614
-	// The machine envronment config values are being moved to the agent config.
-	machineConfig.MachineEnvironment[osenv.JujuContainerType] = string(instance.LXC)
 	cloudConfig, err := cloudinit.New(machineConfig)
 	if err != nil {
 		return nil, err

=== modified file 'environs/cloudinit.go'
--- environs/cloudinit.go	2013-08-22 07:42:43 +0000
+++ environs/cloudinit.go	2013-09-12 22:52:57 +0000
@@ -6,11 +6,11 @@
 import (
 	"fmt"
 
+	"launchpad.net/juju-core/agent"
 	coreCloudinit "launchpad.net/juju-core/cloudinit"
 	"launchpad.net/juju-core/constraints"
 	"launchpad.net/juju-core/environs/cloudinit"
 	"launchpad.net/juju-core/environs/config"
-	"launchpad.net/juju-core/juju/osenv"
 	"launchpad.net/juju-core/state"
 	"launchpad.net/juju-core/state/api"
 	"launchpad.net/juju-core/utils"
@@ -70,10 +70,11 @@
 		return fmt.Errorf("environment configuration has no authorized-keys")
 	}
 	mcfg.AuthorizedKeys = authKeys
-	if mcfg.MachineEnvironment == nil {
-		mcfg.MachineEnvironment = make(map[string]string)
+	if mcfg.AgentEnvironment == nil {
+		mcfg.AgentEnvironment = make(map[string]string)
 	}
-	mcfg.MachineEnvironment[osenv.JujuProviderType] = cfg.Type()
+	mcfg.AgentEnvironment[agent.ProviderType] = cfg.Type()
+	mcfg.AgentEnvironment[agent.ContainerType] = string(mcfg.MachineContainerType)
 	if !mcfg.StateServer {
 		return nil
 	}

=== modified file 'environs/cloudinit/cloudinit.go'
--- environs/cloudinit/cloudinit.go	2013-09-11 20:16:49 +0000
+++ environs/cloudinit/cloudinit.go	2013-09-12 22:52:57 +0000
@@ -93,9 +93,9 @@
 	// commands cannot work.
 	AuthorizedKeys string
 
-	// MachineEnvironment defines additional environment variables to set in
-	// the machine agent upstart script.
-	MachineEnvironment map[string]string
+	// AgentEnvironment defines additional configuration variables to set in
+	// the machine agent config.
+	AgentEnvironment map[string]string
 
 	// Config holds the initial environment configuration.
 	Config *config.Config
@@ -262,6 +262,7 @@
 		StateAddresses: cfg.stateHostAddrs(),
 		APIAddresses:   cfg.apiHostAddrs(),
 		CACert:         cfg.StateInfo.CACert,
+		Values:         cfg.AgentEnvironment,
 	}
 	if cfg.StateServer {
 		return agent.NewStateMachineConfig(
@@ -300,7 +301,7 @@
 	c.AddScripts(fmt.Sprintf("ln -s %v %s", cfg.Tools.Version, shquote(toolsDir)))
 
 	name := "jujud-" + tag
-	conf := upstart.MachineAgentUpstartService(name, toolsDir, cfg.DataDir, "/var/log/juju/", tag, machineId, logConfig, cfg.MachineEnvironment)
+	conf := upstart.MachineAgentUpstartService(name, toolsDir, cfg.DataDir, "/var/log/juju/", tag, machineId, logConfig, nil)
 	cmds, err := conf.InstallCommands()
 	if err != nil {
 		return fmt.Errorf("cannot make cloud-init upstart script for the %s agent: %v", tag, err)

=== modified file 'environs/cloudinit/cloudinit_test.go'
--- environs/cloudinit/cloudinit_test.go	2013-09-12 14:16:48 +0000
+++ environs/cloudinit/cloudinit_test.go	2013-09-12 22:52:57 +0000
@@ -11,12 +11,12 @@
 	gc "launchpad.net/gocheck"
 	"launchpad.net/goyaml"
 
+	"launchpad.net/juju-core/agent"
 	coreCloudinit "launchpad.net/juju-core/cloudinit"
 	"launchpad.net/juju-core/constraints"
 	"launchpad.net/juju-core/environs"
 	"launchpad.net/juju-core/environs/cloudinit"
 	"launchpad.net/juju-core/environs/config"
-	"launchpad.net/juju-core/juju/osenv"
 	"launchpad.net/juju-core/state"
 	"launchpad.net/juju-core/state/api"
 	"launchpad.net/juju-core/testing"
@@ -52,9 +52,9 @@
 	{
 		// precise state server
 		cfg: cloudinit.MachineConfig{
-			MachineId:          "0",
-			AuthorizedKeys:     "sshkey1",
-			MachineEnvironment: map[string]string{osenv.JujuProviderType: "dummy"},
+			MachineId:        "0",
+			AuthorizedKeys:   "sshkey1",
+			AgentEnvironment: map[string]string{agent.ProviderType: "dummy"},
 			// precise currently needs mongo from PPA
 			Tools:           newSimpleTools("1.2.3-precise-amd64"),
 			StateServer:     true,
@@ -90,8 +90,10 @@
 printf '%s\\n' '/var/log/juju/\*\.log {\\n        daily\\n        minsize 5M\\n        maxsize 50M\\n        copytruncate\\n        rotate 7\\n        missingok\\n        compress\\n        delaycompress\\n}' > '/etc/logrotate\.d/juju'
 restart rsyslog
 mkdir -p '/var/lib/juju/agents/machine-0'
+install -m 644 /dev/null '/var/lib/juju/agents/machine-0/format'
+printf '%s\\n' '.*' > '/var/lib/juju/agents/machine-0/format'
 install -m 600 /dev/null '/var/lib/juju/agents/machine-0/agent\.conf'
-printf '%s\\n' 'stateservercert:\\n[^']+stateserverkey:\\n[^']+stateport: 37017\\napiport: 17070\\noldpassword: arble\\nmachinenonce: FAKE_NONCE\\nstateinfo:\\n  addrs:\\n  - localhost:37017\\n  cacert:\\n[^']+  tag: machine-0\\n  password: ""\\noldapipassword: ""\\napiinfo:\\n  addrs:\\n  - localhost:17070\\n  cacert:\\n[^']+  tag: machine-0\\n  password: ""\\n' > '/var/lib/juju/agents/machine-0/agent\.conf'
+printf '%s\\n' '.*' > '/var/lib/juju/agents/machine-0/agent\.conf'
 install -m 600 /dev/null '/var/lib/juju/server\.pem'
 printf '%s\\n' 'SERVER CERT\\n[^']*SERVER KEY\\n[^']*' > '/var/lib/juju/server\.pem'
 mkdir -p /var/lib/juju/db/journal
@@ -102,21 +104,23 @@
 cat >> /etc/init/juju-db\.conf << 'EOF'\\ndescription "juju state database"\\nauthor "Juju Team <juju@lists\.ubuntu\.com>"\\nstart on runlevel \[2345\]\\nstop on runlevel \[!2345\]\\nrespawn\\nnormal exit 0\\n\\nlimit nofile 65000 65000\\nlimit nproc 20000 20000\\n\\nexec /usr/bin/mongod --auth --dbpath=/var/lib/juju/db --sslOnNormalPorts --sslPEMKeyFile '/var/lib/juju/server\.pem' --sslPEMKeyPassword ignored --bind_ip 0\.0\.0\.0 --port 37017 --noprealloc --syslog --smallfiles\\nEOF\\n
 start juju-db
 mkdir -p '/var/lib/juju/agents/bootstrap'
+install -m 644 /dev/null '/var/lib/juju/agents/bootstrap/format'
+printf '%s\\n' '.*' > '/var/lib/juju/agents/bootstrap/format'
 install -m 600 /dev/null '/var/lib/juju/agents/bootstrap/agent\.conf'
-printf '%s\\n' 'stateservercert:\\n[^']+stateserverkey:\\n[^']+stateport: 37017\\napiport: 17070\\noldpassword: arble\\nmachinenonce: FAKE_NONCE\\nstateinfo:\\n  addrs:\\n  - localhost:37017\\n  cacert:\\n[^']+  tag: bootstrap\\n  password: ""\\noldapipassword: ""\\napiinfo:\\n  addrs:\\n  - localhost:17070\\n  cacert:\\n[^']+  tag: bootstrap\\n  password: ""\\n' > '/var/lib/juju/agents/bootstrap/agent\.conf'
+printf '%s\\n' '.*' > '/var/lib/juju/agents/bootstrap/agent\.conf'
 echo 'some-url' > /tmp/provider-state-url
 /var/lib/juju/tools/1\.2\.3-precise-amd64/jujud bootstrap-state --data-dir '/var/lib/juju' --env-config '[^']*' --constraints 'mem=2048M' --debug
 rm -rf '/var/lib/juju/agents/bootstrap'
 ln -s 1\.2\.3-precise-amd64 '/var/lib/juju/tools/machine-0'
-cat >> /etc/init/jujud-machine-0\.conf << 'EOF'\\ndescription "juju machine-0 agent"\\nauthor "Juju Team <juju@lists\.ubuntu\.com>"\\nstart on runlevel \[2345\]\\nstop on runlevel \[!2345\]\\nrespawn\\nnormal exit 0\\nenv JUJU_PROVIDER_TYPE="dummy"\\n\\nlimit nofile 20000 20000\\n\\nexec /var/lib/juju/tools/machine-0/jujud machine --log-file '/var/log/juju/machine-0\.log' --data-dir '/var/lib/juju' --machine-id 0  --debug >> /var/log/juju/machine-0\.log 2>&1\\nEOF\\n
+cat >> /etc/init/jujud-machine-0\.conf << 'EOF'\\ndescription "juju machine-0 agent"\\nauthor "Juju Team <juju@lists\.ubuntu\.com>"\\nstart on runlevel \[2345\]\\nstop on runlevel \[!2345\]\\nrespawn\\nnormal exit 0\\n\\nlimit nofile 20000 20000\\n\\nexec /var/lib/juju/tools/machine-0/jujud machine --log-file '/var/log/juju/machine-0\.log' --data-dir '/var/lib/juju' --machine-id 0  --debug >> /var/log/juju/machine-0\.log 2>&1\\nEOF\\n
 start jujud-machine-0
 `,
 	}, {
 		// raring state server
 		cfg: cloudinit.MachineConfig{
-			MachineId:          "0",
-			AuthorizedKeys:     "sshkey1",
-			MachineEnvironment: map[string]string{osenv.JujuProviderType: "dummy"},
+			MachineId:        "0",
+			AuthorizedKeys:   "sshkey1",
+			AgentEnvironment: map[string]string{agent.ProviderType: "dummy"},
 			// raring provides mongo in the archive
 			Tools:           newSimpleTools("1.2.3-raring-amd64"),
 			StateServer:     true,
@@ -152,8 +156,10 @@
 printf '%s\\n' '/var/log/juju/\*\.log {\\n        daily\\n        minsize 5M\\n        maxsize 50M\\n        copytruncate\\n        rotate 7\\n        missingok\\n        compress\\n        delaycompress\\n}' > '/etc/logrotate\.d/juju'
 restart rsyslog
 mkdir -p '/var/lib/juju/agents/machine-0'
+install -m 644 /dev/null '/var/lib/juju/agents/machine-0/format'
+printf '%s\\n' '.*' > '/var/lib/juju/agents/machine-0/format'
 install -m 600 /dev/null '/var/lib/juju/agents/machine-0/agent\.conf'
-printf '%s\\n' 'stateservercert:\\n[^']+stateserverkey:\\n[^']+stateport: 37017\\napiport: 17070\\noldpassword: arble\\nmachinenonce: FAKE_NONCE\\nstateinfo:\\n  addrs:\\n  - localhost:37017\\n  cacert:\\n[^']+  tag: machine-0\\n  password: ""\\noldapipassword: ""\\napiinfo:\\n  addrs:\\n  - localhost:17070\\n  cacert:\\n[^']+  tag: machine-0\\n  password: ""\\n' > '/var/lib/juju/agents/machine-0/agent\.conf'
+printf '%s\\n' '.*' > '/var/lib/juju/agents/machine-0/agent\.conf'
 install -m 600 /dev/null '/var/lib/juju/server\.pem'
 printf '%s\\n' 'SERVER CERT\\n[^']*SERVER KEY\\n[^']*' > '/var/lib/juju/server\.pem'
 mkdir -p /var/lib/juju/db/journal
@@ -164,24 +170,26 @@
 cat >> /etc/init/juju-db\.conf << 'EOF'\\ndescription "juju state database"\\nauthor "Juju Team <juju@lists\.ubuntu\.com>"\\nstart on runlevel \[2345\]\\nstop on runlevel \[!2345\]\\nrespawn\\nnormal exit 0\\n\\nlimit nofile 65000 65000\\nlimit nproc 20000 20000\\n\\nexec /usr/bin/mongod --auth --dbpath=/var/lib/juju/db --sslOnNormalPorts --sslPEMKeyFile '/var/lib/juju/server\.pem' --sslPEMKeyPassword ignored --bind_ip 0\.0\.0\.0 --port 37017 --noprealloc --syslog --smallfiles\\nEOF\\n
 start juju-db
 mkdir -p '/var/lib/juju/agents/bootstrap'
+install -m 644 /dev/null '/var/lib/juju/agents/bootstrap/format'
+printf '%s\\n' '.*' > '/var/lib/juju/agents/bootstrap/format'
 install -m 600 /dev/null '/var/lib/juju/agents/bootstrap/agent\.conf'
-printf '%s\\n' 'stateservercert:\\n[^']+stateserverkey:\\n[^']+stateport: 37017\\napiport: 17070\\noldpassword: arble\\nmachinenonce: FAKE_NONCE\\nstateinfo:\\n  addrs:\\n  - localhost:37017\\n  cacert:\\n[^']+  tag: bootstrap\\n  password: ""\\noldapipassword: ""\\napiinfo:\\n  addrs:\\n  - localhost:17070\\n  cacert:\\n[^']+  tag: bootstrap\\n  password: ""\\n' > '/var/lib/juju/agents/bootstrap/agent\.conf'
+printf '%s\\n' '.*' > '/var/lib/juju/agents/bootstrap/agent\.conf'
 echo 'some-url' > /tmp/provider-state-url
 /var/lib/juju/tools/1\.2\.3-raring-amd64/jujud bootstrap-state --data-dir '/var/lib/juju' --env-config '[^']*' --constraints 'mem=2048M' --debug
 rm -rf '/var/lib/juju/agents/bootstrap'
 ln -s 1\.2\.3-raring-amd64 '/var/lib/juju/tools/machine-0'
-cat >> /etc/init/jujud-machine-0\.conf << 'EOF'\\ndescription "juju machine-0 agent"\\nauthor "Juju Team <juju@lists\.ubuntu\.com>"\\nstart on runlevel \[2345\]\\nstop on runlevel \[!2345\]\\nrespawn\\nnormal exit 0\\nenv JUJU_PROVIDER_TYPE="dummy"\\n\\nlimit nofile 20000 20000\\n\\nexec /var/lib/juju/tools/machine-0/jujud machine --log-file '/var/log/juju/machine-0\.log' --data-dir '/var/lib/juju' --machine-id 0  --debug >> /var/log/juju/machine-0\.log 2>&1\\nEOF\\n
+cat >> /etc/init/jujud-machine-0\.conf << 'EOF'\\ndescription "juju machine-0 agent"\\nauthor "Juju Team <juju@lists\.ubuntu\.com>"\\nstart on runlevel \[2345\]\\nstop on runlevel \[!2345\]\\nrespawn\\nnormal exit 0\\n\\nlimit nofile 20000 20000\\n\\nexec /var/lib/juju/tools/machine-0/jujud machine --log-file '/var/log/juju/machine-0\.log' --data-dir '/var/lib/juju' --machine-id 0  --debug >> /var/log/juju/machine-0\.log 2>&1\\nEOF\\n
 start jujud-machine-0
 `,
 	}, {
 		cfg: cloudinit.MachineConfig{
-			MachineId:          "99",
-			AuthorizedKeys:     "sshkey1",
-			MachineEnvironment: map[string]string{osenv.JujuProviderType: "dummy"},
-			DataDir:            environs.DataDir,
-			StateServer:        false,
-			Tools:              newSimpleTools("1.2.3-linux-amd64"),
-			MachineNonce:       "FAKE_NONCE",
+			MachineId:        "99",
+			AuthorizedKeys:   "sshkey1",
+			AgentEnvironment: map[string]string{agent.ProviderType: "dummy"},
+			DataDir:          environs.DataDir,
+			StateServer:      false,
+			Tools:            newSimpleTools("1.2.3-linux-amd64"),
+			MachineNonce:     "FAKE_NONCE",
 			StateInfo: &state.Info{
 				Addrs:    []string{"state-addr.testing.invalid:12345"},
 				Tag:      "machine-99",
@@ -209,10 +217,12 @@
 printf '%s\\n' '/var/log/juju/\*\.log {\\n        daily\\n        minsize 5M\\n        maxsize 50M\\n        copytruncate\\n        rotate 7\\n        missingok\\n        compress\\n        delaycompress\\n}' > '/etc/logrotate\.d/juju'
 restart rsyslog
 mkdir -p '/var/lib/juju/agents/machine-99'
+install -m 644 /dev/null '/var/lib/juju/agents/machine-99/format'
+printf '%s\\n' '.*' > '/var/lib/juju/agents/machine-99/format'
 install -m 600 /dev/null '/var/lib/juju/agents/machine-99/agent\.conf'
-printf '%s\\n' 'oldpassword: arble\\nmachinenonce: FAKE_NONCE\\nstateinfo:\\n  addrs:\\n  - state-addr\.testing\.invalid:12345\\n  cacert:\\n[^']+  tag: machine-99\\n  password: ""\\noldapipassword: ""\\napiinfo:\\n  addrs:\\n  - state-addr\.testing\.invalid:54321\\n  cacert:\\n[^']+  tag: machine-99\\n  password: ""\\n' > '/var/lib/juju/agents/machine-99/agent\.conf'
+printf '%s\\n' '.*' > '/var/lib/juju/agents/machine-99/agent\.conf'
 ln -s 1\.2\.3-linux-amd64 '/var/lib/juju/tools/machine-99'
-cat >> /etc/init/jujud-machine-99\.conf << 'EOF'\\ndescription "juju machine-99 agent"\\nauthor "Juju Team <juju@lists\.ubuntu\.com>"\\nstart on runlevel \[2345\]\\nstop on runlevel \[!2345\]\\nrespawn\\nnormal exit 0\\nenv JUJU_PROVIDER_TYPE="dummy"\\n\\nlimit nofile 20000 20000\\n\\nexec /var/lib/juju/tools/machine-99/jujud machine --log-file '/var/log/juju/machine-99\.log' --data-dir '/var/lib/juju' --machine-id 99  --debug >> /var/log/juju/machine-99\.log 2>&1\\nEOF\\n
+cat >> /etc/init/jujud-machine-99\.conf << 'EOF'\\ndescription "juju machine-99 agent"\\nauthor "Juju Team <juju@lists\.ubuntu\.com>"\\nstart on runlevel \[2345\]\\nstop on runlevel \[!2345\]\\nrespawn\\nnormal exit 0\\n\\nlimit nofile 20000 20000\\n\\nexec /var/lib/juju/tools/machine-99/jujud machine --log-file '/var/log/juju/machine-99\.log' --data-dir '/var/lib/juju' --machine-id 99  --debug >> /var/log/juju/machine-99\.log 2>&1\\nEOF\\n
 start jujud-machine-99
 `,
 	}, {
@@ -220,7 +230,7 @@
 			MachineId:            "2/lxc/1",
 			MachineContainerType: "lxc",
 			AuthorizedKeys:       "sshkey1",
-			MachineEnvironment:   map[string]string{osenv.JujuProviderType: "dummy"},
+			AgentEnvironment:     map[string]string{agent.ProviderType: "dummy"},
 			DataDir:              environs.DataDir,
 			StateServer:          false,
 			Tools:                newSimpleTools("1.2.3-linux-amd64"),
@@ -252,10 +262,12 @@
 printf '%s\\n' '/var/log/juju/\*\.log {\\n        daily\\n        minsize 5M\\n        maxsize 50M\\n        copytruncate\\n        rotate 7\\n        missingok\\n        compress\\n        delaycompress\\n}' > '/etc/logrotate\.d/juju'
 restart rsyslog
 mkdir -p '/var/lib/juju/agents/machine-2-lxc-1'
+install -m 644 /dev/null '/var/lib/juju/agents/machine-2-lxc-1/format'
+printf '%s\\n' '.*' > '/var/lib/juju/agents/machine-2-lxc-1/format'
 install -m 600 /dev/null '/var/lib/juju/agents/machine-2-lxc-1/agent\.conf'
-printf '%s\\n' 'oldpassword: arble\\nmachinenonce: FAKE_NONCE\\nstateinfo:\\n  addrs:\\n  - state-addr\.testing\.invalid:12345\\n  cacert:\\n[^']+  tag: machine-2-lxc-1\\n  password: ""\\noldapipassword: ""\\napiinfo:\\n  addrs:\\n  - state-addr\.testing\.invalid:54321\\n  cacert:\\n[^']+  tag: machine-2-lxc-1\\n  password: ""\\n' > '/var/lib/juju/agents/machine-2-lxc-1/agent\.conf'
+printf '%s\\n' '.*' > '/var/lib/juju/agents/machine-2-lxc-1/agent\.conf'
 ln -s 1\.2\.3-linux-amd64 '/var/lib/juju/tools/machine-2-lxc-1'
-cat >> /etc/init/jujud-machine-2-lxc-1\.conf << 'EOF'\\ndescription "juju machine-2-lxc-1 agent"\\nauthor "Juju Team <juju@lists\.ubuntu\.com>"\\nstart on runlevel \[2345\]\\nstop on runlevel \[!2345\]\\nrespawn\\nnormal exit 0\\nenv JUJU_PROVIDER_TYPE="dummy"\\n\\nlimit nofile 20000 20000\\n\\nexec /var/lib/juju/tools/machine-2-lxc-1/jujud machine --log-file '/var/log/juju/machine-2-lxc-1\.log' --data-dir '/var/lib/juju' --machine-id 2/lxc/1  --debug >> /var/log/juju/machine-2-lxc-1\.log 2>&1\\nEOF\\n
+cat >> /etc/init/jujud-machine-2-lxc-1\.conf << 'EOF'\\ndescription "juju machine-2-lxc-1 agent"\\nauthor "Juju Team <juju@lists\.ubuntu\.com>"\\nstart on runlevel \[2345\]\\nstop on runlevel \[!2345\]\\nrespawn\\nnormal exit 0\\n\\nlimit nofile 20000 20000\\n\\nexec /var/lib/juju/tools/machine-2-lxc-1/jujud machine --log-file '/var/log/juju/machine-2-lxc-1\.log' --data-dir '/var/lib/juju' --machine-id 2/lxc/1  --debug >> /var/log/juju/machine-2-lxc-1\.log 2>&1\\nEOF\\n
 start jujud-machine-2-lxc-1
 `,
 	},
@@ -565,15 +577,15 @@
 // checked for by NewCloudInit.
 func (*cloudinitSuite) TestCloudInitVerify(c *gc.C) {
 	cfg := &cloudinit.MachineConfig{
-		StateServer:        true,
-		StateServerCert:    serverCert,
-		StateServerKey:     serverKey,
-		StatePort:          1234,
-		APIPort:            1235,
-		MachineId:          "99",
-		Tools:              newSimpleTools("9.9.9-linux-arble"),
-		AuthorizedKeys:     "sshkey1",
-		MachineEnvironment: map[string]string{osenv.JujuProviderType: "dummy"},
+		StateServer:      true,
+		StateServerCert:  serverCert,
+		StateServerKey:   serverKey,
+		StatePort:        1234,
+		APIPort:          1235,
+		MachineId:        "99",
+		Tools:            newSimpleTools("9.9.9-linux-arble"),
+		AuthorizedKeys:   "sshkey1",
+		AgentEnvironment: map[string]string{agent.ProviderType: "dummy"},
 		StateInfo: &state.Info{
 			Addrs:    []string{"host:98765"},
 			CACert:   []byte(testing.CACert),

=== modified file 'environs/cloudinit_test.go'
--- environs/cloudinit_test.go	2013-09-12 12:38:04 +0000
+++ environs/cloudinit_test.go	2013-09-12 22:52:57 +0000
@@ -9,12 +9,12 @@
 	gc "launchpad.net/gocheck"
 	"launchpad.net/goyaml"
 
+	"launchpad.net/juju-core/agent"
 	"launchpad.net/juju-core/cert"
 	"launchpad.net/juju-core/constraints"
 	"launchpad.net/juju-core/environs"
 	"launchpad.net/juju-core/environs/cloudinit"
 	"launchpad.net/juju-core/environs/config"
-	"launchpad.net/juju-core/juju/osenv"
 	"launchpad.net/juju-core/provider/dummy"
 	"launchpad.net/juju-core/state"
 	"launchpad.net/juju-core/state/api"
@@ -52,10 +52,13 @@
 	err = environs.FinishMachineConfig(mcfg, cfg, constraints.Value{})
 	c.Assert(err, gc.IsNil)
 	c.Assert(mcfg, gc.DeepEquals, &cloudinit.MachineConfig{
-		AuthorizedKeys:     "we-are-the-keys",
-		MachineEnvironment: map[string]string{osenv.JujuProviderType: "dummy"},
-		StateInfo:          &state.Info{Tag: "not touched"},
-		APIInfo:            &api.Info{Tag: "not touched"},
+		AuthorizedKeys: "we-are-the-keys",
+		AgentEnvironment: map[string]string{
+			agent.ProviderType:  "dummy",
+			agent.ContainerType: "",
+		},
+		StateInfo: &state.Info{Tag: "not touched"},
+		APIInfo:   &api.Info{Tag: "not touched"},
 	})
 }
 
@@ -128,12 +131,12 @@
 			Password: "pw2",
 			CACert:   []byte("CA CERT\n" + testing.CACert),
 		},
-		DataDir:            environs.DataDir,
-		Config:             envConfig,
-		StatePort:          envConfig.StatePort(),
-		APIPort:            envConfig.APIPort(),
-		StateServer:        true,
-		MachineEnvironment: map[string]string{osenv.JujuProviderType: "dummy"},
+		DataDir:          environs.DataDir,
+		Config:           envConfig,
+		StatePort:        envConfig.StatePort(),
+		APIPort:          envConfig.APIPort(),
+		StateServer:      true,
+		AgentEnvironment: map[string]string{agent.ProviderType: "dummy"},
 	}
 	script1 := "script1"
 	script2 := "script2"

=== modified file 'environs/manual/agent.go'
--- environs/manual/agent.go	2013-09-03 09:03:02 +0000
+++ environs/manual/agent.go	2013-09-12 22:52:57 +0000
@@ -10,12 +10,12 @@
 	"os/exec"
 	"strings"
 
+	"launchpad.net/juju-core/agent"
 	corecloudinit "launchpad.net/juju-core/cloudinit"
 	"launchpad.net/juju-core/constraints"
 	"launchpad.net/juju-core/environs"
 	"launchpad.net/juju-core/environs/cloudinit"
 	envtools "launchpad.net/juju-core/environs/tools"
-	"launchpad.net/juju-core/juju/osenv"
 	"launchpad.net/juju-core/provider"
 	"launchpad.net/juju-core/state"
 	"launchpad.net/juju-core/state/api"
@@ -81,7 +81,7 @@
 	if err != nil {
 		return "", err
 	}
-	mcfg.MachineEnvironment[osenv.JujuProviderType] = provider.Null
+	mcfg.AgentEnvironment[agent.ProviderType] = provider.Null
 	cloudcfg := corecloudinit.New()
 	if cloudcfg, err = cloudinit.Configure(mcfg, cloudcfg); err != nil {
 		return "", err

=== modified file 'juju/osenv/vars.go'
--- juju/osenv/vars.go	2013-09-03 15:38:37 +0000
+++ juju/osenv/vars.go	2013-09-12 22:52:57 +0000
@@ -10,16 +10,14 @@
 )
 
 const (
-	JujuEnv               = "JUJU_ENV"
-	JujuHome              = "JUJU_HOME"
-	JujuRepository        = "JUJU_REPOSITORY"
-	JujuLxcBridge         = "JUJU_LXC_BRIDGE"
-	JujuProviderType      = "JUJU_PROVIDER_TYPE"
-	JujuContainerType     = "JUJU_CONTAINER_TYPE"
-	JujuStorageDir        = "JUJU_STORAGE_DIR"
-	JujuStorageAddr       = "JUJU_STORAGE_ADDR"
-	JujuSharedStorageDir  = "JUJU_SHARED_STORAGE_DIR"
-	JujuSharedStorageAddr = "JUJU_SHARED_STORAGE_ADDR"
+	JujuEnv        = "JUJU_ENV"
+	JujuHome       = "JUJU_HOME"
+	JujuRepository = "JUJU_REPOSITORY"
+	// TODO(thumper): 2013-09-02 bug 1219630
+	// As much as I'd like to remove JujuContainerType now, it is still
+	// needed as MAAS still needs it at this stage, and we can't fix
+	// everything at once.
+	JujuContainerType = "JUJU_CONTAINER_TYPE"
 )
 
 // JujuHome returns the directory where juju should store application-specific files

=== modified file 'juju/testing/conn.go'
--- juju/testing/conn.go	2013-09-12 14:16:48 +0000
+++ juju/testing/conn.go	2013-09-12 22:52:57 +0000
@@ -12,6 +12,7 @@
 	gc "launchpad.net/gocheck"
 	"launchpad.net/goyaml"
 
+	"launchpad.net/juju-core/agent"
 	"launchpad.net/juju-core/charm"
 	"launchpad.net/juju-core/constraints"
 	"launchpad.net/juju-core/environs"
@@ -309,3 +310,18 @@
 	c.Assert(err, gc.IsNil)
 	return sch
 }
+
+func (s *JujuConnSuite) AgentConfigForTag(c *gc.C, tag string) agent.Config {
+	config, err := agent.NewAgentConfig(
+		agent.AgentConfigParams{
+			DataDir:        s.DataDir(),
+			Tag:            tag,
+			Password:       "dummy-secret",
+			Nonce:          "nonce",
+			StateAddresses: s.StateInfo(c).Addrs,
+			APIAddresses:   s.APIInfo(c).Addrs,
+			CACert:         []byte(testing.CACert),
+		})
+	c.Assert(err, gc.IsNil)
+	return config
+}

=== modified file 'provider/local/environ.go'
--- provider/local/environ.go	2013-09-11 12:18:04 +0000
+++ provider/local/environ.go	2013-09-12 22:52:57 +0000
@@ -464,13 +464,8 @@
 	logDir := env.config.logDir()
 	logConfig := "--debug" // TODO(thumper): specify loggo config
 	machineEnvironment := map[string]string{
-		"USER":                      env.config.user,
-		"HOME":                      osenv.Home(),
-		osenv.JujuProviderType:      env.config.Type(),
-		osenv.JujuStorageDir:        env.config.storageDir(),
-		osenv.JujuStorageAddr:       env.config.storageAddr(),
-		osenv.JujuSharedStorageDir:  env.config.sharedStorageDir(),
-		osenv.JujuSharedStorageAddr: env.config.sharedStorageAddr(),
+		"USER": env.config.user,
+		"HOME": osenv.Home(),
 	}
 	agentService := upstart.MachineAgentUpstartService(
 		env.machineAgentServiceName(),
@@ -496,6 +491,13 @@
 	// wouldn't get this far.
 	cfg := env.config.Config
 	caCert, _ := cfg.CACert()
+	agentValues := map[string]string{
+		agent.ProviderType:      env.config.Type(),
+		agent.StorageDir:        env.config.storageDir(),
+		agent.StorageAddr:       env.config.storageAddr(),
+		agent.SharedStorageDir:  env.config.sharedStorageDir(),
+		agent.SharedStorageAddr: env.config.sharedStorageAddr(),
+	}
 	// NOTE: the state address HAS to be localhost, otherwise the mongo
 	// initialization fails.  There is some magic code somewhere in the mongo
 	// connection code that treats connections from localhost as special, and
@@ -513,6 +515,7 @@
 				StateAddresses: []string{stateAddress},
 				APIAddresses:   []string{apiAddress},
 				CACert:         caCert,
+				Values:         agentValues,
 			},
 			StateServerCert: cert,
 			StateServerKey:  key,

=== modified file 'provider/local/storage/worker.go'
--- provider/local/storage/worker.go	2013-07-31 05:11:24 +0000
+++ provider/local/storage/worker.go	2013-09-12 22:52:57 +0000
@@ -1,24 +1,23 @@
 package storage
 
 import (
-	"os"
-
 	"launchpad.net/loggo"
 	"launchpad.net/tomb"
 
+	"launchpad.net/juju-core/agent"
 	"launchpad.net/juju-core/environs/localstorage"
-	"launchpad.net/juju-core/juju/osenv"
 	"launchpad.net/juju-core/worker"
 )
 
 var logger = loggo.GetLogger("juju.local.storage")
 
 type storageWorker struct {
-	tomb tomb.Tomb
+	config agent.Config
+	tomb   tomb.Tomb
 }
 
-func NewWorker() worker.Worker {
-	w := &storageWorker{}
+func NewWorker(config agent.Config) worker.Worker {
+	w := &storageWorker{config: config}
 	go func() {
 		defer w.tomb.Done()
 		w.tomb.Kill(w.waitForDeath())
@@ -37,8 +36,8 @@
 }
 
 func (s *storageWorker) waitForDeath() error {
-	storageDir := os.Getenv(osenv.JujuStorageDir)
-	storageAddr := os.Getenv(osenv.JujuStorageAddr)
+	storageDir := s.config.Value(agent.StorageDir)
+	storageAddr := s.config.Value(agent.StorageAddr)
 	logger.Infof("serving %s on %s", storageDir, storageAddr)
 
 	storageListener, err := localstorage.Serve(storageAddr, storageDir)
@@ -48,8 +47,8 @@
 	}
 	defer storageListener.Close()
 
-	sharedStorageDir := os.Getenv(osenv.JujuSharedStorageDir)
-	sharedStorageAddr := os.Getenv(osenv.JujuSharedStorageAddr)
+	sharedStorageDir := s.config.Value(agent.SharedStorageDir)
+	sharedStorageAddr := s.config.Value(agent.SharedStorageAddr)
 	logger.Infof("serving %s on %s", sharedStorageDir, sharedStorageAddr)
 
 	sharedStorageListener, err := localstorage.Serve(sharedStorageAddr, sharedStorageDir)

=== modified file 'provider/maas/environ.go'
--- provider/maas/environ.go	2013-09-10 03:43:51 +0000
+++ provider/maas/environ.go	2013-09-12 22:52:57 +0000
@@ -12,6 +12,7 @@
 
 	"launchpad.net/gomaasapi"
 
+	"launchpad.net/juju-core/agent"
 	"launchpad.net/juju-core/constraints"
 	"launchpad.net/juju-core/environs"
 	"launchpad.net/juju-core/environs/cloudinit"
@@ -20,7 +21,6 @@
 	"launchpad.net/juju-core/environs/simplestreams"
 	envtools "launchpad.net/juju-core/environs/tools"
 	"launchpad.net/juju-core/instance"
-	"launchpad.net/juju-core/juju/osenv"
 	"launchpad.net/juju-core/provider"
 	"launchpad.net/juju-core/state"
 	"launchpad.net/juju-core/state/api"
@@ -257,7 +257,7 @@
 	// TODO(thumper): 2013-08-28 bug 1217614
 	// The machine envronment config values are being moved to the agent config.
 	// Explicitly specify that the lxc containers use the network bridge defined above.
-	machineConfig.MachineEnvironment[osenv.JujuLxcBridge] = "br0"
+	machineConfig.AgentEnvironment[agent.LxcBridge] = "br0"
 	userdata, err := environs.ComposeUserData(
 		machineConfig,
 		runCmd,

=== modified file 'worker/deployer/simple.go'
--- worker/deployer/simple.go	2013-09-02 05:41:29 +0000
+++ worker/deployer/simple.go	2013-09-12 22:52:57 +0000
@@ -94,6 +94,7 @@
 		return err
 	}
 	logger.Debugf("API addresses: %q", apiAddrs)
+	containerType := ctx.agentConfig.Value(agent.ContainerType)
 	conf, err := agent.NewAgentConfig(
 		agent.AgentConfigParams{
 			DataDir:        dataDir,
@@ -103,6 +104,9 @@
 			StateAddresses: stateAddrs,
 			APIAddresses:   apiAddrs,
 			CACert:         ctx.agentConfig.CACert(),
+			Values: map[string]string{
+				agent.ContainerType: containerType,
+			},
 		})
 	if err != nil {
 		return err
@@ -132,15 +136,17 @@
 		"--unit-name", unitName,
 		"--debug", // TODO: propagate debug state sensibly
 	}, " ")
+	// TODO(thumper): 2013-09-02 bug 1219630
+	// As much as I'd like to remove JujuContainerType now, it is still
+	// needed as MAAS still needs it at this stage, and we can't fix
+	// everything at once.
 	uconf := &upstart.Conf{
 		Service: *svc,
 		Desc:    "juju unit agent for " + unitName,
 		Cmd:     cmd,
 		Out:     logPath,
-		// Propagate the provider type and container type enviroment variables.
 		Env: map[string]string{
-			osenv.JujuProviderType:  os.Getenv(osenv.JujuProviderType),
-			osenv.JujuContainerType: os.Getenv(osenv.JujuContainerType),
+			osenv.JujuContainerType: containerType,
 		},
 	}
 	return uconf.Install()

=== modified file 'worker/deployer/simple_test.go'
--- worker/deployer/simple_test.go	2013-09-02 06:03:14 +0000
+++ worker/deployer/simple_test.go	2013-09-12 22:52:57 +0000
@@ -304,6 +304,10 @@
 	return []byte(testing.CACert)
 }
 
+func (mock *mockConfig) Value(_ string) string {
+	return ""
+}
+
 func agentConfig(tag, datadir string) agent.Config {
 	return &mockConfig{tag: tag, datadir: datadir}
 }

=== modified file 'worker/provisioner/lxc-broker.go'
--- worker/provisioner/lxc-broker.go	2013-08-22 22:24:54 +0000
+++ worker/provisioner/lxc-broker.go	2013-09-12 22:52:57 +0000
@@ -4,17 +4,15 @@
 package provisioner
 
 import (
-	"os"
-
 	"launchpad.net/loggo"
 
+	"launchpad.net/juju-core/agent"
 	"launchpad.net/juju-core/constraints"
 	"launchpad.net/juju-core/container/lxc"
 	"launchpad.net/juju-core/environs"
 	"launchpad.net/juju-core/environs/cloudinit"
 	"launchpad.net/juju-core/environs/config"
 	"launchpad.net/juju-core/instance"
-	"launchpad.net/juju-core/juju/osenv"
 	"launchpad.net/juju-core/tools"
 )
 
@@ -23,18 +21,20 @@
 var _ environs.InstanceBroker = (*lxcBroker)(nil)
 var _ tools.HasTools = (*lxcBroker)(nil)
 
-func NewLxcBroker(config *config.Config, tools *tools.Tools) environs.InstanceBroker {
+func NewLxcBroker(config *config.Config, tools *tools.Tools, agentConfig agent.Config) environs.InstanceBroker {
 	return &lxcBroker{
-		manager: lxc.NewContainerManager(lxc.ManagerConfig{Name: "juju"}),
-		config:  config,
-		tools:   tools,
+		manager:     lxc.NewContainerManager(lxc.ManagerConfig{Name: "juju"}),
+		config:      config,
+		tools:       tools,
+		agentConfig: agentConfig,
 	}
 }
 
 type lxcBroker struct {
-	manager lxc.ContainerManager
-	config  *config.Config
-	tools   *tools.Tools
+	manager     lxc.ContainerManager
+	config      *config.Config
+	tools       *tools.Tools
+	agentConfig agent.Config
 }
 
 func (broker *lxcBroker) Tools() tools.List {
@@ -49,7 +49,7 @@
 	lxcLogger.Infof("starting lxc container for machineId: %s", machineId)
 
 	// Default to using the host network until we can configure.
-	bridgeDevice := os.Getenv(osenv.JujuLxcBridge)
+	bridgeDevice := broker.agentConfig.Value(agent.LxcBridge)
 	if bridgeDevice == "" {
 		bridgeDevice = lxc.DefaultLxcBridge
 	}

=== modified file 'worker/provisioner/lxc-broker_test.go'
--- worker/provisioner/lxc-broker_test.go	2013-08-22 22:24:54 +0000
+++ worker/provisioner/lxc-broker_test.go	2013-09-12 22:52:57 +0000
@@ -12,6 +12,7 @@
 
 	gc "launchpad.net/gocheck"
 
+	"launchpad.net/juju-core/agent"
 	agenttools "launchpad.net/juju-core/agent/tools"
 	"launchpad.net/juju-core/constraints"
 	"launchpad.net/juju-core/container/lxc"
@@ -19,8 +20,8 @@
 	"launchpad.net/juju-core/environs"
 	"launchpad.net/juju-core/environs/config"
 	"launchpad.net/juju-core/instance"
-	"launchpad.net/juju-core/juju/osenv"
 	jujutesting "launchpad.net/juju-core/juju/testing"
+	"launchpad.net/juju-core/names"
 	"launchpad.net/juju-core/provider"
 	"launchpad.net/juju-core/state"
 	coretesting "launchpad.net/juju-core/testing"
@@ -38,7 +39,8 @@
 
 type lxcBrokerSuite struct {
 	lxcSuite
-	broker environs.InstanceBroker
+	broker      environs.InstanceBroker
+	agentConfig agent.Config
 }
 
 var _ = gc.Suite(&lxcBrokerSuite{})
@@ -77,7 +79,19 @@
 		Version: version.MustParseBinary("2.3.4-foo-bar"),
 		URL:     "http://tools.testing.invalid/2.3.4-foo-bar.tgz",
 	}
-	s.broker = provisioner.NewLxcBroker(coretesting.EnvironConfig(c), tools)
+	config := coretesting.EnvironConfig(c)
+	var err error
+	s.agentConfig, err = agent.NewAgentConfig(
+		agent.AgentConfigParams{
+			DataDir:      "/not/used/here",
+			Tag:          "tag",
+			Password:     "dummy-secret",
+			Nonce:        "nonce",
+			APIAddresses: []string{"10.0.0.1:1234"},
+			CACert:       []byte(coretesting.CACert),
+		})
+	c.Assert(err, gc.IsNil)
+	s.broker = provisioner.NewLxcBroker(config, tools, s.agentConfig)
 }
 
 func (s *lxcBrokerSuite) startInstance(c *gc.C, machineId string) instance.Instance {
@@ -106,7 +120,7 @@
 }
 
 func (s *lxcBrokerSuite) TestStartInstanceWithBridgeEnviron(c *gc.C) {
-	defer coretesting.PatchEnvironment(osenv.JujuLxcBridge, "br0")()
+	s.agentConfig.SetValue(agent.LxcBridge, "br0")
 	machineId := "1/lxc/0"
 	lxc := s.startInstance(c, machineId)
 	c.Assert(lxc.Id(), gc.Equals, instance.Id("juju-machine-1-lxc-0"))
@@ -231,17 +245,19 @@
 	s.CommonProvisionerSuite.TearDownTest(c)
 }
 
-func (s *lxcProvisionerSuite) newLxcProvisioner() *provisioner.Provisioner {
-	return provisioner.NewProvisioner(provisioner.LXC, s.State, s.machineId, s.DataDir())
+func (s *lxcProvisionerSuite) newLxcProvisioner(c *gc.C) *provisioner.Provisioner {
+	machineTag := names.MachineTag(s.machineId)
+	agentConfig := s.AgentConfigForTag(c, machineTag)
+	return provisioner.NewProvisioner(provisioner.LXC, s.State, s.machineId, agentConfig)
 }
 
 func (s *lxcProvisionerSuite) TestProvisionerStartStop(c *gc.C) {
-	p := s.newLxcProvisioner()
+	p := s.newLxcProvisioner(c)
 	c.Assert(p.Stop(), gc.IsNil)
 }
 
 func (s *lxcProvisionerSuite) TestDoesNotStartEnvironMachines(c *gc.C) {
-	p := s.newLxcProvisioner()
+	p := s.newLxcProvisioner(c)
 	defer stop(c, p)
 
 	// Check that an instance is not provisioned when the machine is created.
@@ -264,7 +280,7 @@
 }
 
 func (s *lxcProvisionerSuite) TestContainerStartedAndStopped(c *gc.C) {
-	p := s.newLxcProvisioner()
+	p := s.newLxcProvisioner(c)
 	defer stop(c, p)
 
 	container := s.addContainer(c)

=== modified file 'worker/provisioner/provisioner.go'
--- worker/provisioner/provisioner.go	2013-08-22 22:24:54 +0000
+++ worker/provisioner/provisioner.go	2013-09-12 22:52:57 +0000
@@ -10,6 +10,7 @@
 	"launchpad.net/loggo"
 	"launchpad.net/tomb"
 
+	"launchpad.net/juju-core/agent"
 	agenttools "launchpad.net/juju-core/agent/tools"
 	"launchpad.net/juju-core/environs"
 	"launchpad.net/juju-core/environs/config"
@@ -34,13 +35,13 @@
 
 // Provisioner represents a running provisioning worker.
 type Provisioner struct {
-	pt        ProvisionerType
-	st        *state.State
-	machineId string // Which machine runs the provisioner.
-	dataDir   string
-	machine   *state.Machine
-	environ   environs.Environ
-	tomb      tomb.Tomb
+	pt          ProvisionerType
+	st          *state.State
+	machineId   string // Which machine runs the provisioner.
+	machine     *state.Machine
+	environ     environs.Environ
+	agentConfig agent.Config
+	tomb        tomb.Tomb
 
 	configObserver
 }
@@ -62,12 +63,12 @@
 // NewProvisioner returns a new Provisioner. When new machines
 // are added to the state, it allocates instances from the environment
 // and allocates them to the new machines.
-func NewProvisioner(pt ProvisionerType, st *state.State, machineId, dataDir string) *Provisioner {
+func NewProvisioner(pt ProvisionerType, st *state.State, machineId string, agentConfig agent.Config) *Provisioner {
 	p := &Provisioner{
-		pt:        pt,
-		st:        st,
-		machineId: machineId,
-		dataDir:   dataDir,
+		pt:          pt,
+		st:          st,
+		machineId:   machineId,
+		agentConfig: agentConfig,
 	}
 	go func() {
 		defer p.tomb.Done()
@@ -166,15 +167,16 @@
 			logger.Errorf("cannot get tools from machine for lxc broker")
 			return nil, err
 		}
-		return NewLxcBroker(config, tools), nil
+		return NewLxcBroker(config, tools, p.agentConfig), nil
 	}
 	return nil, fmt.Errorf("unknown provisioner type")
 }
 
 func (p *Provisioner) getAgentTools() (*coretools.Tools, error) {
-	tools, err := agenttools.ReadTools(p.dataDir, version.Current)
+	dataDir := p.agentConfig.DataDir()
+	tools, err := agenttools.ReadTools(dataDir, version.Current)
 	if err != nil {
-		logger.Errorf("cannot read agent tools from %q", p.dataDir)
+		logger.Errorf("cannot read agent tools from %q", dataDir)
 		return nil, err
 	}
 	return tools, nil

=== modified file 'worker/provisioner/provisioner_test.go'
--- worker/provisioner/provisioner_test.go	2013-08-30 18:35:54 +0000
+++ worker/provisioner/provisioner_test.go	2013-09-12 22:52:57 +0000
@@ -285,12 +285,14 @@
 	})
 }
 
-func (s *ProvisionerSuite) newEnvironProvisioner(machineId string) *provisioner.Provisioner {
-	return provisioner.NewProvisioner(provisioner.ENVIRON, s.State, machineId, "")
+func (s *ProvisionerSuite) newEnvironProvisioner(c *gc.C, machineId string) *provisioner.Provisioner {
+	machineTag := names.MachineTag(machineId)
+	agentConfig := s.AgentConfigForTag(c, machineTag)
+	return provisioner.NewProvisioner(provisioner.ENVIRON, s.State, machineId, agentConfig)
 }
 
 func (s *ProvisionerSuite) TestProvisionerStartStop(c *gc.C) {
-	p := s.newEnvironProvisioner("0")
+	p := s.newEnvironProvisioner(c, "0")
 	c.Assert(p.Stop(), gc.IsNil)
 }
 
@@ -304,7 +306,7 @@
 }
 
 func (s *ProvisionerSuite) TestSimple(c *gc.C) {
-	p := s.newEnvironProvisioner("0")
+	p := s.newEnvironProvisioner(c, "0")
 	defer stop(c, p)
 
 	// Check that an instance is provisioned when the machine is created...
@@ -327,14 +329,14 @@
 	c.Assert(err, gc.IsNil)
 
 	// Start a provisioner and check those constraints are used.
-	p := s.newEnvironProvisioner("0")
+	p := s.newEnvironProvisioner(c, "0")
 	defer stop(c, p)
 	s.checkStartInstanceCustom(c, m, "pork", cons)
 }
 
 func (s *ProvisionerSuite) TestProvisionerSetsErrorStatusWhenStartInstanceFailed(c *gc.C) {
 	brokenMsg := breakDummyProvider(c, s.State, "StartInstance")
-	p := s.newEnvironProvisioner("0")
+	p := s.newEnvironProvisioner(c, "0")
 	defer stop(c, p)
 
 	// Check that an instance is not provisioned when the machine is created...
@@ -362,13 +364,13 @@
 
 	// Restart the PA to make sure the machine is skipped again.
 	stop(c, p)
-	p = s.newEnvironProvisioner("0")
+	p = s.newEnvironProvisioner(c, "0")
 	defer stop(c, p)
 	s.checkNoOperations(c)
 }
 
 func (s *ProvisionerSuite) TestProvisioningDoesNotOccurForContainers(c *gc.C) {
-	p := s.newEnvironProvisioner("0")
+	p := s.newEnvironProvisioner(c, "0")
 	defer stop(c, p)
 
 	// create a machine to host the container.
@@ -401,7 +403,7 @@
 	err := s.invalidateEnvironment(c)
 	c.Assert(err, gc.IsNil)
 
-	p := s.newEnvironProvisioner("0")
+	p := s.newEnvironProvisioner(c, "0")
 	defer stop(c, p)
 
 	// try to create a machine
@@ -416,7 +418,7 @@
 	err := s.invalidateEnvironment(c)
 	c.Assert(err, gc.IsNil)
 
-	p := s.newEnvironProvisioner("0")
+	p := s.newEnvironProvisioner(c, "0")
 	defer stop(c, p)
 
 	// try to create a machine
@@ -433,7 +435,7 @@
 }
 
 func (s *ProvisionerSuite) TestProvisioningDoesOccurAfterInvalidEnvironmentPublished(c *gc.C) {
-	p := s.newEnvironProvisioner("0")
+	p := s.newEnvironProvisioner(c, "0")
 	defer stop(c, p)
 
 	// place a new machine into the state
@@ -454,7 +456,7 @@
 }
 
 func (s *ProvisionerSuite) TestProvisioningDoesNotProvisionTheSameMachineAfterRestart(c *gc.C) {
-	p := s.newEnvironProvisioner("0")
+	p := s.newEnvironProvisioner(c, "0")
 	defer stop(c, p)
 
 	// create a machine
@@ -464,7 +466,7 @@
 
 	// restart the PA
 	stop(c, p)
-	p = s.newEnvironProvisioner("0")
+	p = s.newEnvironProvisioner(c, "0")
 	defer stop(c, p)
 
 	// check that there is only one machine known
@@ -478,7 +480,7 @@
 }
 
 func (s *ProvisionerSuite) TestProvisioningStopsInstances(c *gc.C) {
-	p := s.newEnvironProvisioner("0")
+	p := s.newEnvironProvisioner(c, "0")
 	defer stop(c, p)
 
 	// create a machine
@@ -500,14 +502,14 @@
 	c.Assert(m1.Remove(), gc.IsNil)
 
 	// start a new provisioner to shut them both down
-	p = s.newEnvironProvisioner("0")
+	p = s.newEnvironProvisioner(c, "0")
 	defer stop(c, p)
 	s.checkStopInstances(c, i0, i1)
 	s.waitRemoved(c, m0)
 }
 
 func (s *ProvisionerSuite) TestDyingMachines(c *gc.C) {
-	p := s.newEnvironProvisioner("0")
+	p := s.newEnvironProvisioner(c, "0")
 	defer stop(c, p)
 
 	// provision a machine
@@ -527,7 +529,7 @@
 	c.Assert(err, gc.IsNil)
 
 	// start the provisioner and wait for it to reap the useless machine
-	p = s.newEnvironProvisioner("0")
+	p = s.newEnvironProvisioner(c, "0")
 	defer stop(c, p)
 	s.checkNoOperations(c)
 	s.waitRemoved(c, m1)
@@ -539,7 +541,7 @@
 }
 
 func (s *ProvisionerSuite) TestProvisioningRecoversAfterInvalidEnvironmentPublished(c *gc.C) {
-	p := s.newEnvironProvisioner("0")
+	p := s.newEnvironProvisioner(c, "0")
 	defer stop(c, p)
 
 	// place a new machine into the state