-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add netsnmp-cffi as alternative SNMP polling back-end #394
Draft
lunkwill42
wants to merge
21
commits into
master
Choose a base branch
from
feature/use-netsnmpy-cffi
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains 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
In preparation for a new upcoming SNMP back-end, this moves the pysnmp- specific back-end to a submodule of `zino.snmp` so an interchangeable module can be made more easily. The _mib_value_to_python function was imported outside of the original module, so it was renamed to be public instead of internal in order for imports to keep working. Tests and patches that referenced old names were updated, and all mibdump files were also moved.
These classes and type definitions aren't specific for the PySNMP back-end and are therefore moved to a new module from where they can be shared.
The poll device `hcounters` attribute is really what drives the SNMP version selection in Zino. Unfortunately, the default is SNMP v1, and this has worked because PySNMP silently allowed v2c requests like BULK-GET to be sent over sessions that were otherwise v1. Net-SNMP does not allow the sending of v2c PDUs over v1 sessions, so we need to be more strict about this. It also makes no sense to default to v1 in 2025, so this sets `hcounters`' default value to True and fixes a test that implicitly assumed v1 exceptions to be raised.
I don't generally think it is a good idea to check for very specific log output. In this case, the non-int object type returned by the underlying library may depend on the library. netsnmpy-cffi does not generally translate octet strings to `str` unless they have a textual convention of `DisplayString`. That has really no bearing on the actual functionality in this case, so it's quite weird to test for such specific log messages.
The underlying netsnmpy session object will not work with mock values for addresses and community string, so its better to use an actual mocked PollDevice object rather than a 100% Mock.
This ensures all MIB lookups work properly when running the test suite.
MIBs should be vendored verbatim.
This vendors in all the MIB definitions required to run Zino. PySNMP will work with versions translated to Python modules, Net-SNMP needs the original definitions.
The netsnmp-cffi package i still in pre-alpha and has no proper release yet. It will be moved to GitHub and released on PyPI in due time.
This specifically reaches into the PySNMP-based back-end for handling traps, thereby making the test suite green again.
The workflow may need to build netsnmp-cffi from scratch if no pre-made wheels are available.
I.e. have Net-SNMP load MIBs from its defined locations
There is no need to get a new SNMP session here, the instance already has one.
Before this, every Task instance would create its own SNMP session instance. This might not mean much under PySNMP, but is wasteful using netsnmp-cffi, as each open session takes up a socket resource. Having multiple simultaneous sessions/sockets per poll device makes no sense, so `get_snmp_session()` utilizes a `WeakValueDictionary` to keep track of the existing session for each device and returns an existing session if there is one.
These test patches will not work now that everything uses `zino.snmp.get_snmp_session()` - the SNMP class needs to be patched in the correct location.
The new backend needs more or less the same set of tests as the old back-end. This copies the tests for the PySNMP back-end as a baseline for modification.
This rewrites the tests copied from the PySNMP back-end so they instead test the netsnmpy backend.
Some of the PySNMP back-end only had implicit test coverage, which was lost in the rewrite to a multi-backend system. This adds explicit coverage to some of the bits that lost implicit coverage, to keep the coverage numbers up.
|
6 tasks
🦙 MegaLinter status: ✅ SUCCESS
See detailed report in MegaLinter reports |
Test results 3 files 3 suites 1m 23s ⏱️ Results for commit 6e91f30. |
6 tasks
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.
Scope and purpose
Fixes the polling bits of #383 (replaces #391)
This refactors the SNMP agent back-end to be even more amenable to replacement, and moves on to add an alternative implementation based on the new netsnmp-cffi library.
It does not (currently) replace the
trapd
back-end, which continues to work alongside these changese. This is potentially a separate PR, but the drawback is currently that the two back-ends use a different set of MIB files (the original PySNMP back-end vendors MIB files translated to Python code, while this back-end introduces a directory of vendored MIB files in their original format).Contributor Checklist
Every pull request should have this checklist filled out, no matter how small it is.
More information about contributing to Zino can be found in the
README.