fix: confirm Envoy Service via uncached read before clearing Gateway address#9568
fix: confirm Envoy Service via uncached read before clearing Gateway address#9568sealab2066 wants to merge 3 commits into
Conversation
…address A transient empty result from the cached client in envoyServiceForGateway (an informer cache that has not yet observed the Service, or a relist window) was treated as "the Service no longer exists". UpdateGatewayStatusProgrammedCondition then wiped gw.Status.Addresses and set Programmed=False/AddressNotAssigned, and the Gateway did not self-recover. Confirm with an uncached read (GetAPIReader) before concluding the Service is gone. Refs envoyproxy#9567 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Sergei Cherevko <scherevko@scorewarrior.com>
✅ Deploy Preview for cerulean-figolla-1f9435 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e1475f0e77
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if kerrors.IsNotFound(err) { | ||
| return nil, nil | ||
| } | ||
| return nil, err |
There was a problem hiding this comment.
When the cached list is empty but the uncached API read fails, such as an apiserver timeout or 429 while the informer is stale, this returns (nil, err). updateStatusForGateway only logs that error and still calls UpdateGatewayStatusProgrammedCondition with svc == nil (status.go lines 749-762), which clears Gateway.status.addresses and marks AddressNotAssigned; that is exactly the unconfirmed-cache-miss case this fallback is meant to avoid. Treat the failed confirmation as “do not update addresses yet” or make the caller skip the programmed update on service lookup errors.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in 6c6380b. updateStatusForGateway now skips UpdateGatewayStatusProgrammedCondition when the Service lookup returns an error, so a failed uncached confirmation (apiserver timeout/429 with a stale informer) no longer clears the addresses. Added a test covering the propagated-error case.
Address review feedback: when the cached list is empty and the uncached confirmation read itself fails (apiserver timeout/429 while the informer is stale), envoyServiceForGateway returns (nil, err). updateStatusForGateway now skips UpdateGatewayStatusProgrammedCondition on that error instead of clearing gw.Status.Addresses / setting AddressNotAssigned from an inconclusive read. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Sergei Cherevko <scherevko@scorewarrior.com>
|
Related to #9509 (fixes #9432) — same family (stale informer cache in the Gateway status path, both fixed with an uncached read), but a different code path, so the two are complementary rather than duplicates:
Neither supersedes the other; merging both closes the two distinct |
What this PR does / why we need it:
Fixes #9567.
A Gateway backed by a LoadBalancer Service can get permanently stuck at
Programmed=False/AddressNotAssigned, with itsstatus.addressescleared, after a transient empty result from the cached client — even though the backing Service has a valid IP and the data plane keeps routing.updateStatusForGateway→envoyServiceForGatewaydoes a cachedList. When that list transiently returns 0 items (an informer cache that has not yet observed the Service, or a relist window), the caller receivessvc == nil, andUpdateGatewayStatusProgrammedConditiontakes theelse { gw.Status.Addresses = nil }branch, producingAddressNotAssigned. There is no distinction between "the Service is genuinely absent" and "the cache momentarily returned nothing", and no periodic status resync — so once latched, the Gateway does not self-recover (only a controller restart or a forced ServiceMODIFIEDevent fixes it).This confirms with an uncached read (
mgr.GetAPIReader()) before concluding the Service is gone, so a transient cache miss no longer wipes the Gateway address.See #9567 for the full analysis: the trigger is a periodic (every ~10 min) cloud LB re-ensure that touches every Service, the recovery paths (
rollout restart/kubectl annotate svc), and why the already-merged fixes #8962 (#8953) and #9100 (#9094) do not cover this path.Release Notes: Yes (
release-notes/current/bug_fixes/9567-gateway-address-transient-empty-service-list.md).