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

Updated docker upstart script (should fix service docker restart). #173

Merged
merged 3 commits into from
Jun 26, 2014

Conversation

jperville
Copy link
Contributor

This PR updates the template for the upstart service in `/etc/init/docker.conf to match the upstream version.

The story: I was wondering why my docker containers poofed each time I ran service docker restart. Before this PR, typing sudo service docker restart would immediately restart the daemon but the containers are gone in docker ps. The reason is that upstart did not detect the end of docker daemon (tracking the pid of the shell wrapper) so it would not emit the "started" event and the container services would not be stopped cleanly nor restarted cleanly.

After this PR, running sudo service docker restart with containers are running should take a little longer, while docker shuts down the running containers gracefully.

@jperville
Copy link
Contributor Author

Fixed spec to make travis happy (sorry forgot about the spec after changing the template).

@jperville
Copy link
Contributor Author

Feel free to squash the 3 commits of this PR before merging.

@bplunkert
Copy link

Edit: In fact the error below is from running without including PR #173. When I do run from @jperville's code it seems to be fixed.

I just tested this via test-kitchen, including the default recipe with this wrapper recipe:

node.set['docker']['package']['repo_url'] = 'https://get.docker.io/ubuntu'
include_recipe 'docker'

Unfortunately it doesn't seem to resolve the issue for me, as the default recipe is still failing to converge on 14.04:

[2014-06-25T01:30:01+00:00] INFO: template[/etc/default/docker.io] sending stop action to service[docker.io] (immediate)
  * service[docker.io] action stop
[2014-06-25T01:30:01+00:00] INFO: Processing service[docker.io] action stop (docker::upstart line 26)
[2014-06-25T01:30:02+00:00] INFO: service[docker.io] stopped       
  - stop service service[docker.io]       
[2014-06-25T01:30:02+00:00] INFO: template[/etc/default/docker.io] sending start action to service[docker.io] (immediate)       
  * service[docker.io] action start
[2014-06-25T01:30:02+00:00] INFO: Processing service[docker.io] action start (docker::upstart line 26)       
[2014-06-25T01:30:03+00:00] INFO: service[docker.io] started
  - start service service[docker.io]
  * service[docker.io] action start
[2014-06-25T01:30:03+00:00] INFO: Processing service[docker.io] action start (docker::upstart line 26)
================================================================================
Error executing action `start` on resource 'service[docker.io]'       
================================================================================
Chef::Exceptions::Exec       
----------------------       
/sbin/start docker.io returned 1, expected 0       

@jperville
Copy link
Contributor Author

@bplunkert I am using the lxc-docker package, not the ubuntu docker.io package (try to set node['docker']['package']['name'] = 'lxc-docker' in your wrapper cookbook just after setting repo_url; see #171 for details).

@bplunkert
Copy link

@jperville Sorry. I think I may have been mixed up -- it looks like this PR fixes this issue for me, both with and without lxc-docker. I wasn't testing the right fork :)

So anyway it looks good to me! 👍

@bflad
Copy link
Contributor

bflad commented Jun 26, 2014

Thanks both of you for testing this out. Will merge and release in next bugfix version shortly.

bflad added a commit that referenced this pull request Jun 26, 2014
Updated docker upstart script (should fix service docker restart).
@bflad bflad merged commit 4a4ecf6 into sous-chefs:master Jun 26, 2014
@jperville jperville deleted the update-upstart-docker-conf branch June 26, 2014 02:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants