Wait container's removal via Events API

If AutoRemove is set, wait until client get `destroy` events, or get
`detach` events that implies container is detached but not stopped.

Signed-off-by: Zhang Wei <zhangwei555@huawei.com>

api/client/container/run.go

@@ -8,6 +8,7 @@ import (
 	"runtime"
 	"strings"
 	"syscall"
+	"time"
 
 	"golang.org/x/net/context"
 
@@ -143,6 +144,7 @@ func runRun(dockerCli *client.DockerCli, flags *pflag.FlagSet, opts *runOptions,
 		hostConfig.ConsoleSize[0], hostConfig.ConsoleSize[1] = dockerCli.GetTtySize()
 	}
 
+	startTime := time.Now()
 	ctx, cancelFun := context.WithCancel(context.Background())
 
 	createResponse, err := createContainer(ctx, dockerCli, config, hostConfig, networkingConfig, hostConfig.ContainerIDFile, opts.name)
@@ -230,6 +232,11 @@ func runRun(dockerCli *client.DockerCli, flags *pflag.FlagSet, opts *runOptions,
 		}
 
 		reportError(stderr, cmdPath, err.Error(), false)
+		if hostConfig.AutoRemove {
+			if _, errWait := waitExitOrRemoved(dockerCli, context.Background(), createResponse.ID, hostConfig.AutoRemove, startTime); errWait != nil {
+				logrus.Debugf("Error waiting container's removal: %v", errWait)
+			}
+		}
 		return runStartContainerErr(err)
 	}
 
@@ -256,15 +263,9 @@ func runRun(dockerCli *client.DockerCli, flags *pflag.FlagSet, opts *runOptions,
 	var status int
 
 	// Attached mode
-	if !config.Tty {
-		// In non-TTY mode, we can't detach, so we must wait for container exit
-		client.ContainerWait(context.Background(), createResponse.ID)
-	}
-
-	// In TTY mode, there is a race: if the process dies too slowly, the state could
-	// be updated after the getExitCode call and result in the wrong exit code being reported
-	if _, status, err = dockerCli.GetExitCode(ctx, createResponse.ID); err != nil {
-		return fmt.Errorf("tty: status: %d; error: %v;", status, err)
+	status, err = waitExitOrRemoved(dockerCli, ctx, createResponse.ID, hostConfig.AutoRemove, startTime)
+	if err != nil {
+		return fmt.Errorf("Error waiting container to exit: %v", err)
 	}
 
 	if status != 0 {

api/client/container/utils.go

@@ -1,12 +1,100 @@
 package container
 
 import (
+	"fmt"
+	"strconv"
+	"time"
+
 	"golang.org/x/net/context"
 
+	"github.com/Sirupsen/logrus"
 	"github.com/docker/docker/api/client"
+	"github.com/docker/docker/api/client/system"
 	clientapi "github.com/docker/engine-api/client"
+	"github.com/docker/engine-api/types"
+	"github.com/docker/engine-api/types/events"
+	"github.com/docker/engine-api/types/filters"
 )
 
+func waitExitOrRemoved(dockerCli *client.DockerCli, ctx context.Context, containerID string, waitRemove bool, since time.Time) (int, error) {
+	if len(containerID) == 0 {
+		// containerID can never be empty
+		panic("Internal Error: waitExitOrRemoved needs a containerID as parameter")
+	}
+
+	var exitCode int
+	exitChan := make(chan struct{})
+	detachChan := make(chan struct{})
+	destroyChan := make(chan struct{})
+
+	// Start watch events
+	eh := system.InitEventHandler()
+	eh.Handle("die", func(e events.Message) {
+		if len(e.Actor.Attributes) > 0 {
+			for k, v := range e.Actor.Attributes {
+				if k == "exitCode" {
+					var err error
+					if exitCode, err = strconv.Atoi(v); err != nil {
+						logrus.Errorf("Can't convert %q to int: %v", v, err)
+					}
+					close(exitChan)
+					break
+				}
+			}
+		}
+	})
+
+	eh.Handle("detach", func(e events.Message) {
+		exitCode = 0
+		close(detachChan)
+	})
+	eh.Handle("destroy", func(e events.Message) {
+		close(destroyChan)
+	})
+
+	eventChan := make(chan events.Message)
+	go eh.Watch(eventChan)
+	defer close(eventChan)
+
+	// Get events via Events API
+	f := filters.NewArgs()
+	f.Add("type", "container")
+	f.Add("container", containerID)
+	options := types.EventsOptions{
+		Since:   fmt.Sprintf("%d", since.Unix()),
+		Filters: f,
+	}
+	resBody, err := dockerCli.Client().Events(ctx, options)
+	if err != nil {
+		return -1, fmt.Errorf("can't get events from daemon: %v", err)
+	}
+	defer resBody.Close()
+
+	go system.DecodeEvents(resBody, func(event events.Message, err error) error {
+		if err != nil {
+			return nil
+		}
+		eventChan <- event
+		return nil
+	})
+
+	if waitRemove {
+		select {
+		case <-destroyChan:
+			return exitCode, nil
+		case <-detachChan:
+			return 0, nil
+		}
+	} else {
+		select {
+		case <-exitChan:
+			return exitCode, nil
+		case <-detachChan:
+			return 0, nil
+		}
+	}
+}
+
 // getExitCode performs an inspect on the container. It returns
 // the running state and the exit code.
 func getExitCode(dockerCli *client.DockerCli, ctx context.Context, containerID string) (bool, int, error) {

daemon/container.go

@@ -210,7 +210,7 @@ func (daemon *Daemon) verifyContainerSettings(hostConfig *containertypes.HostCon
 		if config.WorkingDir != "" {
 			config.WorkingDir = filepath.FromSlash(config.WorkingDir) // Ensure in platform semantics
 			if !system.IsAbs(config.WorkingDir) {
-				return nil, fmt.Errorf("The working directory '%s' is invalid. It needs to be an absolute path", config.WorkingDir)
+				return nil, fmt.Errorf("the working directory '%s' is invalid, it needs to be an absolute path", config.WorkingDir)
 			}
 		}
 
@@ -239,18 +239,18 @@ func (daemon *Daemon) verifyContainerSettings(hostConfig *containertypes.HostCon
 	}
 
 	if hostConfig.AutoRemove && !hostConfig.RestartPolicy.IsNone() {
-		return nil, fmt.Errorf("Can't create 'AutoRemove' container with restart policy")
+		return nil, fmt.Errorf("can't create 'AutoRemove' container with restart policy")
 	}
 
 	for port := range hostConfig.PortBindings {
 		_, portStr := nat.SplitProtoPort(string(port))
 		if _, err := nat.ParsePort(portStr); err != nil {
-			return nil, fmt.Errorf("Invalid port specification: %q", portStr)
+			return nil, fmt.Errorf("invalid port specification: %q", portStr)
 		}
 		for _, pb := range hostConfig.PortBindings[port] {
 			_, err := nat.NewPort(nat.SplitProtoPort(pb.HostPort))
 			if err != nil {
-				return nil, fmt.Errorf("Invalid port specification: %q", pb.HostPort)
+				return nil, fmt.Errorf("invalid port specification: %q", pb.HostPort)
 			}
 		}
 	}

daemon/daemon.go

@@ -292,10 +292,10 @@ func (daemon *Daemon) restore() error {
 	for id := range removeContainers {
 		removeGroup.Add(1)
 		go func(cid string) {
-			defer removeGroup.Done()
 			if err := daemon.ContainerRm(cid, &types.ContainerRmConfig{ForceRemove: true, RemoveVolume: true}); err != nil {
 				logrus.Errorf("Failed to remove container %s: %s", cid, err)
 			}
+			removeGroup.Done()
 		}(id)
 	}
 	removeGroup.Wait()

docs/reference/api/docker_remote_api.md

@@ -116,6 +116,8 @@ This section lists each version from latest to oldest.  Each listing includes a
 
 [Docker Remote API v1.25](docker_remote_api_v1.25.md) documentation
 
+* `POST /containers/create` now takes `AutoRemove` in HostConfig, auto-removal will be done on daemon side.
+
 ### v1.24 API changes
 
 [Docker Remote API v1.24](docker_remote_api_v1.24.md) documentation

docs/reference/api/docker_remote_api_v1.25.md

@@ -325,6 +325,7 @@ Create a container
              "CapDrop": ["MKNOD"],
              "GroupAdd": ["newgroup"],
              "RestartPolicy": { "Name": "", "MaximumRetryCount": 0 },
+             "AutoRemove": true,
              "NetworkMode": "bridge",
              "Devices": [],
              "Ulimits": [{}],
@@ -599,6 +600,7 @@ Return low-level information on the container `id`
 				"MaximumRetryCount": 2,
 				"Name": "on-failure"
 			},
+			"AutoRemove": true,
 			"LogConfig": {
 				"Config": null,
 				"Type": "json-file"

integration-cli/docker_api_containers_test.go

@@ -476,7 +476,7 @@ func (s *DockerSuite) TestContainerApiBadPort(c *check.C) {
 	status, body, err := sockRequest("POST", "/containers/create", config)
 	c.Assert(err, checker.IsNil)
 	c.Assert(status, checker.Equals, http.StatusInternalServerError)
-	c.Assert(getErrorMessage(c, body), checker.Equals, `Invalid port specification: "aa80"`, check.Commentf("Incorrect error msg: %s", body))
+	c.Assert(getErrorMessage(c, body), checker.Equals, `invalid port specification: "aa80"`, check.Commentf("Incorrect error msg: %s", body))
 }
 
 func (s *DockerSuite) TestContainerApiCreate(c *check.C) {
@@ -700,7 +700,7 @@ func (s *DockerSuite) TestContainerApiInvalidPortSyntax(c *check.C) {
 
 	b, err := readBody(body)
 	c.Assert(err, checker.IsNil)
-	c.Assert(string(b[:]), checker.Contains, "Invalid port")
+	c.Assert(string(b[:]), checker.Contains, "invalid port")
 }
 
 // Issue 7941 - test to make sure a "null" in JSON is just ignored.

man/docker-run.1.md

@@ -468,7 +468,9 @@ its root filesystem mounted as read only prohibiting any writes.
    Restart policy to apply when a container exits (no, on-failure[:max-retry], always, unless-stopped).
 
 **--rm**=*true*|*false*
-   Automatically remove the container when it exits (incompatible with -d). The default is *false*.
+   Automatically remove the container when it exits. The default is *false*.
+   `--rm` flag can work together with `-d`, and auto-removal will be done on daemon side. Note that it's
+incompatible with any restart policy other than `none`.
 
 **--security-opt**=[]
    Security Options

runconfig/opts/parse.go

@@ -103,7 +103,7 @@ type ContainerOptions struct {
 	flHealthTimeout     time.Duration
 	flHealthRetries     int
 	flRuntime           string
-	flAutoRemove        *bool
+	flAutoRemove        bool
 
 	Image string
 	Args  []string
@@ -164,7 +164,7 @@ func AddFlags(flags *pflag.FlagSet) *ContainerOptions {
 	flags.Var(copts.flUlimits, "ulimit", "Ulimit options")
 	flags.StringVarP(&copts.flUser, "user", "u", "", "Username or UID (format: <name|uid>[:<group|gid>])")
 	flags.StringVarP(&copts.flWorkingDir, "workdir", "w", "", "Working directory inside the container")
-	flags.BoolVarP(&copts.flAutoRemove, "rm", false, "Automatically remove the container when it exits")
+	flags.BoolVar(&copts.flAutoRemove, "rm", false, "Automatically remove the container when it exits")
 
 	// Security
 	flags.Var(&copts.flCapAdd, "cap-add", "Add Linux capabilities")