Merge lp:~thumper/golxc/clone into lp:golxc

[Tim Penhey]

Update signature for Create and Clone

Make it possible for the caller to specify additional args for both
Create and Clone. Instead of trying to provide an abstracted
interface for this, just allowing the caller to provide extra
args.

Some test cleanup.

[lp:~thumper/golxc/clone]

10. By Tim Penhey on 2014-03-11

Add a test to show command line args.

[Ian Booth (wallyworld)]

Nice.

review: Approve

=== modified file 'golxc.go'
--- golxc.go	2014-03-05 03:36:19 +0000
+++ golxc.go	2014-03-11 00:35:14 +0000
@@ -11,10 +11,15 @@
 	"fmt"
 	"os"
 	"os/exec"
+	"path"
 	"strconv"
 	"strings"
+
+	"github.com/juju/loggo"
 )
 
+var logger = loggo.GetLogger("golxc")
+
 // Error reports the failure of a LXC command.
 type Error struct {
 	Name   string
@@ -65,7 +70,7 @@
 	Name() string
 
 	// Create creates a new container based on the given template.
-	Create(configFile, template string, templateArgs ...string) error
+	Create(configFile, template string, extraArgs []string, templateArgs []string) error
 
 	// Start runs the container as a daemon.
 	Start(configFile, consoleFile string) error
@@ -74,7 +79,7 @@
 	Stop() error
 
 	// Clone creates a copy of the container, giving the copy the specified name.
-	Clone(name string) (Container, error)
+	Clone(name string, extraArgs []string, templateArgs []string) (Container, error)
 
 	// Freeze freezes all the container's processes.
 	Freeze() error
@@ -127,18 +132,26 @@
 	return &containerFactory{}
 }
 
+const DefaultLXCDir = "/var/lib/lxc"
+
+var ContainerDir = DefaultLXCDir
+
 type container struct {
 	name     string
 	logFile  string
 	logLevel LogLevel
+	// Newer LXC libraries can have containers in non-default locations. The
+	// containerDir is the directory that is the 'home' of this container.
+	containerDir string
 }
 
 type containerFactory struct{}
 
 func (*containerFactory) New(name string) Container {
 	return &container{
-		name:     name,
-		logLevel: LogWarning,
+		name:         name,
+		logLevel:     LogWarning,
+		containerDir: ContainerDir,
 	}
 }
 
@@ -179,7 +192,7 @@
 }
 
 // Create creates a new container based on the given template.
-func (c *container) Create(configFile, template string, templateArgs ...string) error {
+func (c *container) Create(configFile, template string, extraArgs []string, templateArgs []string) error {
 	if c.IsConstructed() {
 		return fmt.Errorf("container %q is already created", c.Name())
 	}
@@ -190,8 +203,12 @@
 	if configFile != "" {
 		args = append(args, "-f", configFile)
 	}
-	args = append(args, "--")
+	if len(extraArgs) != 0 {
+		args = append(args, extraArgs...)
+	}
 	if len(templateArgs) != 0 {
+		// Must be done in two steps due to current language implementation details.
+		args = append(args, "--")
 		args = append(args, templateArgs...)
 	}
 	_, err := run("lxc-create", args...)
@@ -223,7 +240,13 @@
 	if err != nil {
 		return err
 	}
-	return c.Wait(StateRunning, StateStopped)
+	if err := c.Wait(StateRunning, StateStopped); err != nil {
+		return err
+	}
+	if !c.IsRunning() {
+		return fmt.Errorf("container failed to start")
+	}
+	return nil
 }
 
 // Stop terminates the running container.
@@ -249,10 +272,13 @@
 }
 
 // Clone creates a copy of the container, it gets the given name.
-func (c *container) Clone(name string) (Container, error) {
+func (c *container) Clone(name string, extraArgs []string, templateArgs []string) (Container, error) {
 	if !c.IsConstructed() {
 		return nil, fmt.Errorf("container %q is not yet created", c.name)
 	}
+	if c.IsRunning() {
+		return nil, fmt.Errorf("cannot clone a running container")
+	}
 	cc := &container{
 		name: name,
 	}
@@ -263,6 +289,14 @@
 		"-o", c.name,
 		"-n", name,
 	}
+	if len(extraArgs) != 0 {
+		args = append(args, extraArgs...)
+	}
+	if len(templateArgs) != 0 {
+		// Must be done in two steps due to current language implementation details.
+		args = append(args, "--")
+		args = append(args, templateArgs...)
+	}
 	_, err := run("lxc-clone", args...)
 	if err != nil {
 		return nil, err
@@ -388,27 +422,32 @@
 
 // containerHome returns the name of the container directory.
 func (c *container) containerHome() string {
-	return "/var/lib/lxc/" + c.name
+	return path.Join(c.containerDir, c.name)
 }
 
 // rootfs returns the name of the directory containing the
 // root filesystem of the container.
 func (c *container) rootfs() string {
-	return c.containerHome() + "/rootfs/"
+	return path.Join(c.containerHome(), "rootfs")
 }
 
 // run executes the passed command and returns the out.
 func run(name string, args ...string) (string, error) {
+	logger := loggo.GetLogger(fmt.Sprintf("golxc.run.%s", name))
+	logger.Tracef("run: %s %v", name, args)
 	cmd := exec.Command(name, args...)
 	// LXC tools do not use stdout and stderr in a predictable
 	// way; based on experimentation, the most convenient
 	// solution is to combine them and leave the client to
 	// determine sanity as best it can.
 	out, err := cmd.CombinedOutput()
+	result := string(out)
 	if err != nil {
+		logger.Tracef("run failed output: %s", result)
 		return "", runError(name, err, out)
 	}
-	return string(out), nil
+	logger.Tracef("run successful output: %s", result)
+	return result, nil
 }
 
 // runError creates an error if run fails.
 
=== modified file 'golxc_test.go'
--- golxc_test.go	2014-03-05 03:36:19 +0000
+++ golxc_test.go	2014-03-11 00:35:14 +0000
@@ -5,17 +5,18 @@
 
 import (
 	"io/ioutil"
-	. "launchpad.net/gocheck"
 	"os"
 	"os/user"
 	"path/filepath"
-	"testing"
+
+	"github.com/juju/testing"
+	jc "github.com/juju/testing/checkers"
+	"github.com/juju/testing/logging"
+	. "launchpad.net/gocheck"
 
 	"launchpad.net/golxc"
 )
 
-func Test(t *testing.T) { TestingT(t) }
-
 var lxcfile = `# MIRROR to be used by ubuntu template at container creation:
 # Leaving it undefined is fine
 #MIRROR="http://archive.ubuntu.com/ubuntu"
@@ -142,6 +143,15 @@
 	}
 }
 
+func (s *LXCSuite) createContainer(c *C) golxc.Container {
+	container := s.factory.New("golxc")
+	c.Assert(container.IsConstructed(), Equals, false)
+	err := container.Create("", "ubuntu", nil, nil)
+	c.Assert(err, IsNil)
+	c.Assert(container.IsConstructed(), Equals, true)
+	return container
+}
+
 func (s *LXCSuite) TestCreateDestroy(c *C) {
 	// Test clean creation and destroying of a container.
 	lc := s.factory.New("golxc")
@@ -149,7 +159,7 @@
 	home := golxc.ContainerHome(lc)
 	_, err := os.Stat(home)
 	c.Assert(err, ErrorMatches, "stat .*: no such file or directory")
-	err = lc.Create("", "ubuntu")
+	err = lc.Create("", "ubuntu", nil, nil)
 	c.Assert(err, IsNil)
 	c.Assert(lc.IsConstructed(), Equals, true)
 	defer func() {
@@ -165,16 +175,13 @@
 
 func (s *LXCSuite) TestCreateTwice(c *C) {
 	// Test that a container cannot be created twice.
-	lc1 := s.factory.New("golxc")
-	c.Assert(lc1.IsConstructed(), Equals, false)
-	err := lc1.Create("", "ubuntu")
-	c.Assert(err, IsNil)
+	lc1 := s.createContainer(c)
 	c.Assert(lc1.IsConstructed(), Equals, true)
 	defer func() {
 		c.Assert(lc1.Destroy(), IsNil)
 	}()
 	lc2 := s.factory.New("golxc")
-	err = lc2.Create("", "ubuntu")
+	err := lc2.Create("", "ubuntu", nil, nil)
 	c.Assert(err, ErrorMatches, "container .* is already created")
 }
 
@@ -183,7 +190,7 @@
 	// case of an illegal template.
 	lc := s.factory.New("golxc")
 	c.Assert(lc.IsConstructed(), Equals, false)
-	err := lc.Create("", "name-of-a-not-existing-template-for-golxc")
+	err := lc.Create("", "name-of-a-not-existing-template-for-golxc", nil, nil)
 	c.Assert(err, ErrorMatches, `error executing "lxc-create": .*bad template.*`)
 	c.Assert(lc.IsConstructed(), Equals, false)
 }
@@ -211,10 +218,7 @@
 	oldLen := len(lcs)
 	c.Assert(err, IsNil)
 	c.Assert(oldLen >= 0, Equals, true)
-	lc := s.factory.New("golxc")
-	c.Assert(lc.IsConstructed(), Equals, false)
-	c.Assert(lc.Create("", "ubuntu"), IsNil)
-	c.Assert(lc.IsConstructed(), Equals, true)
+	lc := s.createContainer(c)
 	defer func() {
 		c.Assert(lc.Destroy(), IsNil)
 	}()
@@ -226,16 +230,13 @@
 
 func (s *LXCSuite) TestClone(c *C) {
 	// Test the cloning of an existing container.
-	lc1 := s.factory.New("golxc")
-	c.Assert(lc1.IsConstructed(), Equals, false)
-	c.Assert(lc1.Create("", "ubuntu"), IsNil)
-	c.Assert(lc1.IsConstructed(), Equals, true)
+	lc1 := s.createContainer(c)
 	defer func() {
 		c.Assert(lc1.Destroy(), IsNil)
 	}()
 	lcs, _ := s.factory.List()
 	oldLen := len(lcs)
-	lc2, err := lc1.Clone("golxcclone")
+	lc2, err := lc1.Clone("golxcclone", nil, nil)
 	c.Assert(err, IsNil)
 	c.Assert(lc2.IsConstructed(), Equals, true)
 	defer func() {
@@ -252,15 +253,13 @@
 	// Test the cloning of a non-existing container.
 	lc := s.factory.New("golxc")
 	c.Assert(lc.IsConstructed(), Equals, false)
-	_, err := lc.Clone("golxcclone")
+	_, err := lc.Clone("golxcclone", nil, nil)
 	c.Assert(err, ErrorMatches, "container .* is not yet created")
 }
 
 func (s *LXCSuite) TestStartStop(c *C) {
 	// Test starting and stopping a container.
-	lc := s.factory.New("golxc")
-	c.Assert(lc.IsConstructed(), Equals, false)
-	c.Assert(lc.Create("", "ubuntu"), IsNil)
+	lc := s.createContainer(c)
 	defer func() {
 		c.Assert(lc.Destroy(), IsNil)
 	}()
@@ -279,9 +278,7 @@
 
 func (s *LXCSuite) TestStopNotRunning(c *C) {
 	// Test that a not running container can't be stopped.
-	lc := s.factory.New("golxc")
-	c.Assert(lc.IsConstructed(), Equals, false)
-	c.Assert(lc.Create("", "ubuntu"), IsNil)
+	lc := s.createContainer(c)
 	defer func() {
 		c.Assert(lc.Destroy(), IsNil)
 	}()
@@ -296,7 +293,7 @@
 	c.Assert(lc.Wait(), ErrorMatches, "no states specified")
 	c.Assert(lc.Wait(golxc.StateStopped), IsNil)
 	c.Assert(lc.Wait(golxc.StateStopped, golxc.StateRunning), IsNil)
-	c.Assert(lc.Create("", "ubuntu"), IsNil)
+	c.Assert(lc.Create("", "ubuntu", nil, nil), IsNil)
 	defer func() {
 		c.Assert(lc.Destroy(), IsNil)
 	}()
@@ -308,9 +305,7 @@
 
 func (s *LXCSuite) TestFreezeUnfreeze(c *C) {
 	// Test the freezing and unfreezing of a started container.
-	lc := s.factory.New("golxc")
-	c.Assert(lc.IsConstructed(), Equals, false)
-	c.Assert(lc.Create("", "ubuntu"), IsNil)
+	lc := s.createContainer(c)
 	defer func() {
 		c.Assert(lc.Destroy(), IsNil)
 	}()
@@ -327,9 +322,7 @@
 
 func (s *LXCSuite) TestFreezeNotStarted(c *C) {
 	// Test that a not running container can't be frozen.
-	lc := s.factory.New("golxc")
-	c.Assert(lc.IsConstructed(), Equals, false)
-	c.Assert(lc.Create("", "ubuntu"), IsNil)
+	lc := s.createContainer(c)
 	defer func() {
 		c.Assert(lc.Destroy(), IsNil)
 	}()
@@ -352,9 +345,7 @@
 
 func (s *LXCSuite) TestUnfreezeNotFrozen(c *C) {
 	// Test that a running container can't be unfrozen.
-	lc := s.factory.New("golxc")
-	c.Assert(lc.IsConstructed(), Equals, false)
-	c.Assert(lc.Create("", "ubuntu"), IsNil)
+	lc := s.createContainer(c)
 	defer func() {
 		c.Assert(lc.Destroy(), IsNil)
 	}()
@@ -364,3 +355,60 @@
 	}()
 	c.Assert(lc.Unfreeze(), ErrorMatches, "container .* is not frozen")
 }
+
+type commandArgs struct {
+	logging.LoggingSuite
+}
+
+var _ = Suite(&commandArgs{})
+
+func (s *commandArgs) TestCreateArgs(c *C) {
+	s.PatchValue(&golxc.ContainerDir, c.MkDir())
+	testing.PatchExecutableAsEchoArgs(c, s, "lxc-create")
+
+	factory := golxc.Factory()
+	container := factory.New("test")
+	err := container.Create(
+		"config-file", "template",
+		[]string{"extra-1", "extra-2"},
+		[]string{"template-1", "template-2"},
+	)
+	c.Assert(err, IsNil)
+	testing.AssertEchoArgs(
+		c, "lxc-create",
+		"-n", "test",
+		"-t", "template",
+		"-f", "config-file",
+		"extra-1", "extra-2",
+		"--", "template-1", "template-2")
+}
+
+func (s *commandArgs) TestCloneArgs(c *C) {
+	dir := c.MkDir()
+	s.PatchValue(&golxc.ContainerDir, dir)
+	// Patch lxc-info too as clone checks to see if it is running.
+	testing.PatchExecutableAsEchoArgs(c, s, "lxc-info")
+	testing.PatchExecutableAsEchoArgs(c, s, "lxc-clone")
+
+	factory := golxc.Factory()
+	container := factory.New("test")
+	// Make the rootfs for the "test" container so it thinks it is created.
+	rootfs := filepath.Join(dir, "test", "rootfs")
+	err := os.MkdirAll(rootfs, 0755)
+	c.Assert(err, IsNil)
+	c.Assert(rootfs, jc.IsDirectory)
+	c.Assert(container.IsConstructed(), jc.IsTrue)
+	clone, err := container.Clone(
+		"name",
+		[]string{"extra-1", "extra-2"},
+		[]string{"template-1", "template-2"},
+	)
+	c.Assert(err, IsNil)
+	testing.AssertEchoArgs(
+		c, "lxc-clone",
+		"-o", "test",
+		"-n", "name",
+		"extra-1", "extra-2",
+		"--", "template-1", "template-2")
+	c.Assert(clone.Name(), Equals, "name")
+}
 
=== added file 'package_test.go'
--- package_test.go	1970-01-01 00:00:00 +0000
+++ package_test.go	2014-03-11 00:35:14 +0000
@@ -0,0 +1,14 @@
+// Copyright 2013 Canonical Ltd.
+// Licensed under the LGPLv3, see COPYING and COPYING.LESSER file for details.
+
+package golxc_test
+
+import (
+	"testing"
+
+	gc "launchpad.net/gocheck"
+)
+
+func Test(t *testing.T) {
+	gc.TestingT(t)
+}