Skip to content

Commit 6dd8e10

Browse files
WeiZhang555WeiZhang555
committed
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>
1 parent 3c2886d commit 6dd8e10

File tree

9 files changed

+114
-19
lines changed

9 files changed

+114
-19
lines changed

‎api/client/container/run.go‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"runtime"
99
"strings"
1010
"syscall"
11+
"time"
1112

1213
"golang.org/x/net/context"
1314

@@ -143,6 +144,7 @@ func runRun(dockerCli *client.DockerCli, flags *pflag.FlagSet, opts *runOptions,
143144
hostConfig.ConsoleSize[0], hostConfig.ConsoleSize[1] = dockerCli.GetTtySize()
144145
}
145146

147+
startTime := time.Now()
146148
ctx, cancelFun := context.WithCancel(context.Background())
147149

148150
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,
230232
}
231233

232234
reportError(stderr, cmdPath, err.Error(), false)
235+
if hostConfig.AutoRemove {
236+
if _, errWait := waitExitOrRemoved(dockerCli, context.Background(), createResponse.ID, hostConfig.AutoRemove, startTime); errWait != nil {
237+
logrus.Debugf("Error waiting container's removal: %v", errWait)
238+
}
239+
}
233240
return runStartContainerErr(err)
234241
}
235242

@@ -256,15 +263,9 @@ func runRun(dockerCli *client.DockerCli, flags *pflag.FlagSet, opts *runOptions,
256263
var status int
257264

258265
// Attached mode
259-
if !config.Tty {
260-
// In non-TTY mode, we can't detach, so we must wait for container exit
261-
client.ContainerWait(context.Background(), createResponse.ID)
262-
}
263-
264-
// In TTY mode, there is a race: if the process dies too slowly, the state could
265-
// be updated after the getExitCode call and result in the wrong exit code being reported
266-
if _, status, err = dockerCli.GetExitCode(ctx, createResponse.ID); err != nil {
267-
return fmt.Errorf("tty: status: %d; error: %v;", status, err)
266+
status, err = waitExitOrRemoved(dockerCli, ctx, createResponse.ID, hostConfig.AutoRemove, startTime)
267+
if err != nil {
268+
return fmt.Errorf("Error waiting container to exit: %v", err)
268269
}
269270

270271
if status != 0 {

‎api/client/container/utils.go‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,100 @@
11
package container
22

33
import (
4+
"fmt"
5+
"strconv"
6+
"time"
7+
48
"golang.org/x/net/context"
59

10+
"github.com/Sirupsen/logrus"
611
"github.com/docker/docker/api/client"
12+
"github.com/docker/docker/api/client/system"
713
clientapi "github.com/docker/engine-api/client"
14+
"github.com/docker/engine-api/types"
15+
"github.com/docker/engine-api/types/events"
16+
"github.com/docker/engine-api/types/filters"
817
)
918

19+
func waitExitOrRemoved(dockerCli *client.DockerCli, ctx context.Context, containerID string, waitRemove bool, since time.Time) (int, error) {
20+
if len(containerID) == 0 {
21+
// containerID can never be empty
22+
panic("Internal Error: waitExitOrRemoved needs a containerID as parameter")
23+
}
24+
25+
var exitCode int
26+
exitChan := make(chan struct{})
27+
detachChan := make(chan struct{})
28+
destroyChan := make(chan struct{})
29+
30+
// Start watch events
31+
eh := system.InitEventHandler()
32+
eh.Handle("die", func(e events.Message) {
33+
if len(e.Actor.Attributes) > 0 {
34+
for k, v := range e.Actor.Attributes {
35+
if k == "exitCode" {
36+
var err error
37+
if exitCode, err = strconv.Atoi(v); err != nil {
38+
logrus.Errorf("Can't convert %q to int: %v", v, err)
39+
}
40+
close(exitChan)
41+
break
42+
}
43+
}
44+
}
45+
})
46+
47+
eh.Handle("detach", func(e events.Message) {
48+
exitCode = 0
49+
close(detachChan)
50+
})
51+
eh.Handle("destroy", func(e events.Message) {
52+
close(destroyChan)
53+
})
54+
55+
eventChan := make(chan events.Message)
56+
go eh.Watch(eventChan)
57+
defer close(eventChan)
58+
59+
// Get events via Events API
60+
f := filters.NewArgs()
61+
f.Add("type", "container")
62+
f.Add("container", containerID)
63+
options := types.EventsOptions{
64+
Since: fmt.Sprintf("%d", since.Unix()),
65+
Filters: f,
66+
}
67+
resBody, err := dockerCli.Client().Events(ctx, options)
68+
if err != nil {
69+
return -1, fmt.Errorf("can't get events from daemon: %v", err)
70+
}
71+
defer resBody.Close()
72+
73+
go system.DecodeEvents(resBody, func(event events.Message, err error) error {
74+
if err != nil {
75+
return nil
76+
}
77+
eventChan <- event
78+
return nil
79+
})
80+
81+
if waitRemove {
82+
select {
83+
case <-destroyChan:
84+
return exitCode, nil
85+
case <-detachChan:
86+
return 0, nil
87+
}
88+
} else {
89+
select {
90+
case <-exitChan:
91+
return exitCode, nil
92+
case <-detachChan:
93+
return 0, nil
94+
}
95+
}
96+
}
97+
1098
// getExitCode performs an inspect on the container. It returns
1199
// the running state and the exit code.
12100
func getExitCode(dockerCli *client.DockerCli, ctx context.Context, containerID string) (bool, int, error) {

‎daemon/container.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ func (daemon *Daemon) verifyContainerSettings(hostConfig *containertypes.HostCon
210210
if config.WorkingDir != "" {
211211
config.WorkingDir = filepath.FromSlash(config.WorkingDir) // Ensure in platform semantics
212212
if !system.IsAbs(config.WorkingDir) {
213-
return nil, fmt.Errorf("The working directory '%s' is invalid. It needs to be an absolute path", config.WorkingDir)
213+
return nil, fmt.Errorf("the working directory '%s' is invalid, it needs to be an absolute path", config.WorkingDir)
214214
}
215215
}
216216

@@ -239,18 +239,18 @@ func (daemon *Daemon) verifyContainerSettings(hostConfig *containertypes.HostCon
239239
}
240240

241241
if hostConfig.AutoRemove && !hostConfig.RestartPolicy.IsNone() {
242-
return nil, fmt.Errorf("Can't create 'AutoRemove' container with restart policy")
242+
return nil, fmt.Errorf("can't create 'AutoRemove' container with restart policy")
243243
}
244244

245245
for port := range hostConfig.PortBindings {
246246
_, portStr := nat.SplitProtoPort(string(port))
247247
if _, err := nat.ParsePort(portStr); err != nil {
248-
return nil, fmt.Errorf("Invalid port specification: %q", portStr)
248+
return nil, fmt.Errorf("invalid port specification: %q", portStr)
249249
}
250250
for _, pb := range hostConfig.PortBindings[port] {
251251
_, err := nat.NewPort(nat.SplitProtoPort(pb.HostPort))
252252
if err != nil {
253-
return nil, fmt.Errorf("Invalid port specification: %q", pb.HostPort)
253+
return nil, fmt.Errorf("invalid port specification: %q", pb.HostPort)
254254
}
255255
}
256256
}

‎daemon/daemon.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -292,10 +292,10 @@ func (daemon *Daemon) restore() error {
292292
for id := range removeContainers {
293293
removeGroup.Add(1)
294294
go func(cid string) {
295-
defer removeGroup.Done()
296295
if err := daemon.ContainerRm(cid, &types.ContainerRmConfig{ForceRemove: true, RemoveVolume: true}); err != nil {
297296
logrus.Errorf("Failed to remove container %s: %s", cid, err)
298297
}
298+
removeGroup.Done()
299299
}(id)
300300
}
301301
removeGroup.Wait()

‎docs/reference/api/docker_remote_api.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,8 @@ This section lists each version from latest to oldest. Each listing includes a
116116

117117
[Docker Remote API v1.25](docker_remote_api_v1.25.md) documentation
118118

119+
* `POST /containers/create` now takes `AutoRemove` in HostConfig, auto-removal will be done on daemon side.
120+
119121
### v1.24 API changes
120122

121123
[Docker Remote API v1.24](docker_remote_api_v1.24.md) documentation

‎docs/reference/api/docker_remote_api_v1.25.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,7 @@ Create a container
325325
"CapDrop": ["MKNOD"],
326326
"GroupAdd": ["newgroup"],
327327
"RestartPolicy": { "Name": "", "MaximumRetryCount": 0 },
328+
"AutoRemove": true,
328329
"NetworkMode": "bridge",
329330
"Devices": [],
330331
"Ulimits": [{}],
@@ -599,6 +600,7 @@ Return low-level information on the container `id`
599600
"MaximumRetryCount": 2,
600601
"Name": "on-failure"
601602
},
603+
"AutoRemove": true,
602604
"LogConfig": {
603605
"Config": null,
604606
"Type": "json-file"

‎integration-cli/docker_api_containers_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -476,7 +476,7 @@ func (s *DockerSuite) TestContainerApiBadPort(c *check.C) {
476476
status, body, err := sockRequest("POST", "/containers/create", config)
477477
c.Assert(err, checker.IsNil)
478478
c.Assert(status, checker.Equals, http.StatusInternalServerError)
479-
c.Assert(getErrorMessage(c, body), checker.Equals, `Invalid port specification: "aa80"`, check.Commentf("Incorrect error msg: %s", body))
479+
c.Assert(getErrorMessage(c, body), checker.Equals, `invalid port specification: "aa80"`, check.Commentf("Incorrect error msg: %s", body))
480480
}
481481

482482
func (s *DockerSuite) TestContainerApiCreate(c *check.C) {
@@ -700,7 +700,7 @@ func (s *DockerSuite) TestContainerApiInvalidPortSyntax(c *check.C) {
700700

701701
b, err := readBody(body)
702702
c.Assert(err, checker.IsNil)
703-
c.Assert(string(b[:]), checker.Contains, "Invalid port")
703+
c.Assert(string(b[:]), checker.Contains, "invalid port")
704704
}
705705

706706
// Issue 7941 - test to make sure a "null" in JSON is just ignored.

‎man/docker-run.1.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -468,7 +468,9 @@ its root filesystem mounted as read only prohibiting any writes.
468468
Restart policy to apply when a container exits (no, on-failure[:max-retry], always, unless-stopped).
469469

470470
**--rm**=*true*|*false*
471-
Automatically remove the container when it exits (incompatible with -d). The default is *false*.
471+
Automatically remove the container when it exits. The default is *false*.
472+
`--rm` flag can work together with `-d`, and auto-removal will be done on daemon side. Note that it's
473+
incompatible with any restart policy other than `none`.
472474

473475
**--security-opt**=[]
474476
Security Options

‎runconfig/opts/parse.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ type ContainerOptions struct {
103103
flHealthTimeout time.Duration
104104
flHealthRetries int
105105
flRuntime string
106-
flAutoRemove *bool
106+
flAutoRemove bool
107107

108108
Image string
109109
Args []string
@@ -164,7 +164,7 @@ func AddFlags(flags *pflag.FlagSet) *ContainerOptions {
164164
flags.Var(copts.flUlimits, "ulimit", "Ulimit options")
165165
flags.StringVarP(&copts.flUser, "user", "u", "", "Username or UID (format: <name|uid>[:<group|gid>])")
166166
flags.StringVarP(&copts.flWorkingDir, "workdir", "w", "", "Working directory inside the container")
167-
flags.BoolVarP(&copts.flAutoRemove, "rm", false, "Automatically remove the container when it exits")
167+
flags.BoolVar(&copts.flAutoRemove, "rm", false, "Automatically remove the container when it exits")
168168

169169
// Security
170170
flags.Var(&copts.flCapAdd, "cap-add", "Add Linux capabilities")

0 commit comments

Comments
 (0)