Merge lp:~thumper/juju-core/maas-lxc into lp:juju-core/1.16

[Tim Penhey]

Fix LXC containers on MAAS

We can't get a real CI framework soon enough.

When the provisioner was updated to use the API, this
accidentally broke LXC on MAAS, as the secret attributes
are stripped from the environment config for non-manager
nodes. This makes a completely invalid MAAS config, which
means the LCX provisioner task never starts.

There was no easy fix for this, so the correct one had
to do. That is to provide an API call to get what the
containers actually need, and not have them depend on
things they don't need.

I took this opportunity to also simplify the parameters
for the StartContainer call.

https://codereview.appspot.com/20220043/

Fix LXC containers on MAAS

We can't get a real CI framework soon enough.

When the provisioner was updated to use the API, this
accidentally broke LXC on MAAS, as the secret attributes
are stripped from the environment config for non-manager
nodes. This makes a completely invalid MAAS config, which
means the LCX provisioner task never starts.

There was no easy fix for this, so the correct one had
to do. That is to provide an API call to get what the
containers actually need, and not have them depend on
things they don't need.

I took this opportunity to also simplify the parameters
for the StartContainer call.

https://codereview.appspot.com/20220043/

[Tim Penhey (thumper)]

Reviewers: mp+193405_code.launchpad.net,

Message:
Please take a look.

Description:
Fix LXC containers on MAAS

We can't get a real CI framework soon enough.

When the provisioner was updated to use the API, this
accidentally broke LXC on MAAS, as the secret attributes
are stripped from the environment config for non-manager
nodes. This makes a completely invalid MAAS config, which
means the LCX provisioner task never starts.

There was no easy fix for this, so the correct one had
to do. That is to provide an API call to get what the
containers actually need, and not have them depend on
things they don't need.

I took this opportunity to also simplify the parameters
for the StartContainer call.

https://code.launchpad.net/~thumper/juju-core/maas-lxc/+merge/193405

(do not edit description out of merge proposal)

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

Affected files (+143, -107 lines):
   A [revision details]
   M container/lxc/lxc.go
   M container/lxc/lxc_test.go
   M environs/cloudinit.go
   M provider/local/environ.go
   M state/api/params/params.go
   M state/api/provisioner/provisioner.go
   M state/api/provisioner/provisioner_test.go
   M state/apiserver/provisioner/provisioner.go
   M state/apiserver/provisioner/provisioner_test.go
   M worker/provisioner/lxc-broker.go
   M worker/provisioner/lxc-broker_test.go
   M worker/provisioner/provisioner.go

[Dimiter Naydenov (dimitern)]

Very nice! LGTM

https://codereview.appspot.com/20220043/diff/1/state/apiserver/provisioner/provisioner.go
File state/apiserver/provisioner/provisioner.go (right):

https://codereview.appspot.com/20220043/diff/1/state/apiserver/provisioner/provisioner.go#newcode185
state/apiserver/provisioner/provisioner.go:185: // ContainerConfig
returns the current environment's configuration.
Fix the doc string please.

https://codereview.appspot.com/20220043/

[Tim Penhey (thumper)]

And thanks.

https://codereview.appspot.com/20220043/diff/1/state/apiserver/provisioner/provisioner.go
File state/apiserver/provisioner/provisioner.go (right):

https://codereview.appspot.com/20220043/diff/1/state/apiserver/provisioner/provisioner.go#newcode185
state/apiserver/provisioner/provisioner.go:185: // ContainerConfig
returns the current environment's configuration.
On 2013/10/31 11:50:30, dimitern wrote:
> Fix the doc string please.

Done.

https://codereview.appspot.com/20220043/

[John A Meinel (jameinel)]

LGTM

https://codereview.appspot.com/20220043/diff/1/state/api/provisioner/provisioner.go
File state/api/provisioner/provisioner.go (right):

https://codereview.appspot.com/20220043/diff/1/state/api/provisioner/provisioner.go#newcode161
state/api/provisioner/provisioner.go:161: var result
params.ContainerConfig
I would actually think that ContainerConfig would just return the params
struct, rather than splitting it back out into individual args.
That way, if we do decide to add things, we don't have to change the
Client API

https://codereview.appspot.com/20220043/diff/1/worker/provisioner/lxc-broker.go
File worker/provisioner/lxc-broker.go (right):

https://codereview.appspot.com/20220043/diff/1/worker/provisioner/lxc-broker.go#newcode65
worker/provisioner/lxc-broker.go:65: providerType, authorizedKeys,
sslVerification, err := broker.api.ContainerConfig()
Just a question, would it make sense to move this API call up a level
instead of here? Most of our API calls occur in the other layer where
the watchers are going on.

Nothing you have to change, just something I thought of.

https://codereview.appspot.com/20220043/

=== modified file 'container/lxc/lxc.go'
--- container/lxc/lxc.go	2013-10-01 02:33:14 +0000
+++ container/lxc/lxc.go	2013-10-31 11:53:08 +0000
@@ -14,15 +14,9 @@
 	"launchpad.net/golxc"
 	"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/names"
-	"launchpad.net/juju-core/state"
-	"launchpad.net/juju-core/state/api"
-	"launchpad.net/juju-core/tools"
 	"launchpad.net/juju-core/utils"
 )
 
@@ -83,12 +77,9 @@
 type ContainerManager interface {
 	// StartContainer creates and starts a new lxc container for the specified machine.
 	StartContainer(
-		machineId, series, nonce string,
-		network *NetworkConfig,
-		tools *tools.Tools,
-		environConfig *config.Config,
-		stateInfo *state.Info,
-		apiInfo *api.Info) (instance.Instance, error)
+		machineConfig *cloudinit.MachineConfig,
+		series string,
+		network *NetworkConfig) (instance.Instance, error)
 	// StopContainer stops and destroyes the lxc container identified by Instance.
 	StopContainer(instance.Instance) error
 	// ListContainers return a list of containers that have been started by
@@ -113,14 +104,11 @@
 }
 
 func (manager *containerManager) StartContainer(
-	machineId, series, nonce string,
-	network *NetworkConfig,
-	tools *tools.Tools,
-	environConfig *config.Config,
-	stateInfo *state.Info,
-	apiInfo *api.Info) (instance.Instance, error) {
+	machineConfig *cloudinit.MachineConfig,
+	series string,
+	network *NetworkConfig) (instance.Instance, error) {
 
-	name := names.MachineTag(machineId)
+	name := names.MachineTag(machineConfig.MachineId)
 	if manager.name != "" {
 		name = fmt.Sprintf("%s-%s", manager.name, name)
 	}
@@ -137,7 +125,7 @@
 		return nil, err
 	}
 	logger.Tracef("write cloud-init")
-	userDataFilename, err := writeUserData(directory, machineId, nonce, tools, environConfig, stateInfo, apiInfo)
+	userDataFilename, err := writeUserData(machineConfig, directory)
 	if err != nil {
 		logger.Errorf("failed to write user data: %v", err)
 		return nil, err
@@ -300,14 +288,8 @@
 	return configFilename, nil
 }
 
-func writeUserData(
-	directory, machineId, nonce string,
-	tools *tools.Tools,
-	environConfig *config.Config,
-	stateInfo *state.Info,
-	apiInfo *api.Info,
-) (string, error) {
-	userData, err := cloudInitUserData(machineId, nonce, tools, environConfig, stateInfo, apiInfo)
+func writeUserData(machineConfig *cloudinit.MachineConfig, directory string) (string, error) {
+	userData, err := cloudInitUserData(machineConfig)
 	if err != nil {
 		logger.Errorf("failed to create user data: %v", err)
 		return "", err
@@ -320,25 +302,8 @@
 	return userDataFilename, nil
 }
 
-func cloudInitUserData(
-	machineId, nonce string,
-	tools *tools.Tools,
-	environConfig *config.Config,
-	stateInfo *state.Info,
-	apiInfo *api.Info,
-) ([]byte, error) {
-	machineConfig := &cloudinit.MachineConfig{
-		MachineId:            machineId,
-		MachineNonce:         nonce,
-		MachineContainerType: instance.LXC,
-		StateInfo:            stateInfo,
-		APIInfo:              apiInfo,
-		DataDir:              "/var/lib/juju",
-		Tools:                tools,
-	}
-	if err := environs.FinishMachineConfig(machineConfig, environConfig, constraints.Value{}); err != nil {
-		return nil, err
-	}
+func cloudInitUserData(machineConfig *cloudinit.MachineConfig) ([]byte, error) {
+	machineConfig.DataDir = "/var/lib/juju"
 	cloudConfig, err := cloudinit.New(machineConfig)
 	if err != nil {
 		return nil, err

=== modified file 'container/lxc/lxc_test.go'
--- container/lxc/lxc_test.go	2013-10-01 21:21:01 +0000
+++ container/lxc/lxc_test.go	2013-10-31 11:53:08 +0000
@@ -16,10 +16,10 @@
 	"launchpad.net/loggo"
 
 	"launchpad.net/juju-core/container/lxc"
+	"launchpad.net/juju-core/environs"
 	"launchpad.net/juju-core/instance"
 	instancetest "launchpad.net/juju-core/instance/testing"
 	jujutesting "launchpad.net/juju-core/juju/testing"
-	"launchpad.net/juju-core/testing"
 	jc "launchpad.net/juju-core/testing/checkers"
 	"launchpad.net/juju-core/testing/testbase"
 	"launchpad.net/juju-core/tools"
@@ -65,19 +65,17 @@
 )
 
 func StartContainer(c *gc.C, manager lxc.ContainerManager, machineId string) instance.Instance {
-	config := testing.EnvironConfig(c)
 	stateInfo := jujutesting.FakeStateInfo(machineId)
 	apiInfo := jujutesting.FakeAPIInfo(machineId)
-	network := lxc.BridgeNetworkConfig("nic42")
-
-	series := "series"
-	nonce := "fake-nonce"
-	tools := &tools.Tools{
+	machineConfig := environs.NewMachineConfig(machineId, "fake-nonce", stateInfo, apiInfo)
+	machineConfig.Tools = &tools.Tools{
 		Version: version.MustParseBinary("2.3.4-foo-bar"),
 		URL:     "http://tools.testing.invalid/2.3.4-foo-bar.tgz",
 	}
 
-	inst, err := manager.StartContainer(machineId, series, nonce, network, tools, config, stateInfo, apiInfo)
+	series := "series"
+	network := lxc.BridgeNetworkConfig("nic42")
+	inst, err := manager.StartContainer(machineConfig, series, network)
 	c.Assert(err, gc.IsNil)
 	return inst
 }

=== modified file 'environs/cloudinit.go'
--- environs/cloudinit.go	2013-10-02 19:30:07 +0000
+++ environs/cloudinit.go	2013-10-31 11:53:08 +0000
@@ -51,6 +51,23 @@
 	return mcfg
 }
 
+func PopulateMachineConfig(mcfg *cloudinit.MachineConfig,
+	providerType, authorizedKeys string,
+	sslHostnameVerification bool) error {
+
+	if authorizedKeys == "" {
+		return fmt.Errorf("environment configuration has no authorized-keys")
+	}
+	mcfg.AuthorizedKeys = authorizedKeys
+	if mcfg.AgentEnvironment == nil {
+		mcfg.AgentEnvironment = make(map[string]string)
+	}
+	mcfg.AgentEnvironment[agent.ProviderType] = providerType
+	mcfg.AgentEnvironment[agent.ContainerType] = string(mcfg.MachineContainerType)
+	mcfg.DisableSSLHostnameVerification = !sslHostnameVerification
+	return nil
+}
+
 // FinishMachineConfig sets fields on a MachineConfig that can be determined by
 // inspecting a plain config.Config and the machine constraints at the last
 // moment before bootstrapping. It assumes that the supplied Config comes from
@@ -64,18 +81,9 @@
 func FinishMachineConfig(mcfg *cloudinit.MachineConfig, cfg *config.Config, cons constraints.Value) (err error) {
 	defer utils.ErrorContextf(&err, "cannot complete machine configuration")
 
-	// Everything needs the environment's authorized keys.
-	authKeys := cfg.AuthorizedKeys()
-	if authKeys == "" {
-		return fmt.Errorf("environment configuration has no authorized-keys")
-	}
-	mcfg.AuthorizedKeys = authKeys
-	if mcfg.AgentEnvironment == nil {
-		mcfg.AgentEnvironment = make(map[string]string)
-	}
-	mcfg.AgentEnvironment[agent.ProviderType] = cfg.Type()
-	mcfg.AgentEnvironment[agent.ContainerType] = string(mcfg.MachineContainerType)
-	mcfg.DisableSSLHostnameVerification = !cfg.SSLHostnameVerification()
+	if err := PopulateMachineConfig(mcfg, cfg.Type(), cfg.AuthorizedKeys(), cfg.SSLHostnameVerification()); err != nil {
+		return err
+	}
 
 	// The following settings are only appropriate at bootstrap time. At the
 	// moment, the only state server is the bootstrap node, but this

=== modified file 'provider/local/environ.go'
--- provider/local/environ.go	2013-10-16 21:28:54 +0000
+++ provider/local/environ.go	2013-10-31 11:53:08 +0000
@@ -265,17 +265,15 @@
 func (env *localEnviron) StartInstance(cons constraints.Value, possibleTools tools.List,
 	machineConfig *cloudinit.MachineConfig) (instance.Instance, *instance.HardwareCharacteristics, error) {
 
-	machineId := machineConfig.MachineId
 	series := possibleTools.OneSeries()
-	logger.Debugf("StartInstance: %q, %s", machineId, series)
-	agenttools := possibleTools[0]
-	logger.Debugf("tools: %#v", agenttools)
-
+	logger.Debugf("StartInstance: %q, %s", machineConfig.MachineId, series)
+	machineConfig.Tools = possibleTools[0]
+	logger.Debugf("tools: %#v", machineConfig.Tools)
 	network := lxc.BridgeNetworkConfig(env.config.networkBridge())
-	inst, err := env.containerManager.StartContainer(
-		machineId, series, machineConfig.MachineNonce, network,
-		agenttools, env.config.Config,
-		machineConfig.StateInfo, machineConfig.APIInfo)
+	if err := environs.FinishMachineConfig(machineConfig, env.config.Config, cons); err != nil {
+		return nil, nil, err
+	}
+	inst, err := env.containerManager.StartContainer(machineConfig, series, network)
 	if err != nil {
 		return nil, nil, err
 	}

=== modified file 'state/api/params/params.go'
--- state/api/params/params.go	2013-09-18 15:57:30 +0000
+++ state/api/params/params.go	2013-10-31 11:53:08 +0000
@@ -405,3 +405,11 @@
 		Id:   i.Tag,
 	}
 }
+
+// ContainerConfig contains information from the environment config that are
+// needed for container cloud-init.
+type ContainerConfig struct {
+	ProviderType            string
+	AuthorizedKeys          string
+	SSLHostnameVerification bool
+}

=== modified file 'state/api/provisioner/provisioner.go'
--- state/api/provisioner/provisioner.go	2013-10-02 14:26:55 +0000
+++ state/api/provisioner/provisioner.go	2013-10-31 11:53:08 +0000
@@ -154,3 +154,17 @@
 	}
 	return result.Tools, nil
 }
+
+// ContainerConfig returns information from the environment config that are
+// needed for container cloud-init.
+func (st *State) ContainerConfig() (providerType, authorizedKeys string, sslVerification bool, err error) {
+	var result params.ContainerConfig
+	err = st.caller.Call("Provisioner", "", "ContainerConfig", nil, &result)
+	if err != nil {
+		return "", "", false, err
+	}
+	providerType = result.ProviderType
+	authorizedKeys = result.AuthorizedKeys
+	sslVerification = result.SSLHostnameVerification
+	return providerType, authorizedKeys, sslVerification, nil
+}

=== modified file 'state/api/provisioner/provisioner_test.go'
--- state/api/provisioner/provisioner_test.go	2013-10-02 13:14:49 +0000
+++ state/api/provisioner/provisioner_test.go	2013-10-31 11:53:08 +0000
@@ -357,6 +357,14 @@
 	c.Assert(addresses, gc.DeepEquals, apiInfo.Addrs)
 }
 
+func (s *provisionerSuite) TestContainerConfig(c *gc.C) {
+	providerType, authKeys, sslVerify, err := s.provisioner.ContainerConfig()
+	c.Assert(err, gc.IsNil)
+	c.Assert(providerType, gc.Equals, "dummy")
+	c.Assert(authKeys, gc.Equals, "my-keys")
+	c.Assert(sslVerify, jc.IsTrue)
+}
+
 func (s *provisionerSuite) TestCACert(c *gc.C) {
 	caCert, err := s.provisioner.CACert()
 	c.Assert(err, gc.IsNil)

=== modified file 'state/apiserver/provisioner/provisioner.go'
--- state/apiserver/provisioner/provisioner.go	2013-10-02 10:03:25 +0000
+++ state/apiserver/provisioner/provisioner.go	2013-10-31 11:53:08 +0000
@@ -182,6 +182,20 @@
 	return result, nil
 }
 
+// ContainerConfig returns information from the environment config that are
+// needed for container cloud-init.
+func (p *ProvisionerAPI) ContainerConfig() (params.ContainerConfig, error) {
+	result := params.ContainerConfig{}
+	config, err := p.st.EnvironConfig()
+	if err != nil {
+		return result, err
+	}
+	result.ProviderType = config.Type()
+	result.AuthorizedKeys = config.AuthorizedKeys()
+	result.SSLHostnameVerification = config.SSLHostnameVerification()
+	return result, nil
+}
+
 // Status returns the status of each given machine entity.
 func (p *ProvisionerAPI) Status(args params.Entities) (params.StatusResults, error) {
 	result := params.StatusResults{

=== modified file 'state/apiserver/provisioner/provisioner_test.go'
--- state/apiserver/provisioner/provisioner_test.go	2013-10-02 13:14:49 +0000
+++ state/apiserver/provisioner/provisioner_test.go	2013-10-31 11:53:08 +0000
@@ -675,6 +675,14 @@
 	c.Check(results.Results, gc.HasLen, 0)
 }
 
+func (s *provisionerSuite) TestContainerConfig(c *gc.C) {
+	results, err := s.provisioner.ContainerConfig()
+	c.Check(err, gc.IsNil)
+	c.Check(results.ProviderType, gc.Equals, "dummy")
+	c.Check(results.AuthorizedKeys, gc.Equals, "my-keys")
+	c.Check(results.SSLHostnameVerification, jc.IsTrue)
+}
+
 func (s *provisionerSuite) TestToolsRefusesWrongAgent(c *gc.C) {
 	anAuthorizer := s.authorizer
 	anAuthorizer.Tag = "machine-12354"

=== modified file 'worker/provisioner/lxc-broker.go'
--- worker/provisioner/lxc-broker.go	2013-08-30 04:50:15 +0000
+++ worker/provisioner/lxc-broker.go	2013-10-31 11:53:08 +0000
@@ -11,7 +11,6 @@
 	"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/tools"
 )
@@ -21,10 +20,14 @@
 var _ environs.InstanceBroker = (*lxcBroker)(nil)
 var _ tools.HasTools = (*lxcBroker)(nil)
 
-func NewLxcBroker(config *config.Config, tools *tools.Tools, agentConfig agent.Config) environs.InstanceBroker {
+type APICalls interface {
+	ContainerConfig() (providerType, authorizedKeys string, sslVerification bool, err error)
+}
+
+func NewLxcBroker(api APICalls, tools *tools.Tools, agentConfig agent.Config) environs.InstanceBroker {
 	return &lxcBroker{
 		manager:     lxc.NewContainerManager(lxc.ManagerConfig{Name: "juju"}),
-		config:      config,
+		api:         api,
 		tools:       tools,
 		agentConfig: agentConfig,
 	}
@@ -32,7 +35,7 @@
 
 type lxcBroker struct {
 	manager     lxc.ContainerManager
-	config      *config.Config
+	api         APICalls
 	tools       *tools.Tools
 	agentConfig agent.Config
 }
@@ -56,9 +59,20 @@
 	network := lxc.BridgeNetworkConfig(bridgeDevice)
 
 	series := possibleTools.OneSeries()
-	inst, err := broker.manager.StartContainer(
-		machineId, series, machineConfig.MachineNonce, network, possibleTools[0], broker.config,
-		machineConfig.StateInfo, machineConfig.APIInfo)
+	machineConfig.MachineContainerType = instance.LXC
+	machineConfig.Tools = possibleTools[0]
+
+	providerType, authorizedKeys, sslVerification, err := broker.api.ContainerConfig()
+	if err != nil {
+		lxcLogger.Errorf("failed to get container config: %v", err)
+		return nil, nil, err
+	}
+	if err := environs.PopulateMachineConfig(machineConfig, providerType, authorizedKeys, sslVerification); err != nil {
+		lxcLogger.Errorf("failed to populate machine config: %v", err)
+		return nil, nil, err
+	}
+
+	inst, err := broker.manager.StartContainer(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	2013-10-02 13:14:49 +0000
+++ worker/provisioner/lxc-broker_test.go	2013-10-31 11:53:08 +0000
@@ -64,7 +64,6 @@
 		Version: version.MustParseBinary("2.3.4-foo-bar"),
 		URL:     "http://tools.testing.invalid/2.3.4-foo-bar.tgz",
 	}
-	config := coretesting.EnvironConfig(c)
 	var err error
 	s.agentConfig, err = agent.NewAgentConfig(
 		agent.AgentConfigParams{
@@ -76,7 +75,7 @@
 			CACert:       []byte(coretesting.CACert),
 		})
 	c.Assert(err, gc.IsNil)
-	s.broker = provisioner.NewLxcBroker(config, tools, s.agentConfig)
+	s.broker = provisioner.NewLxcBroker(&fakeAPI{}, tools, s.agentConfig)
 }
 
 func (s *lxcBrokerSuite) startInstance(c *gc.C, machineId string) instance.Instance {
@@ -273,3 +272,9 @@
 	s.expectStopped(c, instId)
 	s.waitRemoved(c, container)
 }
+
+type fakeAPI struct{}
+
+func (*fakeAPI) ContainerConfig() (providerType, authorizedKeys string, sslVerification bool, err error) {
+	return "fake", "my-keys", true, nil
+}

=== modified file 'worker/provisioner/provisioner.go'
--- worker/provisioner/provisioner.go	2013-10-02 10:03:25 +0000
+++ worker/provisioner/provisioner.go	2013-10-31 11:53:08 +0000
@@ -15,6 +15,7 @@
 	"launchpad.net/juju-core/environs/config"
 	"launchpad.net/juju-core/instance"
 	apiprovisioner "launchpad.net/juju-core/state/api/provisioner"
+	apiwatcher "launchpad.net/juju-core/state/api/watcher"
 	"launchpad.net/juju-core/state/watcher"
 	coretools "launchpad.net/juju-core/tools"
 	"launchpad.net/juju-core/worker"
@@ -75,26 +76,21 @@
 }
 
 func (p *Provisioner) loop() error {
-	environWatcher, err := p.st.WatchForEnvironConfigChanges()
-	if err != nil {
-		return err
-	}
-	environConfigChanges := environWatcher.Changes()
-	defer watcher.Stop(environWatcher, &p.tomb)
-
-	p.environ, err = worker.WaitForEnviron(environWatcher, p.st, p.tomb.Dying())
-	if err != nil {
-		return err
-	}
-
-	if p.pt != ENVIRON {
-		// Only the environment provisioner cares about
-		// changes to the environment configuration.
-		if err := environWatcher.Stop(); err != nil {
-			return err
-		}
-		// Don't wait for changes on the channel below.
-		environConfigChanges = nil
+	// Only wait for the environment if we are an environmental provisioner.
+	var environConfigChanges <-chan struct{}
+	var environWatcher apiwatcher.NotifyWatcher
+	if p.pt == ENVIRON {
+		environWatcher, err := p.st.WatchForEnvironConfigChanges()
+		if err != nil {
+			return err
+		}
+		environConfigChanges = environWatcher.Changes()
+		defer watcher.Stop(environWatcher, &p.tomb)
+
+		p.environ, err = worker.WaitForEnviron(environWatcher, p.st, p.tomb.Dying())
+		if err != nil {
+			return err
+		}
 	}
 
 	auth, err := NewAPIAuthenticator(p.st)
@@ -177,13 +173,12 @@
 	case ENVIRON:
 		return p.environ, nil
 	case LXC:
-		config := p.environ.Config()
 		tools, err := p.getAgentTools()
 		if err != nil {
 			logger.Errorf("cannot get tools from machine for lxc broker")
 			return nil, err
 		}
-		return NewLxcBroker(config, tools, p.agentConfig), nil
+		return NewLxcBroker(p.st, tools, p.agentConfig), nil
 	}
 	return nil, fmt.Errorf("unknown provisioner type")
 }