759974 - supervisord support for puppet

Status: RESOLVED FIXED

(Infrastructure & Operations :: RelOps: General, task, P3)

Description

[Chris AtLee [:catlee]]

Attachment: supervisord module

we need to run certain processes like Xvfb and metacity on the build slaves. We used to run them under cron, but that sucks. supervisord is a much nicer solution.

this module implements a supervisord module for puppet. you would instantiate it like this:

    supervisord::supervise {
        'xvfb':
            command => "/usr/bin/Xvfb +extension RANDR :2",
            user => cltbld,
            autostart => true,
            autorestart => true;
    }

Comment 1

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 628566 [details] [diff] [review]
supervisord module

Review of attachment 628566 [details] [diff] [review]:
-----------------------------------------------------------------

These are all pretty minor - one more r? round?

::: modules/supervisord/manifests/base.pp
@@ +17,5 @@
> +            purge => true;
> +
> +        "/etc/supervisord.conf.d/00header":
> +            notify => Exec["supervisord_make_config"],
> +            source => "puppet:///modules/supervisord/supervisord.conf.header";

please add owner => root, group => root here.  This prevents spurious changes when puppet runs from a user environment (when it sets the ownership based on the files in the environment).

@@ +41,5 @@
> +                Class["packages::supervisord"],
> +                File["/etc/supervisord.conf"],
> +            ],
> +            enable => true,
> +            ensure => running;

The sysadmins puppet module lists a number of commands  (restart, start, stop, status, etc.).  In particular, I suspect that the restart command (/usr/sbin/supervisorctl update) won't be automatically found by puppet, and puppet may resort to stop and start, which will kill all services.  Does supervisord 2.x not contain supervisorctl?

::: modules/supervisord/manifests/supervise.pp
@@ +4,5 @@
> +    file {
> +        "/etc/supervisord.conf.d/$name":
> +            content => template("supervisord/snippet.erb"),
> +            before => Exec["supervisord_make_config"],
> +            notify => Exec["supervisord_make_config"];

notify implies before, so before is redundant here

Comment 2

[Dustin J. Mitchell [:dustin] (he/him)]

This will need docs on https://wiki.mozilla.org/ReleaseEngineering/PuppetAgain/Modules, too.

Comment 3

[Chris AtLee [:catlee]]

Attachment: Updated supervisor module

Comment 4

[Chris AtLee [:catlee]]

(In reply to Dustin J. Mitchell [:dustin] from comment #1)
> ::: modules/supervisord/manifests/base.pp
> @@ +17,5 @@
> > +            purge => true;
> > +
> > +        "/etc/supervisord.conf.d/00header":
> > +            notify => Exec["supervisord_make_config"],
> > +            source => "puppet:///modules/supervisord/supervisord.conf.header";
> 
> please add owner => root, group => root here.  This prevents spurious
> changes when puppet runs from a user environment (when it sets the ownership
> based on the files in the environment).

I don't think this is required now that we have a site-wide default of root:root/0644.

> @@ +41,5 @@
> > +                Class["packages::supervisord"],
> > +                File["/etc/supervisord.conf"],
> > +            ],
> > +            enable => true,
> > +            ensure => running;
> 
> The sysadmins puppet module lists a number of commands  (restart, start,
> stop, status, etc.).  In particular, I suspect that the restart command
> (/usr/sbin/supervisorctl update) won't be automatically found by puppet, and
> puppet may resort to stop and start, which will kill all services.  Does
> supervisord 2.x not contain supervisorctl?

supervisord 2.x does have supervisorctl. I've set the restart command to be 'supervisorctl reload'. The init script's start, stop and status commands look ok.

> 
> ::: modules/supervisord/manifests/supervise.pp
> @@ +4,5 @@
> > +    file {
> > +        "/etc/supervisord.conf.d/$name":
> > +            content => template("supervisord/snippet.erb"),
> > +            before => Exec["supervisord_make_config"],
> > +            notify => Exec["supervisord_make_config"];
> 
> notify implies before, so before is redundant here

I've removed that.

Comment 5

[Kim Moir [:kmoir] ET]

Comment on attachment 640608 [details] [diff] [review]
Updated supervisor module

Looks good to me, I tested it on my test Linux slave and it worked fine. Very cool, I don't like running stuff from cron either :-)

Comment 6

[Chris AtLee [:catlee]]

docs up at

https://wiki.mozilla.org/ReleaseEngineering/PuppetAgain/Modules/supervisord