Closed Bug 759974 Opened 12 years ago Closed 12 years ago

supervisord support for puppet

Categories

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

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: catlee, Assigned: catlee)

References

Details

(Whiteboard: [puppet])

Attachments

(1 file, 1 obsolete file)

Attached patch supervisord module (obsolete) — Splinter Review
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;
    }
Attachment #628566 - Flags: review?(dustin)
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
Attachment #628566 - Flags: review?(dustin) → review-
Priority: -- → P3
Whiteboard: [puppet]
Attachment #628566 - Attachment is obsolete: true
Attachment #640608 - Flags: review?(kmoir)
(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 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 :-)
Attachment #640608 - Flags: review?(kmoir) → review+
Attachment #640608 - Flags: checked-in+
docs up at

https://wiki.mozilla.org/ReleaseEngineering/PuppetAgain/Modules/supervisord
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Component: Server Operations: RelEng → RelOps
Product: mozilla.org → Infrastructure & Operations
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: