garethr / garethr-docker

Puppet module for managing docker
Apache License 2.0
397 stars 531 forks source link

Docker Community Edition `docker daemon` is not supported on Linux. #701

Open ocastle opened 7 years ago

ocastle commented 7 years ago

Encountered the following error on Ubuntu 16.04 running docker version 17.06.0-ce.

`docker daemon is not supported on Linux. Please run dockerd directly`

The ExecStart= line of the 'service-overrides-debian.conf.erb' template needs to use 'dockerd' as the command instead of 'docker daemon' when using the Community Edition. Editing the value and reloading the systemd daemon confirmed. Puppet run will revert the change, but daemon will continue running until restarted and require the same change to restore service.

grantholly commented 7 years ago

@ocastle would you be willing to test out some changes that I have in a fork? I've been having the same problem with CentOS and docker 17.06.0-ce, but I've hacked together a workaround. Its in my fork https://github.com/grantholly/garethr-docker.

vladgh commented 7 years ago

From my tests, the latest docker (docker-ce_17.06.0~ce-0~ubuntu) on Xenial does not use the /etc/systemd/system/docker.service.d/service-overrides.conf file. The workaround for me was to delete that file altogether, rather than modifying /usr/bin/docker daemon to /usr/bin/dockerd.

I'm not sure, but I think /lib/systemd/system/docker.service is used instead.

ocastle commented 7 years ago

@grantholly yes i'd be willing to take a look at the changes and integrating into my own fork for testing.

Morsicus commented 7 years ago

Hi,

Any news on this issue ? :)

ocastle commented 7 years ago

The changes from @grantholly were integrated into the working fork used for testing and were verified to solve the issue faced on Ubuntu:16.04

LongLiveCHIEF commented 7 years ago

The workaround for me was to delete that file altogether, rather than modifying /usr/bin/docker daemon to /usr/bin/dockerd.

This is related to #703 & #698. You actually shouldn't have to manually perform any workarounds, and should instead be able to pass in dockerd as the docker_command, but it's not writing out to the correct file.

juliantaylor commented 7 years ago

In addition to changing docker_command to dockerd, you also need to override daemon_subcommand to an empty string to get a correct service file override. See https://github.com/garethr/garethr-docker/blob/master/templates/etc/systemd/system/docker.service.d/service-overrides-debian.conf.erb

LongLiveCHIEF commented 7 years ago

Yep, already got that one. Here's what I've done so far.

The challenge I'm working through as we speak is the dual nature of docker_command usage in the module.

In the manifests/run.pp, manifests/image.pp, manifests/exec.pp, etc... docker_command is used as a client library. I can't just change up docker_command = 'dockerd'.

The approach I'm taking right now, is to have 3 parameters (with defaults given):

$docker_command = 'docker'
$daemon_command = 'dockerd'
$daemon_subcommand = undef

Using daemon_subcommand will output a deprecation warning, and shim the correct usage of docker daemon vs dockerd.

This will allow compatibility with any existing downstream manifests while enabling the latest versions of docker to run.

grantholly commented 7 years ago

Is there a preference for making $docker_command work over using facter to figure how to call the docker daemon? I could go either way. I have a pull request open (https://github.com/garethr/garethr-docker/pull/704) that fixes this issue and has been checked to work with at least Ubuntu and CentOS. It could use some review.

LongLiveCHIEF commented 7 years ago

I reviewed the PR, and the only flaw I see is in the assumption that you're always going to have dockerd available. We'd have to declare that this module only supports specific versions of docker, deprecating support for versions of docker > 1.12.

I'm also not certain that the facter technique will work with custom build versions of docker (example being docker-latest package that is released for RedHat family OS's).

LongLiveCHIEF commented 7 years ago

@grantholly would you mind if I pulled your PR (#704) into my fork? While I think it can't do the job on it's own, it will work for like 99% of cases, and will serve as a much more solid base than just assuming /usr/lib/... inside the template files, (which has been bothering me for a while now.)

grantholly commented 7 years ago

@LongLiveCHIEF go for it.

In my PR, there is no assumption that dockerd is installed. The way the fact works is you either get a nil value for $dockerd_binary, as you would in the case of running an older version of docker, or you would get a string value containing the path to the dockerd binary. Here's the details on Facter::Core::Execution#which (http://www.rubydoc.info/gems/facter/2.4.6/Facter/Core/Execution#which-class_method)

In the unit file templates, (https://github.com/garethr/garethr-docker/pull/704/files#diff-7dae3e5d9fd27c34844a4d6e0485e2e0L8) and (https://github.com/garethr/garethr-docker/pull/704/files#diff-1ae24078d52aa1e01fa570dd84a1efddR5) there's a boolean check for the binary. If $dockerd_binary is nil then we'll use the older docker daemon command.

LongLiveCHIEF commented 7 years ago

I like it.

davidgibbons commented 7 years ago

That nil value seems to generate a broken systemd file though?

Specifically the first run facter will return no value. As it runs before the package is installed.

vide commented 7 years ago

Hello, facing this issue too, easy solution (waiting for the PR to be merged), if you're using Hiera:

docker::service::docker_command: 'dockerd'
docker::service::daemon_subcommand: ''

This works perfectly with docker 17.06 under CentOS7

LongLiveCHIEF commented 7 years ago

@vide this will only work for installing and starting the docker service. If you use any of the custom docker resources from this module, the above settings will break your puppet catalogs.

docker_command is used in several different ways at the moment, and for the custom resource types, it's used as the client command. This will break image, run, etc...

davidgibbons commented 7 years ago

I bypassed the issue by declaring FACTER_DOCKERD_BINARY=/usr/bin/dockerd in the packer script that was failing. It's an imperfect solution but allowed my image building process to be unblocked.

On Aug 2, 2017 8:45 AM, "Brian Vanderbusch" notifications@github.com wrote:

@vide https://github.com/vide this will only work for installing and starting the docker service. If you use any of the custom docker resources from this module, the above settings will break your puppet catalogs.

docker_command is used in several different ways at the moment, and for the custom resource types, it's used as the client command. This will break image, run, etc...

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/garethr/garethr-docker/issues/701#issuecomment-319712771, or mute the thread https://github.com/notifications/unsubscribe-auth/AAxu2e2-GsGwlVGtQyO95mMzrHHYVzSPks5sUJmHgaJpZM4OLDoA .

vide commented 7 years ago

@LongLiveCHIEF Nope, I'm overriding only the docker::service class, not the base docker class, it works perfectly, I'm running container and pulling images through puppet without any issue

vide commented 7 years ago

At least it works on the latest released version, 5.3.0. Maybe in master is broken, IDK, on mobile right now, I will check tomorrow

vide commented 7 years ago

@LongLiveCHIEF Still work on current master too. The line that makes it work is

https://github.com/garethr/garethr-docker/blob/master/manifests/init.pp#L530

where the docker::service class is contained without overriding any parameter, so if you set docker::service::docker_command and docker::service::daemon_subcommand in Hiera as I said in my first comment, it works. In fact, it's working for me in my production servers.

LongLiveCHIEF commented 7 years ago

Yeah, it does make sense actually, I just never considered the service manifest as an entry point.

Doing it that way would keep the docker command that's inherited from the normal entry point at it's default value, so even the custom resources from this component will still work correctly.

On Aug 4, 2017 4:50 AM, "Davide Ferrari" notifications@github.com wrote:

@LongLiveCHIEF https://github.com/longlivechief Still work on current master to. The line that makes it work is

https://github.com/garethr/garethr-docker/blob/master/ manifests/init.pp#L530

where the docker::service class is contained without overriding any parameter, so if you set docker::service::docker_command and docker::service::daemon_subcommand in Hiera as I said in my first comment, it works. In fact, it's working for me in my production servers.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/garethr/garethr-docker/issues/701#issuecomment-320207912, or mute the thread https://github.com/notifications/unsubscribe-auth/ADIVgdI6px7udm4CqcUUz75rK4FW0mqsks5sUulogaJpZM4OLDoA .

grantholly commented 7 years ago

I would prefer to do this without any code change as you suggest @vide. I've tested your work and it works like a charm. However, It doesn't make sense for me to add docker::service::docker_command to every single manifest where we have the docker class applied. I would like to do something like class { 'docker::service': docker_command => 'dockerd', daemon_subcommand => '', } in one place (our docker wrapper class). However, this causes a duplicate resource declaration.

LongLiveCHIEF commented 7 years ago

I have this in the master of my fork.

On Aug 22, 2017 5:06 PM, "grantholly" notifications@github.com wrote:

I would prefer to do this without any code change as you suggest @vide https://github.com/vide. I've tested your work and it works like a charm. However, It doesn't make sense for me to add docker::service::docker_command to every single manifest where we have the docker class applied. I would like to do something like Class { 'docker::service': docker_command => 'dockerd', daemon_subcommand => '', } in one place (our docker wrapper class). However, this causes a duplicate resource declaration.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/garethr/garethr-docker/issues/701#issuecomment-324165199, or mute the thread https://github.com/notifications/unsubscribe-auth/ADIVgcAyhjXxpSPcG6i-o0JHea-wqc1tks5sa1DkgaJpZM4OLDoA .

NeatNerdPrime commented 7 years ago

TL;DR : declare the storage_driver parameter specifically.

Hi everyone, this is my 2cents on this issue: In my case, it seemed to "correct" itself by specifying the storage driver. I am running on a debian jessie with the latest kernel from backports. Doing so it does not complain anymore about the dockerd issue... still need to investigate why, but i'm happy that it runs smoothly. YMMV.

This is what my manifest declaration looks like:

` apt::source { 'jessie-backports': location => 'http://ftp.debian.org/debian', release => 'jessie-backports', repos => 'main', }

$kernel_pkgs = ['dkms', 'linux-headers-amd64', 'linux-image-amd64']

package { $kernel_pkgs: ensure => latest, install_options => ['-t', 'jessie-backports'], before => Class['::docker'], require => Apt::Source['jessie-backports'], }

class { '::docker': version => 'latest', docker_users => ['johndoe', 'janedoe'], storage_driver => 'overlay2', require => Package[$kernel_pkgs], }

class { '::docker::compose': ensure => present, version => '1.15.0', }`

Pardon me the formatting.

Justin-DynamicD commented 7 years ago

Quick warning: while setting docker::service::docker_command: in Hiera will work for new installs, it's not bullet proof.

Anyone who's run into this problem in frustration and thus doesn't have a running copy of docker wont be able to get back and running this way, as the module itself tries to run docker before applying the service-overrides.conf file and thus will never get to the step of updating the file.

Thus you will be forced to update it manually or with some in-file puppet magic.

Honestly I think the "process" is backwards in the module. all settings should be set before starting/restarting the service, which removes this "the module sets it back but the service is still running" scenario.

Yggdrasil commented 6 years ago

@Justin-DynamicD Exactly.

We worked around this on already broken nodes by adding this ugly exec:

  contain docker

  exec { 'docker-remove-broken-systemd-overrideconf':
    command => '/bin/rm /etc/systemd/system/docker.service.d/service-overrides.conf',
    unless  => 'grep ExecStart=/usr/bin/dockerd /etc/systemd/system/docker.service.d/service-overrides.conf',
    onlyif  => 'test -f /etc/systemd/system/docker.service.d/service-overrides.conf',
    before  => Class['docker::service'],
  }

It deletes the service-overrides.conf file, if it exists and does not contain the correct ExecStart command yet. Afterward the docker::service class drops in the correct file and the daemon starts correctly.

I'll be removing this monster from our manifests as soon as all nodes are confirmed working.