Merge pull request #2954 from creack/fix_tests

Fix tests

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)

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
+}

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)
 		}
 	})

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)
 
-	// Client disconnect after run -i should keep stdin out in TTY mode
-	container := globalRuntime.List()[0]
+	state := setRaw(t, container)
+	defer unsetRaw(t, container, state)
 
+	// Client disconnect after run -i should keep stdin out in TTY mode
 	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")
 		}

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)
 		}
 	})

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) {

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