Move to easysnmp instead of pysnmp, use snmpwalk instead of manually looping #4
Conversation
…looping The switch to easynmp means that instead of ~90 seconds (~4 seconds per snmpget) the script now takes between 0.5 and 3 seconds to complete...
|
Thank you for providing a new PR to implement snmpwalk into the check. It looks quite good on a first look at the diff. |
| @@ -2,7 +2,7 @@ | |||
|
|
|||
| This plugin uses ```snmpv3``` with ```MD5``` + ```AES``` to check a lot of different values on your Synology DiskStation. | |||
|
|
|||
| This check plugin needs ```pysnmp``` to be installed on your system. You can install it with: ```pip install pysnmp``` | |||
| This check plugin needs ```pysnmp``` to be installed on your system. You can install it with: ```pip install easysnmp``` | |||
There was a problem hiding this comment.
I run into an error while installing easysnmp. The documentation gave me the hint that easysnmp depends on net-snmp (https://easysnmp.readthedocs.io/en/latest/)
| This check plugin needs ```pysnmp``` to be installed on your system. You can install it with: ```pip install easysnmp``` | |
| This check plugin needs ```easysnmp``` to be installed on your system. You can install it with: ```pip install easysnmp``` (Note that ```easysnmp``` needs ```net-snmp``` you might need to install ```libsnmp-dev``` and ```snmp-mibs-downloader``` on your system) |
| @@ -161,7 +171,7 @@ def exitCode(): | |||
| state = 'CRITICAL' | |||
|
|
|||
| update_status = status_translation.get(update_status_nr) | |||
| print state + ' - DSM Version: %s, DSM Update: %s' % (update_dsm_verison, update_status), '| DSMupdate=%sc' % update_status_nr | |||
| print(state + ' - DSM Version: %s, DSM Update: %s' % (update_dsm_verison, update_status), '| DSMupdate=%sc' % update_status_nr) | |||
There was a problem hiding this comment.
This now results in:
(u'OK - DSM Version: DSM 6.2-24922, DSM Update: Unavailable', u'| DSMupdate=2c') instead of
OK - DSM Version: DSM 6.2-24922, DSM Update: Unavailable | DSMupdate=2c
I think once again the parentheses are the problem
There was a problem hiding this comment.
Hmm, python3 needs the parentheses (will not print them!), while python2 will print them verbatim.
See e.g.http://python3porting.com/noconv.html, https://python-future.org/compatible_idioms.html . so if you can live with python2.6+, then you could import future and make it work with python2 and python3.
There was a problem hiding this comment.
As python2 is deprecated I'd opt for only supporting python3 and and state version 0.1 as the last python2 compatible release.
There was a problem hiding this comment.
Hi there,
having those improvements integrated into mainline would be awesome to save system resources. I also would like to express my support for dropping Python 2 compatibility here. Do you have any strong objections against it, @wernerfred?
With kind regards,
Andreas.
There was a problem hiding this comment.
No objections from my side. This PR is outdated and needs to be rebased with conflicts. (#8 already introduced python3 support). So missing changes are only switching from pysnmp to easysnmp.
@amotl As I am lacking of time in the short term would you have time to solve the conflicts and update the PR? I am happy to merge then
There was a problem hiding this comment.
Hi Frederic,
we just submitted #19. For testing purposes, we are also running this in production now. While we have not come across a timeout situation yet (see #17), we believe it works well so far and would be ready to be merged.
Maybe you want to tag 0.3.1 beforehand, in order to mark this as the most recent release based on pysnmp?
With kind regards,
Andreas.
There was a problem hiding this comment.
Maybe you want to tag 0.3.1 beforehand, in order to mark this as the most recent release based on
pysnmp?
Good point.
The easysnmp pr #19 will be a breaking change and qualify for 1.0.0
There was a problem hiding this comment.
Thank you, I've added f2e973a. I think it will be safe to close this PR. Thanks again for your contribution, @kainhofer!
wernerfred
added
the
waiting for contributor action
|
Hi again, we just gave this a spin and would like to report back that it works excellently on Python 3, with all six modes. With, for example, the We attached some guidelines below to outline how we are running it. With kind regards, /cc @Tonkenfo # Acquire sources.
git clone https://github.com/kainhofer/check_synology --branch easysnmp /opt/check_synology_easysnmp
# Install prerequisites.
apt install --yes libsnmp-dev
python3 -m venv /opt/check_synology_easysnmp/.venv
/opt/check_synology_easysnmp/.venv/bin/pip install easysnmptime /opt/check_synology_easysnmp/.venv/bin/python /opt/check_synology_easysnmp/check_synology.py lalala status
real 0m0.801s |
The switch to easynmp means that instead of ~90 seconds (~4 seconds per snmpget) the script now takes between 0.5 and 3 seconds to complete...