Don't create source directory while the daemon is being shutdown, fix #30348 - #33330

coolljt0725 · 2017-05-22T08:54:41Z

If a container mount the socket the daemon is listening on into
container while the daemon is being shutdown, the socket will
not exist on the host, then daemon will assume it's a directory
and create it on the host, this will cause the daemon can't start
next time.

fix issue #30348

To reproduce this issue, you can add following code

--- a/daemon/oci_linux.go
+++ b/daemon/oci_linux.go
@@ -8,6 +8,7 @@ import (
        "sort"
        "strconv"
        "strings"
+       "time"

        "github.com/Sirupsen/logrus"
        "github.com/docker/docker/container"
@@ -666,7 +667,8 @@ func (daemon *Daemon) createSpec(c *container.Container) (*libcontainerd.Spec, e
        if err := daemon.setupIpcDirs(c); err != nil {
                return nil, err
        }
-
+       fmt.Printf("===please stop the daemon===\n")
+       time.Sleep(time.Second * 2)
        ms, err := daemon.setupMounts(c)
        if err != nil {
                return nil, err

step1 run a container which has --restart always and -v /var/run/docker.sock:/sock

$ docker run -ti --restart always -v /var/run/docker.sock:/sock busybox
/ #

step2 exit the the container

/ # exit

and kill the daemon when you see

===please stop the daemon===

in the daemon log

The daemon can't restart again and fail with can't create unix socket /var/run/docker.sock: is a directory.

Signed-off-by: Lei Jitang leijitang@huawei.com

ping @cpuguy83 @thaJeztah

- What I did

- How I did it

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

thaJeztah · 2017-05-22T12:39:30Z

@coolljt0725 looks like build is failing;

11:26:03 # github.com/docker/docker/daemon
11:26:03 daemon/volumes_windows.go:27: not enough arguments in call to mount.Setup
11:26:03 	have (string, number, number)
11:26:03 	want (string, int, int, func() error)

coolljt0725 · 2017-05-23T05:44:57Z

@thaJeztah updated

thaJeztah · 2017-05-30T10:07:47Z

ping @cpuguy83 @mlaventure PTAL

mlaventure

A few nits and a requested change to checkFun prototype.

Otherwise, LGTM. Thanks for finding this!

mlaventure · 2017-05-30T14:42:42Z

nit:

If the daemon is being shutdown, we should not let a container start if it is trying to mount the socket the daemon is listening on. During daemon shutdown, the socket (/var/run/docker.sock by default) doesn't exist anymore causing the call to m.Setup to create at directory instead. This in turn will prevent the daemon to restart.

mlaventure · 2017-05-30T14:45:55Z

I think I'd prefer the callback to be of the form func(m *Mountpoint) error

mlaventure · 2017-05-30T14:46:55Z

nit:

The, optional, checkFun parameter allows doing additional checking before creating the source directory on the host.

mlaventure · 2017-05-30T14:49:26Z

nit:

Before creating the source directory on the host, invoke checkFun if it's not nil. One of the use case is to forbid creating the daemon socket as a directory if the daemon is in the process of shutting down.

coolljt0725 · 2017-05-31T02:50:51Z

@mlaventure update

…oby#30348 If a container mount the socket the daemon is listening on into container while the daemon is being shutdown, the socket will not exist on the host, then daemon will assume it's a directory and create it on the host, this will cause the daemon can't start next time. fix issue moby#30348 To reproduce this issue, you can add following code ``` --- a/daemon/oci_linux.go +++ b/daemon/oci_linux.go @@ -8,6 +8,7 @@ import ( "sort" "strconv" "strings" + "time" "github.com/Sirupsen/logrus" "github.com/docker/docker/container" @@ -666,7 +667,8 @@ func (daemon *Daemon) createSpec(c *container.Container) (*libcontainerd.Spec, e if err := daemon.setupIpcDirs(c); err != nil { return nil, err } - + fmt.Printf("===please stop the daemon===\n") + time.Sleep(time.Second * 2) ms, err := daemon.setupMounts(c) if err != nil { return nil, err ``` step1 run a container which has `--restart always` and `-v /var/run/docker.sock:/sock` ``` $ docker run -ti --restart always -v /var/run/docker.sock:/sock busybox / # ``` step2 exit the the container ``` / # exit ``` and kill the daemon when you see ``` ===please stop the daemon=== ``` in the daemon log The daemon can't restart again and fail with `can't create unix socket /var/run/docker.sock: is a directory`. Signed-off-by: Lei Jitang <leijitang@huawei.com>

mlaventure

LGTM

Thanks!

AkihiroSuda · 2017-06-07T03:27:20Z

+// StoreHosts stores the addresses the daemon is listening on
+func (daemon *Daemon) StoreHosts(hosts []string) {
+	if daemon.hosts == nil {
+		daemon.hosts = make(map[string]bool)


Can we initialize daemon.hosts in NewDaemon()?

I have considered that, but it will have some duplicate code to dealing with the hosts config (https://github.com/moby/moby/blob/master/cmd/dockerd/daemon.go#L157).

AkihiroSuda

LGTM

GordonTheTurtle added the status/0-triage label May 22, 2017

coolljt0725 added status/1-design-review and removed status/0-triage labels May 22, 2017

coolljt0725 force-pushed the fix_sock_is_dir branch from 83328f5 to 806c5e8 Compare May 23, 2017 01:13

thaJeztah added the rebuild/z label May 30, 2017

GordonTheTurtle removed the rebuild/z label May 30, 2017

mlaventure suggested changes May 30, 2017

View reviewed changes

coolljt0725 force-pushed the fix_sock_is_dir branch from 806c5e8 to 7318eba Compare May 31, 2017 02:42

dhiltgen mentioned this pull request May 31, 2017

17.06.0 RC2 tracker docker/for-linux#2

Closed

23 tasks done

thaJeztah added status/2-code-review and removed status/1-design-review labels May 31, 2017

mlaventure approved these changes May 31, 2017

View reviewed changes

mlaventure added the rebuild/z label May 31, 2017

GordonTheTurtle removed the rebuild/z label May 31, 2017

GordonTheTurtle assigned AkihiroSuda Jun 6, 2017

andrewhsu mentioned this pull request Jun 6, 2017

17.06.0 RC3 tracker docker/for-linux#8

Closed

40 tasks done

AkihiroSuda reviewed Jun 7, 2017

View reviewed changes

AkihiroSuda approved these changes Jun 7, 2017

View reviewed changes

AkihiroSuda merged commit cd2255a into moby:master Jun 7, 2017

GordonTheTurtle added this to the 17.06.0 milestone Jun 7, 2017

coolljt0725 deleted the fix_sock_is_dir branch June 7, 2017 03:38

thaJeztah mentioned this pull request Jun 7, 2017

can't create unix socket /var/run/docker.sock: is a directory #30348

Closed

seemethere mentioned this pull request Jun 9, 2017

Don't create source directory while the daemon is being shutdown, fix… docker-archive/docker-ce#48

Merged

thaJeztah added the process/cherry-picked label Jun 10, 2017

thaJeztah mentioned this pull request Jul 13, 2017

vndr github.com/docker/docker from components/engine docker-archive/docker-ce#116

Closed

This was referenced Oct 11, 2018

Don't create source directory while the daemon is being shutdown projectatomic/docker#327

Merged

Don't create source directory while the daemon is being shutdown projectatomic/docker#328

Merged

Navigation Menu

Repository navigation

Don't create source directory while the daemon is being shutdown, fix #30348 - #33330

Don't create source directory while the daemon is being shutdown, fix #30348#33330
AkihiroSuda merged 1 commit into
moby:masterfrom
coolljt0725:fix_sock_is_dir

coolljt0725 commented May 22, 2017

Uh oh!

thaJeztah commented May 22, 2017

Uh oh!

coolljt0725 commented May 23, 2017

Uh oh!

thaJeztah commented May 30, 2017

Uh oh!

mlaventure left a comment

Uh oh!

mlaventure May 30, 2017

Uh oh!

mlaventure May 30, 2017

Uh oh!

mlaventure May 30, 2017

Uh oh!

mlaventure May 30, 2017

Uh oh!

coolljt0725 commented May 31, 2017

Uh oh!

mlaventure left a comment

Uh oh!

AkihiroSuda Jun 7, 2017

Uh oh!

coolljt0725 Jun 7, 2017

Uh oh!

AkihiroSuda left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

coolljt0725 commented May 22, 2017

Uh oh!

thaJeztah commented May 22, 2017

Uh oh!

coolljt0725 commented May 23, 2017

Uh oh!

thaJeztah commented May 30, 2017

Uh oh!

mlaventure left a comment

Choose a reason for hiding this comment

Uh oh!

mlaventure May 30, 2017

Choose a reason for hiding this comment

Uh oh!

mlaventure May 30, 2017

Choose a reason for hiding this comment

Uh oh!

mlaventure May 30, 2017

Choose a reason for hiding this comment

Uh oh!

mlaventure May 30, 2017

Choose a reason for hiding this comment

Uh oh!

coolljt0725 commented May 31, 2017

Uh oh!

mlaventure left a comment

Choose a reason for hiding this comment

Uh oh!

AkihiroSuda Jun 7, 2017

Choose a reason for hiding this comment

Uh oh!

coolljt0725 Jun 7, 2017

Choose a reason for hiding this comment

Uh oh!

AkihiroSuda left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants