Merge lp:~axwalk/juju-core/startinstance-param-struct into lp:~go-bot/juju-core/trunk

[Andrew Wilkins]

Refactor StartInstance to take param struct

This is in preparation for adding an optional
argument to StartInstance that names the
principal unit initially assigned to the
machine. No functional changes here, just
refactoring.

https://codereview.appspot.com/75130046/

Refactor StartInstance to take param struct

This is in preparation for adding an optional
argument to StartInstance that names the
principal unit initially assigned to the
machine. No functional changes here, just
refactoring.

https://codereview.appspot.com/75130046/

[Andrew Wilkins (axwalk)]

Reviewers: mp+210738_code.launchpad.net,

Message:
Please take a look.

Description:
Refactor StartInstance to take param struct

This is in preparation for adding an optional
argument to StartInstance that names the
principal unit initially assigned to the
machine. No functional changes here, just
refactoring.

https://code.launchpad.net/~axwalk/juju-core/startinstance-param-struct/+merge/210738

(do not edit description out of merge proposal)

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

Affected files (+131, -127 lines):
   A [revision details]
   M environs/broker.go
   M environs/jujutest/livetests.go
   M juju/testing/instance.go
   M provider/azure/environ.go
   M provider/common/bootstrap.go
   M provider/common/mock_test.go
   M provider/dummy/environs.go
   M provider/ec2/ec2.go
   M provider/joyent/environ_instance.go
   M provider/local/environ.go
   M provider/maas/environ.go
   M provider/manual/environ.go
   M provider/openstack/provider.go
   M worker/provisioner/kvm-broker.go
   M worker/provisioner/kvm-broker_test.go
   M worker/provisioner/lxc-broker.go
   M worker/provisioner/lxc-broker_test.go
   M worker/provisioner/provisioner_task.go

[Ian Booth (wallyworld)]

LGTM. Seems like a good change regardless of whether extra params are
added or not.

https://codereview.appspot.com/75130046/

=== modified file 'environs/broker.go'
--- environs/broker.go	2013-08-22 22:24:54 +0000
+++ environs/broker.go	2014-03-13 05:11:53 +0000
@@ -10,6 +10,21 @@
 	"launchpad.net/juju-core/tools"
 )
 
+// StartInstanceParams holds parameters for the
+// InstanceBroker.StartInstace method.
+type StartInstanceParams struct {
+	// Constraints is a set of constraints on
+	// the kind of instance to create.
+	Constraints constraints.Value
+
+	// Tools is a list of tools that may be used
+	// to start a Juju agent on the machine.
+	Tools tools.List
+
+	// MachineConfig describes the machine's configuration.
+	MachineConfig *cloudinit.MachineConfig
+}
+
 // TODO(wallyworld) - we want this in the environs/instance package but import loops
 // stop that from being possible right now.
 type InstanceBroker interface {
@@ -19,10 +34,7 @@
 	// unique within an environment, is used by juju to protect against the
 	// consequences of multiple instances being started with the same machine
 	// id.
-	StartInstance(
-		cons constraints.Value, possibleTools tools.List,
-		machineConfig *cloudinit.MachineConfig,
-	) (instance.Instance, *instance.HardwareCharacteristics, error)
+	StartInstance(args StartInstanceParams) (instance.Instance, *instance.HardwareCharacteristics, error)
 
 	// StopInstances shuts down the given instances.
 	StopInstances([]instance.Instance) error

=== modified file 'environs/jujutest/livetests.go'
--- environs/jujutest/livetests.go	2014-03-10 20:22:44 +0000
+++ environs/jujutest/livetests.go	2014-03-13 05:11:53 +0000
@@ -859,7 +859,10 @@
 
 	t.PrepareOnce(c)
 	possibleTools := envtesting.AssertUploadFakeToolsVersions(c, t.Env.Storage(), version.MustParseBinary("5.4.5-precise-amd64"))
-	inst, _, err := t.Env.StartInstance(constraints.Value{}, possibleTools, machineConfig)
+	inst, _, err := t.Env.StartInstance(environs.StartInstanceParams{
+		Tools:         possibleTools,
+		MachineConfig: machineConfig,
+	})
 	if inst != nil {
 		err := t.Env.StopInstances([]instance.Instance{inst})
 		c.Check(err, gc.IsNil)

=== modified file 'juju/testing/instance.go'
--- juju/testing/instance.go	2013-09-30 19:40:06 +0000
+++ juju/testing/instance.go	2014-03-13 05:11:53 +0000
@@ -98,5 +98,9 @@
 	stateInfo := FakeStateInfo(machineId)
 	apiInfo := FakeAPIInfo(machineId)
 	machineConfig := environs.NewMachineConfig(machineId, machineNonce, stateInfo, apiInfo)
-	return env.StartInstance(cons, possibleTools, machineConfig)
+	return env.StartInstance(environs.StartInstanceParams{
+		Constraints:   cons,
+		Tools:         possibleTools,
+		MachineConfig: machineConfig,
+	})
 }

=== modified file 'provider/azure/environ.go'
--- provider/azure/environ.go	2014-03-03 10:10:44 +0000
+++ provider/azure/environ.go	2014-03-13 05:11:53 +0000
@@ -13,7 +13,6 @@
 
 	"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/environs/imagemetadata"
 	"launchpad.net/juju-core/environs/instances"
@@ -24,7 +23,6 @@
 	"launchpad.net/juju-core/provider/common"
 	"launchpad.net/juju-core/state"
 	"launchpad.net/juju-core/state/api"
-	"launchpad.net/juju-core/tools"
 	"launchpad.net/juju-core/utils/parallel"
 )
 
@@ -363,24 +361,23 @@
 }
 
 // StartInstance is specified in the InstanceBroker interface.
-func (env *azureEnviron) StartInstance(cons constraints.Value, possibleTools tools.List,
-	machineConfig *cloudinit.MachineConfig) (_ instance.Instance, _ *instance.HardwareCharacteristics, err error) {
+func (env *azureEnviron) StartInstance(args environs.StartInstanceParams) (_ instance.Instance, _ *instance.HardwareCharacteristics, err error) {
 
 	// Declaring "err" in the function signature so that we can "defer"
 	// any cleanup that needs to run during error returns.
 
-	err = environs.FinishMachineConfig(machineConfig, env.Config(), cons)
+	err = environs.FinishMachineConfig(args.MachineConfig, env.Config(), args.Constraints)
 	if err != nil {
 		return nil, nil, err
 	}
 
 	// Pick envtools.  Needed for the custom data (which is what we normally
 	// call userdata).
-	machineConfig.Tools = possibleTools[0]
-	logger.Infof("picked tools %q", machineConfig.Tools)
+	args.MachineConfig.Tools = args.Tools[0]
+	logger.Infof("picked tools %q", args.MachineConfig.Tools)
 
 	// Compose userdata.
-	userData, err := makeCustomData(machineConfig)
+	userData, err := makeCustomData(args.MachineConfig)
 	if err != nil {
 		return nil, nil, fmt.Errorf("custom data: %v", err)
 	}
@@ -409,8 +406,8 @@
 		}
 	}()
 
-	series := possibleTools.OneSeries()
-	instanceType, sourceImageName, err := env.selectInstanceTypeAndImage(cons, series, location)
+	series := args.Tools.OneSeries()
+	instanceType, sourceImageName, err := env.selectInstanceTypeAndImage(args.Constraints, series, location)
 	if err != nil {
 		return nil, nil, err
 	}

=== modified file 'provider/common/bootstrap.go'
--- provider/common/bootstrap.go	2014-03-10 20:22:44 +0000
+++ provider/common/bootstrap.go	2014-03-13 05:11:53 +0000
@@ -69,7 +69,11 @@
 	}
 
 	fmt.Fprintln(ctx.GetStderr(), "Launching instance")
-	inst, hw, err := env.StartInstance(cons, selectedTools, machineConfig)
+	inst, hw, err := env.StartInstance(environs.StartInstanceParams{
+		Constraints:   cons,
+		Tools:         selectedTools,
+		MachineConfig: machineConfig,
+	})
 	if err != nil {
 		return fmt.Errorf("cannot start bootstrap instance: %v", err)
 	}

=== modified file 'provider/common/mock_test.go'
--- provider/common/mock_test.go	2014-03-03 10:10:44 +0000
+++ provider/common/mock_test.go	2014-03-13 05:11:53 +0000
@@ -45,12 +45,8 @@
 func (env *mockEnviron) AllInstances() ([]instance.Instance, error) {
 	return env.allInstances()
 }
-func (env *mockEnviron) StartInstance(
-	cons constraints.Value, possibleTools tools.List, mcfg *cloudinit.MachineConfig,
-) (
-	instance.Instance, *instance.HardwareCharacteristics, error,
-) {
-	return env.startInstance(cons, possibleTools, mcfg)
+func (env *mockEnviron) StartInstance(args environs.StartInstanceParams) (instance.Instance, *instance.HardwareCharacteristics, error) {
+	return env.startInstance(args.Constraints, args.Tools, args.MachineConfig)
 }
 
 func (env *mockEnviron) StopInstances(instances []instance.Instance) error {

=== modified file 'provider/dummy/environs.go'
--- provider/dummy/environs.go	2014-03-05 19:41:34 +0000
+++ provider/dummy/environs.go	2014-03-13 05:11:53 +0000
@@ -38,7 +38,6 @@
 	"launchpad.net/juju-core/constraints"
 	"launchpad.net/juju-core/environs"
 	"launchpad.net/juju-core/environs/bootstrap"
-	"launchpad.net/juju-core/environs/cloudinit"
 	"launchpad.net/juju-core/environs/config"
 	"launchpad.net/juju-core/environs/imagemetadata"
 	"launchpad.net/juju-core/environs/simplestreams"
@@ -53,7 +52,6 @@
 	"launchpad.net/juju-core/state/api"
 	"launchpad.net/juju-core/state/apiserver"
 	"launchpad.net/juju-core/testing"
-	coretools "launchpad.net/juju-core/tools"
 	"launchpad.net/juju-core/utils"
 )
 
@@ -677,11 +675,10 @@
 }
 
 // StartInstance is specified in the InstanceBroker interface.
-func (e *environ) StartInstance(cons constraints.Value, possibleTools coretools.List,
-	machineConfig *cloudinit.MachineConfig) (instance.Instance, *instance.HardwareCharacteristics, error) {
+func (e *environ) StartInstance(args environs.StartInstanceParams) (instance.Instance, *instance.HardwareCharacteristics, error) {
 
 	defer delay()
-	machineId := machineConfig.MachineId
+	machineId := args.MachineConfig.MachineId
 	logger.Infof("dummy startinstance, machine %s", machineId)
 	if err := e.checkBroken("StartInstance"); err != nil {
 		return nil, nil, err
@@ -692,20 +689,20 @@
 	}
 	estate.mu.Lock()
 	defer estate.mu.Unlock()
-	if machineConfig.MachineNonce == "" {
+	if args.MachineConfig.MachineNonce == "" {
 		return nil, nil, fmt.Errorf("cannot start instance: missing machine nonce")
 	}
 	if _, ok := e.Config().CACert(); !ok {
 		return nil, nil, fmt.Errorf("no CA certificate in environment configuration")
 	}
-	if machineConfig.StateInfo.Tag != names.MachineTag(machineId) {
-		return nil, nil, fmt.Errorf("entity tag must match started machine")
-	}
-	if machineConfig.APIInfo.Tag != names.MachineTag(machineId) {
-		return nil, nil, fmt.Errorf("entity tag must match started machine")
-	}
-	logger.Infof("would pick tools from %s", possibleTools)
-	series := possibleTools.OneSeries()
+	if args.MachineConfig.StateInfo.Tag != names.MachineTag(machineId) {
+		return nil, nil, fmt.Errorf("entity tag must match started machine")
+	}
+	if args.MachineConfig.APIInfo.Tag != names.MachineTag(machineId) {
+		return nil, nil, fmt.Errorf("entity tag must match started machine")
+	}
+	logger.Infof("would pick tools from %s", args.Tools)
+	series := args.Tools.OneSeries()
 	i := &dummyInstance{
 		id:           instance.Id(fmt.Sprintf("%s-%d", e.name, estate.maxId)),
 		ports:        make(map[instance.Port]bool),
@@ -721,12 +718,12 @@
 		// We will just assume the instance hardware characteristics exactly matches
 		// the supplied constraints (if specified).
 		hc = &instance.HardwareCharacteristics{
-			Arch:     cons.Arch,
-			Mem:      cons.Mem,
-			RootDisk: cons.RootDisk,
-			CpuCores: cons.CpuCores,
-			CpuPower: cons.CpuPower,
-			Tags:     cons.Tags,
+			Arch:     args.Constraints.Arch,
+			Mem:      args.Constraints.Mem,
+			RootDisk: args.Constraints.RootDisk,
+			CpuCores: args.Constraints.CpuCores,
+			CpuPower: args.Constraints.CpuPower,
+			Tags:     args.Constraints.Tags,
 		}
 		// Fill in some expected instance hardware characteristics if constraints not specified.
 		if hc.Arch == nil {
@@ -751,11 +748,11 @@
 	estate.ops <- OpStartInstance{
 		Env:          e.name,
 		MachineId:    machineId,
-		MachineNonce: machineConfig.MachineNonce,
-		Constraints:  cons,
+		MachineNonce: args.MachineConfig.MachineNonce,
+		Constraints:  args.Constraints,
 		Instance:     i,
-		Info:         machineConfig.StateInfo,
-		APIInfo:      machineConfig.APIInfo,
+		Info:         args.MachineConfig.StateInfo,
+		APIInfo:      args.MachineConfig.APIInfo,
 		Secret:       e.ecfg().secret(),
 	}
 	return i, hc, nil

=== modified file 'provider/ec2/ec2.go'
--- provider/ec2/ec2.go	2014-03-12 11:04:20 +0000
+++ provider/ec2/ec2.go	2014-03-13 05:11:53 +0000
@@ -18,7 +18,6 @@
 
 	"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/environs/imagemetadata"
 	"launchpad.net/juju-core/environs/instances"
@@ -365,50 +364,49 @@
 const ebsStorage = "ebs"
 
 // StartInstance is specified in the InstanceBroker interface.
-func (e *environ) StartInstance(cons constraints.Value, possibleTools tools.List,
-	machineConfig *cloudinit.MachineConfig) (instance.Instance, *instance.HardwareCharacteristics, error) {
+func (e *environ) StartInstance(args environs.StartInstanceParams) (instance.Instance, *instance.HardwareCharacteristics, error) {
 
-	arches := possibleTools.Arches()
+	arches := args.Tools.Arches()
 	stor := ebsStorage
 	sources, err := imagemetadata.GetMetadataSources(e)
 	if err != nil {
 		return nil, nil, err
 	}
 
-	series := possibleTools.OneSeries()
+	series := args.Tools.OneSeries()
 	spec, err := findInstanceSpec(sources, e.Config().ImageStream(), &instances.InstanceConstraint{
 		Region:      e.ecfg().region(),
 		Series:      series,
 		Arches:      arches,
-		Constraints: cons,
+		Constraints: args.Constraints,
 		Storage:     &stor,
 	})
 	if err != nil {
 		return nil, nil, err
 	}
-	tools, err := possibleTools.Match(tools.Filter{Arch: spec.Image.Arch})
+	tools, err := args.Tools.Match(tools.Filter{Arch: spec.Image.Arch})
 	if err != nil {
 		return nil, nil, fmt.Errorf("chosen architecture %v not present in %v", spec.Image.Arch, arches)
 	}
 
-	machineConfig.Tools = tools[0]
-	if err := environs.FinishMachineConfig(machineConfig, e.Config(), cons); err != nil {
+	args.MachineConfig.Tools = tools[0]
+	if err := environs.FinishMachineConfig(args.MachineConfig, e.Config(), args.Constraints); err != nil {
 		return nil, nil, err
 	}
 
-	userData, err := environs.ComposeUserData(machineConfig)
+	userData, err := environs.ComposeUserData(args.MachineConfig)
 	if err != nil {
 		return nil, nil, fmt.Errorf("cannot make user data: %v", err)
 	}
 	logger.Debugf("ec2 user data; %d bytes", len(userData))
 	cfg := e.Config()
-	groups, err := e.setUpGroups(machineConfig.MachineId, cfg.StatePort(), cfg.APIPort())
+	groups, err := e.setUpGroups(args.MachineConfig.MachineId, cfg.StatePort(), cfg.APIPort())
 	if err != nil {
 		return nil, nil, fmt.Errorf("cannot set up groups: %v", err)
 	}
 	var instResp *ec2.RunInstancesResp
 
-	device, diskSize := getDiskSize(cons)
+	device, diskSize := getDiskSize(args.Constraints)
 	for a := shortAttempt.Start(); a.Next(); {
 		instResp, err = e.ec2().RunInstances(&ec2.RunInstances{
 			ImageId:             spec.Image.Id,

=== modified file 'provider/joyent/environ_instance.go'
--- provider/joyent/environ_instance.go	2014-03-10 00:10:29 +0000
+++ provider/joyent/environ_instance.go	2014-03-13 05:11:53 +0000
@@ -4,15 +4,11 @@
 package joyent
 
 import (
-	"launchpad.net/juju-core/constraints"
-	"launchpad.net/juju-core/environs/cloudinit"
+	"launchpad.net/juju-core/environs"
 	"launchpad.net/juju-core/instance"
-	"launchpad.net/juju-core/tools"
 )
 
-func (env *environ) StartInstance(
-	cons constraints.Value, possibleTools tools.List, machineConf *cloudinit.MachineConfig,
-) (
+func (env *environ) StartInstance(args environs.StartInstanceParams) (
 	instance.Instance, *instance.HardwareCharacteristics, error,
 ) {
 	// Please note that in order to fulfil the demands made of Instances and

=== modified file 'provider/local/environ.go'
--- provider/local/environ.go	2014-03-09 20:54:18 +0000
+++ provider/local/environ.go	2014-03-13 05:11:53 +0000
@@ -38,7 +38,6 @@
 	"launchpad.net/juju-core/state"
 	"launchpad.net/juju-core/state/api"
 	"launchpad.net/juju-core/state/api/params"
-	"launchpad.net/juju-core/tools"
 	"launchpad.net/juju-core/version"
 	"launchpad.net/juju-core/worker/terminationworker"
 )
@@ -286,24 +285,23 @@
 }
 
 // StartInstance is specified in the InstanceBroker interface.
-func (env *localEnviron) StartInstance(cons constraints.Value, possibleTools tools.List,
-	machineConfig *cloudinit.MachineConfig) (instance.Instance, *instance.HardwareCharacteristics, error) {
+func (env *localEnviron) StartInstance(args environs.StartInstanceParams) (instance.Instance, *instance.HardwareCharacteristics, error) {
 
-	series := possibleTools.OneSeries()
-	logger.Debugf("StartInstance: %q, %s", machineConfig.MachineId, series)
-	machineConfig.Tools = possibleTools[0]
-	machineConfig.MachineContainerType = env.config.container()
-	logger.Debugf("tools: %#v", machineConfig.Tools)
+	series := args.Tools.OneSeries()
+	logger.Debugf("StartInstance: %q, %s", args.MachineConfig.MachineId, series)
+	args.MachineConfig.Tools = args.Tools[0]
+	args.MachineConfig.MachineContainerType = env.config.container()
+	logger.Debugf("tools: %#v", args.MachineConfig.Tools)
 	network := container.BridgeNetworkConfig(env.config.networkBridge())
-	if err := environs.FinishMachineConfig(machineConfig, env.config.Config, cons); err != nil {
+	if err := environs.FinishMachineConfig(args.MachineConfig, env.config.Config, args.Constraints); err != nil {
 		return nil, nil, err
 	}
 	// TODO: evaluate the impact of setting the contstraints on the
 	// machineConfig for all machines rather than just state server nodes.
 	// This limiation is why the constraints are assigned directly here.
-	machineConfig.Constraints = cons
-	machineConfig.AgentEnvironment[agent.Namespace] = env.config.namespace()
-	inst, hardware, err := env.containerManager.StartContainer(machineConfig, series, network)
+	args.MachineConfig.Constraints = args.Constraints
+	args.MachineConfig.AgentEnvironment[agent.Namespace] = env.config.namespace()
+	inst, hardware, err := env.containerManager.StartContainer(args.MachineConfig, series, network)
 	if err != nil {
 		return nil, nil, err
 	}

=== modified file 'provider/maas/environ.go'
--- provider/maas/environ.go	2014-03-03 10:10:44 +0000
+++ provider/maas/environ.go	2014-03-13 05:11:53 +0000
@@ -16,7 +16,6 @@
 	"launchpad.net/juju-core/agent"
 	"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/environs/imagemetadata"
 	"launchpad.net/juju-core/environs/simplestreams"
@@ -230,16 +229,15 @@
 }
 
 // StartInstance is specified in the InstanceBroker interface.
-func (environ *maasEnviron) StartInstance(cons constraints.Value, possibleTools tools.List,
-	machineConfig *cloudinit.MachineConfig) (instance.Instance, *instance.HardwareCharacteristics, error) {
+func (environ *maasEnviron) StartInstance(args environs.StartInstanceParams) (instance.Instance, *instance.HardwareCharacteristics, error) {
 
 	var inst *maasInstance
 	var err error
-	if node, tools, err := environ.acquireNode(cons, possibleTools); err != nil {
+	if node, tools, err := environ.acquireNode(args.Constraints, args.Tools); err != nil {
 		return nil, nil, fmt.Errorf("cannot run instances: %v", err)
 	} else {
 		inst = &maasInstance{maasObject: &node, environ: environ}
-		machineConfig.Tools = tools
+		args.MachineConfig.Tools = tools
 	}
 	defer func() {
 		if err != nil {
@@ -258,15 +256,15 @@
 	if err != nil {
 		return nil, nil, err
 	}
-	if err := environs.FinishMachineConfig(machineConfig, environ.Config(), cons); err != nil {
+	if err := environs.FinishMachineConfig(args.MachineConfig, environ.Config(), args.Constraints); err != nil {
 		return nil, nil, err
 	}
 	// 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.AgentEnvironment[agent.LxcBridge] = "br0"
+	args.MachineConfig.AgentEnvironment[agent.LxcBridge] = "br0"
 	userdata, err := environs.ComposeUserData(
-		machineConfig,
+		args.MachineConfig,
 		runCmd,
 		createBridgeNetwork(),
 		linkBridgeInInterfaces(),
@@ -278,7 +276,7 @@
 	}
 	logger.Debugf("maas user data; %d bytes", len(userdata))
 
-	series := possibleTools.OneSeries()
+	series := args.Tools.OneSeries()
 	if err := environ.startNode(*inst.maasObject, series, userdata); err != nil {
 		return nil, nil, err
 	}

=== modified file 'provider/manual/environ.go'
--- provider/manual/environ.go	2014-03-12 14:04:20 +0000
+++ provider/manual/environ.go	2014-03-13 05:11:53 +0000
@@ -17,7 +17,6 @@
 	"launchpad.net/juju-core/agent"
 	"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/environs/httpstorage"
 	"launchpad.net/juju-core/environs/manual"
@@ -29,7 +28,6 @@
 	"launchpad.net/juju-core/provider/common"
 	"launchpad.net/juju-core/state"
 	"launchpad.net/juju-core/state/api"
-	"launchpad.net/juju-core/tools"
 	"launchpad.net/juju-core/utils/ssh"
 	"launchpad.net/juju-core/worker/localstorage"
 	"launchpad.net/juju-core/worker/terminationworker"
@@ -64,7 +62,7 @@
 var errNoStartInstance = errors.New("manual provider cannot start instances")
 var errNoStopInstance = errors.New("manual provider cannot stop instances")
 
-func (*manualEnviron) StartInstance(constraints.Value, tools.List, *cloudinit.MachineConfig) (instance.Instance, *instance.HardwareCharacteristics, error) {
+func (*manualEnviron) StartInstance(args environs.StartInstanceParams) (instance.Instance, *instance.HardwareCharacteristics, error) {
 	return nil, nil, errNoStartInstance
 }
 

=== modified file 'provider/openstack/provider.go'
--- provider/openstack/provider.go	2014-03-12 11:04:20 +0000
+++ provider/openstack/provider.go	2014-03-13 05:11:53 +0000
@@ -24,7 +24,6 @@
 
 	"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/environs/imagemetadata"
 	"launchpad.net/juju-core/environs/instances"
@@ -741,31 +740,30 @@
 }
 
 // StartInstance is specified in the InstanceBroker interface.
-func (e *environ) StartInstance(cons constraints.Value, possibleTools tools.List,
-	machineConfig *cloudinit.MachineConfig) (instance.Instance, *instance.HardwareCharacteristics, error) {
+func (e *environ) StartInstance(args environs.StartInstanceParams) (instance.Instance, *instance.HardwareCharacteristics, error) {
 
-	series := possibleTools.OneSeries()
-	arches := possibleTools.Arches()
+	series := args.Tools.OneSeries()
+	arches := args.Tools.Arches()
 	spec, err := findInstanceSpec(e, &instances.InstanceConstraint{
 		Region:      e.ecfg().region(),
 		Series:      series,
 		Arches:      arches,
-		Constraints: cons,
+		Constraints: args.Constraints,
 	})
 	if err != nil {
 		return nil, nil, err
 	}
-	tools, err := possibleTools.Match(tools.Filter{Arch: spec.Image.Arch})
+	tools, err := args.Tools.Match(tools.Filter{Arch: spec.Image.Arch})
 	if err != nil {
 		return nil, nil, fmt.Errorf("chosen architecture %v not present in %v", spec.Image.Arch, arches)
 	}
 
-	machineConfig.Tools = tools[0]
+	args.MachineConfig.Tools = tools[0]
 
-	if err := environs.FinishMachineConfig(machineConfig, e.Config(), cons); err != nil {
+	if err := environs.FinishMachineConfig(args.MachineConfig, e.Config(), args.Constraints); err != nil {
 		return nil, nil, err
 	}
-	userData, err := environs.ComposeUserData(machineConfig)
+	userData, err := environs.ComposeUserData(args.MachineConfig)
 	if err != nil {
 		return nil, nil, fmt.Errorf("cannot make user data: %v", err)
 	}
@@ -791,7 +789,7 @@
 		}
 	}
 	cfg := e.Config()
-	groups, err := e.setUpGroups(machineConfig.MachineId, cfg.StatePort(), cfg.APIPort())
+	groups, err := e.setUpGroups(args.MachineConfig.MachineId, cfg.StatePort(), cfg.APIPort())
 	if err != nil {
 		return nil, nil, fmt.Errorf("cannot set up groups: %v", err)
 	}
@@ -800,7 +798,7 @@
 		groupNames[i] = nova.SecurityGroupName{g.Name}
 	}
 	var opts = nova.RunServerOpts{
-		Name:               e.machineFullName(machineConfig.MachineId),
+		Name:               e.machineFullName(args.MachineConfig.MachineId),
 		FlavorId:           spec.InstanceType.Id,
 		ImageId:            spec.Image.Id,
 		UserData:           userData,

=== modified file 'worker/provisioner/kvm-broker.go'
--- worker/provisioner/kvm-broker.go	2014-03-07 03:00:24 +0000
+++ worker/provisioner/kvm-broker.go	2014-03-13 05:11:53 +0000
@@ -7,11 +7,9 @@
 	"github.com/juju/loggo"
 
 	"launchpad.net/juju-core/agent"
-	"launchpad.net/juju-core/constraints"
 	"launchpad.net/juju-core/container"
 	"launchpad.net/juju-core/container/kvm"
 	"launchpad.net/juju-core/environs"
-	"launchpad.net/juju-core/environs/cloudinit"
 	"launchpad.net/juju-core/instance"
 	"launchpad.net/juju-core/tools"
 )
@@ -50,14 +48,10 @@
 }
 
 // StartInstance is specified in the Broker interface.
-func (broker *kvmBroker) StartInstance(
-	cons constraints.Value,
-	possibleTools tools.List,
-	machineConfig *cloudinit.MachineConfig,
-) (instance.Instance, *instance.HardwareCharacteristics, error) {
+func (broker *kvmBroker) StartInstance(args environs.StartInstanceParams) (instance.Instance, *instance.HardwareCharacteristics, error) {
 
 	// TODO: refactor common code out of the container brokers.
-	machineId := machineConfig.MachineId
+	machineId := args.MachineConfig.MachineId
 	kvmLogger.Infof("starting kvm container for machineId: %s", machineId)
 
 	// TODO: Default to using the host network until we can configure.  Yes,
@@ -70,9 +64,9 @@
 	network := container.BridgeNetworkConfig(bridgeDevice)
 
 	// TODO: series doesn't necessarily need to be the same as the host.
-	series := possibleTools.OneSeries()
-	machineConfig.MachineContainerType = instance.KVM
-	machineConfig.Tools = possibleTools[0]
+	series := args.Tools.OneSeries()
+	args.MachineConfig.MachineContainerType = instance.KVM
+	args.MachineConfig.Tools = args.Tools[0]
 
 	config, err := broker.api.ContainerConfig()
 	if err != nil {
@@ -80,7 +74,7 @@
 		return nil, nil, err
 	}
 	if err := environs.PopulateMachineConfig(
-		machineConfig,
+		args.MachineConfig,
 		config.ProviderType,
 		config.AuthorizedKeys,
 		config.SSLHostnameVerification,
@@ -91,7 +85,7 @@
 		return nil, nil, err
 	}
 
-	inst, hardware, err := broker.manager.StartContainer(machineConfig, series, network)
+	inst, hardware, err := broker.manager.StartContainer(args.MachineConfig, series, network)
 	if err != nil {
 		kvmLogger.Errorf("failed to start container: %v", err)
 		return nil, nil, err

=== modified file 'worker/provisioner/kvm-broker_test.go'
--- worker/provisioner/kvm-broker_test.go	2014-02-21 03:57:49 +0000
+++ worker/provisioner/kvm-broker_test.go	2014-03-13 05:11:53 +0000
@@ -86,7 +86,11 @@
 	machineConfig := environs.NewMachineConfig(machineId, machineNonce, stateInfo, apiInfo)
 	cons := constraints.Value{}
 	possibleTools := s.broker.(coretools.HasTools).Tools()
-	kvm, _, err := s.broker.StartInstance(cons, possibleTools, machineConfig)
+	kvm, _, err := s.broker.StartInstance(environs.StartInstanceParams{
+		Constraints:   cons,
+		Tools:         possibleTools,
+		MachineConfig: machineConfig,
+	})
 	c.Assert(err, gc.IsNil)
 	return kvm
 }

=== modified file 'worker/provisioner/lxc-broker.go'
--- worker/provisioner/lxc-broker.go	2014-03-07 03:00:24 +0000
+++ worker/provisioner/lxc-broker.go	2014-03-13 05:11:53 +0000
@@ -7,11 +7,9 @@
 	"github.com/juju/loggo"
 
 	"launchpad.net/juju-core/agent"
-	"launchpad.net/juju-core/constraints"
 	"launchpad.net/juju-core/container"
 	"launchpad.net/juju-core/container/lxc"
 	"launchpad.net/juju-core/environs"
-	"launchpad.net/juju-core/environs/cloudinit"
 	"launchpad.net/juju-core/instance"
 	"launchpad.net/juju-core/state/api/params"
 	"launchpad.net/juju-core/tools"
@@ -51,10 +49,9 @@
 }
 
 // StartInstance is specified in the Broker interface.
-func (broker *lxcBroker) StartInstance(cons constraints.Value, possibleTools tools.List,
-	machineConfig *cloudinit.MachineConfig) (instance.Instance, *instance.HardwareCharacteristics, error) {
+func (broker *lxcBroker) StartInstance(args environs.StartInstanceParams) (instance.Instance, *instance.HardwareCharacteristics, error) {
 	// TODO: refactor common code out of the container brokers.
-	machineId := machineConfig.MachineId
+	machineId := args.MachineConfig.MachineId
 	lxcLogger.Infof("starting lxc container for machineId: %s", machineId)
 
 	// Default to using the host network until we can configure.
@@ -64,9 +61,9 @@
 	}
 	network := container.BridgeNetworkConfig(bridgeDevice)
 
-	series := possibleTools.OneSeries()
-	machineConfig.MachineContainerType = instance.LXC
-	machineConfig.Tools = possibleTools[0]
+	series := args.Tools.OneSeries()
+	args.MachineConfig.MachineContainerType = instance.LXC
+	args.MachineConfig.Tools = args.Tools[0]
 
 	config, err := broker.api.ContainerConfig()
 	if err != nil {
@@ -74,7 +71,7 @@
 		return nil, nil, err
 	}
 	if err := environs.PopulateMachineConfig(
-		machineConfig,
+		args.MachineConfig,
 		config.ProviderType,
 		config.AuthorizedKeys,
 		config.SSLHostnameVerification,
@@ -85,7 +82,7 @@
 		return nil, nil, err
 	}
 
-	inst, hardware, err := broker.manager.StartContainer(machineConfig, series, network)
+	inst, hardware, err := broker.manager.StartContainer(args.MachineConfig, series, network)
 	if err != nil {
 		lxcLogger.Errorf("failed to start container: %v", err)
 		return nil, nil, err

=== modified file 'worker/provisioner/lxc-broker_test.go'
--- worker/provisioner/lxc-broker_test.go	2014-03-12 03:05:33 +0000
+++ worker/provisioner/lxc-broker_test.go	2014-03-13 05:11:53 +0000
@@ -88,7 +88,11 @@
 	machineConfig := environs.NewMachineConfig(machineId, machineNonce, stateInfo, apiInfo)
 	cons := constraints.Value{}
 	possibleTools := s.broker.(coretools.HasTools).Tools()
-	lxc, _, err := s.broker.StartInstance(cons, possibleTools, machineConfig)
+	lxc, _, err := s.broker.StartInstance(environs.StartInstanceParams{
+		Constraints:   cons,
+		Tools:         possibleTools,
+		MachineConfig: machineConfig,
+	})
 	c.Assert(err, gc.IsNil)
 	return lxc
 }

=== modified file 'worker/provisioner/provisioner_task.go'
--- worker/provisioner/provisioner_task.go	2014-01-10 21:01:03 +0000
+++ worker/provisioner/provisioner_task.go	2014-03-13 05:11:53 +0000
@@ -395,7 +395,11 @@
 	if err != nil {
 		return err
 	}
-	inst, metadata, err := task.broker.StartInstance(cons, possibleTools, machineConfig)
+	inst, metadata, err := task.broker.StartInstance(environs.StartInstanceParams{
+		Constraints:   cons,
+		Tools:         possibleTools,
+		MachineConfig: machineConfig,
+	})
 	if err != nil {
 		// Set the state to error, so the machine will be skipped next
 		// time until the error is resolved, but don't return an