Closed Bug 643063 Opened 9 years ago Closed 9 years ago

tuck slavealloc deployment stuff into build/tools

Categories

(Release Engineering :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: dustin, Assigned: dustin)

References

Details

(Whiteboard: [slavealloc])

Attachments

(1 file, 2 obsolete files)

slavealloc will need an initscript and an nginx configuration.  Let's stick those in build/tools along with the rest of slavealloc for the moment.  They can move to puppet-manifests when puppet learns to set up a slavealloc host.
Assignee: nobody → dustin
Attached patch m643063-tools-r1.patch (obsolete) — Splinter Review
This is functional (inside the VPN): 
  http://slavealloc.build.mozilla.org/
but I'd love any feedback on the nginx config.
Attachment #521007 - Flags: review?(bear)
Comment on attachment 521007 [details] [diff] [review]
m643063-tools-r1.patch

I find that I often need to set X-Forwarded-For and Host for proxy_set_header to cover the bases of what downstream services need - but not a big concern, it may just make log processing simpler.

Is the rewrite rule in the / Location missing something? It strip any parameters form the url being rewritten?  I would think that 

rewrite ^/(.*) /ui/$1 last;

would be preferred.  The ^/ is there because I like to be really specific.  I use last instead of permanent because permanent will send an 304 roundtrip back to the client which isn't really needed if you are just masking what the real url is for a proxy from the consumers of the proxy

If not, then flip this to an r+
Attachment #521007 - Flags: review?(bear) → review-
(In reply to comment #2)
> Comment on attachment 521007 [details] [diff] [review]
> m643063-tools-r1.patch
> 
> I find that I often need to set X-Forwarded-For and Host for proxy_set_header
> to cover the bases of what downstream services need - but not a big concern, it
> may just make log processing simpler.

The twisted daemon doesn't pay attention to these (I think it can, but I've never tried to set that up), so no need there.  We can add those params later if they're required.

> Is the rewrite rule in the / Location missing something? It strip any
> parameters form the url being rewritten?  I would think that 
> 
> rewrite ^/(.*) /ui/$1 last;

It's
  location = / { .. }
so it *only* applies to the root URI, and is only in place so that web a browser will get to the right place if you just type 'slavealloc.build.mozilla.org'.  The correct URL frag for the UI is /ui/.  /css/slavealloc.css should be a 404.

Hopefully that's enough for r+?
Duplicate of this bug: 643813
Comment on attachment 521007 [details] [diff] [review]
m643063-tools-r1.patch

Oops, this nginx doesn't cover the /gettac URL!  I'll have another in a moment.
Attachment #521007 - Attachment is obsolete: true
Attached patch m643063-tools-r2.patch (obsolete) — Splinter Review
Attachment #521051 - Flags: review?(bear)
This adds a 'cd $BASEDIR', since the RotatingLogFile stuff in the .tac files doesn't respect --rundir.

Third time's the charm?
Attachment #521051 - Attachment is obsolete: true
Attachment #521051 - Flags: review?(bear)
Attachment #521063 - Flags: review?(bear)
Comment on attachment 521063 [details] [diff] [review]
m643063-tools-r3.patch

oh duh! completely zoned in on the rewrite and didn't pay attention to it's spot in the config file.

yes, that's what I get for trying to get a review done while heading out the door... :/
Attachment #521063 - Flags: review?(bear) → review+
Attachment #521063 - Flags: checked-in+
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.