fix(gateway): surface swallowed NetworkGateway reconcile errors - #370
Open
ecv wants to merge 1 commit into
Open
Conversation
Reconcile logged and then dropped three failures: every BGPAdvertisement write in the apply loop, the self-address publish, and the crash-recovery orphan sweep. The Ready condition was computed from the engine result alone, so a node that had advertised nothing still reported Ready=True with reason EngineHealthy, and Reconcile returned nil, so nothing retried either. Advertisement errors are now collected across the loop (the remaining rules are still applied first, one bad rule must not stop the others), joined with any self-address failure, and reported as Ready=False with reason AdvertisementFailed and a message naming what failed. The orphan sweep failure is returned as well. Both return after the status write, following the existing EngineReconcileFailed path, so the failure lands on the object and controller-runtime retries with backoff. publishSelfAddress now writes status.sRv6Address through the caller's own object rather than a copy, so the Ready condition written moments later is not lost to a resourceVersion conflict on exactly the pass whose outcome matters. Related to #365
ecv
force-pushed
the
fix/gateway-surface-advertisement-failures
branch
from
August 13, 2026 01:24
822ddab to
9d5f3dd
Compare
ecv
marked this pull request as ready for review
August 13, 2026 01:57
ecv
requested review from
bmertens-datum,
privateip and
scotwells
and removed request for
bmertens-datum
August 13, 2026 01:57
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The gateway node programs its rules, then advertises the routes that steer traffic to them. Programming failures propagated. Advertisement failures were logged and dropped, and the Ready condition was computed from the engine result alone.
A node that programmed every rule and advertised none reported Ready, returned success, and never retried. The one signal an operator would check said the opposite of what was true.
Advertisement failures are now collected across the pass, so one bad rule still does not stop the others, and then reported. Ready says
AdvertisementFailedwith the rules named, and the error goes back to controller-runtime so the pass retries with backoff.The node's own reachability address is treated the same way, since a node that cannot publish where it is reachable is in no better shape.
The orphan sweep is returned rather than logged past. It is crash recovery, and the generation cutoff is captured specifically for it. Status is still written before it runs, so a sweep failure does not hide the condition.
Engine failure keeps its precedence. If the engine is degraded, that is the root cause and the condition still says so.
One thing found on the way
Publishing the self-address wrote status through a copy, leaving the caller holding a stale resource version. The Ready condition written later in the same pass was then lost to a conflict, logged and forgotten. It updates in place now. Without this, the fix above would report correctly and then fail to persist on exactly the passes that matter.
Test plan
task linttask buildtask test:unit, including three new cases: advertisement failure surfaces on the condition and returns, a clean pass still reports healthy, and an orphan sweep failure propagates with the condition already writtentask test:e2eCI is the gate; this machine cannot build the module.
Note the reconcilers this touches are not registered by any binary on main yet. That arrives with #352, so there is no runtime behavior change until it lands.
Related to #365