-
Notifications
You must be signed in to change notification settings - Fork 385
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
feat: set full URI for the envoy-gateway service using name and namespace #4533
Conversation
…pace Signed-off-by: Rajat Vig <rvig@etsy.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4533 +/- ##
==========================================
- Coverage 65.72% 65.67% -0.05%
==========================================
Files 211 211
Lines 31669 31672 +3
==========================================
- Hits 20813 20801 -12
- Misses 9656 9668 +12
- Partials 1200 1203 +3 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Rajat Vig <rvig@etsy.com>
@@ -132,6 +134,7 @@ func expectedProxyContainers(infra *ir.ProxyInfra, | |||
TrustedCA: filepath.Join("/sds", common.SdsCAFilename), | |||
}, | |||
MaxHeapSizeBytes: maxHeapSizeBytes, | |||
XdsServerHost: ptr.To(fmt.Sprintf("envoy-gateway.%s.svc.%s", namespace, dnsDomain)), |
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.
can we use config.EnvoyGatewayServiceName
here ?
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.
done
can you share more details about the benifit? reduce DNS pressure? |
Signed-off-by: Rajat Vig <rvig@etsy.com>
Yes. It would reduce the number of DNS resolve requests as just The typical You can test it out by running and comparing against a CoreDNS pod with logging turned on to see the diference in number of queries.
Should add, this is not true when |
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.
LGTM thanks !
@rajatvig looks like this broken xds_cluster dns reslove on IPv6 cluster? |
…nd namespace (envoyproxy#4533)" This reverts commit 3e8730f.
…pace (envoyproxy#4533) * feat: set full URI for the envoy-gateway service using name and namespace Signed-off-by: Rajat Vig <rvig@etsy.com> * Use the correct namespace and dnsdomain from Gateway config Signed-off-by: Rajat Vig <rvig@etsy.com> * Use constant from config Signed-off-by: Rajat Vig <rvig@etsy.com> --------- Signed-off-by: Rajat Vig <rvig@etsy.com>
…pace (envoyproxy#4533) * feat: set full URI for the envoy-gateway service using name and namespace Signed-off-by: Rajat Vig <rvig@etsy.com> * Use the correct namespace and dnsdomain from Gateway config Signed-off-by: Rajat Vig <rvig@etsy.com> * Use constant from config Signed-off-by: Rajat Vig <rvig@etsy.com> --------- Signed-off-by: Rajat Vig <rvig@etsy.com>
…nd namespace (envoyproxy#4533)" This reverts commit 3e8730f. Signed-off-by: zirain <zirain2009@gmail.com>
…nd namespace (envoyproxy#4533)" This reverts commit 3e8730f.
…nd namespace (envoyproxy#4533)" This reverts commit 3e8730f. Signed-off-by: zirain <zirain2009@gmail.com>
Signed-off-by: Rajat Vig rvig@etsy.com
What type of PR is this?
Sets the
xdsServerHost
in Kubernetes to be fully qualified Service Name using the namespace.What this PR does / why we need it:
Instead of defaulting to
envoy-gateway
sets thexdsServerHost
to beenvoy-gateway.envoy-gateway-system.svc.cluster.local
.Release Notes: No