diff --git a/README.rst b/README.rst index b4fe04b18..fa9e76b33 100644 --- a/README.rst +++ b/README.rst @@ -253,6 +253,51 @@ in terms of disk space. Whether ping checks are created automatically for devices. +``OPENWISP_MONITORING_AUTO_DEVICE_CONFIG_CHECK`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + ++--------------+-------------+ +| **type**: | ``bool`` | ++--------------+-------------+ +| **default**: | ``True`` | ++--------------+-------------+ + +``config_modified`` checks are run peridically to monitor whether device +configuration is applied properly. +This setting allows you to choose whether ``config_modified`` 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`` | ++--------------+-----------+ + +``config_modified`` check changes the device health status to ``PROBLEM`` if it's +configuration status has been in the modified state for more than the time defined in +``OPENWISP_MONITORING_AUTO_DEVICE_CONFIG_CHECK``. It's 5 minutes by default. + +**Note**: If the setting ``OPENWISP_MONITORING_AUTO_DEVICE_CONFIG_CHECK`` +is disabled then this setting need not be declared. + +``OPENWISP_MONITORING_DEVICE_CONFIG_CHECK_RETENTION_POLICY`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + ++--------------+-------------+ +| **type**: | ``time`` | ++--------------+-------------+ +| **default**: | ``48h0m0s`` | ++--------------+-------------+ + +This setting allows to modify the duration for which the metric data generated +by ``config_modified`` check is to be retained. + +**Note**: If the setting ``OPENWISP_MONITORING_AUTO_DEVICE_CONFIG_CHECK`` +is disabled then this setting need not be declared. + ``OPENWISP_MONITORING_AUTO_CHARTS`` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/openwisp_monitoring/check/apps.py b/openwisp_monitoring/check/apps.py index 4bffb7915..5507b5258 100644 --- a/openwisp_monitoring/check/apps.py +++ b/openwisp_monitoring/check/apps.py @@ -1,8 +1,13 @@ from django.apps import AppConfig from django.utils.translation import ugettext_lazy as _ +from .utils import manage_config_modified_retention_policy + class CheckConfig(AppConfig): name = 'openwisp_monitoring.check' label = 'check' verbose_name = _('Network Monitoring Checks') + + def ready(self): + manage_config_modified_retention_policy() diff --git a/openwisp_monitoring/check/base/models.py b/openwisp_monitoring/check/base/models.py index 7bfc053b9..c769b3ab5 100644 --- a/openwisp_monitoring/check/base/models.py +++ b/openwisp_monitoring/check/base/models.py @@ -2,8 +2,9 @@ from django.contrib.contenttypes.fields import GenericForeignKey from django.contrib.contenttypes.models import ContentType -from django.db import models +from django.db import models, transaction from django.db.models.signals import post_save +from django.dispatch import receiver from django.utils.functional import cached_property from django.utils.module_loading import import_string from django.utils.translation import gettext_lazy as _ @@ -76,15 +77,13 @@ def perform_check(self, store=True): if app_settings.AUTO_PING: - from django.db import transaction - from django.dispatch import receiver from openwisp_monitoring.check.tasks import auto_create_ping @receiver(post_save, sender=Device, dispatch_uid='auto_ping') def auto_ping_receiver(sender, instance, created, **kwargs): """ Implements OPENWISP_MONITORING_AUTO_PING - The creation step is executed in the backround + 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 @@ -96,6 +95,28 @@ def auto_ping_receiver(sender, instance, created, **kwargs): model=sender.__name__.lower(), app_label=sender._meta.app_label, object_id=str(instance.pk), - created=created, + ) + ) + + +if app_settings.AUTO_CONFIG_CHECK: + from openwisp_monitoring.check.tasks import auto_create_config_modified + + @receiver(post_save, sender=Device, dispatch_uid='auto_config_modified') + def auto_config_modified_receiver(sender, instance, created, **kwargs): + """ + Implements OPENWISP_MONITORING_AUTO_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_modified.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..020cc3ff8 100644 --- a/openwisp_monitoring/check/classes/__init__.py +++ b/openwisp_monitoring/check/classes/__init__.py @@ -1 +1,2 @@ +from .config_modified import ConfigModified # noqa from .ping import Ping # noqa diff --git a/openwisp_monitoring/check/classes/base.py b/openwisp_monitoring/check/classes/base.py new file mode 100644 index 000000000..a535baf9a --- /dev/null +++ b/openwisp_monitoring/check/classes/base.py @@ -0,0 +1,38 @@ +from django.contrib.contenttypes.models import ContentType +from django.core.exceptions import ValidationError +from swapper import load_model + +from openwisp_controller.config.models import Device + +Check = load_model('check', 'Check') +Metric = load_model('monitoring', 'Metric') + + +class BaseCheck(object): + def validate_instance(self): + # check instance is of type device + obj = self.related_object + if not obj or not isinstance(obj, Device): + message = 'A related device is required to perform this operation' + raise ValidationError({'content_type': message, 'object_id': message}) + + def _get_or_create_metric(self, field_name): + """ + Gets or creates metric + """ + check = self.check_instance + if check.object_id and check.content_type: + obj_id = check.object_id + ct = check.content_type + else: + obj_id = str(check.id) + ct = ContentType.objects.get_for_model(Check) + options = dict( + name=check.name, + object_id=obj_id, + content_type=ct, + field_name=field_name, + key=self.__class__.__name__.lower(), + ) + metric, created = Metric.objects.get_or_create(**options) + return metric, created diff --git a/openwisp_monitoring/check/classes/config_modified.py b/openwisp_monitoring/check/classes/config_modified.py new file mode 100644 index 000000000..773cadab5 --- /dev/null +++ b/openwisp_monitoring/check/classes/config_modified.py @@ -0,0 +1,40 @@ +from swapper import load_model + +from ..settings import CONFIG_CHECK_MAX_TIME +from ..utils import CONFIG_CHECK_RP +from .base import BaseCheck + +AlertSettings = load_model('monitoring', 'AlertSettings') + + +class ConfigModified(BaseCheck): + def __init__(self, check, params): + self.check_instance = check + self.related_object = check.content_object + self.params = params + + def validate(self): + self.validate_instance() + + def check(self, store=True): + if not hasattr(self.related_object, 'config'): + return + result = 0 if self.related_object.config.status == 'applied' else 1 + if store: + self.get_metric().write( + result, retention_policy=CONFIG_CHECK_RP, + ) + return result + + def get_metric(self): + metric, created = self._get_or_create_metric(field_name='config_modified') + if created: + self._create_alert_setting(metric) + return metric + + def _create_alert_setting(self, metric): + alert_s = AlertSettings( + metric=metric, operator='>', value=0, seconds=CONFIG_CHECK_MAX_TIME * 60 + ) + alert_s.full_clean() + alert_s.save() diff --git a/openwisp_monitoring/check/classes/ping.py b/openwisp_monitoring/check/classes/ping.py index 3a46a9b28..6f36331d5 100644 --- a/openwisp_monitoring/check/classes/ping.py +++ b/openwisp_monitoring/check/classes/ping.py @@ -1,26 +1,23 @@ import subprocess -from django.contrib.contenttypes.models import ContentType from django.core.exceptions import ValidationError from jsonschema import draft7_format_checker, validate from jsonschema.exceptions import ValidationError as SchemaError from swapper import load_model -from openwisp_controller.config.models import Device - from ... import settings as monitoring_settings from .. import settings as app_settings from ..exceptions import OperationalError +from .base import BaseCheck Chart = load_model('monitoring', 'Chart') -Check = load_model('check', 'Check') Metric = load_model('monitoring', 'Metric') AlertSettings = load_model('monitoring', 'AlertSettings') -class Ping(object): +class Ping(BaseCheck): schema = { - '$schema': 'http://json-schema.org/draft-04/schema#', + '$schema': 'http://json-schema.org/draft-07/schema#', 'type': 'object', 'additionalProperties': False, 'properties': { @@ -58,13 +55,6 @@ def validate(self): self.validate_instance() self.validate_params() - def validate_instance(self): - # check instance is of type device - obj = self.related_object - if not obj or not isinstance(obj, Device): - message = 'A related device is required ' 'to perform this operation' - raise ValidationError({'content_type': message, 'object_id': message}) - def validate_params(self): try: validate(self.params, self.schema, format_checker=draft7_format_checker) @@ -166,21 +156,7 @@ def _get_metric(self): """ Gets or creates metric """ - check = self.check_instance - if check.object_id and check.content_type: - obj_id = check.object_id - ct = check.content_type - else: - obj_id = str(check.id) - ct = ContentType.objects.get_for_model(Check) - options = dict( - name=check.name, - object_id=obj_id, - content_type=ct, - field_name='reachable', - key=self.__class__.__name__.lower(), - ) - metric, created = Metric.objects.get_or_create(**options) + metric, created = self._get_or_create_metric(field_name='reachable') if created: self._create_alert_settings(metric) self._create_charts(metric) diff --git a/openwisp_monitoring/check/migrations/0001_initial.py b/openwisp_monitoring/check/migrations/0001_initial.py index 360a3dc47..9ad56bb09 100644 --- a/openwisp_monitoring/check/migrations/0001_initial.py +++ b/openwisp_monitoring/check/migrations/0001_initial.py @@ -9,6 +9,8 @@ import collections import swapper +from ..settings import CHECK_CLASSES + class Migration(migrations.Migration): @@ -58,7 +60,7 @@ class Migration(migrations.Migration): ( 'check', models.CharField( - choices=[('openwisp_monitoring.check.classes.Ping', 'Ping')], + choices=CHECK_CLASSES, db_index=True, help_text='Select check type', max_length=128, diff --git a/openwisp_monitoring/check/migrations/0003_create_ping.py b/openwisp_monitoring/check/migrations/0003_create_ping.py index 6eb6058a3..0174988be 100644 --- a/openwisp_monitoring/check/migrations/0003_create_ping.py +++ b/openwisp_monitoring/check/migrations/0003_create_ping.py @@ -11,7 +11,6 @@ def create_device_ping(apps, schema_editor): model=Device.__name__.lower(), app_label=Device._meta.app_label, object_id=str(device.pk), - created=True, ) diff --git a/openwisp_monitoring/check/migrations/0004_create_config_modified.py b/openwisp_monitoring/check/migrations/0004_create_config_modified.py new file mode 100644 index 000000000..6b24f56d3 --- /dev/null +++ b/openwisp_monitoring/check/migrations/0004_create_config_modified.py @@ -0,0 +1,33 @@ +from django.db import migrations +from openwisp_monitoring.check.settings import AUTO_CONFIG_CHECK +from openwisp_monitoring.check.tasks import auto_create_config_modified + + +def add_config_modified_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_modified.delay( + model=Device.__name__.lower(), + app_label=Device._meta.app_label, + object_id=str(device.pk), + ) + + +def remove_config_modified_checks(apps, schema_editor): + Check = apps.get_model('config', 'Device') + Check.objects.filter(name='Configuration Modified').delete() + + +class Migration(migrations.Migration): + + dependencies = [ + ('check', '0003_create_ping'), + ] + + operations = [ + migrations.RunPython( + add_config_modified_checks, reverse_code=remove_config_modified_checks + ), + ] diff --git a/openwisp_monitoring/check/settings.py b/openwisp_monitoring/check/settings.py index ba36fff2c..f40ae5107 100644 --- a/openwisp_monitoring/check/settings.py +++ b/openwisp_monitoring/check/settings.py @@ -3,7 +3,20 @@ CHECK_CLASSES = getattr( settings, 'OPENWISP_MONITORING_CHECK_CLASSES', - (('openwisp_monitoring.check.classes.Ping', 'Ping'),), + ( + ('openwisp_monitoring.check.classes.Ping', 'Ping'), + ('openwisp_monitoring.check.classes.ConfigModified', 'Configuration Modified'), + ), ) 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 +) +CONFIG_CHECK_RETENTION_POLICY = getattr( + settings, 'OPENWISP_MONITORING_DEVICE_CONFIG_CHECK_RETENTION_POLICY', '48h0m0s', +) 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 cf0c2ef3d..fa258c1fb 100644 --- a/openwisp_monitoring/check/tasks.py +++ b/openwisp_monitoring/check/tasks.py @@ -35,18 +35,15 @@ def perform_check(uuid): @shared_task -def auto_create_ping(model, app_label, object_id, created): +def auto_create_ping(model, app_label, object_id): """ Called by openwisp_monitoring.check.models.auto_ping_receiver """ Check = load_model('check', 'Check') ping_path = 'openwisp_monitoring.check.classes.Ping' - has_check = ( - Check.objects.filter( - object_id=object_id, content_type__model='device', check=ping_path - ).count() - > 0 - ) + has_check = Check.objects.filter( + object_id=object_id, content_type__model='device', check=ping_path + ).exists() # create new check only if necessary if has_check: return @@ -54,3 +51,27 @@ def auto_create_ping(model, app_label, object_id, created): 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_modified(model, app_label, object_id): + """ + Called by openwisp_monitoring.check.models.auto_config_modified_receiver + """ + Check = load_model('check', 'Check') + config_modified_path = 'openwisp_monitoring.check.classes.ConfigModified' + has_check = Check.objects.filter( + object_id=object_id, content_type__model='device', check=config_modified_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 Modified', + check=config_modified_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 fc59496db..7c21851e5 100644 --- a/openwisp_monitoring/check/tests/test_models.py +++ b/openwisp_monitoring/check/tests/test_models.py @@ -1,21 +1,39 @@ +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 swapper import load_model +from openwisp_controller.config.models import Device + from ...device.tests import TestDeviceMonitoringMixin -from ..classes import Ping -from ..settings import CHECK_CLASSES +from .. import settings as app_settings +from ..classes import ConfigModified, Ping +from ..tasks import auto_create_config_modified, auto_create_ping Check = load_model('check', 'Check') +Metric = load_model('monitoring', 'Metric') +AlertSettings = load_model('monitoring', 'AlertSettings') class TestModels(TestDeviceMonitoringMixin, TransactionTestCase): - _PING = CHECK_CLASSES[0][0] + _PING = app_settings.CHECK_CLASSES[0][0] + _CONFIG_MODIFIED = app_settings.CHECK_CLASSES[1][0] def test_check_str(self): c = Check(name='Test check') self.assertEqual(str(c), c.name) + def test_check_no_content_type(self): + check = Check( + name='Configuration Modified', + check='openwisp_monitoring.check.classes.ConfigModified', + ) + m = check.check_instance.get_metric() + self.assertEqual(m, Metric.objects.first()) + def test_check_str_with_relation(self): obj = self._create_user() c = Check(name='Check', content_object=obj) @@ -23,39 +41,129 @@ 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 Modified check Class'): + c = Check( + name='Configuration Modified class check', check=self._CONFIG_MODIFIED + ) + self.assertEqual(c.check_class, ConfigModified) 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 Modified check instance'): + c = Check( + name='Configuration Modified class check', + check=self._CONFIG_MODIFIED, + content_object=obj, + params={}, + ) + i = c.check_instance + self.assertIsInstance(i, ConfigModified) + 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 Modified check validation'): + check = Check(name='Ping check', check=self._CONFIG_MODIFIED, 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.get(name='Ping') + self.assertEqual(c1.content_object, d) + self.assertIn('Ping', c1.check) + with self.subTest('Test AUTO_CONFIG_CHECK'): + c2 = Check.objects.get(name='Configuration Modified') + self.assertEqual(c2.content_object, d) + self.assertIn('ConfigModified', 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_device_modified_problem_config_modified(self): + 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.get(name='Configuration Modified') + check.perform_check() + self.assertEqual(Metric.objects.count(), 1) + self.assertEqual(AlertSettings.objects.count(), 1) + m = Metric.objects.first() + self.assertEqual(m.content_object, d) + self.assertEqual(m.key, 'configmodified') + dm = d.monitoring + # Health status should not change as threshold time not crossed + self.assertEqual(dm.status, 'ok') + m.write(1, time=now() - timedelta(minutes=10)) + dm.refresh_from_db() + self.assertEqual(dm.status, 'problem') + + @patch( + 'openwisp_monitoring.device.base.models.app_settings.CRITICAL_DEVICE_METRICS', + [{'key': 'configmodified', 'field_name': 'config_modified'}], + ) + @patch('openwisp_monitoring.check.settings.AUTO_PING', False) + def test_config_modified_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.get(name='Configuration Modified') + 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)) + m.write(1, time=now() - timedelta(minutes=10)) + 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_modified.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) diff --git a/openwisp_monitoring/check/tests/test_ping.py b/openwisp_monitoring/check/tests/test_ping.py index ba9b021a8..00cddc29d 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.get(name='Ping') result = check.perform_check() self.assertEqual(Metric.objects.count(), 1) self.assertEqual(Chart.objects.count(), 3) diff --git a/openwisp_monitoring/check/utils.py b/openwisp_monitoring/check/utils.py index 8c60b3cb0..e6d5709a5 100644 --- a/openwisp_monitoring/check/utils.py +++ b/openwisp_monitoring/check/utils.py @@ -1,6 +1,7 @@ -from __future__ import absolute_import, unicode_literals +from ..db import timeseries_db +from . import settings as app_settings -from .tasks import run_checks +CONFIG_CHECK_RP = 'config_modified' def run_checks_async(): @@ -8,4 +9,14 @@ def run_checks_async(): Calls celery task run_checks is run in a background worker """ + from .tasks import run_checks + run_checks.delay() + + +def manage_config_modified_retention_policy(): + """ + creates or updates the ``config_modified`` retention policy + """ + duration = app_settings.CONFIG_CHECK_RETENTION_POLICY + timeseries_db.create_or_alter_retention_policy(CONFIG_CHECK_RP, duration) diff --git a/openwisp_monitoring/db/backends/influxdb/tests.py b/openwisp_monitoring/db/backends/influxdb/tests.py index f07937475..bab34f642 100644 --- a/openwisp_monitoring/db/backends/influxdb/tests.py +++ b/openwisp_monitoring/db/backends/influxdb/tests.py @@ -3,6 +3,11 @@ from django.core.exceptions import ValidationError from django.test import TestCase from django.utils.timezone import now +from openwisp_monitoring.check.settings import ( + CONFIG_CHECK_RETENTION_POLICY as config_modified_rp_duration, +) +from openwisp_monitoring.check.utils import CONFIG_CHECK_RP as config_modified_rp_name +from openwisp_monitoring.check.utils import manage_config_modified_retention_policy from openwisp_monitoring.device.settings import SHORT_RETENTION_POLICY from openwisp_monitoring.device.utils import SHORT_RP, manage_short_retention_policy from openwisp_monitoring.monitoring.tests import TestMonitoringMixin @@ -172,8 +177,15 @@ def test_get_query_30d(self): def test_retention_policy(self): manage_short_retention_policy() + manage_config_modified_retention_policy() rp = timeseries_db.get_list_retention_policies() - 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) + self.assertEqual(len(rp), 3) + with self.subTest('Test Short retention policy'): + self.assertEqual(rp[1]['name'], SHORT_RP) + self.assertEqual(rp[1]['default'], False) + duration = SHORT_RETENTION_POLICY + self.assertEqual(rp[1]['duration'], duration) + with self.subTest('Test Config Modified retention policy'): + self.assertEqual(rp[2]['name'], config_modified_rp_name) + self.assertEqual(rp[2]['default'], False) + self.assertEqual(rp[2]['duration'], config_modified_rp_duration) diff --git a/openwisp_monitoring/device/apps.py b/openwisp_monitoring/device/apps.py index 343b5d50b..a1681eeb0 100644 --- a/openwisp_monitoring/device/apps.py +++ b/openwisp_monitoring/device/apps.py @@ -2,11 +2,12 @@ from django.core.cache import cache from django.db.models.signals import post_delete, post_save from django.utils.translation import gettext_lazy as _ -from django_netjsonconfig.signals import checksum_requested +from django_netjsonconfig.signals import checksum_requested, config_modified from openwisp_notifications.signals import notify from openwisp_notifications.types import register_notification_type from swapper import load_model +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 @@ -22,6 +23,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): @@ -92,12 +94,12 @@ def trigger_device_recovery_checks(cls, instance, **kwargs): trigger_device_checks.delay(pk=instance.pk) @classmethod - def connect_is_working_changed(self): + def connect_is_working_changed(cls): from openwisp_controller.connection.models import DeviceConnection from openwisp_controller.connection.signals import is_working_changed is_working_changed.connect( - self.is_working_changed_receiver, + cls.is_working_changed_receiver, sender=DeviceConnection, dispatch_uid='is_working_changed_monitoring', ) @@ -162,3 +164,23 @@ 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.get(name='Configuration Modified') + perform_check.delay(check.id) diff --git a/openwisp_monitoring/device/tests/__init__.py b/openwisp_monitoring/device/tests/__init__.py index 77d7ec670..e94420c3b 100644 --- a/openwisp_monitoring/device/tests/__init__.py +++ b/openwisp_monitoring/device/tests/__init__.py @@ -8,6 +8,7 @@ from openwisp_controller.config.models import Config, Device from openwisp_controller.config.tests import CreateConfigTemplateMixin +from ...check.utils import manage_config_modified_retention_policy from ...monitoring.tests import TestMonitoringMixin from ..utils import manage_short_retention_policy @@ -24,6 +25,7 @@ class TestDeviceMonitoringMixin(CreateConfigTemplateMixin, TestMonitoringMixin): def setUpClass(cls): super().setUpClass() manage_short_retention_policy() + manage_config_modified_retention_policy() class DeviceMonitoringTestCase(TestDeviceMonitoringMixin, TestCase): diff --git a/openwisp_monitoring/device/tests/test_models.py b/openwisp_monitoring/device/tests/test_models.py index c672dbcef..6a31c0288 100644 --- a/openwisp_monitoring/device/tests/test_models.py +++ b/openwisp_monitoring/device/tests/test_models.py @@ -3,6 +3,7 @@ from unittest.mock import patch from django.core.exceptions import ObjectDoesNotExist, ValidationError +from django_netjsonconfig.signals import config_modified from openwisp_notifications.signals import notify from swapper import load_model @@ -626,3 +627,20 @@ def test_is_working_none_true(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_modified_path = 'openwisp_monitoring.check.classes.ConfigModified' + Check.objects.create( + name='Configuration Modified', + content_object=c.device, + check=config_modified_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/device/tests/test_recovery.py b/openwisp_monitoring/device/tests/test_recovery.py index 07639308c..d310c7856 100644 --- a/openwisp_monitoring/device/tests/test_recovery.py +++ b/openwisp_monitoring/device/tests/test_recovery.py @@ -40,7 +40,7 @@ def test_trigger_device_recovery_task(self, mocked_method): del data['resources'] del data['interfaces'] auto_create_ping.delay( - model='device', app_label='config', object_id=str(d.pk), created=True, + model='device', app_label='config', object_id=str(d.pk), ) check = Check.objects.first() check.perform_check() diff --git a/openwisp_monitoring/monitoring/base/models.py b/openwisp_monitoring/monitoring/base/models.py index 8c6134bb7..f07a1dd50 100644 --- a/openwisp_monitoring/monitoring/base/models.py +++ b/openwisp_monitoring/monitoring/base/models.py @@ -157,7 +157,15 @@ def check_threshold(self, value, time=None): return 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): @@ -170,6 +178,7 @@ 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, diff --git a/tests/openwisp2/sample_check/migrations/0001_initial.py b/tests/openwisp2/sample_check/migrations/0001_initial.py index 73c803f14..06b3e306b 100644 --- a/tests/openwisp2/sample_check/migrations/0001_initial.py +++ b/tests/openwisp2/sample_check/migrations/0001_initial.py @@ -8,6 +8,8 @@ import model_utils.fields import uuid +from openwisp_monitoring.check.settings import CHECK_CLASSES + class Migration(migrations.Migration): @@ -56,7 +58,7 @@ class Migration(migrations.Migration): ( 'check', models.CharField( - choices=[('openwisp_monitoring.check.classes.Ping', 'Ping')], + choices=CHECK_CLASSES, db_index=True, help_text='Select check type', max_length=128,