Merge remote-tracking branch 'dotcloud/master' into dhrp/docs

AUTHORS

@@ -7,6 +7,7 @@ Caleb Spare <cespare@gmail.com>
 Charles Hooper <charles.hooper@dotcloud.com>
 Daniel Mizyrycki <daniel.mizyrycki@dotcloud.com>
 Daniel Robinson <gottagetmac@gmail.com>
+Dominik Honnef <dominik@honnef.co>
 Don Spaulding <donspauldingii@gmail.com>
 ezbercih <cem.ezberci@gmail.com>
 Frederick F. Kautz IV <fkautz@alumni.cmu.edu>

CONTRIBUTING.md

@@ -0,0 +1,69 @@
+# Contributing to Docker
+
+Want to hack on Docker? Awesome! There are instructions to get you
+started on the website: http://docker.io/gettingstarted.html
+
+They are probably not perfect, please let us know if anything feels
+wrong or incomplete.
+
+## Contribution guidelines
+
+### Pull requests are always welcome
+
+We are always thrilled to receive pull requests, and do our best to
+process them as fast as possible. Not sure if that typo is worth a pull
+request? Do it! We will appreciate it.
+
+If your pull request is not accepted on the first try, don't be
+discouraged! If there's a problem with the implementation, hopefully you
+received feedback on what to improve.
+
+We're trying very hard to keep Docker lean and focused. We don't want it
+to do everything for everybody. This means that we might decide against
+incorporating a new feature. However, there might be a way to implement
+that feature *on top of* docker.
+
+### Discuss your design on the mailing list
+
+We recommend discussing your plans [on the mailing
+list](https://groups.google.com/forum/?fromgroups#!forum/docker-club)
+before starting to code - especially for more ambitious contributions.
+This gives other contributors a chance to point you in the right
+direction, give feedback on your design, and maybe point out if someone
+else is working on the same thing.
+
+### Create issues...
+
+Any significant improvement should be documented as [a github
+issue](https://github.com/dotcloud/docker/issues) before anybody
+starts working on it.
+
+### ...but check for existing issues first!
+
+Please take a moment to check that an issue doesn't already exist
+documenting your bug report or improvement proposal. If it does, it
+never hurts to add a quick "+1" or "I have this problem too". This will
+help prioritize the most common problems and requests.
+
+### Conventions
+
+Fork the repo and make changes on your fork in a feature branch:
+
+- If it's a bugfix branch, name it XXX-something where XXX is the number of the issue
+- If it's a feature branch, create an enhancement issue to announce your intentions, and name it XXX-something where XXX is the number of the issue.
+
+Submit unit tests for your changes.  Golang has a great testing suite built
+in: use it! Take a look at existing tests for inspiration. Run the full test
+suite against your change and the master.
+
+Submit any relevant updates or additions to documentation.
+
+Add clean code:
+
+- Universally formatted code promotes ease of writing, reading, and maintenance.  We suggest using gofmt before committing your changes. There's a git pre-commit hook made for doing so.
+- curl -o .git/hooks/pre-commit https://raw.github.com/edsrzf/gofmt-git-hook/master/fmt-check && chmod +x .git/hooks/pre-commit
+
+Pull requests descriptions should be as clear as possible and include a
+referenced to all the issues that they address.
+
+Add your name to the AUTHORS file.

Makefile

@@ -41,3 +41,6 @@ endif
 
 test: all
 	@(cd $(DOCKER_DIR); sudo -E go test $(GO_OPTIONS))
+
+fmt:
+	@gofmt -s -l -w .

README.md

@@ -192,11 +192,10 @@ echo "Daemon received: $(docker logs $JOB)"
 Contributing to Docker
 ======================
 
-Want to hack on Docker? Awesome! There are instructions to get you started on the website: http://docker.io/documentation/contributing/contributing.html 
+Want to hack on Docker? Awesome! There are instructions to get you started on the website: http://docs.docker.io/en/latest/contributing/contributing/
 
 They are probably not perfect, please let us know if anything feels wrong or incomplete.
 
-### Pull requests are always welcome
 
 Note
 ----
@@ -206,26 +205,6 @@ Please find it under docs/sources/ and read more about it https://github.com/dot
 
 Please feel free to fix / update the documentation and send us pull requests. More tutorials are also welcome.
 
-### Discuss your design on the mailing list
-
-We recommend discussing your plans [on the mailing list](https://groups.google.com/forum/?fromgroups#!forum/docker-club) before starting to code - especially for more ambitious contributions. This gives other contributors a chance to point
-you in the right direction, give feedback on your design, and maybe point out if someone else is working on the same thing.
-
-### Create issues...
-
-Any significant improvement should be documented as [a github issue](https://github.com/dotcloud/docker/issues) before anybody starts working on it.
-
-### ...but check for existing issues first!
-
-Please take a moment to check that an issue doesn't already exist documenting your bug report or improvement proposal.
-If it does, it never hurts to add a quick "+1" or "I have this problem too". This will help prioritize the most common problems and requests.
-
-
-### Write tests
-
-Golang has a great testing suite built in: use it! Take a look at existing tests for inspiration.
-
-
 
 Setting up a dev environment
 ----------------------------

archive_test.go

@@ -6,19 +6,27 @@ import (
 	"os"
 	"os/exec"
 	"testing"
+	"time"
 )
 
 func TestCmdStreamLargeStderr(t *testing.T) {
-	// This test checks for deadlock; thus, the main failure mode of this test is deadlocking.
-	// FIXME implement a timeout to avoid blocking the whole test suite when this test fails
 	cmd := exec.Command("/bin/sh", "-c", "dd if=/dev/zero bs=1k count=1000 of=/dev/stderr; echo hello")
 	out, err := CmdStream(cmd)
 	if err != nil {
 		t.Fatalf("Failed to start command: " + err.Error())
 	}
-	_, err = io.Copy(ioutil.Discard, out)
-	if err != nil {
-		t.Fatalf("Command should not have failed (err=%s...)", err.Error()[:100])
+	errCh := make(chan error)
+	go func() {
+		_, err := io.Copy(ioutil.Discard, out)
+		errCh <- err
+	}()
+	select {
+	case err := <-errCh:
+		if err != nil {
+			t.Fatalf("Command should not have failed (err=%s...)", err.Error()[:100])
+		}
+	case <-time.After(5 * time.Second):
+		t.Fatalf("Command did not complete in 5 seconds; probable deadlock")
 	}
 }
 

auth/auth.go

@@ -1,6 +1,7 @@
 package auth
 
 import (
+	"bytes"
 	"encoding/base64"
 	"encoding/json"
 	"errors"
@@ -111,7 +112,7 @@ func Login(authConfig *AuthConfig) (string, error) {
 		return "", errors.New(errMsg)
 	}
 
-	b := strings.NewReader(string(jsonBody))
+	b := bytes.NewReader(jsonBody)
 	req1, err := http.Post(REGISTRY_SERVER+"/v1/users", "application/json; charset=utf-8", b)
 	if err != nil {
 		errMsg = fmt.Sprintf("Server Error: %s", err)
@@ -130,6 +131,7 @@ func Login(authConfig *AuthConfig) (string, error) {
 		status = "Account Created\n"
 		storeConfig = true
 	} else if reqStatusCode == 400 {
+		// FIXME: This should be 'exists', not 'exist'. Need to change on the server first.
 		if string(reqBody) == "Username or email already exist" {
 			client := &http.Client{}
 			req, err := http.NewRequest("GET", REGISTRY_SERVER+"/v1/users", nil)
@@ -151,11 +153,11 @@ func Login(authConfig *AuthConfig) (string, error) {
 				return "", errors.New(status)
 			}
 		} else {
-			status = fmt.Sprintf("Registration: %s", string(reqBody))
+			status = fmt.Sprintf("Registration: %s", reqBody)
 			return "", errors.New(status)
 		}
 	} else {
-		status = fmt.Sprintf("[%s] : %s", reqStatusCode, string(reqBody))
+		status = fmt.Sprintf("[%s] : %s", reqStatusCode, reqBody)
 		return "", errors.New(status)
 	}
 	if storeConfig {

commands.go

@@ -3,7 +3,6 @@ package docker
 import (
 	"bytes"
 	"encoding/json"
-	"errors"
 	"fmt"
 	"github.com/dotcloud/docker/auth"
 	"github.com/dotcloud/docker/rcli"
@@ -17,9 +16,10 @@ import (
 	"sync"
 	"text/tabwriter"
 	"time"
+	"unicode"
 )
 
-const VERSION = "0.1.0"
+const VERSION = "0.1.1"
 
 func (srv *Server) Name() string {
 	return "docker"
@@ -28,7 +28,7 @@ func (srv *Server) Name() string {
 // FIXME: Stop violating DRY by repeating usage here and in Subcmd declarations
 func (srv *Server) Help() string {
 	help := "Usage: docker COMMAND [arg...]\n\nA self-sufficient runtime for linux containers.\n\nCommands:\n"
-	for _, cmd := range [][]interface{}{
+	for _, cmd := range [][]string{
 		{"attach", "Attach to a running container"},
 		{"commit", "Create a new image from a container's changes"},
 		{"diff", "Inspect changes on a container's filesystem"},
@@ -62,29 +62,80 @@ func (srv *Server) Help() string {
 
 // 'docker login': login / register a user to registry service.
 func (srv *Server) CmdLogin(stdin io.ReadCloser, stdout io.Writer, args ...string) error {
+	// Read a line on raw terminal with support for simple backspace
+	// sequences and echo.
+	//
+	// This function is necessary because the login command must be done in a
+	// raw terminal for two reasons:
+	// - we have to read a password (without echoing it);
+	// - the rcli "protocol" only supports cannonical and raw modes and you
+	//   can't tune it once the command as been started.
+	var readStringOnRawTerminal = func(stdin io.Reader, stdout io.Writer, echo bool) string {
+		char := make([]byte, 1)
+		buffer := make([]byte, 64)
+		var i = 0
+		for i < len(buffer) {
+			n, err := stdin.Read(char)
+			if n > 0 {
+				if char[0] == '\r' || char[0] == '\n' {
+					stdout.Write([]byte{'\n'})
+					break
+				} else if char[0] == 127 || char[0] == '\b' {
+					if i > 0 {
+						if echo {
+							stdout.Write([]byte{'\b', ' ', '\b'})
+						}
+						i--
+					}
+				} else if !unicode.IsSpace(rune(char[0])) &&
+					!unicode.IsControl(rune(char[0])) {
+					if echo {
+						stdout.Write(char)
+					}
+					buffer[i] = char[0]
+					i++
+				}
+			}
+			if err != nil {
+				if err != io.EOF {
+					fmt.Fprint(stdout, "Read error: %v\n", err)
+				}
+				break
+			}
+		}
+		return string(buffer[:i])
+	}
+	var readAndEchoString = func(stdin io.Reader, stdout io.Writer) string {
+		return readStringOnRawTerminal(stdin, stdout, true)
+	}
+	var readString = func(stdin io.Reader, stdout io.Writer) string {
+		return readStringOnRawTerminal(stdin, stdout, false)
+	}
+
 	cmd := rcli.Subcmd(stdout, "login", "", "Register or Login to the docker registry server")
 	if err := cmd.Parse(args); err != nil {
 		return nil
 	}
+
 	var username string
 	var password string
 	var email string
 
 	fmt.Fprint(stdout, "Username (", srv.runtime.authConfig.Username, "): ")
-	fmt.Fscanf(stdin, "%s", &username)
+	username = readAndEchoString(stdin, stdout)
 	if username == "" {
 		username = srv.runtime.authConfig.Username
 	}
 	if username != srv.runtime.authConfig.Username {
 		fmt.Fprint(stdout, "Password: ")
-		fmt.Fscanf(stdin, "%s", &password)
+		password = readString(stdin, stdout)
 
 		if password == "" {
-			return errors.New("Error : Password Required\n")
+			return fmt.Errorf("Error : Password Required")
 		}
 
 		fmt.Fprint(stdout, "Email (", srv.runtime.authConfig.Email, "): ")
-		fmt.Fscanf(stdin, "%s", &email)
+		email = readAndEchoString(stdin, stdout)
 		if email == "" {
 			email = srv.runtime.authConfig.Email
 		}
@@ -95,12 +146,12 @@ func (srv *Server) CmdLogin(stdin io.ReadCloser, stdout io.Writer, args ...strin
 	newAuthConfig := auth.NewAuthConfig(username, password, email, srv.runtime.root)
 	status, err := auth.Login(newAuthConfig)
 	if err != nil {
-		fmt.Fprintf(stdout, "Error : %s\n", err)
+		fmt.Fprintln(stdout, "Error:", err)
 	} else {
 		srv.runtime.authConfig = newAuthConfig
 	}
 	if status != "" {
-		fmt.Fprintf(stdout, status)
+		fmt.Fprint(stdout, status)
 	}
 	return nil
 }
@@ -119,7 +170,7 @@ func (srv *Server) CmdWait(stdin io.ReadCloser, stdout io.Writer, args ...string
 		if container := srv.runtime.Get(name); container != nil {
 			fmt.Fprintln(stdout, container.Wait())
 		} else {
-			return errors.New("No such container: " + name)
+			return fmt.Errorf("No such container: %s", name)
 		}
 	}
 	return nil
@@ -152,6 +203,12 @@ func (srv *Server) CmdInfo(stdin io.ReadCloser, stdout io.Writer, args ...string
 		len(srv.runtime.List()),
 		VERSION,
 		imgcount)
+
+	if !rcli.DEBUG_FLAG {
+		return nil
+	}
+	fmt.Fprintln(stdout, "debug mode enabled")
+	fmt.Fprintf(stdout, "fds: %d\ngoroutines: %d\n", getTotalUsedFds(), runtime.NumGoroutine())
 	return nil
 }
 
@@ -169,9 +226,9 @@ func (srv *Server) CmdStop(stdin io.ReadCloser, stdout io.Writer, args ...string
 			if err := container.Stop(); err != nil {
 				return err
 			}
-			fmt.Fprintln(stdout, container.Id)
+			fmt.Fprintln(stdout, container.ShortId())
 		} else {
-			return errors.New("No such container: " + name)
+			return fmt.Errorf("No such container: %s", name)
 		}
 	}
 	return nil
@@ -191,9 +248,9 @@ func (srv *Server) CmdRestart(stdin io.ReadCloser, stdout io.Writer, args ...str
 			if err := container.Restart(); err != nil {
 				return err
 			}
-			fmt.Fprintln(stdout, container.Id)
+			fmt.Fprintln(stdout, container.ShortId())
 		} else {
-			return errors.New("No such container: " + name)
+			return fmt.Errorf("No such container: %s", name)
 		}
 	}
 	return nil
@@ -213,9 +270,9 @@ func (srv *Server) CmdStart(stdin io.ReadCloser, stdout io.Writer, args ...strin
 			if err := container.Start(); err != nil {
 				return err
 			}
-			fmt.Fprintln(stdout, container.Id)
+			fmt.Fprintln(stdout, container.ShortId())
 		} else {
-			return errors.New("No such container: " + name)
+			return fmt.Errorf("No such container: %s", name)
 		}
 	}
 	return nil
@@ -268,7 +325,7 @@ func (srv *Server) CmdPort(stdin io.ReadCloser, stdout io.Writer, args ...string
 	name := cmd.Arg(0)
 	privatePort := cmd.Arg(1)
 	if container := srv.runtime.Get(name); container == nil {
-		return errors.New("No such container: " + name)
+		return fmt.Errorf("No such container: %s", name)
 	} else {
 		if frontend, exists := container.NetworkSettings.PortMapping[privatePort]; !exists {
 			return fmt.Errorf("No private port '%s' allocated on %s", privatePort, name)
@@ -312,10 +369,10 @@ func (srv *Server) CmdHistory(stdin io.ReadCloser, stdout io.Writer, args ...str
 	}
 	w := tabwriter.NewWriter(stdout, 20, 1, 3, ' ', 0)
 	defer w.Flush()
-	fmt.Fprintf(w, "ID\tCREATED\tCREATED BY\n")
+	fmt.Fprintln(w, "ID\tCREATED\tCREATED BY")
 	return image.WalkHistory(func(img *Image) error {
 		fmt.Fprintf(w, "%s\t%s\t%s\n",
-			srv.runtime.repositories.ImageName(img.Id),
+			srv.runtime.repositories.ImageName(img.ShortId()),
 			HumanDuration(time.Now().Sub(img.Created))+" ago",
 			strings.Join(img.ContainerConfig.Cmd, " "),
 		)
@@ -331,7 +388,7 @@ func (srv *Server) CmdRm(stdin io.ReadCloser, stdout io.Writer, args ...string)
 	for _, name := range cmd.Args() {
 		container := srv.runtime.Get(name)
 		if container == nil {
-			return errors.New("No such container: " + name)
+			return fmt.Errorf("No such container: %s", name)
 		}
 		if err := srv.runtime.Destroy(container); err != nil {
 			fmt.Fprintln(stdout, "Error destroying container "+name+": "+err.Error())
@@ -349,7 +406,7 @@ func (srv *Server) CmdKill(stdin io.ReadCloser, stdout io.Writer, args ...string
 	for _, name := range cmd.Args() {
 		container := srv.runtime.Get(name)
 		if container == nil {
-			return errors.New("No such container: " + name)
+			return fmt.Errorf("No such container: %s", name)
 		}
 		if err := container.Kill(); err != nil {
 			fmt.Fprintln(stdout, "Error killing container "+name+": "+err.Error())
@@ -368,7 +425,7 @@ func (srv *Server) CmdImport(stdin io.ReadCloser, stdout io.Writer, args ...stri
 	}
 	src := cmd.Arg(0)
 	if src == "" {
-		return errors.New("Not enough arguments")
+		return fmt.Errorf("Not enough arguments")
 	} else if src == "-" {
 		archive = stdin
 	} else {
@@ -381,7 +438,7 @@ func (srv *Server) CmdImport(stdin io.ReadCloser, stdout io.Writer, args ...stri
 			u.Host = src
 			u.Path = ""
 		}
-		fmt.Fprintf(stdout, "Downloading from %s\n", u.String())
+		fmt.Fprintln(stdout, "Downloading from", u)
 		// Download with curl (pretty progress bar)
 		// If curl is not available, fallback to http.Get()
 		resp, err = Download(u.String(), stdout)
@@ -401,7 +458,7 @@ func (srv *Server) CmdImport(stdin io.ReadCloser, stdout io.Writer, args ...stri
 			return err
 		}
 	}
-	fmt.Fprintln(stdout, img.Id)
+	fmt.Fprintln(stdout, img.ShortId())
 	return nil
 }
 
@@ -507,7 +564,7 @@ func (srv *Server) CmdImages(stdin io.ReadCloser, stdout io.Writer, args ...stri
 	}
 	w := tabwriter.NewWriter(stdout, 20, 1, 3, ' ', 0)
 	if !*quiet {
-		fmt.Fprintf(w, "REPOSITORY\tTAG\tID\tCREATED\tPARENT\n")
+		fmt.Fprintln(w, "REPOSITORY\tTAG\tID\tCREATED\tPARENT")
 	}
 	var allImages map[string]*Image
 	var err error
@@ -534,7 +591,7 @@ func (srv *Server) CmdImages(stdin io.ReadCloser, stdout io.Writer, args ...stri
 				for idx, field := range []string{
 					/* REPOSITORY */ name,
 					/* TAG */ tag,
-					/* ID */ id,
+					/* ID */ TruncateId(id),
 					/* CREATED */ HumanDuration(time.Now().Sub(image.Created)) + " ago",
 					/* PARENT */ srv.runtime.repositories.ImageName(image.Parent),
 				} {
@@ -546,7 +603,7 @@ func (srv *Server) CmdImages(stdin io.ReadCloser, stdout io.Writer, args ...stri
 				}
 				w.Write([]byte{'\n'})
 			} else {
-				stdout.Write([]byte(image.Id + "\n"))
+				stdout.Write([]byte(image.ShortId() + "\n"))
 			}
 		}
 	}
@@ -557,7 +614,7 @@ func (srv *Server) CmdImages(stdin io.ReadCloser, stdout io.Writer, args ...stri
 				for idx, field := range []string{
 					/* REPOSITORY */ "<none>",
 					/* TAG */ "<none>",
-					/* ID */ id,
+					/* ID */ TruncateId(id),
 					/* CREATED */ HumanDuration(time.Now().Sub(image.Created)) + " ago",
 					/* PARENT */ srv.runtime.repositories.ImageName(image.Parent),
 				} {
@@ -569,7 +626,7 @@ func (srv *Server) CmdImages(stdin io.ReadCloser, stdout io.Writer, args ...stri
 				}
 				w.Write([]byte{'\n'})
 			} else {
-				stdout.Write([]byte(image.Id + "\n"))
+				stdout.Write([]byte(image.ShortId() + "\n"))
 			}
 		}
 	}
@@ -590,7 +647,7 @@ func (srv *Server) CmdPs(stdin io.ReadCloser, stdout io.Writer, args ...string)
 	}
 	w := tabwriter.NewWriter(stdout, 12, 1, 3, ' ', 0)
 	if !*quiet {
-		fmt.Fprintf(w, "ID\tIMAGE\tCOMMAND\tCREATED\tSTATUS\tCOMMENT\n")
+		fmt.Fprintln(w, "ID\tIMAGE\tCOMMAND\tCREATED\tSTATUS\tCOMMENT")
 	}
 	for _, container := range srv.runtime.List() {
 		if !container.State.Running && !*flAll {
@@ -602,7 +659,7 @@ func (srv *Server) CmdPs(stdin io.ReadCloser, stdout io.Writer, args ...string)
 				command = Trunc(command, 20)
 			}
 			for idx, field := range []string{
-				/* ID */ container.Id,
+				/* ID */ container.ShortId(),
 				/* IMAGE */ srv.runtime.repositories.ImageName(container.Image),
 				/* COMMAND */ command,
 				/* CREATED */ HumanDuration(time.Now().Sub(container.Created)) + " ago",
@@ -617,7 +674,7 @@ func (srv *Server) CmdPs(stdin io.ReadCloser, stdout io.Writer, args ...string)
 			}
 			w.Write([]byte{'\n'})
 		} else {
-			stdout.Write([]byte(container.Id + "\n"))
+			stdout.Write([]byte(container.ShortId() + "\n"))
 		}
 	}
 	if !*quiet {
@@ -643,7 +700,7 @@ func (srv *Server) CmdCommit(stdin io.ReadCloser, stdout io.Writer, args ...stri
 	if err != nil {
 		return err
 	}
-	fmt.Fprintln(stdout, img.Id)
+	fmt.Fprintln(stdout, img.ShortId())
 	return nil
 }
 
@@ -666,7 +723,7 @@ func (srv *Server) CmdExport(stdin io.ReadCloser, stdout io.Writer, args ...stri
 		}
 		return nil
 	}
-	return errors.New("No such container: " + name)
+	return fmt.Errorf("No such container: %s", name)
 }
 
 func (srv *Server) CmdDiff(stdin io.ReadCloser, stdout io.Writer, args ...string) error {
@@ -677,10 +734,10 @@ func (srv *Server) CmdDiff(stdin io.ReadCloser, stdout io.Writer, args ...string
 		return nil
 	}
 	if cmd.NArg() < 1 {
-		return errors.New("Not enough arguments")
+		return fmt.Errorf("Not enough arguments")
 	}
 	if container := srv.runtime.Get(cmd.Arg(0)); container == nil {
-		return errors.New("No such container")
+		return fmt.Errorf("No such container")
 	} else {
 		changes, err := container.Changes()
 		if err != nil {
@@ -722,14 +779,11 @@ func (srv *Server) CmdLogs(stdin io.ReadCloser, stdout io.Writer, args ...string
 		}
 		return nil
 	}
-	return errors.New("No such container: " + cmd.Arg(0))
+	return fmt.Errorf("No such container: %s", cmd.Arg(0))
 }
 
 func (srv *Server) CmdAttach(stdin io.ReadCloser, stdout io.Writer, args ...string) error {
-	cmd := rcli.Subcmd(stdout, "attach", "[OPTIONS]", "Attach to a running container")
-	flStdin := cmd.Bool("i", false, "Attach to stdin")
-	flStdout := cmd.Bool("o", true, "Attach to stdout")
-	flStderr := cmd.Bool("e", true, "Attach to stderr")
+	cmd := rcli.Subcmd(stdout, "attach", "CONTAINER", "Attach to a running container")
 	if err := cmd.Parse(args); err != nil {
 		return nil
 	}
@@ -740,33 +794,56 @@ func (srv *Server) CmdAttach(stdin io.ReadCloser, stdout io.Writer, args ...stri
 	name := cmd.Arg(0)
 	container := srv.runtime.Get(name)
 	if container == nil {
-		return errors.New("No such container: " + name)
+		return fmt.Errorf("No such container: %s", name)
 	}
-	var wg sync.WaitGroup
-	if *flStdin {
-		cStdin, err := container.StdinPipe()
-		if err != nil {
-			return err
-		}
-		wg.Add(1)
-		go func() { io.Copy(cStdin, stdin); wg.Add(-1) }()
+
+	cStdout, err := container.StdoutPipe()
+	if err != nil {
+		return err
 	}
-	if *flStdout {
-		cStdout, err := container.StdoutPipe()
-		if err != nil {
-			return err
-		}
-		wg.Add(1)
-		go func() { io.Copy(stdout, cStdout); wg.Add(-1) }()
+	cStderr, err := container.StderrPipe()
+	if err != nil {
+		return err
 	}
-	if *flStderr {
-		cStderr, err := container.StderrPipe()
+
+	var wg sync.WaitGroup
+	if container.Config.OpenStdin {
+		cStdin, err := container.StdinPipe()
 		if err != nil {
 			return err
 		}
 		wg.Add(1)
-		go func() { io.Copy(stdout, cStderr); wg.Add(-1) }()
-	}
+		go func() {
+			Debugf("Begin stdin pipe [attach]")
+			io.Copy(cStdin, stdin)
+
+			// When stdin get closed, it means the client has been detached
+			// Make sure all pipes are closed.
+			if err := cStdout.Close(); err != nil {
+				Debugf("Error closing stdin pipe: %s", err)
+			}
+			if err := cStderr.Close(); err != nil {
+				Debugf("Error closing stderr pipe: %s", err)
+			}
+
+			wg.Add(-1)
+			Debugf("End of stdin pipe [attach]")
+		}()
+	}
+	wg.Add(1)
+	go func() {
+		Debugf("Begin stdout pipe [attach]")
+		io.Copy(stdout, cStdout)
+		wg.Add(-1)
+		Debugf("End of stdout pipe [attach]")
+	}()
+	wg.Add(1)
+	go func() {
+		Debugf("Begin stderr pipe [attach]")
+		io.Copy(stdout, cStderr)
+		wg.Add(-1)
+		Debugf("End of stderr pipe [attach]")
+	}()
 	wg.Wait()
 	return nil
 }
@@ -889,7 +966,7 @@ func (srv *Server) CmdRun(stdin io.ReadCloser, stdout io.Writer, args ...string)
 		if err := container.Start(); err != nil {
 			return err
 		}
-		fmt.Fprintln(stdout, container.Id)
+		fmt.Fprintln(stdout, container.ShortId())
 	}
 	return nil
 }

commands_test.go

@@ -0,0 +1,168 @@
+package docker
+
+import (
+	"bufio"
+	"fmt"
+	"io"
+	"strings"
+	"testing"
+	"time"
+)
+
+func closeWrap(args ...io.Closer) error {
+	e := false
+	ret := fmt.Errorf("Error closing elements")
+	for _, c := range args {
+		if err := c.Close(); err != nil {
+			e = true
+			ret = fmt.Errorf("%s\n%s", ret, err)
+		}
+	}
+	if e {
+		return ret
+	}
+	return nil
+}
+
+func setTimeout(t *testing.T, msg string, d time.Duration, f func()) {
+	c := make(chan bool)
+
+	// Make sure we are not too long
+	go func() {
+		time.Sleep(d)
+		c <- true
+	}()
+	go func() {
+		f()
+		c <- false
+	}()
+	if <-c {
+		t.Fatal(msg)
+	}
+}
+
+func assertPipe(input, output string, r io.Reader, w io.Writer, count int) error {
+	for i := 0; i < count; i++ {
+		if _, err := w.Write([]byte(input)); err != nil {
+			return err
+		}
+		o, err := bufio.NewReader(r).ReadString('\n')
+		if err != nil {
+			return err
+		}
+		if strings.Trim(o, " \r\n") != output {
+			return fmt.Errorf("Unexpected output. Expected [%s], received [%s]", output, o)
+		}
+	}
+	return nil
+}
+
+// Expected behaviour: the process dies when the client disconnects
+func TestRunDisconnect(t *testing.T) {
+	runtime, err := newTestRuntime()
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer nuke(runtime)
+
+	srv := &Server{runtime: runtime}
+
+	stdin, stdinPipe := io.Pipe()
+	stdout, stdoutPipe := io.Pipe()
+	c1 := make(chan struct{})
+	go func() {
+		if err := srv.CmdRun(stdin, stdoutPipe, "-i", GetTestImage(runtime).Id, "/bin/cat"); err != nil {
+			t.Fatal(err)
+		}
+		close(c1)
+	}()
+
+	setTimeout(t, "Read/Write assertion timed out", 2*time.Second, func() {
+		if err := assertPipe("hello\n", "hello", stdout, stdinPipe, 15); err != nil {
+			t.Fatal(err)
+		}
+	})
+
+	// Close pipes (simulate disconnect)
+	if err := closeWrap(stdin, stdinPipe, stdout, stdoutPipe); err != nil {
+		t.Fatal(err)
+	}
+
+	// as the pipes are close, we expect the process to die,
+	// therefore CmdRun to unblock. Wait for CmdRun
+	setTimeout(t, "Waiting for CmdRun timed out", 2*time.Second, func() {
+		<-c1
+	})
+
+	// Check the status of the container
+	container := runtime.containers.Back().Value.(*Container)
+	if container.State.Running {
+		t.Fatalf("/bin/cat is still running after closing stdin")
+	}
+}
+
+// Expected behaviour, the process stays alive when the client disconnects
+func TestAttachDisconnect(t *testing.T) {
+	runtime, err := newTestRuntime()
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer nuke(runtime)
+
+	srv := &Server{runtime: runtime}
+
+	container, err := runtime.Create(
+		&Config{
+			Image:     GetTestImage(runtime).Id,
+			Memory:    33554432,
+			Cmd:       []string{"/bin/cat"},
+			OpenStdin: true,
+		},
+	)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer runtime.Destroy(container)
+
+	// Start the process
+	if err := container.Start(); err != nil {
+		t.Fatal(err)
+	}
+
+	stdin, stdinPipe := io.Pipe()
+	stdout, stdoutPipe := io.Pipe()
+
+	// Attach to it
+	c1 := make(chan struct{})
+	go func() {
+		if err := srv.CmdAttach(stdin, stdoutPipe, container.Id); err != nil {
+			t.Fatal(err)
+		}
+		close(c1)
+	}()
+
+	setTimeout(t, "First read/write assertion timed out", 2*time.Second, func() {
+		if err := assertPipe("hello\n", "hello", stdout, stdinPipe, 15); err != nil {
+			t.Fatal(err)
+		}
+	})
+	// Close pipes (client disconnects)
+	if err := closeWrap(stdin, stdinPipe, stdout, stdoutPipe); err != nil {
+		t.Fatal(err)
+	}
+
+	// Wait for attach to finish, the client disconnected, therefore, Attach finished his job
+	setTimeout(t, "Waiting for CmdAttach timed out", 2*time.Second, func() {
+		<-c1
+	})
+	// We closed stdin, expect /bin/cat to still be running
+	// Wait a little bit to make sure container.monitor() did his thing
+	err = container.WaitTimeout(500 * time.Millisecond)
+	if err == nil || !container.State.Running {
+		t.Fatalf("/bin/cat is not running after closing stdin")
+	}
+
+	// Try to avoid the timeoout in destroy. Best effort, don't check error
+	cStdin, _ := container.StdinPipe()
+	cStdin.Close()
+}

container.go

@@ -2,7 +2,6 @@ package docker
 
 import (
 	"encoding/json"
-	"errors"
 	"fmt"
 	"github.com/dotcloud/docker/rcli"
 	"github.com/kr/pty"
@@ -41,9 +40,11 @@ type Container struct {
 	stdin       io.ReadCloser
 	stdinPipe   io.WriteCloser
 
-	stdoutLog *os.File
-	stderrLog *os.File
-	runtime   *Runtime
+	ptyStdinMaster  io.Closer
+	ptyStdoutMaster io.Closer
+	ptyStderrMaster io.Closer
+
+	runtime *Runtime
 }
 
 type Config struct {
@@ -154,39 +155,49 @@ func (container *Container) startPty() error {
 	if err != nil {
 		return err
 	}
+	container.ptyStdoutMaster = stdoutMaster
 	container.cmd.Stdout = stdoutSlave
 
 	stderrMaster, stderrSlave, err := pty.Open()
 	if err != nil {
 		return err
 	}
+	container.ptyStderrMaster = stderrMaster
 	container.cmd.Stderr = stderrSlave
 
 	// Copy the PTYs to our broadcasters
 	go func() {
 		defer container.stdout.Close()
+		Debugf("[startPty] Begin of stdout pipe")
 		io.Copy(container.stdout, stdoutMaster)
+		Debugf("[startPty] End of stdout pipe")
 	}()
 
 	go func() {
 		defer container.stderr.Close()
+		Debugf("[startPty] Begin of stderr pipe")
 		io.Copy(container.stderr, stderrMaster)
+		Debugf("[startPty] End of stderr pipe")
 	}()
 
 	// stdin
 	var stdinSlave io.ReadCloser
 	if container.Config.OpenStdin {
-		stdinMaster, stdinSlave, err := pty.Open()
+		var stdinMaster io.WriteCloser
+		stdinMaster, stdinSlave, err = pty.Open()
 		if err != nil {
 			return err
 		}
+		container.ptyStdinMaster = stdinMaster
 		container.cmd.Stdin = stdinSlave
 		// FIXME: The following appears to be broken.
 		// "cannot set terminal process group (-1): Inappropriate ioctl for device"
 		// container.cmd.SysProcAttr = &syscall.SysProcAttr{Setctty: true, Setsid: true}
 		go func() {
 			defer container.stdin.Close()
+			Debugf("[startPty] Begin of stdin pipe")
 			io.Copy(stdinMaster, container.stdin)
+			Debugf("[startPty] End of stdin pipe")
 		}()
 	}
 	if err := container.cmd.Start(); err != nil {
@@ -210,13 +221,18 @@ func (container *Container) start() error {
 		}
 		go func() {
 			defer stdin.Close()
+			Debugf("Begin of stdin pipe [start]")
 			io.Copy(stdin, container.stdin)
+			Debugf("End of stdin pipe [start]")
 		}()
 	}
 	return container.cmd.Start()
 }
 
 func (container *Container) Start() error {
+	if container.State.Running {
+		return fmt.Errorf("The container %s is already running.", container.Id)
+	}
 	if err := container.EnsureMounted(); err != nil {
 		return err
 	}
@@ -256,6 +272,14 @@ func (container *Container) Start() error {
 		container.Config.Env...,
 	)
 
+	// Setup logging of stdout and stderr to disk
+	if err := container.runtime.LogToDisk(container.stdout, container.logPath("stdout")); err != nil {
+		return err
+	}
+	if err := container.runtime.LogToDisk(container.stderr, container.logPath("stderr")); err != nil {
+		return err
+	}
+
 	var err error
 	if container.Config.Tty {
 		container.cmd.Env = append(
@@ -339,24 +363,53 @@ func (container *Container) allocateNetwork() error {
 	return nil
 }
 
-func (container *Container) releaseNetwork() error {
-	err := container.network.Release()
+func (container *Container) releaseNetwork() {
+	container.network.Release()
 	container.network = nil
 	container.NetworkSettings = &NetworkSettings{}
-	return err
 }
 
 func (container *Container) monitor() {
 	// Wait for the program to exit
-	container.cmd.Wait()
+	Debugf("Waiting for process")
+	if err := container.cmd.Wait(); err != nil {
+		// Discard the error as any signals or non 0 returns will generate an error
+		Debugf("%s: Process: %s", container.Id, err)
+	}
+	Debugf("Process finished")
+
 	exitCode := container.cmd.ProcessState.Sys().(syscall.WaitStatus).ExitStatus()
 
 	// Cleanup
-	if err := container.releaseNetwork(); err != nil {
-		log.Printf("%v: Failed to release network: %v", container.Id, err)
+	container.releaseNetwork()
+	if container.Config.OpenStdin {
+		if err := container.stdin.Close(); err != nil {
+			Debugf("%s: Error close stdin: %s", container.Id, err)
+		}
+	}
+	if err := container.stdout.Close(); err != nil {
+		Debugf("%s: Error close stdout: %s", container.Id, err)
+	}
+	if err := container.stderr.Close(); err != nil {
+		Debugf("%s: Error close stderr: %s", container.Id, err)
 	}
-	container.stdout.Close()
-	container.stderr.Close()
+
+	if container.ptyStdinMaster != nil {
+		if err := container.ptyStdinMaster.Close(); err != nil {
+			Debugf("%s: Error close pty stdin master: %s", container.Id, err)
+		}
+	}
+	if container.ptyStdoutMaster != nil {
+		if err := container.ptyStdoutMaster.Close(); err != nil {
+			Debugf("%s: Error close pty stdout master: %s", container.Id, err)
+		}
+	}
+	if container.ptyStderrMaster != nil {
+		if err := container.ptyStderrMaster.Close(); err != nil {
+			Debugf("%s: Error close pty stderr master: %s", container.Id, err)
+		}
+	}
+
 	if err := container.Unmount(); err != nil {
 		log.Printf("%v: Failed to umount filesystem: %v", container.Id, err)
 	}
@@ -368,7 +421,15 @@ func (container *Container) monitor() {
 
 	// Report status back
 	container.State.setStopped(exitCode)
-	container.ToDisk()
+	if err := container.ToDisk(); err != nil {
+		// FIXME: there is a race condition here which causes this to fail during the unit tests.
+		// If another goroutine was waiting for Wait() to return before removing the container's root
+		// from the filesystem... At this point it may already have done so.
+		// This is because State.setStopped() has already been called, and has caused Wait()
+		// to return.
+		// FIXME: why are we serializing running state to disk in the first place?
+		//log.Printf("%s: Failed to dump configuration to the disk: %s", container.Id, err)
+	}
 }
 
 func (container *Container) kill() error {
@@ -397,8 +458,8 @@ func (container *Container) Stop() error {
 
 	// 1. Send a SIGTERM
 	if output, err := exec.Command("lxc-kill", "-n", container.Id, "15").CombinedOutput(); err != nil {
-		log.Printf(string(output))
-		log.Printf("Failed to send SIGTERM to the process, force killing")
+		log.Print(string(output))
+		log.Print("Failed to send SIGTERM to the process, force killing")
 		if err := container.Kill(); err != nil {
 			return err
 		}
@@ -453,7 +514,7 @@ func (container *Container) WaitTimeout(timeout time.Duration) error {
 
 	select {
 	case <-time.After(timeout):
-		return errors.New("Timed Out")
+		return fmt.Errorf("Timed Out")
 	case <-done:
 		return nil
 	}
@@ -500,6 +561,14 @@ func (container *Container) Unmount() error {
 	return Unmount(container.RootfsPath())
 }
 
+// ShortId returns a shorthand version of the container's id for convenience.
+// A collision with other container shorthands is very unlikely, but possible.
+// In case of a collision a lookup with Runtime.Get() will fail, and the caller
+// will need to use a langer prefix, or the full-length container Id.
+func (container *Container) ShortId() string {
+	return TruncateId(container.Id)
+}
+
 func (container *Container) logPath(name string) string {
 	return path.Join(container.root, fmt.Sprintf("%s-%s.log", container.Id, name))
 }

container_test.go

@@ -39,6 +39,117 @@ func TestIdFormat(t *testing.T) {
 	}
 }
 
+func TestMultipleAttachRestart(t *testing.T) {
+	runtime, err := newTestRuntime()
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer nuke(runtime)
+	container, err := runtime.Create(
+		&Config{
+			Image: GetTestImage(runtime).Id,
+			Cmd: []string{"/bin/sh", "-c",
+				"i=1; while [ $i -le 5 ]; do i=`expr $i + 1`;  echo hello; done"},
+			Memory: 33554432,
+		},
+	)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer runtime.Destroy(container)
+
+	// Simulate 3 client attaching to the container and stop/restart
+
+	stdout1, err := container.StdoutPipe()
+	if err != nil {
+		t.Fatal(err)
+	}
+	stdout2, err := container.StdoutPipe()
+	if err != nil {
+		t.Fatal(err)
+	}
+	stdout3, err := container.StdoutPipe()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := container.Start(); err != nil {
+		t.Fatal(err)
+	}
+	l1, err := bufio.NewReader(stdout1).ReadString('\n')
+	if err != nil {
+		t.Fatal(err)
+	}
+	if strings.Trim(l1, " \r\n") != "hello" {
+		t.Fatalf("Unexpected output. Expected [%s], received [%s]", "hello", l1)
+	}
+	l2, err := bufio.NewReader(stdout2).ReadString('\n')
+	if err != nil {
+		t.Fatal(err)
+	}
+	if strings.Trim(l2, " \r\n") != "hello" {
+		t.Fatalf("Unexpected output. Expected [%s], received [%s]", "hello", l2)
+	}
+	l3, err := bufio.NewReader(stdout3).ReadString('\n')
+	if err != nil {
+		t.Fatal(err)
+	}
+	if strings.Trim(l3, " \r\n") != "hello" {
+		t.Fatalf("Unexpected output. Expected [%s], received [%s]", "hello", l3)
+	}
+
+	if err := container.Stop(); err != nil {
+		t.Fatal(err)
+	}
+
+	stdout1, err = container.StdoutPipe()
+	if err != nil {
+		t.Fatal(err)
+	}
+	stdout2, err = container.StdoutPipe()
+	if err != nil {
+		t.Fatal(err)
+	}
+	stdout3, err = container.StdoutPipe()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := container.Start(); err != nil {
+		t.Fatal(err)
+	}
+	timeout := make(chan bool)
+	go func() {
+		l1, err = bufio.NewReader(stdout1).ReadString('\n')
+		if err != nil {
+			t.Fatal(err)
+		}
+		if strings.Trim(l1, " \r\n") != "hello" {
+			t.Fatalf("Unexpected output. Expected [%s], received [%s]", "hello", l1)
+		}
+		l2, err = bufio.NewReader(stdout2).ReadString('\n')
+		if err != nil {
+			t.Fatal(err)
+		}
+		if strings.Trim(l2, " \r\n") != "hello" {
+			t.Fatalf("Unexpected output. Expected [%s], received [%s]", "hello", l2)
+		}
+		l3, err = bufio.NewReader(stdout3).ReadString('\n')
+		if err != nil {
+			t.Fatal(err)
+		}
+		if strings.Trim(l3, " \r\n") != "hello" {
+			t.Fatalf("Unexpected output. Expected [%s], received [%s]", "hello", l3)
+		}
+		timeout <- false
+	}()
+	go func() {
+		time.Sleep(3 * time.Second)
+		timeout <- true
+	}()
+	if <-timeout {
+		t.Fatalf("Timeout reading from the process")
+	}
+}
+
 func TestCommitRun(t *testing.T) {
 	runtime, err := newTestRuntime()
 	if err != nil {
@@ -89,20 +200,73 @@ func TestCommitRun(t *testing.T) {
 		t.Fatal(err)
 	}
 	defer runtime.Destroy(container2)
-
 	stdout, err := container2.StdoutPipe()
+	if err != nil {
+		t.Fatal(err)
+	}
 	stderr, err := container2.StderrPipe()
+	if err != nil {
+		t.Fatal(err)
+	}
 	if err := container2.Start(); err != nil {
 		t.Fatal(err)
 	}
 	container2.Wait()
 	output, err := ioutil.ReadAll(stdout)
+	if err != nil {
+		t.Fatal(err)
+	}
 	output2, err := ioutil.ReadAll(stderr)
-	stdout.Close()
-	stderr.Close()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := stdout.Close(); err != nil {
+		t.Fatal(err)
+	}
+	if err := stderr.Close(); err != nil {
+		t.Fatal(err)
+	}
 	if string(output) != "hello\n" {
-		t.Fatalf("\nout: %s\nerr: %s\n", string(output), string(output2))
+		t.Fatalf("Unexpected output. Expected %s, received: %s (err: %s)", "hello\n", output, output2)
+	}
+}
+
+func TestStart(t *testing.T) {
+	runtime, err := newTestRuntime()
+	if err != nil {
+		t.Fatal(err)
 	}
+	defer nuke(runtime)
+	container, err := runtime.Create(
+		&Config{
+			Image:     GetTestImage(runtime).Id,
+			Memory:    33554432,
+			Cmd:       []string{"/bin/cat"},
+			OpenStdin: true,
+		},
+	)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer runtime.Destroy(container)
+
+	if err := container.Start(); err != nil {
+		t.Fatal(err)
+	}
+
+	// Give some time to the process to start
+	container.WaitTimeout(500 * time.Millisecond)
+
+	if !container.State.Running {
+		t.Errorf("Container should be running")
+	}
+	if err := container.Start(); err == nil {
+		t.Fatalf("A running containter should be able to be started")
+	}
+
+	// Try to avoid the timeoout in destroy. Best effort, don't check error
+	cStdin, _ := container.StdinPipe()
+	cStdin.Close()
 }
 
 func TestRun(t *testing.T) {
@@ -208,11 +372,9 @@ func TestExitCode(t *testing.T) {
 	defer nuke(runtime)
 
 	trueContainer, err := runtime.Create(&Config{
-
 		Image: GetTestImage(runtime).Id,
 		Cmd:   []string{"/bin/true", ""},
-	},
-	)
+	})
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -220,12 +382,14 @@ func TestExitCode(t *testing.T) {
 	if err := trueContainer.Run(); err != nil {
 		t.Fatal(err)
 	}
+	if trueContainer.State.ExitCode != 0 {
+		t.Errorf("Unexpected exit code %d (expected 0)", trueContainer.State.ExitCode)
+	}
 
 	falseContainer, err := runtime.Create(&Config{
 		Image: GetTestImage(runtime).Id,
 		Cmd:   []string{"/bin/false", ""},
-	},
-	)
+	})
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -233,13 +397,8 @@ func TestExitCode(t *testing.T) {
 	if err := falseContainer.Run(); err != nil {
 		t.Fatal(err)
 	}
-
-	if trueContainer.State.ExitCode != 0 {
-		t.Errorf("Unexpected exit code %v", trueContainer.State.ExitCode)
-	}
-
 	if falseContainer.State.ExitCode != 1 {
-		t.Errorf("Unexpected exit code %v", falseContainer.State.ExitCode)
+		t.Errorf("Unexpected exit code %d (expected 1)", falseContainer.State.ExitCode)
 	}
 }
 
@@ -295,32 +454,62 @@ func TestRestartStdin(t *testing.T) {
 	defer runtime.Destroy(container)
 
 	stdin, err := container.StdinPipe()
+	if err != nil {
+		t.Fatal(err)
+	}
 	stdout, err := container.StdoutPipe()
+	if err != nil {
+		t.Fatal(err)
+	}
 	if err := container.Start(); err != nil {
 		t.Fatal(err)
 	}
-	io.WriteString(stdin, "hello world")
-	stdin.Close()
+	if _, err := io.WriteString(stdin, "hello world"); err != nil {
+		t.Fatal(err)
+	}
+	if err := stdin.Close(); err != nil {
+		t.Fatal(err)
+	}
 	container.Wait()
 	output, err := ioutil.ReadAll(stdout)
-	stdout.Close()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := stdout.Close(); err != nil {
+		t.Fatal(err)
+	}
 	if string(output) != "hello world" {
-		t.Fatal(string(output))
+		t.Fatalf("Unexpected output. Expected %s, received: %s", "hello world", string(output))
 	}
 
 	// Restart and try again
 	stdin, err = container.StdinPipe()
+	if err != nil {
+		t.Fatal(err)
+	}
 	stdout, err = container.StdoutPipe()
+	if err != nil {
+		t.Fatal(err)
+	}
 	if err := container.Start(); err != nil {
 		t.Fatal(err)
 	}
-	io.WriteString(stdin, "hello world #2")
-	stdin.Close()
+	if _, err := io.WriteString(stdin, "hello world #2"); err != nil {
+		t.Fatal(err)
+	}
+	if err := stdin.Close(); err != nil {
+		t.Fatal(err)
+	}
 	container.Wait()
 	output, err = ioutil.ReadAll(stdout)
-	stdout.Close()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := stdout.Close(); err != nil {
+		t.Fatal(err)
+	}
 	if string(output) != "hello world #2" {
-		t.Fatal(string(output))
+		t.Fatalf("Unexpected output. Expected %s, received: %s", "hello world #2", string(output))
 	}
 }
 
@@ -504,18 +693,31 @@ func TestStdin(t *testing.T) {
 	defer runtime.Destroy(container)
 
 	stdin, err := container.StdinPipe()
+	if err != nil {
+		t.Fatal(err)
+	}
 	stdout, err := container.StdoutPipe()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := container.Start(); err != nil {
+		t.Fatal(err)
+	}
 	defer stdin.Close()
 	defer stdout.Close()
-	if err := container.Start(); err != nil {
+	if _, err := io.WriteString(stdin, "hello world"); err != nil {
+		t.Fatal(err)
+	}
+	if err := stdin.Close(); err != nil {
 		t.Fatal(err)
 	}
-	io.WriteString(stdin, "hello world")
-	stdin.Close()
 	container.Wait()
 	output, err := ioutil.ReadAll(stdout)
+	if err != nil {
+		t.Fatal(err)
+	}
 	if string(output) != "hello world" {
-		t.Fatal(string(output))
+		t.Fatalf("Unexpected output. Expected %s, received: %s", "hello world", string(output))
 	}
 }
 
@@ -538,18 +740,31 @@ func TestTty(t *testing.T) {
 	defer runtime.Destroy(container)
 
 	stdin, err := container.StdinPipe()
+	if err != nil {
+		t.Fatal(err)
+	}
 	stdout, err := container.StdoutPipe()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := container.Start(); err != nil {
+		t.Fatal(err)
+	}
 	defer stdin.Close()
 	defer stdout.Close()
-	if err := container.Start(); err != nil {
+	if _, err := io.WriteString(stdin, "hello world"); err != nil {
+		t.Fatal(err)
+	}
+	if err := stdin.Close(); err != nil {
 		t.Fatal(err)
 	}
-	io.WriteString(stdin, "hello world")
-	stdin.Close()
 	container.Wait()
 	output, err := ioutil.ReadAll(stdout)
+	if err != nil {
+		t.Fatal(err)
+	}
 	if string(output) != "hello world" {
-		t.Fatal(string(output))
+		t.Fatalf("Unexpected output. Expected %s, received: %s", "hello world", string(output))
 	}
 }
 
@@ -568,6 +783,7 @@ func TestEnv(t *testing.T) {
 		t.Fatal(err)
 	}
 	defer runtime.Destroy(container)
+
 	stdout, err := container.StdoutPipe()
 	if err != nil {
 		t.Fatal(err)
@@ -673,7 +889,7 @@ func BenchmarkRunSequencial(b *testing.B) {
 			b.Fatal(err)
 		}
 		if string(output) != "foo" {
-			b.Fatalf("Unexecpted output: %v", string(output))
+			b.Fatalf("Unexpected output: %s", output)
 		}
 		if err := runtime.Destroy(container); err != nil {
 			b.Fatal(err)

contrib/mkimage-busybox.sh

@@ -35,6 +35,5 @@ do
     cp -a /dev/$X dev
 done
 
-tar -cf- . | docker put busybox
-docker run -i -a -u root busybox /bin/echo Success.
-
+tar -cf- . | docker import - busybox
+docker run -i -u root busybox /bin/echo Success.

docs/sources/contributing/contributing.rst

@@ -51,8 +51,26 @@ documenting your bug report or improvement proposal. If it does, it
 never hurts to add a quick "+1" or "I have this problem too". This will
 help prioritize the most common problems and requests.
 
-Write tests
+Conventions
 ~~~~~~~~~~~
 
-Golang has a great testing suite built in: use it! Take a look at
-existing tests for inspiration.
+Fork the repo and make changes on your fork in a feature branch:
+
+- If it's a bugfix branch, name it XXX-something where XXX is the number of the issue
+- If it's a feature branch, create an enhancement issue to announce your intentions, and name it XXX-something where XXX is the number of the issue.
+
+Submit unit tests for your changes.  Golang has a great testing suite built
+in: use it! Take a look at existing tests for inspiration. Run the full test
+suite against your change and the master.
+
+Submit any relevant updates or additions to documentation.
+
+Add clean code:
+
+- Universally formatted code promotes ease of writing, reading, and maintenance.  We suggest using gofmt before committing your changes. There's a git pre-commit hook made for doing so.
+- curl -o .git/hooks/pre-commit https://raw.github.com/edsrzf/gofmt-git-hook/master/fmt-check && chmod +x .git/hooks/pre-commit
+
+Pull requests descriptions should be as clear as possible and include a
+referenced to all the issues that they address.
+
+Add your name to the AUTHORS file.

graph.go

@@ -10,10 +10,14 @@ import (
 	"time"
 )
 
+// A Graph is a store for versioned filesystem images and the relationship between them.
 type Graph struct {
-	Root string
+	Root    string
+	idIndex *TruncIndex
 }
 
+// NewGraph instantiates a new graph at the given root path in the filesystem.
+// `root` will be created if it doesn't exist.
 func NewGraph(root string) (*Graph, error) {
 	abspath, err := filepath.Abs(root)
 	if err != nil {
@@ -23,9 +27,26 @@ func NewGraph(root string) (*Graph, error) {
 	if err := os.Mkdir(root, 0700); err != nil && !os.IsExist(err) {
 		return nil, err
 	}
-	return &Graph{
-		Root: abspath,
-	}, nil
+	graph := &Graph{
+		Root:    abspath,
+		idIndex: NewTruncIndex(),
+	}
+	if err := graph.restore(); err != nil {
+		return nil, err
+	}
+	return graph, nil
+}
+
+func (graph *Graph) restore() error {
+	dir, err := ioutil.ReadDir(graph.Root)
+	if err != nil {
+		return err
+	}
+	for _, v := range dir {
+		id := v.Name()
+		graph.idIndex.Add(id)
+	}
+	return nil
 }
 
 // FIXME: Implement error subclass instead of looking at the error text
@@ -34,6 +55,8 @@ func (graph *Graph) IsNotExist(err error) bool {
 	return err != nil && strings.Contains(err.Error(), "does not exist")
 }
 
+// Exists returns true if an image is registered at the given id.
+// If the image doesn't exist or if an error is encountered, false is returned.
 func (graph *Graph) Exists(id string) bool {
 	if _, err := graph.Get(id); err != nil {
 		return false
@@ -41,7 +64,12 @@ func (graph *Graph) Exists(id string) bool {
 	return true
 }
 
-func (graph *Graph) Get(id string) (*Image, error) {
+// Get returns the image with the given id, or an error if the image doesn't exist.
+func (graph *Graph) Get(name string) (*Image, error) {
+	id, err := graph.idIndex.Get(name)
+	if err != nil {
+		return nil, err
+	}
 	// FIXME: return nil when the image doesn't exist, instead of an error
 	img, err := LoadImage(graph.imageRoot(id))
 	if err != nil {
@@ -54,6 +82,7 @@ func (graph *Graph) Get(id string) (*Image, error) {
 	return img, nil
 }
 
+// Create creates a new image and registers it in the graph.
 func (graph *Graph) Create(layerData Archive, container *Container, comment string) (*Image, error) {
 	img := &Image{
 		Id:      GenerateId(),
@@ -71,6 +100,8 @@ func (graph *Graph) Create(layerData Archive, container *Container, comment stri
 	return img, nil
 }
 
+// Register imports a pre-existing image into the graph.
+// FIXME: pass img as first argument
 func (graph *Graph) Register(layerData Archive, img *Image) error {
 	if err := ValidateId(img.Id); err != nil {
 		return err
@@ -92,9 +123,11 @@ func (graph *Graph) Register(layerData Archive, img *Image) error {
 		return err
 	}
 	img.graph = graph
+	graph.idIndex.Add(img.Id)
 	return nil
 }
 
+// Mktemp creates a temporary sub-directory inside the graph's filesystem.
 func (graph *Graph) Mktemp(id string) (string, error) {
 	tmp, err := NewGraph(path.Join(graph.Root, ":tmp:"))
 	if err != nil {
@@ -106,12 +139,15 @@ func (graph *Graph) Mktemp(id string) (string, error) {
 	return tmp.imageRoot(id), nil
 }
 
+// Garbage returns the "garbage", a staging area for deleted images.
+// This allows images to be deleted atomically by os.Rename(), instead of
+// os.RemoveAll() which is prone to race conditions.
 func (graph *Graph) Garbage() (*Graph, error) {
 	return NewGraph(path.Join(graph.Root, ":garbage:"))
 }
 
-// Check if given error is "not empty"
-// Note: this is the way golang do it internally with os.IsNotExists
+// Check if given error is "not empty".
+// Note: this is the way golang does it internally with os.IsNotExists.
 func isNotEmpty(err error) bool {
 	switch pe := err.(type) {
 	case nil:
@@ -124,13 +160,21 @@ func isNotEmpty(err error) bool {
 	return strings.Contains(err.Error(), " not empty")
 }
 
-func (graph *Graph) Delete(id string) error {
+// Delete atomically removes an image from the graph.
+func (graph *Graph) Delete(name string) error {
+	id, err := graph.idIndex.Get(name)
+	if err != nil {
+		return err
+	}
 	garbage, err := graph.Garbage()
 	if err != nil {
 		return err
 	}
+	graph.idIndex.Delete(id)
 	err = os.Rename(graph.imageRoot(id), garbage.imageRoot(id))
 	if err != nil {
+		// FIXME: this introduces a race condition in Delete() if the image is already present
+		// in garbage. Let's store at random names in grabage instead.
 		if isNotEmpty(err) {
 			Debugf("The image %s is already present in garbage. Removing it.", id)
 			if err = os.RemoveAll(garbage.imageRoot(id)); err != nil {
@@ -150,14 +194,20 @@ func (graph *Graph) Delete(id string) error {
 	return nil
 }
 
+// Undelete moves an image back from the garbage to the main graph.
 func (graph *Graph) Undelete(id string) error {
 	garbage, err := graph.Garbage()
 	if err != nil {
 		return err
 	}
-	return os.Rename(garbage.imageRoot(id), graph.imageRoot(id))
+	if err := os.Rename(garbage.imageRoot(id), graph.imageRoot(id)); err != nil {
+		return err
+	}
+	graph.idIndex.Add(id)
+	return nil
 }
 
+// GarbageCollect definitely deletes all images moved to the garbage.
 func (graph *Graph) GarbageCollect() error {
 	garbage, err := graph.Garbage()
 	if err != nil {
@@ -166,6 +216,7 @@ func (graph *Graph) GarbageCollect() error {
 	return os.RemoveAll(garbage.Root)
 }
 
+// Map returns a list of all images in the graph, addressable by ID.
 func (graph *Graph) Map() (map[string]*Image, error) {
 	// FIXME: this should replace All()
 	all, err := graph.All()
@@ -179,6 +230,7 @@ func (graph *Graph) Map() (map[string]*Image, error) {
 	return images, nil
 }
 
+// All returns a list of all images in the graph.
 func (graph *Graph) All() ([]*Image, error) {
 	var images []*Image
 	err := graph.WalkAll(func(image *Image) {
@@ -187,6 +239,8 @@ func (graph *Graph) All() ([]*Image, error) {
 	return images, err
 }
 
+// WalkAll iterates over each image in the graph, and passes it to a handler.
+// The walking order is undetermined.
 func (graph *Graph) WalkAll(handler func(*Image)) error {
 	files, err := ioutil.ReadDir(graph.Root)
 	if err != nil {
@@ -203,6 +257,10 @@ func (graph *Graph) WalkAll(handler func(*Image)) error {
 	return nil
 }
 
+// ByParent returns a lookup table of images by their parent.
+// If an image of id ID has 3 children images, then the value for key ID
+// will be a list of 3 images.
+// If an image has no children, it will not have an entry in the table.
 func (graph *Graph) ByParent() (map[string][]*Image, error) {
 	byParent := make(map[string][]*Image)
 	err := graph.WalkAll(func(image *Image) {
@@ -219,6 +277,8 @@ func (graph *Graph) ByParent() (map[string][]*Image, error) {
 	return byParent, err
 }
 
+// Heads returns all heads in the graph, keyed by id.
+// A head is an image which is not the parent of another image in the graph.
 func (graph *Graph) Heads() (map[string]*Image, error) {
 	heads := make(map[string]*Image)
 	byParent, err := graph.ByParent()

graph_test.go

@@ -120,6 +120,29 @@ func TestMount(t *testing.T) {
 	}()
 }
 
+// Test that an image can be deleted by its shorthand prefix
+func TestDeletePrefix(t *testing.T) {
+	graph := tempGraph(t)
+	defer os.RemoveAll(graph.Root)
+	img := createTestImage(graph, t)
+	if err := graph.Delete(TruncateId(img.Id)); err != nil {
+		t.Fatal(err)
+	}
+	assertNImages(graph, t, 0)
+}
+
+func createTestImage(graph *Graph, t *testing.T) *Image {
+	archive, err := fakeTar()
+	if err != nil {
+		t.Fatal(err)
+	}
+	img, err := graph.Create(archive, nil, "Test image")
+	if err != nil {
+		t.Fatal(err)
+	}
+	return img
+}
+
 func TestDelete(t *testing.T) {
 	graph := tempGraph(t)
 	defer os.RemoveAll(graph.Root)

image.go

@@ -150,6 +150,10 @@ func (image *Image) Changes(rw string) ([]Change, error) {
 	return Changes(layers, rw)
 }
 
+func (image *Image) ShortId() string {
+	return TruncateId(image.Id)
+}
+
 func ValidateId(id string) error {
 	if id == "" {
 		return fmt.Errorf("Image id can't be empty")

network.go

@@ -1,7 +1,6 @@
 package docker
 
 import (
-	"bytes"
 	"encoding/binary"
 	"errors"
 	"fmt"
@@ -30,40 +29,25 @@ func networkRange(network *net.IPNet) (net.IP, net.IP) {
 }
 
 // Converts a 4 bytes IP into a 32 bit integer
-func ipToInt(ip net.IP) (int32, error) {
-	buf := bytes.NewBuffer(ip.To4())
-	var n int32
-	if err := binary.Read(buf, binary.BigEndian, &n); err != nil {
-		return 0, err
-	}
-	return n, nil
+func ipToInt(ip net.IP) int32 {
+	return int32(binary.BigEndian.Uint32(ip.To4()))
 }
 
 // Converts 32 bit integer into a 4 bytes IP address
-func intToIp(n int32) (net.IP, error) {
-	var buf bytes.Buffer
-	if err := binary.Write(&buf, binary.BigEndian, &n); err != nil {
-		return net.IP{}, err
-	}
-	ip := net.IPv4(0, 0, 0, 0).To4()
-	for i := 0; i < net.IPv4len; i++ {
-		ip[i] = buf.Bytes()[i]
-	}
-	return ip, nil
+func intToIp(n int32) net.IP {
+	b := make([]byte, 4)
+	binary.BigEndian.PutUint32(b, uint32(n))
+	return net.IP(b)
 }
 
 // Given a netmask, calculates the number of available hosts
-func networkSize(mask net.IPMask) (int32, error) {
+func networkSize(mask net.IPMask) int32 {
 	m := net.IPv4Mask(0, 0, 0, 0)
 	for i := 0; i < net.IPv4len; i++ {
 		m[i] = ^mask[i]
 	}
-	buf := bytes.NewBuffer(m)
-	var n int32
-	if err := binary.Read(buf, binary.BigEndian, &n); err != nil {
-		return 0, err
-	}
-	return n + 1, nil
+
+	return int32(binary.BigEndian.Uint32(m)) + 1
 }
 
 // Wrapper around the iptables command
@@ -211,66 +195,97 @@ func newPortAllocator(start, end int) (*PortAllocator, error) {
 
 // IP allocator: Atomatically allocate and release networking ports
 type IPAllocator struct {
-	network *net.IPNet
-	queue   chan (net.IP)
+	network       *net.IPNet
+	queueAlloc    chan allocatedIP
+	queueReleased chan net.IP
+	inUse         map[int32]struct{}
+}
+
+type allocatedIP struct {
+	ip  net.IP
+	err error
 }
 
-func (alloc *IPAllocator) populate() error {
+func (alloc *IPAllocator) run() {
 	firstIP, _ := networkRange(alloc.network)
-	size, err := networkSize(alloc.network.Mask)
-	if err != nil {
-		return err
-	}
-	// The queue size should be the network size - 3
-	// -1 for the network address, -1 for the broadcast address and
-	// -1 for the gateway address
-	alloc.queue = make(chan net.IP, size-3)
-	for i := int32(1); i < size-1; i++ {
-		ipNum, err := ipToInt(firstIP)
-		if err != nil {
-			return err
+	ipNum := ipToInt(firstIP)
+	ownIP := ipToInt(alloc.network.IP)
+	size := networkSize(alloc.network.Mask)
+
+	pos := int32(1)
+	max := size - 2 // -1 for the broadcast address, -1 for the gateway address
+	for {
+		var (
+			newNum int32
+			inUse  bool
+		)
+
+		// Find first unused IP, give up after one whole round
+		for attempt := int32(0); attempt < max; attempt++ {
+			newNum = ipNum + pos
+
+			pos = pos%max + 1
+
+			// The network's IP is never okay to use
+			if newNum == ownIP {
+				continue
+			}
+
+			if _, inUse = alloc.inUse[newNum]; !inUse {
+				// We found an unused IP
+				break
+			}
 		}
-		ip, err := intToIp(ipNum + int32(i))
-		if err != nil {
-			return err
+
+		ip := allocatedIP{ip: intToIp(newNum)}
+		if inUse {
+			ip.err = errors.New("No unallocated IP available")
 		}
-		// Discard the network IP (that's the host IP address)
-		if ip.Equal(alloc.network.IP) {
-			continue
+
+		select {
+		case alloc.queueAlloc <- ip:
+			alloc.inUse[newNum] = struct{}{}
+		case released := <-alloc.queueReleased:
+			r := ipToInt(released)
+			delete(alloc.inUse, r)
+
+			if inUse {
+				// If we couldn't allocate a new IP, the released one
+				// will be the only free one now, so instantly use it
+				// next time
+				pos = r - ipNum
+			} else {
+				// Use same IP as last time
+				if pos == 1 {
+					pos = max
+				} else {
+					pos--
+				}
+			}
 		}
-		alloc.queue <- ip
 	}
-	return nil
 }
 
 func (alloc *IPAllocator) Acquire() (net.IP, error) {
-	select {
-	case ip := <-alloc.queue:
-		return ip, nil
-	default:
-		return net.IP{}, errors.New("No more IP addresses available")
-	}
-	return net.IP{}, nil
+	ip := <-alloc.queueAlloc
+	return ip.ip, ip.err
 }
 
-func (alloc *IPAllocator) Release(ip net.IP) error {
-	select {
-	case alloc.queue <- ip:
-		return nil
-	default:
-		return errors.New("Too many IP addresses have been released")
-	}
-	return nil
+func (alloc *IPAllocator) Release(ip net.IP) {
+	alloc.queueReleased <- ip
 }
 
-func newIPAllocator(network *net.IPNet) (*IPAllocator, error) {
+func newIPAllocator(network *net.IPNet) *IPAllocator {
 	alloc := &IPAllocator{
-		network: network,
+		network:       network,
+		queueAlloc:    make(chan allocatedIP),
+		queueReleased: make(chan net.IP),
+		inUse:         make(map[int32]struct{}),
 	}
-	if err := alloc.populate(); err != nil {
-		return nil, err
-	}
-	return alloc, nil
+
+	go alloc.run()
+
+	return alloc
 }
 
 // Network interface represents the networking stack of a container
@@ -297,7 +312,7 @@ func (iface *NetworkInterface) AllocatePort(port int) (int, error) {
 }
 
 // Release: Network cleanup - release all resources
-func (iface *NetworkInterface) Release() error {
+func (iface *NetworkInterface) Release() {
 	for _, port := range iface.extPorts {
 		if err := iface.manager.portMapper.Unmap(port); err != nil {
 			log.Printf("Unable to unmap port %v: %v", port, err)
@@ -307,7 +322,8 @@ func (iface *NetworkInterface) Release() error {
 		}
 
 	}
-	return iface.manager.ipAllocator.Release(iface.IPNet.IP)
+
+	iface.manager.ipAllocator.Release(iface.IPNet.IP)
 }
 
 // Network Manager manages a set of network interfaces
@@ -342,10 +358,7 @@ func newNetworkManager(bridgeIface string) (*NetworkManager, error) {
 	}
 	network := addr.(*net.IPNet)
 
-	ipAllocator, err := newIPAllocator(network)
-	if err != nil {
-		return nil, err
-	}
+	ipAllocator := newIPAllocator(network)
 
 	portAllocator, err := newPortAllocator(portRangeStart, portRangeEnd)
 	if err != nil {

network_test.go

@@ -28,8 +28,8 @@ func TestNetworkRange(t *testing.T) {
 	if !last.Equal(net.ParseIP("192.168.0.255")) {
 		t.Error(last.String())
 	}
-	if size, err := networkSize(network.Mask); err != nil || size != 256 {
-		t.Error(size, err)
+	if size := networkSize(network.Mask); size != 256 {
+		t.Error(size)
 	}
 
 	// Class A test
@@ -41,8 +41,8 @@ func TestNetworkRange(t *testing.T) {
 	if !last.Equal(net.ParseIP("10.255.255.255")) {
 		t.Error(last.String())
 	}
-	if size, err := networkSize(network.Mask); err != nil || size != 16777216 {
-		t.Error(size, err)
+	if size := networkSize(network.Mask); size != 16777216 {
+		t.Error(size)
 	}
 
 	// Class A, random IP address
@@ -64,8 +64,8 @@ func TestNetworkRange(t *testing.T) {
 	if !last.Equal(net.ParseIP("10.1.2.3")) {
 		t.Error(last.String())
 	}
-	if size, err := networkSize(network.Mask); err != nil || size != 1 {
-		t.Error(size, err)
+	if size := networkSize(network.Mask); size != 1 {
+		t.Error(size)
 	}
 
 	// 31bit mask
@@ -77,8 +77,8 @@ func TestNetworkRange(t *testing.T) {
 	if !last.Equal(net.ParseIP("10.1.2.3")) {
 		t.Error(last.String())
 	}
-	if size, err := networkSize(network.Mask); err != nil || size != 2 {
-		t.Error(size, err)
+	if size := networkSize(network.Mask); size != 2 {
+		t.Error(size)
 	}
 
 	// 26bit mask
@@ -90,54 +90,130 @@ func TestNetworkRange(t *testing.T) {
 	if !last.Equal(net.ParseIP("10.1.2.63")) {
 		t.Error(last.String())
 	}
-	if size, err := networkSize(network.Mask); err != nil || size != 64 {
-		t.Error(size, err)
+	if size := networkSize(network.Mask); size != 64 {
+		t.Error(size)
 	}
 }
 
 func TestConversion(t *testing.T) {
 	ip := net.ParseIP("127.0.0.1")
-	i, err := ipToInt(ip)
-	if err != nil {
-		t.Fatal(err)
-	}
+	i := ipToInt(ip)
 	if i == 0 {
 		t.Fatal("converted to zero")
 	}
-	conv, err := intToIp(i)
-	if err != nil {
-		t.Fatal(err)
-	}
+	conv := intToIp(i)
 	if !ip.Equal(conv) {
 		t.Error(conv.String())
 	}
 }
 
 func TestIPAllocator(t *testing.T) {
-	gwIP, n, _ := net.ParseCIDR("127.0.0.1/29")
-	alloc, err := newIPAllocator(&net.IPNet{IP: gwIP, Mask: n.Mask})
-	if err != nil {
-		t.Fatal(err)
+	expectedIPs := []net.IP{
+		0: net.IPv4(127, 0, 0, 2),
+		1: net.IPv4(127, 0, 0, 3),
+		2: net.IPv4(127, 0, 0, 4),
+		3: net.IPv4(127, 0, 0, 5),
+		4: net.IPv4(127, 0, 0, 6),
 	}
-	var lastIP net.IP
+
+	gwIP, n, _ := net.ParseCIDR("127.0.0.1/29")
+	alloc := newIPAllocator(&net.IPNet{IP: gwIP, Mask: n.Mask})
+	// Pool after initialisation (f = free, u = used)
+	// 2(f) - 3(f) - 4(f) - 5(f) - 6(f)
+	//  ↑
+
+	// Check that we get 5 IPs, from 127.0.0.2–127.0.0.6, in that
+	// order.
 	for i := 0; i < 5; i++ {
 		ip, err := alloc.Acquire()
 		if err != nil {
 			t.Fatal(err)
 		}
-		lastIP = ip
+
+		assertIPEquals(t, expectedIPs[i], ip)
 	}
-	ip, err := alloc.Acquire()
+	// Before loop begin
+	// 2(f) - 3(f) - 4(f) - 5(f) - 6(f)
+	//  ↑
+
+	// After i = 0
+	// 2(u) - 3(f) - 4(f) - 5(f) - 6(f)
+	//         ↑
+
+	// After i = 1
+	// 2(u) - 3(u) - 4(f) - 5(f) - 6(f)
+	//                ↑
+
+	// After i = 2
+	// 2(u) - 3(u) - 4(u) - 5(f) - 6(f)
+	//                       ↑
+
+	// After i = 3
+	// 2(u) - 3(u) - 4(u) - 5(u) - 6(f)
+	//                              ↑
+
+	// After i = 4
+	// 2(u) - 3(u) - 4(u) - 5(u) - 6(u)
+	//  ↑
+
+	// Check that there are no more IPs
+	_, err := alloc.Acquire()
 	if err == nil {
 		t.Fatal("There shouldn't be any IP addresses at this point")
 	}
-	// Release 1 IP
-	alloc.Release(lastIP)
-	ip, err = alloc.Acquire()
-	if err != nil {
-		t.Fatal(err)
+
+	// Release some IPs in non-sequential order
+	alloc.Release(expectedIPs[3])
+	// 2(u) - 3(u) - 4(u) - 5(f) - 6(u)
+	//                       ↑
+
+	alloc.Release(expectedIPs[2])
+	// 2(u) - 3(u) - 4(f) - 5(f) - 6(u)
+	//                       ↑
+
+	alloc.Release(expectedIPs[4])
+	// 2(u) - 3(u) - 4(f) - 5(f) - 6(f)
+	//                       ↑
+
+	// Make sure that IPs are reused in sequential order, starting
+	// with the first released IP
+	newIPs := make([]net.IP, 3)
+	for i := 0; i < 3; i++ {
+		ip, err := alloc.Acquire()
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		newIPs[i] = ip
+	}
+	// Before loop begin
+	// 2(u) - 3(u) - 4(f) - 5(f) - 6(f)
+	//                       ↑
+
+	// After i = 0
+	// 2(u) - 3(u) - 4(f) - 5(u) - 6(f)
+	//                              ↑
+
+	// After i = 1
+	// 2(u) - 3(u) - 4(f) - 5(u) - 6(u)
+	//                ↑
+
+	// After i = 2
+	// 2(u) - 3(u) - 4(u) - 5(u) - 6(u)
+	//                       ↑
+
+	assertIPEquals(t, expectedIPs[3], newIPs[0])
+	assertIPEquals(t, expectedIPs[4], newIPs[1])
+	assertIPEquals(t, expectedIPs[2], newIPs[2])
+
+	_, err = alloc.Acquire()
+	if err == nil {
+		t.Fatal("There shouldn't be any IP addresses at this point")
 	}
-	if !ip.Equal(lastIP) {
-		t.Fatal(ip.String())
+}
+
+func assertIPEquals(t *testing.T, ip1, ip2 net.IP) {
+	if !ip1.Equal(ip2) {
+		t.Fatalf("Expected IP %s, got %s", ip1, ip2)
 	}
 }

rcli/http.go

@@ -20,7 +20,7 @@ func ListenAndServeHTTP(addr string, service Service) error {
 		func(w http.ResponseWriter, r *http.Request) {
 			cmd, args := URLToCall(r.URL)
 			if err := call(service, r.Body, &AutoFlush{w}, append([]string{cmd}, args...)...); err != nil {
-				fmt.Fprintf(w, "Error: "+err.Error()+"\n")
+				fmt.Fprintln(w, "Error:", err.Error())
 			}
 		}))
 }

rcli/tcp.go

@@ -51,8 +51,8 @@ func ListenAndServe(proto, addr string, service Service) error {
 					CLIENT_SOCKET = conn
 				}
 				if err := Serve(conn, service); err != nil {
-					log.Printf("Error: " + err.Error() + "\n")
-					fmt.Fprintf(conn, "Error: "+err.Error()+"\n")
+					log.Println("Error:", err.Error())
+					fmt.Fprintln(conn, "Error:", err.Error())
 				}
 				conn.Close()
 			}()

registry.go

@@ -1,6 +1,7 @@
 package docker
 
 import (
+	"bytes"
 	"encoding/json"
 	"fmt"
 	"github.com/dotcloud/docker/auth"
@@ -20,7 +21,7 @@ func NewImgJson(src []byte) (*Image, error) {
 	ret := &Image{}
 
 	Debugf("Json string: {%s}\n", src)
-	// FIXME: Is there a cleaner way to "puryfy" the input json?
+	// FIXME: Is there a cleaner way to "purify" the input json?
 	if err := json.Unmarshal(src, ret); err != nil {
 		return nil, err
 	}
@@ -32,7 +33,7 @@ func NewImgJson(src []byte) (*Image, error) {
 func NewMultipleImgJson(src []byte) ([]*Image, error) {
 	ret := []*Image{}
 
-	dec := json.NewDecoder(strings.NewReader(string(src)))
+	dec := json.NewDecoder(bytes.NewReader(src))
 	for {
 		m := &Image{}
 		if err := dec.Decode(m); err == io.EOF {
@@ -135,7 +136,7 @@ func (graph *Graph) getRemoteImage(stdout io.Writer, imgId string, authConfig *a
 	if err != nil {
 		return nil, nil, err
 	}
-	return img, res.Body, nil
+	return img, ProgressReader(res.Body, int(res.ContentLength), stdout), nil
 }
 
 func (graph *Graph) PullImage(stdout io.Writer, imgId string, authConfig *auth.AuthConfig) error {
@@ -183,10 +184,10 @@ func (graph *Graph) PullRepository(stdout io.Writer, remote, askedTag string, re
 	if err != nil {
 		return err
 	}
+	defer res.Body.Close()
 	if res.StatusCode != 200 {
 		return fmt.Errorf("HTTP code: %d", res.StatusCode)
 	}
-	defer res.Body.Close()
 	rawJson, err := ioutil.ReadAll(res.Body)
 	if err != nil {
 		return err
@@ -226,7 +227,7 @@ func (graph *Graph) PushImage(stdout io.Writer, imgOrig *Image, authConfig *auth
 		fmt.Fprintf(stdout, "Pushing %s metadata\n", img.Id)
 
 		// FIXME: try json with UTF8
-		jsonData := strings.NewReader(string(jsonRaw))
+		jsonData := bytes.NewReader(jsonRaw)
 		req, err := http.NewRequest("PUT", REGISTRY_ENDPOINT+"/images/"+img.Id+"/json", jsonData)
 		if err != nil {
 			return err
@@ -237,6 +238,7 @@ func (graph *Graph) PushImage(stdout io.Writer, imgOrig *Image, authConfig *auth
 		if err != nil {
 			return fmt.Errorf("Failed to upload metadata: %s", err)
 		}
+		defer res.Body.Close()
 		if res.StatusCode != 200 {
 			switch res.StatusCode {
 			case 204:
@@ -256,9 +258,13 @@ func (graph *Graph) PushImage(stdout io.Writer, imgOrig *Image, authConfig *auth
 		req2, err := http.NewRequest("PUT", REGISTRY_ENDPOINT+"/images/"+img.Id+"/layer", nil)
 		req2.SetBasicAuth(authConfig.Username, authConfig.Password)
 		res2, err := client.Do(req2)
-		if err != nil || res2.StatusCode != 307 {
+		if err != nil {
 			return fmt.Errorf("Registry returned error: %s", err)
 		}
+		res2.Body.Close()
+		if res2.StatusCode != 307 {
+			return fmt.Errorf("Registry returned unexpected HTTP status code %d, expected 307", res2.StatusCode)
+		}
 		url, err := res2.Location()
 		if err != nil || url == nil {
 			return fmt.Errorf("Failed to retrieve layer upload location: %s", err)
@@ -267,25 +273,28 @@ func (graph *Graph) PushImage(stdout io.Writer, imgOrig *Image, authConfig *auth
 		// FIXME: Don't do this :D. Check the S3 requierement and implement chunks of 5MB
 		// FIXME2: I won't stress it enough, DON'T DO THIS! very high priority
 		layerData2, err := Tar(path.Join(graph.Root, img.Id, "layer"), Gzip)
-		layerData, err := Tar(path.Join(graph.Root, img.Id, "layer"), Gzip)
+		tmp, err := ioutil.ReadAll(layerData2)
 		if err != nil {
-			return fmt.Errorf("Failed to generate layer archive: %s", err)
+			return err
 		}
-		req3, err := http.NewRequest("PUT", url.String(), layerData)
+		layerLength := len(tmp)
+
+		layerData, err := Tar(path.Join(graph.Root, img.Id, "layer"), Gzip)
 		if err != nil {
-			return err
+			return fmt.Errorf("Failed to generate layer archive: %s", err)
 		}
-		tmp, err := ioutil.ReadAll(layerData2)
+		req3, err := http.NewRequest("PUT", url.String(), ProgressReader(layerData.(io.ReadCloser), layerLength, stdout))
 		if err != nil {
 			return err
 		}
-		req3.ContentLength = int64(len(tmp))
+		req3.ContentLength = int64(layerLength)
 
 		req3.TransferEncoding = []string{"none"}
 		res3, err := client.Do(req3)
 		if err != nil {
 			return fmt.Errorf("Failed to upload layer: %s", err)
 		}
+		res3.Body.Close()
 		if res3.StatusCode != 200 {
 			return fmt.Errorf("Received HTTP code %d while uploading layer", res3.StatusCode)
 		}
@@ -315,12 +324,13 @@ func (graph *Graph) pushTag(remote, revision, tag string, authConfig *auth.AuthC
 	req.Header.Add("Content-type", "application/json")
 	req.SetBasicAuth(authConfig.Username, authConfig.Password)
 	res, err := client.Do(req)
-	if err != nil || (res.StatusCode != 200 && res.StatusCode != 201) {
-		if res != nil {
-			return fmt.Errorf("Internal server error: %d trying to push tag %s on %s", res.StatusCode, tag, remote)
-		}
+	if err != nil {
 		return err
 	}
+	res.Body.Close()
+	if res.StatusCode != 200 && res.StatusCode != 201 {
+		return fmt.Errorf("Internal server error: %d trying to push tag %s on %s", res.StatusCode, tag, remote)
+	}
 	Debugf("Result of push tag: %d\n", res.StatusCode)
 	switch res.StatusCode {
 	default:

runtime.go

@@ -21,6 +21,7 @@ type Runtime struct {
 	graph          *Graph
 	repositories   *TagStore
 	authConfig     *auth.AuthConfig
+	idIndex        *TruncIndex
 }
 
 var sysInitPath string
@@ -47,7 +48,11 @@ func (runtime *Runtime) getContainerElement(id string) *list.Element {
 	return nil
 }
 
-func (runtime *Runtime) Get(id string) *Container {
+func (runtime *Runtime) Get(name string) *Container {
+	id, err := runtime.idIndex.Get(name)
+	if err != nil {
+		return nil
+	}
 	e := runtime.getContainerElement(id)
 	if e == nil {
 		return nil
@@ -72,6 +77,7 @@ func (runtime *Runtime) Create(config *Config) (*Container, error) {
 	// Generate id
 	id := GenerateId()
 	// Generate default hostname
+	// FIXME: the lxc template no longer needs to set a default hostname
 	if config.Hostname == "" {
 		config.Hostname = id[:12]
 	}
@@ -140,15 +146,9 @@ func (runtime *Runtime) Register(container *Container) error {
 	} else {
 		container.stdinPipe = NopWriteCloser(ioutil.Discard) // Silently drop stdin
 	}
-	// Setup logging of stdout and stderr to disk
-	if err := runtime.LogToDisk(container.stdout, container.logPath("stdout")); err != nil {
-		return err
-	}
-	if err := runtime.LogToDisk(container.stderr, container.logPath("stderr")); err != nil {
-		return err
-	}
 	// done
 	runtime.containers.PushBack(container)
+	runtime.idIndex.Add(container.Id)
 	return nil
 }
 
@@ -157,7 +157,7 @@ func (runtime *Runtime) LogToDisk(src *writeBroadcaster, dst string) error {
 	if err != nil {
 		return err
 	}
-	src.AddWriter(NopWriteCloser(log))
+	src.AddWriter(log)
 	return nil
 }
 
@@ -178,6 +178,7 @@ func (runtime *Runtime) Destroy(container *Container) error {
 		}
 	}
 	// Deregister the container before removing its directory, to avoid race conditions
+	runtime.idIndex.Delete(container.Id)
 	runtime.containers.Remove(element)
 	if err := os.RemoveAll(container.root); err != nil {
 		return fmt.Errorf("Unable to remove filesystem for %v: %v", container.Id, err)
@@ -229,6 +230,7 @@ func (runtime *Runtime) restore() error {
 	return nil
 }
 
+// FIXME: harmonize with NewGraph()
 func NewRuntime() (*Runtime, error) {
 	return NewRuntimeFromDirectory("/var/lib/docker")
 }
@@ -266,6 +268,7 @@ func NewRuntimeFromDirectory(root string) (*Runtime, error) {
 		graph:          g,
 		repositories:   repositories,
 		authConfig:     authConfig,
+		idIndex:        NewTruncIndex(),
 	}
 
 	if err := runtime.restore(); err != nil {

tags.go

@@ -106,7 +106,7 @@ func (store *TagStore) ImageName(id string) string {
 	if names, exists := store.ById()[id]; exists && len(names) > 0 {
 		return names[0]
 	}
-	return id
+	return TruncateId(id)
 }
 
 func (store *TagStore) Set(repoName, tag, imageName string, force bool) error {

term/termios_darwin.go

@@ -1,8 +1,8 @@
 package term
 
 import (
-    "syscall"
-    "unsafe"
+	"syscall"
+	"unsafe"
 )
 
 const (
@@ -14,19 +14,19 @@ const (
 // mode and returns the previous state of the terminal so that it can be
 // restored.
 func MakeRaw(fd int) (*State, error) {
-    var oldState State
-    if _, _, err := syscall.Syscall6(syscall.SYS_IOCTL, uintptr(fd), uintptr(getTermios), uintptr(unsafe.Pointer(&oldState.termios)), 0, 0, 0); err != 0 {
-        return nil, err
-    }
+	var oldState State
+	if _, _, err := syscall.Syscall6(syscall.SYS_IOCTL, uintptr(fd), uintptr(getTermios), uintptr(unsafe.Pointer(&oldState.termios)), 0, 0, 0); err != 0 {
+		return nil, err
+	}
 
-    newState := oldState.termios
-    newState.Iflag &^= ISTRIP | INLCR | IGNCR | IXON | IXOFF
-    newState.Iflag |= ICRNL
-    newState.Oflag |= ONLCR
-    newState.Lflag &^= ECHO | ICANON | ISIG
-    if _, _, err := syscall.Syscall6(syscall.SYS_IOCTL, uintptr(fd), uintptr(setTermios), uintptr(unsafe.Pointer(&newState)), 0, 0, 0); err != 0 {
-        return nil, err
-    }
+	newState := oldState.termios
+	newState.Iflag &^= ISTRIP | INLCR | IGNCR | IXON | IXOFF
+	newState.Iflag |= ICRNL
+	newState.Oflag |= ONLCR
+	newState.Lflag &^= ECHO | ICANON | ISIG
+	if _, _, err := syscall.Syscall6(syscall.SYS_IOCTL, uintptr(fd), uintptr(setTermios), uintptr(unsafe.Pointer(&newState)), 0, 0, 0); err != 0 {
+		return nil, err
+	}
 
-    return &oldState, nil
-}
+	return &oldState, nil
+}

utils.go

@@ -6,7 +6,9 @@ import (
 	"errors"
 	"fmt"
 	"github.com/dotcloud/docker/rcli"
+	"index/suffixarray"
 	"io"
+	"io/ioutil"
 	"net/http"
 	"os"
 	"os/exec"
@@ -220,6 +222,7 @@ func (w *writeBroadcaster) AddWriter(writer io.WriteCloser) {
 	w.writers.PushBack(writer)
 }
 
+// FIXME: Is that function used?
 func (w *writeBroadcaster) RemoveWriter(writer io.WriteCloser) {
 	for e := w.writers.Front(); e != nil; e = e.Next() {
 		v := e.Value.(io.Writer)
@@ -252,9 +255,94 @@ func (w *writeBroadcaster) Close() error {
 		writer := e.Value.(io.WriteCloser)
 		writer.Close()
 	}
+	w.writers.Init()
 	return nil
 }
 
 func newWriteBroadcaster() *writeBroadcaster {
 	return &writeBroadcaster{list.New()}
 }
+
+func getTotalUsedFds() int {
+	if fds, err := ioutil.ReadDir(fmt.Sprintf("/proc/%d/fd", os.Getpid())); err != nil {
+		Debugf("Error opening /proc/%d/fd: %s", os.Getpid(), err)
+	} else {
+		return len(fds)
+	}
+	return -1
+}
+
+// TruncIndex allows the retrieval of string identifiers by any of their unique prefixes.
+// This is used to retrieve image and container IDs by more convenient shorthand prefixes.
+type TruncIndex struct {
+	index *suffixarray.Index
+	ids   map[string]bool
+	bytes []byte
+}
+
+func NewTruncIndex() *TruncIndex {
+	return &TruncIndex{
+		index: suffixarray.New([]byte{' '}),
+		ids:   make(map[string]bool),
+		bytes: []byte{' '},
+	}
+}
+
+func (idx *TruncIndex) Add(id string) error {
+	if strings.Contains(id, " ") {
+		return fmt.Errorf("Illegal character: ' '")
+	}
+	if _, exists := idx.ids[id]; exists {
+		return fmt.Errorf("Id already exists: %s", id)
+	}
+	idx.ids[id] = true
+	idx.bytes = append(idx.bytes, []byte(id+" ")...)
+	idx.index = suffixarray.New(idx.bytes)
+	return nil
+}
+
+func (idx *TruncIndex) Delete(id string) error {
+	if _, exists := idx.ids[id]; !exists {
+		return fmt.Errorf("No such id: %s", id)
+	}
+	before, after, err := idx.lookup(id)
+	if err != nil {
+		return err
+	}
+	delete(idx.ids, id)
+	idx.bytes = append(idx.bytes[:before], idx.bytes[after:]...)
+	idx.index = suffixarray.New(idx.bytes)
+	return nil
+}
+
+func (idx *TruncIndex) lookup(s string) (int, int, error) {
+	offsets := idx.index.Lookup([]byte(" "+s), -1)
+	//log.Printf("lookup(%s): %v (index bytes: '%s')\n", s, offsets, idx.index.Bytes())
+	if offsets == nil || len(offsets) == 0 || len(offsets) > 1 {
+		return -1, -1, fmt.Errorf("No such id: %s", s)
+	}
+	offsetBefore := offsets[0] + 1
+	offsetAfter := offsetBefore + strings.Index(string(idx.bytes[offsetBefore:]), " ")
+	return offsetBefore, offsetAfter, nil
+}
+
+func (idx *TruncIndex) Get(s string) (string, error) {
+	before, after, err := idx.lookup(s)
+	//log.Printf("Get(%s) bytes=|%s| before=|%d| after=|%d|\n", s, idx.bytes, before, after)
+	if err != nil {
+		return "", err
+	}
+	return string(idx.bytes[before:after]), err
+}
+
+// TruncateId returns a shorthand version of a string identifier for convenience.
+// A collision with other shorthands is very unlikely, but possible.
+// In case of a collision a lookup with TruncIndex.Get() will fail, and the caller
+// will need to use a langer prefix, or the full-length Id.
+func TruncateId(id string) string {
+	shortLen := 12
+	if len(id) < shortLen {
+		shortLen = len(id)
+	}
+	return id[:shortLen]
+}

utils_test.go

@@ -124,3 +124,85 @@ func TestWriteBroadcaster(t *testing.T) {
 
 	writer.Close()
 }
+
+// Test the behavior of TruncIndex, an index for querying IDs from a non-conflicting prefix.
+func TestTruncIndex(t *testing.T) {
+	index := NewTruncIndex()
+	// Get on an empty index
+	if _, err := index.Get("foobar"); err == nil {
+		t.Fatal("Get on an empty index should return an error")
+	}
+
+	// Spaces should be illegal in an id
+	if err := index.Add("I have a space"); err == nil {
+		t.Fatalf("Adding an id with ' ' should return an error")
+	}
+
+	id := "99b36c2c326ccc11e726eee6ee78a0baf166ef96"
+	// Add an id
+	if err := index.Add(id); err != nil {
+		t.Fatal(err)
+	}
+	// Get a non-existing id
+	assertIndexGet(t, index, "abracadabra", "", true)
+	// Get the exact id
+	assertIndexGet(t, index, id, id, false)
+	// The first letter should match
+	assertIndexGet(t, index, id[:1], id, false)
+	// The first half should match
+	assertIndexGet(t, index, id[:len(id)/2], id, false)
+	// The second half should NOT match
+	assertIndexGet(t, index, id[len(id)/2:], "", true)
+
+	id2 := id[:6] + "blabla"
+	// Add an id
+	if err := index.Add(id2); err != nil {
+		t.Fatal(err)
+	}
+	// Both exact IDs should work
+	assertIndexGet(t, index, id, id, false)
+	assertIndexGet(t, index, id2, id2, false)
+
+	// 6 characters or less should conflict
+	assertIndexGet(t, index, id[:6], "", true)
+	assertIndexGet(t, index, id[:4], "", true)
+	assertIndexGet(t, index, id[:1], "", true)
+
+	// 7 characters should NOT conflict
+	assertIndexGet(t, index, id[:7], id, false)
+	assertIndexGet(t, index, id2[:7], id2, false)
+
+	// Deleting a non-existing id should return an error
+	if err := index.Delete("non-existing"); err == nil {
+		t.Fatalf("Deleting a non-existing id should return an error")
+	}
+
+	// Deleting id2 should remove conflicts
+	if err := index.Delete(id2); err != nil {
+		t.Fatal(err)
+	}
+	// id2 should no longer work
+	assertIndexGet(t, index, id2, "", true)
+	assertIndexGet(t, index, id2[:7], "", true)
+	assertIndexGet(t, index, id2[:11], "", true)
+
+	// conflicts between id and id2 should be gone
+	assertIndexGet(t, index, id[:6], id, false)
+	assertIndexGet(t, index, id[:4], id, false)
+	assertIndexGet(t, index, id[:1], id, false)
+
+	// non-conflicting substrings should still not conflict
+	assertIndexGet(t, index, id[:7], id, false)
+	assertIndexGet(t, index, id[:15], id, false)
+	assertIndexGet(t, index, id, id, false)
+}
+
+func assertIndexGet(t *testing.T, index *TruncIndex, input, expectedResult string, expectError bool) {
+	if result, err := index.Get(input); err != nil && !expectError {
+		t.Fatalf("Unexpected error getting '%s': %s", input, err)
+	} else if err == nil && expectError {
+		t.Fatalf("Getting '%s' should return an error", input)
+	} else if result != expectedResult {
+		t.Fatalf("Getting '%s' returned '%s' instead of '%s'", input, result, expectedResult)
+	}
+}