Allow the local provider to use kvm.

Allow the local provider to use kvm.

Add a 'container' config value for the local provider.
The allows a local provider to use kvm instead of lxc
if the host machine is kvm capable.

TODO: prereqs needs to check for other needed packages.

https://code.launchpad.net/~thumper/juju-core/kvm-local-provider/+merge/196465

Requires: https://code.launchpad.net/~thumper/juju-core/container-factory/+merge/196073

(do not edit description out of merge proposal)

[thumper, 2013-11-25 04:35:22]

Please take a look.

[thumper, 2013-12-02 04:22:52]

Please take a look.

[wallyworld, 2013-12-02 07:10:57]

LGTM with primarily the consolidation of the code to check kvm container
dependencies with the code to handle kvm initialisation. I also would like the
command code in apt moved. I'm not sure about the need for a ContainerConfig
interface but anytime we need to rely on UnknownAttrs() seems a bit hacky.

https://codereview.appspot.com/31870043/diff/20001/container/kvm/libvirt.go
File container/kvm/libvirt.go (right):

https://codereview.appspot.com/31870043/diff/20001/container/kvm/libvirt.go#n...
container/kvm/libvirt.go:36: output, err = utils.RunCommand(command, args...)
\o/

https://codereview.appspot.com/31870043/diff/20001/provider/local/config.go
File provider/local/config.go (right):

https://codereview.appspot.com/31870043/diff/20001/provider/local/config.go#n...
provider/local/config.go:23: // to explicitly get from the config unknown
params.
My brain found it hard to parse the 2nd line above

https://codereview.appspot.com/31870043/diff/20001/provider/local/environprov...
File provider/local/environprovider.go (right):

https://codereview.appspot.com/31870043/diff/20001/provider/local/environprov...
provider/local/environprovider.go:49: containerType, ok :=
cfg.UnknownAttrs()[containerConfigKey].(string)
Part of me really doesn't like this UnknownAttrs() business and would love to
see a proper cfg.ContainerType() API available. Not sure if we should introduce
a ContainerConfig interface and cast to that?

https://codereview.appspot.com/31870043/diff/20001/provider/local/environprov...
provider/local/environprovider.go:148: if localConfig.container() != "lxc" &&
localConfig.container() != "kvm" {
Can we use the ContainerType consts here instead of "lxc", "kvm"

https://codereview.appspot.com/31870043/diff/20001/provider/local/prereqs.go
File provider/local/prereqs.go (right):

https://codereview.appspot.com/31870043/diff/20001/provider/local/prereqs.go#...
provider/local/prereqs.go:50: const kvmNeedsUbuntu = `Sorry, but KVM support
with the local provider is only supported
s/but//

https://codereview.appspot.com/31870043/diff/20001/provider/local/prereqs.go#...
provider/local/prereqs.go:154: packagesNeeded := []string{"libvirt-bin",
"uvtool-libvirt", "kvm"}
There's also code in the container.kvm package to deal with installing the
required packages - see initialisation.go

I'd prefer this verifyKvm() function to be moved there so it's all done in one
place.

https://codereview.appspot.com/31870043/diff/20001/utils/apt.go
File utils/apt.go (right):

https://codereview.appspot.com/31870043/diff/20001/utils/apt.go#newcode87
utils/apt.go:87: func RunCommand(command string, args ...string) (output string,
err error) {
I'd like to see this in a file called command.go since it's not really anything
to do with apt per se. And tests in command_test.go

[thumper, 2013-12-02 21:29:24]

Please take a look.

https://codereview.appspot.com/31870043/diff/20001/provider/local/config.go
File provider/local/config.go (right):

https://codereview.appspot.com/31870043/diff/20001/provider/local/config.go#n...
provider/local/config.go:23: // to explicitly get from the config unknown
params.
On 2013/12/02 07:10:58, wallyworld wrote:
> My brain found it hard to parse the 2nd line above

tweaked slightly

https://codereview.appspot.com/31870043/diff/20001/provider/local/environprov...
File provider/local/environprovider.go (right):

https://codereview.appspot.com/31870043/diff/20001/provider/local/environprov...
provider/local/environprovider.go:49: containerType, ok :=
cfg.UnknownAttrs()[containerConfigKey].(string)
On 2013/12/02 07:10:58, wallyworld wrote:
> Part of me really doesn't like this UnknownAttrs() business and would love to
> see a proper cfg.ContainerType() API available. Not sure if we should
introduce
> a ContainerConfig interface and cast to that?

Refactored for better reading and understanding and taking the very small
overhead of creating the local config twice.

https://codereview.appspot.com/31870043/diff/20001/provider/local/environprov...
provider/local/environprovider.go:148: if localConfig.container() != "lxc" &&
localConfig.container() != "kvm" {
On 2013/12/02 07:10:58, wallyworld wrote:
> Can we use the ContainerType consts here instead of "lxc", "kvm"

Yes.

https://codereview.appspot.com/31870043/diff/20001/provider/local/prereqs.go
File provider/local/prereqs.go (right):

https://codereview.appspot.com/31870043/diff/20001/provider/local/prereqs.go#...
provider/local/prereqs.go:50: const kvmNeedsUbuntu = `Sorry, but KVM support
with the local provider is only supported
On 2013/12/02 07:10:58, wallyworld wrote:
> s/but//

Done.

https://codereview.appspot.com/31870043/diff/20001/provider/local/prereqs.go#...
provider/local/prereqs.go:154: packagesNeeded := []string{"libvirt-bin",
"uvtool-libvirt", "kvm"}
On 2013/12/02 07:10:58, wallyworld wrote:
> There's also code in the container.kvm package to deal with installing the
> required packages - see initialisation.go
> 
> I'd prefer this verifyKvm() function to be moved there so it's all done in one
> place.

Done.

https://codereview.appspot.com/31870043/diff/20001/utils/apt.go
File utils/apt.go (right):

https://codereview.appspot.com/31870043/diff/20001/utils/apt.go#newcode87
utils/apt.go:87: func RunCommand(command string, args ...string) (output string,
err error) {
On 2013/12/02 07:10:58, wallyworld wrote:
> I'd like to see this in a file called command.go since it's not really
anything
> to do with apt per se. And tests in command_test.go

Done.