More control around lxc networking.

More control around lxc networking.

This branch adds a NetworkConfig inside the lxc package
that is used to configure the network of the container.

This is used by the lxc-broker, and it uses an environment
variable to determine the link for the network.

The MAAS provider uses this environment variable to have
the container use the bridge "br0" instead of the default
"lxcbr0", as this allows the DHCP request from the container
to be serviced by the MAAS DHCP server rather than the
lxc bridge.  This gives the containers inside MAAS a visible
IP address from outside the host.

https://code.launchpad.net/~thumper/juju-core/lxc-network-config/+merge/177735

Requires: https://code.launchpad.net/~thumper/juju-core/machine-agent-environment/+merge/177707

(do not edit description out of merge proposal)

[thumper, 2013-07-31 03:25:14]

Please take a look.

[wallyworld, 2013-07-31 03:45:51]

LGTM but I'd really like to see tests for the new functionality. There's not
much coverage there.

https://codereview.appspot.com/12005049/diff/1/container/lxc/lxc.go
File container/lxc/lxc.go (right):

https://codereview.appspot.com/12005049/diff/1/container/lxc/lxc.go#newcode48
container/lxc/lxc.go:48: networkType string
not sure whether we want to alias the type of these attributes so they are not
straight strings? it seems to be common practice to do so

https://codereview.appspot.com/12005049/diff/1/container/lxc/lxc_test.go
File container/lxc/lxc_test.go (right):

https://codereview.appspot.com/12005049/diff/1/container/lxc/lxc_test.go#newc...
container/lxc/lxc_test.go:100: 
I think we should also check that the network config file matches the network
config passed to manger.StartContainer()

https://codereview.appspot.com/12005049/diff/1/environs/local/environ.go
File environs/local/environ.go (right):

https://codereview.appspot.com/12005049/diff/1/environs/local/environ.go#newc...
environs/local/environ.go:286: inst, err := env.containerManager.StartContainer(
There should be a test that local provider lxc network config file is as
expected

https://codereview.appspot.com/12005049/diff/1/environs/maas/environ.go
File environs/maas/environ.go (right):

https://codereview.appspot.com/12005049/diff/1/environs/maas/environ.go#newco...
environs/maas/environ.go:312:
machineConfig.MachineEnvironment["JUJU_LXC_BRIDGE"] = "br0"
tests?

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

https://codereview.appspot.com/12005049/diff/1/worker/provisioner/lxc-broker....
worker/provisioner/lxc-broker.go:47: 
tests?

[thumper, 2013-07-31 05:48:12]

Please take a look.

https://codereview.appspot.com/12005049/diff/1/container/lxc/lxc.go
File container/lxc/lxc.go (right):

https://codereview.appspot.com/12005049/diff/1/container/lxc/lxc.go#newcode48
container/lxc/lxc.go:48: networkType string
On 2013/07/31 03:45:51, wallyworld wrote:
> not sure whether we want to alias the type of these attributes so they are not
> straight strings? it seems to be common practice to do so

I decided to just keep them private instead.  No one can mess with them.

The only way to construct the NetworkConfig struct is to use one of the methods
below.

https://codereview.appspot.com/12005049/diff/1/container/lxc/lxc_test.go
File container/lxc/lxc_test.go (right):

https://codereview.appspot.com/12005049/diff/1/container/lxc/lxc_test.go#newc...
container/lxc/lxc_test.go:100: 
On 2013/07/31 03:45:51, wallyworld wrote:
> I think we should also check that the network config file matches the network
> config passed to manger.StartContainer()

Mildly overkill, but done.

https://codereview.appspot.com/12005049/diff/1/environs/local/environ.go
File environs/local/environ.go (right):

https://codereview.appspot.com/12005049/diff/1/environs/local/environ.go#newc...
environs/local/environ.go:286: inst, err := env.containerManager.StartContainer(
On 2013/07/31 03:45:51, wallyworld wrote:
> There should be a test that local provider lxc network config file is as
> expected

This is covered by the lxc tests.

https://codereview.appspot.com/12005049/diff/1/environs/maas/environ.go
File environs/maas/environ.go (right):

https://codereview.appspot.com/12005049/diff/1/environs/maas/environ.go#newco...
environs/maas/environ.go:312:
machineConfig.MachineEnvironment["JUJU_LXC_BRIDGE"] = "br0"
On 2013/07/31 03:45:51, wallyworld wrote:
> tests?

There are already tests that cover the writing out of cloudinit scripts.

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

https://codereview.appspot.com/12005049/diff/1/worker/provisioner/lxc-broker....
worker/provisioner/lxc-broker.go:47: 
On 2013/07/31 03:45:51, wallyworld wrote:
> tests?

adding