Merge lp:~themue/golxc/001-added-sudo-and-first-test into lp:golxc

[Frank Mueller]

golxc: add first container tests

The package now has the first functions for the
testing of the container. Two external scripts
help to perform the tests as a root.

https://codereview.appspot.com/6852065/

[lp:~themue/golxc/001-added-sudo-and-first-test]

6. By Frank Mueller on 2012-11-20

golxc: added test for running container

7. By Frank Mueller on 2012-11-20

golxc: added more tests

[Roger Peppe (rogpeppe)]

lots of comments but nothing major.
i'm not familiar with lxc, so all superficial, i'm afraid.

https://codereview.appspot.com/6852065/diff/6001/golxc.go
File golxc.go (right):

https://codereview.appspot.com/6852065/diff/6001/golxc.go#newcode9
golxc.go:9: // Frank Mueller <email address hidden>
do we need individual attributions?

https://codereview.appspot.com/6852065/diff/6001/golxc.go#newcode29
golxc.go:29: ContainerStarting = 1 << iota
you can omit the "1 << iota" from here and the following constants.

however, ISTM that there's no particular reason for these to be bit
masks. the only place that we make use of their bitmask nature is in the
unexported Container.wait, and it would be just as easy (actually
easier, i think) to create a bitmask explicitly there.

also, how about:

type State uint

and using that as the state type, perhaps renaming the constants to
StateRunning, etc.

https://codereview.appspot.com/6852065/diff/6001/golxc.go#newcode52
golxc.go:52: var go2state = map[int]string{
if the states weren't bitmasks, this could just be []string and you'd
get the same result.

https://codereview.appspot.com/6852065/diff/6001/golxc.go#newcode82
golxc.go:82: var go2log = map[int]string{
[]string

https://codereview.appspot.com/6852065/diff/6001/golxc.go#newcode111
golxc.go:111: outbuf, _, err := run("lxc-ls", args...)
run("lxc-ls", "-1")

https://codereview.appspot.com/6852065/diff/6001/golxc.go#newcode113
golxc.go:113: log.Fatalf("cannot list containers: %v", err)
d

https://codereview.appspot.com/6852065/diff/6001/golxc.go#newcode117
golxc.go:117: containers := []*Container{}
containers := make([]*Container, len(names)) ?

and then containers[i] = New(name) below.

https://codereview.appspot.com/6852065/diff/6001/golxc.go#newcode142
golxc.go:142: "-t", template,
you could add "--" here unconditionally.
then the below lines could be simply:

args = append(args, tmplArgs)

https://codereview.appspot.com/6852065/diff/6001/golxc.go#newcode150
golxc.go:150: log.Fatalf("cannot create container %q: %v", c.name, err)
d

https://codereview.appspot.com/6852065/diff/6001/golxc.go#newcode173
golxc.go:173: log.Fatalf("cannot start container %q: %v", c.name, err)
d

https://codereview.appspot.com/6852065/diff/6001/golxc.go#newcode189
golxc.go:189: log.Fatalf("cannot stop container %q: %v", c.name, err)
d

https://codereview.appspot.com/6852065/diff/6001/golxc.go#newcode212
golxc.go:212: log.Fatalf("cannot clone container %q: %v", c.name, err)
d

https://codereview.appspot.com/6852065/diff/6001/golxc.go#newcode231
golxc.go:231: log.Fatalf("cannot freeze container %q: %v", c.name, err)
d

https://codereview.appspot.com/6852065/diff/6001/golxc.go#newcode250
golxc.go:250: log.Fatalf("cannot unfreeze container %q: %v", c.name,
err)
d

https://codereview.appspot.com/6852065/diff/6001/golxc.go#newcode269
golxc.go:269: log.Fatalf("cannot destroy container %q: %v", c.name, err)
d

https://codereview.appspot.com/6852065/diff/6001/golxc.go#newcode282
golxc.go:282: log.Fatalf("cannot retrieve container info for %q: %v",
c.name, err)
d

https://codereview.appspot.com/6852065/diff/6001/golxc.go#newcode289
golxc.go:289: return -1, -1, err
a m...

[lp:~themue/golxc/001-added-sudo-and-first-test]

8. By Frank Mueller on 2012-11-20

golxc: simplification after review

[Roger Peppe (rogpeppe)]

LGTM apart from the error messages. i think we don't want to discard the
important information printed to stderr.

https://codereview.appspot.com/6852065/diff/6001/golxc.go
File golxc.go (right):

https://codereview.appspot.com/6852065/diff/6001/golxc.go#newcode9
golxc.go:9: // Frank Mueller <email address hidden>
On 2012/11/20 16:34:04, TheMue wrote:
> On 2012/11/20 15:20:11, rog wrote:
> > do we need individual attributions?

> Got it from packages like goamz. But personally don't need it.

i think goamz is different because it really is almost all gustavo's
work. YMMV, whatever.

https://codereview.appspot.com/6852065/diff/6001/golxc.go#newcode370
golxc.go:370: log.Fatalf("error executing %q: %v",
strings.Join(cmd.Args, " "), err)
On 2012/11/20 16:34:04, TheMue wrote:
> On 2012/11/20 15:20:11, rog wrote:
> > d
> > but i wonder whether the error message here should actually
incorporate the
> > error printed to stderr. nothing that calls run actually uses the
stderr
> > returned, and the error returned by cmd.Run is, in most cases,
almost
> > meaningless.

> The err here is an error of Command executing the command, not the
stderr of the
> executed command. I changed it to mode it more clear. So far we have
no usage
> for stderr, but it's also not everything implemented. If there stays
no need it
> will be removed.

my point is that if a command fails, the only message we'll get will be
"exit status 1" or whatever, and the useful error message that the
command has printed to stderr will be discarded.

i think we should put at least some of that information into the err
return.

https://codereview.appspot.com/6852065/diff/11001/golxc.go
File golxc.go (right):

https://codereview.appspot.com/6852065/diff/11001/golxc.go#newcode35
golxc.go:35: // LogLevel represents a containers log level.
s/containers/container's/

https://codereview.appspot.com/6852065/diff/11001/golxc.go#newcode86
golxc.go:86: c.logLevel = logLevel
is it really worth having the setter method here?
could we just export the relevant fields?

https://codereview.appspot.com/6852065/diff/11001/golxc.go#newcode273
golxc.go:273: func (c *Container) wait(states ...State) error {
given that nothing is actually making use of the functionality yet, we
could just have wait(state State) and not bother with combining states
at all. just a then this function is only 5 lines. just a thought.

https://codereview.appspot.com/6852065/

[lp:~themue/golxc/001-added-sudo-and-first-test]

9. By Frank Mueller on 2012-11-21

golxc: more changes after review

[William Reade (fwereade)]

Not sure about the tests -- it looks like there's plenty of opportunity
from pollution from failures. Is there some motivation for this
behaviour that I've missed?

Also, it kinda looks as though the tests will fail if your system is
already running a container. Can you see any way around this?

https://codereview.appspot.com/6852065/diff/7007/golxc.go
File golxc.go (right):

https://codereview.appspot.com/6852065/diff/7007/golxc.go#newcode217
golxc.go:217: _, _, err := run("lxc-wait", "-n", c.name, "-s",
combinedStates)
I guess LXC will complain about impossible state combinations?

https://codereview.appspot.com/6852065/diff/7007/golxc.go#newcode257
golxc.go:257: // String returns a short information about the container.
s/a short/brief/

perhaps?

https://codereview.appspot.com/6852065/diff/7007/golxc_test.go
File golxc_test.go (right):

https://codereview.appspot.com/6852065/diff/7007/golxc_test.go#newcode20
golxc_test.go:20: lc.Destroy()
Shouldn't these be destroyed inline via defer?

https://codereview.appspot.com/6852065/

[lp:~themue/golxc/001-added-sudo-and-first-test]

10. By Frank Mueller on 2012-11-22

golxc: hardened package and added more tests after review

[William Reade (fwereade)]

A few more comments...

https://codereview.appspot.com/6852065/diff/6002/golxc.go
File golxc.go (right):

https://codereview.appspot.com/6852065/diff/6002/golxc.go#newcode180
golxc.go:180: // Unfreeze thaws all a froozen container's processes.
s/rooz/roz/

https://codereview.appspot.com/6852065/diff/6002/golxc.go#newcode186
golxc.go:186: return fmt.Errorf("container %q is not froozen", c.name)
ditto

https://codereview.appspot.com/6852065/diff/6002/golxc.go#newcode219
golxc.go:219: switch len(states) {
I'm not sure the switch is a big win over a simple err return when no
states are supplied -- the one-state case may be more efficient but I'm
not sure it really aids clarity. Up to you though.

https://codereview.appspot.com/6852065/diff/6002/golxc.go#newcode271
golxc.go:271: // String returns a brief information about the container.
s/a //

?

https://codereview.appspot.com/6852065/diff/6002/golxc_test.go
File golxc_test.go (right):

https://codereview.appspot.com/6852065/diff/6002/golxc_test.go#newcode25
golxc_test.go:25: c.Assert(lc.IsConstructed(), Equals, true)
Shouldn't we defer the Destroy at this point?

https://codereview.appspot.com/6852065/diff/6002/golxc_test.go#newcode40
golxc_test.go:40: defer lc1.Destroy()
This should be either 1 or 2 lines down, shouldn't it? And surely we
should be asserting no error?

https://codereview.appspot.com/6852065/diff/6002/golxc_test.go#newcode74
golxc_test.go:74: defer lc.Destroy()
Handle errors? (after checking no error on preceding Create)

https://codereview.appspot.com/6852065/diff/6002/golxc_test.go#newcode87
golxc_test.go:87: defer lc1.Destroy()
ditto

https://codereview.appspot.com/6852065/diff/6002/golxc_test.go#newcode91
golxc_test.go:91: defer lc2.Destroy()
ditto

https://codereview.appspot.com/6852065/diff/6002/golxc_test.go#newcode116
golxc_test.go:116: defer lc.Destroy()
ditto

https://codereview.appspot.com/6852065/diff/6002/golxc_test.go#newcode137
golxc_test.go:137: defer lc.Destroy()
ditto

https://codereview.appspot.com/6852065/diff/6002/golxc_test.go#newcode138
golxc_test.go:138: go lc.Start("", "")
Destroy works even if the container is running?

https://codereview.appspot.com/6852065/diff/6002/golxc_test.go#newcode161
golxc_test.go:161: // Test that a not running container can't be
froozen.
s/rooz/roz/

https://codereview.appspot.com/6852065/diff/6002/golxc_test.go#newcode171
golxc_test.go:171: // Test that a non-existing container can't be
froozen.
ditto

https://codereview.appspot.com/6852065/diff/6002/golxc_test.go#newcode179
golxc_test.go:179: // Test that a non-existing container can't be
unfroozen.
ditto

https://codereview.appspot.com/6852065/diff/6002/golxc_test.go#newcode186
golxc_test.go:186: func (l *LXCSuite) TestUnfreezeNotFroozen(c *C) {
ditto

https://codereview.appspot.com/6852065/diff/6002/golxc_test.go#newcode187
golxc_test.go:187: // Test that a running container can't be unfroozen.
ditto

https://codereview.appspot.com/6852065/diff/6002/golxc_test.go#newcode193
golxc_test.go:193: defer lc.Stop()
error checking

https://codereview.appspot.com/6852065/diff/6002/golxc_test.go#newcode195
golxc_test.go:195: c.Assert(err, ErrorMatches, "container .* is not
froozen...

[lp:~themue/golxc/001-added-sudo-and-first-test]

11. By Frank Mueller on 2012-11-22

golxc: better error checking after review

[William Reade (fwereade)]

LGTM!

https://codereview.appspot.com/6852065/

[Roger Peppe (rogpeppe)]

looks good to me apart from (still) the error messages.
a few minor suggestions in addition to that.

https://codereview.appspot.com/6852065/diff/6002/golxc.go
File golxc.go (right):

https://codereview.appspot.com/6852065/diff/6002/golxc.go#newcode219
golxc.go:219: switch len(states) {
On 2012/11/22 14:35:57, fwereade wrote:
> I'm not sure the switch is a big win over a simple err return when no
states are
> supplied -- the one-state case may be more efficient but I'm not sure
it really
> aids clarity. Up to you though.

i think i agree here. we don't mind about a single non-escaping
allocation. i'd lose the "case 1" and make the "case 0" into an if that
returns the error.

https://codereview.appspot.com/6852065/diff/6002/golxc.go#newcode271
golxc.go:271: // String returns a brief information about the container.
On 2012/11/22 14:35:57, fwereade wrote:
> s/a //

> ?

s/a brief // ?

https://codereview.appspot.com/6852065/diff/6002/golxc.go#newcode277
golxc.go:277: return fmt.Sprintf("container %q has state %q and pid %d",
c.name, state, pid)
i'd be tempted to go a little more terse than this.

something like this, as an example, perhaps?

container "foo" (STARTING, pid 2353)

https://codereview.appspot.com/6852065/diff/6002/golxc.go#newcode304
golxc.go:304: return outbuf.String(), errbuf.String(), err
i'm still not happy with this. if we call Container.Start and get the
error "exit status 1", that's an extremely unhelpful error. for an
example of what might be a reasonable approach, see how lbox deals with
error returns from running bzr. (search for bzrError in the lbox
source). you might want to tailor that for the way that lxc tends to
print error messages.

https://codereview.appspot.com/6852065/diff/9003/golxc_test.go
File golxc_test.go (right):

https://codereview.appspot.com/6852065/diff/9003/golxc_test.go#newcode12
golxc_test.go:12: type LXCSuite struct{}
i wonder if it might be worth Skipping the suite (with a warning,
perhaps) if we're not running as root.

https://codereview.appspot.com/6852065/

[lp:~themue/golxc/001-added-sudo-and-first-test]

12. By Frank Mueller on 2012-11-22

golxc: added error type and test skipping if not root

[Roger Peppe (rogpeppe)]

much better, but still not *quite* there.

i'd like to see at least one test that verifies the new error logic.

https://codereview.appspot.com/6852065/diff/5003/golxc.go
File golxc.go (right):

https://codereview.appspot.com/6852065/diff/5003/golxc.go#newcode29
golxc.go:29: if strings.HasPrefix(e.StdErr, e.Name) {
perhaps slightly more robust to do: HasPrefix(e.StdErr, e.Name + ": ")
and more obvious to the reader what the extra 2 chars are too.

also, what if the error output is more than one line? i'm not sure if
it's best to discard all but the first line in that case, or remove the
prefix from all lines. you could do the latter and join the lines with
semicolons, i suppose. might be worth discussing with gustavo, as he has
had definite opinions on this subject in the past.

https://codereview.appspot.com/6852065/

[lp:~themue/golxc/001-added-sudo-and-first-test]

13. By Frank Mueller on 2012-11-23

golxc: improved error handling after review

[Roger Peppe (rogpeppe)]

LGTM with one suggestion below.

i'm worried i might have led you up the garden path with this one though
- please check with niemeyer that this logic is acceptable.

thanks for bearing with me!

https://codereview.appspot.com/6852065/diff/18/golxc.go
File golxc.go (right):

https://codereview.appspot.com/6852065/diff/18/golxc.go#newcode326
golxc.go:326: if len(output) == 0 {
i think this might be better as something like:

     e := &Error{name, err, nil}
     for _, l := range strings.Split(string(output), "\n") {
         if l != "" {
             e.Output = append(e.Output, l)
         }
     }
     return e

i think that's equivalent, except it also removes all blank lines, which
could be considered a good thing.

if you wanted, this would also be a good place to strip the command
prefix from all the lines.

https://codereview.appspot.com/6852065/

=== added file 'export_test.go'
--- export_test.go	1970-01-01 00:00:00 +0000
+++ export_test.go	2012-11-23 13:49:20 +0000
@@ -0,0 +1,6 @@
+package golxc
+
+// ContainerHome returns the name of the container directory.
+func ContainerHome(c *Container) string {
+	return c.containerHome()
+}
\ No newline at end of file

=== modified file 'golxc.go'
--- golxc.go	2012-11-16 12:34:50 +0000
+++ golxc.go	2012-11-23 13:49:20 +0000
@@ -1,158 +1,119 @@
 //
-// goamz - Go package to interact with Linux Containers (LXC).
+// golxc - Go package to interact with Linux Containers (LXC).
 //
-// https://wiki.ubuntu.com/golxc
+// https://launchpad.net/golxc/
 //
 // Copyright (c) 2012 Canonical Ltd.
 //
-// Written by Aram Hăvărneanu <[e-mail]>
-// and Frank Mueller <[e-mail]>
-//
 
 package golxc
 
 import (
-	"bytes"
 	"fmt"
-	"log"
 	"os"
 	"os/exec"
 	"strconv"
 	"strings"
 )
 
-const (
-	// ContainerStopped represents a stopped container.
-	ContainerStopped = 1 << iota
-
-	// ContainerStarting represents a starting container.
-	ContainerStarting = 1 << iota
-
-	// ContainerRunning represents a running container.
-	ContainerRunning = 1 << iota
-
-	// ContainerAborting represents a starting container after
-	// an error occured.
-	ContainerAborting = 1 << iota
-
-	// ContainerStopping represents a stopping container.
-	ContainerStopping = 1 << iota
-)
-
-// state2go maps lxc state strings to the according go constants.
-var state2go = map[string]int{
-	"STOPPED":  ContainerStopped,
-	"STARTING": ContainerStarting,
-	"RUNNING":  ContainerRunning,
-	"ABORTING": ContainerAborting,
-	"STOPPING": ContainerStopping,
-}
-
-// go2state maps go state constants to the according lxc strings.
-var go2state = map[int]string{
-	ContainerStopped:  "STOPPED",
-	ContainerStarting: "STARTING",
-	ContainerRunning:  "RUNNING",
-	ContainerAborting: "ABORTING",
-	ContainerStopping: "STOPPING",
-}
-
-const (
-	LogDebug = iota
-	LogInfo
-	LogNotice
-	LogWarning
-	LogError
-	LogCritical
-	LogFatal
-)
-
-// log2go maps the lxc log strings to the according go constants.
-var log2go = map[string]int{
-	"DEBUG":  LogDebug,
-	"INFO":   LogInfo,
-	"NOTICE": LogNotice,
-	"WARN":   LogWarning,
-	"ERROR":  LogError,
-	"CRIT":   LogCritical,
-	"FATAL":  LogFatal,
-}
-
-// go2log maps the go log constants to the according lxc strings.
-var go2log = map[int]string{
-	LogDebug:    "DEBUG",
-	LogInfo:     "INFO",
-	LogNotice:   "NOTICE",
-	LogWarning:  "WARN",
-	LogError:    "ERROR",
-	LogCritical: "CRIT",
-	LogFatal:    "FATAL",
-}
+// Error reports the failure of a LXC command.
+type Error struct {
+	Name   string
+	Err    error
+	Output []string
+}
+
+func (e Error) Error() string {
+	if e.Output == nil {
+		return fmt.Sprintf("error executing %q: %v", e.Name, e.Err)
+	}
+	if len(e.Output) == 1 {
+		var info string
+		if strings.HasPrefix(e.Output[0], e.Name+": ") {
+			info = e.Output[0][len(e.Name)+2:]
+		} else {
+			info = e.Output[0]
+		}
+		return fmt.Sprintf("error executing %q: %v", e.Name, info)
+	}
+	return fmt.Sprintf("error executing %q: %s", e.Name, strings.Join(e.Output, "; "))
+}
+
+// State represents a container state.
+type State string
+
+const (
+	StateUnknown  State = "UNKNOWN"
+	StateStopped  State = "STOPPED"
+	StateStarting State = "STARTING"
+	StateRunning  State = "RUNNING"
+	StateAborting State = "ABORTING"
+	StateStopping State = "STOPPING"
+)
+
+// LogLevel represents a container's log level.
+type LogLevel string
+
+const (
+	LogDebug    LogLevel = "DEBUG"
+	LogInfo     LogLevel = "INFO"
+	LogNotice   LogLevel = "NOTICE"
+	LogWarning  LogLevel = "WARN"
+	LogError    LogLevel = "ERROR"
+	LogCritical LogLevel = "CRIT"
+	LogFatal    LogLevel = "FATAL"
+)
 
 // Container represents a linux container instance and provides
 // operations to create, maintain and destroy the container. 
 type Container struct {
 	name     string
-	logFile  string
-	logLevel string
+	LogFile  string
+	LogLevel LogLevel
 }
 
 // New returns a container instance which can then be used for operations
 // like Create(), Start(), Stop() or Destroy().
 func New(name string) *Container {
-	c := &Container{
+	return &Container{
 		name: name,
 	}
-	// Debugging infos.
-	state, pid, err := c.Info()
-	if err != nil {
-		log.Fatalf("cannot retrieve container info for %q: %v", c.name, err)
-		return nil
-	}
-	log.Printf("container %q has state %q and pid %d", c.name, go2state[state], pid)
-	return c
 }
 
 // List lists the existing containers on the system.
 func List() ([]*Container, error) {
-	args := []string{"-1"}
-	cmd := exec.Command("lxc-ls", args...)
-	outbuf, _, err := run(cmd)
+	stdout, err := run("lxc-ls", "-1")
 	if err != nil {
-		log.Fatalf("cannot list containers: %v", err)
 		return nil, err
 	}
-	names := nameSet(outbuf)
-	containers := []*Container{}
-	for _, name := range names {
-		containers = append(containers, New(name))
+	names := nameSet(stdout)
+	containers := make([]*Container, len(names))
+	for i, name := range names {
+		containers[i] = New(name)
 	}
 	return containers, nil
 }
 
-// SetLog configures the logging for the lxc commands.
-func (c *Container) SetLog(logFile string, logLevel int) {
-	c.logFile = logFile
-	c.logLevel = go2log[logLevel]
+// Name returns the name of the container.
+func (c *Container) Name() string {
+	return c.name
 }
 
 // Create creates a new container based on the given template.
 func (c *Container) Create(template string, tmplArgs ...string) error {
 	if c.IsConstructed() {
-		return nil
+		return fmt.Errorf("container %q is already created", c.Name())
 	}
 	args := []string{
 		"-n", c.name,
 		"-t", template,
+		"--",
 	}
 	if len(tmplArgs) != 0 {
-		// Arguments after -- will be passed to the template.
-		args = append(append(args, "--"), tmplArgs...)
+		args = append(args, tmplArgs...)
 	}
-	cmd := exec.Command("lxc-create", args...)
-	_, _, err := run(cmd)
+	_, err := run("lxc-create", args...)
 	if err != nil {
-		log.Fatalf("cannot create container %q: %v", c.name, err)
 		return err
 	}
 	return nil
@@ -160,6 +121,9 @@
 
 // Start runs the container as a daemon.
 func (c *Container) Start(configFile, consoleFile string) error {
+	if !c.IsConstructed() {
+		return fmt.Errorf("container %q is not yet created", c.name)
+	}
 	args := []string{
 		"--daemon",
 		"-n", c.name,
@@ -170,33 +134,32 @@
 	if consoleFile != "" {
 		args = append(args, "-c", consoleFile)
 	}
-	if c.logFile != "" {
-		args = append(args, "-o", c.logFile, "-l", c.logLevel)
+	if c.LogFile != "" {
+		args = append(args, "-o", c.LogFile, "-l", string(c.LogLevel))
 	}
-	cmd := exec.Command("lxc-start", args...)
-	_, _, err := run(cmd)
+	_, err := run("lxc-start", args...)
 	if err != nil {
-		log.Fatalf("cannot start container %q: %v", c.name, err)
 		return err
 	}
-	return c.wait(ContainerRunning)
+	return c.Wait(StateRunning)
 }
 
 // Stop terminates the running container.
 func (c *Container) Stop() error {
+	if !c.IsConstructed() {
+		return fmt.Errorf("container %q is not yet created", c.name)
+	}
 	args := []string{
 		"-n", c.name,
 	}
-	if c.logFile != "" {
-		args = append(args, "-o", c.logFile, "-l", c.logLevel)
+	if c.LogFile != "" {
+		args = append(args, "-o", c.LogFile, "-l", string(c.LogLevel))
 	}
-	cmd := exec.Command("lxc-stop", args...)
-	_, _, err := run(cmd)
+	_, err := run("lxc-stop", args...)
 	if err != nil {
-		log.Fatalf("cannot stop container %q: %v", c.name, err)
 		return err
 	}
-	return c.wait(ContainerStopped)
+	return c.Wait(StateStopped)
 }
 
 // Clone creates a copy of the container, it gets the given name.
@@ -214,50 +177,50 @@
 		"-o", c.name,
 		"-n", name,
 	}
-	cmd := exec.Command("lxc-clone", args...)
-	_, _, err := run(cmd)
+	_, err := run("lxc-clone", args...)
 	if err != nil {
-		log.Fatalf("cannot clone container %q: %v", c.name, err)
 		return nil, err
 	}
 	return cc, nil
 }
 
-// Freeze freezes all the container'ss processes.
+// Freeze freezes all the container's processes.
 func (c *Container) Freeze() error {
 	if !c.IsConstructed() {
 		return fmt.Errorf("container %q is not yet created", c.name)
 	}
+	if !c.IsRunning() {
+		return fmt.Errorf("container %q is not running", c.name)
+	}
 	args := []string{
 		"-n", c.name,
 	}
-	if c.logFile != "" {
-		args = append(args, "-o", c.logFile, "-l", c.logLevel)
+	if c.LogFile != "" {
+		args = append(args, "-o", c.LogFile, "-l", string(c.LogLevel))
 	}
-	cmd := exec.Command("lxc-freeze", args...)
-	_, _, err := run(cmd)
+	_, err := run("lxc-freeze", args...)
 	if err != nil {
-		log.Fatalf("cannot freeze container %q: %v", c.name, err)
 		return err
 	}
 	return nil
 }
 
-// Unfreeze thaws all a froozen container's processes.
+// Unfreeze thaws all frozen container's processes.
 func (c *Container) Unfreeze() error {
 	if !c.IsConstructed() {
 		return fmt.Errorf("container %q is not yet created", c.name)
 	}
+	if c.IsRunning() {
+		return fmt.Errorf("container %q is not frozen", c.name)
+	}
 	args := []string{
 		"-n", c.name,
 	}
-	if c.logFile != "" {
-		args = append(args, "-o", c.logFile, "-l", c.logLevel)
+	if c.LogFile != "" {
+		args = append(args, "-o", c.LogFile, "-l", string(c.LogLevel))
 	}
-	cmd := exec.Command("lxc-unfreeze", args...)
-	_, _, err := run(cmd)
+	_, err := run("lxc-unfreeze", args...)
 	if err != nil {
-		log.Fatalf("cannot unfreeze container %q: %v", c.name, err)
 		return err
 	}
 	return nil
@@ -271,34 +234,41 @@
 	if err := c.Stop(); err != nil {
 		return err
 	}
-	args := []string{
-		"-n", c.name,
-	}
-	cmd := exec.Command("lxc-destroy", args...)
-	_, _, err := run(cmd)
-	if err != nil {
-		log.Fatalf("cannot destroy container %q: %v", c.name, err)
+	_, err := run("lxc-destroy", "-n", c.name)
+	if err != nil {
+		return err
+	}
+	return nil
+}
+
+// Wait waits for one of the specified container states.
+func (c *Container) Wait(states ...State) error {
+	if len(states) == 0 {
+		return fmt.Errorf("no states specified")
+	}
+	stateStrs := make([]string, len(states))
+	for i, state := range states {
+		stateStrs[i] = string(state)
+	}
+	waitStates := strings.Join(stateStrs, "|")
+	_, err := run("lxc-wait", "-n", c.name, "-s", waitStates)
+	if err != nil {
 		return err
 	}
 	return nil
 }
 
 // Info returns the status and the process id of the container.
-func (c *Container) Info() (int, int, error) {
-	args := []string{
-		"-n", c.name,
-	}
-	cmd := exec.Command("lxc-info", args...)
-	outbuf, _, err := run(cmd)
+func (c *Container) Info() (State, int, error) {
+	stdout, err := run("lxc-info", "-n", c.name)
 	if err != nil {
-		log.Fatalf("cannot retrieve container info for %q: %v", c.name, err)
-		return -1, -1, err
+		return StateUnknown, -1, err
 	}
-	kv := keyValues(outbuf, ": ")
-	state := state2go[kv["state"]]
+	kv := keyValues(stdout, ": ")
+	state := State(kv["state"])
 	pid, err := strconv.Atoi(kv["pid"])
 	if err != nil {
-		return -1, -1, err
+		return StateUnknown, -1, fmt.Errorf("cannot read the pid: %v", err)
 	}
 	return state, pid, nil
 }
@@ -318,7 +288,16 @@
 	if err != nil {
 		return false
 	}
-	return state == ContainerRunning
+	return state == StateRunning
+}
+
+// String returns information about the container.
+func (c *Container) String() string {
+	state, pid, err := c.Info()
+	if err != nil {
+		return fmt.Sprintf("cannot retrieve container info for %q: %v", c.name, err)
+	}
+	return fmt.Sprintf("container %q (%s, pid %d)", c.name, state, pid)
 }
 
 // containerHome returns the name of the container directory.
@@ -332,54 +311,40 @@
 	return c.containerHome() + "/rootfs/"
 }
 
-// wait waits for the passed container status.
-func (c *Container) wait(status int) error {
-	lxcStatus := []string{}
-	if status&ContainerStarting != 0 {
-		lxcStatus = append(lxcStatus, "STARTING")
-	}
-	if status&ContainerRunning != 0 {
-		lxcStatus = append(lxcStatus, "RUNNING")
-	}
-	if status&ContainerAborting != 0 {
-		lxcStatus = append(lxcStatus, "ABORTING")
-	}
-	if status&ContainerStopping != 0 {
-		lxcStatus = append(lxcStatus, "STOPPING")
-	}
-	if status&ContainerStopped != 0 {
-		lxcStatus = append(lxcStatus, "STOPPED")
-	}
-	lxcStatusStr := strings.Join(lxcStatus, "|")
-	args := []string{
-		"-n", c.name,
-		"-s", lxcStatusStr,
-	}
-	cmd := exec.Command("lxc-wait", args...)
-	_, _, err := run(cmd)
+// run executes the passed command and returns the output.
+func run(name string, args ...string) (string, error) {
+	cmd := exec.Command(name, args...)
+	output, err := cmd.CombinedOutput()
 	if err != nil {
-		log.Fatalf("cannot wait for container %q: %v", c.name, err)
-		return err
+		return "", runError(name, err, output)
 	}
-	return nil
+	return string(output), nil
 }
 
-// run executes the passed command and returns stdout and stderr.
-func run(cmd *exec.Cmd) (outbuf, errbuf bytes.Buffer, err error) {
-	cmd.Stdout = &outbuf
-	cmd.Stderr = &errbuf
-	err = cmd.Run()
-	if err != nil {
-		log.Fatalf("error executing %q: %v", strings.Join(cmd.Args, " "), err)
-		return
-	}
-	return
+// runError creates an error if run fails.
+func runError(name string, err error, output []byte) error {
+	if len(output) == 0 {
+		return &Error{name, err, nil}
+	}
+	// Remove leading and/or trailing blank lines some
+	// of the LXC outputs contain.
+	lines := strings.Split(string(output), "\n")
+	if len(lines) > 0 && lines[0] == "" {
+		lines = lines[1:]
+	}
+	if len(lines) > 0 && lines[len(lines)-1] == "" {
+		lines = lines[:len(lines)-1]
+	}
+	if len(lines) == 0 {
+		return &Error{name, err, nil}
+	}
+	return &Error{name, err, lines}
 }
 
 // keyValues retrieves key/value pairs out of a command output.
-func keyValues(raw bytes.Buffer, sep string) map[string]string {
+func keyValues(raw string, sep string) map[string]string {
 	kv := map[string]string{}
-	lines := strings.Split(raw.String(), "\n")
+	lines := strings.Split(raw, "\n")
 	for _, line := range lines {
 		parts := strings.SplitN(line, sep, 2)
 		if len(parts) == 2 {
@@ -390,10 +355,10 @@
 }
 
 // nameSet retrieves a set of names out of a command output.
-func nameSet(raw bytes.Buffer) []string {
+func nameSet(raw string) []string {
 	collector := map[string]struct{}{}
 	set := []string{}
-	lines := strings.Split(raw.String(), "\n")
+	lines := strings.Split(raw, "\n")
 	for _, line := range lines {
 		name := strings.TrimSpace(line)
 		if name != "" {

=== added file 'golxc_test.go'
--- golxc_test.go	1970-01-01 00:00:00 +0000
+++ golxc_test.go	2012-11-23 13:49:20 +0000
@@ -0,0 +1,247 @@
+package golxc_test
+
+import (
+	. "launchpad.net/gocheck"
+	"launchpad.net/golxc"
+	"os"
+	"os/user"
+	"testing"
+)
+
+func Test(t *testing.T) { TestingT(t) }
+
+type LXCSuite struct{}
+
+var _ = Suite(&LXCSuite{})
+
+func (s *LXCSuite) SetUpSuite(c *C) {
+	u, err := user.Current()
+	c.Assert(err, IsNil)
+	if u.Uid != "0" {
+		// Has to be run as root!
+		c.Skip("tests must run as root")
+	}
+}
+
+func (s *LXCSuite) TestCreateDestroy(c *C) {
+	// Test clean creation and destroying of a container.
+	lc := golxc.New("golxc")
+	c.Assert(lc.IsConstructed(), Equals, false)
+	home := golxc.ContainerHome(lc)
+	_, err := os.Stat(home)
+	c.Assert(err, ErrorMatches, "stat .*: no such file or directory")
+	err = lc.Create("ubuntu")
+	c.Assert(err, IsNil)
+	c.Assert(lc.IsConstructed(), Equals, true)
+	defer func() {
+		err = lc.Destroy()
+		c.Assert(err, IsNil)
+		_, err = os.Stat(home)
+		c.Assert(err, ErrorMatches, "stat .*: no such file or directory")
+	}()
+	fi, err := os.Stat(golxc.ContainerHome(lc))
+	c.Assert(err, IsNil)
+	c.Assert(fi.IsDir(), Equals, true)
+}
+
+func (s *LXCSuite) TestCreateTwice(c *C) {
+	// Test that a container cannot be created twice.
+	lc1 := golxc.New("golxc")
+	c.Assert(lc1.IsConstructed(), Equals, false)
+	err := lc1.Create("ubuntu")
+	c.Assert(err, IsNil)
+	c.Assert(lc1.IsConstructed(), Equals, true)
+	defer func() {
+		c.Assert(lc1.Destroy(), IsNil)
+	}()
+	lc2 := golxc.New("golxc")
+	err = lc2.Create("ubuntu")
+	c.Assert(err, ErrorMatches, "container .* is already created")
+}
+
+func (s *LXCSuite) TestCreateIllegalTemplate(c *C) {
+	// Test that a container creation fails correctly in
+	// case of an illegal template.
+	lc := golxc.New("golxc")
+	c.Assert(lc.IsConstructed(), Equals, false)
+	err := lc.Create("name-of-a-not-existing-template-for-golxc")
+	c.Assert(err, ErrorMatches, `error executing "lxc-create": No config file specified, .*`)
+	c.Assert(lc.IsConstructed(), Equals, false)
+}
+
+func (l *LXCSuite) TestDestroyNotCreated(c *C) {
+	// Test that a non-existing container can't be destroyed.
+	lc := golxc.New("golxc")
+	c.Assert(lc.IsConstructed(), Equals, false)
+	err := lc.Destroy()
+	c.Assert(err, ErrorMatches, "container .* is not yet created")
+}
+
+func contains(lcs []*golxc.Container, lc *golxc.Container) bool {
+	for _, clc := range lcs {
+		if clc.Name() == lc.Name() {
+			return true
+		}
+	}
+	return false
+}
+
+func (s *LXCSuite) TestList(c *C) {
+	// Test the listing of created containers.
+	lcs, err := golxc.List()
+	oldLen := len(lcs)
+	c.Assert(err, IsNil)
+	c.Assert(oldLen >= 0, Equals, true)
+	lc := golxc.New("golxc")
+	c.Assert(lc.IsConstructed(), Equals, false)
+	c.Assert(lc.Create("ubuntu"), IsNil)
+	c.Assert(lc.IsConstructed(), Equals, true)
+	defer func() {
+		c.Assert(lc.Destroy(), IsNil)
+	}()
+	lcs, _ = golxc.List()
+	newLen := len(lcs)
+	c.Assert(newLen == oldLen+1, Equals, true)
+	c.Assert(contains(lcs, lc), Equals, true)
+}
+
+func (s *LXCSuite) TestClone(c *C) {
+	// Test the cloning of an existing container.
+	lc1 := golxc.New("golxc")
+	c.Assert(lc1.IsConstructed(), Equals, false)
+	c.Assert(lc1.Create("ubuntu"), IsNil)
+	c.Assert(lc1.IsConstructed(), Equals, true)
+	defer func() {
+		c.Assert(lc1.Destroy(), IsNil)
+	}()
+	lcs, _ := golxc.List()
+	oldLen := len(lcs)
+	lc2, err := lc1.Clone("golxcclone")
+	c.Assert(err, IsNil)
+	c.Assert(lc2.IsConstructed(), Equals, true)
+	defer func() {
+		c.Assert(lc2.Destroy(), IsNil)
+	}()
+	lcs, _ = golxc.List()
+	newLen := len(lcs)
+	c.Assert(newLen == oldLen+1, Equals, true)
+	c.Assert(contains(lcs, lc1), Equals, true)
+	c.Assert(contains(lcs, lc2), Equals, true)
+}
+
+func (s *LXCSuite) TestCloneNotCreated(c *C) {
+	// Test the cloning of a non-existing container.
+	lc := golxc.New("golxc")
+	c.Assert(lc.IsConstructed(), Equals, false)
+	_, err := lc.Clone("golxcclone")
+	c.Assert(err, ErrorMatches, "container .* is not yet created")
+}
+
+func (s *LXCSuite) TestStartStop(c *C) {
+	// Test starting and stopping a container.
+	lc := golxc.New("golxc")
+	c.Assert(lc.IsConstructed(), Equals, false)
+	c.Assert(lc.Create("ubuntu"), IsNil)
+	defer func() {
+		c.Assert(lc.Destroy(), IsNil)
+	}()
+	c.Assert(lc.Start("", ""), IsNil)
+	c.Assert(lc.IsRunning(), Equals, true)
+	c.Assert(lc.Stop(), IsNil)
+	c.Assert(lc.IsRunning(), Equals, false)
+}
+
+func (l *LXCSuite) TestStartNotCreated(c *C) {
+	// Test that a non-existing container can't be started.
+	lc := golxc.New("golxc")
+	c.Assert(lc.IsConstructed(), Equals, false)
+	c.Assert(lc.Start("", ""), ErrorMatches, "container .* is not yet created")
+}
+
+func (l *LXCSuite) TestStopNotRunning(c *C) {
+	// Test that a not running container can't be stopped.
+	lc := golxc.New("golxc")
+	c.Assert(lc.IsConstructed(), Equals, false)
+	c.Assert(lc.Create("ubuntu"), IsNil)
+	defer func() {
+		c.Assert(lc.Destroy(), IsNil)
+	}()
+	c.Assert(lc.Stop(), IsNil)
+}
+
+func (s *LXCSuite) TestWait(c *C) {
+	// Test waiting for one of a number of states of a container.
+	// ATTN: Using a not reached state blocks the test until timeout!
+	lc := golxc.New("golxc")
+	c.Assert(lc.IsConstructed(), Equals, false)
+	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)
+	defer func() {
+		c.Assert(lc.Destroy(), IsNil)
+	}()
+	go func() {
+		c.Assert(lc.Start("", ""), IsNil)
+	}()
+	c.Assert(lc.Wait(golxc.StateRunning), IsNil)
+}
+
+func (l *LXCSuite) TestFreezeUnfreeze(c *C) {
+	// Test the freezing and unfreezing of a started container.
+	lc := golxc.New("golxc")
+	c.Assert(lc.IsConstructed(), Equals, false)
+	c.Assert(lc.Create("ubuntu"), IsNil)
+	defer func() {
+		c.Assert(lc.Destroy(), IsNil)
+	}()
+	c.Assert(lc.Start("", ""), IsNil)
+	defer func() {
+		c.Assert(lc.Stop(), IsNil)
+	}()
+	c.Assert(lc.IsRunning(), Equals, true)
+	c.Assert(lc.Freeze(), IsNil)
+	c.Assert(lc.IsRunning(), Equals, false)
+	c.Assert(lc.Unfreeze(), IsNil)
+	c.Assert(lc.IsRunning(), Equals, true)
+}
+
+func (l *LXCSuite) TestFreezeNotStarted(c *C) {
+	// Test that a not running container can't be frozen.
+	lc := golxc.New("golxc")
+	c.Assert(lc.IsConstructed(), Equals, false)
+	c.Assert(lc.Create("ubuntu"), IsNil)
+	defer func() {
+		c.Assert(lc.Destroy(), IsNil)
+	}()
+	c.Assert(lc.Freeze(), ErrorMatches, "container .* is not running")
+}
+
+func (l *LXCSuite) TestFreezeNotCreated(c *C) {
+	// Test that a non-existing container can't be frozen.
+	lc := golxc.New("golxc")
+	c.Assert(lc.IsConstructed(), Equals, false)
+	c.Assert(lc.Freeze(), ErrorMatches, "container .* is not yet created")
+}
+
+func (l *LXCSuite) TestUnfreezeNotCreated(c *C) {
+	// Test that a non-existing container can't be unfrozen.
+	lc := golxc.New("golxc")
+	c.Assert(lc.IsConstructed(), Equals, false)
+	c.Assert(lc.Unfreeze(), ErrorMatches, "container .* is not yet created")
+}
+
+func (l *LXCSuite) TestUnfreezeNotFrozen(c *C) {
+	// Test that a running container can't be unfrozen.
+	lc := golxc.New("golxc")
+	c.Assert(lc.IsConstructed(), Equals, false)
+	c.Assert(lc.Create("ubuntu"), IsNil)
+	defer func() {
+		c.Assert(lc.Destroy(), IsNil)
+	}()
+	c.Assert(lc.Start("", ""), IsNil)
+	defer func() {
+		c.Assert(lc.Stop(), IsNil)
+	}()
+	c.Assert(lc.Unfreeze(), ErrorMatches, "container .* is not frozen")
+}

=== added file 'golxc_test.sh'
--- golxc_test.sh	1970-01-01 00:00:00 +0000
+++ golxc_test.sh	2012-11-23 13:49:20 +0000
@@ -0,0 +1,8 @@
+#!/bin/sh
+sudo sh -c "
+    export GOMAXPROCS=\"$GOMAXPROCS\"
+    export GOPATH=\"$GOPATH\"
+    export GOROOT=\"$GOROOT\"
+    export PATH=\"$PATH\"
+    go test $*
+"
\ No newline at end of file