Merge lp:~axwalk/juju-core/refactor-environs-manual into lp:~go-bot/juju-core/trunk

[Andrew Wilkins]

Refactor environs/{cloudinit,manual}

environs/cloudinit has been modified to
have two sub-Configure methods:
ConfigureBase and ConfigureJuju. The
former initialises a cloud-init config
to setup a base OS with SSH keys; the
latter does the rest.

environs/manual has been modified to
expose provisionMachineAgent (now with
a capital P), which now just takes a
hostname and *cloudinit.MachineConfig.
We will use this later to install
machine agents into cloud images.

https://codereview.appspot.com/26570047/

Refactor environs/{cloudinit,manual}

environs/cloudinit has been modified to
have two sub-Configure methods:
ConfigureBase and ConfigureJuju. The
former initialises a cloud-init config
to setup a base OS with SSH keys; the
latter does the rest.

environs/manual has been modified to
expose provisionMachineAgent (now with
a capital P), which now just takes a
hostname and *cloudinit.MachineConfig.
We will use this later to install
machine agents into cloud images.

https://codereview.appspot.com/26570047/

[Andrew Wilkins (axwalk)]

Reviewers: mp+195342_code.launchpad.net,

Message:
Please take a look.

Description:
Refactor environs/{cloudinit,manual}

environs/cloudinit has been modified to
have two sub-Configure methods:
ConfigureBase and ConfigureJuju. The
former initialises a cloud-init config
to setup a base OS with SSH keys; the
latter does the rest.

environs/manual has been modified to
expose provisionMachineAgent (now with
a capital P), which now just takes a
hostname and *cloudinit.MachineConfig.
We will use this later to install
machine agents into cloud images.

https://code.launchpad.net/~axwalk/juju-core/refactor-environs-manual/+merge/195342

(do not edit description out of merge proposal)

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

Affected files (+108, -130 lines):
   A [revision details]
   M container/userdata.go
   M environs/cloudinit.go
   M environs/cloudinit/cloudinit.go
   M environs/cloudinit/cloudinit_test.go
   M environs/manual/agent.go
   M environs/manual/agent_test.go
   M environs/manual/bootstrap.go
   M environs/manual/provisioner.go

[Roger Peppe (rogpeppe)]

LGTM with one thought below.

https://codereview.appspot.com/26570047/diff/1/environs/cloudinit/cloudinit.go
File environs/cloudinit/cloudinit.go (right):

https://codereview.appspot.com/26570047/diff/1/environs/cloudinit/cloudinit.go#newcode145
environs/cloudinit/cloudinit.go:145: func ConfigureBase(authorizedKeys
string, c *cloudinit.Config) error {
Passing thought: I wonder if this function and the above might work
better with the cloudinit.Config as the first argument, as if it was the
receiver.

Also I'm not entirely sure that "Base" is quite right as a name - with
that name, it sounds as if it's a necessary base that others must build
on, but since the cloudinit config is passed in, that's not necessarily
the case.

As an alternative, we could make it return the new config:

func BaseConfig(authorizedKeys string) *cloudinit.Config

(no need to return an error, I think). Then it really would be a base,
because everything would start with it.

Or alternatively rename it to something else; ConfigureBasic perhaps?
I think my preference is for the above though.

https://codereview.appspot.com/26570047/diff/1/environs/manual/agent.go
File environs/manual/agent.go (left):

https://codereview.appspot.com/26570047/diff/1/environs/manual/agent.go#oldcode70
environs/manual/agent.go:70: var mcfg *cloudinit.MachineConfig
It's great to see all this go.

https://codereview.appspot.com/26570047/

[Andrew Wilkins (axwalk)]

https://codereview.appspot.com/26570047/diff/1/environs/cloudinit/cloudinit.go
File environs/cloudinit/cloudinit.go (right):

https://codereview.appspot.com/26570047/diff/1/environs/cloudinit/cloudinit.go#newcode145
environs/cloudinit/cloudinit.go:145: func ConfigureBase(authorizedKeys
string, c *cloudinit.Config) error {
On 2013/11/15 08:36:09, rog wrote:
> Passing thought: I wonder if this function and the above might work
better with
> the cloudinit.Config as the first argument, as if it was the receiver.

Making it a receiver seems wrong to me, as that would suggest to me that
it's the cloudinit.Config configuring something else.

As for the order here: I don't have a strong preference, I was just
keeping it consistent with Configure (input first, output second).

> Also I'm not entirely sure that "Base" is quite right as a name - with
that
> name, it sounds as if it's a necessary base that others must build on,
but since
> the cloudinit config is passed in, that's not necessarily the case.

I think it may just be confusing, given how generic the word "base" is.

ConfigureBase produces cloud-init that configures a "base OS image".
That is then built on by adding the juju bits (via ConfigureJuju).

> As an alternative, we could make it return the new config:

> func BaseConfig(authorizedKeys string) *cloudinit.Config

> (no need to return an error, I think). Then it really would be a base,
because
> everything would start with it.

The thing is, not *everything* starts with it. Manual
bootstrap/provisioning doesn't use ConfigureBase, as they already have a
"base".

> Or alternatively rename it to something else; ConfigureBasic perhaps?
> I think my preference is for the above though.

To me, Basic and Base have more or less the same connotations (except
Basic has an additional meaning of not-complex). I want to say
ConfigurePristine, but it isn't exactly (it adds SSH keys...).

https://codereview.appspot.com/26570047/

[Andrew Wilkins (axwalk)]

On 2013/11/15 09:10:42, axw wrote:

https://codereview.appspot.com/26570047/diff/1/environs/cloudinit/cloudinit.go
> File environs/cloudinit/cloudinit.go (right):

https://codereview.appspot.com/26570047/diff/1/environs/cloudinit/cloudinit.go#newcode145
> environs/cloudinit/cloudinit.go:145: func ConfigureBase(authorizedKeys
string, c
> *cloudinit.Config) error {
> On 2013/11/15 08:36:09, rog wrote:
> > Passing thought: I wonder if this function and the above might work
better
> with
> > the cloudinit.Config as the first argument, as if it was the
receiver.

> Making it a receiver seems wrong to me, as that would suggest to me
that it's
> the cloudinit.Config configuring something else.

> As for the order here: I don't have a strong preference, I was just
keeping it
> consistent with Configure (input first, output second).

> > Also I'm not entirely sure that "Base" is quite right as a name -
with that
> > name, it sounds as if it's a necessary base that others must build
on, but
> since
> > the cloudinit config is passed in, that's not necessarily the case.

> I think it may just be confusing, given how generic the word "base"
is.

> ConfigureBase produces cloud-init that configures a "base OS image".
That is
> then built on by adding the juju bits (via ConfigureJuju).

> > As an alternative, we could make it return the new config:
> >
> > func BaseConfig(authorizedKeys string) *cloudinit.Config
> >
> > (no need to return an error, I think). Then it really would be a
base, because
> > everything would start with it.

> The thing is, not *everything* starts with it. Manual
bootstrap/provisioning
> doesn't use ConfigureBase, as they already have a "base".

> > Or alternatively rename it to something else; ConfigureBasic
perhaps?
> > I think my preference is for the above though.

> To me, Basic and Base have more or less the same connotations (except
Basic has
> an additional meaning of not-complex). I want to say
ConfigurePristine, but it
> isn't exactly (it adds SSH keys...).

As per discussion on IRC, ConfigureBase and ConfigureJuju can be
rejoined. I had thought I would need to split them, but as it turns out
I won't.

https://codereview.appspot.com/26570047/

[Andrew Wilkins (axwalk)]

Please take a look.

https://codereview.appspot.com/26570047/

[Roger Peppe (rogpeppe)]

LGTM with a couple of trivial thoughts below.

https://codereview.appspot.com/26570047/diff/20001/environs/cloudinit.go
File environs/cloudinit.go (right):

https://codereview.appspot.com/26570047/diff/20001/environs/cloudinit.go#newcode127
environs/cloudinit.go:127: // to run on the instance. Use with care.
We should probably document that the scripts run before anything else.

https://codereview.appspot.com/26570047/diff/20001/environs/cloudinit/cloudinit.go
File environs/cloudinit/cloudinit.go (right):

https://codereview.appspot.com/26570047/diff/20001/environs/cloudinit/cloudinit.go#newcode129
environs/cloudinit/cloudinit.go:129: // configuration to initialise a
Juju machine agent.
s/initialise/initialize/

might as well use the usual american spelling everywhere.

https://codereview.appspot.com/26570047/diff/20001/environs/cloudinit/cloudinit.go#newcode138
environs/cloudinit/cloudinit.go:138: c.SetOutput(cloudinit.OutAll, "|
tee -a /var/log/cloud-init-output.log", "")
Much nicer having this at the start, thanks.

https://codereview.appspot.com/26570047/

[Andrew Wilkins (axwalk)]

Please take a look.

https://codereview.appspot.com/26570047/diff/20001/environs/cloudinit.go
File environs/cloudinit.go (right):

https://codereview.appspot.com/26570047/diff/20001/environs/cloudinit.go#newcode127
environs/cloudinit.go:127: // to run on the instance. Use with care.
On 2013/11/15 10:01:44, rog wrote:
> We should probably document that the scripts run before anything else.

Done. Not quite before *everything* else, just the runcmds.

https://codereview.appspot.com/26570047/diff/20001/environs/cloudinit/cloudinit.go
File environs/cloudinit/cloudinit.go (right):

https://codereview.appspot.com/26570047/diff/20001/environs/cloudinit/cloudinit.go#newcode129
environs/cloudinit/cloudinit.go:129: // configuration to initialise a
Juju machine agent.
On 2013/11/15 10:01:44, rog wrote:
> s/initialise/initialize/

> might as well use the usual american spelling everywhere.

Done.

https://codereview.appspot.com/26570047/

[Andrew Wilkins (axwalk)]

On 2013/11/15 10:12:35, axw wrote:
> Please take a look.

I'm an idiot. We *do* need separate Configure methods: for (non-manual)
synchronous bootstrap. There we only want a base OS image with SSH keys
and cloud-init output logs configured. For manual bootstrap we don't
care, but for non-manual we do care.

I'll have to reinstate the two methods, but I'll try to come up with a
better name.

https://codereview.appspot.com/26570047/

=== modified file 'container/userdata.go'
--- container/userdata.go	2013-11-11 21:54:29 +0000
+++ container/userdata.go	2013-11-15 10:10:33 +0000
@@ -11,6 +11,7 @@
 
 	"launchpad.net/loggo"
 
+	coreCloudinit "launchpad.net/juju-core/cloudinit"
 	"launchpad.net/juju-core/environs/cloudinit"
 	"launchpad.net/juju-core/utils"
 )
@@ -37,7 +38,8 @@
 func cloudInitUserData(machineConfig *cloudinit.MachineConfig) ([]byte, error) {
 	// consider not having this line hardcoded...
 	machineConfig.DataDir = "/var/lib/juju"
-	cloudConfig, err := cloudinit.New(machineConfig)
+	cloudConfig := coreCloudinit.New()
+	err := cloudinit.Configure(machineConfig, cloudConfig)
 	if err != nil {
 		return nil, err
 	}

=== modified file 'environs/cloudinit.go'
--- environs/cloudinit.go	2013-11-07 09:09:55 +0000
+++ environs/cloudinit.go	2013-11-15 10:10:33 +0000
@@ -124,13 +124,14 @@
 
 // ComposeUserData puts together a binary (gzipped) blob of user data.
 // The additionalScripts are additional command lines that you need cloudinit
-// to run on the instance.  Use with care.
+// to run on the instance; they are executed before all other cloud-init
+// runcmds.  Use with care.
 func ComposeUserData(cfg *cloudinit.MachineConfig, additionalScripts ...string) ([]byte, error) {
 	cloudcfg := coreCloudinit.New()
 	for _, script := range additionalScripts {
 		cloudcfg.AddRunCmd(script)
 	}
-	cloudcfg, err := cloudinit.Configure(cfg, cloudcfg)
+	err := cloudinit.Configure(cfg, cloudcfg)
 	if err != nil {
 		return nil, err
 	}

=== modified file 'environs/cloudinit/cloudinit.go'
--- environs/cloudinit/cloudinit.go	2013-11-01 04:00:55 +0000
+++ environs/cloudinit/cloudinit.go	2013-11-15 10:10:33 +0000
@@ -125,16 +125,19 @@
 	return base64.StdEncoding.EncodeToString(data)
 }
 
-func New(cfg *MachineConfig) (*cloudinit.Config, error) {
-	c := cloudinit.New()
-	return Configure(cfg, c)
-}
-
-func Configure(cfg *MachineConfig, c *cloudinit.Config) (*cloudinit.Config, error) {
+// Configure updates the provided cloudinit.Config with
+// configuration to initialize a Juju machine agent.
+func Configure(cfg *MachineConfig, c *cloudinit.Config) error {
 	if err := verifyConfig(cfg); err != nil {
-		return nil, err
+		return err
 	}
+
+	// General options.
+	c.SetAptUpgrade(true)
+	c.SetAptUpdate(true)
+	c.SetOutput(cloudinit.OutAll, "| tee -a /var/log/cloud-init-output.log", "")
 	c.AddSSHAuthorizedKeys(cfg.AuthorizedKeys)
+
 	c.AddPackage("git")
 	// Perfectly reasonable to install lxc on environment instances and kvm
 	// containers.
@@ -161,7 +164,7 @@
 	}
 	toolsJson, err := json.Marshal(cfg.Tools)
 	if err != nil {
-		return nil, err
+		return err
 	}
 	c.AddScripts(
 		"bin="+shquote(cfg.jujuTools()),
@@ -176,7 +179,7 @@
 	)
 
 	if err := cfg.addLogging(c); err != nil {
-		return nil, err
+		return err
 	}
 
 	// We add the machine agent's configuration info
@@ -188,7 +191,7 @@
 	machineTag := names.MachineTag(cfg.MachineId)
 	_, err = cfg.addAgentInfo(c, machineTag)
 	if err != nil {
-		return nil, err
+		return err
 	}
 
 	// Add the cloud archive cloud-tools pocket to apt sources
@@ -212,7 +215,7 @@
 		certKey := string(cfg.StateServerCert) + string(cfg.StateServerKey)
 		c.AddFile(cfg.dataFile("server.pem"), certKey, 0600)
 		if err := cfg.addMongoToBoot(c); err != nil {
-			return nil, err
+			return err
 		}
 		// We temporarily give bootstrap-state a directory
 		// of its own so that it can get the state info via the
@@ -223,7 +226,7 @@
 		// We leave it for the time being for backward compatibility.
 		acfg, err := cfg.addAgentInfo(c, "bootstrap")
 		if err != nil {
-			return nil, err
+			return err
 		}
 		cons := cfg.Constraints.String()
 		if cons != "" {
@@ -241,15 +244,7 @@
 		)
 	}
 
-	if err := cfg.addMachineAgentToBoot(c, machineTag, cfg.MachineId); err != nil {
-		return nil, err
-	}
-
-	// general options
-	c.SetAptUpgrade(true)
-	c.SetAptUpdate(true)
-	c.SetOutput(cloudinit.OutAll, "| tee -a /var/log/cloud-init-output.log", "")
-	return c, nil
+	return cfg.addMachineAgentToBoot(c, machineTag, cfg.MachineId)
 }
 
 func (cfg *MachineConfig) addLogging(c *cloudinit.Config) error {

=== modified file 'environs/cloudinit/cloudinit_test.go'
--- environs/cloudinit/cloudinit_test.go	2013-10-23 23:54:55 +0000
+++ environs/cloudinit/cloudinit_test.go	2013-11-15 10:10:33 +0000
@@ -352,7 +352,8 @@
 		if test.setEnvConfig {
 			test.cfg.Config = minimalConfig(c)
 		}
-		ci, err := cloudinit.New(&test.cfg)
+		ci := coreCloudinit.New()
+		err := cloudinit.Configure(&test.cfg, ci)
 		c.Assert(err, gc.IsNil)
 		c.Check(ci, gc.NotNil)
 		// render the cloudinit config to bytes, and then
@@ -394,9 +395,8 @@
 		test.cfg.Config = minimalConfig(c)
 		c.Logf("test %d (Configure)", i)
 		cloudcfg := coreCloudinit.New()
-		ci, err := cloudinit.Configure(&test.cfg, cloudcfg)
+		err := cloudinit.Configure(&test.cfg, cloudcfg)
 		c.Assert(err, gc.IsNil)
-		c.Check(ci, gc.NotNil)
 	}
 }
 
@@ -406,10 +406,9 @@
 	script := "test script"
 	cloudcfg.AddRunCmd(script)
 	cloudinitTests[0].cfg.Config = minimalConfig(c)
-	ci, err := cloudinit.Configure(&cloudinitTests[0].cfg, cloudcfg)
+	err := cloudinit.Configure(&cloudinitTests[0].cfg, cloudcfg)
 	c.Assert(err, gc.IsNil)
-	c.Check(ci, gc.NotNil)
-	data, err := ci.Render()
+	data, err := cloudcfg.Render()
 	c.Assert(err, gc.IsNil)
 
 	ciContent := make(map[interface{}]interface{})
@@ -683,16 +682,16 @@
 		MachineNonce: "FAKE_NONCE",
 	}
 	// check that the base configuration does not give an error
-	_, err := cloudinit.New(cfg)
+	ci := coreCloudinit.New()
+	err := cloudinit.Configure(cfg, ci)
 	c.Assert(err, gc.IsNil)
 
 	for i, test := range verifyTests {
 		c.Logf("test %d. %s", i, test.err)
 		cfg1 := *cfg
 		test.mutate(&cfg1)
-		t, err := cloudinit.New(&cfg1)
+		err = cloudinit.Configure(&cfg1, ci)
 		c.Assert(err, gc.ErrorMatches, "invalid machine configuration: "+test.err)
-		c.Assert(t, gc.IsNil)
 	}
 }
 

=== modified file 'environs/manual/agent.go'
--- environs/manual/agent.go	2013-11-01 04:06:00 +0000
+++ environs/manual/agent.go	2013-11-15 10:10:33 +0000
@@ -9,49 +9,23 @@
 	"os"
 	"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"
-	"launchpad.net/juju-core/environs/config"
-	"launchpad.net/juju-core/provider"
-	"launchpad.net/juju-core/state"
-	"launchpad.net/juju-core/state/api"
-	"launchpad.net/juju-core/tools"
 	"launchpad.net/juju-core/utils"
 )
 
-type provisionMachineAgentArgs struct {
-	host          string
-	dataDir       string
-	bootstrap     bool
-	environConfig *config.Config
-	machineId     string
-	nonce         string
-	stateFileURL  string
-	stateInfo     *state.Info
-	apiInfo       *api.Info
-	tools         *tools.Tools
-
-	// agentEnv is an optional map of
-	// arbitrary key/value pairs to pass
-	// into the machine agent.
-	agentEnv map[string]string
-}
-
-// provisionMachineAgent connects to a machine over SSH,
-// copies across the tools, and installs a machine agent.
-func provisionMachineAgent(args provisionMachineAgentArgs) error {
-	logger.Infof("Provisioning machine agent on %s", args.host)
-	script, err := provisionMachineAgentScript(args)
+// ProvisionMachineAgent connects to a machine over SSH,
+// and installs a machine agent.
+func ProvisionMachineAgent(host string, mcfg *cloudinit.MachineConfig) error {
+	logger.Infof("Provisioning machine agent on %s", host)
+	script, err := provisionMachineAgentScript(mcfg)
 	if err != nil {
 		return err
 	}
 	scriptBase64 := base64.StdEncoding.EncodeToString([]byte(script))
 	script = fmt.Sprintf(`F=$(mktemp); echo %s | base64 -d > $F; . $F`, scriptBase64)
 	cmd := sshCommand(
-		args.host,
+		host,
 		fmt.Sprintf("sudo bash -c '%s'", script),
 		allocateTTY,
 	)
@@ -62,31 +36,13 @@
 }
 
 // provisionMachineAgentScript generates the script necessary
-// to install a machine agent on the specified host.
-func provisionMachineAgentScript(args provisionMachineAgentArgs) (string, error) {
+// to install a machine agent with the provided MachineConfig.
+func provisionMachineAgentScript(mcfg *cloudinit.MachineConfig) (string, error) {
 	// We generate a cloud-init config, which we'll then pull the runcmds
 	// and prerequisite packages out of. Rather than generating a cloud-config,
 	// we'll just generate a shell script.
-	var mcfg *cloudinit.MachineConfig
-	if args.bootstrap {
-		mcfg = environs.NewBootstrapMachineConfig(args.stateFileURL)
-	} else {
-		mcfg = environs.NewMachineConfig(args.machineId, args.nonce, args.stateInfo, args.apiInfo)
-	}
-	if args.dataDir != "" {
-		mcfg.DataDir = args.dataDir
-	}
-	mcfg.Tools = args.tools
-	err := environs.FinishMachineConfig(mcfg, args.environConfig, constraints.Value{})
-	if err != nil {
-		return "", err
-	}
-	mcfg.AgentEnvironment[agent.ProviderType] = provider.Null
-	for k, v := range args.agentEnv {
-		mcfg.AgentEnvironment[k] = v
-	}
 	cloudcfg := corecloudinit.New()
-	if cloudcfg, err = cloudinit.Configure(mcfg, cloudcfg); err != nil {
+	if err := cloudinit.Configure(mcfg, cloudcfg); err != nil {
 		return "", err
 	}
 
@@ -148,7 +104,7 @@
 		}
 		cmds = append(cmds, "apt-add-repository -y "+utils.ShQuote(src.Source))
 	}
-	if cfg.AptUpdate() {
+	if len(cfg.AptSources()) > 0 {
 		cmds = append(cmds, "apt-get -y update")
 	}
 	// Note: explicitly ignoring apt_upgrade, so as not to trample the target

=== modified file 'environs/manual/agent_test.go'
--- environs/manual/agent_test.go	2013-11-01 04:06:00 +0000
+++ environs/manual/agent_test.go	2013-11-15 10:10:33 +0000
@@ -6,6 +6,9 @@
 import (
 	gc "launchpad.net/gocheck"
 
+	"launchpad.net/juju-core/constraints"
+	"launchpad.net/juju-core/environs"
+	"launchpad.net/juju-core/environs/cloudinit"
 	"launchpad.net/juju-core/environs/config"
 	envtools "launchpad.net/juju-core/environs/tools"
 	_ "launchpad.net/juju-core/provider/dummy"
@@ -34,20 +37,21 @@
 	return testConfig
 }
 
-func (s *agentSuite) getArgs(c *gc.C, stateServer bool, vers version.Binary) provisionMachineAgentArgs {
-	tools := &tools.Tools{Version: vers}
-	tools.URL = "file:///var/lib/juju/storage/" + envtools.StorageName(vers)
-	return provisionMachineAgentArgs{
-		bootstrap:     stateServer,
-		environConfig: dummyConfig(c, stateServer, vers),
-		machineId:     "0",
-		nonce:         "ya",
-		stateFileURL:  "http://whatever/dotcom",
-		tools:         tools,
-		// stateInfo *state.Info
-		// apiInfo *api.Info
-		agentEnv: make(map[string]string),
-	}
+func (s *agentSuite) getMachineConfig(c *gc.C, stateServer bool, vers version.Binary) *cloudinit.MachineConfig {
+	var mcfg *cloudinit.MachineConfig
+	if stateServer {
+		mcfg = environs.NewBootstrapMachineConfig("http://whatever/dotcom")
+	} else {
+		mcfg = environs.NewMachineConfig("0", "ya", nil, nil)
+	}
+	mcfg.Tools = &tools.Tools{
+		Version: vers,
+		URL:     "file:///var/lib/juju/storage/" + envtools.StorageName(vers),
+	}
+	environConfig := dummyConfig(c, stateServer, vers)
+	err := environs.FinishMachineConfig(mcfg, environConfig, constraints.Value{})
+	c.Assert(err, gc.IsNil)
+	return mcfg
 }
 
 var allSeries = [...]string{"precise", "quantal", "raring", "saucy"}
@@ -62,7 +66,7 @@
 func (s *agentSuite) TestAptSources(c *gc.C) {
 	for _, series := range allSeries {
 		vers := version.MustParseBinary("1.16.0-" + series + "-amd64")
-		script, err := provisionMachineAgentScript(s.getArgs(c, true, vers))
+		script, err := provisionMachineAgentScript(s.getMachineConfig(c, true, vers))
 		c.Assert(err, gc.IsNil)
 
 		// Only Precise requires the cloud-tools pocket.

=== modified file 'environs/manual/bootstrap.go'
--- environs/manual/bootstrap.go	2013-10-02 10:38:04 +0000
+++ environs/manual/bootstrap.go	2013-11-15 10:10:33 +0000
@@ -7,11 +7,11 @@
 	"errors"
 	"fmt"
 
+	"launchpad.net/juju-core/constraints"
 	"launchpad.net/juju-core/environs"
 	envtools "launchpad.net/juju-core/environs/tools"
 	"launchpad.net/juju-core/instance"
 	"launchpad.net/juju-core/provider/common"
-	"launchpad.net/juju-core/state"
 	"launchpad.net/juju-core/tools"
 	"launchpad.net/juju-core/worker/localstorage"
 )
@@ -108,15 +108,17 @@
 
 	// Finally, provision the machine agent.
 	stateFileURL := fmt.Sprintf("file://%s/%s", storageDir, common.StateFile)
-	err = provisionMachineAgent(provisionMachineAgentArgs{
-		host:          args.Host,
-		dataDir:       args.DataDir,
-		environConfig: args.Environ.Config(),
-		stateFileURL:  stateFileURL,
-		bootstrap:     true,
-		nonce:         state.BootstrapNonce,
-		tools:         &tools,
-		agentEnv:      agentEnv,
-	})
-	return err
+	mcfg := environs.NewBootstrapMachineConfig(stateFileURL)
+	if args.DataDir != "" {
+		mcfg.DataDir = args.DataDir
+	}
+	mcfg.Tools = &tools
+	err = environs.FinishMachineConfig(mcfg, args.Environ.Config(), constraints.Value{})
+	if err != nil {
+		return err
+	}
+	for k, v := range agentEnv {
+		mcfg.AgentEnvironment[k] = v
+	}
+	return ProvisionMachineAgent(args.Host, mcfg)
 }

=== modified file 'environs/manual/provisioner.go'
--- environs/manual/provisioner.go	2013-11-08 02:52:25 +0000
+++ environs/manual/provisioner.go	2013-11-15 10:10:33 +0000
@@ -11,6 +11,9 @@
 
 	"launchpad.net/loggo"
 
+	"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/instance"
 	"launchpad.net/juju-core/juju"
@@ -80,16 +83,13 @@
 	}
 
 	// Gather the information needed by the machine agent to run the provisioning script.
-	provisioningArgs, err := createProvisioningArgs(client, machineId, series, arch)
+	mcfg, err := createMachineConfig(client, machineId, series, arch, nonce, args.DataDir)
 	if err != nil {
 		return machineId, err
 	}
-	provisioningArgs.host = args.Host
-	provisioningArgs.dataDir = args.DataDir
-	provisioningArgs.nonce = nonce
 
 	// Finally, provision the machine agent.
-	err = provisionMachineAgent(*provisioningArgs)
+	err = ProvisionMachineAgent(args.Host, mcfg)
 	if err != nil {
 		return machineId, err
 	}
@@ -170,12 +170,11 @@
 	return machineInfo.Machine, series, *hc.Arch, nil
 }
 
-func createProvisioningArgs(client *api.Client, machineId, series, arch string) (*provisionMachineAgentArgs, error) {
+func createMachineConfig(client *api.Client, machineId, series, arch, nonce, dataDir string) (*cloudinit.MachineConfig, error) {
 	configParameters, err := client.MachineConfig(machineId, series, arch)
 	if err != nil {
 		return nil, err
 	}
-
 	stateInfo := &state.Info{
 		Addrs:    configParameters.StateAddrs,
 		Password: configParameters.Password,
@@ -192,13 +191,14 @@
 	if err != nil {
 		return nil, err
 	}
-
-	return &provisionMachineAgentArgs{
-		environConfig: environConfig,
-		machineId:     machineId,
-		bootstrap:     false,
-		stateInfo:     stateInfo,
-		apiInfo:       apiInfo,
-		tools:         configParameters.Tools,
-	}, nil
+	mcfg := environs.NewMachineConfig(machineId, nonce, stateInfo, apiInfo)
+	if dataDir != "" {
+		mcfg.DataDir = dataDir
+	}
+	mcfg.Tools = configParameters.Tools
+	err = environs.FinishMachineConfig(mcfg, environConfig, constraints.Value{})
+	if err != nil {
+		return nil, err
+	}
+	return mcfg, nil
 }