Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add only= tag #757

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add only= tag #757

wants to merge 1 commit into from

Conversation

RickyGrassmuck
Copy link

The new tag only= allows the user to have more granular control over which checked services
get added to the routing table.

Example:
Nomad publishes service checks for the HTTP, RPC, and Serf services to consul. Registering the tag below results in
a routing table that splits requests for nomad.service.example.com between the three different protocols.

Service Tag

    "urlprefix-nomad.service.example.com:443/ proto=https tlsskipverify=true"

Resulting Routing Table Entries

nomad        nomad.service.example.com:443/        https://172.16.1.8:4646        tlsskipverify=true        33%
nomad        nomad.service.example.com:443/        https://172.16.1.8:4647        tlsskipverify=true        33%
nomad        nomad.service.example.com:443/        https://172.16.1.8:4648        tlsskipverify=true        33%

Since we really only want to route the HTTP traffic, this commit allows you to use the rule below to only generate
a single routing table entry based on the port number associated with our HTTP port.

Service Tag

    "urlprefix-nomad-0.service.example.com:443/ proto=https only=4646 tlsskipverify=true"

Resulting Routing Table Entries

nomad        nomad.service.example.com:443/        https://172.16.1.8:4646        tlsskipverify=true        100.00%

@CLAassistant
Copy link

CLAassistant commented Mar 20, 2020

CLA assistant check
All committers have signed the CLA.

The new tag `only=` allows the user to have more granular control over which checked services
get added to the routing table.
@RickyGrassmuck
Copy link
Author

@nathanejohnson Sorry, I had signed the CLA but had accidentally messed up the email address in one of the commits so it wasn't registering. Went ahead and squashed everything down to 1 commit and re-signed the CLA so it should be good now.

@nathanejohnson
Copy link
Member

nathanejohnson commented Dec 3, 2020

@rigrassm tentatively this looks good, could you add a test case in routecmd_test.go to cover this option. Now the interesting part here is this means consul registry will have a route option that doesn't exist otherwise. On the other hand, filtering doesn't make much sense in other cases. I went looking for documentation specific to consul tags and it's not really there other than a brief mention in the Quickstart. I would have asked you to update that documentation, but since it doesn't exist I'll make it a point to create that page and roll it out with the next release.

TLDR; add a test case and I'll merge this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants