search

garethr / garethr-docker

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

Missing dependency on service override file and systemd reload #407

Closed bcnewlin closed 8 years ago

bcnewlin commented 8 years ago

I am running Docker 1.8.3 on CentOS 7.

I am specifying multiple parameters for starting the Docker daemon such as DNS and storage options. In the new version of the Puppet module there is a bug causing these options to not load correctly.

The problem is in Docker::Service where the service-overrides files are templated. These files notify docker-systemd-reload, but there is no dependency to insure that the reload occurs before the docker service is started. To be honest, there is not even a dependency insuring that the file is present before the service is started.

In my deployment this caused the service to be started using the default installation service file with none of my custom options. The file was templated and/or loaded after the service was started. This means a restart of the service would include my custom options, but unfortunately the specific options I was using (dm.thinpooldev) caused the restart to fail.

Because Docker::Service is using the same docker-systemd-reload exec that Docker::Run uses for individual containers, I cannot see a way to resolve this issue without changes to the module. I cannot impose a dependency in my code that uses docker-systemd-reload as it will affect all containers in addition to the service. So something like Exec['docker-systemd-reload'] -> Service['docker'] will create a dependency cycle.

The only immediate solution that came to mind is to have the service notify a differently named but otherwise identical execution resource (service-systemd-reload?) so that an appropriate dependency can be introduced for the service only.

While I am on CentOS, from looking at the code I believe the problem exists on Debian and ArchLinux as well.

garethr commented 8 years ago

@bcnewlin thanks for the detailed report. I think your suggested approach would work. If you have time to put up a PR that would be much appreciated. Thanks again.

bcnewlin commented 8 years ago

I can put up a PR after making and testing the changes in my deployment, as time allows. Although I am new to GitHub and Puppet so the testing and such will take some time. My priority of course is to get my deployment working.

garethr commented 8 years ago

@bcnewlin do the changes in https://github.com/garethr/garethr-docker/pull/398 look like the same issue? From the description I think so but wanted to check?

bcnewlin commented 8 years ago

@garethr Yes, that seems to be the same issue and they have pursued a similar fix.