Closed Bug 1607532 Opened 5 years ago Closed 5 years ago

Please update tokenserver config in production

Categories

(Cloud Services :: Operations: Miscellaneous, task, P1)

Product:
Cloud Services
Component:
Operations: Miscellaneous
Type:
task

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: rachel, Unassigned)

Details

Related to the change to allow tracking for node_type in Sync which went live with tokenserver 1.5.3, it looks like we also need to adjust the prod tokenserver config so the changes will work as intended:

We'll need the following added to the tokenserver prod config:

[tokenserver]
node_type_patterns =
  mysql: '^.+-\d\.sync\.services\.mozilla\.com$'
  spanner: '^.+-g\.sync\.services\.mozilla\.com$'
Priority: -- → P1

Related issue tracking client side changes is here: 1582317

:jrgm: - Do you think it'd be do-able to get this one applied in the next day or two? We're hoping to have this live ahead of the durable sync rollout (monday) so we can track sync usage more accurately.

Flags: needinfo?(jrgm)

So the commit for the node_type change is not in 1.5.3 (and yes, it's an overlapping set of branches in play at that time which makes the history hard to read). But it's in 1.5.4, which is on stage now. The above config could be added for that with 1.5.4 if testing of that build looks fine (i.e., beyond running the loadtest, if any is required).

mysql: '^.+-\d.sync.services.mozilla.com$'

Should that be \d+?

Flags: needinfo?(jrgm)

(In reply to John Morrison [:jrgm] from comment #3)

So the commit for the node_type change is not in 1.5.3 (and yes, it's an overlapping set of branches in play at that time which makes the history hard to read). But it's in 1.5.4, which is on stage now. The above config could be added for that with 1.5.4 if testing of that build looks fine (i.e., beyond running the loadtest, if any is required).

Oooh, good catch. The dates for the merges got me. We're good with rolling this into 1.5.4; thank you!

(In reply to John Morrison [:jrgm] from comment #4)

mysql: '^.+-\d.sync.services.mozilla.com$'

Should that be \d+?

Nope, they're good as-is. We're matching against: sync-<id-number>-<gcp-region>-g.sync.services.mozilla.com (spanner) and sync-<id-number>-<aws-region>.sync.service.mozilla.com (mysql)

Ah, so valid paths might look like

mysql: sync-123-us-east-1.sync.service.mozilla.com
spanner: sync-123-us-west1-a-g.sync.services.mozilla.com

? (based off of Google and AWS docs)

AWS currently doesn't have more than three zones per region so single digit would work. Honestly, this could probably work with a simpler regex check like if re.match("\d-g\.sync", uri) since AWS will probably never adopt "1-g.sync1" as a valid AWS region.

(In reply to JR Conlin [:jrconlin,:jconlin] from comment #7)

Ah, so valid paths might look like

mysql: sync-123-us-east-1.sync.service.mozilla.com
spanner: sync-123-us-west1-a-g.sync.services.mozilla.com

? (based off of Google and AWS docs)

AWS currently doesn't have more than three zones per region so single digit would work. Honestly, this could probably work with a simpler regex check like if re.match("\d-g\.sync", uri) since AWS will probably never adopt "1-g.sync1" as a valid AWS region.

A simpler regex check right now would involve code changes which I'd like to try and avoid if possible given our timeline.

:eolson:, can you confirm the requested config will match against our current sync hosts?

Flags: needinfo?(eolson)

The hostnames are chosen by us. We do not specify zones in the AWS based hostnames and I don't think we would do that in the GCP based hostnames either

Flags: needinfo?(eolson)

So, looking at the patch from rfk, these are fnmatch style patterns, not regexes. (They get compiled into regexes[1], but the pattern can only be a simple match[2] like mysql:*sync*.services.mozilla.com. So what's an appropriate glob pattern to distinguish between the two?

[1] https://github.com/mozilla-services/tokenserver/pull/149/files#diff-9f91fcbd4f5f6bb2523792172be8014aR162
[2] https://github.com/mozilla-services/tokenserver/pull/149/files#diff-1dabea4ca2355fba94549b6c80fc560eR63-R64

Or did I read that wrong?

This may need to rely on precendence - https://github.com/mozilla-services/tokenserver/pull/149/files#diff-1dabea4ca2355fba94549b6c80fc560eR74-R82

i.e., 'spanner:-g.sync.services.mozilla.com' if hostname ends in -g, else 'mysql:.sync.services.mozilla.com'

Bugzilla helpfully removed my * globs from the above. So, take two:
i.e., 'spanner:*-g.sync.services.mozilla.com' if hostname ends in -g, else 'mysql:*.sync.services.mozilla.com'

Yup, thanks :jrgm: - And after trying to compile locally (commenting out the tokenserver include in syncserver and replacing with a locally zipped tokenserver), we may not want the quotes either. So, something like this:

[tokenserver]
node_type_patterns =
    spanner:*g.sync.services.mozilla.com
    mysql:*

I think it might make sense to just keep the default case simple in order to make sure we catch everything else just in case. What do you think?

:pjenvey: - can you confirm config values here?

Flags: needinfo?(pjenvey)

r+

Flags: needinfo?(pjenvey)

Damn - this isn't being reported by the client :( I neglected to add node_type to https://searchfox.org/mozilla-central/source/services/common/tokenserverclient.js#360

(actually, in lines 372-378)

You need to log in before you can comment on or make changes to this bug.