-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
docs: support for peer label in DNS lookups #15374
Conversation
- Lookup a service, optionally in a cluster peer: | ||
|
||
```text | ||
[<tag>.]<service>.service[.<namespace>.ns][.<peer>.peer][.<partition>.ap].<domain> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't currently support this format with .service.p1.peer
in 1.14.0, so we'll need to remove anything referring to it. I believe we'll only be able to use the new .peer
syntax with virtual
entries for the time-being.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good - I'll move that stuff out into another PR ...
... once the accuracy of some of the other content is reviewed (so I can batch all my changes).
I'm particularly not sure about the changes I made to the ### Service Virtual IP Lookups
section.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to self: .peer
support varies by service lookup type and version.
.virtual
has supported as of 1.14.0.
.service
and .node
have support as of 1.14.2.
.ingress
and .connect
lack support.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some comments on what to keep as part of this and what's for the future!
Consul assigns a virtual IP to each service in the service mesh. | ||
A downstream service in the mesh must address an upstream service | ||
using its virtual IP in the following scenarios: | ||
- The upstream service is imported from a cluster peer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can still address an upstream service imported from a cluster peer using explicit upstreams. In that case you wouldn't need to use the virtual IP. So the only time we must address an upstream using its virtual ip is when the downstream has tproxy enabled.
"The peer name is an optional part of the FQDN, and it is used to query for the virtual IP
of a service imported from that peer." <- i think this sentence is still accurate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ndhanushkodi : So if using explicit upstreams, you can use either a .virtual
or a .service
lookup? I guess I'm still confused about when you'd need to use a .virtual
lookup versus just using a .service
lookup all the time (as of 1.14.2).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the only time we need .virtual
"when the downstream has tproxy enabled"? And in all other cases, use .service
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jkirschner-hashicorp If a downstream application is using an explicit upstream, it would be communicating to localhost:<port>
, and would not be using the IP address returned from a Consul DNS lookup.
Accessing a virtual address requires the use of tproxy. The VIPs are allocated from an IP range that is not likely to be present on a user's internal network, and thus are not routable by the upstream network. The application's sidecar needs to have tproxy enabled so that Envoy is correctly configured to map the destination [V]IP to the correct upstream service.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If a downstream application is using an explicit upstream, it would be communicating to localhost:, and would not be using the IP address returned from a Consul DNS lookup.
@blake : I don't think I had ever considered how namespace, partition, and peer are specified in an explicit upstream. Somehow I had thought that format would use a DNS-style lookup, but it doesn't. It uses proxy.upstreams[].destination_*
fields.
I think I understand now:
- For mesh:
- Implicit upstreams with transparent proxy must use the
.virtual
format (if using Consul DNS format rather than Kube DNS format) - DNS formats doesn't come into the picture at all for explicit upstreams
- Implicit upstreams with transparent proxy must use the
- For service discovery, use
.service
lookups.
Side note-to-self: We don't document destination_peer
(if there is such a field) in the upstream config reference: https://developer.hashicorp.com/consul/docs/connect/registration/service-registration#upstream-configuration-reference
It also seems like there's duplicate content between https://developer.hashicorp.com/consul/docs/connect/registration/sidecar-service and the link above that can lead to inconsistencies, like the omission of destination namespace and partition in one place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Though, use of this .virtual
format requires that dns.enableRedirection
is set to true
, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't document destination_peer (if there is such a field) in the upstream config reference: https://developer.hashicorp.com/consul/docs/connect/registration/service-registration#upstream-configuration-reference
This field is documented on that page. At the time of writing this comment, it is documented in the third row in the table of supported parameters under Upstream configuration reference.
It also seems like there's duplicate content between https://developer.hashicorp.com/consul/docs/connect/registration/sidecar-service…
This page shows the default config options used by Consul when a service registration has Connect enabled using the minimal "connect": { "sidecar_service": {} }
config stanza. It also shows an example of how these parameters can be overridden with minimal additional configuration.
The /docs/connect/registration/service-registration contains an exhaustive list of all parameters supported for a proxy service registration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This field is documented on that page. At the time of writing this comment, it is documented in the third row in the table of supported parameters under Upstream configuration reference.
Ugh, didn't see that. Thanks!
@@ -444,33 +473,46 @@ If you need more complex behavior, please use the | |||
|
|||
### Service Virtual IP Lookups | |||
|
|||
To find the unique virtual IP allocated for a service: | |||
To find the unique virtual IP allocated for a [Connect-capable](/docs/connect) service: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to self before resubmitting PR: add cross-references to when to use virtual IP lookups back in
@im2nguyen, @trujillo-adam: This is finally ready for education team review! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some comments and feedback. Let me know when you want me to have another look.
RFC 2782 style lookups have the same behavior as | ||
[standard service lookups](#standard-lookup), | ||
but differ in format. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RFC 2782 style lookups have the same behavior as | |
[standard service lookups](#standard-lookup), | |
but differ in format. | |
You can use RFC 2782-style lookups to query for services. |
We don't really describe the behavior of standard lookups--just gave some examples of how to format the lookups--so there isn't really anything to compare it to. Even if there were details about the behavior, it would be better to just state the behavior of this lookup format here.
|
||
The lookup uses the datacenter of the Consul agent acting as the DNS server. | ||
|
||
- Lookup a service in a cluster peer: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Lookup a service in a cluster peer: | |
- Add the `peer` segment to lookup a service in a cluster peer: |
Correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly. They need to specify two labels. If the peer name is "foo", they'd have to add foo.peer
.
At a higher level, I'm wondering about the benefit of changing this text from "context in which the format is useful" to "actions you need to take to use the format below". It feels strange to me to have one of the "valid formats for standard service lookups" be titled "lookup a local service" and the next be in a different form: "add the peer
segment to lookup a service in a cluster peer".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would still prefer that we describe what users are supposed to do in each of these rather than leaving it for them to figure it out based on the snippet we're providing. That is my reason for these suggestions -- we're giving the what without the how.
[<tag>.]<service>.service.<peer>.peer.<domain> | ||
``` | ||
|
||
- Lookup a service in a WAN federated datacenter: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Lookup a service in a WAN federated datacenter: | |
- Specify the name of a datacenter to lookup a service in a WAN federated datacenter: |
WAN federated datacenter, or cluster peer in service lookups. | ||
In each format, all fields in square brackets are optional. | ||
|
||
- Lookup a service, optionally in a cluster peer: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Lookup a service, optionally in a cluster peer: | |
- Lookup a service in a cluster peer: |
I was a little confused by the word "optionally" in this sentence. The syntax shows the peer
segment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything in square brackets is optional, as stated in Line 412:
In each format, all fields in square brackets are optional.
If you don't include .peer-name.peer
, then it does a local lookup. Partition and namespace are also optional.
Therefore, I'm inclined to decline this change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still don't think this line makes sense. The explanation actually confirms that we should change it because we already state in the introduction that the peer is optional.
This format supports `.service` lookup types in Consul 1.14.2 or later | ||
and supports `.virtual` lookup types in Consul 1.14.0 or later. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This format supports `.service` lookup types in Consul 1.14.2 or later | |
and supports `.virtual` lookup types in Consul 1.14.0 or later. | |
~> **Compatibility warning**: Consul 1.14.2+ is required for `.service` lookup types. |
We should only call out the minor version where the change was introduced.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We still need to mention somewhere which lookup types are supported by each format. This suggested change removes mention that .virtual
lookup types are supported.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll mention that .service and .virtual types are supported above the compatibility warning.
The `peer` is a cluster peer of the `partition`. | ||
If no `peer` is specified, the service lookup applies to the specified `partition` | ||
rather than one of its cluster peers. | ||
|
||
For example, assume that an agent in the `default` partition is acting as the | ||
DNS server, the `k8s-app-1` partition is in the same datacenter, and | ||
the `k8s-app-1` partition has a cluster peer named `billing`. | ||
To perform a lookup of non-mesh service `api` in namespace `foo` | ||
of that cluster peer, use the query | ||
`api.service.foo.ns.billing.peer.k8s-app-1.partition.consul`. | ||
|
||
- Lookup a service, optionally in a WAN federated datacenter: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The `peer` is a cluster peer of the `partition`. | |
If no `peer` is specified, the service lookup applies to the specified `partition` | |
rather than one of its cluster peers. | |
For example, assume that an agent in the `default` partition is acting as the | |
DNS server, the `k8s-app-1` partition is in the same datacenter, and | |
the `k8s-app-1` partition has a cluster peer named `billing`. | |
To perform a lookup of non-mesh service `api` in namespace `foo` | |
of that cluster peer, use the query | |
`api.service.foo.ns.billing.peer.k8s-app-1.partition.consul`. | |
- Lookup a service, optionally in a WAN federated datacenter: | |
Specify the name of a peered cluster in the `peer` segment. | |
Specify the name of the admin partition in the cluster in the `partition` segment. | |
If you do not include a `peer`, the service lookup queries the `partition` instead of one of the peered clusters. | |
In the following example, a Consul agent in the `default` partition acts as the DNS server, the `k8s-app-1` partition is in the same datacenter, and | |
the `k8s-app-1` partition has a cluster peer named `billing`. | |
To perform a lookup of non-mesh service `api` in namespace `foo` | |
of that cluster peer, use the query | |
`api.service.foo.ns.billing.peer.k8s-app-1.partition.consul`. | |
- Lookup a service in a WAN federated datacenter: |
Did I restate that correctly? The original text is a little unclear.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took your 2nd paragraph, but changed the first paragraph to the following:
If `.<peer>.peer` is included, the `<peer>` label identifies a cluster peer
of the query's partition in which to lookup the specified `<service>`.
If the query does not specify a partition,
the query uses the partition of the Consul agent that received the DNS query.
I agree that the original text wasn't clear. Let me know how much this change helps (or not)!
Cluster peering must be used to connect datacenters containing admin partitions, | ||
not WAN federation. Therefore, this format is not applicable to lookup a partition | ||
in a different datacenter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cluster peering must be used to connect datacenters containing admin partitions, | |
not WAN federation. Therefore, this format is not applicable to lookup a partition | |
in a different datacenter. | |
You must establish a peer connection between your Consul clusters to connect datacenters containing admin partitions. You cannot query an admin partition in another datacenter over a WAN federated network. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The limitation is not on connection / querying. WAN federation and admin partitions are mutually exclusive. If two datacenters are WAN federated, they can't have (non-default) admin partitions.
I'm not sure if that nuance is important here. I'll have to think about whether there's a clearer way than the original text...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New suggestion:
Cluster peering must be used to connect datacenters containing admin partitions, | |
not WAN federation. Therefore, this format is not applicable to lookup a partition | |
in a different datacenter. | |
This format does not support specifying both a partition and another datacenter. | |
Datacenters containing admin partitions can only be connected using cluster peering, | |
not WAN federation. To lookup a service in a partition of another datacenter, | |
you must establish a cluster peering relationship with that partition and | |
use the cluster peer lookup format. |
@trujillo-adam : I considered all of the feedback. In a few cases, I decided not to make the suggested changes (and left a comment with my thinking). Let me know your follow-up thoughts - it's ready for review again. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a few comments and suggestions
|
||
The lookup uses the datacenter of the Consul agent acting as the DNS server. | ||
|
||
- Lookup a service in a cluster peer: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would still prefer that we describe what users are supposed to do in each of these rather than leaving it for them to figure it out based on the snippet we're providing. That is my reason for these suggestions -- we're giving the what without the how.
The RFC 2782 lookup format supports the same suffixes as standard lookups | ||
for optionally specifying a cluster peer or WAN federated datacenter, | ||
but differs in how service name and optional tag are specified. | ||
The valid RFC 2782 lookup formats below assume a local service lookup: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The RFC 2782 lookup format supports the same suffixes as standard lookups | |
for optionally specifying a cluster peer or WAN federated datacenter, | |
but differs in how service name and optional tag are specified. | |
The valid RFC 2782 lookup formats below assume a local service lookup: | |
The RFC 2782 lookup format supports the same suffixes as standard lookups for specifying cluster peers or WAN federated datacenters, but it follows a different format for specifying service names and tags. | |
The following RFC 2782 lookups show how to format queries for a local service: |
Switch to active voice
I have the same comment as above w/r/t relying on users to recognize the differences by looking at the snippet rather than helping them with a description.
Per [RFC 2782](https://tools.ietf.org/html/rfc2782), SRV queries must | ||
prepend an underscore (`_`) to the RFC 2782 `service` and `protocol` values | ||
in a query to prevent DNS collisions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Per [RFC 2782](https://tools.ietf.org/html/rfc2782), SRV queries must | |
prepend an underscore (`_`) to the RFC 2782 `service` and `protocol` values | |
in a query to prevent DNS collisions. | |
Per [RFC 2782](https://tools.ietf.org/html/rfc2782), SRV queries must | |
prepend an underscore (`_`) to the RFC 2782 `service` and `protocol` values | |
in a query to prevent DNS collisions. |
Ah, this is the explanation. Consider moving this into the opening paragraph just before we introduce the example formats.
WAN federated datacenter, or cluster peer in service lookups. | ||
In each format, all fields in square brackets are optional. | ||
|
||
- Lookup a service, optionally in a cluster peer: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still don't think this line makes sense. The explanation actually confirms that we should change it because we already state in the introduction that the peer is optional.
If `.<peer>.peer` is included, the `<peer>` label identifies a cluster peer | ||
of the query's partition in which to lookup the specified `<service>`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If `.<peer>.peer` is included, the `<peer>` label identifies a cluster peer | |
of the query's partition in which to lookup the specified `<service>`. | |
A lookups that contains the `.<peer>.peer` label searches for `<service>` in a cluster connected to the query's partition with a [peer connection](/consul/docs/connect/cluster-peering) . |
Feel free to ignore this one. I was trying to use active voice and more precise language, but this might be more convoluted.
Datacenters containing admin partitions can only be connected using cluster peering, | ||
not WAN federation. To lookup a service in a partition of another datacenter, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Datacenters containing admin partitions can only be connected using cluster peering, | |
not WAN federation. To lookup a service in a partition of another datacenter, | |
You can only use peer connections to join datacenters that contain admin partitions, not WAN federation. To lookup a service in a partition of another datacenter, |
Active voice
To lookup a namespaced service in a cluster peer, | ||
the [canonical format](#canonical-format) must be used instead. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To lookup a namespaced service in a cluster peer, | |
the [canonical format](#canonical-format) must be used instead. | |
Use the [canonical format](#canonical-format) to lookup services bound to a namespace in a cluster peer. |
It's a bit of a gray area, but I think "namespace" is a noun. I recall seeing it in the K8s docs as verb, but it read as poor grammar to me. "Bound" might not be the right verb, either. I won't die on this hill if you disagree.
Use the following query formats to perform a service virtual IP lookup | ||
on namespaced or partitioned services: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use the following query formats to perform a service virtual IP lookup | |
on namespaced or partitioned services: | |
Use the following query formats to perform a service virtual IP lookup | |
on services within a namespace or partition: |
Same comment as above about parts of speech w/r/t namespace (and partition).
- Use the [canonical format](#canonical-format) to optionally specify | ||
namespace, peer, and/or partition: | ||
|
||
```text | ||
<service>.virtual[.<namespace>.ns][.<peer>.peer][.<partition>.ap].<domain> | ||
``` | ||
|
||
- Specify cluster peer and optional namespace in the lookup: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Use the [canonical format](#canonical-format) to optionally specify | |
namespace, peer, and/or partition: | |
```text | |
<service>.virtual[.<namespace>.ns][.<peer>.peer][.<partition>.ap].<domain> | |
``` | |
- Specify cluster peer and optional namespace in the lookup: | |
- Use the [canonical format](#canonical-format) to specify | |
namespace, peer, and/or partition: | |
```text | |
<service>.virtual[.<namespace>.ns][.<peer>.peer][.<partition>.ap].<domain> |
- Specify cluster peer and namespace in the lookup:
Do we need to keep prefacing as these parts as optional?
This pull request has been automatically flagged for inactivity because it has not been acted upon in the last 60 days. It will be closed if no new activity occurs in the next 30 days. Please feel free to re-open to resurrect the change if you feel this has happened by mistake. Thank you for your contributions. |
Closing due to inactivity. If you feel this was a mistake or you wish to re-open at any time in the future, please leave a comment and it will be re-surfaced for the maintainers to review. |
Ready for education team review at this preview link