Browse code

On startup, actually shutdown the container.

When a container is left running after the daemon exits (e.g. the daemon
is SIGKILL'd or crashes), it should stop any running containers when the
daemon starts back up.

What actually happens is the daemon only sends the container's
configured stop signal and does not check if it has exited.
If the container does not actually exit then it is left running.

This fixes this unexpected behavior by calling the same function to shut
down the container that the daemon shutdown process does.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>

Brian Goff authored on 2020/04/08 09:03:32
Showing 2 changed files
... ...
@@ -349,10 +349,11 @@ func (daemon *Daemon) restore() error {
349 349
 					return
350 350
 				}
351 351
 			} else if !daemon.configStore.LiveRestoreEnabled {
352
-				if err := daemon.kill(c, c.StopSignal()); err != nil && !errdefs.IsNotFound(err) {
352
+				if err := daemon.shutdownContainer(c); err != nil && !errdefs.IsNotFound(err) {
353 353
 					logrus.WithError(err).WithField("container", c.ID).Error("error shutting down container")
354 354
 					return
355 355
 				}
356
+				c.ResetRestartManager(false)
356 357
 			}
357 358
 
358 359
 			if c.IsRunning() || c.IsPaused() {
359 360
new file mode 100644
... ...
@@ -0,0 +1,51 @@
0
+package container
1
+
2
+import (
3
+	"context"
4
+	"testing"
5
+
6
+	"github.com/docker/docker/api/types"
7
+	"github.com/docker/docker/integration/internal/container"
8
+	"github.com/docker/docker/testutil/daemon"
9
+	"gotest.tools/v3/assert"
10
+	is "gotest.tools/v3/assert/cmp"
11
+	"gotest.tools/v3/skip"
12
+)
13
+
14
+// Make sure a container that does not exit when it upon receiving it's stop signal is actually shutdown on daemon
15
+// startup.
16
+func TestContainerKillOnDaemonStart(t *testing.T) {
17
+	skip.If(t, testEnv.IsRemoteDaemon, "cannot start daemon on remote test run")
18
+	skip.If(t, testEnv.DaemonInfo.OSType == "windows")
19
+	skip.If(t, testEnv.IsRootless, "scenario doesn't work with rootless mode")
20
+
21
+	t.Parallel()
22
+
23
+	d := daemon.New(t)
24
+	defer d.Cleanup(t)
25
+
26
+	d.StartWithBusybox(t, "--iptables=false")
27
+	defer d.Stop(t)
28
+
29
+	client := d.NewClientT(t)
30
+	ctx := context.Background()
31
+
32
+	// The intention of this container is to ignore stop signals.
33
+	// Sadly this means the test will take longer, but at least this test can be parallelized.
34
+	id := container.Run(ctx, t, client, container.WithCmd("/bin/sh", "-c", "while true; do echo hello; sleep 1; done"))
35
+	defer func() {
36
+		err := client.ContainerRemove(ctx, id, types.ContainerRemoveOptions{Force: true})
37
+		assert.NilError(t, err)
38
+	}()
39
+
40
+	inspect, err := client.ContainerInspect(ctx, id)
41
+	assert.NilError(t, err)
42
+	assert.Assert(t, inspect.State.Running)
43
+
44
+	assert.NilError(t, d.Kill())
45
+	d.Start(t)
46
+
47
+	inspect, err = client.ContainerInspect(ctx, id)
48
+	assert.Check(t, is.Nil(err))
49
+	assert.Assert(t, !inspect.State.Running)
50
+}