Fix route_check.py: intersection retry and snapshot reorder#4650
Merged
deepak-singhal0408 merged 1 commit intoJul 14, 2026
Merged
Conversation
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
mike-dubrovsky
force-pushed
the
route-check-correctness
branch
from
7905d88 to
3597574
Compare
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
mike-dubrovsky
force-pushed
the
route-check-correctness
branch
from
3597574 to
29bb6fd
Compare
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Collaborator
|
Hi, there are workflow run(s) waiting for approval, you may be first-time contributor. I will notify maintainers to help approve once PR is approved. Thanks! ---Powered by SONiC BuildBot
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates scripts/route_check.py to improve correctness of suppress-fib-pending mitigation decisions by (1) using an intersection-based retry strategy in check_frr_pending_routes() and (2) reordering FRR polling to complete before taking ASIC/APPL DB snapshots, so cross-check conditions are evaluated against temporally aligned observations. It also updates existing unit tests and adds a new test module to validate the intersection retry behavior.
Changes:
- Implement intersection-based accumulation across FRR polling retries in
check_frr_pending_routes(). - Reorder
check_routes_for_namespace()so FRR polling completes before ASIC/APPL DB route snapshots are taken. - Standardize “missed FRR route” entries to dicts (
{'prefix', 'protocol'}) and update/add tests accordingly.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| scripts/route_check.py | Adds intersection-based retry logic for FRR polling and reorders FRR polling ahead of ASIC/APPL snapshots; updates missing-route representation to dicts. |
| tests/check_frr_pending_routes_test.py | New unit tests validating the intersection accumulator behavior for missed/failed FRR routes. |
| tests/fetch_routes_chunk_test.py | Updates chunked JSON parsing tests to assert on dict-form missing routes (prefix/protocol). |
| tests/route_check_test.py | Updates mocked FRR polling return shape to dict-form missed routes. |
| tests/route_check_test_data.py | Updates expected missed_FRR_routes fixtures to dict-form entries. |
mike-dubrovsky
force-pushed
the
route-check-correctness
branch
from
29bb6fd to
e86ed34
Compare
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
mike-dubrovsky
force-pushed
the
route-check-correctness
branch
from
e86ed34 to
e0e4dec
Compare
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
mike-dubrovsky
force-pushed
the
route-check-correctness
branch
from
e0e4dec to
ef3572f
Compare
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
mike-dubrovsky
force-pushed
the
route-check-correctness
branch
from
ef3572f to
e4f70d1
Compare
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Replace the overwrite-on-each-iteration retry loop with an intersection accumulator so that only routes stuck in every poll window are mitigated, avoiding premature mitigation of routes that are still converging. Add unit tests for the intersection logic with dict-format missing routes. Signed-off-by: mike-dubrovsky <mdubrovs@cisco.com>
mike-dubrovsky
force-pushed
the
route-check-correctness
branch
from
e4f70d1 to
1076912
Compare
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
deepak-singhal0408
approved these changes
Jul 13, 2026
deepak-singhal0408
left a comment
deepak-singhal0408
left a comment
Contributor
There was a problem hiding this comment.
All review comments addressed. Changes LGTM.
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
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
Split from #4576 per @deepak-singhal0408's review — Bucket B (correctness, land after Bucket A).
Depends on #4649 (dict-format bugfix). Includes two improvements:
check_frr_pending_routes(): only routes stuck in every poll window are mitigated, preventing premature mitigation of converging routes.rt_appl_miss/rt_asic_missare temporally aligned with FRR results when the mitigation cross-check fires.Keeps the ijson JSON parser (no text-streaming change).
Test plan
tests/check_frr_pending_routes_test.py— intersection accumulator cases