[action] [PR:27972] [test][snmp] Retry physical-entity MIB walk on torn/incomplete read in test_transceiver_info - #1419
Merged
Conversation
…n test_transceiver_info
### Description of PR
Summary:
Fixes an intermittent `KeyError: 'entPhysName'` (or `'entPhysDescr'`) failure in `snmp/test_snmp_phy_entity.py::test_transceiver_info` by making the underlying SNMP physical-entity MIB fetch resilient to torn/mid-walk reads.
Fixes # (issue)
### Type of change
- [x] Bug fix
- [ ] Testbed and Framework(new/improvement)
- [ ] New Test case
- [ ] Skipped for non-supported platforms
- [ ] Test case improvement
### Back port request
- [ ] 202311
- [ ] 202405
- [ ] 202411
- [ ] 202505
- [x] 202511
- [x] 202512
- [x] 202605
### Approach
#### What is the motivation for this PR?
`test_transceiver_info` (and other tests in `snmp/test_snmp_phy_entity.py` that share the `snmp_physical_entity_and_sensor_info` fixture) intermittently fail (roughly every 1-2 weeks on some platforms) with:
```
name_to_snmp_facts[values['entPhysName']] = values
KeyError: 'entPhysName'
```
or the equivalent for `entPhysDescr`, on a specific entity OID whose returned dict is missing required fields (e.g. `{'entPhysModelName': '', 'entPhysIsFRU': 2}`).
This was previously masked by a conditional `xfail` tied to `sonic-net/sonic-buildimage#22213`, but that issue has since been closed, so the `conditional_mark` plugin no longer applies the xfail and the intermittent race now surfaces as a hard failure.
#### How did you do it?
Root-caused via the `snmp_facts` ansible module (`ansible/library/snmp_facts.py`): `_collect_physical_entities` performs a single SNMP walk over the `entPhysicalTable` subtree, which - because of how SNMP table OIDs are structured (`<table>.<column>.<instance>`) - effectively walks **one MIB column at a time** across all rows (all `entPhysDescr` rows, then all `entPhysContainedIn` rows, ..., then `entPhysName`, etc.), rather than taking one atomic snapshot of the whole table. If a physical entity (e.g. a transceiver) is added, removed, or re-indexed by the SNMP subagent while this walk is still in progress, some columns for that entity can end up populated while others are missing entirely - a torn/inconsistent read for that particular OID.
Fix (test-side, in `tests/snmp/test_snmp_phy_entity.py`): in `get_entity_and_sensor_mib()`, after fetching the physical-entity MIB, validate that every returned row has both `entPhysDescr` and `entPhysName` (mandatory columns per RFC 2737 for any valid table row). If any row is incomplete, log a warning and retry the whole SNMP fetch (bounded: 60s timeout / 10s interval) before handing the result to the tests, rather than crashing with a raw `KeyError`. This converts the transient torn-read race into a resilient retry, while still surfacing a real, persistent incompleteness as a failure if it doesn't resolve after retries.
#### How did you verify/test it?
- Reproduced the original `KeyError: 'entPhysName'` failure history on the affected testbed (Nokia TH6 platform).
- Applied the fix and ran `snmp/test_snmp_phy_entity.py::test_transceiver_info` live against the same testbed/DUT **6 times total** (1 initial run + 5 repeated back-to-back runs to check for stability):
| Run | Result | Duration |
|-----|--------|----------|
| 1 (initial) | PASSED | ~6m16s |
| 2 | PASSED | ~6m34s |
| 3 | PASSED | ~6m35s |
| 4 | PASSED | ~6m22s |
| 5 | PASSED | ~6m31s |
| 6 | PASSED | ~6m27s |
**6/6 passed.**
- Confirmed via `py_compile` and `flake8` that the change is syntactically valid and lint-clean.
#### Any platform specific information?
Observed on Nokia TH6 platform (`Nokia-IXR7220-H6-O256`), but the root cause (a non-atomic, multi-column SNMP table walk racing with subagent-side reindexing) is generic and not platform-specific; the fix applies to all platforms using this fixture.
#### Supported testbed topology if it's a new test case?
N/A (not a new test case).
### Documentation
N/A
Signed-off-by: Sonic Build Admin <sonicbld@microsoft.com>
Collaborator
Author
|
Original PR: sonic-net/sonic-mgmt#27972 |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Collaborator
Author
|
/azp run |
1 similar comment
Collaborator
Author
|
/azp run |
|
Azure Pipelines: 1 pipeline(s) were filtered out due to trigger conditions. |
12 tasks
|
Azure Pipelines: 1 pipeline(s) were filtered out due to trigger conditions. |
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.
Description of PR
Summary:
Fixes an intermittent
KeyError: 'entPhysName'(or'entPhysDescr') failure insnmp/test_snmp_phy_entity.py::test_transceiver_infoby making the underlying SNMP physical-entity MIB fetch resilient to torn/mid-walk reads.Fixes # (issue)
Type of change
Back port request
Approach
What is the motivation for this PR?
test_transceiver_info(and other tests insnmp/test_snmp_phy_entity.pythat share thesnmp_physical_entity_and_sensor_infofixture) intermittently fail (roughly every 1-2 weeks on some platforms) with:or the equivalent for
entPhysDescr, on a specific entity OID whose returned dict is missing required fields (e.g.{'entPhysModelName': '', 'entPhysIsFRU': 2}).This was previously masked by a conditional
xfailtied tosonic-net/sonic-buildimage#22213, but that issue has since been closed, so theconditional_markplugin no longer applies the xfail and the intermittent race now surfaces as a hard failure.How did you do it?
Root-caused via the
snmp_factsansible module (ansible/library/snmp_facts.py):_collect_physical_entitiesperforms a single SNMP walk over theentPhysicalTablesubtree, which - because of how SNMP table OIDs are structured (<table>.<column>.<instance>) - effectively walks one MIB column at a time across all rows (allentPhysDescrrows, then allentPhysContainedInrows, ..., thenentPhysName, etc.), rather than taking one atomic snapshot of the whole table. If a physical entity (e.g. a transceiver) is added, removed, or re-indexed by the SNMP subagent while this walk is still in progress, some columns for that entity can end up populated while others are missing entirely - a torn/inconsistent read for that particular OID.Fix (test-side, in
tests/snmp/test_snmp_phy_entity.py): inget_entity_and_sensor_mib(), after fetching the physical-entity MIB, validate that every returned row has bothentPhysDescrandentPhysName(mandatory columns per RFC 2737 for any valid table row). If any row is incomplete, log a warning and retry the whole SNMP fetch (bounded: 60s timeout / 10s interval) before handing the result to the tests, rather than crashing with a rawKeyError. This converts the transient torn-read race into a resilient retry, while still surfacing a real, persistent incompleteness as a failure if it doesn't resolve after retries.How did you verify/test it?
Reproduced the original
KeyError: 'entPhysName'failure history on the affected testbed (Nokia TH6 platform).Applied the fix and ran
snmp/test_snmp_phy_entity.py::test_transceiver_infolive against the same testbed/DUT 6 times total (1 initial run + 5 repeated back-to-back runs to check for stability):6/6 passed.
Confirmed via
py_compileandflake8that the change is syntactically valid and lint-clean.Any platform specific information?
Observed on Nokia TH6 platform (
Nokia-IXR7220-H6-O256), but the root cause (a non-atomic, multi-column SNMP table walk racing with subagent-side reindexing) is generic and not platform-specific; the fix applies to all platforms using this fixture.Supported testbed topology if it's a new test case?
N/A (not a new test case).
Documentation
N/A
Signed-off-by: Sonic Build Admin sonicbld@microsoft.com