diff --git a/commands.go b/commands.go index 5dd76fe187..846a66c7a4 100644 --- a/commands.go +++ b/commands.go @@ -2392,8 +2392,27 @@ 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() + } + }() + // When TTY is ON, use regular copy if setRawTerminal { _, err = io.Copy(stdout, br) @@ -2405,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) 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 +} diff --git a/integration/api_test.go b/integration/api_test.go index a66cbe561f..3d5a2e42e0 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) } }) @@ -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() @@ -877,7 +914,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 +993,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..50cd230ce9 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" @@ -31,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) @@ -213,7 +255,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 +310,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) } }) @@ -295,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() @@ -306,31 +349,22 @@ 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, 15); err != nil { + if err := assertPipe("hello\n", "hello", stdout, stdinPipe, 150); err != nil { t.Fatal(err) } }) @@ -340,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) @@ -431,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, 15); err != nil { + 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() { @@ -479,7 +518,7 @@ func TestAttachDetach(t *testing.T) { } }() - var container *docker.Container + container := waitContainerStart(t, 10*time.Second) setTimeout(t, "Reading container's id timed out", 10*time.Second, func() { buf := make([]byte, 1024) @@ -488,8 +527,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]) } @@ -498,6 +535,9 @@ func TestAttachDetach(t *testing.T) { <-ch }) + state := setRaw(t, container) + defer unsetRaw(t, container, state) + stdin, stdinPipe = io.Pipe() stdout, stdoutPipe = io.Pipe() cli = docker.NewDockerCli(stdin, stdoutPipe, ioutil.Discard, testDaemonProto, testDaemonAddr) @@ -513,7 +553,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) } @@ -521,18 +561,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") @@ -551,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 { @@ -558,7 +599,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() @@ -575,7 +619,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) } @@ -583,17 +627,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() { @@ -648,7 +691,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) } }) @@ -714,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) @@ -729,25 +773,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) { @@ -757,6 +782,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") } 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/term/term.go b/term/term.go index 8c53a20ca6..ea94b44ade 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("Invalid 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) { 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