Merge lp:~thumper/juju-core/kvm-local-provider into lp:~go-bot/juju-core/trunk

[Tim Penhey]

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.

https://codereview.appspot.com/31870043/

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://codereview.appspot.com/31870043/

[Tim Penhey (thumper)]

Reviewers: mp+196465_code.launchpad.net,

Message:
Please take a look.

Description:
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)

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

Affected files (+91, -13 lines):
   A [revision details]
   M provider/local/config.go
   M provider/local/environ.go
   M provider/local/environprovider.go
   M provider/local/prereqs.go
   M provider/local/prereqs_test.go

[Tim Penhey (thumper)]

Please take a look.

https://codereview.appspot.com/31870043/

[Ian Booth (wallyworld)]

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#newcode36
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#newcode23
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/environprovider.go
File provider/local/environprovider.go (right):

https://codereview.appspot.com/31870043/diff/20001/provider/local/environprovider.go#newcode49
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/environprovider.go#newcode148
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#newcode50
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#newcode154
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

https://codereview.appspot.com/31870043/

[Jorge Castro (jorge)]

Thumper please remember to submit an update to the local provider docs as you finish this up!

[Tim Penhey (thumper)]

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#newcode23
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/environprovider.go
File provider/local/environprovider.go (right):

https://codereview.appspot.com/31870043/diff/20001/provider/local/environprovider.go#newcode49
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/environprovider.go#newcode148
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#newcode50
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#newcode154
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.

https://codereview.appspot.com/31870043/

=== modified file 'container/kvm/initialisation.go'
--- container/kvm/initialisation.go	2013-11-15 06:36:39 +0000
+++ container/kvm/initialisation.go	2013-12-02 21:28:17 +0000
@@ -4,6 +4,9 @@
 package kvm
 
 import (
+	"fmt"
+	"strings"
+
 	"launchpad.net/juju-core/container"
 	"launchpad.net/juju-core/utils"
 )
@@ -33,3 +36,46 @@
 func ensureDependencies() error {
 	return utils.AptGetInstall(requiredPackages...)
 }
+
+const kvmNeedsUbuntu = `Sorry, KVM support with the local provider is only supported
+on the Ubuntu OS.`
+
+const kvmNotSupported = `KVM is not currently supported with the current settings.
+You could try running 'kvm-ok' yourself as root to get the full rationale as to
+why it isn't supported, or potentially some BIOS settings to change to enable
+KVM support.`
+
+const neetToInstallKVMOk = `kvm-ok is not installed. Please install the cpu-checker package.
+    sudo apt-get install cpu-checker`
+
+const missingKVMDeps = `Some required packages are missing for KVM to work:
+
+    sudo apt-get install %s
+`
+
+// VerifyKVMEnabled makes sure that the host OS is Ubuntu, and that the required
+// packages are installed, and that the host CPU is able to support KVM.
+func VerifyKVMEnabled() error {
+	if !utils.IsUbuntu() {
+		return fmt.Errorf(kvmNeedsUbuntu)
+	}
+	supported, err := IsKVMSupported()
+	if err != nil {
+		// Missing the kvm-ok package.
+		return fmt.Errorf(neetToInstallKVMOk)
+	}
+	if !supported {
+		return fmt.Errorf(kvmNotSupported)
+	}
+	// Check for other packages needed.
+	toInstall := []string{}
+	for _, pkg := range requiredPackages {
+		if !utils.IsPackageInstalled(pkg) {
+			toInstall = append(toInstall, pkg)
+		}
+	}
+	if len(toInstall) > 0 {
+		return fmt.Errorf(missingKVMDeps, strings.Join(toInstall, " "))
+	}
+	return nil
+}

=== modified file 'container/kvm/libvirt.go'
--- container/kvm/libvirt.go	2013-11-22 03:34:42 +0000
+++ container/kvm/libvirt.go	2013-12-02 21:28:17 +0000
@@ -16,9 +16,10 @@
 
 import (
 	"fmt"
-	"os/exec"
 	"regexp"
 	"strings"
+
+	"launchpad.net/juju-core/utils"
 )
 
 var (
@@ -32,17 +33,9 @@
 // run the command and return the combined output.
 func run(command string, args ...string) (output string, err error) {
 	logger.Tracef("%s %v", command, args)
-	cmd := exec.Command(command, args...)
-	out, err := cmd.CombinedOutput()
-	output = string(out)
+	output, err = utils.RunCommand(command, args...)
 	logger.Tracef("output: %v", output)
-	if err != nil {
-		return output, err
-	}
-	if !cmd.ProcessState.Success() {
-		return output, fmt.Errorf("%s returned non-zero exit", command)
-	}
-	return output, nil
+	return output, err
 }
 
 // SyncImages updates the local cached images by reading the simplestreams

=== modified file 'provider/local/config.go'
--- provider/local/config.go	2013-10-03 03:03:09 +0000
+++ provider/local/config.go	2013-12-02 21:28:17 +0000
@@ -9,6 +9,7 @@
 	"path/filepath"
 
 	"launchpad.net/juju-core/environs/config"
+	"launchpad.net/juju-core/instance"
 	"launchpad.net/juju-core/schema"
 	"launchpad.net/juju-core/utils"
 )
@@ -22,6 +23,7 @@
 		"root-dir":            schema.String(),
 		"bootstrap-ip":        schema.String(),
 		"network-bridge":      schema.String(),
+		"container":           schema.String(),
 		"storage-port":        schema.ForceInt(),
 		"shared-storage-port": schema.ForceInt(),
 	}
@@ -32,6 +34,7 @@
 	configDefaults = schema.Defaults{
 		"root-dir":            "",
 		"network-bridge":      "lxcbr0",
+		"container":           string(instance.LXC),
 		"bootstrap-ip":        schema.Omit,
 		"storage-port":        8040,
 		"shared-storage-port": 8041,
@@ -64,7 +67,7 @@
 
 // Since it is technically possible for two different users on one machine to
 // have the same local provider name, we need to have a simple way to
-// namespace the file locations, but more importantly the lxc containers.
+// namespace the file locations, but more importantly the containers.
 func (c *environConfig) namespace() string {
 	return fmt.Sprintf("%s-%s", c.user, c.Name())
 }
@@ -73,6 +76,10 @@
 	return c.attrs["root-dir"].(string)
 }
 
+func (c *environConfig) container() instance.ContainerType {
+	return instance.ContainerType(c.attrs["container"].(string))
+}
+
 func (c *environConfig) networkBridge() string {
 	return c.attrs["network-bridge"].(string)
 }

=== modified file 'provider/local/environ.go'
--- provider/local/environ.go	2013-11-26 12:24:48 +0000
+++ provider/local/environ.go	2013-12-02 21:28:17 +0000
@@ -17,7 +17,7 @@
 	agenttools "launchpad.net/juju-core/agent/tools"
 	"launchpad.net/juju-core/constraints"
 	"launchpad.net/juju-core/container"
-	"launchpad.net/juju-core/container/lxc"
+	"launchpad.net/juju-core/container/factory"
 	"launchpad.net/juju-core/environs"
 	"launchpad.net/juju-core/environs/bootstrap"
 	"launchpad.net/juju-core/environs/cloudinit"
@@ -194,11 +194,15 @@
 	env.config = ecfg
 	env.name = ecfg.Name()
 
-	env.containerManager = lxc.NewContainerManager(
+	env.containerManager, err = factory.NewContainerManager(
+		ecfg.container(),
 		container.ManagerConfig{
 			Name:   env.config.namespace(),
 			LogDir: env.config.logDir(),
 		})
+	if err != nil {
+		return err
+	}
 
 	// Here is the end of normal config setting.
 	if ecfg.bootstrapped() {

=== modified file 'provider/local/environprovider.go'
--- provider/local/environprovider.go	2013-10-14 15:01:07 +0000
+++ provider/local/environprovider.go	2013-12-02 21:28:17 +0000
@@ -12,6 +12,7 @@
 
 	"launchpad.net/juju-core/environs"
 	"launchpad.net/juju-core/environs/config"
+	"launchpad.net/juju-core/instance"
 	"launchpad.net/juju-core/provider"
 	"launchpad.net/juju-core/utils"
 	"launchpad.net/juju-core/version"
@@ -41,7 +42,12 @@
 		}
 		cfg = newCfg
 	}
-	if err := VerifyPrerequisites(); err != nil {
+	// Do the initial validation on the config.
+	localConfig, err := providerInstance.newConfig(cfg)
+	if err != nil {
+		return nil, err
+	}
+	if err := VerifyPrerequisites(localConfig.container()); err != nil {
 		logger.Errorf("failed verification of local provider prerequisites: %v", err)
 		return nil, err
 	}
@@ -109,6 +115,11 @@
 		if err != nil {
 			return nil, fmt.Errorf("old config is not a valid local config: %v", old)
 		}
+		if localConfig.container() != oldLocalConfig.container() {
+			return nil, fmt.Errorf("cannot change container from %q to %q",
+				oldLocalConfig.container(),
+				localConfig.container())
+		}
 		if localConfig.rootDir() != oldLocalConfig.rootDir() {
 			return nil, fmt.Errorf("cannot change root-dir from %q to %q",
 				oldLocalConfig.rootDir(),
@@ -130,6 +141,10 @@
 				localConfig.sharedStoragePort())
 		}
 	}
+	// Currently only supported containers are "lxc" and "kvm".
+	if localConfig.container() != instance.LXC && localConfig.container() != instance.KVM {
+		return nil, fmt.Errorf("unsupported container type: %q", localConfig.container())
+	}
 	dir := utils.NormalizePath(localConfig.rootDir())
 	if dir == "." {
 		dir = config.JujuHomePath(cfg.Name())

=== modified file 'provider/local/prereqs.go'
--- provider/local/prereqs.go	2013-08-09 06:42:05 +0000
+++ provider/local/prereqs.go	2013-12-02 21:28:17 +0000
@@ -9,8 +9,10 @@
 	"os"
 	"os/exec"
 	"runtime"
-	"strings"
 
+	"launchpad.net/juju-core/container/kvm"
+	"launchpad.net/juju-core/instance"
+	"launchpad.net/juju-core/utils"
 	"launchpad.net/juju-core/version"
 )
 
@@ -61,14 +63,20 @@
 // VerifyPrerequisites verifies the prerequisites of
 // the local machine (machine 0) for running the local
 // provider.
-func VerifyPrerequisites() error {
+func VerifyPrerequisites(containerType instance.ContainerType) error {
 	if goos != "linux" {
 		return fmt.Errorf(errUnsupportedOS, goos)
 	}
 	if err := verifyMongod(); err != nil {
 		return err
 	}
-	return verifyLxc()
+	switch containerType {
+	case instance.LXC:
+		return verifyLxc()
+	case instance.KVM:
+		return kvm.VerifyKVMEnabled()
+	}
+	return fmt.Errorf("Unknown container type specified in the config.")
 }
 
 func verifyMongod() error {
@@ -91,16 +99,8 @@
 	return nil
 }
 
-func isUbuntu() bool {
-	out, err := exec.Command("lsb_release", "-i", "-s").CombinedOutput()
-	if err != nil {
-		return false
-	}
-	return strings.TrimSpace(string(out)) == "Ubuntu"
-}
-
 func wrapMongodNotExist(err error) error {
-	if isUbuntu() {
+	if utils.IsUbuntu() {
 		series := version.Current.Series
 		args := []interface{}{err, installMongodUbuntu}
 		format := "%v\n%s\n%s"
@@ -115,7 +115,7 @@
 }
 
 func wrapLxcNotFound(err error) error {
-	if isUbuntu() {
+	if utils.IsUbuntu() {
 		return fmt.Errorf("%v\n%s", err, installLxcUbuntu)
 	}
 	return fmt.Errorf("%v\n%s", err, installLxcGeneric)

=== modified file 'provider/local/prereqs_test.go'
--- provider/local/prereqs_test.go	2013-09-20 02:53:59 +0000
+++ provider/local/prereqs_test.go	2013-12-02 21:28:17 +0000
@@ -10,6 +10,7 @@
 
 	gc "launchpad.net/gocheck"
 
+	"launchpad.net/juju-core/instance"
 	"launchpad.net/juju-core/testing/testbase"
 )
 
@@ -59,17 +60,17 @@
 		goos = old
 	}(goos)
 	goos = "windows"
-	err := VerifyPrerequisites()
+	err := VerifyPrerequisites(instance.LXC)
 	c.Assert(err, gc.ErrorMatches, "Unsupported operating system: windows(.|\n)*")
 }
 
 func (s *prereqsSuite) TestMongoPrereq(c *gc.C) {
-	err := VerifyPrerequisites()
+	err := VerifyPrerequisites(instance.LXC)
 	c.Assert(err, gc.ErrorMatches, "(.|\n)*MongoDB server must be installed(.|\n)*")
 	c.Assert(err, gc.ErrorMatches, "(.|\n)*apt-get install mongodb-server(.|\n)*")
 
 	os.Setenv("JUJUTEST_LSB_RELEASE_ID", "NotUbuntu")
-	err = VerifyPrerequisites()
+	err = VerifyPrerequisites(instance.LXC)
 	c.Assert(err, gc.ErrorMatches, "(.|\n)*MongoDB server must be installed(.|\n)*")
 	c.Assert(err, gc.Not(gc.ErrorMatches), "(.|\n)*apt-get install(.|\n)*")
 
@@ -77,7 +78,7 @@
 	c.Assert(err, gc.IsNil)
 	err = ioutil.WriteFile(filepath.Join(s.tmpdir, "lxc-ls"), nil, 0777)
 	c.Assert(err, gc.IsNil)
-	err = VerifyPrerequisites()
+	err = VerifyPrerequisites(instance.LXC)
 	c.Assert(err, gc.IsNil)
 }
 
@@ -85,17 +86,17 @@
 	err := ioutil.WriteFile(mongodPath, nil, 0777)
 	c.Assert(err, gc.IsNil)
 
-	err = VerifyPrerequisites()
+	err = VerifyPrerequisites(instance.LXC)
 	c.Assert(err, gc.ErrorMatches, "(.|\n)*Linux Containers \\(LXC\\) userspace tools must be\ninstalled(.|\n)*")
 	c.Assert(err, gc.ErrorMatches, "(.|\n)*apt-get install lxc(.|\n)*")
 
 	os.Setenv("JUJUTEST_LSB_RELEASE_ID", "NotUbuntu")
-	err = VerifyPrerequisites()
+	err = VerifyPrerequisites(instance.LXC)
 	c.Assert(err, gc.ErrorMatches, "(.|\n)*Linux Containers \\(LXC\\) userspace tools must be installed(.|\n)*")
 	c.Assert(err, gc.Not(gc.ErrorMatches), "(.|\n)*apt-get install(.|\n)*")
 
 	err = ioutil.WriteFile(lxclsPath, nil, 0777)
 	c.Assert(err, gc.IsNil)
-	err = VerifyPrerequisites()
+	err = VerifyPrerequisites(instance.LXC)
 	c.Assert(err, gc.IsNil)
 }

=== modified file 'utils/apt.go'
--- utils/apt.go	2013-11-15 06:36:39 +0000
+++ utils/apt.go	2013-12-02 21:28:17 +0000
@@ -9,6 +9,7 @@
 	"os"
 	"os/exec"
 	"regexp"
+	"strings"
 
 	"launchpad.net/loggo"
 )
@@ -72,3 +73,19 @@
 	}
 	return string(bytes.Join(aptProxyRE.FindAll(out, -1), []byte("\n"))), nil
 }
+
+// IsUbuntu executes lxb_release to see if the host OS is Ubuntu.
+func IsUbuntu() bool {
+	out, err := RunCommand("lsb_release", "-i", "-s")
+	if err != nil {
+		return false
+	}
+	return strings.TrimSpace(out) == "Ubuntu"
+}
+
+// IsPackageInstalled uses dpkg-query to determine if the `packageName`
+// package is installed.
+func IsPackageInstalled(packageName string) bool {
+	_, err := RunCommand("dpkg-query", "--status", packageName)
+	return err == nil
+}

=== modified file 'utils/apt_test.go'
--- utils/apt_test.go	2013-11-15 06:36:39 +0000
+++ utils/apt_test.go	2013-12-02 21:28:17 +0000
@@ -1,14 +1,16 @@
 // Copyright 2012, 2013 Canonical Ltd.
 // Licensed under the AGPLv3, see LICENCE file for details.
 
-package utils
+package utils_test
 
 import (
 	"fmt"
 
 	gc "launchpad.net/gocheck"
 
+	jc "launchpad.net/juju-core/testing/checkers"
 	"launchpad.net/juju-core/testing/testbase"
+	"launchpad.net/juju-core/utils"
 )
 
 type AptSuite struct {
@@ -18,9 +20,9 @@
 var _ = gc.Suite(&AptSuite{})
 
 func (s *AptSuite) TestOnePackage(c *gc.C) {
-	cmdChan, cleanup := testbase.HookCommandOutput(&AptCommandOutput, []byte{}, nil)
+	cmdChan, cleanup := testbase.HookCommandOutput(&utils.AptCommandOutput, []byte{}, nil)
 	defer cleanup()
-	err := AptGetInstall("test-package")
+	err := utils.AptGetInstall("test-package")
 	c.Assert(err, gc.IsNil)
 	cmd := <-cmdChan
 	c.Assert(cmd.Args, gc.DeepEquals, []string{
@@ -35,9 +37,9 @@
 	const expected = `E: frobnicator failure detected`
 	cmdError := fmt.Errorf("error")
 	cmdExpectedError := fmt.Errorf("apt-get failed: error")
-	cmdChan, cleanup := testbase.HookCommandOutput(&AptCommandOutput, []byte(expected), cmdError)
+	cmdChan, cleanup := testbase.HookCommandOutput(&utils.AptCommandOutput, []byte(expected), cmdError)
 	defer cleanup()
-	err := AptGetInstall("foo")
+	err := utils.AptGetInstall("foo")
 	c.Assert(err, gc.DeepEquals, cmdExpectedError)
 	cmd := <-cmdChan
 	c.Assert(cmd.Args, gc.DeepEquals, []string{
@@ -48,9 +50,9 @@
 }
 
 func (s *AptSuite) TestConfigProxyEmpty(c *gc.C) {
-	cmdChan, cleanup := testbase.HookCommandOutput(&AptCommandOutput, []byte{}, nil)
+	cmdChan, cleanup := testbase.HookCommandOutput(&utils.AptCommandOutput, []byte{}, nil)
 	defer cleanup()
-	out, err := AptConfigProxy()
+	out, err := utils.AptConfigProxy()
 	c.Assert(err, gc.IsNil)
 	cmd := <-cmdChan
 	c.Assert(cmd.Args, gc.DeepEquals, []string{
@@ -63,9 +65,9 @@
 func (s *AptSuite) TestConfigProxyConfigured(c *gc.C) {
 	const expected = `Acquire::http::Proxy "10.0.3.1:3142";
 Acquire::https::Proxy "false";`
-	cmdChan, cleanup := testbase.HookCommandOutput(&AptCommandOutput, []byte(expected), nil)
+	cmdChan, cleanup := testbase.HookCommandOutput(&utils.AptCommandOutput, []byte(expected), nil)
 	defer cleanup()
-	out, err := AptConfigProxy()
+	out, err := utils.AptConfigProxy()
 	c.Assert(err, gc.IsNil)
 	cmd := <-cmdChan
 	c.Assert(cmd.Args, gc.DeepEquals, []string{
@@ -83,9 +85,9 @@
 		expected = `Acquire::http::Proxy  "10.0.3.1:3142";
 Acquire::https::Proxy "false";`
 	)
-	cmdChan, cleanup := testbase.HookCommandOutput(&AptCommandOutput, []byte(output), nil)
+	cmdChan, cleanup := testbase.HookCommandOutput(&utils.AptCommandOutput, []byte(output), nil)
 	defer cleanup()
-	out, err := AptConfigProxy()
+	out, err := utils.AptConfigProxy()
 	c.Assert(err, gc.IsNil)
 	cmd := <-cmdChan
 	c.Assert(cmd.Args, gc.DeepEquals, []string{
@@ -99,9 +101,9 @@
 	const expected = `E: frobnicator failure detected`
 	cmdError := fmt.Errorf("error")
 	cmdExpectedError := fmt.Errorf("apt-config failed: error")
-	cmdChan, cleanup := testbase.HookCommandOutput(&AptCommandOutput, []byte(expected), cmdError)
+	cmdChan, cleanup := testbase.HookCommandOutput(&utils.AptCommandOutput, []byte(expected), cmdError)
 	defer cleanup()
-	out, err := AptConfigProxy()
+	out, err := utils.AptConfigProxy()
 	c.Assert(err, gc.DeepEquals, cmdExpectedError)
 	cmd := <-cmdChan
 	c.Assert(cmd.Args, gc.DeepEquals, []string{
@@ -110,3 +112,37 @@
 	})
 	c.Assert(out, gc.Equals, "")
 }
+
+func (s *AptSuite) patchLsbRelease(c *gc.C, name string) {
+	content := fmt.Sprintf("#!/bin/bash --norc\necho %s", name)
+	patchExecutable(s, c.MkDir(), "lsb_release", content)
+}
+
+func (s *AptSuite) TestIsUbuntu(c *gc.C) {
+	s.patchLsbRelease(c, "Ubuntu")
+	c.Assert(utils.IsUbuntu(), jc.IsTrue)
+}
+
+func (s *AptSuite) TestIsNotUbuntu(c *gc.C) {
+	s.patchLsbRelease(c, "Windows NT")
+	c.Assert(utils.IsUbuntu(), jc.IsFalse)
+}
+
+func (s *AptSuite) patchDpkgQuery(c *gc.C, installed bool) {
+	rc := 0
+	if !installed {
+		rc = 1
+	}
+	content := fmt.Sprintf("#!/bin/bash --norc\nexit %v", rc)
+	patchExecutable(s, c.MkDir(), "dpkg-query", content)
+}
+
+func (s *AptSuite) TestIsPackageInstalled(c *gc.C) {
+	s.patchDpkgQuery(c, true)
+	c.Assert(utils.IsPackageInstalled("foo-bar"), jc.IsTrue)
+}
+
+func (s *AptSuite) TestIsPackageNotInstalled(c *gc.C) {
+	s.patchDpkgQuery(c, false)
+	c.Assert(utils.IsPackageInstalled("foo-bar"), jc.IsFalse)
+}

=== added file 'utils/command.go'
--- utils/command.go	1970-01-01 00:00:00 +0000
+++ utils/command.go	2013-12-02 21:28:17 +0000
@@ -0,0 +1,19 @@
+// Copyright 2013 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package utils
+
+import (
+	"os/exec"
+)
+
+// RunCommand executes the command and return the combined output.
+func RunCommand(command string, args ...string) (output string, err error) {
+	cmd := exec.Command(command, args...)
+	out, err := cmd.CombinedOutput()
+	output = string(out)
+	if err != nil {
+		return output, err
+	}
+	return output, nil
+}

=== added file 'utils/command_test.go'
--- utils/command_test.go	1970-01-01 00:00:00 +0000
+++ utils/command_test.go	2013-12-02 21:28:17 +0000
@@ -0,0 +1,52 @@
+// Copyright 2012, 2013 Canonical Ltd.
+// Licensed under the AGPLv3, see LICENCE file for details.
+
+package utils_test
+
+import (
+	"io/ioutil"
+	"path/filepath"
+
+	gc "launchpad.net/gocheck"
+
+	"launchpad.net/juju-core/testing/testbase"
+	"launchpad.net/juju-core/utils"
+)
+
+type EnvironmentPatcher interface {
+	PatchEnvironment(name, value string)
+}
+
+func patchExecutable(patcher EnvironmentPatcher, dir, execName, script string) {
+	patcher.PatchEnvironment("PATH", dir)
+	filename := filepath.Join(dir, execName)
+	ioutil.WriteFile(filename, []byte(script), 0755)
+}
+
+type commandSuite struct {
+	testbase.LoggingSuite
+}
+
+var _ = gc.Suite(&commandSuite{})
+
+func (s *commandSuite) TestRunCommandCombinesOutput(c *gc.C) {
+	content := `#!/bin/bash --norc
+echo stdout
+echo stderr 1>&2
+`
+	patchExecutable(s, c.MkDir(), "test-output", content)
+	output, err := utils.RunCommand("test-output")
+	c.Assert(err, gc.IsNil)
+	c.Assert(output, gc.Equals, "stdout\nstderr\n")
+}
+
+func (s *commandSuite) TestRunCommandNonZeroExit(c *gc.C) {
+	content := `#!/bin/bash --norc
+echo stdout
+exit 42
+`
+	patchExecutable(s, c.MkDir(), "test-output", content)
+	output, err := utils.RunCommand("test-output")
+	c.Assert(err, gc.ErrorMatches, `exit status 42`)
+	c.Assert(output, gc.Equals, "stdout\n")
+}