From a5054184a172da0b9abd2470946bb15ab5fb0516 Mon Sep 17 00:00:00 2001 From: John Costa Date: Thu, 28 Mar 2013 16:32:31 -0400 Subject: [PATCH 01/47] move documentation changes to reStructuredText docs under website. https://github.com/dotcloud/docker/issues/42 --- docs/sources/contributing/contributing.rst | 24 +++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) 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. From 760736b3f3f289c2dba013aaef385b1e7b555feb Mon Sep 17 00:00:00 2001 From: John Costa Date: Thu, 28 Mar 2013 20:04:21 -0400 Subject: [PATCH 02/47] add contributing guidlines md file --- CONTRIBUTING.md | 69 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) create mode 100644 CONTRIBUTING.md 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. From bd3c6793a183b9b848e34312a0215cb9078f2240 Mon Sep 17 00:00:00 2001 From: John Costa Date: Fri, 29 Mar 2013 07:06:58 -0400 Subject: [PATCH 03/47] remove dulicated text and correct contributing link --- README.md | 23 +---------------------- 1 file changed, 1 insertion(+), 22 deletions(-) diff --git a/README.md b/README.md index 7fcfa4c360..495bbe83a3 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 ---------------------------- From 232dbb186416df290fc698e92b3a120b92497559 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Fri, 29 Mar 2013 07:44:58 -0700 Subject: [PATCH 04/47] Improve the containers unit tests (add error checking) --- container_test.go | 132 +++++++++++++++++++++++++++++++++++----------- 1 file changed, 100 insertions(+), 32 deletions(-) diff --git a/container_test.go b/container_test.go index fb7cdc2efd..5f18f3ec9d 100644 --- a/container_test.go +++ b/container_test.go @@ -91,17 +91,33 @@ func TestCommitRun(t *testing.T) { 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", string(output), string(output2)) } } @@ -208,11 +224,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 +234,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 +249,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 +306,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 +545,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 +592,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 +635,7 @@ func TestEnv(t *testing.T) { t.Fatal(err) } defer runtime.Destroy(container) + stdout, err := container.StdoutPipe() if err != nil { t.Fatal(err) From 9442d6b349295818d9a0ddecc10ca19019ead935 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Fri, 29 Mar 2013 07:49:26 -0700 Subject: [PATCH 05/47] Remove the options of CmdAttach --- commands.go | 31 ++++++++++++------------------- 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/commands.go b/commands.go index 99d0c341e5..d6a268659f 100644 --- a/commands.go +++ b/commands.go @@ -726,10 +726,7 @@ func (srv *Server) CmdLogs(stdin io.ReadCloser, stdout io.Writer, args ...string } 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 } @@ -743,7 +740,7 @@ func (srv *Server) CmdAttach(stdin io.ReadCloser, stdout io.Writer, args ...stri return errors.New("No such container: " + name) } var wg sync.WaitGroup - if *flStdin { + if container.Config.OpenStdin { cStdin, err := container.StdinPipe() if err != nil { return err @@ -751,22 +748,18 @@ func (srv *Server) CmdAttach(stdin io.ReadCloser, stdout io.Writer, args ...stri 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) }() + cStdout, err := container.StdoutPipe() + if err != nil { + return err } - if *flStderr { - cStderr, err := container.StderrPipe() - if err != nil { - return err - } - wg.Add(1) - go func() { io.Copy(stdout, cStderr); wg.Add(-1) }() + wg.Add(1) + go func() { io.Copy(stdout, cStdout); wg.Add(-1) }() + cStderr, err := container.StderrPipe() + if err != nil { + return err } + wg.Add(1) + go func() { io.Copy(stdout, cStderr); wg.Add(-1) }() wg.Wait() return nil } From ccac5b138253ab87846089cb5af9b4b77a7fd9d5 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Fri, 29 Mar 2013 08:18:43 -0700 Subject: [PATCH 06/47] Add debug infos --- commands.go | 21 ++++++++++++++++++--- container.go | 11 +++++++++++ 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/commands.go b/commands.go index cd91d07cff..c09bbd0034 100644 --- a/commands.go +++ b/commands.go @@ -798,20 +798,35 @@ func (srv *Server) CmdAttach(stdin io.ReadCloser, stdout io.Writer, args ...stri return err } wg.Add(1) - go func() { io.Copy(cStdin, stdin); wg.Add(-1) }() + go func() { + Debugf("Begin stdin pipe [attach]") + io.Copy(cStdin, stdin) + wg.Add(-1) + Debugf("End of stdin pipe [attach]") + }() } cStdout, err := container.StdoutPipe() if err != nil { return err } wg.Add(1) - go func() { io.Copy(stdout, cStdout); wg.Add(-1) }() + go func() { + Debugf("Begin stdout pipe [attach]") + io.Copy(stdout, cStdout) + wg.Add(-1) + Debugf("End of stdout pipe [attach]") + }() 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 stderr pipe [attach]") + io.Copy(stdout, cStderr) + wg.Add(-1) + Debugf("End of stderr pipe [attach]") + }() wg.Wait() return nil } diff --git a/container.go b/container.go index c0175b96b1..ace99936da 100644 --- a/container.go +++ b/container.go @@ -165,12 +165,16 @@ func (container *Container) startPty() error { // 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 @@ -186,7 +190,9 @@ func (container *Container) startPty() error { // 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,7 +216,9 @@ 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() @@ -348,7 +356,10 @@ func (container *Container) releaseNetwork() error { func (container *Container) monitor() { // Wait for the program to exit + Debugf("Waiting for process") container.cmd.Wait() + Debugf("Process finished") + exitCode := container.cmd.ProcessState.Sys().(syscall.WaitStatus).ExitStatus() // Cleanup From d17f78c373d5d8cd015626c7972e0e2365eab2a7 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Fri, 29 Mar 2013 08:19:42 -0700 Subject: [PATCH 07/47] Harmonize the error management. Use fmt.Errorf instead of errors.New --- commands.go | 29 ++++++++++++++--------------- container.go | 3 +-- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/commands.go b/commands.go index c09bbd0034..a66e4fb822 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" @@ -132,7 +131,7 @@ func (srv *Server) CmdLogin(stdin io.ReadCloser, stdout io.Writer, args ...strin 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, "): ") @@ -171,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 @@ -223,7 +222,7 @@ func (srv *Server) CmdStop(stdin io.ReadCloser, stdout io.Writer, args ...string } fmt.Fprintln(stdout, container.Id) } else { - return errors.New("No such container: " + name) + return fmt.Errorf("No such container: %s", name) } } return nil @@ -245,7 +244,7 @@ func (srv *Server) CmdRestart(stdin io.ReadCloser, stdout io.Writer, args ...str } fmt.Fprintln(stdout, container.Id) } else { - return errors.New("No such container: " + name) + return fmt.Errorf("No such container: %s", name) } } return nil @@ -267,7 +266,7 @@ func (srv *Server) CmdStart(stdin io.ReadCloser, stdout io.Writer, args ...strin } fmt.Fprintln(stdout, container.Id) } else { - return errors.New("No such container: " + name) + return fmt.Errorf("No such container: %s", name) } } return nil @@ -320,7 +319,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) @@ -383,7 +382,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()) @@ -401,7 +400,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()) @@ -420,7 +419,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 { @@ -718,7 +717,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 { @@ -729,10 +728,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 { @@ -774,7 +773,7 @@ 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 { @@ -789,7 +788,7 @@ 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 container.Config.OpenStdin { diff --git a/container.go b/container.go index ace99936da..90c6df95b6 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" @@ -464,7 +463,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 } From 69c2250ec2fd23dc9d31372cb433b7c18a917d09 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Fri, 29 Mar 2013 08:29:59 -0700 Subject: [PATCH 08/47] Add some error checking in container monitor --- container.go | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/container.go b/container.go index 90c6df95b6..80d5727d5a 100644 --- a/container.go +++ b/container.go @@ -356,7 +356,10 @@ func (container *Container) releaseNetwork() error { func (container *Container) monitor() { // Wait for the program to exit Debugf("Waiting for process") - container.cmd.Wait() + 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() @@ -365,8 +368,12 @@ func (container *Container) monitor() { if err := container.releaseNetwork(); err != nil { log.Printf("%v: Failed to release network: %v", 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 err := container.Unmount(); err != nil { log.Printf("%v: Failed to umount filesystem: %v", container.Id, err) } @@ -378,7 +385,9 @@ func (container *Container) monitor() { // Report status back container.State.setStopped(exitCode) - container.ToDisk() + if err := container.ToDisk(); err != nil { + log.Printf("%s: Failed to dump configuration to the disk: %s", container.Id, err) + } } func (container *Container) kill() error { From 7a565a0479b6a797a0cdc9a4156e57ce811032b3 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Fri, 29 Mar 2013 08:41:48 -0700 Subject: [PATCH 09/47] Remove unused variable from container struct --- container.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/container.go b/container.go index 80d5727d5a..c974c5f1cb 100644 --- a/container.go +++ b/container.go @@ -40,9 +40,7 @@ type Container struct { stdin io.ReadCloser stdinPipe io.WriteCloser - stdoutLog *os.File - stderrLog *os.File - runtime *Runtime + runtime *Runtime } type Config struct { From 0f7a4534c16cda07344a7231d6012268f4aa53aa Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Fri, 29 Mar 2013 08:46:06 -0700 Subject: [PATCH 10/47] Do not log non-running containers --- container.go | 8 ++++++++ runtime.go | 9 +-------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/container.go b/container.go index c0175b96b1..629af3a039 100644 --- a/container.go +++ b/container.go @@ -256,6 +256,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( diff --git a/runtime.go b/runtime.go index c719fdd602..37726da0ec 100644 --- a/runtime.go +++ b/runtime.go @@ -140,13 +140,6 @@ 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) return nil @@ -157,7 +150,7 @@ func (runtime *Runtime) LogToDisk(src *writeBroadcaster, dst string) error { if err != nil { return err } - src.AddWriter(NopWriteCloser(log)) + src.AddWriter(log) return nil } From 9740102990b059d014fad5f8652306cf7ff62d94 Mon Sep 17 00:00:00 2001 From: Louis Opter Date: Thu, 28 Mar 2013 12:11:47 -0700 Subject: [PATCH 11/47] Fix output in the login command It was broken because the terminal is in raw mode. This changeset adds code in the login commmand to do a little bit of interpretation on the user input (something usually done by the terminal emulator itself). --- commands.go | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 55 insertions(+), 3 deletions(-) diff --git a/commands.go b/commands.go index 99d0c341e5..80c4ff4142 100644 --- a/commands.go +++ b/commands.go @@ -17,6 +17,7 @@ import ( "sync" "text/tabwriter" "time" + "unicode" ) const VERSION = "0.1.0" @@ -62,29 +63,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 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") } fmt.Fprint(stdout, "Email (", srv.runtime.authConfig.Email, "): ") - fmt.Fscanf(stdin, "%s", &email) + email = readAndEchoString(stdin, stdout) if email == "" { email = srv.runtime.authConfig.Email } From 99b36c2c326ccc11e726eee6ee78a0baf166ef96 Mon Sep 17 00:00:00 2001 From: Solomon Hykes Date: Fri, 29 Mar 2013 21:13:59 -0700 Subject: [PATCH 12/47] Add comments in graph.go --- graph.go | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/graph.go b/graph.go index 6f93f2b827..09bc1d5bed 100644 --- a/graph.go +++ b/graph.go @@ -10,10 +10,13 @@ import ( "time" ) +// A Graph is a store for versioned filesystem images, and the relationship between them. type Graph struct { Root string } +// NewGraph instanciates 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 { @@ -34,6 +37,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,6 +46,7 @@ func (graph *Graph) Exists(id string) bool { return true } +// Get returns the image with the given id, or an error if the image doesn't exist. func (graph *Graph) Get(id string) (*Image, error) { // FIXME: return nil when the image doesn't exist, instead of an error img, err := LoadImage(graph.imageRoot(id)) @@ -54,6 +60,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 +78,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 @@ -95,6 +104,7 @@ func (graph *Graph) Register(layerData Archive, img *Image) error { 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,6 +116,9 @@ 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 ot 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:")) } @@ -124,6 +137,7 @@ func isNotEmpty(err error) bool { return strings.Contains(err.Error(), " not empty") } +// Delete atomically removes an image from the graph. func (graph *Graph) Delete(id string) error { garbage, err := graph.Garbage() if err != nil { @@ -150,6 +164,7 @@ 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 { @@ -158,6 +173,7 @@ func (graph *Graph) Undelete(id string) error { return os.Rename(garbage.imageRoot(id), graph.imageRoot(id)) } +// GarbageCollect definitely deletes all images moved to the garbage func (graph *Graph) GarbageCollect() error { garbage, err := graph.Garbage() if err != nil { @@ -166,6 +182,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 +196,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 +205,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 +223,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 +243,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() From 0b2eee8b33448c27c4d2b29b995a56ce3a411963 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Fri, 29 Mar 2013 09:31:47 -0700 Subject: [PATCH 13/47] Fix #108, reattach now works properly --- utils.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/utils.go b/utils.go index 1c61f12cad..fbbad34081 100644 --- a/utils.go +++ b/utils.go @@ -220,6 +220,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,6 +253,7 @@ func (w *writeBroadcaster) Close() error { writer := e.Value.(io.WriteCloser) writer.Close() } + w.writers.Init() return nil } From 81eac415ad03d93596a0d8da92bdccd9ecd4ee63 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Fri, 29 Mar 2013 09:42:42 -0700 Subject: [PATCH 14/47] Do not close the stdin of the process when the client deattaches himslef --- commands.go | 1 - 1 file changed, 1 deletion(-) diff --git a/commands.go b/commands.go index a66e4fb822..acd7d02921 100644 --- a/commands.go +++ b/commands.go @@ -909,7 +909,6 @@ func (srv *Server) CmdRun(stdin io.ReadCloser, stdout io.Writer, args ...string) if !config.Detach { Go(func() error { _, err := io.Copy(cmdStdin, stdin) - cmdStdin.Close() return err }) } From 0ebdca5e6144b0176a4b161cb1bd40efc0dc7efe Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Fri, 29 Mar 2013 10:56:49 -0700 Subject: [PATCH 15/47] Add unit test for multiple attach / restart --- container_test.go | 112 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 111 insertions(+), 1 deletion(-) diff --git a/container_test.go b/container_test.go index fb7cdc2efd..cc2661190f 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,7 +200,6 @@ func TestCommitRun(t *testing.T) { t.Fatal(err) } defer runtime.Destroy(container2) - stdout, err := container2.StdoutPipe() stderr, err := container2.StderrPipe() if err := container2.Start(); err != nil { From 68278b66c5294690d051d6a0fe157e2b965d7b1a Mon Sep 17 00:00:00 2001 From: Robert Obryk Date: Fri, 29 Mar 2013 22:09:25 +0100 Subject: [PATCH 16/47] Added a timeout to TestCmdStreamLargeStderr. --- archive_test.go | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) 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") } } From 2d4c5ddbdd3258dfd079e3e1f38253e4e6e5a16b Mon Sep 17 00:00:00 2001 From: Caleb Spare Date: Fri, 29 Mar 2013 23:58:30 -0700 Subject: [PATCH 17/47] Gofmt. --- term/termios_darwin.go | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) 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 +} From 8cceafc8348f8e9de71b09274458848cd8e8de6d Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Sat, 30 Mar 2013 06:55:47 -0700 Subject: [PATCH 18/47] Add unit test for the #125 scenario. Reattach after client disconnect on stdin allocated container --- commands_test.go | 128 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 128 insertions(+) create mode 100644 commands_test.go diff --git a/commands_test.go b/commands_test.go new file mode 100644 index 0000000000..50f5131a5e --- /dev/null +++ b/commands_test.go @@ -0,0 +1,128 @@ +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(chan bool)) { + c := make(chan bool) + + // Make sure we are not too long + go func() { + time.Sleep(d) + c <- true + }() + go f(c) + if timeout := <-c; timeout { + t.Fatalf("Timeout: %s", 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 +} + +// Test the behavior of a client disconnection. +// We expect a client disconnect to leave the stdin of the container open +// Therefore a process will keep his stdin open when a client disconnects +func TestReattachAfterDisconnect(t *testing.T) { + runtime, err := newTestRuntime() + if err != nil { + t.Fatal(err) + } + defer nuke(runtime) + + // FIXME: low down the timeout (after #230) + setTimeout(t, "TestReattachAfterDisconnect", 12*time.Second, func(timeout chan bool) { + + 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("CmdRun should generate a read/write on closed pipe error. No error found.") + } + close(c1) + }() + + 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) + } + + container := runtime.containers.Back().Value.(*Container) + + // Recreate the pipes + stdin, stdinPipe = io.Pipe() + stdout, stdoutPipe = io.Pipe() + + // Attach to it + c2 := make(chan struct{}) + go func() { + if err := srv.CmdAttach(stdin, stdoutPipe, container.Id); err != nil { + t.Fatal(err) + } + close(c2) + }() + + if err := assertPipe("hello\n", "hello", stdout, stdinPipe, 15); err != nil { + t.Fatal(err) + } + + // Close pipes + if err := closeWrap(stdin, stdinPipe, stdout, stdoutPipe); err != nil { + t.Fatal(err) + } + + // FIXME: when #230 will be finished, send SIGINT instead of SIGTERM + // we expect cat to stay alive so SIGTERM will have no effect + // and Stop will timeout + if err := container.Stop(); err != nil { + t.Fatal(err) + } + // Wait for run and attach to finish + <-c1 + <-c2 + + // Finished, no timeout + timeout <- false + }) +} From 5252ab697c927d7b51109a1bb1a68d6a3dfc88b3 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Sat, 30 Mar 2013 09:05:53 -0700 Subject: [PATCH 19/47] Store the master ptys in order to close them when the process dies (#228) --- container.go | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/container.go b/container.go index d128ce98dd..7799ff8053 100644 --- a/container.go +++ b/container.go @@ -40,6 +40,10 @@ type Container struct { stdin io.ReadCloser stdinPipe io.WriteCloser + ptyStdinMaster io.Closer + ptyStdoutMaster io.Closer + ptyStderrMaster io.Closer + runtime *Runtime } @@ -151,12 +155,14 @@ 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 @@ -181,6 +187,7 @@ func (container *Container) startPty() error { 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" @@ -380,6 +387,23 @@ func (container *Container) monitor() { 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) } From 7efde5eb8350ad709608e619b01af4108cc9d567 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Sat, 30 Mar 2013 09:07:54 -0700 Subject: [PATCH 20/47] Fix a scope issue preventing the close of slave stdin pty (#228) --- container.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/container.go b/container.go index 7799ff8053..dc58d549a2 100644 --- a/container.go +++ b/container.go @@ -183,7 +183,8 @@ func (container *Container) startPty() error { // 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 } From 47607494027be3d37cbb01a07e026e99f9c5151b Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Sat, 30 Mar 2013 09:08:53 -0700 Subject: [PATCH 21/47] Close the containers stdin when the process dies --- container.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/container.go b/container.go index dc58d549a2..ccab0f7494 100644 --- a/container.go +++ b/container.go @@ -382,6 +382,11 @@ func (container *Container) monitor() { if err := container.releaseNetwork(); err != nil { log.Printf("%v: Failed to release network: %v", container.Id, err) } + 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) } From b336d928febc0f9212d56b28430bbdf37d2cfcda Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Sat, 30 Mar 2013 09:47:09 -0700 Subject: [PATCH 22/47] Make sure to close all pipes when detaching client (#228) --- commands.go | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/commands.go b/commands.go index a66e4fb822..b48aa4af9b 100644 --- a/commands.go +++ b/commands.go @@ -790,6 +790,16 @@ func (srv *Server) CmdAttach(stdin io.ReadCloser, stdout io.Writer, args ...stri if container == nil { 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 container.Config.OpenStdin { cStdin, err := container.StdinPipe() @@ -800,14 +810,20 @@ func (srv *Server) CmdAttach(stdin io.ReadCloser, stdout io.Writer, args ...stri 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]") }() } - cStdout, err := container.StdoutPipe() - if err != nil { - return err - } wg.Add(1) go func() { Debugf("Begin stdout pipe [attach]") @@ -815,10 +831,6 @@ func (srv *Server) CmdAttach(stdin io.ReadCloser, stdout io.Writer, args ...stri wg.Add(-1) Debugf("End of stdout pipe [attach]") }() - cStderr, err := container.StderrPipe() - if err != nil { - return err - } wg.Add(1) go func() { Debugf("Begin stderr pipe [attach]") From 99f9b697165f1f081499b45b7789508dcf0c2553 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Sat, 30 Mar 2013 10:33:10 -0700 Subject: [PATCH 23/47] Add debug infos in CmdInfo to know the amount of fds and goroutines in use --- commands.go | 6 ++++++ utils.go | 10 ++++++++++ 2 files changed, 16 insertions(+) diff --git a/commands.go b/commands.go index b48aa4af9b..891d3d10c6 100644 --- a/commands.go +++ b/commands.go @@ -203,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.Fprintf(stdout, "debug mode enabled\n") + fmt.Fprintf(stdout, "fds: %d\ngoroutines: %d\n", getTotalUsedFds(), runtime.NumGoroutine()) return nil } diff --git a/utils.go b/utils.go index fbbad34081..3c6c3c91ee 100644 --- a/utils.go +++ b/utils.go @@ -7,6 +7,7 @@ import ( "fmt" "github.com/dotcloud/docker/rcli" "io" + "io/ioutil" "net/http" "os" "os/exec" @@ -260,3 +261,12 @@ func (w *writeBroadcaster) Close() error { 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 +} From 0b9a3c86a26b4b7eb463a6c73cb030bc851fdd64 Mon Sep 17 00:00:00 2001 From: Solomon Hykes Date: Sun, 31 Mar 2013 02:02:01 -0700 Subject: [PATCH 24/47] Show shorthand container IDs for convenience. Shorthand IDs (or any non-conflicting prefix) can be used to lookup containers --- commands.go | 12 ++++---- container.go | 12 ++++++++ runtime.go | 12 +++++++- utils.go | 64 ++++++++++++++++++++++++++++++++++++++++ utils_test.go | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 175 insertions(+), 7 deletions(-) diff --git a/commands.go b/commands.go index 2dcccb48a4..30c23ed301 100644 --- a/commands.go +++ b/commands.go @@ -226,7 +226,7 @@ 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 fmt.Errorf("No such container: %s", name) } @@ -248,7 +248,7 @@ 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 fmt.Errorf("No such container: %s", name) } @@ -270,7 +270,7 @@ 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 fmt.Errorf("No such container: %s", name) } @@ -659,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", @@ -674,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 { @@ -965,7 +965,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/container.go b/container.go index ccab0f7494..a03614c011 100644 --- a/container.go +++ b/container.go @@ -555,6 +555,18 @@ 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 { + shortLen := 12 + if len(container.Id) < shortLen { + shortLen = len(container.Id) + } + return container.Id[:shortLen] +} + func (container *Container) logPath(name string) string { return path.Join(container.root, fmt.Sprintf("%s-%s.log", container.Id, name)) } diff --git a/runtime.go b/runtime.go index 37726da0ec..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] } @@ -142,6 +148,7 @@ func (runtime *Runtime) Register(container *Container) error { } // done runtime.containers.PushBack(container) + runtime.idIndex.Add(container.Id) return nil } @@ -171,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) @@ -222,6 +230,7 @@ func (runtime *Runtime) restore() error { return nil } +// FIXME: harmonize with NewGraph() func NewRuntime() (*Runtime, error) { return NewRuntimeFromDirectory("/var/lib/docker") } @@ -259,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/utils.go b/utils.go index 3c6c3c91ee..a87544ff3c 100644 --- a/utils.go +++ b/utils.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "github.com/dotcloud/docker/rcli" + "index/suffixarray" "io" "io/ioutil" "net/http" @@ -270,3 +271,66 @@ func getTotalUsedFds() int { } 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 +} 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) + } +} From 8293a0d53350b9ebb584988efe3096c313a13ad4 Mon Sep 17 00:00:00 2001 From: Solomon Hykes Date: Sun, 31 Mar 2013 02:18:04 -0700 Subject: [PATCH 25/47] Bumped version to 0.1.1 --- commands.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/commands.go b/commands.go index 30c23ed301..4cad9f68c1 100644 --- a/commands.go +++ b/commands.go @@ -19,7 +19,7 @@ import ( "unicode" ) -const VERSION = "0.1.0" +const VERSION = "0.1.1" func (srv *Server) Name() string { return "docker" From a6779bcae2df4805b8024c56f81ee3afac46ff93 Mon Sep 17 00:00:00 2001 From: Solomon Hykes Date: Sun, 31 Mar 2013 02:44:56 -0700 Subject: [PATCH 26/47] Revert regression introduced in 81eac415ad03d93596a0d8da92bdccd9ecd4ee63, which caused 'docker run -i' to never close stdin --- commands.go | 1 + 1 file changed, 1 insertion(+) diff --git a/commands.go b/commands.go index 4cad9f68c1..4ccdba67d6 100644 --- a/commands.go +++ b/commands.go @@ -927,6 +927,7 @@ func (srv *Server) CmdRun(stdin io.ReadCloser, stdout io.Writer, args ...string) if !config.Detach { Go(func() error { _, err := io.Copy(cmdStdin, stdin) + cmdStdin.Close() return err }) } From b64dfdd8cd636c5138dbd700f60e41651bae3e15 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Sun, 31 Mar 2013 13:04:41 -0700 Subject: [PATCH 27/47] Add a progress bar to docker pull --- registry.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/registry.go b/registry.go index 3e62ad96ee..e36348fdb4 100644 --- a/registry.go +++ b/registry.go @@ -135,7 +135,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 { From 1fc9405537a1c528a59356ec36189a61db146701 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Sun, 31 Mar 2013 13:53:47 -0700 Subject: [PATCH 28/47] Add progress bar on docker push --- registry.go | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/registry.go b/registry.go index e36348fdb4..24df146f13 100644 --- a/registry.go +++ b/registry.go @@ -267,19 +267,21 @@ 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) From d949e2804a35497bc041537734dfeff150ea7e21 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Sun, 31 Mar 2013 14:15:10 -0700 Subject: [PATCH 29/47] Add a check to avoid double start (resulting in dockerd to panic) and unit test for it --- container.go | 3 +++ container_test.go | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/container.go b/container.go index a03614c011..f49aa80d54 100644 --- a/container.go +++ b/container.go @@ -230,6 +230,9 @@ func (container *Container) start() error { } 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 } diff --git a/container_test.go b/container_test.go index fae1c3c19c..fdc7f5703a 100644 --- a/container_test.go +++ b/container_test.go @@ -231,6 +231,40 @@ func TestCommitRun(t *testing.T) { } } +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") + } +} + func TestRun(t *testing.T) { runtime, err := newTestRuntime() if err != nil { From cfeed391d76105e4131a021d2005966d01eb4e68 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Sun, 31 Mar 2013 20:52:35 -0700 Subject: [PATCH 30/47] Change the commands unit tests in order to reflect the proper behaviour of CmdRun --- commands_test.go | 102 +++++++++++++++++------------------------------ 1 file changed, 37 insertions(+), 65 deletions(-) diff --git a/commands_test.go b/commands_test.go index 6c05bfe77d..5a9117eb12 100644 --- a/commands_test.go +++ b/commands_test.go @@ -24,7 +24,7 @@ func closeWrap(args ...io.Closer) error { return nil } -func setTimeout(t *testing.T, msg string, d time.Duration, f func(chan bool)) { +func setTimeout(t *testing.T, msg string, d time.Duration, f func()) { c := make(chan bool) // Make sure we are not too long @@ -32,9 +32,12 @@ func setTimeout(t *testing.T, msg string, d time.Duration, f func(chan bool)) { time.Sleep(d) c <- true }() - go f(c) - if timeout := <-c; timeout { - t.Fatalf("Timeout: %s", msg) + go func() { + f() + c <- false + }() + if <-c { + t.Fatal(msg) } } @@ -54,77 +57,46 @@ func assertPipe(input, output string, r io.Reader, w io.Writer, count int) error return nil } -/* -// Test the behavior of a client disconnection. -// We expect a client disconnect to leave the stdin of the container open -// Therefore a process will keep his stdin open when a client disconnects -func TestReattachAfterDisconnect(t *testing.T) { +// 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) - // FIXME: low down the timeout (after #230) - setTimeout(t, "TestReattachAfterDisconnect", 12*time.Second, func(timeout chan bool) { + srv := &Server{runtime: 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("CmdRun should generate a read/write on closed pipe error. No error found.") - } - close(c1) - }() + 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) - } - - container := runtime.containers.Back().Value.(*Container) - - // Recreate the pipes - stdin, stdinPipe = io.Pipe() - stdout, stdoutPipe = io.Pipe() - - // Attach to it - c2 := make(chan struct{}) - go func() { - if err := srv.CmdAttach(stdin, stdoutPipe, container.Id); err != nil { - t.Fatal(err) - } - close(c2) - }() - - if err := assertPipe("hello\n", "hello", stdout, stdinPipe, 15); err != nil { - t.Fatal(err) - } - - // Close pipes - if err := closeWrap(stdin, stdinPipe, stdout, stdoutPipe); err != nil { - t.Fatal(err) - } - - // FIXME: when #230 will be finished, send SIGINT instead of SIGTERM - // we expect cat to stay alive so SIGTERM will have no effect - // and Stop will timeout - if err := container.Stop(); err != nil { - t.Fatal(err) - } - // Wait for run and attach to finish - <-c1 - <-c2 - - // Finished, no timeout - timeout <- false }) + + // 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") + } } -*/ From 6f9a67a7c7cb717ad1a575df3e4c0fd2ec8bc651 Mon Sep 17 00:00:00 2001 From: Dominik Honnef Date: Sat, 30 Mar 2013 23:32:10 +0100 Subject: [PATCH 31/47] Make IP allocator lazy Instead of allocating all possible IPs in advance, generate them as needed. A loop will cycle through all possible IPs in sequential order, allocating them as needed and marking them as in use. Once the loop exhausts all IPs, it will wrap back to the beginning. IPs that are already in use will be skipped. When an IP is released, it will be cleared and be available for allocation again. Two decisions went into this design: 1) Minimize memory footprint by only allocating IPs that are actually in use 2) Minimize reuse of released IP addresses to avoid sending traffic to the wrong containers As a side effect, the functions for IP/Mask<->int conversion have been rewritten to never be able to fail in order to reduce the amount of error returns. Fixes gh-231 --- AUTHORS | 1 + container.go | 9 +-- network.go | 169 ++++++++++++++++++++++++++---------------------- network_test.go | 140 ++++++++++++++++++++++++++++++--------- 4 files changed, 203 insertions(+), 116 deletions(-) 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/container.go b/container.go index f49aa80d54..c605833399 100644 --- a/container.go +++ b/container.go @@ -363,11 +363,10 @@ 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() { @@ -382,9 +381,7 @@ func (container *Container) monitor() { 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) 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) } } From ad1e8a9b0f0e57327d71827c1161c21f48910e42 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Sun, 31 Mar 2013 21:48:18 -0700 Subject: [PATCH 32/47] Add unit test for CmdAttach --- commands_test.go | 66 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/commands_test.go b/commands_test.go index 5a9117eb12..ebbe88c637 100644 --- a/commands_test.go +++ b/commands_test.go @@ -100,3 +100,69 @@ func TestRunDisconnect(t *testing.T) { 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() +} From 54443c092caf9c1e11c47ea30400084d009af6e9 Mon Sep 17 00:00:00 2001 From: Solomon Hykes Date: Sun, 31 Mar 2013 22:04:59 -0700 Subject: [PATCH 33/47] gofmt --- container.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/container.go b/container.go index c605833399..ee2bb1b3b1 100644 --- a/container.go +++ b/container.go @@ -363,7 +363,7 @@ func (container *Container) allocateNetwork() error { return nil } -func (container *Container) releaseNetwork() { +func (container *Container) releaseNetwork() { container.network.Release() container.network = nil container.NetworkSettings = &NetworkSettings{} From a52a28b60946463e8208b8cd5d737ba79f23a8b8 Mon Sep 17 00:00:00 2001 From: Solomon Hykes Date: Sun, 31 Mar 2013 22:05:14 -0700 Subject: [PATCH 34/47] Temporarily disable a broken test (waiting for @creack to fix it), and silence a warning which pollutes unit tests but is complicated to fix --- commands_test.go | 2 ++ container.go | 8 +++++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/commands_test.go b/commands_test.go index 50f5131a5e..6c05bfe77d 100644 --- a/commands_test.go +++ b/commands_test.go @@ -54,6 +54,7 @@ func assertPipe(input, output string, r io.Reader, w io.Writer, count int) error return nil } +/* // Test the behavior of a client disconnection. // We expect a client disconnect to leave the stdin of the container open // Therefore a process will keep his stdin open when a client disconnects @@ -126,3 +127,4 @@ func TestReattachAfterDisconnect(t *testing.T) { timeout <- false }) } +*/ diff --git a/container.go b/container.go index ee2bb1b3b1..502109ef96 100644 --- a/container.go +++ b/container.go @@ -422,7 +422,13 @@ func (container *Container) monitor() { // Report status back container.State.setStopped(exitCode) if err := container.ToDisk(); err != nil { - log.Printf("%s: Failed to dump configuration to the disk: %s", container.Id, err) + // 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) } } From 1632566ecb133fb0853aec6715c5c06a4a9e41da Mon Sep 17 00:00:00 2001 From: Solomon Hykes Date: Sun, 31 Mar 2013 22:11:55 -0700 Subject: [PATCH 35/47] Show shorthand image IDs for convenience. Shorthand IDs (or any non-conflicting prefix) can be used to lookup images --- commands.go | 14 +++++++------- container.go | 6 +----- graph.go | 42 ++++++++++++++++++++++++++++++++++++------ image.go | 4 ++++ tags.go | 2 +- utils.go | 12 ++++++++++++ 6 files changed, 61 insertions(+), 19 deletions(-) diff --git a/commands.go b/commands.go index 4ccdba67d6..105f76be5e 100644 --- a/commands.go +++ b/commands.go @@ -372,7 +372,7 @@ func (srv *Server) CmdHistory(stdin io.ReadCloser, stdout io.Writer, args ...str fmt.Fprintf(w, "ID\tCREATED\tCREATED BY\n") 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, " "), ) @@ -458,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 } @@ -591,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), } { @@ -603,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")) } } } @@ -614,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), } { @@ -626,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")) } } } @@ -700,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 } diff --git a/container.go b/container.go index 502109ef96..a7094bf5ec 100644 --- a/container.go +++ b/container.go @@ -566,11 +566,7 @@ func (container *Container) Unmount() error { // 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 { - shortLen := 12 - if len(container.Id) < shortLen { - shortLen = len(container.Id) - } - return container.Id[:shortLen] + return TruncateId(container.Id) } func (container *Container) logPath(name string) string { diff --git a/graph.go b/graph.go index 09bc1d5bed..ca126dc0e5 100644 --- a/graph.go +++ b/graph.go @@ -12,7 +12,8 @@ import ( // A Graph is a store for versioned filesystem images, and the relationship between them. type Graph struct { - Root string + Root string + idIndex *TruncIndex } // NewGraph instanciates a new graph at the given root path in the filesystem. @@ -26,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 @@ -47,7 +65,11 @@ func (graph *Graph) Exists(id string) bool { } // Get returns the image with the given id, or an error if the image doesn't exist. -func (graph *Graph) Get(id string) (*Image, error) { +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 { @@ -101,6 +123,7 @@ func (graph *Graph) Register(layerData Archive, img *Image) error { return err } img.graph = graph + graph.idIndex.Add(img.Id) return nil } @@ -143,8 +166,11 @@ func (graph *Graph) Delete(id string) error { 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 { @@ -170,7 +196,11 @@ func (graph *Graph) Undelete(id string) error { 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 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/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/utils.go b/utils.go index a87544ff3c..381af1fe38 100644 --- a/utils.go +++ b/utils.go @@ -334,3 +334,15 @@ func (idx *TruncIndex) Get(s string) (string, error) { } 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] +} From 91b1f9eee9729fc9845eaecdad88a7a5050a33de Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Sun, 31 Mar 2013 22:42:10 -0700 Subject: [PATCH 36/47] Avoid destroy() timeout by closing stdin in TestStart() --- container_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/container_test.go b/container_test.go index 45d334ca22..571a093767 100644 --- a/container_test.go +++ b/container_test.go @@ -263,6 +263,10 @@ func TestStart(t *testing.T) { 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) { From 6b59cc8a1031d78530efb7582c936ba8221da847 Mon Sep 17 00:00:00 2001 From: Jonathan Rudenberg Date: Mon, 1 Apr 2013 09:49:51 -0400 Subject: [PATCH 37/47] Close HTTP response bodies Fixes #285. --- registry.go | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/registry.go b/registry.go index 3e62ad96ee..ea2f1459f9 100644 --- a/registry.go +++ b/registry.go @@ -183,10 +183,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 @@ -237,6 +237,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 +257,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) @@ -286,6 +291,7 @@ func (graph *Graph) PushImage(stdout io.Writer, imgOrig *Image, authConfig *auth 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 +321,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: From 7830cf91663f071315c3d0406bc3de0f8a58c2b6 Mon Sep 17 00:00:00 2001 From: Caleb Spare Date: Sat, 30 Mar 2013 00:20:40 -0700 Subject: [PATCH 38/47] Don't use a strings.Reader where a bytes.Reader will do. There are several places where a []byte is converted to a string and then fed into strings.NewReader(). --- auth/auth.go | 3 ++- registry.go | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/auth/auth.go b/auth/auth.go index 045176ed48..e6aa048136 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) diff --git a/registry.go b/registry.go index 3e62ad96ee..4c51265981 100644 --- a/registry.go +++ b/registry.go @@ -1,6 +1,7 @@ package docker import ( + "bytes" "encoding/json" "fmt" "github.com/dotcloud/docker/auth" @@ -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 { @@ -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 From 15b30881570369564bde3f7d8faf4491b2ebfaab Mon Sep 17 00:00:00 2001 From: Caleb Spare Date: Sat, 30 Mar 2013 00:22:56 -0700 Subject: [PATCH 39/47] Don't convert []byte to string unnecessarily. --- auth/auth.go | 4 ++-- container_test.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/auth/auth.go b/auth/auth.go index e6aa048136..886d210f61 100644 --- a/auth/auth.go +++ b/auth/auth.go @@ -152,11 +152,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/container_test.go b/container_test.go index fdc7f5703a..45d334ca22 100644 --- a/container_test.go +++ b/container_test.go @@ -227,7 +227,7 @@ func TestCommitRun(t *testing.T) { t.Fatal(err) } if string(output) != "hello\n" { - t.Fatalf("Unexpected output. Expected %s, received: %s (err: %s)", "hello\n", string(output), string(output2)) + t.Fatalf("Unexpected output. Expected %s, received: %s (err: %s)", "hello\n", output, output2) } } @@ -885,7 +885,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) From c298a91f95dafb056cede9036f5e81f259b8f3e9 Mon Sep 17 00:00:00 2001 From: Caleb Spare Date: Sat, 30 Mar 2013 00:23:12 -0700 Subject: [PATCH 40/47] Use a *println or *print function instead of *printf where appropriate. --- commands.go | 14 +++++++------- container.go | 4 ++-- rcli/http.go | 2 +- rcli/tcp.go | 4 ++-- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/commands.go b/commands.go index 105f76be5e..ad9688d9c8 100644 --- a/commands.go +++ b/commands.go @@ -146,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 } @@ -207,7 +207,7 @@ func (srv *Server) CmdInfo(stdin io.ReadCloser, stdout io.Writer, args ...string if !rcli.DEBUG_FLAG { return nil } - fmt.Fprintf(stdout, "debug mode enabled\n") + fmt.Fprintln(stdout, "debug mode enabled") fmt.Fprintf(stdout, "fds: %d\ngoroutines: %d\n", getTotalUsedFds(), runtime.NumGoroutine()) return nil } @@ -369,7 +369,7 @@ 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.ShortId()), @@ -438,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) @@ -564,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 @@ -647,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 { diff --git a/container.go b/container.go index a7094bf5ec..8f993a35e1 100644 --- a/container.go +++ b/container.go @@ -458,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 } 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() }() From 13d2b086386196e9f4f03e4a18d508a65f6fc5ff Mon Sep 17 00:00:00 2001 From: Caleb Spare Date: Sat, 30 Mar 2013 00:22:24 -0700 Subject: [PATCH 41/47] A few spelling/grammar corrections. --- auth/auth.go | 1 + commands.go | 2 +- graph.go | 20 ++++++++++---------- registry.go | 2 +- 4 files changed, 13 insertions(+), 12 deletions(-) diff --git a/auth/auth.go b/auth/auth.go index 886d210f61..2c282af218 100644 --- a/auth/auth.go +++ b/auth/auth.go @@ -131,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) diff --git a/commands.go b/commands.go index ad9688d9c8..a98a27cbb9 100644 --- a/commands.go +++ b/commands.go @@ -65,7 +65,7 @@ func (srv *Server) CmdLogin(stdin io.ReadCloser, stdout io.Writer, args ...strin // 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 a + // 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 diff --git a/graph.go b/graph.go index ca126dc0e5..0f65aa205f 100644 --- a/graph.go +++ b/graph.go @@ -10,13 +10,13 @@ import ( "time" ) -// A Graph is a store for versioned filesystem images, and the relationship between them. +// A Graph is a store for versioned filesystem images and the relationship between them. type Graph struct { Root string idIndex *TruncIndex } -// NewGraph instanciates a new graph at the given root path in the filesystem. +// 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) @@ -140,14 +140,14 @@ func (graph *Graph) Mktemp(id string) (string, error) { } // Garbage returns the "garbage", a staging area for deleted images. -// This allows images ot be deleted atomically by os.Rename(), instead of -// os.RemoveAll() which is prone to race conditions +// 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: @@ -190,7 +190,7 @@ func (graph *Graph) Delete(id string) error { return nil } -// Undelete moves an image back from the garbage to the main graph +// 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 { @@ -203,7 +203,7 @@ func (graph *Graph) Undelete(id string) error { return nil } -// GarbageCollect definitely deletes all images moved to the garbage +// GarbageCollect definitely deletes all images moved to the garbage. func (graph *Graph) GarbageCollect() error { garbage, err := graph.Garbage() if err != nil { @@ -212,7 +212,7 @@ func (graph *Graph) GarbageCollect() error { return os.RemoveAll(garbage.Root) } -// Map returns a list of all images in the graph, addressable by ID +// 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() @@ -226,7 +226,7 @@ func (graph *Graph) Map() (map[string]*Image, error) { return images, nil } -// All returns a list of all images in the graph +// 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) { diff --git a/registry.go b/registry.go index 4c51265981..63cfd40350 100644 --- a/registry.go +++ b/registry.go @@ -21,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 } From 887f509d1dd2a5e34fbfb19ccd2c4b1eb3127dbd Mon Sep 17 00:00:00 2001 From: Caleb Spare Date: Sat, 30 Mar 2013 00:36:27 -0700 Subject: [PATCH 42/47] Don't use an interface{} where a string will do. --- commands.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/commands.go b/commands.go index a98a27cbb9..0c896f29ba 100644 --- a/commands.go +++ b/commands.go @@ -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"}, From 9b13d21fc980b5ac12e035d9c8cbe3ef2c36a079 Mon Sep 17 00:00:00 2001 From: Caleb Spare Date: Mon, 1 Apr 2013 13:05:00 -0700 Subject: [PATCH 43/47] Add a 'fmt' target to the Makefile. A convenience for gofmting all the code, including subpackages. --- Makefile | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Makefile b/Makefile index e716762d31..6a9719078e 100644 --- a/Makefile +++ b/Makefile @@ -41,3 +41,6 @@ endif test: all @(cd $(DOCKER_DIR); sudo -E go test $(GO_OPTIONS)) + +fmt: + @find . -name "*.go" -exec gofmt -l -w {} \; From cc2558bf10817b086fcb9783474d47db24d2cb3a Mon Sep 17 00:00:00 2001 From: Paul Hammond Date: Mon, 1 Apr 2013 14:34:12 -0700 Subject: [PATCH 44/47] Fix mkimage-busybox --- contrib/mkimage-busybox.sh | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) 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. From 650dff73bdd48058ec0186cbda90683ff2287df0 Mon Sep 17 00:00:00 2001 From: Francisco Souza Date: Mon, 1 Apr 2013 18:50:25 -0300 Subject: [PATCH 45/47] makefile: simplify "fmt" target, and include -s flag --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 6a9719078e..4a3e6567ff 100644 --- a/Makefile +++ b/Makefile @@ -43,4 +43,4 @@ test: all @(cd $(DOCKER_DIR); sudo -E go test $(GO_OPTIONS)) fmt: - @find . -name "*.go" -exec gofmt -l -w {} \; + @gofmt -s -l -w . From 7ad2e022fb737286b5803066bde6919e2d544c4e Mon Sep 17 00:00:00 2001 From: Solomon Hykes Date: Mon, 1 Apr 2013 16:02:02 -0700 Subject: [PATCH 46/47] Add test to reproduce issue #306 --- graph_test.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) 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) From ff5cb8e864b87d3976b368ae544010f5736d1da2 Mon Sep 17 00:00:00 2001 From: Solomon Hykes Date: Mon, 1 Apr 2013 16:04:44 -0700 Subject: [PATCH 47/47] Images can be removed by short-hand ID. Solves #306. --- graph.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/graph.go b/graph.go index 0f65aa205f..0c84dc4252 100644 --- a/graph.go +++ b/graph.go @@ -161,7 +161,11 @@ func isNotEmpty(err error) bool { } // Delete atomically removes an image from the graph. -func (graph *Graph) Delete(id string) error { +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