Skip to content

[test][snmp] Retry physical-entity MIB walk on torn/incomplete read in test_transceiver_info - #27972

Merged
bingwang-ms merged 1 commit into
sonic-net:masterfrom
bingwang-ms:fix/snmp-transceiver-info-torn-mib-walk
Sep 16, 2026
Merged

bingwang-ms merged 1 commit into
sonic-net:masterfrom
bingwang-ms:fix/snmp-transceiver-info-torn-mib-walk

Conversation

@bingwang-ms

@bingwang-ms bingwang-ms commented Sep 16, 2026 •

Copy link
Copy Markdown
Collaborator

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

  • 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
  • 202511
  • 202512
  • 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

test_transceiver_info (and other snmp/test_snmp_phy_entity.py tests
sharing the snmp_physical_entity_and_sensor_info fixture) could
intermittently fail with:

    KeyError: 'entPhysName'

or a similar KeyError on 'entPhysDescr'.

Root cause: the snmp_facts ansible module walks the entPhysicalTable
one MIB column at a time (all entPhysDescr rows, then all
entPhysContainedIn rows, ..., then all entPhysName rows, etc.)
rather than taking a single atomic snapshot of the whole table. If a
physical entity (e.g. a transceiver) is added/removed/reindexed by the
SNMP subagent while the walk is still in progress, some columns for
that entity can end up populated while others are missing entirely,
producing a torn/inconsistent read for that OID.

This was previously masked by an xfail condition tied to
github.com/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 test failure.

Fix: in get_entity_and_sensor_mib(), validate that every returned
physical-entity row has both entPhysDescr and entPhysName (mandatory
per RFC 2737) and retry the SNMP walk (up to 60s, 10s interval) if any
row is incomplete, before handing the result to the tests. This
converts the transient torn-read race into a resilient retry instead
of a hard failure, while still surfacing a real, persistent failure if
the incompleteness doesn't resolve.

Signed-off-by: bingwang <bingwang@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: bingwang <bingwang@microsoft.com>
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@bingwang-ms
bingwang-ms added this pull request to the merge queue Sep 16, 2026
Merged via the queue into sonic-net:master with commit e548f3e Sep 16, 2026
30 checks passed
@mssonicbld

Copy link
Copy Markdown
Collaborator

Cherry-pick PR to msft-202512: Azure/sonic-mgmt.msft#1419

@mssonicbld

Copy link
Copy Markdown
Collaborator

This PR has backport request label(s) for branch(es): 202511,msft-202512,202605, but is missing required test information. Please make sure you tick the tested branch(es) in the Tested branch section and provide test evidence (e.g., 202511: <test result>) in the Test result section as well in your PR description.

---Powered by SONiC BuildBot

@StormLiangMS

Copy link
Copy Markdown
Collaborator

@bingwang-ms

Pls update 202605 test result for cherrypick ask.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants