From 4fc8c96c41c3a08e8ca25d234bf92cc4fc69e81b Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Fri, 19 Jun 2026 13:28:07 -0400 Subject: [PATCH] fix(snmp): bundle pysnmp in image and port poller to the asyncio HLAPI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The SNMP plugin ships in the image but logged "pysnmp not installed — SNMP polling disabled" on every poll, so polling never worked. Two coupled defects: 1. The Dockerfile installed only `.[ansible]`, so the `snmp` extra (pysnmp) was never bundled even though the plugin is first-party and shipped. 2. poller.py used the synchronous pysnmp HLAPI (`next(getCmd(...))`), which pysnmp-lextudio 6.x removed — it's asyncio-only now — so even with the dep present, polling would have thrown and silently returned nothing. The 5.x line that still has the sync API isn't safe on the image's Python 3.13. Fix: - Dockerfile: install `.[ansible,snmp]`. - poller.py: `poll_device_sync` → `async def poll_device` on the asyncio HLAPI, with a dual-version import (pysnmp 7.x `pysnmp.hlapi.v3arch.asyncio`/`get_cmd` + async `UdpTransportTarget.create`; pysnmp-lextudio 6.2.x `pysnmp.hlapi.asyncio`/`getCmd` + direct `UdpTransportTarget`) so a dependency bump can't silently re-break it. - scheduler.py: await poll_device directly; drop the run_in_executor wrapper and the now-unused asyncio import. - Add tests/plugins/snmp/test_poller.py covering the version→mpModel mapping, that the poller is a coroutine, and the graceful no-pysnmp path. Note: CI confirms import/load and the no-pysnmp path, but has no SNMP target — live polling against real devices is verified after deploy. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_016Jg27rgypiW2efULXJDtMC --- Dockerfile | 6 +- plugins/snmp/poller.py | 91 +++++++++++++++++++++---------- plugins/snmp/scheduler.py | 11 +--- tests/plugins/snmp/__init__.py | 0 tests/plugins/snmp/test_poller.py | 32 +++++++++++ 5 files changed, 101 insertions(+), 39 deletions(-) create mode 100644 tests/plugins/snmp/__init__.py create mode 100644 tests/plugins/snmp/test_poller.py diff --git a/Dockerfile b/Dockerfile index e74ceca..fc8a570 100644 --- a/Dockerfile +++ b/Dockerfile @@ -19,8 +19,10 @@ WORKDIR /app COPY pyproject.toml . COPY steward/ steward/ -# .[ansible] pulls the full Ansible package so the playbook runner works in-image. -RUN pip install --no-cache-dir '.[ansible]' +# Bundle the extras whose first-party plugins ship in the image: [ansible] for +# the playbook runner, [snmp] (pysnmp) for the SNMP poller. Without them those +# bundled plugins load but silently no-op for want of their runtime dependency. +RUN pip install --no-cache-dir '.[ansible,snmp]' COPY alembic.ini . # First-party plugins ship inside the image (bundled root at /app/plugins). diff --git a/plugins/snmp/poller.py b/plugins/snmp/poller.py index 6a837f6..22c7c84 100644 --- a/plugins/snmp/poller.py +++ b/plugins/snmp/poller.py @@ -1,11 +1,13 @@ # plugins/snmp/poller.py """ -Synchronous SNMP GET helper, run via executor. +Asynchronous SNMP GET helper. -Requires pysnmp-lextudio (maintained pysnmp fork): +Requires pysnmp-lextudio (the maintained pysnmp fork), bundled into the Docker +image via the `snmp` extra (`pip install .[snmp]`): pip install 'steward[snmp]' -If pysnmp is not installed, poll_device() returns an empty dict and logs a warning. +If pysnmp is not installed, poll_device() returns an empty dict and logs a +warning — SNMP polling is then simply disabled, nothing else breaks. """ from __future__ import annotations import logging @@ -22,39 +24,74 @@ def _pysnmp_available() -> bool: def _mp_model(version: str) -> int: - """Map version string to pysnmp mpModel integer.""" + """Map an SNMP version string to pysnmp's mpModel int (0 = v1, 1 = v2c).""" return 0 if version == "1" else 1 -def poll_device_sync( +async def poll_device( host: str, port: int, community: str, version: str, oids: list[dict], ) -> dict[str, float]: - """ - Perform SNMP GET for each OID and return {label: float_value}. - Non-numeric OIDs (strings, etc.) are skipped. - Returns empty dict on any error. + """Perform an SNMP GET for each OID and return ``{label: float_value}``. + + Non-numeric OIDs (strings, etc.) are skipped. Returns an empty dict on any + error (unreachable host, wrong community, …) so a flaky device never breaks + the poll loop. + + pysnmp's HLAPI is asyncio-only as of v6. The import path moved between major + versions, so we support both rather than let a dependency bump silently + re-break polling: + • pysnmp-lextudio 6.2.x → ``pysnmp.hlapi.asyncio`` (``getCmd`` + a directly + constructed ``UdpTransportTarget``). + • canonical pysnmp 7.x → ``pysnmp.hlapi.v3arch.asyncio`` (``get_cmd`` + the + async ``UdpTransportTarget.create``). """ if not _pysnmp_available(): logger.warning("pysnmp not installed — SNMP polling disabled. " "Install with: pip install 'steward[snmp]'") return {} - from pysnmp.hlapi import ( - CommunityData, - ContextData, - ObjectIdentity, - ObjectType, - SnmpEngine, - UdpTransportTarget, - getCmd, - ) + try: + # canonical pysnmp 7.x + from pysnmp.hlapi.v3arch.asyncio import ( + CommunityData, + ContextData, + ObjectIdentity, + ObjectType, + SnmpEngine, + UdpTransportTarget, + get_cmd as _get_cmd, + ) + _transport_is_async = True + except ImportError: + # pysnmp-lextudio 6.2.x + from pysnmp.hlapi.asyncio import ( + CommunityData, + ContextData, + ObjectIdentity, + ObjectType, + SnmpEngine, + UdpTransportTarget, + getCmd as _get_cmd, + ) + _transport_is_async = False + + engine = SnmpEngine() + + # Same host/port for every OID on this device, so build the transport once. + try: + if _transport_is_async: + transport = await UdpTransportTarget.create((host, port), timeout=5, retries=1) + else: + transport = UdpTransportTarget((host, port), timeout=5, retries=1) + except Exception as exc: + logger.debug("SNMP transport setup failed for %s:%s: %s", host, port, exc) + return {} results: dict[str, float] = {} - engine = SnmpEngine() for oid_cfg in oids: oid = oid_cfg["oid"] @@ -62,14 +99,12 @@ def poll_device_sync( scale = float(oid_cfg.get("scale", 1.0)) try: - error_indication, error_status, error_index, var_binds = next( - getCmd( - engine, - CommunityData(community, mpModel=_mp_model(version)), - UdpTransportTarget((host, port), timeout=5, retries=1), - ContextData(), - ObjectType(ObjectIdentity(oid)), - ) + error_indication, error_status, error_index, var_binds = await _get_cmd( + engine, + CommunityData(community, mpModel=_mp_model(version)), + transport, + ContextData(), + ObjectType(ObjectIdentity(oid)), ) except Exception as exc: logger.debug("SNMP GET %s@%s OID %s failed: %s", host, port, oid, exc) @@ -88,7 +123,7 @@ def poll_device_sync( try: results[label] = float(val) * scale except (TypeError, ValueError): - # Non-numeric type (e.g. OctetString description) — skip + # Non-numeric type (e.g. OctetString description) — skip. logger.debug("SNMP non-numeric value for %s label=%s: %r", oid, label, val) return results diff --git a/plugins/snmp/scheduler.py b/plugins/snmp/scheduler.py index d1a1d85..e0d4df7 100644 --- a/plugins/snmp/scheduler.py +++ b/plugins/snmp/scheduler.py @@ -1,6 +1,5 @@ # plugins/snmp/scheduler.py from __future__ import annotations -import asyncio import logging from typing import TYPE_CHECKING @@ -27,15 +26,13 @@ def make_poll_task(app: "Quart") -> ScheduledTask: async def _do_poll(app: "Quart") -> None: - from .poller import poll_device_sync + from .poller import poll_device from steward.core.alerts import record_metric devices: list[dict] = app.config["PLUGINS"]["snmp"].get("devices", []) if not devices: return - loop = asyncio.get_event_loop() - async with app.db_sessionmaker() as session: async with session.begin(): for device in devices: @@ -52,11 +49,7 @@ async def _do_poll(app: "Quart") -> None: continue try: - readings = await loop.run_in_executor( - None, - poll_device_sync, - host, port, community, version, oids, - ) + readings = await poll_device(host, port, community, version, oids) except Exception: logger.exception("SNMP poll failed for device %s (%s)", name, host) continue diff --git a/tests/plugins/snmp/__init__.py b/tests/plugins/snmp/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/plugins/snmp/test_poller.py b/tests/plugins/snmp/test_poller.py new file mode 100644 index 0000000..151f928 --- /dev/null +++ b/tests/plugins/snmp/test_poller.py @@ -0,0 +1,32 @@ +"""Unit tests for the SNMP poller (no pysnmp / no network). + +The unit lane doesn't install the `snmp` extra, so these exercise the parts that +don't need pysnmp: the version→mpModel mapping, that the poller is async, and the +graceful "pysnmp missing → empty result" path. Live polling against a real device +is verified out-of-band (CI has no SNMP target). Uses asyncio.run() directly so it +doesn't depend on the pytest-asyncio mode. +""" +import asyncio +import inspect + +from plugins.snmp import poller + + +def test_poll_device_is_coroutine(): + # The scheduler awaits it directly (no executor) — it must be async. + assert inspect.iscoroutinefunction(poller.poll_device) + + +def test_mp_model_maps_version_to_int(): + assert poller._mp_model("1") == 0 # SNMP v1 + assert poller._mp_model("2c") == 1 # SNMP v2c + assert poller._mp_model("2") == 1 # anything non-"1" → v2c model + + +def test_poll_device_without_pysnmp_returns_empty(monkeypatch): + # When the optional dep is absent, polling is disabled gracefully (no raise). + monkeypatch.setattr(poller, "_pysnmp_available", lambda: False) + out = asyncio.run( + poller.poll_device("192.0.2.1", 161, "public", "2c", [{"oid": "1.3.6.1.2.1.1.3.0"}]) + ) + assert out == {}