Skip to content
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

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

Merged
merged 6 commits into from
Nov 21, 2024

Conversation

zirain
Copy link
Member

@zirain zirain commented Nov 20, 2024

this's a huge changes seperated from #4690.
we still need a PR to fix the bootstrap, especially for admin endpoint.

@zirain zirain requested a review from a team as a code owner November 20, 2024 04:45
@@ -99,6 +100,10 @@ func (t *Translator) ProcessListeners(gateways []*GatewayContext, xdsIR resource
if !isReady {
continue
}

// EG always use `::` and set ipv4_compact with true to support both IPv4 and IPv6
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here is the key changes

Copy link
Member

@zhaohuabing zhaohuabing Nov 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also use :: and ipv4_compact for IPv4 only environment? What if IPv6 is not supported by the underlying OS?

Copy link
Member

@zhaohuabing zhaohuabing Nov 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zirain Could you please help me understand:

  • why prefer :: and ipv4_compact over addtional address?
  • would this approbach work with IPv4 only or underlying OS doesn't support IPv6?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for my poor understanding, this's just like net.Listen("tcp", ":8000") in golang, or maybe I'm wrong.

Copy link

codecov bot commented Nov 20, 2024

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 ⚠️
Additional details and impacted files
@@            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     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@zhaohuabing
Copy link
Member

zhaohuabing commented Nov 20, 2024

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

@arkodg
Copy link
Contributor

arkodg commented Nov 20, 2024

@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

Signed-off-by: zirain <zirain2009@gmail.com>
Signed-off-by: zirain <zirain2009@gmail.com>
Signed-off-by: zirain <zirain2009@gmail.com>
Signed-off-by: zirain <zirain2009@gmail.com>
@zirain zirain force-pushed the ipv6-dynamic-listener branch from d1f4f7d to ee4568a Compare November 20, 2024 18:54
Signed-off-by: zirain <zirain2009@gmail.com>
Copy link
Contributor

@arkodg arkodg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thanks !

@zhaohuabing
Copy link
Member

@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
Copy link
Member

zhaohuabing commented Nov 21, 2024

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
Copy link
Member Author

zirain commented Nov 21, 2024

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.

@zirain zirain merged commit 78da42c into envoyproxy:main Nov 21, 2024
24 checks passed
@zirain zirain deleted the ipv6-dynamic-listener branch November 21, 2024 09:08
zhaohuabing pushed a commit to zhaohuabing/gateway that referenced this pull request Nov 22, 2024
…xy#4743)

* enable IPv4Compact

Signed-off-by: zirain <zirain2009@gmail.com>

* fix xds test

Signed-off-by: zirain <zirain2009@gmail.com>

* release-notes

Signed-off-by: zirain <zirain2009@gmail.com>

* nit

Signed-off-by: zirain <zirain2009@gmail.com>

* gen

Signed-off-by: zirain <zirain2009@gmail.com>

---------

Signed-off-by: zirain <zirain2009@gmail.com>
(cherry picked from commit 78da42c)
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
zhaohuabing pushed a commit to zhaohuabing/gateway that referenced this pull request Nov 22, 2024
…xy#4743)

* enable IPv4Compact

Signed-off-by: zirain <zirain2009@gmail.com>

* fix xds test

Signed-off-by: zirain <zirain2009@gmail.com>

* release-notes

Signed-off-by: zirain <zirain2009@gmail.com>

* nit

Signed-off-by: zirain <zirain2009@gmail.com>

* gen

Signed-off-by: zirain <zirain2009@gmail.com>

---------

Signed-off-by: zirain <zirain2009@gmail.com>
(cherry picked from commit 78da42c)
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
zhaohuabing added a commit that referenced this pull request Nov 27, 2024
* fix: tcp listener is rejected when no route attached (#4681)

* fix: tcp listener is rejected when no route attached

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* change cluter name

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* fix listener connection limit test

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* fix listener connetcp keepalive  test

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* fix tcp endpoint stats test

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* fix tcp-route-enable-req-resp-sizes-stats

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* fix extensionpolicy-tcp-udp-http test

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* fix lint

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

---------

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
(cherry picked from commit f99c36c)
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* fix: remove backendrefs validation (#4705)

* remove backendrefs validation

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* add tests

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* add tests

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

---------

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
Co-authored-by: zirain <zirain2009@gmail.com>
(cherry picked from commit 5068698)
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* fix: translator reports errors for existing clusters and secretes (#4707)

* fix: existing clusters and secretes

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* fix cluster index for SP

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* minor change

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* minor change

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* minor change

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* minor change

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* fix lint

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* add comment

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* remove index

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* fix lint

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

---------

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

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

* enable IPv4Compact

Signed-off-by: zirain <zirain2009@gmail.com>

* fix xds test

Signed-off-by: zirain <zirain2009@gmail.com>

* release-notes

Signed-off-by: zirain <zirain2009@gmail.com>

* nit

Signed-off-by: zirain <zirain2009@gmail.com>

* gen

Signed-off-by: zirain <zirain2009@gmail.com>

---------

Signed-off-by: zirain <zirain2009@gmail.com>
(cherry picked from commit 78da42c)
Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* Fix: frequent 503 errors when connecting to a Service experiencing high Pod churn (#4754)

* Revert "fix: some status updates are discarded by the status updater (#4337)"

This reverts commit 14830c7.

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* store update events and process it later

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* rename method

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* add release note

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

---------

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>

* xds: use V4_PREFERRED dnsLookupFamily by default (#4745)

* use Cluster_V4_PREFERRED

Signed-off-by: zirain <zirain2009@gmail.com>

* release notes

Signed-off-by: zirain <zirain2009@gmail.com>

---------

Signed-off-by: zirain <zirain2009@gmail.com>

---------

Signed-off-by: Huabing Zhao <zhaohuabing@gmail.com>
Signed-off-by: zirain <zirain2009@gmail.com>
Co-authored-by: zirain <zirain2009@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants