diff --git a/AUTHORS b/AUTHORS index 80928e0e61..382a11b5b0 100644 --- a/AUTHORS +++ b/AUTHORS @@ -7,6 +7,7 @@ Caleb Spare Charles Hooper Daniel Mizyrycki Daniel Robinson +Dominik Honnef Don Spaulding ezbercih Frederick F. Kautz IV diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000000..f956d37101 --- /dev/null +++ b/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. diff --git a/Makefile b/Makefile index e716762d31..4a3e6567ff 100644 --- a/Makefile +++ b/Makefile @@ -41,3 +41,6 @@ endif test: all @(cd $(DOCKER_DIR); sudo -E go test $(GO_OPTIONS)) + +fmt: + @gofmt -s -l -w . diff --git a/README.md b/README.md index bb69f47979..66ca656f68 100644 --- a/README.md +++ b/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 ---------------------------- diff --git a/archive_test.go b/archive_test.go index 35185628a5..f583604497 100644 --- a/archive_test.go +++ b/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") } } diff --git a/auth/auth.go b/auth/auth.go index 045176ed48..2c282af218 100644 --- a/auth/auth.go +++ b/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 { diff --git a/commands.go b/commands.go index 99d0c341e5..0c896f29ba 100644 --- a/commands.go +++ b/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 */ "", /* TAG */ "", - /* 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) } + + cStdout, err := container.StdoutPipe() + if err != nil { + return err + } + cStderr, err := container.StderrPipe() + if err != nil { + return err + } + var wg sync.WaitGroup - if *flStdin { + if container.Config.OpenStdin { cStdin, err := container.StdinPipe() if err != nil { return err } wg.Add(1) - go func() { io.Copy(cStdin, stdin); wg.Add(-1) }() - } - if *flStdout { - cStdout, err := container.StdoutPipe() - if err != nil { - return err - } - wg.Add(1) - go func() { io.Copy(stdout, cStdout); wg.Add(-1) }() - } - if *flStderr { - cStderr, err := container.StderrPipe() - 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 } diff --git a/commands_test.go b/commands_test.go new file mode 100644 index 0000000000..ebbe88c637 --- /dev/null +++ b/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() +} diff --git a/container.go b/container.go index c0175b96b1..8f993a35e1 100644 --- a/container.go +++ b/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) + } } - container.stdout.Close() - container.stderr.Close() + 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) + } + + 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)) } diff --git a/container_test.go b/container_test.go index fb7cdc2efd..571a093767 100644 --- a/container_test.go +++ b/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) - output2, err := ioutil.ReadAll(stderr) - stdout.Close() - stderr.Close() - if string(output) != "hello\n" { - t.Fatalf("\nout: %s\nerr: %s\n", string(output), string(output2)) + if err != nil { + t.Fatal(err) } + output2, err := ioutil.ReadAll(stderr) + 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("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() - defer stdin.Close() - defer stdout.Close() + if err != nil { + t.Fatal(err) + } if err := container.Start(); err != nil { t.Fatal(err) } - io.WriteString(stdin, "hello world") - stdin.Close() + defer stdin.Close() + defer stdout.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) + 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() - defer stdin.Close() - defer stdout.Close() + if err != nil { + t.Fatal(err) + } if err := container.Start(); err != nil { t.Fatal(err) } - io.WriteString(stdin, "hello world") - stdin.Close() + defer stdin.Close() + defer stdout.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) + 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) diff --git a/contrib/mkimage-busybox.sh b/contrib/mkimage-busybox.sh index 6c091d2c4a..909ad4794a 100755 --- a/contrib/mkimage-busybox.sh +++ b/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. diff --git a/docs/sources/contributing/contributing.rst b/docs/sources/contributing/contributing.rst index d7f0258439..689c4207ce 100644 --- a/docs/sources/contributing/contributing.rst +++ b/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. diff --git a/graph.go b/graph.go index 6f93f2b827..0c84dc4252 100644 --- a/graph.go +++ b/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() diff --git a/graph_test.go b/graph_test.go index 61bac92d9e..8ac8d10478 100644 --- a/graph_test.go +++ b/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) diff --git a/image.go b/image.go index 1cd475f19b..19e5387f97 100644 --- a/image.go +++ b/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") diff --git a/network.go b/network.go index dd2b3e8bc7..c050609d16 100644 --- a/network.go +++ b/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{} } -func (alloc *IPAllocator) populate() error { +type allocatedIP struct { + ip net.IP + err error +} + +func (alloc *IPAllocator) run() { firstIP, _ := networkRange(alloc.network) - size, err := networkSize(alloc.network.Mask) - 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 := allocatedIP{ip: intToIp(newNum)} + if inUse { + ip.err = errors.New("No unallocated IP available") + } + + 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-- + } + } + } } - // 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 - } - ip, err := intToIp(ipNum + int32(i)) - if err != nil { - return err - } - // Discard the network IP (that's the host IP address) - if ip.Equal(alloc.network.IP) { - continue - } - 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 { diff --git a/network_test.go b/network_test.go index 53e79a13b0..a9d3cac454 100644 --- a/network_test.go +++ b/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 } - if !ip.Equal(lastIP) { - t.Fatal(ip.String()) + // 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") + } +} + +func assertIPEquals(t *testing.T, ip1, ip2 net.IP) { + if !ip1.Equal(ip2) { + t.Fatalf("Expected IP %s, got %s", ip1, ip2) } } diff --git a/rcli/http.go b/rcli/http.go index cc8d3b149e..3eeb2c2a97 100644 --- a/rcli/http.go +++ b/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()) } })) } diff --git a/rcli/tcp.go b/rcli/tcp.go index a1fa669023..ff7e191f42 100644 --- a/rcli/tcp.go +++ b/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() }() diff --git a/registry.go b/registry.go index 3e62ad96ee..1788ced5c7 100644 --- a/registry.go +++ b/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) - if err != nil { - return fmt.Errorf("Failed to generate layer archive: %s", err) - } - req3, err := http.NewRequest("PUT", url.String(), layerData) - if err != nil { - return err - } tmp, err := ioutil.ReadAll(layerData2) if err != nil { return err } - req3.ContentLength = int64(len(tmp)) + layerLength := len(tmp) + + layerData, err := Tar(path.Join(graph.Root, img.Id, "layer"), Gzip) + if err != nil { + return fmt.Errorf("Failed to generate layer archive: %s", err) + } + req3, err := http.NewRequest("PUT", url.String(), ProgressReader(layerData.(io.ReadCloser), layerLength, stdout)) + if err != nil { + return err + } + 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: diff --git a/runtime.go b/runtime.go index c719fdd602..9122f0c664 100644 --- a/runtime.go +++ b/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 { diff --git a/tags.go b/tags.go index 4f2b92e0bb..1b9cd19c83 100644 --- a/tags.go +++ b/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 { diff --git a/term/termios_darwin.go b/term/termios_darwin.go index 4365b60339..7df54e7828 100644 --- a/term/termios_darwin.go +++ b/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 -} \ No newline at end of file + return &oldState, nil +} diff --git a/utils.go b/utils.go index 1c61f12cad..381af1fe38 100644 --- a/utils.go +++ b/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] +} diff --git a/utils_test.go b/utils_test.go index dbdcda434c..192b042ba2 100644 --- a/utils_test.go +++ b/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) + } +}