807839 - Replace zeus trafficscript rule for splitting between reg/node

Status: VERIFIED FIXED

(Cloud Services Graveyard :: Server: Sync, defect)

Description

[Ryan Kelly [:rfkelly]]

Per email discussion, the implementation of the user API exposed on auth.services.mozilla.com is currently split between two services:

   Requests to /user/1.0/<username>/node/weave are directed to node*.reg.smc
   All other requests are directed to web*.reg.smc

The splitting is currently achieved by a Zeus TrafficScript rule.  We're planning to migrate away from Zeus, so we need a new way to handle this.

I see two options:

1) Continue to handle this at the auth.smc VIP level, by copying the trafficscript logic into whatever replaces Zeus.

2) Send all traffic through to web*.reg.smc initially, and have it explicitly proxy to node*.reg.smc when needed.

I have a slight preference for option (2) as I think it makes the code and the deployment a bit simpler to understand overall.  Option (1) has the advantage of less overhead since each node request has one less hop to travel through.

Ultimately this should be an Ops call, depending on which option makes the deployment simpler/cleaner/easier.  I believe atoll has expressed a preference for option (2) via email, but I wanted to move the final decision into a bug for posterity.

Comment 1

[:Atoll]

I prefer (2), since that maintains an existing method (web.reg forwards to various backend VIPs - in this case, web.sreg and node.reg), rather than anything the app developers can't see and can't test in (1).

Comment 2

[Ryan Kelly [:rfkelly]]

Thinking this through some more, I wonder if the node*.reg servers are actually useful at all.  They were originally intended as a separate external entry point for the client, but it doesn't sound like we will ever actually implement that in production.

So what if we have reg talk directly to both sreg and snode to fulfill its duties? 

In other words, rather than something like this:

    reg  -->  sreg
     |
     -------> node  -->  snode

Just cut out the middleman and have:

    reg  -->  sreg
     |
     -------> snode

Comment 3

[:Atoll]

I think the original theory for the node servers was that non-Sync products might want to request nodes, and two or three steps away from that on some future paths involves "node allocation SLA". In my opinion, reg + snode is just as easily scaled to provide "node allocation SLA" - if we ever desire to someday - rather than reg + node + snode, which is more moving pieces for no useful effect in any case.

Comment 4

[Ryan Kelly [:rfkelly]]

Attachment: server-reg patch to use an optional node-assignment backend

(In reply to Richard Soderberg [:atoll] from comment #3)
> I think the original theory for the node servers was that non-Sync products
> might want to request nodes

I think (hope?) that by the time we need this, it'll be handled by tokenserver and friends rather than the existing node-assignment servers.  So if we can simplify stuff in the meantime, I think that'll be a win.

I'll also note that taking node.reg out of the equation also removes the principal benefit for option (1) listed above - there is now no 'extra hop' cost to doing it inside server-reg rather than with a separate service.

The attached patch implements the basic idea, which is similar to the code currently in server-sreg - load an optional 'nodes' backend, delegate node-assignment calls to it, but continue to offer a "fallback_node" option for folks who want to skip all this hassle on smaller deployments.

Comment 5

[Toby Elliott [:telliott]]

Yeah, this was done in preparation for tokenserver, where the roles of account-creation and node assignment would be done by two separate services. 

That, plus the idea that only we needed this particular chunk of code, so splitting it out and keeping it away from users who wanted to set up a 1-node system made their lives a little easier.

Comment 6

[:Atoll]

What sections and/or section parameters from node "nodes.cfg" would need to be merged into reg "sync.cfg" as part of this patch?

Comment 7

[Ryan Kelly [:rfkelly]]

(In reply to Richard Soderberg [:atoll] from comment #6)
> What sections and/or section parameters from node "nodes.cfg" would need to
> be merged into reg "sync.cfg" as part of this patch?

Just the "nodes" section.  I will go through the dev and stage config files and make sure there's nothing lurking in any other sections that would be relevant here.

Comment 8

[Ryan Kelly [:rfkelly]]

Committed in http://hg.mozilla.org/services/server-reg/rev/84105730250c

I have looked through the config files and I believe that copying the [nodes] section from node 
"nodes.cfg" into reg "sync.conf" should be the only necessary configuration change.  The reg machines will also need to be able to send traffic to the snode machines.

I'll make a tag and deployment bug for trying this out on stage.

Comment 9

[Ryan Kelly [:rfkelly]]

Attachment: server-reg patch to handle AlreadyWrittenError from node backend

Actually, my initial patch is incomplete.  Node-assignment backends may, instead of just returning the assigned node, raise an AlreadyWrittenError with the node as payload.

That's a bit yuck to deal with without introducing a hard dependency on server-node-assignment, since you need to exception class in order to catch it properly.  In the attached patch I've chosen to stub it out if mozsvcnodes can't be imported.

(I'm not sure I see the point of AlreadyWrittenError, but I'd rather change things in one place rather than fiddling with both server-reg and server-node-assignment)

Comment 10

[Toby Elliott [:telliott]]

AlreadyWrittenError exists because the code deployed on node and snode are the same thing. Therefort, when snode does the write, the function that proxies needs to be able to return "good, but take no action" as a result.

Comment 11

[Ryan Kelly [:rfkelly]]

Ah, I see - so the controller doesn't try to write it into LDAP twice.  Thanks.

Comment 12

[Ryan Kelly [:rfkelly]]

Committed in http://hg.mozilla.org/services/server-reg/rev/9fb239c62e49

Comment 13

[Ryan Kelly [:rfkelly]]

Attachment: server-reg patch to use "sync" as product name, not "weave"

It turns out that the zeus script was also re-writing "/node/weave" to "/node/sync" when forwarding traffic to the node servers.  So we have to use "sync" as the product name when using the MozSvcNodes API directly, rather than "weave".  Patch attached.

Comment 14

[Toby Elliott [:telliott]]

Comment on attachment 681813 [details] [diff] [review]
server-reg patch to use "sync" as product name, not "weave"

It'd probably be a good idea to just search the entire codebase for weave references at this point.

Comment 15

[Ryan Kelly [:rfkelly]]

http://hg.mozilla.org/services/server-reg/rev/9fb7ac8d65a6

Comment 16

[James Bonacci [:jbonacci]]

yea, been sitting around awhile, huh?
:-)