fix: retry reconcile on transient errors during reconcile by patrostkowski · Pull Request #6299 · envoyproxy/gateway

patrostkowski · 2025-06-12T18:06:07Z

What type of PR is this?

fix: #6284

What this PR does / why we need it:

This PR:

Introduces isTransientError to detect transient Kubernetes errors and enable proper reconciliation retries.
Ignores non-transient errors from the provider layer and publishes them to the translation layer - the errors mostly come from missing referenced resources or misconfigurations that can be caught later in the Gateway API translator and surfaced in the resource status.

Description updated by @zhaohuabing on Jul 3 2025.

Which issue(s) this PR fixes:

Fixes #6284

Release Notes: Yes

test/helm/gateway-addons-helm/e2e.out.yaml

internal/provider/kubernetes/controller.go

codecov · 2025-06-13T09:39:14Z

Codecov Report

Attention: Patch coverage is 27.44186% with 156 lines in your changes missing coverage. Please review.

Project coverage is 70.73%. Comparing base (a394c61) to head (80b953e).
Report is 7 commits behind head on main.

Files with missing lines	Patch %	Lines
internal/provider/kubernetes/controller.go	27.44%	150 Missing and 6 partials ⚠️

❌ Your patch status has failed because the patch coverage (27.44%) is below the target coverage (60.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6299      +/-   ##
==========================================
- Coverage   70.92%   70.73%   -0.19%     
==========================================
  Files         220      220              
  Lines       37260    37354      +94     
==========================================
- Hits        26426    26423       -3     
- Misses       9289     9383      +94     
- Partials     1545     1548       +3

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

guydc · 2025-06-13T14:25:12Z

@arkodg, @patrostkowski
My question here would be: In case of a non-transient error, we will still store inconsistent resources (partial? invalid?) that may lead to corruption of XDS. Is there a way to avoid this, e.g. by using previously-stored resources for the affected GWC if they exist? This would be more aligned and compatible with previous behavior, where ALL XDS is essentially not updated in case of ANY error in this area of provider?

guydc · 2025-06-18T16:09:10Z

Notes from the community meeting:

Provider layer should publish an accurate state-of-the-world.
If transient errors make it impossible to build an accurate snapshot, the reconciliation should be retried by returning early with an error
If validation errors are encountered, state should be published, so that status can be updated and user notified about issues with resources.
If users want to implement alternative strategies for handling invalid configuration (e.g. pausing XDS updates), that should be implemented in other layers.

arkodg · 2025-06-19T01:21:00Z

can we also log the transient error

arkodg · 2025-06-24T21:46:52Z

@patrostkowski still working on this ? we'd like to get this into the patch release scheduled to be out next week

zhaohuabing · 2025-06-26T02:41:57Z

It seems @patrostkowski is currently unavailable. I’ll take over from here since we need this in the upcoming patch.

patrostkowski · 2025-06-26T09:22:44Z

thanks @zhaohuabing and sorry, I am very busy time atm with other things 😞

zhaohuabing · 2025-06-26T09:28:41Z

thanks @zhaohuabing and sorry, I am very busy time atm with other things 😞

No worries. I'll take care of it.

internal/provider/kubernetes/controller.go

arkodg · 2025-07-02T23:40:46Z

internal/provider/kubernetes/controller.go

 			service := new(corev1.Service)
 			err := r.client.Get(ctx, types.NamespacedName{Namespace: string(*backendRef.Namespace), Name: string(backendRef.Name)}, service)
 			if err != nil {
+				if isTransientError(err) {


is going to hard to remember to add this for every client call

Yeah, we can figure out a better approach to handle this later.

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

internal/provider/kubernetes/controller.go

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

arkodg

LGTM thanks

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

…#6299) * fix: add isTransientError helper to classify retryable errors Introduces isTransientError to detect transient Kubernetes errors and enable proper reconciliation retries. Signed-off-by: Patryk Rostkowski <[email protected]> handle errors from processing BackendRefs Signed-off-by: Huabing (Robin) Zhao <[email protected]> handle errors from processing ConfigMap Signed-off-by: Huabing (Robin) Zhao <[email protected]> * skip invalid GatewayClass Signed-off-by: Huabing (Robin) Zhao <[email protected]> * address comment Signed-off-by: Huabing (Robin) Zhao <[email protected]> * handle all transient errors Signed-off-by: Huabing (Robin) Zhao <[email protected]> * don't skip failed GCs Signed-off-by: Huabing (Robin) Zhao <[email protected]> --------- Signed-off-by: Patryk Rostkowski <[email protected]> Signed-off-by: Huabing (Robin) Zhao <[email protected]> Co-authored-by: Huabing (Robin) Zhao <[email protected]> (cherry picked from commit 71ce56f) Signed-off-by: Huabing (Robin) Zhao <[email protected]>

* fix(translator): ext-proc full duplex streamed trailers and validation (#6323) * fix ext proc validation and trailer management for full duplex streamed mode Signed-off-by: Guy Daich <[email protected]> Signed-off-by: shawnh2 <[email protected]> * feat: disable automountServiceAccountToken for proxy and ratelimit (#6364) Signed-off-by: Jeff Davis <[email protected]> * bugfix: make EnvoyPatchPolicy able to replace telemetry cluster (#6367) Signed-off-by: zirain <[email protected]> Signed-off-by: shawnh2 <[email protected]> * feat: add validation of section name for Gateway listener (#6343) * add validation of section name Signed-off-by: kkk777-7 <[email protected]> * update error status reason Signed-off-by: kkk777-7 <[email protected]> * refactor: define as function of validate section name for gateway listener Signed-off-by: kkk777-7 <[email protected]> Signed-off-by: shawnh2 <[email protected]> * fix: add configMap indexers for EEP reconciler (#6369) Signed-off-by: Rudrakh Panigrahi <[email protected]> * fix: use buildEndpointType for access and tracing (#6370) Signed-off-by: zirain <[email protected]> * fix: default accesslog not working (#6441) * fix default accesslog Signed-off-by: zirain <[email protected]> * release notes Signed-off-by: zirain <[email protected]> --------- Signed-off-by: zirain <[email protected]> Signed-off-by: shawnh2 <[email protected]> * chore: fix cve (#6446) * fix cve Signed-off-by: zirain <[email protected]> * lint Signed-off-by: zirain <[email protected]> --------- Signed-off-by: zirain <[email protected]> * fix: Do not set backendRequestTimeout when Retries are set (#6421) * fix: Do not set backendRequestTimeout when Retries are set Signed-off-by: sudipto baral <[email protected]> * fix: update comment Signed-off-by: sudipto baral <[email protected]> --------- Signed-off-by: sudipto baral <[email protected]> * gatewayapi: don't append gwcResource if there's invalid GatewayClass (#6379) * gatewayapi: don't process gloabal resources when acceptedGateways is 0 Signed-off-by: zirain <[email protected]> * update Signed-off-by: zirain <[email protected]> * fix test Signed-off-by: zirain <[email protected]> * don't skip gateways Signed-off-by: zirain <[email protected]> --------- Signed-off-by: zirain <[email protected]> Signed-off-by: shawnh2 <[email protected]> * fix testdata Signed-off-by: shawnh2 <[email protected]> * fix k8s provider controller Signed-off-by: shawnh2 <[email protected]> * fix: retry reconcile on transient errors during reconcile (#6299) * fix: add isTransientError helper to classify retryable errors Introduces isTransientError to detect transient Kubernetes errors and enable proper reconciliation retries. Signed-off-by: Patryk Rostkowski <[email protected]> handle errors from processing BackendRefs Signed-off-by: Huabing (Robin) Zhao <[email protected]> handle errors from processing ConfigMap Signed-off-by: Huabing (Robin) Zhao <[email protected]> * skip invalid GatewayClass Signed-off-by: Huabing (Robin) Zhao <[email protected]> * address comment Signed-off-by: Huabing (Robin) Zhao <[email protected]> * handle all transient errors Signed-off-by: Huabing (Robin) Zhao <[email protected]> * don't skip failed GCs Signed-off-by: Huabing (Robin) Zhao <[email protected]> --------- Signed-off-by: Patryk Rostkowski <[email protected]> Signed-off-by: Huabing (Robin) Zhao <[email protected]> Co-authored-by: Huabing (Robin) Zhao <[email protected]> (cherry picked from commit 71ce56f) Signed-off-by: Huabing (Robin) Zhao <[email protected]> * fix: fix bug in hostname overlap detection (#6332) fix bug in hostname overlap detection Signed-off-by: Rudrakh Panigrahi <[email protected]> (cherry picked from commit e78e268) Signed-off-by: Huabing (Robin) Zhao <[email protected]> * fix telemetry with host port not working (#6460) Signed-off-by: zirain <[email protected]> (cherry picked from commit c0a2ce7) Signed-off-by: Huabing (Robin) Zhao <[email protected]> * bugfix: BackendTlsPolicy should not reference across namespace (#6309) * bugfix: BackendTlsPolicy should not reference across namespace Signed-off-by: zirain <[email protected]> (cherry picked from commit 9925189) Signed-off-by: Huabing (Robin) Zhao <[email protected]> --------- Signed-off-by: Guy Daich <[email protected]> Signed-off-by: shawnh2 <[email protected]> Signed-off-by: Jeff Davis <[email protected]> Signed-off-by: zirain <[email protected]> Signed-off-by: kkk777-7 <[email protected]> Signed-off-by: Rudrakh Panigrahi <[email protected]> Signed-off-by: sudipto baral <[email protected]> Signed-off-by: Patryk Rostkowski <[email protected]> Signed-off-by: Huabing (Robin) Zhao <[email protected]> Co-authored-by: Guy Daich <[email protected]> Co-authored-by: Jeff Davis <[email protected]> Co-authored-by: zirain <[email protected]> Co-authored-by: Kota Kimura <[email protected]> Co-authored-by: Rudrakh Panigrahi <[email protected]> Co-authored-by: Sudipto Baral <[email protected]> Co-authored-by: Patryk Rostkowski <[email protected]> Co-authored-by: Huabing (Robin) Zhao <[email protected]>

…#6299) * fix: add isTransientError helper to classify retryable errors Introduces isTransientError to detect transient Kubernetes errors and enable proper reconciliation retries. Signed-off-by: Patryk Rostkowski <[email protected]> handle errors from processing BackendRefs Signed-off-by: Huabing (Robin) Zhao <[email protected]> handle errors from processing ConfigMap Signed-off-by: Huabing (Robin) Zhao <[email protected]> * skip invalid GatewayClass Signed-off-by: Huabing (Robin) Zhao <[email protected]> * address comment Signed-off-by: Huabing (Robin) Zhao <[email protected]> * handle all transient errors Signed-off-by: Huabing (Robin) Zhao <[email protected]> * don't skip failed GCs Signed-off-by: Huabing (Robin) Zhao <[email protected]> --------- Signed-off-by: Patryk Rostkowski <[email protected]> Signed-off-by: Huabing (Robin) Zhao <[email protected]> Co-authored-by: Huabing (Robin) Zhao <[email protected]> Signed-off-by: Tjeerd Jan van der Molen <[email protected]>

* fix(translator): ext-proc full duplex streamed trailers and validation (envoyproxy#6323) * fix ext proc validation and trailer management for full duplex streamed mode Signed-off-by: Guy Daich <[email protected]> Signed-off-by: shawnh2 <[email protected]> * feat: disable automountServiceAccountToken for proxy and ratelimit (envoyproxy#6364) Signed-off-by: Jeff Davis <[email protected]> * bugfix: make EnvoyPatchPolicy able to replace telemetry cluster (envoyproxy#6367) Signed-off-by: zirain <[email protected]> Signed-off-by: shawnh2 <[email protected]> * feat: add validation of section name for Gateway listener (envoyproxy#6343) * add validation of section name Signed-off-by: kkk777-7 <[email protected]> * update error status reason Signed-off-by: kkk777-7 <[email protected]> * refactor: define as function of validate section name for gateway listener Signed-off-by: kkk777-7 <[email protected]> Signed-off-by: shawnh2 <[email protected]> * fix: add configMap indexers for EEP reconciler (envoyproxy#6369) Signed-off-by: Rudrakh Panigrahi <[email protected]> * fix: use buildEndpointType for access and tracing (envoyproxy#6370) Signed-off-by: zirain <[email protected]> * fix: default accesslog not working (envoyproxy#6441) * fix default accesslog Signed-off-by: zirain <[email protected]> * release notes Signed-off-by: zirain <[email protected]> --------- Signed-off-by: zirain <[email protected]> Signed-off-by: shawnh2 <[email protected]> * chore: fix cve (envoyproxy#6446) * fix cve Signed-off-by: zirain <[email protected]> * lint Signed-off-by: zirain <[email protected]> --------- Signed-off-by: zirain <[email protected]> * fix: Do not set backendRequestTimeout when Retries are set (envoyproxy#6421) * fix: Do not set backendRequestTimeout when Retries are set Signed-off-by: sudipto baral <[email protected]> * fix: update comment Signed-off-by: sudipto baral <[email protected]> --------- Signed-off-by: sudipto baral <[email protected]> * gatewayapi: don't append gwcResource if there's invalid GatewayClass (envoyproxy#6379) * gatewayapi: don't process gloabal resources when acceptedGateways is 0 Signed-off-by: zirain <[email protected]> * update Signed-off-by: zirain <[email protected]> * fix test Signed-off-by: zirain <[email protected]> * don't skip gateways Signed-off-by: zirain <[email protected]> --------- Signed-off-by: zirain <[email protected]> Signed-off-by: shawnh2 <[email protected]> * fix testdata Signed-off-by: shawnh2 <[email protected]> * fix k8s provider controller Signed-off-by: shawnh2 <[email protected]> * fix: retry reconcile on transient errors during reconcile (envoyproxy#6299) * fix: add isTransientError helper to classify retryable errors Introduces isTransientError to detect transient Kubernetes errors and enable proper reconciliation retries. Signed-off-by: Patryk Rostkowski <[email protected]> handle errors from processing BackendRefs Signed-off-by: Huabing (Robin) Zhao <[email protected]> handle errors from processing ConfigMap Signed-off-by: Huabing (Robin) Zhao <[email protected]> * skip invalid GatewayClass Signed-off-by: Huabing (Robin) Zhao <[email protected]> * address comment Signed-off-by: Huabing (Robin) Zhao <[email protected]> * handle all transient errors Signed-off-by: Huabing (Robin) Zhao <[email protected]> * don't skip failed GCs Signed-off-by: Huabing (Robin) Zhao <[email protected]> --------- Signed-off-by: Patryk Rostkowski <[email protected]> Signed-off-by: Huabing (Robin) Zhao <[email protected]> Co-authored-by: Huabing (Robin) Zhao <[email protected]> (cherry picked from commit 71ce56f) Signed-off-by: Huabing (Robin) Zhao <[email protected]> * fix: fix bug in hostname overlap detection (envoyproxy#6332) fix bug in hostname overlap detection Signed-off-by: Rudrakh Panigrahi <[email protected]> (cherry picked from commit e78e268) Signed-off-by: Huabing (Robin) Zhao <[email protected]> * fix telemetry with host port not working (envoyproxy#6460) Signed-off-by: zirain <[email protected]> (cherry picked from commit c0a2ce7) Signed-off-by: Huabing (Robin) Zhao <[email protected]> * bugfix: BackendTlsPolicy should not reference across namespace (envoyproxy#6309) * bugfix: BackendTlsPolicy should not reference across namespace Signed-off-by: zirain <[email protected]> (cherry picked from commit 9925189) Signed-off-by: Huabing (Robin) Zhao <[email protected]> --------- Signed-off-by: Guy Daich <[email protected]> Signed-off-by: shawnh2 <[email protected]> Signed-off-by: Jeff Davis <[email protected]> Signed-off-by: zirain <[email protected]> Signed-off-by: kkk777-7 <[email protected]> Signed-off-by: Rudrakh Panigrahi <[email protected]> Signed-off-by: sudipto baral <[email protected]> Signed-off-by: Patryk Rostkowski <[email protected]> Signed-off-by: Huabing (Robin) Zhao <[email protected]> Co-authored-by: Guy Daich <[email protected]> Co-authored-by: Jeff Davis <[email protected]> Co-authored-by: zirain <[email protected]> Co-authored-by: Kota Kimura <[email protected]> Co-authored-by: Rudrakh Panigrahi <[email protected]> Co-authored-by: Sudipto Baral <[email protected]> Co-authored-by: Patryk Rostkowski <[email protected]> Co-authored-by: Huabing (Robin) Zhao <[email protected]> Signed-off-by: shawnh2 <[email protected]>

patrostkowski requested a review from a team as a code owner June 12, 2025 18:06

patrostkowski force-pushed the fix/transient-errors-6284 branch from c45000a to a69693a Compare June 12, 2025 19:15

patrostkowski commented Jun 12, 2025

View reviewed changes

test/helm/gateway-addons-helm/e2e.out.yaml Outdated Show resolved Hide resolved

rudrakhp reviewed Jun 12, 2025

View reviewed changes

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

patrostkowski force-pushed the fix/transient-errors-6284 branch from a69693a to ba30677 Compare June 16, 2025 05:50

arkodg added the cherrypick/release-v1.4.2 label Jun 24, 2025

zhaohuabing marked this pull request as draft June 26, 2025 02:40

zhaohuabing force-pushed the fix/transient-errors-6284 branch 3 times, most recently from fcd7f69 to 40ec0c9 Compare June 26, 2025 03:31

zhaohuabing force-pushed the fix/transient-errors-6284 branch 2 times, most recently from 5eeaad0 to 5313be2 Compare June 26, 2025 11:29

zhaohuabing self-assigned this Jun 26, 2025

zhaohuabing force-pushed the fix/transient-errors-6284 branch 2 times, most recently from 74541e2 to 27b84b4 Compare June 26, 2025 13:14

zhaohuabing changed the title ~~fix: add isTransientError helper to classify retryable errors during reconcile~~ fix: retry reconcile onvtransient errors during reconcile and use the last state to allow Envoy continue serving traffic Jun 26, 2025

zhaohuabing force-pushed the fix/transient-errors-6284 branch 3 times, most recently from 6f9a261 to 9b633af Compare June 27, 2025 01:11

zhaohuabing reviewed Jun 27, 2025

View reviewed changes

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

zhaohuabing force-pushed the fix/transient-errors-6284 branch from a3b57a9 to 63c2eee Compare June 27, 2025 02:33

zhaohuabing requested a review from cnvergence July 2, 2025 07:21

shawnh2 mentioned this pull request Jul 2, 2025

[release/v1.4] cherry pick v1.4.2 #6452

Merged

arkodg reviewed Jul 2, 2025

View reviewed changes

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

arkodg reviewed Jul 2, 2025

View reviewed changes

zhaohuabing added 2 commits July 3, 2025 01:05

handle all transient errors

2d1edd9

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

remove debug info

8704fd8

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

zhaohuabing force-pushed the fix/transient-errors-6284 branch from 9926890 to 8704fd8 Compare July 3, 2025 01:08

zhaohuabing requested a review from arkodg July 3, 2025 01:12

arkodg reviewed Jul 3, 2025

View reviewed changes

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

address comment

24d37d6

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

zhaohuabing requested a review from arkodg July 3, 2025 01:28

arkodg previously approved these changes Jul 3, 2025

View reviewed changes

arkodg requested review from a team and shahar-h and removed request for shahar-h July 3, 2025 01:36

don't skip failed GCs

c40c4cc

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

zhaohuabing dismissed arkodg’s stale review via c40c4cc July 3, 2025 04:44

minor change

80b953e

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

arkodg approved these changes Jul 3, 2025

View reviewed changes

guydc approved these changes Jul 3, 2025

View reviewed changes

arkodg merged commit 71ce56f into envoyproxy:main Jul 3, 2025
27 of 28 checks passed

zirain mentioned this pull request Jul 6, 2025

Gateway with invalid EnvoyProxy should not be provisioned #6400

Closed

BrewTestBot mentioned this pull request Aug 8, 2025

egctl 1.5.0 Homebrew/homebrew-core#232799

Merged

Conversation

patrostkowski commented Jun 12, 2025 • edited by zhaohuabing Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Jun 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

guydc commented Jun 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

guydc commented Jun 18, 2025

Uh oh!

arkodg commented Jun 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

arkodg commented Jun 24, 2025

Uh oh!

zhaohuabing commented Jun 26, 2025

Uh oh!

patrostkowski commented Jun 26, 2025

Uh oh!

zhaohuabing commented Jun 26, 2025

Uh oh!

Uh oh!

Uh oh!

arkodg Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

zhaohuabing Jul 3, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

arkodg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

patrostkowski commented Jun 12, 2025 •

edited by zhaohuabing

Loading

codecov bot commented Jun 13, 2025 •

edited

Loading

guydc commented Jun 13, 2025 •

edited

Loading

arkodg commented Jun 19, 2025 •

edited

Loading