From ad43d88af5bda8dc5b3d06f64de380bb985191ba 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 --- integration/api_test.go | 6 +++--- integration/commands_test.go | 14 +++++++------- integration/container_test.go | 2 +- integration/server_test.go | 4 ++-- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/integration/api_test.go b/integration/api_test.go index a66cbe561f..bd6280af8b 100644 --- a/integration/api_test.go +++ b/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/integration/commands_test.go b/integration/commands_test.go index 37bedf7f0c..7daebf3cd2 100644 --- a/integration/commands_test.go +++ b/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/integration/container_test.go b/integration/container_test.go index 93a00a7286..05eb48728c 100644 --- a/integration/container_test.go +++ b/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/integration/server_test.go b/integration/server_test.go index 494e23fef3..24e109ab76 100644 --- a/integration/server_test.go +++ b/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 77c94175bdc387e7f43876f7097b970f67116054 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 --- utils/utils.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/utils.go b/utils/utils.go index cfdc73bb2e..f62aa12ff5 100644 --- a/utils/utils.go +++ b/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 e535f544c7bc9c32b23e5505110a5513ff36be5a 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) --- integration/commands_test.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/integration/commands_test.go b/integration/commands_test.go index 7daebf3cd2..8bc1c99ec7 100644 --- a/integration/commands_test.go +++ b/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 fbebe20bc69648c046e3818ca744ae246092a782 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. --- container.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/container.go b/container.go index 49cb33b536..3e5f8a8d74 100644 --- a/container.go +++ b/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 67e9e0e11bb932ef9113ac91b2c1b0af6ee4db6d 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) --- integration/commands_test.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/integration/commands_test.go b/integration/commands_test.go index 8bc1c99ec7..f2b14482b2 100644 --- a/integration/commands_test.go +++ b/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 63d6cbe3e4d6f29c2491b0f1f505ef79b7191d8e 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) --- commands.go | 6 ++++++ integration/commands_test.go | 10 +++++----- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/commands.go b/commands.go index db752447f0..24218d284c 100644 --- a/commands.go +++ b/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/integration/commands_test.go b/integration/commands_test.go index f2b14482b2..75d09facb4 100644 --- a/integration/commands_test.go +++ b/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 aa68656cd3aefe2a64dc259e8d19c72010e4f85b 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 --- term/term.go | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/term/term.go b/term/term.go index 8c53a20ca6..50425a8602 100644 --- a/term/term.go +++ b/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 c13821ad0bfa596a8bbc06a494500ee39a35e429 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 --- commands.go | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/commands.go b/commands.go index 24218d284c..dbf295cf13 100644 --- a/commands.go +++ b/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 697be6aaa009cd2bea5f07ae0b0780703e6565e1 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 --- integration/commands_test.go | 66 ++++++++++++++++++++++++------------ 1 file changed, 44 insertions(+), 22 deletions(-) diff --git a/integration/commands_test.go b/integration/commands_test.go index 75d09facb4..fa8f25836c 100644 --- a/integration/commands_test.go +++ b/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 2e6a958612d65a0665a9396372fe82706987d085 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) --- integration/commands_test.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/integration/commands_test.go b/integration/commands_test.go index fa8f25836c..90333a052c 100644 --- a/integration/commands_test.go +++ b/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 2ec1146679598837cd8bab62dc672bcda2a9610c 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 --- integration/commands_test.go | 20 +------------------- 1 file changed, 1 insertion(+), 19 deletions(-) diff --git a/integration/commands_test.go b/integration/commands_test.go index 90333a052c..ba9399218a 100644 --- a/integration/commands_test.go +++ b/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 86c00be180f1e6831ca426576a55f5106f156448 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 --- integration/commands_test.go | 47 ++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 24 deletions(-) diff --git a/integration/commands_test.go b/integration/commands_test.go index ba9399218a..50cd230ce9 100644 --- a/integration/commands_test.go +++ b/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 34353e782e1cdbd6aae078b3e660875e703d35ff 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 --- integration/server_test.go | 4 ++-- term/term.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/integration/server_test.go b/integration/server_test.go index 24e109ab76..494e23fef3 100644 --- a/integration/server_test.go +++ b/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/term/term.go b/term/term.go index 50425a8602..f7d9930ad0 100644 --- a/term/term.go +++ b/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 ab35aef6b5b15c7c2d7aff4315025563b93ee379 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 --- integration/api_test.go | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/integration/api_test.go b/integration/api_test.go index bd6280af8b..3d5a2e42e0 100644 --- a/integration/api_test.go +++ b/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()