From 014494a3f46f8e4be6e288860c23726665ea81da Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Thu, 28 Nov 2013 16:12:45 -0800 Subject: [PATCH 01/14] Make race condition more obvious by performing more asserts Upstream-commit: ad43d88af5bda8dc5b3d06f64de380bb985191ba Component: engine --- components/engine/integration/api_test.go | 6 +++--- components/engine/integration/commands_test.go | 14 +++++++------- components/engine/integration/container_test.go | 2 +- components/engine/integration/server_test.go | 4 ++-- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/components/engine/integration/api_test.go b/components/engine/integration/api_test.go index a66cbe561f..bd6280af8b 100644 --- a/components/engine/integration/api_test.go +++ b/components/engine/integration/api_test.go @@ -454,7 +454,7 @@ func TestGetContainersTop(t *testing.T) { // Make sure sh spawn up cat setTimeout(t, "read/write assertion timed out", 2*time.Second, func() { in, out := containerAttach(eng, containerID, t) - if err := assertPipe("hello\n", "hello", out, in, 15); err != nil { + if err := assertPipe("hello\n", "hello", out, in, 150); err != nil { t.Fatal(err) } }) @@ -877,7 +877,7 @@ func TestPostContainersAttach(t *testing.T) { }) setTimeout(t, "read/write assertion timed out", 2*time.Second, func() { - if err := assertPipe("hello\n", string([]byte{1, 0, 0, 0, 0, 0, 0, 6})+"hello", stdout, stdinPipe, 15); err != nil { + if err := assertPipe("hello\n", string([]byte{1, 0, 0, 0, 0, 0, 0, 6})+"hello", stdout, stdinPipe, 150); err != nil { t.Fatal(err) } }) @@ -956,7 +956,7 @@ func TestPostContainersAttachStderr(t *testing.T) { }) setTimeout(t, "read/write assertion timed out", 2*time.Second, func() { - if err := assertPipe("hello\n", string([]byte{2, 0, 0, 0, 0, 0, 0, 6})+"hello", stdout, stdinPipe, 15); err != nil { + if err := assertPipe("hello\n", string([]byte{2, 0, 0, 0, 0, 0, 0, 6})+"hello", stdout, stdinPipe, 150); err != nil { t.Fatal(err) } }) diff --git a/components/engine/integration/commands_test.go b/components/engine/integration/commands_test.go index 37bedf7f0c..7daebf3cd2 100644 --- a/components/engine/integration/commands_test.go +++ b/components/engine/integration/commands_test.go @@ -213,7 +213,7 @@ func TestRunExit(t *testing.T) { }() setTimeout(t, "Read/Write assertion timed out", 2*time.Second, func() { - if err := assertPipe("hello\n", "hello", stdout, stdinPipe, 15); err != nil { + if err := assertPipe("hello\n", "hello", stdout, stdinPipe, 150); err != nil { t.Fatal(err) } }) @@ -268,7 +268,7 @@ func TestRunDisconnect(t *testing.T) { }() setTimeout(t, "Read/Write assertion timed out", 2*time.Second, func() { - if err := assertPipe("hello\n", "hello", stdout, stdinPipe, 15); err != nil { + if err := assertPipe("hello\n", "hello", stdout, stdinPipe, 150); err != nil { t.Fatal(err) } }) @@ -330,7 +330,7 @@ func TestRunDisconnectTty(t *testing.T) { container := globalRuntime.List()[0] setTimeout(t, "Read/Write assertion timed out", 2*time.Second, func() { - if err := assertPipe("hello\n", "hello", stdout, stdinPipe, 15); err != nil { + if err := assertPipe("hello\n", "hello", stdout, stdinPipe, 150); err != nil { t.Fatal(err) } }) @@ -432,7 +432,7 @@ func TestRunDetach(t *testing.T) { }() setTimeout(t, "First read/write assertion timed out", 2*time.Second, func() { - if err := assertPipe("hello\n", "hello", stdout, stdinPipe, 15); err != nil { + if err := assertPipe("hello\n", "hello", stdout, stdinPipe, 150); err != nil { t.Fatal(err) } }) @@ -513,7 +513,7 @@ func TestAttachDetach(t *testing.T) { }() setTimeout(t, "First read/write assertion timed out", 2*time.Second, func() { - if err := assertPipe("hello\n", "hello", stdout, stdinPipe, 15); err != nil { + if err := assertPipe("hello\n", "hello", stdout, stdinPipe, 150); err != nil { if err != io.ErrClosedPipe { t.Fatal(err) } @@ -575,7 +575,7 @@ func TestAttachDetachTruncatedID(t *testing.T) { }() setTimeout(t, "First read/write assertion timed out", 2*time.Second, func() { - if err := assertPipe("hello\n", "hello", stdout, stdinPipe, 15); err != nil { + if err := assertPipe("hello\n", "hello", stdout, stdinPipe, 150); err != nil { if err != io.ErrClosedPipe { t.Fatal(err) } @@ -648,7 +648,7 @@ func TestAttachDisconnect(t *testing.T) { }() setTimeout(t, "First read/write assertion timed out", 2*time.Second, func() { - if err := assertPipe("hello\n", "hello", stdout, stdinPipe, 15); err != nil { + if err := assertPipe("hello\n", "hello", stdout, stdinPipe, 150); err != nil { t.Fatal(err) } }) diff --git a/components/engine/integration/container_test.go b/components/engine/integration/container_test.go index 93a00a7286..05eb48728c 100644 --- a/components/engine/integration/container_test.go +++ b/components/engine/integration/container_test.go @@ -462,7 +462,7 @@ func TestKillDifferentUser(t *testing.T) { setTimeout(t, "read/write assertion timed out", 2*time.Second, func() { out, _ := container.StdoutPipe() in, _ := container.StdinPipe() - if err := assertPipe("hello\n", "hello", out, in, 15); err != nil { + if err := assertPipe("hello\n", "hello", out, in, 150); err != nil { t.Fatal(err) } }) diff --git a/components/engine/integration/server_test.go b/components/engine/integration/server_test.go index 494e23fef3..24e109ab76 100644 --- a/components/engine/integration/server_test.go +++ b/components/engine/integration/server_test.go @@ -183,11 +183,11 @@ func TestCreateStartRestartStopStartKillRm(t *testing.T) { t.Fatal(err) } - if err := srv.ContainerRestart(id, 15); err != nil { + if err := srv.ContainerRestart(id, 150); err != nil { t.Fatal(err) } - if err := srv.ContainerStop(id, 15); err != nil { + if err := srv.ContainerStop(id, 150); err != nil { t.Fatal(err) } From 04d46060bc1c40bb4aef4ac5c01b31c8954bcb26 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Thu, 28 Nov 2013 16:57:51 -0800 Subject: [PATCH 02/14] Make CopyEscapable consistent with Copy and return `nil` in case of success instead of io.EOF Upstream-commit: 77c94175bdc387e7f43876f7097b970f67116054 Component: engine --- components/engine/utils/utils.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/engine/utils/utils.go b/components/engine/utils/utils.go index cfdc73bb2e..f62aa12ff5 100644 --- a/components/engine/utils/utils.go +++ b/components/engine/utils/utils.go @@ -543,7 +543,7 @@ func CopyEscapable(dst io.Writer, src io.ReadCloser) (written int64, err error) if err := src.Close(); err != nil { return 0, err } - return 0, io.EOF + return 0, nil } } // ---- End of docker From 9f9072c9ad5071d4c9f602b142cecdcd4e68520f Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Fri, 29 Nov 2013 07:39:51 -0800 Subject: [PATCH 03/14] Make sure the container is running before testing against it (TestAttachDetach) Upstream-commit: e535f544c7bc9c32b23e5505110a5513ff36be5a Component: engine --- components/engine/integration/commands_test.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/components/engine/integration/commands_test.go b/components/engine/integration/commands_test.go index 7daebf3cd2..8bc1c99ec7 100644 --- a/components/engine/integration/commands_test.go +++ b/components/engine/integration/commands_test.go @@ -481,6 +481,17 @@ func TestAttachDetach(t *testing.T) { var container *docker.Container + setTimeout(t, "Waiting for the container to be started timed out", 10*time.Second, func() { + for { + l := globalRuntime.List() + if len(l) == 1 && l[0].State.IsRunning() { + container = l[0] + break + } + time.Sleep(10 * time.Millisecond) + } + }) + setTimeout(t, "Reading container's id timed out", 10*time.Second, func() { buf := make([]byte, 1024) n, err := stdout.Read(buf) @@ -488,8 +499,6 @@ func TestAttachDetach(t *testing.T) { t.Fatal(err) } - container = globalRuntime.List()[0] - if strings.Trim(string(buf[:n]), " \r\n") != container.ID { t.Fatalf("Wrong ID received. Expect %s, received %s", container.ID, buf[:n]) } From fb39460944229839245f7fad35705dceaa054f99 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Fri, 29 Nov 2013 07:40:44 -0800 Subject: [PATCH 04/14] Add a GetPtyMaster() method to container to retrieve the pty from an other package. We could also have put ptyMaster public, but then we need to ignore it in json otherwise the Marshalling fails. I think it is cleaner that way. Upstream-commit: fbebe20bc69648c046e3818ca744ae246092a782 Component: engine --- components/engine/container.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/components/engine/container.go b/components/engine/container.go index 49cb33b536..3e5f8a8d74 100644 --- a/components/engine/container.go +++ b/components/engine/container.go @@ -24,6 +24,11 @@ import ( "time" ) +var ( + ErrNotATTY = errors.New("The PTY is not a file") + ErrNoTTY = errors.New("No PTY found") +) + type Container struct { sync.Mutex root string // Path to the "home" of the container, including metadata. @@ -1405,3 +1410,13 @@ func (container *Container) Exposes(p Port) bool { _, exists := container.Config.ExposedPorts[p] return exists } + +func (container *Container) GetPtyMaster() (*os.File, error) { + if container.ptyMaster == nil { + return nil, ErrNoTTY + } + if pty, ok := container.ptyMaster.(*os.File); ok { + return pty, nil + } + return nil, ErrNotATTY +} From eab6c90e20b2bb5608e1962a9c244e07d4bb451a Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Fri, 29 Nov 2013 07:42:26 -0800 Subject: [PATCH 05/14] Make the PTY in raw mode before assert test (TestAttachDetach) Upstream-commit: 67e9e0e11bb932ef9113ac91b2c1b0af6ee4db6d Component: engine --- components/engine/integration/commands_test.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/components/engine/integration/commands_test.go b/components/engine/integration/commands_test.go index 8bc1c99ec7..f2b14482b2 100644 --- a/components/engine/integration/commands_test.go +++ b/components/engine/integration/commands_test.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/dotcloud/docker" "github.com/dotcloud/docker/engine" + "github.com/dotcloud/docker/term" "github.com/dotcloud/docker/utils" "io" "io/ioutil" @@ -507,6 +508,17 @@ func TestAttachDetach(t *testing.T) { <-ch }) + pty, err := container.GetPtyMaster() + if err != nil { + t.Fatal(err) + } + + state, err := term.MakeRaw(pty.Fd()) + if err != nil { + t.Fatal(err) + } + defer term.RestoreTerminal(pty.Fd(), state) + stdin, stdinPipe = io.Pipe() stdout, stdoutPipe = io.Pipe() cli = docker.NewDockerCli(stdin, stdoutPipe, ioutil.Discard, testDaemonProto, testDaemonAddr) From 4a151bb6e6a51fb03ace70284d22650e91ab18b5 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Fri, 29 Nov 2013 09:11:20 -0800 Subject: [PATCH 06/14] Actually test the detach (was not the case before) Upstream-commit: 63d6cbe3e4d6f29c2491b0f1f505ef79b7191d8e Component: engine --- components/engine/commands.go | 6 ++++++ components/engine/integration/commands_test.go | 10 +++++----- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/components/engine/commands.go b/components/engine/commands.go index db752447f0..24218d284c 100644 --- a/components/engine/commands.go +++ b/components/engine/commands.go @@ -2394,6 +2394,12 @@ func (cli *DockerCli) hijack(method, path string, setRawTerminal bool, in io.Rea if stdout != nil { receiveStdout = utils.Go(func() (err error) { + defer func() { + if in != nil { + in.Close() + } + }() + // When TTY is ON, use regular copy if setRawTerminal { _, err = io.Copy(stdout, br) diff --git a/components/engine/integration/commands_test.go b/components/engine/integration/commands_test.go index f2b14482b2..75d09facb4 100644 --- a/components/engine/integration/commands_test.go +++ b/components/engine/integration/commands_test.go @@ -542,18 +542,18 @@ func TestAttachDetach(t *testing.T) { }) setTimeout(t, "Escape sequence timeout", 5*time.Second, func() { - stdinPipe.Write([]byte{16, 17}) - if err := stdinPipe.Close(); err != nil { - t.Fatal(err) - } + stdinPipe.Write([]byte{16}) + time.Sleep(100 * time.Millisecond) + stdinPipe.Write([]byte{17}) }) - closeWrap(stdin, stdinPipe, stdout, stdoutPipe) // wait for CmdRun to return setTimeout(t, "Waiting for CmdAttach timed out", 15*time.Second, func() { <-ch }) + closeWrap(stdin, stdinPipe, stdout, stdoutPipe) + time.Sleep(500 * time.Millisecond) if !container.State.IsRunning() { t.Fatal("The detached container should be still running") From caf6f256a41a95678c5bf17e09e95cef37e07aee Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Fri, 29 Nov 2013 09:52:44 -0800 Subject: [PATCH 07/14] Fix term.RestoreTerminal behavior Upstream-commit: aa68656cd3aefe2a64dc259e8d19c72010e4f85b Component: engine --- components/engine/term/term.go | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/components/engine/term/term.go b/components/engine/term/term.go index 8c53a20ca6..50425a8602 100644 --- a/components/engine/term/term.go +++ b/components/engine/term/term.go @@ -1,12 +1,17 @@ package term import ( + "errors" "os" "os/signal" "syscall" "unsafe" ) +var ( + ErrInvalidState = errors.New("Invlide terminal state") +) + type State struct { termios Termios } @@ -47,8 +52,14 @@ func IsTerminal(fd uintptr) bool { // Restore restores the terminal connected to the given file descriptor to a // previous state. func RestoreTerminal(fd uintptr, state *State) error { + if state == nil { + return ErrInvalidState + } _, _, err := syscall.Syscall(syscall.SYS_IOCTL, fd, uintptr(setTermios), uintptr(unsafe.Pointer(&state.termios))) - return err + if err != 0 { + return err + } + return nil } func SaveState(fd uintptr) (*State, error) { From 27de84e1e00cb3175b66787b822aba9ec5400165 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Fri, 29 Nov 2013 09:55:15 -0800 Subject: [PATCH 08/14] Make sure the termcaps are restored after hijack Upstream-commit: c13821ad0bfa596a8bbc06a494500ee39a35e429 Component: engine --- components/engine/commands.go | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/components/engine/commands.go b/components/engine/commands.go index 24218d284c..dbf295cf13 100644 --- a/components/engine/commands.go +++ b/components/engine/commands.go @@ -2392,10 +2392,23 @@ func (cli *DockerCli) hijack(method, path string, setRawTerminal bool, in io.Rea var receiveStdout chan error + var oldState *term.State + + if in != nil && setRawTerminal && cli.isTerminal && os.Getenv("NORAW") == "" { + oldState, err = term.SetRawTerminal(cli.terminalFd) + if err != nil { + return err + } + defer term.RestoreTerminal(cli.terminalFd, oldState) + } + if stdout != nil { receiveStdout = utils.Go(func() (err error) { defer func() { if in != nil { + if setRawTerminal && cli.isTerminal { + term.RestoreTerminal(cli.terminalFd, oldState) + } in.Close() } }() @@ -2411,14 +2424,6 @@ func (cli *DockerCli) hijack(method, path string, setRawTerminal bool, in io.Rea }) } - if in != nil && setRawTerminal && cli.isTerminal && os.Getenv("NORAW") == "" { - oldState, err := term.SetRawTerminal(cli.terminalFd) - if err != nil { - return err - } - defer term.RestoreTerminal(cli.terminalFd, oldState) - } - sendStdin := utils.Go(func() error { if in != nil { io.Copy(rwc, in) From e107dad4846cfc2488a897ce8ddaf3da41f7a890 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Fri, 29 Nov 2013 09:57:59 -0800 Subject: [PATCH 09/14] Create helper function for tests Upstream-commit: 697be6aaa009cd2bea5f07ae0b0780703e6565e1 Component: engine --- .../engine/integration/commands_test.go | 66 ++++++++++++------- 1 file changed, 44 insertions(+), 22 deletions(-) diff --git a/components/engine/integration/commands_test.go b/components/engine/integration/commands_test.go index 75d09facb4..fa8f25836c 100644 --- a/components/engine/integration/commands_test.go +++ b/components/engine/integration/commands_test.go @@ -32,6 +32,47 @@ func closeWrap(args ...io.Closer) error { return nil } +func setRaw(t *testing.T, c *docker.Container) *term.State { + pty, err := c.GetPtyMaster() + if err != nil { + t.Fatal(err) + } + state, err := term.MakeRaw(pty.Fd()) + if err != nil { + t.Fatal(err) + } + return state +} + +func unsetRaw(t *testing.T, c *docker.Container, state *term.State) { + pty, err := c.GetPtyMaster() + if err != nil { + t.Fatal(err) + } + term.RestoreTerminal(pty.Fd(), state) +} + +func waitContainerStart(t *testing.T, timeout time.Duration) *docker.Container { + var container *docker.Container + + setTimeout(t, "Waiting for the container to be started timed out", timeout, func() { + for { + l := globalRuntime.List() + if len(l) == 1 && l[0].State.IsRunning() { + container = l[0] + break + } + time.Sleep(10 * time.Millisecond) + } + }) + + if container == nil { + t.Fatal("An error occured while waiting for the container to start") + } + + return container +} + func setTimeout(t *testing.T, msg string, d time.Duration, f func()) { c := make(chan bool) @@ -480,18 +521,7 @@ func TestAttachDetach(t *testing.T) { } }() - var container *docker.Container - - setTimeout(t, "Waiting for the container to be started timed out", 10*time.Second, func() { - for { - l := globalRuntime.List() - if len(l) == 1 && l[0].State.IsRunning() { - container = l[0] - break - } - time.Sleep(10 * time.Millisecond) - } - }) + container := waitContainerStart(t, 10*time.Second) setTimeout(t, "Reading container's id timed out", 10*time.Second, func() { buf := make([]byte, 1024) @@ -508,16 +538,8 @@ func TestAttachDetach(t *testing.T) { <-ch }) - pty, err := container.GetPtyMaster() - if err != nil { - t.Fatal(err) - } - - state, err := term.MakeRaw(pty.Fd()) - if err != nil { - t.Fatal(err) - } - defer term.RestoreTerminal(pty.Fd(), state) + state := setRaw(t, container) + defer unsetRaw(t, container, state) stdin, stdinPipe = io.Pipe() stdout, stdoutPipe = io.Pipe() From 30abdabd65874dd2a439f3defded80d5531c2ba6 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Fri, 29 Nov 2013 10:03:36 -0800 Subject: [PATCH 10/14] Fix TestAttachDetachTruncatedID (behavior + tty issue) Upstream-commit: 2e6a958612d65a0665a9396372fe82706987d085 Component: engine --- components/engine/integration/commands_test.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/components/engine/integration/commands_test.go b/components/engine/integration/commands_test.go index fa8f25836c..90333a052c 100644 --- a/components/engine/integration/commands_test.go +++ b/components/engine/integration/commands_test.go @@ -601,7 +601,10 @@ func TestAttachDetachTruncatedID(t *testing.T) { } }) - container := globalRuntime.List()[0] + container := waitContainerStart(t, 10*time.Second) + + state := setRaw(t, container) + defer unsetRaw(t, container, state) stdin, stdinPipe = io.Pipe() stdout, stdoutPipe = io.Pipe() @@ -626,17 +629,16 @@ func TestAttachDetachTruncatedID(t *testing.T) { }) setTimeout(t, "Escape sequence timeout", 5*time.Second, func() { - stdinPipe.Write([]byte{16, 17}) - if err := stdinPipe.Close(); err != nil { - t.Fatal(err) - } + stdinPipe.Write([]byte{16}) + time.Sleep(100 * time.Millisecond) + stdinPipe.Write([]byte{17}) }) - closeWrap(stdin, stdinPipe, stdout, stdoutPipe) // wait for CmdRun to return setTimeout(t, "Waiting for CmdAttach timed out", 15*time.Second, func() { <-ch }) + closeWrap(stdin, stdinPipe, stdout, stdoutPipe) time.Sleep(500 * time.Millisecond) if !container.State.IsRunning() { From 90e04ec65d4c91f36afffcfa27acb8b5dbb955dd Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Fri, 29 Nov 2013 10:17:04 -0800 Subject: [PATCH 11/14] Remove an unit test from integrations test Upstream-commit: 2ec1146679598837cd8bab62dc672bcda2a9610c Component: engine --- .../engine/integration/commands_test.go | 20 +------------------ 1 file changed, 1 insertion(+), 19 deletions(-) diff --git a/components/engine/integration/commands_test.go b/components/engine/integration/commands_test.go index 90333a052c..ba9399218a 100644 --- a/components/engine/integration/commands_test.go +++ b/components/engine/integration/commands_test.go @@ -774,25 +774,6 @@ func TestCmdLogs(t *testing.T) { } } -// Expected behaviour: using / as a bind mount source should throw an error -func TestRunErrorBindMountRootSource(t *testing.T) { - - cli := docker.NewDockerCli(nil, nil, ioutil.Discard, testDaemonProto, testDaemonAddr) - defer cleanup(globalEngine, t) - - c := make(chan struct{}) - go func() { - defer close(c) - if err := cli.CmdRun("-v", "/:/tmp", unitTestImageID, "echo 'should fail'"); err == nil { - t.Fatal("should have failed to run when using / as a source for the bind mount") - } - }() - - setTimeout(t, "CmdRun timed out", 5*time.Second, func() { - <-c - }) -} - // Expected behaviour: error out when attempting to bind mount non-existing source paths func TestRunErrorBindNonExistingSource(t *testing.T) { @@ -802,6 +783,7 @@ func TestRunErrorBindNonExistingSource(t *testing.T) { c := make(chan struct{}) go func() { defer close(c) + // This check is made at runtime, can't be "unit tested" if err := cli.CmdRun("-v", "/i/dont/exist:/tmp", unitTestImageID, "echo 'should fail'"); err == nil { t.Fatal("should have failed to run when using /i/dont/exist as a source for the bind mount") } From 2135aa6acaad8f97bf78f40c6796a6f92b615a31 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Fri, 29 Nov 2013 10:17:25 -0800 Subject: [PATCH 12/14] Fix behavior of tty tests Upstream-commit: 86c00be180f1e6831ca426576a55f5106f156448 Component: engine --- .../engine/integration/commands_test.go | 47 +++++++++---------- 1 file changed, 23 insertions(+), 24 deletions(-) diff --git a/components/engine/integration/commands_test.go b/components/engine/integration/commands_test.go index ba9399218a..50cd230ce9 100644 --- a/components/engine/integration/commands_test.go +++ b/components/engine/integration/commands_test.go @@ -337,7 +337,8 @@ func TestRunDisconnect(t *testing.T) { }) } -// Expected behaviour: the process dies when the client disconnects +// Expected behaviour: the process stay alive when the client disconnects +// but the client detaches. func TestRunDisconnectTty(t *testing.T) { stdin, stdinPipe := io.Pipe() @@ -348,29 +349,20 @@ func TestRunDisconnectTty(t *testing.T) { c1 := make(chan struct{}) go func() { + defer close(c1) // We're simulating a disconnect so the return value doesn't matter. What matters is the // fact that CmdRun returns. if err := cli.CmdRun("-i", "-t", unitTestImageID, "/bin/cat"); err != nil { utils.Debugf("Error CmdRun: %s", err) } - - close(c1) }() - setTimeout(t, "Waiting for the container to be started timed out", 10*time.Second, func() { - for { - // Client disconnect after run -i should keep stdin out in TTY mode - l := globalRuntime.List() - if len(l) == 1 && l[0].State.IsRunning() { - break - } - time.Sleep(10 * time.Millisecond) - } - }) + container := waitContainerStart(t, 10*time.Second) + + state := setRaw(t, container) + defer unsetRaw(t, container, state) // Client disconnect after run -i should keep stdin out in TTY mode - container := globalRuntime.List()[0] - setTimeout(t, "Read/Write assertion timed out", 2*time.Second, func() { if err := assertPipe("hello\n", "hello", stdout, stdinPipe, 150); err != nil { t.Fatal(err) @@ -382,8 +374,12 @@ func TestRunDisconnectTty(t *testing.T) { t.Fatal(err) } + // wait for CmdRun to return + setTimeout(t, "Waiting for CmdRun timed out", 5*time.Second, func() { + <-c1 + }) + // In tty mode, we expect the process to stay alive even after client's stdin closes. - // Do not wait for run to finish // Give some time to monitor to do his thing container.WaitTimeout(500 * time.Millisecond) @@ -473,27 +469,28 @@ func TestRunDetach(t *testing.T) { cli.CmdRun("-i", "-t", unitTestImageID, "cat") }() + container := waitContainerStart(t, 10*time.Second) + + state := setRaw(t, container) + defer unsetRaw(t, container, state) + setTimeout(t, "First read/write assertion timed out", 2*time.Second, func() { if err := assertPipe("hello\n", "hello", stdout, stdinPipe, 150); err != nil { t.Fatal(err) } }) - container := globalRuntime.List()[0] - setTimeout(t, "Escape sequence timeout", 5*time.Second, func() { - stdinPipe.Write([]byte{16, 17}) - if err := stdinPipe.Close(); err != nil { - t.Fatal(err) - } + stdinPipe.Write([]byte{16}) + time.Sleep(100 * time.Millisecond) + stdinPipe.Write([]byte{17}) }) - closeWrap(stdin, stdinPipe, stdout, stdoutPipe) - // wait for CmdRun to return setTimeout(t, "Waiting for CmdRun timed out", 15*time.Second, func() { <-ch }) + closeWrap(stdin, stdinPipe, stdout, stdoutPipe) time.Sleep(500 * time.Millisecond) if !container.State.IsRunning() { @@ -594,6 +591,7 @@ func TestAttachDetachTruncatedID(t *testing.T) { cli := docker.NewDockerCli(stdin, stdoutPipe, ioutil.Discard, testDaemonProto, testDaemonAddr) defer cleanup(globalEngine, t) + // Discard the CmdRun output go stdout.Read(make([]byte, 1024)) setTimeout(t, "Starting container timed out", 2*time.Second, func() { if err := cli.CmdRun("-i", "-t", "-d", unitTestImageID, "cat"); err != nil { @@ -759,6 +757,7 @@ func TestRunAutoRemove(t *testing.T) { } func TestCmdLogs(t *testing.T) { + t.Skip("Test not impemented") cli := docker.NewDockerCli(nil, ioutil.Discard, ioutil.Discard, testDaemonProto, testDaemonAddr) defer cleanup(globalEngine, t) From ccfe71909eaf7c270ba6ed46e015aad98fc429c3 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Fri, 29 Nov 2013 10:32:52 -0800 Subject: [PATCH 13/14] Reduce the timeout for restart/stop Upstream-commit: 34353e782e1cdbd6aae078b3e660875e703d35ff Component: engine --- components/engine/integration/server_test.go | 4 ++-- components/engine/term/term.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/components/engine/integration/server_test.go b/components/engine/integration/server_test.go index 24e109ab76..494e23fef3 100644 --- a/components/engine/integration/server_test.go +++ b/components/engine/integration/server_test.go @@ -183,11 +183,11 @@ func TestCreateStartRestartStopStartKillRm(t *testing.T) { t.Fatal(err) } - if err := srv.ContainerRestart(id, 150); err != nil { + if err := srv.ContainerRestart(id, 15); err != nil { t.Fatal(err) } - if err := srv.ContainerStop(id, 150); err != nil { + if err := srv.ContainerStop(id, 15); err != nil { t.Fatal(err) } diff --git a/components/engine/term/term.go b/components/engine/term/term.go index 50425a8602..f7d9930ad0 100644 --- a/components/engine/term/term.go +++ b/components/engine/term/term.go @@ -9,7 +9,7 @@ import ( ) var ( - ErrInvalidState = errors.New("Invlide terminal state") + ErrInvalidState = errors.New("Invlid terminal state") ) type State struct { From 1f411096dda7f65bfe701a012e46f64c0b5f5b29 Mon Sep 17 00:00:00 2001 From: "Guillaume J. Charmes" Date: Fri, 29 Nov 2013 13:43:37 -0800 Subject: [PATCH 14/14] Add unit test to check bind / server side Upstream-commit: ab35aef6b5b15c7c2d7aff4315025563b93ee379 Component: engine --- components/engine/integration/api_test.go | 37 +++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/components/engine/integration/api_test.go b/components/engine/integration/api_test.go index bd6280af8b..3d5a2e42e0 100644 --- a/components/engine/integration/api_test.go +++ b/components/engine/integration/api_test.go @@ -743,6 +743,43 @@ func TestPostContainersStart(t *testing.T) { containerKill(eng, containerID, t) } +// Expected behaviour: using / as a bind mount source should throw an error +func TestRunErrorBindMountRootSource(t *testing.T) { + eng := NewTestEngine(t) + defer mkRuntimeFromEngine(eng, t).Nuke() + srv := mkServerFromEngine(eng, t) + + containerID := createTestContainer( + eng, + &docker.Config{ + Image: unitTestImageID, + Cmd: []string{"/bin/cat"}, + OpenStdin: true, + }, + t, + ) + + hostConfigJSON, err := json.Marshal(&docker.HostConfig{ + Binds: []string{"/:/tmp"}, + }) + + req, err := http.NewRequest("POST", "/containers/"+containerID+"/start", bytes.NewReader(hostConfigJSON)) + if err != nil { + t.Fatal(err) + } + + req.Header.Set("Content-Type", "application/json") + + r := httptest.NewRecorder() + if err := docker.ServeRequest(srv, docker.APIVERSION, r, req); err != nil { + t.Fatal(err) + } + if r.Code != http.StatusInternalServerError { + containerKill(eng, containerID, t) + t.Fatal("should have failed to run when using / as a source for the bind mount") + } +} + func TestPostContainersStop(t *testing.T) { eng := NewTestEngine(t) defer mkRuntimeFromEngine(eng, t).Nuke()