envoyproxy / gateway

Manages Envoy Proxy as a Standalone or Kubernetes-based Application Gateway
https://gateway.envoyproxy.io
Apache License 2.0
1.64k stars 354 forks source link

xds: always use `::` and `IPv4Compact` for dynamic listener #4743

Closed zirain closed 4 days ago

zirain commented 5 days ago

this's a huge changes seperated from https://github.com/envoyproxy/gateway/pull/4690. we still need a PR to fix the bootstrap, especially for admin endpoint.

codecov[bot] commented 5 days ago

Codecov Report

Attention: Patch coverage is 71.42857% with 2 lines in your changes missing coverage. Please review.

Project coverage is 65.60%. Comparing base (2def6a4) to head (12ad43f). Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
internal/cmd/envoy/shutdown_manager.go 0.00% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #4743 +/- ## ========================================== - Coverage 65.62% 65.60% -0.03% ========================================== Files 211 211 Lines 31984 31961 -23 ========================================== - Hits 20990 20968 -22 + Misses 9755 9753 -2 - Partials 1239 1240 +1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.


🚨 Try these New Features:

zhaohuabing commented 5 days ago

@zirain let's hold this after v1.2.2 because it changes too many files and would make cherry-picking harder.

arkodg commented 5 days ago

@zirain let's hold this after v1.2.2 because it changes too many files and would make cherry-picking harder.

@zhaohuabing imo this should of v1.2 since its part of the dual stack feature

zhaohuabing commented 5 days ago

@zirain let's hold this after v1.2.2 because it changes too many files and would make cherry-picking harder.

@zhaohuabing imo this should of v1.2 since its part of the dual stack feature

OK, let's cherry-pick this to v1.2.2

zhaohuabing commented 4 days ago

LGTM thanks!

One non-blocking comment: Envoy now listens on all IP Families by default including IPv4 and IPv6, regardless of the IPFamily setting. It might be usefult to clarify this behavior in the IPFamily API docs to avoid potential confusion.

zirain commented 4 days ago

LGTM thanks!

One non-blocking comment: Envoy now listens on all IP Families by default including IPv4 and IPv6, regardless of the IPFamily setting. It might be usefult to clarify this behavior in the IPFamily API docs to avoid potential confusion.

Will update these once we make an agreement on admin endpoint.