fix: use rooted FQDN for xDS cluster address to avoid DNS search expansion#14291
Conversation
…nsion The auto-generated xDS service address is an in-cluster FQDN with 4 dots and no trailing dot. Under the default ndots:5 resolver config, Envoy's strict_dns cluster treats it as a relative name and expands every search domain on each re-resolution cycle, producing a burst of NXDOMAIN lookups (~96 wasted CoreDNS queries/min per gateway pod with the 5s service TTL). Append a trailing dot so the name is absolute. Only the kgateway-generated FQDN is normalized; an explicitly configured XdsServiceHost (which may be an IP or short name) is left untouched. Fixes kgateway-dev#14284 Signed-off-by: omar <[email protected]>
gateway-bot
added
do-not-merge/description-invalid
kind/fix
There was a problem hiding this comment.
Pull request overview
This PR reduces unnecessary DNS traffic from gateway pods by rendering the internally-generated xDS service hostname as a rooted (absolute) FQDN (trailing dot), preventing resolver search-domain expansion under default ndots:5 behavior.
Changes:
- Append a trailing dot to the default (auto-derived)
XdsServiceHostused for the xDS cluster address. - Update the e2e deployer assertion to expect a rooted xDS service FQDN.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/kgateway/controller/start.go | Root the auto-derived xDS Service FQDN by adding a trailing dot to avoid DNS search expansion. |
| test/e2e/features/deployer/suite.go | Adjust e2e assertion to match the rooted xDS cluster socket address. |
| xdsHost = kubeutils.ServiceFQDN(metav1.ObjectMeta{ | ||
| Name: globalSettings.XdsServiceName, | ||
| Namespace: namespaces.GetPodNamespace(), | ||
| }) | ||
| }) + "." |
| g.Expect(xdsSocketAddress.GetAddress()).To(gomega.Equal( | ||
| fmt.Sprintf("%s.%s.svc.cluster.local", wellknown.DefaultXdsService, testInstallation.Metadata.InstallNamespace), | ||
| ), "xds socket address points to kgateway service, in installation namespace") | ||
| fmt.Sprintf("%s.%s.svc.cluster.local.", wellknown.DefaultXdsService, testInstallation.Metadata.InstallNamespace), | ||
| ), "xds socket address points to kgateway service, in installation namespace, as a rooted FQDN") |
There was a problem hiding this comment.
I disagree with Copilot here. For one, this pattern already exists in the test. For another, our e2e environment is tightly enough controlled that we aren't really worried about environments with a non-default cluster domain. I think your existing code is fine.
Guard against a double trailing dot when ServiceFQDN already returns a rooted name (e.g. a CLUSTER_DOMAIN env var set with a trailing dot, which GetClusterDomainName returns verbatim). A double dot would itself break DNS resolution. Signed-off-by: omar <[email protected]>
| // NXDOMAIN lookups. TrimSuffix keeps this idempotent in case the FQDN is | ||
| // already rooted (e.g. a CLUSTER_DOMAIN env var set with a trailing dot), | ||
| // avoiding a double dot that would itself break resolution. | ||
| xdsHost = strings.TrimSuffix(kubeutils.ServiceFQDN(metav1.ObjectMeta{ |
There was a problem hiding this comment.
I agree with Copilot on this one. TrimRight is available and, if used, would make the code match what the comment states the code is doing.
JCigan
left a comment
JCigan
left a comment
There was a problem hiding this comment.
Fairly small thing, but I think you should make the change Copilot is suggesting in start.go and use strings.TrimRight instead of strings.TrimSuffix. Don't think you need to make the other change it's suggesting.
| // NXDOMAIN lookups. TrimSuffix keeps this idempotent in case the FQDN is | ||
| // already rooted (e.g. a CLUSTER_DOMAIN env var set with a trailing dot), | ||
| // avoiding a double dot that would itself break resolution. | ||
| xdsHost = strings.TrimSuffix(kubeutils.ServiceFQDN(metav1.ObjectMeta{ |
There was a problem hiding this comment.
I agree with Copilot on this one. TrimRight is available and, if used, would make the code match what the comment states the code is doing.
| g.Expect(xdsSocketAddress.GetAddress()).To(gomega.Equal( | ||
| fmt.Sprintf("%s.%s.svc.cluster.local", wellknown.DefaultXdsService, testInstallation.Metadata.InstallNamespace), | ||
| ), "xds socket address points to kgateway service, in installation namespace") | ||
| fmt.Sprintf("%s.%s.svc.cluster.local.", wellknown.DefaultXdsService, testInstallation.Metadata.InstallNamespace), | ||
| ), "xds socket address points to kgateway service, in installation namespace, as a rooted FQDN") |
There was a problem hiding this comment.
I disagree with Copilot here. For one, this pattern already exists in the test. For another, our e2e environment is tightly enough controlled that we aren't really worried about environments with a non-default cluster domain. I think your existing code is fine.
Description
The Envoy bootstrap rendered by kgateway sets the
xds_clustersocket address to the kgateway control-plane FQDN without a trailing dot, e.g.kgateway.kgateway-system.svc.cluster.local. Because this name has 4 dots and is not rooted, the defaultndots:5resolver config treats it as a relative name. Thexds_clusterusesenvoy.cluster.strict_dnswithrespect_dns_ttl: true, so on every re-resolution cycle (every 5s, matching the kgateway Service TTL) Envoy's c-ares resolver walks every search domain before the bare-name attempt — 8 NXDOMAIN lookups followed by the successful one. At a 5s TTL this is ~96 wasted CoreDNS queries per minute per gateway pod, scaling with pods and replicas.The address originates in
pkg/kgateway/controller/start.go, wherekubeutils.ServiceFQDN(...)returns the dotless FQDN. This change appends a trailing dot so the name is absolute (rooted), so the resolver skips search-domain expansion entirely (1–2 queries instead of 9–10).Only the kgateway-generated FQDN is normalized. An explicitly configured
XdsServiceHost(which may be an IP literal or a short name) is intentionally left untouched.The e2e deployer assertion (
xdsClusterAssertion) is updated to expect the rooted FQDN. The deployer golden tests setXdsHostexplicitly (mirroring the user-supplied branch), so they are unaffected.Fixes #14284
Change Type
/kind fix
Changelog