chef-cookbooks / runit

Development repository for the Chef Runit Cookbook
https://supermarket.chef.io/cookbooks/runit
Apache License 2.0
107 stars 197 forks source link

Allow specifying supervisor_user and supervisor_group #187

Closed dalehamel closed 8 years ago

dalehamel commented 8 years ago

@jtimberman @cwjohnston @slyness for review

According to the official runit docs the accepted way of allowing a non-root user to control a runit service is to:

I've added support for this via the 'supervisor_owner' and 'supervisor_group' attributes. I've also added unit tests for all cases I could think of to ensure it works as expected

A tangible use case (that affects me) is that we often want the user initiating a deploy (for example via capistrano) to be able to restart services. Rather than having to whitelist every possible service (we have lots), it makes sense to allow that user / group to just always have the privilege to manage the services.

dwradcliffe commented 8 years ago

LGTM

dalehamel commented 8 years ago

After testing in production, it turns out that the create action refuses to deal with pipes (https://github.com/chef/chef/blob/master/lib/chef/provider/file.rb#L246-L251), but 'touch' works fine for updating the owner/group.

I've revised the patch accordingly, and can confirm this is working in production.

cwjohnston commented 8 years ago

@dalehamel @dwradcliffe Thank you for this excellent pull request! I really appreciate the documentation and tests that you've thoughtfully added.

Unfortunately I'm having trouble running the test-kitchen service suite from this branch. Seems like we are missing run and log templates for the new services added in test/cookbooks/runit_test/recipes/service.rb:

[2016-04-27T18:43:30+00:00] ERROR: runit_service[supervisor_owner] (runit_test::service line 227) had an error: Chef::Exceptions::FileNotFound: template[/etc/sv/supervisor_owner/run] (/tmp/kitchen/cache/cookbooks/runit/libraries/provider_runit_service.rb line 82) had an error: Chef::Exceptions::FileNotFound: Cookbook 'runit_test' (1.0.1) does not contain a file at any of these locations:

Perhaps you have some uncommitted files locally? Otherwise feel free to copy from the existing run and log templates. The majority of the run templates in the runit_test cookbook are set up for executing socat to listen on a unique port.

dalehamel commented 8 years ago

Sorry I missed those, I'll look into this shortly

On Wednesday, 27 April 2016, Cameron Johnston notifications@github.com wrote:

@dalehamel https://github.com/dalehamel @dwradcliffe https://github.com/dwradcliffe Thank you for this excellent pull request! I really appreciate the documentation and tests that you've thoughtfully added.

Unfortunately I'm having trouble running the test-kitchen service suite from this branch. Seems like we are missing run and log templates for the new services added in test/cookbooks/runit_test/recipes/service.rb:

[2016-04-27T18:43:30+00:00] ERROR: runit_service[supervisor_owner](runit_test::service line 227) had an error: Chef::Exceptions::FileNotFound: template[/etc/sv/supervisor_owner/run](/tmp/kitchen/cache/cookbooks/runit/libraries/provider_runit_service.rb line 82) had an error: Chef::Exceptions::FileNotFound: Cookbook 'runit_test' (1.0.1) does not contain a file at any of these locations:

Perhaps you have some uncommitted files locally? Otherwise feel free to copy from the existing run and log templates. The majority of the run templates in the runit_test cookbook are set up for executing socat to listen on a unique port.

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/hw-cookbooks/runit/pull/187#issuecomment-215190252

dalehamel commented 8 years ago

@cwjohnston should be good to go now, I don't use test kitchen though so I can't confirm.

On a side note, any chance we can set up travis or something to catch this sort of stuff in CI?

cwjohnston commented 8 years ago

Thanks, I'll take another look at it.

On a side note, any chance we can set up travis or something to catch this sort of stuff in CI?

I am working on this presently. 👍

cwjohnston commented 8 years ago

@dalehamel As you've indicated that you don't run test-kitchen, will you please apply this patch and update the PR with the result? At that point I think all tests will pass and I can avoid creating an additional PR to include the fix.

Thanks!

dalehamel commented 8 years ago

@cwjohnston patch has been applied and pushed!

dalehamel commented 8 years ago

👏 @cwjohnston awesome, how does the release cycle work? When can I expect to be able to move to 1.7.8?

cwjohnston commented 8 years ago

@dalehamel 1.7.8 is now available via the supermarket. thanks!