Fix: frequent 503 errors when connecting to a Service experiencing high Pod churn by zhaohuabing · Pull Request #4754 · envoyproxy/gateway

zhaohuabing · 2024-11-21T13:48:27Z

Reverts #4337
Fixes: #4685

Release Note: Yes

To avoid regression of the bug fixed by #4337, a temporary buffer has been added to store events received before the Updater is enabled.
These events will be sent to the update channel once the Updater is enabled.

Tested the fix in my local env:

➜  k -n envoy-gateway-system logs envoy-gateway-7f7d68bd-q74m6 |grep "status update"                        
2024-11-26T05:56:03.461Z        INFO    provider        kubernetes/status_updater.go:185        received a status update while disabled, storing for later      {"runner": "provider", "event": {
"name":"ha-gateway","namespace":"gateway-upgrade-infra"}}
2024-11-26T05:56:03.764Z        INFO    provider        kubernetes/status_updater.go:185        received a status update while disabled, storing for later      {"runner": "provider", "event": {
"name":"upgrade"}}                                                                              
2024-11-26T05:56:03.767Z        INFO    provider        kubernetes/status_updater.go:185        received a status update while disabled, storing for later      {"runner": "provider", "event": {
"name":"http-backend-eg-upgrade","namespace":"gateway-upgrade-infra"}}
2024-11-26T05:56:03.767Z        INFO    provider        kubernetes/status_updater.go:185        received a status update while disabled, storing for later      {"runner": "provider", "event": {
"name":"ha-gateway","namespace":"gateway-upgrade-infra"}}
2024-11-26T05:56:28.146Z        INFO    provider        kubernetes/status_updater.go:139        started status update handler   {"runner": "provider"}
2024-11-26T05:56:28.146Z        INFO    provider        kubernetes/status_updater.go:197        sending stored status update    {"runner": "provider", "event": {"name":"ha-gateway","namespace":
"gateway-upgrade-infra"}}       
2024-11-26T05:56:28.146Z        INFO    provider        kubernetes/status_updater.go:197        sending stored status update    {"runner": "provider", "event": {"name":"upgrade"}}
2024-11-26T05:56:28.146Z        INFO    provider        kubernetes/status_updater.go:197        sending stored status update    {"runner": "provider", "event": {"name":"http-backend-eg-upgrade"
,"namespace":"gateway-upgrade-infra"}}  
2024-11-26T05:56:28.146Z        INFO    provider        kubernetes/status_updater.go:197        sending stored status update    {"runner": "provider", "event": {"name":"ha-gateway","namespace":
"gateway-upgrade-infra"}}

If anyone wants to verify this PR, you can do so by using this image zhaohuabing/gateway-dev:52e6d8577

codecov · 2024-11-21T13:55:52Z

Codecov Report

Attention: Patch coverage is 60.60606% with 13 lines in your changes missing coverage. Please review.

Project coverage is 65.58%. Comparing base (48a0310) to head (3b9ba84).
Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
internal/provider/kubernetes/status_updater.go	60.60%	12 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4754      +/-   ##
==========================================
- Coverage   65.60%   65.58%   -0.02%     
==========================================
  Files         211      211              
  Lines       31961    31989      +28     
==========================================
+ Hits        20968    20981      +13     
- Misses       9753     9766      +13     
- Partials     1240     1242       +2

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

internal/provider/kubernetes/status_updater.go

…4337)" This reverts commit 14830c7. Signed-off-by: Huabing Zhao <[email protected]>

Signed-off-by: Huabing Zhao <[email protected]>

internal/provider/kubernetes/status_updater.go

zirain · 2024-11-27T01:33:16Z

should we change the PR title?

arkodg · 2024-11-27T01:40:45Z

should we change the PR title?

yeah +1, because this PR does more than just revert

zirain · 2024-11-27T02:14:02Z

/retest

zirain · 2024-11-27T02:47:15Z

follow up: need a test to make sure refactor won't broken again.

…gh Pod churn (envoyproxy#4754) * Revert "fix: some status updates are discarded by the status updater (envoyproxy#4337)" This reverts commit 14830c7. Signed-off-by: Huabing Zhao <[email protected]> * store update events and process it later Signed-off-by: Huabing Zhao <[email protected]> * rename method Signed-off-by: Huabing Zhao <[email protected]> * add release note Signed-off-by: Huabing Zhao <[email protected]> --------- Signed-off-by: Huabing Zhao <[email protected]>

* fix: tcp listener is rejected when no route attached (#4681) * fix: tcp listener is rejected when no route attached Signed-off-by: Huabing Zhao <[email protected]> * change cluter name Signed-off-by: Huabing Zhao <[email protected]> * fix listener connection limit test Signed-off-by: Huabing Zhao <[email protected]> * fix listener connetcp keepalive test Signed-off-by: Huabing Zhao <[email protected]> * fix tcp endpoint stats test Signed-off-by: Huabing Zhao <[email protected]> * fix tcp-route-enable-req-resp-sizes-stats Signed-off-by: Huabing Zhao <[email protected]> * fix extensionpolicy-tcp-udp-http test Signed-off-by: Huabing Zhao <[email protected]> * fix lint Signed-off-by: Huabing Zhao <[email protected]> --------- Signed-off-by: Huabing Zhao <[email protected]> (cherry picked from commit f99c36c) Signed-off-by: Huabing Zhao <[email protected]> * fix: remove backendrefs validation (#4705) * remove backendrefs validation Signed-off-by: Huabing Zhao <[email protected]> * add tests Signed-off-by: Huabing Zhao <[email protected]> * add tests Signed-off-by: Huabing Zhao <[email protected]> --------- Signed-off-by: Huabing Zhao <[email protected]> Co-authored-by: zirain <[email protected]> (cherry picked from commit 5068698) Signed-off-by: Huabing Zhao <[email protected]> * fix: translator reports errors for existing clusters and secretes (#4707) * fix: existing clusters and secretes Signed-off-by: Huabing Zhao <[email protected]> * fix cluster index for SP Signed-off-by: Huabing Zhao <[email protected]> * minor change Signed-off-by: Huabing Zhao <[email protected]> * minor change Signed-off-by: Huabing Zhao <[email protected]> * minor change Signed-off-by: Huabing Zhao <[email protected]> * minor change Signed-off-by: Huabing Zhao <[email protected]> * fix lint Signed-off-by: Huabing Zhao <[email protected]> * add comment Signed-off-by: Huabing Zhao <[email protected]> * remove index Signed-off-by: Huabing Zhao <[email protected]> * fix lint Signed-off-by: Huabing Zhao <[email protected]> --------- Signed-off-by: Huabing Zhao <[email protected]> * xds: always use `::` and `IPv4Compact` for dynamic listener (#4743) * enable IPv4Compact Signed-off-by: zirain <[email protected]> * fix xds test Signed-off-by: zirain <[email protected]> * release-notes Signed-off-by: zirain <[email protected]> * nit Signed-off-by: zirain <[email protected]> * gen Signed-off-by: zirain <[email protected]> --------- Signed-off-by: zirain <[email protected]> (cherry picked from commit 78da42c) Signed-off-by: Huabing Zhao <[email protected]> * 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 <[email protected]> * store update events and process it later Signed-off-by: Huabing Zhao <[email protected]> * rename method Signed-off-by: Huabing Zhao <[email protected]> * add release note Signed-off-by: Huabing Zhao <[email protected]> --------- Signed-off-by: Huabing Zhao <[email protected]> * xds: use V4_PREFERRED dnsLookupFamily by default (#4745) * use Cluster_V4_PREFERRED Signed-off-by: zirain <[email protected]> * release notes Signed-off-by: zirain <[email protected]> --------- Signed-off-by: zirain <[email protected]> --------- Signed-off-by: Huabing Zhao <[email protected]> Signed-off-by: zirain <[email protected]> Co-authored-by: zirain <[email protected]>

…gh Pod churn (envoyproxy#4754) * Revert "fix: some status updates are discarded by the status updater (envoyproxy#4337)" This reverts commit 14830c7. Signed-off-by: Huabing Zhao <[email protected]> * store update events and process it later Signed-off-by: Huabing Zhao <[email protected]> * rename method Signed-off-by: Huabing Zhao <[email protected]> * add release note Signed-off-by: Huabing Zhao <[email protected]> --------- Signed-off-by: Huabing Zhao <[email protected]> (cherry picked from commit 8ec3095) Signed-off-by: Guy Daich <[email protected]>

* fix: BackendTlsPolicy specify multiple targetRefs of the same service, only one will work (#4630) * add tests Signed-off-by: Huabing Zhao <[email protected]> * fix matching comparison Signed-off-by: Huabing Zhao <[email protected]> * add release note Signed-off-by: Huabing Zhao <[email protected]> * fix lint Signed-off-by: Huabing Zhao <[email protected]> * fix lint Signed-off-by: Huabing Zhao <[email protected]> --------- Signed-off-by: Huabing Zhao <[email protected]> (cherry picked from commit 44c2f74) Signed-off-by: Guy Daich <[email protected]> * fix: tcp listener is rejected when no route attached (#4681) * fix: tcp listener is rejected when no route attached Signed-off-by: Huabing Zhao <[email protected]> * change cluter name Signed-off-by: Huabing Zhao <[email protected]> * fix listener connection limit test Signed-off-by: Huabing Zhao <[email protected]> * fix listener connetcp keepalive test Signed-off-by: Huabing Zhao <[email protected]> * fix tcp endpoint stats test Signed-off-by: Huabing Zhao <[email protected]> * fix tcp-route-enable-req-resp-sizes-stats Signed-off-by: Huabing Zhao <[email protected]> * fix extensionpolicy-tcp-udp-http test Signed-off-by: Huabing Zhao <[email protected]> * fix lint Signed-off-by: Huabing Zhao <[email protected]> --------- Signed-off-by: Huabing Zhao <[email protected]> (cherry picked from commit f99c36c) Signed-off-by: Guy Daich <[email protected]> * 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 <[email protected]> * store update events and process it later Signed-off-by: Huabing Zhao <[email protected]> * rename method Signed-off-by: Huabing Zhao <[email protected]> * add release note Signed-off-by: Huabing Zhao <[email protected]> --------- Signed-off-by: Huabing Zhao <[email protected]> (cherry picked from commit 8ec3095) Signed-off-by: Guy Daich <[email protected]> --------- Signed-off-by: Huabing Zhao <[email protected]> Signed-off-by: Guy Daich <[email protected]> Co-authored-by: Huabing Zhao <[email protected]>

…gh Pod churn (envoyproxy#4754) * Revert "fix: some status updates are discarded by the status updater (envoyproxy#4337)" This reverts commit 14830c7. Signed-off-by: Huabing Zhao <[email protected]> * store update events and process it later Signed-off-by: Huabing Zhao <[email protected]> * rename method Signed-off-by: Huabing Zhao <[email protected]> * add release note Signed-off-by: Huabing Zhao <[email protected]> --------- Signed-off-by: Huabing Zhao <[email protected]>

) * fix: tcp listener is rejected when no route attached (envoyproxy#4681) * fix: tcp listener is rejected when no route attached Signed-off-by: Huabing Zhao <[email protected]> * change cluter name Signed-off-by: Huabing Zhao <[email protected]> * fix listener connection limit test Signed-off-by: Huabing Zhao <[email protected]> * fix listener connetcp keepalive test Signed-off-by: Huabing Zhao <[email protected]> * fix tcp endpoint stats test Signed-off-by: Huabing Zhao <[email protected]> * fix tcp-route-enable-req-resp-sizes-stats Signed-off-by: Huabing Zhao <[email protected]> * fix extensionpolicy-tcp-udp-http test Signed-off-by: Huabing Zhao <[email protected]> * fix lint Signed-off-by: Huabing Zhao <[email protected]> --------- Signed-off-by: Huabing Zhao <[email protected]> (cherry picked from commit f99c36c) Signed-off-by: Huabing Zhao <[email protected]> * fix: remove backendrefs validation (envoyproxy#4705) * remove backendrefs validation Signed-off-by: Huabing Zhao <[email protected]> * add tests Signed-off-by: Huabing Zhao <[email protected]> * add tests Signed-off-by: Huabing Zhao <[email protected]> --------- Signed-off-by: Huabing Zhao <[email protected]> Co-authored-by: zirain <[email protected]> (cherry picked from commit 5068698) Signed-off-by: Huabing Zhao <[email protected]> * fix: translator reports errors for existing clusters and secretes (envoyproxy#4707) * fix: existing clusters and secretes Signed-off-by: Huabing Zhao <[email protected]> * fix cluster index for SP Signed-off-by: Huabing Zhao <[email protected]> * minor change Signed-off-by: Huabing Zhao <[email protected]> * minor change Signed-off-by: Huabing Zhao <[email protected]> * minor change Signed-off-by: Huabing Zhao <[email protected]> * minor change Signed-off-by: Huabing Zhao <[email protected]> * fix lint Signed-off-by: Huabing Zhao <[email protected]> * add comment Signed-off-by: Huabing Zhao <[email protected]> * remove index Signed-off-by: Huabing Zhao <[email protected]> * fix lint Signed-off-by: Huabing Zhao <[email protected]> --------- Signed-off-by: Huabing Zhao <[email protected]> * xds: always use `::` and `IPv4Compact` for dynamic listener (envoyproxy#4743) * enable IPv4Compact Signed-off-by: zirain <[email protected]> * fix xds test Signed-off-by: zirain <[email protected]> * release-notes Signed-off-by: zirain <[email protected]> * nit Signed-off-by: zirain <[email protected]> * gen Signed-off-by: zirain <[email protected]> --------- Signed-off-by: zirain <[email protected]> (cherry picked from commit 78da42c) Signed-off-by: Huabing Zhao <[email protected]> * Fix: frequent 503 errors when connecting to a Service experiencing high Pod churn (envoyproxy#4754) * Revert "fix: some status updates are discarded by the status updater (envoyproxy#4337)" This reverts commit 14830c7. Signed-off-by: Huabing Zhao <[email protected]> * store update events and process it later Signed-off-by: Huabing Zhao <[email protected]> * rename method Signed-off-by: Huabing Zhao <[email protected]> * add release note Signed-off-by: Huabing Zhao <[email protected]> --------- Signed-off-by: Huabing Zhao <[email protected]> * xds: use V4_PREFERRED dnsLookupFamily by default (envoyproxy#4745) * use Cluster_V4_PREFERRED Signed-off-by: zirain <[email protected]> * release notes Signed-off-by: zirain <[email protected]> --------- Signed-off-by: zirain <[email protected]> --------- Signed-off-by: Huabing Zhao <[email protected]> Signed-off-by: zirain <[email protected]> Co-authored-by: zirain <[email protected]>

zhaohuabing requested a review from a team as a code owner November 21, 2024 13:48

zhaohuabing marked this pull request as draft November 21, 2024 13:48

zhaohuabing mentioned this pull request Nov 21, 2024

503s and _No_route_to_host errors due to routing to non-existent Endpoints #4685

Closed

zhaohuabing commented Nov 22, 2024

View reviewed changes

internal/provider/kubernetes/status_updater.go Outdated Show resolved Hide resolved

zhaohuabing force-pushed the revert-4337-fix-missing-status-update branch from 045b2af to ec74784 Compare November 22, 2024 03:02

zhaohuabing marked this pull request as ready for review November 22, 2024 03:07

zhaohuabing force-pushed the revert-4337-fix-missing-status-update branch from 841520a to cf28b14 Compare November 26, 2024 05:51

zhaohuabing requested a review from arkodg November 26, 2024 06:00

zhaohuabing mentioned this pull request Nov 26, 2024

fix: decouple gateway status updates from the reconciler #4767

Merged

zhaohuabing added cherrypick/release-v1.1.4 cherrypick/release-v1.2.2 labels Nov 26, 2024

zhaohuabing force-pushed the revert-4337-fix-missing-status-update branch from e8c105f to 2c471bf Compare November 26, 2024 06:07

zhaohuabing added 2 commits November 26, 2024 06:07

Revert "fix: some status updates are discarded by the status updater (#…

c3a22e2

…4337)" This reverts commit 14830c7. Signed-off-by: Huabing Zhao <[email protected]>

store update events and process it later

bce5a67

Signed-off-by: Huabing Zhao <[email protected]>

zhaohuabing force-pushed the revert-4337-fix-missing-status-update branch 3 times, most recently from a2d7838 to 52e6d85 Compare November 26, 2024 06:22

rename method

07c9337

Signed-off-by: Huabing Zhao <[email protected]>

zhaohuabing force-pushed the revert-4337-fix-missing-status-update branch from 52e6d85 to 07c9337 Compare November 26, 2024 06:33

zhaohuabing added 2 commits November 26, 2024 08:19

add release note

708dfcc

Signed-off-by: Huabing Zhao <[email protected]>

Merge branch 'main' into revert-4337-fix-missing-status-update

3b9ba84

arkodg reviewed Nov 26, 2024

View reviewed changes

internal/provider/kubernetes/status_updater.go Show resolved Hide resolved

zhaohuabing requested a review from arkodg November 27, 2024 00:44

arkodg approved these changes Nov 27, 2024

View reviewed changes

arkodg requested review from a team November 27, 2024 01:25

zhaohuabing changed the title ~~Revert "fix: some status updates are discarded by the status updater"~~ Fix: frequent 503 errors when connecting to a Service experiencing high Pod churn Nov 27, 2024

zirain approved these changes Nov 27, 2024

View reviewed changes

zhaohuabing merged commit 8ec3095 into main Nov 27, 2024

zhaohuabing deleted the revert-4337-fix-missing-status-update branch November 27, 2024 02:54

zhaohuabing mentioned this pull request Nov 27, 2024

[release/v1.2] cherry pick bug fixes and IPv6 to v1.2.2 #4765

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

Fix: frequent 503 errors when connecting to a Service experiencing high Pod churn#4754
zhaohuabing merged 5 commits intomainfrom
revert-4337-fix-missing-status-update

zhaohuabing commented Nov 21, 2024 •

edited

Loading

Uh oh!

codecov bot commented Nov 21, 2024 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

zirain commented Nov 27, 2024

Uh oh!

arkodg commented Nov 27, 2024

Uh oh!

zirain commented Nov 27, 2024

Uh oh!

zirain commented Nov 27, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

zhaohuabing commented Nov 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Nov 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

zirain commented Nov 27, 2024

Uh oh!

arkodg commented Nov 27, 2024

Uh oh!

zirain commented Nov 27, 2024

Uh oh!

zirain commented Nov 27, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

zhaohuabing commented Nov 21, 2024 •

edited

Loading

codecov bot commented Nov 21, 2024 •

edited

Loading