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

Generate no cidfile by default, even when deploying as service. #172

Merged
merged 1 commit into from
Jun 26, 2014

Conversation

jperville
Copy link
Contributor

This PR remove the default value of /var/run/#{service_name}.cid when deploying container as a service. In other words, this means that containers will be deployed without a cidfile unless docker_container is configured with an explicit value for its cidfile attribute.

The main reason for that (slightly breaking) change is that :

  • cidfiles are created by default in /var/run which is flushed at every reboot (tmpfs)
  • docker start won't recreate cidfiles (only docker run --cidfile will)

My conclusion is that since the cidfiles we generate in /var/run do not survive reboots they cannot be relied upon and should not be generated by default.

My other reasons:

  • containers can run just fine without cidfiles
  • the service scripts do not use them (fgrep -r cid shows that no code depends on them in the cookbook)
  • cidfiles complicate redeploys (docker run won't recreate a named container when there is a stale cidfile, see Removing a container should also remove its cidfile. #164)

In the past I hacked around the cidfile issue by configuring my docker_container resources with cidfile '' but recent dockers (post 0.9) choke when docker run is invoked with --cidfile=''.

@djdefi
Copy link

djdefi commented Jun 24, 2014

I seem to be having lots of issues with cidfiles also. Even with the 0.35.0 update yesterday that addressed some of the cid issues I am still having to manually delete cidfiles

@fangpenlin
Copy link

+1 on disable cidfile by default, it's really annoying. I need to delete it every time before running a container, otherwise it will simply fail.

@bflad
Copy link
Contributor

bflad commented Jun 26, 2014

Personally I'm a fan of removing the cidfiles as well. They are more trouble than they are worth it seems. I'll merge this in and refactor the now extraneous cidfile method out.

bflad added a commit that referenced this pull request Jun 26, 2014
Generate no cidfile by default, even when deploying as service.
@bflad bflad merged commit cbe9b8d into sous-chefs:master Jun 26, 2014
@jperville jperville deleted the no-cidfile-by-default branch June 26, 2014 02:33
@jperville
Copy link
Contributor Author

Thank you @bflad for the quick merge! I didn't refactor out the cidfile method because I wanted my patch to be as simple and unobtrusive as possible. I'm very happy now :-)

@djdefi
Copy link

djdefi commented Jun 26, 2014

Awesome, redeploy works great now!

@bflad
Copy link
Contributor

bflad commented Jun 26, 2014

🎉 sweet!

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.

4 participants