Skip to content

Block C migration miss: discovery.py:_snmp_get still uses pysnmp 6.x API #1

@commitconfirm

Description

@commitconfirm

Summary

controller/app/discovery.py:_snmp_get() raises an ImportError at
first call:

cannot import name 'getCmd' from 'pysnmp.hlapi.asyncio'

The project is correctly pinned to pysnmp==7.1.24 in
controller/requirements.txt. The rest of the SNMP code path
(controller/app/snmp_collector.py, used by all Block C collectors)
was migrated to the pysnmp 7.x API as a v1.0 prerequisite. This one
function in discovery.py was missed because it pre-dates the SNMP
collector refactor and was never refactored to use it.

Root cause

pysnmp 7.x removed the 6.x camelCase API names. discovery.py:_snmp_get
still uses them:

# controller/app/discovery.py, ~line 773
from pysnmp.hlapi.asyncio import (
    CommunityData, ContextData, ObjectIdentity, ObjectType,
    SnmpEngine, UdpTransportTarget, getCmd,        # <-- 6.x camelCase
)
transport = UdpTransportTarget((ip, 161), ...)    # <-- 6.x sync ctor
...
await getCmd(...)                                  # <-- 6.x name

The 7.x equivalents (already used throughout snmp_collector.py):

  • getCmdget_cmd (snake_case)
  • UdpTransportTarget(...)await UdpTransportTarget.create(...) (async factory)

The import path pysnmp.hlapi.asyncio itself is still valid in 7.x (it is
the backward-compat alias for pysnmp.hlapi.v3arch.asyncio); only the
function names and the transport constructor changed.

Impact

SNMP discovery probes fail before any UDP packet is sent. Sweep IPs that
should have surfaced sysDescr / sysObjectID / sysName via SNMP fall
through to the non-SNMP classification path. Onboarding-time classifier
discrimination loses its strongest signal.

No impact on Block C collectors (ARP/MAC/LLDP/routes) - those use
snmp_collector.py, which is correctly on the 7.x API.

Fix

Three-line change in _snmp_get:

  1. Import get_cmd instead of getCmd.
  2. await UdpTransportTarget.create((ip, 161), timeout=3, retries=1)
    instead of the sync constructor.
  3. Call await get_cmd(...) instead of await getCmd(...).

Mechanical pattern - mirrors what snmp_collector.py lines 40-49 and
~380, ~499 already do.

Related cleanup (worth bundling)

_snmp_get instantiates a fresh SnmpEngine() per call. This is the
exact UDP-socket-leak pattern the LEAK-FIX (mnm commit ebbfef8)
removed from snmp_collector.py by introducing a shared
_get_engine() singleton. The remaining per-call leak in _snmp_get
is slower than the pre-LEAK-FIX rate (because discovery is operator-
triggered, not continuous), but it's the same class of bug. The fix
should switch _snmp_get to use snmp_collector._get_engine() rather
than creating its own SnmpEngine().

Acceptance criteria

  • cannot import name 'getCmd' no longer appears in controller logs
    during SNMP discovery.
  • Lab devices with valid SNMP credentials surface sysDescr /
    sysObjectID / sysName / sysContact / sysLocation /
    sysUpTime via the discovery path again.
  • FD count on the controller is flat across N discovery probes
    (LEAK-FIX-pattern regression check; pattern in
    tests/unit/test_snmp_collector.py::test_walk_table_does_not_leak_fds_real_socket_path).
  • No regressions in snmp_collector.py consumers (ARP/MAC/LLDP/routes
    Block C collectors).

Lesson (for CLAUDE.md)

When migrating a dependency with API breaking changes, grep the
entire codebase for the old API names
, not just the high-traffic
modules. The Block C migration covered snmp_collector.py (the SNMP
abstraction layer) and left discovery.py:_snmp_get (a pre-existing
inline SNMP caller) as a straggler. Two greps would have caught it
at migration time:

grep -rn "getCmd\\|bulkCmd\\|nextCmd\\|setCmd" .
grep -rn "UdpTransportTarget(" .   # 6.x sync constructor

Both grep results should be exhaustively reviewed during any pysnmp
(or other dep-with-major-version-restructure) upgrade.

References

  • pysnmp 7.x migration notes: https://docs.lextudio.com/pysnmp/
    (snake_case rename + v3arch.asyncio reorganization)
  • snmp_collector.py migration commit: 6b548e2 chore: migrate pysnmp-lextudio to pysnmp 7.x
  • LEAK-FIX commit (related shared-engine pattern): ebbfef8 fix(controller): SnmpEngine FD leak in snmp_collector

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingdependenciesDependency version pins, library upgrades, package management

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions