search

nickstenning / honcho

Honcho: a python clone of Foreman. For managing Procfile-based applications.
http://pypi.python.org/pypi/honcho
MIT License
1.6k stars 146 forks source link

Fix stop/ start issue #131 #144

Closed gamb closed 8 years ago

gamb commented 9 years ago

This approach stops child processing hanging with exported upstart process by leveraging setuid (http://upstart.ubuntu.com/cookbook/#setuid). service {app} restart now works correctly for me.

gamb commented 9 years ago

Oops meant to reference #131 , also I was on honcho 0.5.0. Accidentally replaced group_name.

nickstenning commented 9 years ago

Hey, thanks for this. I'm happy with moving the environment variable declarations into the upstart config, but the setuid directive was introduced in Upstart v1.4, meaning that this change will generate upstart configs that don't work on older distributions (in particular, Ubuntu Lucid).

I'm pretty sure you could fix the restart problem by just putting exec in front of {{ process.cmd }}. Do you fancy trying that?

nickstenning commented 9 years ago

Also, I note you've removed the shellquote filter for the environment variables, and changed {{ group_name }} to {{ app }}-{{ name }}. I don't think we want or need either of those changes.

gamb commented 9 years ago

Yep happy to give that a go later on, and tidy the commit up (shellquote etc). I think the fallback might be:

exec start-stop-daemon --start -c {user} --exec {command}

nickstenning commented 9 years ago

I have a preference for su over start-stop-daemon because the latter is (IIRC) Ubuntu-specific.

nickstenning commented 9 years ago

The chdir change is fine, I think.

gamb commented 9 years ago

See your point with start-stop-daemon, although doesn't upstart itself imply an ~Ubuntu-specific target?

nickstenning commented 9 years ago

That's a fair point, actually. Whatever is easiest, then. To be honest I think su(1) is still a better bet. You're not going to find a unix that doesn't have it, and we're not using any features of start-stop-daemon that su doesn't have.

msabramo commented 9 years ago

Does this still need an update?