Delay return of control during startup

[jhmartin]

Give consul time to install its HUP signal handler to avoid immediate shutdown when invoked via knife ssh, fixes #125.

[coveralls]

Coverage remained the same at 100.0% when pulling a11891d on jhmartin:125_Startup-delay into 57b4105 on johnbellone:master.

[tgwizard]

Should this really be part of the init script and not the chef setup process?

[jhmartin]

@tgwizard See hashicorp/consul#677 ; the issue occurs when called outside of chef. It just so happens that knife ssh ... /etc/init.d/consul start also triggers the behavior.

[tgwizard]

@jhmartin ah, ok. Still, this will affect starts and stops when not running through knife ssh etc., slowing everything down. Wouldn't

knife ssh ... "/etc/init.d/consul start && sleep 3"

work for you?

[jhmartin]

@tgwizard I suppose it would, but that means other users also have to know about this behavior and think about each invocation mechanism (Rundeck, mcollective, ssh-for-loop, fabric, ansible, etc) and determine if it requires a timing-safe wrapper. Startup scripts do not normally have that kind of environmental sensitivity, so having consul allow it violates Least Surprise.

[dpkp]

I think this needs to be in the init script. You can trigger this bug simply by writing a recipe that does notifies :reload, 'service[consul]' -- i.e., what the README does in sample code. If some other resource has notified a restart then you'll hit this at the end of the chef run when delayed notifications are processed (this happened to us).

Might I suggest checking consul info in a loop instead of sleeping 3 secs? For the upstart template you can do this via:

# Fix for upstream issue. see https://github.com/johnbellone/consul-cookbook/issues/125
post-start script
  echo "Sleep a bit to allow consul time to install signal handlers"
  while ! consul info; do sleep 1; done
end script

[do you mind adding this patch ^^^ for templates/default/consul.conf.erb ? Dont think it makes sense for me to do a separate PR for upstart here.]

[dpkp]

further testing and it seems like we can't rely on consul info as a signal that startup is done. sleep and pray may be the best answer here for now.

[johnbellone]

I am not a huge fan of this. Is it really what the Consul folks suggest?

[dpkp]

Is there an alternate fix for this? afaik, hashicorp currently recommends adding sleep to startup command: hashicorp/consul#677 (comment)

[tgwizard]

If we add it, perhaps it should be customizable? Not everyone will have the use-case of starting consul over ssh like this.

[johnbellone]

I am willing to put these changes in if someone wants to submit a PR. I'm going to have one more release prior to merging in #126.

[jhmartin]

I've updated this PR against current master and made the sleep configurable, but defaults to a safe value. The value can be set to 0 if the environment does not need the extra safety.

@dpkp I'm not familar enough w/upstart -- you can submit a PR against my branch though and I'll include it, or you can submit a separate PR.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.