-
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
doc: Added doc for running ingress/GwAPI in host network mode #31839
doc: Added doc for running ingress/GwAPI in host network mode #31839
Conversation
Documentation/network/servicemesh/gateway-api/host-network-mode.rst
Outdated
Show resolved
Hide resolved
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.
left a few suggestions but overall lgtm
6fb0a99
to
16e89ee
Compare
Documentation/network/servicemesh/gateway-api/host-network-mode.rst
Outdated
Show resolved
Hide resolved
Documentation/network/servicemesh/gateway-api/host-network-mode.rst
Outdated
Show resolved
Hide resolved
bced1f4
to
9abd528
Compare
Documentation/network/servicemesh/gateway-api/host-network-mode.rst
Outdated
Show resolved
Hide resolved
Documentation/network/servicemesh/gateway-api/host-network-mode.rst
Outdated
Show resolved
Hide resolved
Documentation/network/servicemesh/gateway-api/host-network-mode.rst
Outdated
Show resolved
Hide resolved
Documentation/network/servicemesh/gateway-api/host-network-mode.rst
Outdated
Show resolved
Hide resolved
ad4f667
to
ecf28b1
Compare
Documentation/network/servicemesh/gateway-api/host-network-mode.rst
Outdated
Show resolved
Hide resolved
Documentation/network/servicemesh/gateway-api/host-network-mode.rst
Outdated
Show resolved
Hide resolved
Documentation/network/servicemesh/gateway-api/host-network-mode.rst
Outdated
Show resolved
Hide resolved
Documentation/network/servicemesh/gateway-api/host-network-mode.rst
Outdated
Show resolved
Hide resolved
Documentation/network/servicemesh/gateway-api/host-network-mode.rst
Outdated
Show resolved
Hide resolved
Documentation/network/servicemesh/gateway-api/host-network-mode.rst
Outdated
Show resolved
Hide resolved
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.
Looks good to me - except one small change. But this is definitely ready for review (undraft) 🎉 Thanks @PhilipSchmid
side note: maybe squashing the commits into one (with an updated commit message)?
290c126
to
725871f
Compare
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 ✔️
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.
This is great, thank you! I left a couple of small nits for you on grammar, wording and structure. Let me know what you think!
There are some suggestions which are applicable to both the Ingress and Gateway API sections. For brevity, I just left them in the Gateway API section.
Documentation/network/servicemesh/gateway-api/host-network-mode.rst
Outdated
Show resolved
Hide resolved
Documentation/network/servicemesh/gateway-api/host-network-mode.rst
Outdated
Show resolved
Hide resolved
Documentation/network/servicemesh/gateway-api/host-network-mode.rst
Outdated
Show resolved
Hide resolved
Documentation/network/servicemesh/gateway-api/host-network-mode.rst
Outdated
Show resolved
Hide resolved
Documentation/network/servicemesh/gateway-api/host-network-mode.rst
Outdated
Show resolved
Hide resolved
Documentation/network/servicemesh/gateway-api/host-network-mode.rst
Outdated
Show resolved
Hide resolved
Documentation/network/servicemesh/gateway-api/host-network-mode.rst
Outdated
Show resolved
Hide resolved
Documentation/network/servicemesh/gateway-api/host-network-mode.rst
Outdated
Show resolved
Hide resolved
Documentation/network/servicemesh/gateway-api/host-network-mode.rst
Outdated
Show resolved
Hide resolved
Thanks, @learnitall, for your feedback! I addressed everything directly in c1b09de and will squash all commits as soon as we have all reviews in. |
Documentation/network/servicemesh/gateway-api/host-network-mode.rst
Outdated
Show resolved
Hide resolved
Documentation/network/servicemesh/gateway-api/host-network-mode.rst
Outdated
Show resolved
Hide resolved
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.
Great thank you! LGTM! I found one small grammar nit, but it's non-blocking.
* Added documentation about how to expose the Cilium Ingress Controller / GwAPI Envoy listener on the host network * Fixed faulty Helm flag in Helm values comment * Unified GwAPI `l7Proxy=true` requirement to work everywhere with Helm flags, not agent flags. * Fixed RST title level inconsistency in the troubleshooting guide Signed-off-by: Philip Schmid <philip.schmid@isovalent.com>
5d347df
to
9084828
Compare
Squashed and rebased in the hope we can soon merge it 🤞. |
/test |
Checks passing and relevant reviews are in -> marking as |
I see that #31646 was also backported to v1.15. Do we want some docs there as well? |
@julianwiedmann Thanks for mentioning this. But it's not necessary to backport this PR. The author backport of the partially related PR only backported the actual fix (merge of envoy listeners) - and not the part that was related to the feature that gets documented with this PR. See backport note:
|
This PR adds documentation for #30840 and #31646.
l7Proxy=true
requirement to work everywhere with Helm flags, not agent flags.ToDos:
NET_BIND_SERVICE
permissions to use privileged ports.Fix the following Sphinx warning:/src/Documentation/network/servicemesh/gateway-api/host-network-mode.rst:4: WARNING: duplicate label gs_gateway_host_network_mode, other instance in /src/Documentation/network/servicemesh/gateway-api/gateway-api.rst
The problem is that I'm defining.. _gs_gateway_host_network_mode:
withinhost-network-mode.rst
, which is also included ingateway-api.rst
. Hence, it's included twice as we definesource_suffix = ['.rst', '.md']
withincilium/Documentation/conf.py
. Possible workaround: Renamehost-network-mode.rst
tohost-network-mode.inc
. It's basically the same issue as described here: "WARNING: duplicate label" because of include sphinx-doc/sphinx#1668@docs: Any suggestion to get rid of this warning nicely?Fixed via 421ceaa. Thanks for the input, @qmonnet!Release note:
cc @mhofstetter, @networkop