mirror of
https://github.com/moby/moby.git
synced 2022-11-09 12:21:53 -05:00
Using waitExitOrRemoved for docker start
Currently start command will invoke getExitCode - which is based on Inspect API - to get returned exit code after container exits. There's two race conditions here: if container is started with Restart Policy, there's chance that the container is restarted quickly before it calls getExitCode, under such circumstance, the exit code is wrong. if container is started with --rm, it's possible that container is removed before getExitCode, in this situation, you can't get correct exit code either. Replace getExitCode with waitExitOrRemoved can solve this problem. Signed-off-by: Zhang Wei <zhangwei555@huawei.com>
This commit is contained in:
parent
754ac49268
commit
b8464c1c9b
2 changed files with 29 additions and 8 deletions
|
@ -90,8 +90,8 @@ func runStart(dockerCli *client.DockerCli, opts *startOptions) error {
|
|||
resp, errAttach := dockerCli.Client().ContainerAttach(ctx, c.ID, options)
|
||||
if errAttach != nil && errAttach != httputil.ErrPersistEOF {
|
||||
// ContainerAttach return an ErrPersistEOF (connection closed)
|
||||
// means server met an error and put it in Hijacked connection
|
||||
// keep the error and read detailed error message from hijacked connection
|
||||
// means server met an error and already put it in Hijacked connection,
|
||||
// we would keep the error and read the detailed error message from hijacked connection
|
||||
return errAttach
|
||||
}
|
||||
defer resp.Close()
|
||||
|
@ -103,14 +103,22 @@ func runStart(dockerCli *client.DockerCli, opts *startOptions) error {
|
|||
return errHijack
|
||||
})
|
||||
|
||||
// 3. Start the container.
|
||||
// 3. We should open a channel for receiving status code of the container
|
||||
// no matter it's detached, removed on daemon side(--rm) or exit normally.
|
||||
statusChan, statusErr := waitExitOrRemoved(dockerCli, context.Background(), c.ID, c.HostConfig.AutoRemove)
|
||||
|
||||
// 4. Start the container.
|
||||
if err := dockerCli.Client().ContainerStart(ctx, c.ID, types.ContainerStartOptions{}); err != nil {
|
||||
cancelFun()
|
||||
<-cErr
|
||||
if c.HostConfig.AutoRemove && statusErr == nil {
|
||||
// wait container to be removed
|
||||
<-statusChan
|
||||
}
|
||||
return err
|
||||
}
|
||||
|
||||
// 4. Wait for attachment to break.
|
||||
// 5. Wait for attachment to break.
|
||||
if c.Config.Tty && dockerCli.IsTerminalOut() {
|
||||
if err := dockerCli.MonitorTtySize(ctx, c.ID, false); err != nil {
|
||||
fmt.Fprintf(dockerCli.Err(), "Error monitoring TTY size: %s\n", err)
|
||||
|
@ -119,11 +127,12 @@ func runStart(dockerCli *client.DockerCli, opts *startOptions) error {
|
|||
if attchErr := <-cErr; attchErr != nil {
|
||||
return attchErr
|
||||
}
|
||||
_, status, err := getExitCode(dockerCli, ctx, c.ID)
|
||||
if err != nil {
|
||||
return err
|
||||
|
||||
if statusErr != nil {
|
||||
return fmt.Errorf("can't get container's exit code: %v", statusErr)
|
||||
}
|
||||
if status != 0 {
|
||||
|
||||
if status := <-statusChan; status != 0 {
|
||||
return cli.StatusError{StatusCode: status}
|
||||
}
|
||||
} else {
|
||||
|
|
|
@ -185,3 +185,15 @@ func (s *DockerSuite) TestStartAttachWithRename(c *check.C) {
|
|||
_, stderr, _, _ := runCommandWithStdoutStderr(exec.Command(dockerBinary, "start", "-a", "before"))
|
||||
c.Assert(stderr, checker.Not(checker.Contains), "No such container")
|
||||
}
|
||||
|
||||
func (s *DockerSuite) TestStartReturnCorrectExitCode(c *check.C) {
|
||||
dockerCmd(c, "create", "--restart=on-failure:2", "--name", "withRestart", "busybox", "sh", "-c", "exit 11")
|
||||
dockerCmd(c, "create", "--rm", "--name", "withRm", "busybox", "sh", "-c", "exit 12")
|
||||
|
||||
_, exitCode, err := dockerCmdWithError("start", "-a", "withRestart")
|
||||
c.Assert(err, checker.NotNil)
|
||||
c.Assert(exitCode, checker.Equals, 11)
|
||||
_, exitCode, err = dockerCmdWithError("start", "-a", "withRm")
|
||||
c.Assert(err, checker.NotNil)
|
||||
c.Assert(exitCode, checker.Equals, 12)
|
||||
}
|
||||
|
|
Loading…
Reference in a new issue