-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
cilium-dbg: Expose Cilium network routing status #32036
Conversation
Expose the routing mode currently active in the cilium-dbg status CLI. Signed-off-by: Joe Stringer <joe@cilium.io>
/test |
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.
Thanks Joe! I'm surprised we don't have this already.
The changes to the API are breaking, but since this API resource is only consumed by utilities which are built alongside the API, we shouldn't have to worry about breaking compatibility. It's not expected for an external consumer to rely on the existing of the HostRouting
object, as external consumers of Cilium's API aren't expected in general, and we don't package different Cilium client and server versions into the same image.
A bit of bikeshedding, but ... IMO the BPF host routing is still an experimental feature (e.g., in the ENI mode it's discouraged). For me "legacy" reads as if something is bad / not wanted. Maybe we could improve the wording by just saying |
@brb I think that's fair criticism. Note we've been using this terminology for several releases now and I think it's actually been pretty effective at nudging users to try to upgrade their kernels and get BPF host routing enabled. That said I didn't have a strong opinion on this. @borkmann would you like to chime in on the above comment from Martynas? |
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.
Nice addition 👍 Agreed that off
sounds a bit nicer than legacy
if there are some cases where it will never be supported. If that wording has inspired FOMO and encouraged some folks to upgrade, I'm guessing it will also inspire the same for EKS users
if sr.Routing.InterHostRoutingMode == models.RoutingInterHostRoutingModeTunnel { | ||
status = status + " [" + sr.Routing.TunnelProtocol + "]" | ||
} | ||
status = status + "\tHost: " + sr.Routing.IntraHostRoutingMode | ||
|
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.
Thanks, that's been on my list for quite a while 🙂. Did you consider also exposing the tunnel port?
Expose the routing mode currently active in the cilium-dbg status CLI.
Example: