-
Notifications
You must be signed in to change notification settings - Fork 398
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
feature: add hostNetwork to the podSpec #1944
Conversation
Signed-off-by: tanujd11 <[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
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1944 +/- ##
==========================================
- Coverage 64.37% 64.33% -0.04%
==========================================
Files 112 112
Lines 15877 15879 +2
==========================================
- Hits 10221 10216 -5
- Misses 5007 5013 +6
- Partials 649 650 +1 ☔ View full report in Codecov by Sentry. |
Signed-off-by: tanujd11 <[email protected]>
fa31d25
to
93649d8
Compare
/retest |
Before adding this, I want to clearify the use case. The use of HostNetwork is discouraged. |
This pull request has been automatically marked as stale because it has not had activity in the last 30 days. Please feel free to give a status update now, ping for review, when it's ready. Thank you for your contributions! |
@zirain Although the use of hostnetwork is not encouraged, it is an exception in the scenario of gateways. Firstly, gateways need to expose ports such as 80 and 443, and hostnetwork is very convenient for this. Moreover, a gateway may be shared by many applications, so there will be no port conflicts. Therefore, even in a production environment, it is very convenient to use it in this way. Otherwise, it would be necessary to rely on external load balancing to expose ports 80 and 443, which would greatly increase the complexity for users, especially in situations where there is no external load balancing. |
🚀 Deployed on https://gateway-pr-1944-preview--eg-preview.netlify.app |
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.
Re-activate this issue. @fanux and his team come to me and provided me some contexts on this request, their team tries to put EG into production, but this is a blocker for them.
In their scenarios, I think this is reasonable and this would be also a common request when deploying single EG with mergeGateways enabled in a cluster, which means there will be only one single envoyproxy instance, and will be no listeners conflicts. AND in private cloud clusters, there offen will be no loadbalancer implementations, if the envoyproxy needs to expose http-80/https-443, this cannot be worked easily, which needs some changes to kubernetes.
So I am +1 for it.
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
Im still not convinced this is a good idea, the project supports multiple ways to setup a gateway in non LoadBalancer mode
Setting up |
What type of PR is this?
Feature
What this PR does / why we need it:
added hostNetwork to the podSpec
Which issue(s) this PR fixes:
Fixes #1928