diff --git a/api_test.go b/api_test.go index 35667c74f1..4af9c78058 100644 --- a/api_test.go +++ b/api_test.go @@ -371,8 +371,8 @@ func TestGetContainersJSON(t *testing.T) { if err := json.Unmarshal(r.Body.Bytes(), &containers); err != nil { t.Fatal(err) } - if len(containers) != 1 { - t.Fatalf("Expected %d container, %d found (started with: %d)", 1, len(containers), beginLen) + if len(containers) != beginLen+1 { + t.Fatalf("Expected %d container, %d found (started with: %d)", beginLen+1, len(containers), beginLen) } if containers[0].ID != container.ID { t.Fatalf("Container ID mismatch. Expected: %s, received: %s\n", container.ID, containers[0].ID) diff --git a/commands.go b/commands.go index befe8a6bcf..aab705cc43 100644 --- a/commands.go +++ b/commands.go @@ -1597,12 +1597,10 @@ func (cli *DockerCli) CmdRun(args ...string) error { cli.forwardAllSignals(runResult.ID) } - //start the container - if _, _, err = cli.call("POST", "/containers/"+runResult.ID+"/start", hostConfig); err != nil { - return err - } - - var wait chan struct{} + var ( + wait chan struct{} + errCh chan error + ) if !config.AttachStdout && !config.AttachStderr { // Make this asynchrone in order to let the client write to stdin before having to read the ID @@ -1621,7 +1619,6 @@ func (cli *DockerCli) CmdRun(args ...string) error { } v := url.Values{} - v.Set("logs", "1") v.Set("stream", "1") var out, stderr io.Writer @@ -1641,18 +1638,18 @@ func (cli *DockerCli) CmdRun(args ...string) error { } } - signals := make(chan os.Signal, 1) - signal.Notify(signals, syscall.SIGINT, syscall.SIGTERM) - go func() { - for sig := range signals { - fmt.Printf("\nReceived signal: %s; cleaning up\n", sig) - if err := cli.CmdStop("-t", "4", runResult.ID); err != nil { - fmt.Printf("failed to stop container: %v", err) - } - } - }() + errCh = utils.Go(func() error { + return cli.hijack("POST", "/containers/"+runResult.ID+"/attach?"+v.Encode(), config.Tty, cli.in, out, stderr) + }) + } - if err := cli.hijack("POST", "/containers/"+runResult.ID+"/attach?"+v.Encode(), config.Tty, cli.in, out, stderr); err != nil { + //start the container + if _, _, err = cli.call("POST", "/containers/"+runResult.ID+"/start", hostConfig); err != nil { + return err + } + + if errCh != nil { + if err := <-errCh; err != nil { utils.Debugf("Error hijack: %s", err) return err } @@ -1667,8 +1664,7 @@ func (cli *DockerCli) CmdRun(args ...string) error { return err } if autoRemove { - _, _, err = cli.call("DELETE", "/containers/"+runResult.ID, nil) - if err != nil { + if _, _, err = cli.call("DELETE", "/containers/"+runResult.ID, nil); err != nil { return err } } @@ -1753,6 +1749,7 @@ func (cli *DockerCli) call(method, path string, data interface{}) ([]byte, int, return nil, -1, err } defer resp.Body.Close() + body, err := ioutil.ReadAll(resp.Body) if err != nil { return nil, -1, err diff --git a/commands_test.go b/commands_test.go index c5fa83d3e4..2034269eaf 100644 --- a/commands_test.go +++ b/commands_test.go @@ -84,7 +84,7 @@ func TestRunHostname(t *testing.T) { } }) - setTimeout(t, "CmdRun timed out", 5*time.Second, func() { + setTimeout(t, "CmdRun timed out", 10*time.Second, func() { <-c }) @@ -115,7 +115,7 @@ func TestRunWorkdir(t *testing.T) { } }) - setTimeout(t, "CmdRun timed out", 5*time.Second, func() { + setTimeout(t, "CmdRun timed out", 10*time.Second, func() { <-c }) @@ -399,6 +399,8 @@ func TestRunDetach(t *testing.T) { } }) + closeWrap(stdin, stdinPipe, stdout, stdoutPipe) + // wait for CmdRun to return setTimeout(t, "Waiting for CmdRun timed out", 15*time.Second, func() { <-ch @@ -422,30 +424,52 @@ func TestAttachDetach(t *testing.T) { cli := NewDockerCli(stdin, stdoutPipe, ioutil.Discard, testDaemonProto, testDaemonAddr) defer cleanup(globalRuntime) - go stdout.Read(make([]byte, 1024)) - setTimeout(t, "Starting container timed out", 2*time.Second, func() { + ch := make(chan struct{}) + go func() { + defer close(ch) if err := cli.CmdRun("-i", "-t", "-d", unitTestImageID, "cat"); err != nil { t.Fatal(err) } - }) + }() - container := globalRuntime.List()[0] + var container *Container + + setTimeout(t, "Reading container's id timed out", 10*time.Second, func() { + buf := make([]byte, 1024) + n, err := stdout.Read(buf) + if err != nil { + t.Fatal(err) + } + + container = globalRuntime.List()[0] + + if strings.Trim(string(buf[:n]), " \r\n") != container.ShortID() { + t.Fatalf("Wrong ID received. Expect %s, received %s", container.ShortID(), buf[:n]) + } + }) + setTimeout(t, "Starting container timed out", 10*time.Second, func() { + <-ch + }) stdin, stdinPipe = io.Pipe() stdout, stdoutPipe = io.Pipe() cli = NewDockerCli(stdin, stdoutPipe, ioutil.Discard, testDaemonProto, testDaemonAddr) - ch := make(chan struct{}) + ch = make(chan struct{}) go func() { defer close(ch) if err := cli.CmdAttach(container.ShortID()); err != nil { - t.Fatal(err) + if err != io.ErrClosedPipe { + t.Fatal(err) + } } }() 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) + if err != io.ErrClosedPipe { + t.Fatal(err) + } } }) @@ -455,6 +479,7 @@ func TestAttachDetach(t *testing.T) { t.Fatal(err) } }) + closeWrap(stdin, stdinPipe, stdout, stdoutPipe) // wait for CmdRun to return setTimeout(t, "Waiting for CmdAttach timed out", 15*time.Second, func() { @@ -568,7 +593,7 @@ func TestRunAutoRemove(t *testing.T) { } }) - setTimeout(t, "CmdRun timed out", 5*time.Second, func() { + setTimeout(t, "CmdRun timed out", 10*time.Second, func() { <-c }) diff --git a/container.go b/container.go index f17cabbce4..c2188be625 100644 --- a/container.go +++ b/container.go @@ -99,6 +99,8 @@ type BindMap struct { } var ( + ErrContainerStart = errors.New("The container failed to start. Unkown error") + ErrContainerStartTimeout = errors.New("The container failed to start due to timed out.") ErrInvalidWorikingDirectory = errors.New("The working directory is invalid. It needs to be an absolute path.") ErrConflictTtySigProxy = errors.New("TTY mode (-t) already imply signal proxying (-sig-proxy)") ErrConflictAttachDetach = errors.New("Conflicting options: -a and -d") @@ -838,12 +840,43 @@ func (container *Container) Start(hostConfig *HostConfig) (err error) { container.ToDisk() container.SaveHostConfig(hostConfig) go container.monitor(hostConfig) - return nil + + defer utils.Debugf("Container running: %v", container.State.Running) + // We wait for the container to be fully running. + // Timeout after 5 seconds. In case of broken pipe, just retry. + // Note: The container can run and finish correctly before + // the end of this loop + for now := time.Now(); time.Since(now) < 5*time.Second; { + // If the container dies while waiting for it, just reutrn + if !container.State.Running { + return nil + } + output, err := exec.Command("lxc-info", "-n", container.ID).CombinedOutput() + if err != nil { + utils.Debugf("Error with lxc-info: %s (%s)", err, output) + + output, err = exec.Command("lxc-info", "-n", container.ID).CombinedOutput() + if err != nil { + utils.Debugf("Second Error with lxc-info: %s (%s)", err, output) + return err + } + + } + if strings.Contains(string(output), "RUNNING") { + return nil + } + utils.Debugf("Waiting for the container to start (running: %v): %s\n", container.State.Running, output) + time.Sleep(50 * time.Millisecond) + } + + if container.State.Running { + return ErrContainerStartTimeout + } + return ErrContainerStart } func (container *Container) Run() error { - hostConfig := &HostConfig{} - if err := container.Start(hostConfig); err != nil { + if err := container.Start(&HostConfig{}); err != nil { return err } container.Wait() diff --git a/runtime_test.go b/runtime_test.go index 75081fff1e..74feb02976 100644 --- a/runtime_test.go +++ b/runtime_test.go @@ -212,22 +212,16 @@ func TestRuntimeCreate(t *testing.T) { } // Make sure crete with bad parameters returns an error - _, err = runtime.Create( - &Config{ - Image: GetTestImage(runtime).ID, - }, - ) - if err == nil { + if _, err = runtime.Create(&Config{Image: GetTestImage(runtime).ID}); err == nil { t.Fatal("Builder.Create should throw an error when Cmd is missing") } - _, err = runtime.Create( + if _, err := runtime.Create( &Config{ Image: GetTestImage(runtime).ID, Cmd: []string{}, }, - ) - if err == nil { + ); err == nil { t.Fatal("Builder.Create should throw an error when Cmd is empty") } @@ -258,11 +252,11 @@ func TestRuntimeCreate(t *testing.T) { func TestDestroy(t *testing.T) { runtime := mkRuntime(t) defer nuke(runtime) + container, err := runtime.Create(&Config{ Image: GetTestImage(runtime).ID, Cmd: []string{"ls", "-al"}, - }, - ) + }) if err != nil { t.Fatal(err) } @@ -327,11 +321,14 @@ func TestGet(t *testing.T) { } func startEchoServerContainer(t *testing.T, proto string) (*Runtime, *Container, string) { - var err error - runtime := mkRuntime(t) - port := 5554 - var container *Container - var strPort string + var ( + err error + container *Container + strPort string + runtime = mkRuntime(t) + port = 5554 + ) + for { port += 1 strPort = strconv.Itoa(port) @@ -359,8 +356,7 @@ func startEchoServerContainer(t *testing.T, proto string) (*Runtime, *Container, t.Logf("Port %v already in use", strPort) } - hostConfig := &HostConfig{} - if err := container.Start(hostConfig); err != nil { + if err := container.Start(&HostConfig{}); err != nil { nuke(runtime) t.Fatal(err) } diff --git a/server_test.go b/server_test.go index 0dae862071..c7bfa415db 100644 --- a/server_test.go +++ b/server_test.go @@ -192,11 +192,11 @@ func TestCreateStartRestartStopStartKillRm(t *testing.T) { t.Fatal(err) } - if err := srv.ContainerRestart(id, 1); err != nil { + if err := srv.ContainerRestart(id, 15); err != nil { t.Fatal(err) } - if err := srv.ContainerStop(id, 1); err != nil { + if err := srv.ContainerStop(id, 15); err != nil { t.Fatal(err) }