-
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
docs: egressgw: document incompatibility with Clustermesh #27918
Conversation
@YutaroHayakawa do you know if there is any way of determining whether a node is set up for Clustermesh? So that we could at least give users a warning similar to cilium/pkg/egressgateway/manager.go Lines 212 to 215 in 0da3f7e
|
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.
One nit, but otherwise LGTM!
Umm, I guess we can depend on
@giorio94 Do you have any other good ideas? |
The current way of subscribing to node events is not compatible with Clustermesh (see cilium#25794). Reflect this in the documentation. Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
3b7b552
to
35bd70d
Compare
Let's merge the docs change as-is then, and add the check once @giorio94 had a look. |
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, thank you!
@youngnick I think we don't need the full CI here, but feel free to disagree 👍 |
I agree, merging. |
Your proposal makes sense to me. The optional tag is not strictly required, as the ClusterMesh object is always provided, although it is nil if not initialized. |
The current way of subscribing to node events is not compatible with Clustermesh (see #25794). Reflect this in the documentation.