-
Notifications
You must be signed in to change notification settings - Fork 530
fix: rename listener using its protocol and port #6544
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
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6544 +/- ##
==========================================
+ Coverage 71.08% 71.15% +0.07%
==========================================
Files 225 225
Lines 39264 39348 +84
==========================================
+ Hits 27911 27999 +88
+ Misses 9738 9736 -2
+ Partials 1615 1613 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
5e3755b
to
b32b48f
Compare
8ccca85
to
a6d9776
Compare
Signed-off-by: Huabing (Robin) Zhao <[email protected]>
Signed-off-by: Huabing (Robin) Zhao <[email protected]>
Signed-off-by: Huabing (Robin) Zhao <[email protected]>
Signed-off-by: Huabing (Robin) Zhao <[email protected]>
Signed-off-by: Huabing (Robin) Zhao <[email protected]>
Signed-off-by: Huabing (Robin) Zhao <[email protected]>
Signed-off-by: Huabing (Robin) Zhao <[email protected]>
Signed-off-by: Huabing (Robin) Zhao <[email protected]>
Signed-off-by: Huabing (Robin) Zhao <[email protected]>
Signed-off-by: Huabing (Robin) Zhao <[email protected]>
Signed-off-by: Huabing (Robin) Zhao <[email protected]>
Signed-off-by: Huabing (Robin) Zhao <[email protected]>
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
Why do we need this PR?
Currently, the xDS listener is named after one of the owning Gateway Listeners. When multiple Gateway Listeners are configured listening on the same port, the generated xDS listener name is derived from a randomly selected Gateway Listener. This can cause unnecessary listener drains in Envoy if one of the Gateway Listeners is deleted or a new one is created using the same port.
To solve this issue, we need to rename the xDS listener to keep it consistent on the same port.
Some prior arts in this area
From what I'v seen in these projects, there are two approaches for the xDS listener naming scheme:
0.0.0.0_443
- we've discussed that we don't want the0.0.0.0
part as all the user listeners are listening on0.0.0.0
, and we'll need protocol prefix because EG also supports UDP.https-1~https-2
- we shouldn't consider this patter as this will also cause xDS listener to drain.This PR resolves the issue by naming the xDS listener based on its listening port and protocol, ensuring consistent naming even when multiple Gateways share the same port.
Proposed xDS listener scheme
Named after the listening port and protocol.
default/gateway-1/http
, after:tcp-80
default/gateway-1/https
, after:tcp-443
default/gateway-1/tls
, after:tcp-443
default/gateway-1/tcp
, after:tcp-81
default/gateway-1/udp
, after:udp-82
Named after the listening port and application protocol.
default/gateway-1/http
, after:http-80
default/gateway-1/https
, after:https-443
default/gateway-1/tls
, after:tls-443
default/gateway-1/tcp
, after:tcp-81
default/gateway-1/udp
, after:udp-82
For each option, the listener statPrefix will also be renamed using the same xDS listener name.
I'm leaning slightly toward option2 - unless we need to support combinations of http/https/tcp/tls on the same port, and we can fall back to
tcp-port
for that.Update: final decision is option 1.
The name of FilterChain name, RouteConfig, VirtualHost have, and listener statPrefix are also changed accordingly.move to a separate PR to limit the scope of this one.Notice: xDS listener is a de-fact API surface - it's used by
EnvoyPatchPolicy
andExtensionManager
to modify xDS resources, and users may rely on it for configuring metrics or monitoring. To avoid introducing breaking change in v1.5 ,this change is gated by theXDSNameSchemeV2
runtime flag. This flag is disabled by default in v1.5, and it will be enabled in v1.6.This PR also:
Adds Gateway Resource to the Listener metadata, so themove to a separate PR to limit the scope of this one.ExtensionManager
can use them as a replacement of the previous listener name if the Gateway and Listener resource names are required in its mutation processing.fix: #6534
release-note: yes