diff --git a/README.rst b/README.rst index bb8d7c482..9f81c9d56 100644 --- a/README.rst +++ b/README.rst @@ -94,6 +94,7 @@ Available Features * Extensible metrics and charts: it's possible to define new metrics and new charts * API to retrieve the chart metrics and status information of each device based on `NetJSON DeviceMonitoring `_ +* Collection of monitoring information via `SNMP`_ ------------ @@ -503,15 +504,29 @@ configuration status of a device changes, this ensures the check reacts quickly to events happening in the network and informs the user promptly if there's anything that is not working as intended. -SNMP DeviceMonitoring -~~~~~~~~~~~~~~~~~~~~~ +SNMP +~~~~ -This check provides an alternative protocol to collect monitoring information from devices that don't have -the ability to use `openwisp-config agent `_. +This check provides an alternative way to collect monitoring information from devices that don't have +the ability to use `openwisp-monitoring agent `_. The information is collected via SNMP using `netengine `_. The devices need to have an SNMP daemon in order for this to work. This check is disabled by default, you may choose to enable auto creation of this check by setting -`OPENWISP_MONITORING_AUTO_SNMP_DEVICEMONITORING <#OPENWISP_MONITORING_AUTO_SNMP_DEVICEMONITORING>`_ to ``True``. +`OPENWISP_MONITORING_AUTO_SNMP <#OPENWISP_MONITORING_AUTO_SNMP>`_ to ``True``. + +Instructions to use this feature: + +1. Register your device to OpenWISP via admin or `api `_. +2. Create SNMP access credentials and add them to your device: + + In the admin, go to ``Access Credentials > Add Access Credentials`` and select a type suitable for your device's backend. + Then either enable ``Auto Add``, or add it to your device manually. + Alternatively, use the `api endpoint `_. +3. Create an SNMP check: + + In the admin, go to ``Check > Add Check``. Choose ``content type: config | Device``, ``Check type: SNMP Device Monitoring`` + and write your device's UUID in the ``Object id`` section. + You can alternatively use the setting `OPENWISP_MONITORING_AUTO_SNMP <#OPENWISP_MONITORING_AUTO_SNMP>`_. Settings -------- @@ -554,8 +569,8 @@ Whether ping checks are created automatically for devices. This setting allows you to choose whether `config_applied <#configuration-applied>`_ checks should be created automatically for newly registered devices. It's enabled by default. -``OPENWISP_MONITORING_AUTO_SNMP_DEVICEMONITORING`` -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +``OPENWISP_MONITORING_AUTO_SNMP`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +--------------+-------------+ | **type**: | ``bool`` | diff --git a/openwisp_monitoring/check/apps.py b/openwisp_monitoring/check/apps.py index 17fdf6c4b..c4ad3702c 100644 --- a/openwisp_monitoring/check/apps.py +++ b/openwisp_monitoring/check/apps.py @@ -33,11 +33,11 @@ def _connect_signals(self): dispatch_uid='auto_config_check', ) - if app_settings.AUTO_SNMP_DEVICEMONITORING: - from .base.models import auto_snmp_devicemonitoring_receiver + if app_settings.AUTO_SNMP: + from .base.models import auto_snmp_receiver post_save.connect( - auto_snmp_devicemonitoring_receiver, + auto_snmp_receiver, sender=load_model('config', 'Device'), - dispatch_uid='auto_snmp_devicemonitoring', + dispatch_uid='auto_snmp', ) diff --git a/openwisp_monitoring/check/base/models.py b/openwisp_monitoring/check/base/models.py index 4f6a76644..4413b85cb 100644 --- a/openwisp_monitoring/check/base/models.py +++ b/openwisp_monitoring/check/base/models.py @@ -122,7 +122,7 @@ def auto_config_check_receiver(sender, instance, created, **kwargs): ) -def auto_snmp_devicemonitoring_receiver(sender, instance, created, **kwargs): +def auto_snmp_receiver(sender, instance, created, **kwargs): """ Implements OPENWISP_MONITORING_AUTO_DEVICE_SNMP_DEVICEMONITORING The creation step is executed in the background diff --git a/openwisp_monitoring/check/classes/__init__.py b/openwisp_monitoring/check/classes/__init__.py index ff9cb43ac..75c2e3320 100644 --- a/openwisp_monitoring/check/classes/__init__.py +++ b/openwisp_monitoring/check/classes/__init__.py @@ -1,3 +1,3 @@ from .config_applied import ConfigApplied # noqa from .ping import Ping # noqa -from .snmp_devicemonitoring import SnmpDeviceMonitoring # noqa +from .snmp_devicemonitoring import Snmp # noqa diff --git a/openwisp_monitoring/check/classes/snmp_devicemonitoring.py b/openwisp_monitoring/check/classes/snmp_devicemonitoring.py index 753abaff3..628d9cbbf 100644 --- a/openwisp_monitoring/check/classes/snmp_devicemonitoring.py +++ b/openwisp_monitoring/check/classes/snmp_devicemonitoring.py @@ -15,7 +15,7 @@ AlertSettings = load_model('monitoring', 'AlertSettings') -class SnmpDeviceMonitoring(BaseCheck, MetricChartsMixin): +class Snmp(BaseCheck, MetricChartsMixin): def check(self, store=True): result = self.netengine_instance.to_dict() self._init_previous_data(data=getattr(self.related_object, 'data', {})) diff --git a/openwisp_monitoring/check/settings.py b/openwisp_monitoring/check/settings.py index 08cd6e3f9..8f92042eb 100644 --- a/openwisp_monitoring/check/settings.py +++ b/openwisp_monitoring/check/settings.py @@ -5,13 +5,10 @@ ( ('openwisp_monitoring.check.classes.Ping', 'Ping'), ('openwisp_monitoring.check.classes.ConfigApplied', 'Configuration Applied'), - ( - 'openwisp_monitoring.check.classes.SnmpDeviceMonitoring', - 'SNMP Device Monitoring', - ), + ('openwisp_monitoring.check.classes.Snmp', 'SNMP Device Monitoring',), ), ) AUTO_PING = get_settings_value('AUTO_PING', True) AUTO_CONFIG_CHECK = get_settings_value('AUTO_DEVICE_CONFIG_CHECK', True) -AUTO_SNMP_DEVICEMONITORING = get_settings_value('AUTO_SNMP_DEVICEMONITORING', False) +AUTO_SNMP = get_settings_value('AUTO_SNMP', False) MANAGEMENT_IP_ONLY = get_settings_value('MANAGEMENT_IP_ONLY', True) diff --git a/openwisp_monitoring/check/tasks.py b/openwisp_monitoring/check/tasks.py index f7ba31f0f..799f70aef 100644 --- a/openwisp_monitoring/check/tasks.py +++ b/openwisp_monitoring/check/tasks.py @@ -105,10 +105,10 @@ def auto_create_snmp_devicemonitoring( model, app_label, object_id, check_model=None, content_type_model=None ): """ - Called by openwisp_monitoring.check.models.auto_snmp_devicemonitoring_receiver + Called by openwisp_monitoring.check.models.auto_snmp_receiver """ Check = check_model or get_check_model() - devicemonitoring_path = 'openwisp_monitoring.check.classes.SnmpDeviceMonitoring' + devicemonitoring_path = 'openwisp_monitoring.check.classes.Snmp' has_check = Check.objects.filter( object_id=object_id, content_type__model='device', check=devicemonitoring_path ).exists() diff --git a/openwisp_monitoring/check/tests/test_models.py b/openwisp_monitoring/check/tests/test_models.py index e44bed8e1..d1e134acc 100644 --- a/openwisp_monitoring/check/tests/test_models.py +++ b/openwisp_monitoring/check/tests/test_models.py @@ -9,7 +9,7 @@ from ...device.tests import TestDeviceMonitoringMixin from .. import settings as app_settings -from ..classes import ConfigApplied, Ping, SnmpDeviceMonitoring +from ..classes import ConfigApplied, Ping, Snmp from ..tasks import ( auto_create_config_check, auto_create_ping, @@ -53,11 +53,8 @@ def test_check_class(self): ) self.assertEqual(c.check_class, ConfigApplied) with self.subTest('Test DeviceMonitoring check Class'): - c = Check( - name='SnmpDeviceMonitoring class check', - check=self._SNMP_DEVICEMONITORING, - ) - self.assertEqual(c.check_class, SnmpDeviceMonitoring) + c = Check(name='Snmp class check', check=self._SNMP_DEVICEMONITORING,) + self.assertEqual(c.check_class, Snmp) def test_base_check_class(self): path = 'openwisp_monitoring.check.classes.base.BaseCheck' @@ -96,7 +93,7 @@ def test_check_instance(self): params={}, ) i = c.check_instance - self.assertIsInstance(i, SnmpDeviceMonitoring) + self.assertIsInstance(i, Snmp) self.assertEqual(i.related_object, obj) self.assertEqual(i.params, c.params) @@ -121,7 +118,7 @@ def test_validation(self): def test_auto_check_creation(self): self.assertEqual(Check.objects.count(), 0) d = self._create_device(organization=self._create_org()) - self.assertEqual(Check.objects.count(), 3) + self.assertEqual(Check.objects.count(), 2) with self.subTest('Test AUTO_PING'): c1 = Check.objects.filter(check=self._PING).first() self.assertEqual(c1.content_object, d) @@ -134,7 +131,7 @@ def test_auto_check_creation(self): def test_device_deleted(self): self.assertEqual(Check.objects.count(), 0) d = self._create_device(organization=self._create_org()) - self.assertEqual(Check.objects.count(), 3) + self.assertEqual(Check.objects.count(), 2) d.delete() self.assertEqual(Check.objects.count(), 0) @@ -145,7 +142,7 @@ def test_config_modified_device_problem(self): self._create_config(status='modified', organization=self._create_org()) d = Device.objects.first() d.monitoring.update_status('ok') - self.assertEqual(Check.objects.count(), 3) + self.assertEqual(Check.objects.count(), 2) self.assertEqual(Metric.objects.count(), 0) self.assertEqual(AlertSettings.objects.count(), 0) check = Check.objects.filter(check=self._CONFIG_APPLIED).first() @@ -174,7 +171,7 @@ def test_config_error(self): self._create_config(status='error', organization=self._create_org()) dm = Device.objects.first().monitoring dm.update_status('ok') - self.assertEqual(Check.objects.count(), 3) + self.assertEqual(Check.objects.count(), 2) self.assertEqual(Metric.objects.count(), 0) self.assertEqual(AlertSettings.objects.count(), 0) check = Check.objects.filter(check=self._CONFIG_APPLIED).first() @@ -207,7 +204,7 @@ def test_config_error(self): @patch('openwisp_monitoring.check.settings.AUTO_PING', False) def test_config_check_critical_metric(self): self._create_config(status='modified', organization=self._create_org()) - self.assertEqual(Check.objects.count(), 3) + self.assertEqual(Check.objects.count(), 2) d = Device.objects.first() dm = d.monitoring dm.update_status('ok') @@ -226,7 +223,7 @@ def test_config_check_critical_metric(self): def test_no_duplicate_check_created(self): self._create_config(organization=self._create_org()) - self.assertEqual(Check.objects.count(), 3) + self.assertEqual(Check.objects.count(), 2) d = Device.objects.first() auto_create_config_check.delay( model=Device.__name__.lower(), @@ -249,7 +246,7 @@ def test_device_unreachable_no_config_check(self): self._create_config(status='modified', organization=self._create_org()) d = self.device_model.objects.first() d.monitoring.update_status('critical') - self.assertEqual(Check.objects.count(), 3) + self.assertEqual(Check.objects.count(), 2) c2 = Check.objects.filter(check=self._CONFIG_APPLIED).first() c2.perform_check() self.assertEqual(Metric.objects.count(), 0) @@ -260,7 +257,7 @@ def test_device_unknown_no_config_check(self): self._create_config(status='modified', organization=self._create_org()) d = self.device_model.objects.first() d.monitoring.update_status('unknown') - self.assertEqual(Check.objects.count(), 3) + self.assertEqual(Check.objects.count(), 2) c2 = Check.objects.filter(check=self._CONFIG_APPLIED).first() c2.perform_check() self.assertEqual(Metric.objects.count(), 0) diff --git a/openwisp_monitoring/check/tests/test_ping.py b/openwisp_monitoring/check/tests/test_ping.py index c635a9d67..7fdec2104 100644 --- a/openwisp_monitoring/check/tests/test_ping.py +++ b/openwisp_monitoring/check/tests/test_ping.py @@ -201,7 +201,7 @@ def test_store_result(self, mocked_method): device.management_ip = '10.40.0.1' device.save() # check created automatically by autoping - self.assertEqual(Check.objects.count(), 3) + self.assertEqual(Check.objects.count(), 2) self.assertEqual(Metric.objects.count(), 0) self.assertEqual(Chart.objects.count(), 0) self.assertEqual(AlertSettings.objects.count(), 0) diff --git a/openwisp_monitoring/check/tests/test_utils.py b/openwisp_monitoring/check/tests/test_utils.py index 36a29e561..d79312c4b 100644 --- a/openwisp_monitoring/check/tests/test_utils.py +++ b/openwisp_monitoring/check/tests/test_utils.py @@ -6,7 +6,7 @@ from ...check.tests.utils import MockOpenWRT from ...device.tests import TestDeviceMonitoringMixin -from ..classes import Ping, SnmpDeviceMonitoring +from ..classes import Ping, Snmp from ..settings import CHECK_CLASSES from ..tasks import perform_check from ..utils import run_checks_async @@ -25,12 +25,12 @@ def _create_check(self): # check is automatically created via django signal @patch.object(Ping, '_command', return_value=_FPING_REACHABLE) - @patch.object(SnmpDeviceMonitoring, 'netengine_instance', MockOpenWRT) + @patch.object(Snmp, 'netengine_instance', MockOpenWRT) def test_run_checks_async_success(self, mocked_method): self._create_check() run_checks_async() - @patch.object(SnmpDeviceMonitoring, 'netengine_instance', MockOpenWRT) + @patch.object(Snmp, 'netengine_instance', MockOpenWRT) @patch.object(Ping, '_command', return_value=_FPING_REACHABLE) def test_management_command(self, mocked_method): self._create_check() diff --git a/openwisp_monitoring/device/tests/test_transactions.py b/openwisp_monitoring/device/tests/test_transactions.py index 808ec8881..72b673c58 100644 --- a/openwisp_monitoring/device/tests/test_transactions.py +++ b/openwisp_monitoring/device/tests/test_transactions.py @@ -8,9 +8,8 @@ from openwisp_controller.connection.tests.base import CreateConnectionsMixin from openwisp_utils.tests import catch_signal -from ...check.classes import Ping, SnmpDeviceMonitoring +from ...check.classes import Ping from ...check.tests import _FPING_REACHABLE, _FPING_UNREACHABLE -from ...check.tests.utils import MockOpenWRT from ..tasks import trigger_device_checks from . import DeviceMonitoringTransactionTestcase @@ -55,8 +54,6 @@ def test_trigger_device_recovery_task(self, mocked_method): self._post_data(d.id, d.key, data) mock.assert_called_once() - @patch.object(SnmpDeviceMonitoring, 'netengine_instance', MockOpenWRT) - @patch('openwisp_monitoring.check.settings.AUTO_SNMP_DEVICEMONITORING', False) @patch.object(Ping, '_command', return_value=_FPING_UNREACHABLE) @patch.object(DeviceMonitoring, 'update_status') def test_trigger_device_recovery_task_regression( @@ -69,7 +66,7 @@ def test_trigger_device_recovery_task_regression( self.assertTrue(Check.objects.exists()) # we expect update_status() to be called once (by the check) # and not a second time directly by our code - self.assertEqual(mocked_update_status.call_count, 3) + self.assertEqual(mocked_update_status.call_count, 1) @patch.object(Check, 'perform_check') def test_is_working_false_true(self, perform_check): diff --git a/tests/openwisp2/settings.py b/tests/openwisp2/settings.py index feb3d8ea3..719fbe814 100644 --- a/tests/openwisp2/settings.py +++ b/tests/openwisp2/settings.py @@ -192,7 +192,6 @@ OPENWISP_MONITORING_MAC_VENDOR_DETECTION = False OPENWISP_MONITORING_API_URLCONF = 'openwisp_monitoring.urls' OPENWISP_MONITORING_API_BASEURL = 'http://testserver' - OPENWISP_MONITORING_AUTO_SNMP_DEVICEMONITORING = True # Temporarily added to identify slow tests TEST_RUNNER = 'openwisp_utils.tests.TimeLoggingTestRunner'