From 0041002cc94f5efbb7aaaad75427d3e74a867f10 Mon Sep 17 00:00:00 2001 From: Hardik Jain Date: Sat, 25 Jul 2020 13:36:00 +0530 Subject: [PATCH] [feature] Add a check which inspects device configuration status periodically #54 [influxdb] Add support for retention_policy in read --- README.rst | 41 +++++ openwisp_monitoring/check/apps.py | 9 + openwisp_monitoring/check/base/models.py | 21 ++- openwisp_monitoring/check/classes/__init__.py | 1 + openwisp_monitoring/check/classes/base.py | 4 +- .../check/classes/config_applied.py | 35 ++++ .../migrations/0005_create_config_applied.py | 38 ++++ openwisp_monitoring/check/settings.py | 12 +- openwisp_monitoring/check/tasks.py | 24 +++ .../check/tests/test_models.py | 165 +++++++++++++++--- openwisp_monitoring/check/tests/test_ping.py | 4 +- .../db/backends/influxdb/client.py | 12 +- .../db/backends/influxdb/tests.py | 36 +++- openwisp_monitoring/device/apps.py | 25 ++- .../device/tests/test_models.py | 18 ++ openwisp_monitoring/monitoring/base/models.py | 26 ++- openwisp_monitoring/monitoring/metrics.py | 6 + requirements-test.txt | 1 + 18 files changed, 432 insertions(+), 46 deletions(-) create mode 100644 openwisp_monitoring/check/classes/config_applied.py create mode 100644 openwisp_monitoring/check/migrations/0005_create_config_applied.py diff --git a/README.rst b/README.rst index b05380424..c0b1a05a8 100644 --- a/README.rst +++ b/README.rst @@ -213,6 +213,20 @@ command is used to collect these metrics. You may choose to disable auto creation of this check by setting `OPENWISP_MONITORING_AUTO_PING <#OPENWISP_MONITORING_AUTO_PING>`_ to ``False``. +``Configuration applied`` +~~~~~~~~~~~~~~~~~~~~~~~~~ + +This check ensures that the `openwisp-config agent `_ +is running and applying configuration changes in a timely manner. + +By default, if a configuration object stays in the ``modified`` state +for more than 5 minutes (configurable via +`OPENWISP_MONITORING_DEVICE_CONFIG_CHECK_MAX_TIME <#OPENWISP_MONITORING_DEVICE_CONFIG_CHECK_MAX_TIME>`_), +the check will fail and set the health status of the device to ``PROBLEM``. + +You may choose to disable auto creation of this check by using the +setting `OPENWISP_MONITORING_AUTO_DEVICE_CONFIG_CHECK <#OPENWISP_MONITORING_AUTO_DEVICE_CONFIG_CHECK>`_. + Settings -------- @@ -242,6 +256,33 @@ in terms of disk space. Whether ping checks are created automatically for devices. +``OPENWISP_MONITORING_AUTO_DEVICE_CONFIG_CHECK`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + ++--------------+-------------+ +| **type**: | ``bool`` | ++--------------+-------------+ +| **default**: | ``True`` | ++--------------+-------------+ + +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_DEVICE_CONFIG_CHECK_MAX_TIME`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + ++--------------+-----------+ +| **type**: | ``int`` | ++--------------+-----------+ +| **default**: | ``5`` | ++--------------+-----------+ + +Defines the maximum amount of minutes the device configuration can stay in the *modified* +state before its health status is changed to ``PROBLEM`` and an alert is sent. + +**Note**: The setting will be ignored if ``OPENWISP_MONITORING_AUTO_DEVICE_CONFIG_CHECK`` +is ``False``. + ``OPENWISP_MONITORING_AUTO_CHARTS`` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/openwisp_monitoring/check/apps.py b/openwisp_monitoring/check/apps.py index 479d9640d..8086d617d 100644 --- a/openwisp_monitoring/check/apps.py +++ b/openwisp_monitoring/check/apps.py @@ -22,3 +22,12 @@ def _connect_signals(self): sender=load_model('config', 'Device'), dispatch_uid='auto_ping', ) + + if app_settings.AUTO_CONFIG_CHECK: + from .base.models import auto_config_check_receiver + + post_save.connect( + auto_config_check_receiver, + sender=load_model('config', 'Device'), + dispatch_uid='auto_config_check', + ) diff --git a/openwisp_monitoring/check/base/models.py b/openwisp_monitoring/check/base/models.py index 64331b254..1e4df09f3 100644 --- a/openwisp_monitoring/check/base/models.py +++ b/openwisp_monitoring/check/base/models.py @@ -8,7 +8,7 @@ from django.utils.translation import gettext_lazy as _ from jsonfield import JSONField from openwisp_monitoring.check import settings as app_settings -from openwisp_monitoring.check.tasks import auto_create_ping +from openwisp_monitoring.check.tasks import auto_create_config_check, auto_create_ping from openwisp_utils.base import TimeStampedEditableModel @@ -97,3 +97,22 @@ def auto_ping_receiver(sender, instance, created, **kwargs): object_id=str(instance.pk), ) ) + + +def auto_config_check_receiver(sender, instance, created, **kwargs): + """ + Implements OPENWISP_MONITORING_AUTO_DEVICE_CONFIG_CHECK + The creation step is executed in the background + """ + # we need to skip this otherwise this task will be executed + # every time the configuration is requested via checksum + if not created: + return + with transaction.atomic(): + transaction.on_commit( + lambda: auto_create_config_check.delay( + model=sender.__name__.lower(), + app_label=sender._meta.app_label, + object_id=str(instance.pk), + ) + ) diff --git a/openwisp_monitoring/check/classes/__init__.py b/openwisp_monitoring/check/classes/__init__.py index 0fe4032e1..33bf8293c 100644 --- a/openwisp_monitoring/check/classes/__init__.py +++ b/openwisp_monitoring/check/classes/__init__.py @@ -1 +1,2 @@ +from .config_applied import ConfigApplied # noqa from .ping import Ping # noqa diff --git a/openwisp_monitoring/check/classes/base.py b/openwisp_monitoring/check/classes/base.py index 3ab391061..71ef7582d 100644 --- a/openwisp_monitoring/check/classes/base.py +++ b/openwisp_monitoring/check/classes/base.py @@ -30,7 +30,7 @@ def validate_params(self): def check(self, store=True): raise NotImplementedError - def _get_or_create_metric(self): + def _get_or_create_metric(self, configuration=None): """ Gets or creates metric """ @@ -44,7 +44,7 @@ def _get_or_create_metric(self): options = dict( object_id=obj_id, content_type=ct, - configuration=self.__class__.__name__.lower(), + configuration=configuration or self.__class__.__name__.lower(), ) metric, created = Metric._get_or_create(**options) return metric, created diff --git a/openwisp_monitoring/check/classes/config_applied.py b/openwisp_monitoring/check/classes/config_applied.py new file mode 100644 index 000000000..afdfce498 --- /dev/null +++ b/openwisp_monitoring/check/classes/config_applied.py @@ -0,0 +1,35 @@ +from swapper import load_model + +from ...device.utils import SHORT_RP +from ..settings import CONFIG_CHECK_MAX_TIME +from .base import BaseCheck + +AlertSettings = load_model('monitoring', 'AlertSettings') + + +class ConfigApplied(BaseCheck): + def check(self, store=True): + # If the device is down do not run config applied checks + if self.related_object.monitoring.status == 'critical': + return + if not hasattr(self.related_object, 'config'): + return + result = int(self.related_object.config.status == 'applied') + if store: + self._get_metric().write( + result, retention_policy=SHORT_RP, + ) + return result + + def _get_metric(self): + metric, created = self._get_or_create_metric(configuration='config_applied') + if created: + self._create_alert_setting(metric) + return metric + + def _create_alert_setting(self, metric): + alert_s = AlertSettings( + metric=metric, operator='<', value=1, seconds=CONFIG_CHECK_MAX_TIME * 60 + ) + alert_s.full_clean() + alert_s.save() diff --git a/openwisp_monitoring/check/migrations/0005_create_config_applied.py b/openwisp_monitoring/check/migrations/0005_create_config_applied.py new file mode 100644 index 000000000..22e8cc197 --- /dev/null +++ b/openwisp_monitoring/check/migrations/0005_create_config_applied.py @@ -0,0 +1,38 @@ +from django.db import migrations +from openwisp_monitoring.check.settings import AUTO_CONFIG_CHECK +from openwisp_monitoring.check.tasks import auto_create_config_check + + +def add_config_applied_checks(apps, schema_editor): + if not AUTO_CONFIG_CHECK: + return + Device = apps.get_model('config', 'Device') + for device in Device.objects.all(): + auto_create_config_check.delay( + model=Device.__name__.lower(), + app_label=Device._meta.app_label, + object_id=str(device.pk), + ) + + +def remove_config_applied_checks(apps, schema_editor): + Check = apps.get_model('check', 'Check') + Metric = apps.get_model('monitoring', 'Metric') + Check.objects.filter( + check='openwisp_monitoring.check.classes.ConfigApplied' + ).delete() + Metric.objects.filter(configuration='config_applied').delete() + + +class Migration(migrations.Migration): + + dependencies = [ + ('check', '0004_rename_active_to_is_active'), + ('monitoring', '0016_metric_configuration'), + ] + + operations = [ + migrations.RunPython( + add_config_applied_checks, reverse_code=remove_config_applied_checks + ), + ] diff --git a/openwisp_monitoring/check/settings.py b/openwisp_monitoring/check/settings.py index ba36fff2c..70e09d896 100644 --- a/openwisp_monitoring/check/settings.py +++ b/openwisp_monitoring/check/settings.py @@ -3,7 +3,17 @@ CHECK_CLASSES = getattr( settings, 'OPENWISP_MONITORING_CHECK_CLASSES', - (('openwisp_monitoring.check.classes.Ping', 'Ping'),), + ( + ('openwisp_monitoring.check.classes.Ping', 'Ping'), + ('openwisp_monitoring.check.classes.ConfigApplied', 'Configuration Applied'), + ), ) AUTO_PING = getattr(settings, 'OPENWISP_MONITORING_AUTO_PING', True) +AUTO_CONFIG_CHECK = getattr( + settings, 'OPENWISP_MONITORING_AUTO_DEVICE_CONFIG_CHECK', True +) +# Input in minutes +CONFIG_CHECK_MAX_TIME = getattr( + settings, 'OPENWISP_MONITORING_DEVICE_CONFIG_CHECK_MAX_TIME', 5 +) MANAGEMENT_IP_ONLY = getattr(settings, 'OPENWISP_MONITORING_MANAGEMENT_IP_ONLY', True) diff --git a/openwisp_monitoring/check/tasks.py b/openwisp_monitoring/check/tasks.py index 2ed22fc42..3cbbf398b 100644 --- a/openwisp_monitoring/check/tasks.py +++ b/openwisp_monitoring/check/tasks.py @@ -52,3 +52,27 @@ def auto_create_ping(model, app_label, object_id): check = Check(name='Ping', check=ping_path, content_type=ct, object_id=object_id) check.full_clean() check.save() + + +@shared_task +def auto_create_config_check(model, app_label, object_id): + """ + Called by openwisp_monitoring.check.models.auto_config_check_receiver + """ + Check = load_model('check', 'Check') + config_check_path = 'openwisp_monitoring.check.classes.ConfigApplied' + has_check = Check.objects.filter( + object_id=object_id, content_type__model='device', check=config_check_path, + ).exists() + # create new check only if necessary + if has_check: + return + ct = ContentType.objects.get(app_label=app_label, model=model) + check = Check( + name='Configuration Applied', + check=config_check_path, + content_type=ct, + object_id=object_id, + ) + check.full_clean() + check.save() diff --git a/openwisp_monitoring/check/tests/test_models.py b/openwisp_monitoring/check/tests/test_models.py index c60c7cef4..116922790 100644 --- a/openwisp_monitoring/check/tests/test_models.py +++ b/openwisp_monitoring/check/tests/test_models.py @@ -1,17 +1,27 @@ +from datetime import timedelta +from unittest.mock import patch + from django.core.exceptions import ValidationError from django.test import TransactionTestCase +from django.utils.timezone import now +from freezegun import freeze_time from swapper import load_model from ...device.tests import TestDeviceMonitoringMixin -from ..classes import Ping -from ..settings import CHECK_CLASSES +from .. import settings as app_settings +from ..classes import ConfigApplied, Ping +from ..tasks import auto_create_config_check, auto_create_ping Check = load_model('check', 'Check') Metric = load_model('monitoring', 'Metric') +AlertSettings = load_model('monitoring', 'AlertSettings') +Device = load_model('config', 'device') +Notification = load_model('openwisp_notifications', 'Notification') class TestModels(TestDeviceMonitoringMixin, TransactionTestCase): - _PING = CHECK_CLASSES[0][0] + _PING = app_settings.CHECK_CLASSES[0][0] + _CONFIG_APPLIED = app_settings.CHECK_CLASSES[1][0] def test_check_str(self): c = Check(name='Test check') @@ -29,8 +39,14 @@ def test_check_str_with_relation(self): self.assertEqual(str(c), expected) def test_check_class(self): - c = Check(name='Ping class check', check=self._PING) - self.assertEqual(c.check_class, Ping) + with self.subTest('Test Ping check Class'): + c = Check(name='Ping class check', check=self._PING) + self.assertEqual(c.check_class, Ping) + with self.subTest('Test Configuration Applied check Class'): + c = Check( + name='Configuration Applied class check', check=self._CONFIG_APPLIED + ) + self.assertEqual(c.check_class, ConfigApplied) def test_base_check_class(self): path = 'openwisp_monitoring.check.classes.base.BaseCheck' @@ -42,34 +58,131 @@ def test_base_check_class(self): def test_check_instance(self): obj = self._create_device(organization=self._create_org()) - c = Check( - name='Ping class check', check=self._PING, content_object=obj, params={} - ) - i = c.check_instance - self.assertIsInstance(i, Ping) - self.assertEqual(i.related_object, obj) - self.assertEqual(i.params, c.params) + with self.subTest('Test Ping check instance'): + c = Check( + name='Ping class check', check=self._PING, content_object=obj, params={} + ) + i = c.check_instance + self.assertIsInstance(i, Ping) + self.assertEqual(i.related_object, obj) + self.assertEqual(i.params, c.params) + with self.subTest('Test Configuration Applied check instance'): + c = Check( + name='Configuration Applied class check', + check=self._CONFIG_APPLIED, + content_object=obj, + params={}, + ) + i = c.check_instance + self.assertIsInstance(i, ConfigApplied) + self.assertEqual(i.related_object, obj) + self.assertEqual(i.params, c.params) def test_validation(self): - check = Check(name='Ping check', check=self._PING, params={}) - try: - check.full_clean() - except ValidationError as e: - self.assertIn('device', str(e)) - else: - self.fail('ValidationError not raised') - - def test_auto_ping(self): + with self.subTest('Test Ping check validation'): + check = Check(name='Ping check', check=self._PING, params={}) + try: + check.full_clean() + except ValidationError as e: + self.assertIn('device', str(e)) + else: + self.fail('ValidationError not raised') + with self.subTest('Test Configuration Applied check validation'): + check = Check(name='Config check', check=self._CONFIG_APPLIED, params={}) + try: + check.full_clean() + except ValidationError as e: + self.assertIn('device', str(e)) + else: + self.fail('ValidationError not raised') + + 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(), 1) - c = Check.objects.first() - self.assertEqual(c.content_object, d) - self.assertIn('Ping', c.check) + 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) + self.assertEqual(self._PING, c1.check) + with self.subTest('Test AUTO_CONFIG_CHECK'): + c2 = Check.objects.filter(check=self._CONFIG_APPLIED).first() + self.assertEqual(c2.content_object, d) + self.assertEqual(self._CONFIG_APPLIED, c2.check) def test_device_deleted(self): self.assertEqual(Check.objects.count(), 0) d = self._create_device(organization=self._create_org()) - self.assertEqual(Check.objects.count(), 1) + self.assertEqual(Check.objects.count(), 2) d.delete() self.assertEqual(Check.objects.count(), 0) + + @patch('openwisp_monitoring.check.settings.AUTO_PING', False) + def test_config_modified_device_problem(self): + self._create_admin() + self.assertEqual(Check.objects.count(), 0) + self._create_config(status='modified', organization=self._create_org()) + d = Device.objects.first() + 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() + with freeze_time(now() - timedelta(minutes=10)): + check.perform_check() + self.assertEqual(Metric.objects.count(), 1) + self.assertEqual(AlertSettings.objects.count(), 1) + # Check needs to be run again without mocking time for threshold crossed + check.perform_check() + m = Metric.objects.first() + self.assertEqual(m.content_object, d) + self.assertEqual(m.key, 'config_applied') + dm = d.monitoring + self.assertFalse(m.is_healthy) + self.assertEqual(dm.status, 'problem') + self.assertEqual(Notification.objects.count(), 1) + + @patch( + 'openwisp_monitoring.device.base.models.app_settings.CRITICAL_DEVICE_METRICS', + [{'key': 'config_applied', 'field_name': 'config_applied'}], + ) + @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(), 2) + d = Device.objects.first() + dm = d.monitoring + check = Check.objects.filter(check=self._CONFIG_APPLIED).first() + check.perform_check() + self.assertEqual(Metric.objects.count(), 1) + self.assertEqual(AlertSettings.objects.count(), 1) + m = Metric.objects.first() + self.assertTrue(dm.is_metric_critical(m)) + with freeze_time(now() - timedelta(minutes=10)): + check.perform_check() + dm.refresh_from_db() + self.assertEqual(dm.status, 'critical') + + def test_no_duplicate_check_created(self): + self._create_config(organization=self._create_org()) + self.assertEqual(Check.objects.count(), 2) + d = Device.objects.first() + auto_create_config_check.delay( + model=Device.__name__.lower(), + app_label=Device._meta.app_label, + object_id=str(d.pk), + ) + auto_create_ping.delay( + model=Device.__name__.lower(), + app_label=Device._meta.app_label, + object_id=str(d.pk), + ) + self.assertEqual(Check.objects.count(), 2) + + 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(), 2) + c2 = Check.objects.filter(check=self._CONFIG_APPLIED).first() + c2.perform_check() + self.assertEqual(Metric.objects.count(), 0) + self.assertIsNone(c2.perform_check()) diff --git a/openwisp_monitoring/check/tests/test_ping.py b/openwisp_monitoring/check/tests/test_ping.py index ba9b021a8..48899e07b 100644 --- a/openwisp_monitoring/check/tests/test_ping.py +++ b/openwisp_monitoring/check/tests/test_ping.py @@ -201,11 +201,11 @@ 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(), 1) + self.assertEqual(Check.objects.count(), 2) self.assertEqual(Metric.objects.count(), 0) self.assertEqual(Chart.objects.count(), 0) self.assertEqual(AlertSettings.objects.count(), 0) - check = Check.objects.first() + check = Check.objects.filter(check=self._PING).first() result = check.perform_check() self.assertEqual(Metric.objects.count(), 1) self.assertEqual(Chart.objects.count(), 3) diff --git a/openwisp_monitoring/db/backends/influxdb/client.py b/openwisp_monitoring/db/backends/influxdb/client.py index c93bb63de..b05477ecd 100644 --- a/openwisp_monitoring/db/backends/influxdb/client.py +++ b/openwisp_monitoring/db/backends/influxdb/client.py @@ -7,6 +7,7 @@ from django.conf import settings from django.core.exceptions import ValidationError from django.utils.functional import cached_property +from django.utils.timezone import now from django.utils.translation import gettext_lazy as _ from influxdb import InfluxDBClient @@ -112,11 +113,10 @@ def write(self, name, values, **kwargs): 'tags': kwargs.get('tags'), 'fields': values, } - timestamp = kwargs.get('timestamp') + timestamp = kwargs.get('timestamp') or now() if isinstance(timestamp, datetime): - timestamp = timestamp.strftime('%Y-%m-%dT%H:%M:%SZ') - if timestamp: - point['time'] = timestamp + timestamp = timestamp.isoformat(sep='T', timespec='microseconds') + point['time'] = timestamp self.get_db.write( {'points': [point]}, { @@ -130,11 +130,13 @@ def read(self, key, fields, tags, **kwargs): since = kwargs.get('since') order = kwargs.get('order') limit = kwargs.get('limit') + rp = kwargs.get('retention_policy') if extra_fields and extra_fields != '*': fields = ', '.join([fields] + extra_fields) elif extra_fields == '*': fields = '*' - q = f'SELECT {fields} FROM {key}' + from_clause = f'{rp}.{key}' if rp else key + q = f'SELECT {fields} FROM {from_clause}' conditions = [] if since: conditions.append(f'time >= {since}') diff --git a/openwisp_monitoring/db/backends/influxdb/tests.py b/openwisp_monitoring/db/backends/influxdb/tests.py index 90e1eebd0..b94970d8c 100644 --- a/openwisp_monitoring/db/backends/influxdb/tests.py +++ b/openwisp_monitoring/db/backends/influxdb/tests.py @@ -1,4 +1,4 @@ -from datetime import timedelta +from datetime import datetime, timedelta from django.core.exceptions import ValidationError from django.test import TestCase @@ -11,6 +11,7 @@ from .. import timeseries_db Chart = load_model('monitoring', 'Chart') +Notification = load_model('openwisp_notifications', 'Notification') class TestDatabaseClient(TestMonitoringMixin, TestCase): @@ -176,7 +177,8 @@ def test_retention_policy(self): self.assertEqual(len(rp), 2) self.assertEqual(rp[1]['name'], SHORT_RP) self.assertEqual(rp[1]['default'], False) - self.assertEqual(rp[1]['duration'], SHORT_RETENTION_POLICY) + duration = SHORT_RETENTION_POLICY + self.assertEqual(rp[1]['duration'], duration) def test_query_set(self): c = self._create_chart(configuration='histogram') @@ -208,3 +210,33 @@ def test_read_order(self): with self.assertRaises(timeseries_db.client_error) as e: metric_data = m.read(limit=2, order='invalid') self.assertIn('Invalid order "invalid" passed.', str(e)) + + def test_read_with_rp(self): + self._create_admin() + manage_short_retention_policy() + with self.subTest( + 'Test metric write on short retention_policy immediate alert' + ): + m = self._create_general_metric(name='dummy') + self._create_alert_settings(metric=m, operator='<', value=1, seconds=0) + m.write(0, retention_policy=SHORT_RP) + self.assertEqual(m.read(retention_policy=SHORT_RP)[0][m.field_name], 0) + self.assertFalse(m.is_healthy) + self.assertEqual(Notification.objects.count(), 1) + with self.subTest( + 'Test metric write on short retention_policy with deferred alert' + ): + m2 = self._create_general_metric(name='dummy2') + self._create_alert_settings(metric=m2, operator='<', value=1, seconds=60) + m.write(0, retention_policy=SHORT_RP, time=now() - timedelta(minutes=2)) + self.assertEqual(m.read(retention_policy=SHORT_RP)[0][m.field_name], 0) + self.assertFalse(m.is_healthy) + self.assertEqual(Notification.objects.count(), 1) + + def test_metric_write_microseconds_precision(self): + m = self._create_object_metric( + name='wlan0', key='wlan0', configuration='clients' + ) + m.write('00:14:5c:00:00:00', time=datetime(2020, 7, 31, 22, 5, 47, 235142)) + m.write('00:23:4a:00:00:00', time=datetime(2020, 7, 31, 22, 5, 47, 235152)) + self.assertEqual(len(m.read()), 2) diff --git a/openwisp_monitoring/device/apps.py b/openwisp_monitoring/device/apps.py index 487b5838c..6b4b32879 100644 --- a/openwisp_monitoring/device/apps.py +++ b/openwisp_monitoring/device/apps.py @@ -6,9 +6,10 @@ from openwisp_notifications.types import register_notification_type from swapper import load_model -from openwisp_controller.config.signals import checksum_requested +from openwisp_controller.config.signals import checksum_requested, config_modified from openwisp_controller.connection.signals import is_working_changed +from ..check import settings as check_settings from . import settings as app_settings from .signals import device_metrics_received, health_status_changed from .utils import get_device_recovery_cache_key, manage_short_retention_policy @@ -24,6 +25,7 @@ def ready(self): self.register_notifcation_types() self.connect_is_working_changed() self.connect_device_signals() + self.connect_config_modified() self.device_recovery_detection() def connect_device_signals(self): @@ -172,3 +174,24 @@ def register_notifcation_types(self): ), }, ) + + @classmethod + def connect_config_modified(cls): + from openwisp_controller.config.models import Config + + if check_settings.AUTO_CONFIG_CHECK: + config_modified.connect( + cls.config_modified_receiver, + sender=Config, + dispatch_uid='config_modified', + ) + + @classmethod + def config_modified_receiver(cls, sender, instance, **kwargs): + from ..check.tasks import perform_check + + DeviceData = load_model('device_monitoring', 'DeviceData') + device = DeviceData.objects.get(config=instance) + check = device.checks.filter(check__contains='ConfigApplied') + if check: + perform_check.delay(check.first().id) diff --git a/openwisp_monitoring/device/tests/test_models.py b/openwisp_monitoring/device/tests/test_models.py index 126fe1e54..a90107030 100644 --- a/openwisp_monitoring/device/tests/test_models.py +++ b/openwisp_monitoring/device/tests/test_models.py @@ -8,6 +8,7 @@ from paramiko.ssh_exception import NoValidConnectionsError from swapper import load_model +from openwisp_controller.config.signals import config_modified from openwisp_controller.connection.tests.base import CreateConnectionsMixin from openwisp_utils.tests import catch_signal @@ -675,3 +676,20 @@ def test_is_working_no_recovery_notification(self, notify_send, perform_check): dc.save() notify_send.assert_not_called() perform_check.assert_not_called() + + @patch('openwisp_monitoring.check.tasks.perform_check.delay') + def test_config_modified_receiver(self, mock_method): + c = self._create_config(status='applied', organization=self._create_org()) + config_applied_path = 'openwisp_monitoring.check.classes.ConfigApplied' + Check.objects.create( + name='Configuration Applied', + content_object=c.device, + check=config_applied_path, + ) + c.config = {'general': {'description': 'test'}} + c.full_clean() + with catch_signal(config_modified) as handler: + c.save() + handler.assert_called_once() + self.assertEqual(c.status, 'modified') + self.assertEqual(mock_method.call_count, 1) diff --git a/openwisp_monitoring/monitoring/base/models.py b/openwisp_monitoring/monitoring/base/models.py index 05fa5854e..6dc810177 100644 --- a/openwisp_monitoring/monitoring/base/models.py +++ b/openwisp_monitoring/monitoring/base/models.py @@ -144,7 +144,7 @@ def content_type_key(self): except AttributeError: return None - def check_threshold(self, value, time=None): + def check_threshold(self, value, time=None, retention_policy=None): """ checks if the threshold is crossed and notifies users accordingly @@ -153,7 +153,7 @@ def check_threshold(self, value, time=None): alert_settings = self.alertsettings except ObjectDoesNotExist: return - crossed = alert_settings._is_crossed_by(value, time) + crossed = alert_settings._is_crossed_by(value, time, retention_policy) first_time = False # situation has not changed if (not crossed and self.is_healthy) or (crossed and self.is_healthy is False): @@ -186,7 +186,15 @@ def check_threshold(self, value, time=None): if alert_settings.is_active: self._notify_users(notification_type, alert_settings) - def write(self, value, time=None, database=None, check=True, extra_values=None): + def write( + self, + value, + time=None, + database=None, + check=True, + extra_values=None, + retention_policy=None, + ): """ write timeseries data """ values = {self.field_name: value} if extra_values and isinstance(extra_values, dict): @@ -202,13 +210,14 @@ def write(self, value, time=None, database=None, check=True, extra_values=None): tags=self.tags, timestamp=time, database=database, + retention_policy=retention_policy, ) post_metric_write.send(**signal_kwargs) # check can be disabled, # mostly for automated testing and debugging purposes if not check: return - self.check_threshold(value, time) + self.check_threshold(value, time, retention_policy) def read(self, **kwargs): """ reads timeseries data """ @@ -495,7 +504,7 @@ def _time_crossed(self, time): threshold_time = timezone.now() - timedelta(seconds=self.seconds) return time < threshold_time - def _is_crossed_by(self, current_value, time=None): + def _is_crossed_by(self, current_value, time=None, retention_policy=None): """ do current_value and time cross the threshold? """ value_crossed = self._value_crossed(current_value) if value_crossed is NotImplemented: @@ -508,7 +517,12 @@ def _is_crossed_by(self, current_value, time=None): # retrieves latest measurements up to the maximum # threshold in seconds allowed plus a small margin since = 'now() - {0}s'.format(int(self._SECONDS_MAX * 1.05)) - points = self.metric.read(since=since, limit=None, order='-time') + points = self.metric.read( + since=since, + limit=None, + order='-time', + retention_policy=retention_policy, + ) # loop on each measurement starting from the most recent for i, point in enumerate(points): # skip the first point because it was just added before this diff --git a/openwisp_monitoring/monitoring/metrics.py b/openwisp_monitoring/monitoring/metrics.py index 8a3ad2f2b..cd151bbfe 100644 --- a/openwisp_monitoring/monitoring/metrics.py +++ b/openwisp_monitoring/monitoring/metrics.py @@ -12,6 +12,12 @@ 'field_name': 'reachable', 'related_fields': ['loss', 'rtt_min', 'rtt_max', 'rtt_avg'], }, + 'config_applied': { + 'label': _('Configuration Applied'), + 'name': 'Configuration Applied', + 'key': 'config_applied', + 'field_name': 'config_applied', + }, 'traffic': { 'label': _('Traffic'), 'name': '{name}', diff --git a/requirements-test.txt b/requirements-test.txt index 2b15f2f56..46b35502b 100644 --- a/requirements-test.txt +++ b/requirements-test.txt @@ -3,3 +3,4 @@ redis django-redis mock-ssh-server>=0.8.0,<0.9.0 channels_redis +freezegun