Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix systemd docker daemon conflicting with daemon.json #27473

Closed
wants to merge 1 commit into from
Closed

Bugfix systemd docker daemon conflicting with daemon.json #27473

wants to merge 1 commit into from

Conversation

samrocketman
Copy link

- What I did

On Ubuntu package:

# apt-cache show docker.io
Package: docker.io
Priority: optional
Section: universe/admin
Installed-Size: 54232
Maintainer: Ubuntu Developers <ubuntu-devel-discuss@lists.ubuntu.com>
Original-Maintainer: Paul Tagliamonte <paultag@debian.org>
Architecture: amd64
Version: 1.12.1-0ubuntu13~16.04.1
Replaces: docker (<< 1.5~)
Depends: adduser, containerd (>= 0.2.3~), iptables, runc (>= 1.0.0~rc1~), init-system-helpers (>= 1.18~), lsb-base (>= 4.1+Debian11ubuntu7), libapparmor1 (>= 2.6~devel), libc6 (>= 2.14), libdevmapper1.02.1 (>= 2:1.02.97)
Recommends: ca-certificates, cgroupfs-mount | cgroup-lite, git, ubuntu-fan, xz-utils, apparmor
Suggests: aufs-tools, btrfs-tools, debootstrap, docker-doc, rinse, zfs-fuse | zfsutils
Breaks: docker (<< 1.5~)
Filename: pool/universe/d/docker.io/docker.io_1.12.1-0ubuntu13~16.04.1_amd64.deb
Size: 10581186
MD5sum: 08676d065c43fc6bcb450cf52c782ed5
SHA1: 8f51b8c7b0fbc3eb879e13a76286cbe13b096d34
SHA256: 0987a665d090d90b0096fbdcab7d18316cad6a1754dbc70f56ab74c36ba53c51
...

I created /etc/default/daemon.json with the contents.

{
    "hosts": ["tcp://127.0.0.1:2700"],
    "tlsverify": true,
    "tlscacert": "/etc/docker/ca.pem",
    "tlscert": "/etc/docker/cert.pem",
    "tlskey": "/etc/docker/key.pem"
}

Starting the docker daemon (systemctl start docker.service) causes the error

unable to configure the Docker daemon with file
/etc/docker/daemon.json: the following directives are specified both as
a flag and in the configuration file: hosts: (from flag: [fd://], from
file: [tcp://127.0.0.1:2700])

- How I did it

Patch the docker.service and the error goes away.

- How to verify it

  1. Install the above mentioned docker.io package in Ubuntu 16.04.
  2. Patch /lib/systemd/system/docker.service with the changes in this PR.
  3. Restart the daemon and it should succeed.
  4. Configure the hosts field of /etc/docker/daemon.json. Restart and watch restart success.

- Description for the changelog

systemd: fix conflicting hosts option between docker.service and daemon.json

cute cat

fixes #22339

recommended `/etc/docker/daemon.json`.  An example use case is
configuring daemon.json with TLS verify and setting the listening
interface with the hosts field.

Example JSON which causes docker daemon to fail to start.

```json
{
    "hosts": ["tcp://127.0.0.1:2700"],
    "tlsverify": true,
    "tlscacert": "/etc/docker/ca.pem",
    "tlscert": "/etc/docker/cert.pem",
    "tlskey": "/etc/docker/key.pem"
}
```

Error encountered:

> unable to configure the Docker daemon with file
> /etc/docker/daemon.json: the following directives are specified both as
> a flag and in the configuration file: hosts: (from flag: [fd://], from
> file: [tcp://127.0.0.1:2700])

fixes #22339

Signed-off-by: Sam Gleske <sam.mxracer@gmail.com>
@@ -9,7 +9,7 @@ Type=notify
# the default is not to use systemd for cgroups because the delegate issues still
# exists and systemd currently does not support the cgroup feature set required
# for containers run by docker
ExecStart=/usr/bin/dockerd -H fd://
ExecStart=/usr/bin/dockerd
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is not correct, as -H fd:// is required for socket activation (see https://github.com/docker/docker/pull/27473/files#diff-ff907ce70a8c7e795bde1de91be6fa68R5 above)

Copy link
Author

@samrocketman samrocketman Oct 18, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Except, my docker instance works just fine without that option. That option also directly conflicts with daemon.json. It's not possible to configure the hosts field in daemon.json so long as that option exists.

Docker should provide a default socket if none is specified (and it does).

Copy link
Author

@samrocketman samrocketman Oct 18, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW: my /lib/systemd/system/docker.service file has...

...
ExecStart=/usr/bin/dockerd $DOCKER_OPTS
...

And in /etc/default/docker I have DOCKER_OPTS="".

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, it's really not recommended to edit the docker.service directly (e.g. to add $DOCKER_OPTS. Doing so may result in the unit file not being updated if a new version becomes available. To make changes, always use an override ("drop-in") file https://docs.docker.com/engine/admin/systemd/#/custom-docker-daemon-options

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what issues you're describing about the docker socket. When I make the described changes docker still creates a socket and I can run docker client commands just fine (e.g. docker images).

The current configuration makes configuring hosts via daemon.json not possible. It means people are forced to configure the daemon via /etc/default/docker which clearly states as not recommended.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An excerpt from /etc/default/docker:

# Here in Debian, this file is sourced by:
#   - /etc/init.d/docker (sysvinit)
#   - /etc/init/docker (upstart)
#   - systemd's docker.service

# Use of this file for configuring your Docker daemon is discouraged.

# The recommended alternative is "/etc/docker/daemon.json", as described in:
#   https://docs.docker.com/v1.11/engine/reference/commandline/daemon/#daemon-configuration-file

Notice Use of this file for configuring your Docker daemon is discouraged.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That message is (although not incorrect) probably added by the Ubuntu packagers, as it's not in the official packages from this repository https://github.com/docker/docker/blob/v1.12.2/contrib/init/sysvinit-debian/docker.default#L3-L8

The socket is still created with this change, however socket activation no longer works; socket activation allows docker to be automatically started when you're trying to connect to it (and without the service running before that)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thaJeztah what change do you think would be appropriate here? Do you recognize the issue I'm raising and agree it's an issue?

It seems a bit of a chicken and egg scenario. I want to use daemon.json but -H fd:// is preventing me (although, I could add that to the hosts of the daemon.json).

Copy link
Author

@samrocketman samrocketman Oct 18, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is socket activation necessary here? I stopped the docker daemon and then ran docker images. It seems it didn't start the docker daemon for me or perhaps I misunderstand. Instead, I got the following message.

Cannot connect to the Docker daemon. Is the docker daemon running on this host?

@samrocketman samrocketman changed the title Bugfix systemd docker daemon conflicting with Bugfix systemd docker daemon conflicting with daemon.json Oct 18, 2016
@samrocketman
Copy link
Author

samrocketman commented Oct 18, 2016

I'm going to open a new issue related to this and amend my commit.

EDIT: I'll just let this PR serve as my issue unless I'm required otherwise.

@justincormack
Copy link
Contributor

justincormack commented Oct 18, 2016

We don't recommend using socket activation, it means container restart does
not work for example.

@thaJeztah
Copy link
Member

@justincormack that was for RPM based installs (for which we removed it), on .deb based installs, socket activation is still in use

@justincormack
Copy link
Contributor

@thaJeztah why? It causes the same issues. I think we should remove it everywhere.

@thaJeztah
Copy link
Member

@justincormack I'm not against removing it, as I don't think it's widely used (actually suggested that on other occasions), but based on #24804 (comment) its not causing issues for deb-based systems

@samrocketman
Copy link
Author

samrocketman commented Oct 20, 2016

its not causing issues for deb-based systems

It's causing issues on my deb-based system because I wish to make use of daemon.json. Have you tried to configure this in any other deb-based system? I could always test other debian flavors with vagrant.

@thaJeztah
Copy link
Member

It's causing issues on my deb-based system because I wish to make use of daemon.json

@samrocketman that comment was about the reason socket activation was not a problem for deb-based installs for restarting.

A related discussion of the conflict between daemon.json and fd:// / socket activation is here; #25471

@samrocketman
Copy link
Author

Thanks for the reference, I've subscribed for updates.

@LK4D4
Copy link
Contributor

LK4D4 commented Nov 3, 2016

@thaJeztah what's up here?

@thaJeztah
Copy link
Member

@LK4D4 this is about removing socket activation from all systemd unit files, but there was no consensus yet if that's a good thing to do

ping @tonistiigi

@tonistiigi
Copy link
Member

Socket activation works fine when you just specify it in the json file and leave out the fd:// from unit file. But I think it would be much more obvious to the user if we would just merge the host arrays instead of forcing the user to use one file or the another.

@tonistiigi
Copy link
Member

I think socket activation is a useful feature. If we don't use it we are not following the best practices for unit files. It's a shame that selinux and systemd can't figure out how to work together in fedora. For the perspective of specifying hosts, socket activation is no different. We would have the same problem if you specify -H unix:///var/run/docker.sock in unit file. The only difference is that we have different rules for merging config and cli options and merging config and default options. If we don't want to merge them I think we could also just have an exception for fd://.

@thaJeztah
Copy link
Member

thaJeztah commented Dec 8, 2016

We discussed this in the maintainers meeting, and this PR in its current form cannot be merged due to it being needed for socket activation.

We are open to having a config command that shows the active configuration, and a change in policy for the daemon.json to merge list options, instead of error out if both a flag an daemon.json is specified.

I'll open an issue for tracking later

update: there's an issue tracking this already, see #21559

@nandlalyadav57
Copy link

I am facing an issue while updating below entries in docker daemon.json
{
"log-driver": "json-file",
"log-opts": {
"max-size": "100m",
"max-file": "5"
}
}

if i used override options my containers won't come up

[systemm]# cat /lib/systemd/system/docker.service
[Unit]
Description=Docker Application Container Engine
Documentation=http://docs.docker.com
After=network.target
Wants=docker-storage-setup.service
Requires=docker-cleanup.timer

[Service]
Type=notify
NotifyAccess=main
EnvironmentFile=-/run/containers/registries.conf
EnvironmentFile=-/etc/sysconfig/docker
EnvironmentFile=-/etc/sysconfig/docker-storage
EnvironmentFile=-/etc/sysconfig/docker-network
Environment=GOTRACEBACK=crash
Environment=DOCKER_HTTP_HOST_COMPAT=1
Environment=PATH=/usr/libexec/docker:/usr/bin:/usr/sbin
ExecStart=/usr/bin/dockerd-current
--add-runtime docker-runc=/usr/libexec/docker/docker-runc-current
--default-runtime=docker-runc
--exec-opt native.cgroupdriver=systemd
--userland-proxy-path=/usr/libexec/docker/docker-proxy-current
--init-path=/usr/libexec/docker/docker-init-current
--seccomp-profile=/etc/docker/seccomp.json
$OPTIONS
$DOCKER_STORAGE_OPTIONS
$DOCKER_NETWORK_OPTIONS
$ADD_REGISTRY
$BLOCK_REGISTRY
$INSECURE_REGISTRY
$REGISTRIES
ExecReload=/bin/kill -s HUP $MAINPID
LimitNOFILE=1048576
LimitNPROC=1048576
LimitCORE=infinity
TimeoutStartSec=0
Restart=on-abnormal
KillMode=process

[Install]
WantedBy=multi-user.target

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot load hosts in configuration json file when starting docker daemon
8 participants