diff --git a/README.rst b/README.rst index a4a34188a..e04fbe77e 100644 --- a/README.rst +++ b/README.rst @@ -247,6 +247,47 @@ in terms of disk space. Whether ping checks are created automatically for devices. +``OPENWISP_MONITORING_AUTO_CONFIG_MODIFIED`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + ++--------------+-------------+ +| **type**: | ``bool`` | ++--------------+-------------+ +| **default**: | ``True`` | ++--------------+-------------+ + +Whether ``config_modified`` checks are created automatically for devices. + +``OPENWISP_MONITORING_CONFIG_MODIFIED_MAX_TIME`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + ++--------------+-----------+ +| **type**: | ``int`` | ++--------------+-----------+ +| **default**: | ``5`` | ++--------------+-----------+ + +After ``config`` is ``modified``, if the ``modified`` status does not change after a +fixed **duration** then ``device`` health status shall change to ``problem``. +This fixed **duration** is 5 minutes by defaut and can be changed with the help of this setting. +The input represents corresponding duration in minutes. + +**Note**: If the setting ``AUTO_CONFIG_STATUS`` is disabled then this setting need not be declared. + +``OPENWISP_MONITORING_CONFIG_MODIFIED_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 ``AUTO_CONFIG_MODIFIED`` 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 92f1aade1..7cfad176b 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 ugettext_lazy as _ @@ -75,27 +76,45 @@ def perform_check(self, store=True): return self.check_instance.check(store=True) -if app_settings.AUTO_PING: - from django.db import transaction - from django.dispatch import receiver +@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 background + """ + # we need to skip this otherwise this task will be executed + # every time the configuration is requested via checksum 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 - """ - # 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_ping.delay( - model=sender.__name__.lower(), - app_label=sender._meta.app_label, - object_id=str(instance.pk), - created=created, - ) + if not app_settings.AUTO_PING or not created: + return + with transaction.atomic(): + transaction.on_commit( + lambda: auto_create_ping.delay( + model=sender.__name__.lower(), + app_label=sender._meta.app_label, + object_id=str(instance.pk), + ) + ) + + +@receiver(post_save, sender=Device, dispatch_uid='auto_config_modified') +def auto_config_modified_receiver(sender, instance, created, **kwargs): + """ + Implements OPENWISP_MONITORING_AUTO_CONFIG_MODIFIED + 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 + from openwisp_monitoring.check.tasks import auto_create_config_modified + + if not app_settings.AUTO_CONFIG_MODIFIED or 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..4926f4c87 --- /dev/null +++ b/openwisp_monitoring/check/classes/base.py @@ -0,0 +1,39 @@ +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 + +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( + app_label=check._meta.app_label, model=check.__class__.__name__.lower() + ) + 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..1a202b7f3 --- /dev/null +++ b/openwisp_monitoring/check/classes/config_modified.py @@ -0,0 +1,40 @@ +from swapper import load_model + +from ..settings import CONFIG_MODIFIED_MAX_TIME +from ..utils import CONFIG_MODIFIED_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_MODIFIED_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_MODIFIED_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 2668ecc13..6f36331d5 100644 --- a/openwisp_monitoring/check/classes/ping.py +++ b/openwisp_monitoring/check/classes/ping.py @@ -1,25 +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') 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': { @@ -57,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) @@ -165,23 +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( - app_label=check._meta.app_label, model=check.__class__.__name__.lower() - ) - 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..0a37bd584 --- /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_MODIFIED +from openwisp_monitoring.check.tasks import auto_create_config_modified + + +def add_config_modified_checks(apps, schema_editor): + if not AUTO_CONFIG_MODIFIED: + 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='Config 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..edf355b67 100644 --- a/openwisp_monitoring/check/settings.py +++ b/openwisp_monitoring/check/settings.py @@ -3,7 +3,21 @@ 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', 'Config Modified'), + ), ) AUTO_PING = getattr(settings, 'OPENWISP_MONITORING_AUTO_PING', True) +AUTO_CONFIG_MODIFIED = getattr( + settings, 'OPENWISP_MONITORING_AUTO_CONFIG_MODIFIED', True +) +# Input in minutes +CONFIG_MODIFIED_MAX_TIME = getattr( + settings, 'OPENWISP_MONITORING_CONFIG_MODIFIED_MAXIMUM_TIME', 5 +) +# Input in days +CONFIG_MODIFIED_RETENTION_POLICY = getattr( + settings, 'OPENWISP_MONITORING_CONFIG_STATUS_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..791d2feaf 100644 --- a/openwisp_monitoring/check/tasks.py +++ b/openwisp_monitoring/check/tasks.py @@ -35,7 +35,7 @@ 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 """ @@ -54,3 +54,32 @@ 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, + ).count() + > 0 + ) + # create new check only if necessary + if has_check: + return + ct = ContentType.objects.get(app_label=app_label, model=model) + check = Check( + name='Config 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..d226e4792 100644 --- a/openwisp_monitoring/check/tests/test_models.py +++ b/openwisp_monitoring/check/tests/test_models.py @@ -1,21 +1,37 @@ +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 ...device.tests import TestDeviceMonitoringMixin -from ..classes import Ping -from ..settings import CHECK_CLASSES +from .. import settings as app_settings +from ..classes import ConfigModified, Ping Check = load_model('check', 'Check') +DeviceData = load_model('device_monitoring', 'DeviceData') +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='Config 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,28 +39,54 @@ 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 Config Modified check Class'): + c = Check(name='Config 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 Config Modified check instance'): + c = Check( + name='Config 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') + 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 Config 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') + @patch('openwisp_monitoring.check.settings.AUTO_CONFIG_MODIFIED', False) def test_auto_ping(self): self.assertEqual(Check.objects.count(), 0) d = self._create_device(organization=self._create_org()) @@ -53,9 +95,61 @@ def test_auto_ping(self): self.assertEqual(c.content_object, d) self.assertIn('Ping', c.check) - def test_device_deleted(self): + @patch('openwisp_monitoring.check.settings.AUTO_PING', False) + def test_auto_config_modified(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('ConfigModified', c.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(), 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()) + dd = DeviceData.objects.first() + # check created automatically by auto_config_modified + self.assertEqual(Check.objects.count(), 1) + self.assertEqual(Metric.objects.count(), 0) + self.assertEqual(AlertSettings.objects.count(), 0) + check = Check.objects.first() + check.perform_check() + self.assertEqual(Metric.objects.count(), 1) + self.assertEqual(AlertSettings.objects.count(), 1) + m = Metric.objects.first() + self.assertEqual(m.content_object, dd) + self.assertEqual(m.key, 'configmodified') + dm = dd.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(), 1) + dd = DeviceData.objects.first() + dm = dd.monitoring + check = Check.objects.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)) + m.write(1, time=now() - timedelta(minutes=10)) + dm.refresh_from_db() + self.assertEqual(dm.status, 'critical') diff --git a/openwisp_monitoring/check/tests/test_ping.py b/openwisp_monitoring/check/tests/test_ping.py index 11af8fb18..ef0bb69ae 100644 --- a/openwisp_monitoring/check/tests/test_ping.py +++ b/openwisp_monitoring/check/tests/test_ping.py @@ -193,6 +193,7 @@ def test_schema_violation(self): else: self.fail('ValidationError not raised') + @patch.object(settings, 'AUTO_CONFIG_MODIFIED', False) @patch.object(Ping, '_command', return_value=_FPING_OUTPUT) def test_store_result(self, mocked_method): self.assertEqual(Check.objects.count(), 0) diff --git a/openwisp_monitoring/check/utils.py b/openwisp_monitoring/check/utils.py index 8c60b3cb0..6b5e7af16 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 ..monitoring.utils import create_retention_policy +from . import settings as app_settings -from .tasks import run_checks +CONFIG_MODIFIED_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_MODIFIED_RETENTION_POLICY + create_retention_policy(CONFIG_MODIFIED_RP, duration) diff --git a/openwisp_monitoring/device/apps.py b/openwisp_monitoring/device/apps.py index 930d72611..48c6eb100 100644 --- a/openwisp_monitoring/device/apps.py +++ b/openwisp_monitoring/device/apps.py @@ -2,10 +2,11 @@ from django.core.cache import cache from django.db.models.signals import post_delete, post_save from django.utils.translation import ugettext_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 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 @@ -20,6 +21,7 @@ def ready(self): manage_short_retention_policy() self.connect_is_working_changed() self.connect_device_signals() + self.connect_config_modified() self.device_recovery_detection() def connect_device_signals(self): @@ -90,12 +92,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', ) @@ -133,3 +135,23 @@ def is_working_changed_receiver(cls, instance, is_working, **kwargs): device_monitoring.update_status(status) opts['data'] = {'email_subject': f'[{status.upper()}] {device}'} notify.send(**opts) + + @classmethod + def connect_config_modified(cls): + from openwisp_controller.config.models import Config + + if check_settings.AUTO_CONFIG_MODIFIED: + 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='Config Modified') + perform_check.delay(check.id) diff --git a/openwisp_monitoring/device/tests/__init__.py b/openwisp_monitoring/device/tests/__init__.py index defedafae..429b51ce2 100644 --- a/openwisp_monitoring/device/tests/__init__.py +++ b/openwisp_monitoring/device/tests/__init__.py @@ -7,6 +7,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 @@ -23,6 +24,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 e063d4c64..ad8c404db 100644 --- a/openwisp_monitoring/device/tests/test_models.py +++ b/openwisp_monitoring/device/tests/test_models.py @@ -3,11 +3,16 @@ from unittest.mock import patch from django.core.exceptions import ObjectDoesNotExist, ValidationError +from django_netjsonconfig.signals import config_modified from swapper import load_model from openwisp_controller.connection.models import Credentials, DeviceConnection from openwisp_utils.tests import catch_signal +from ...check.settings import ( + CONFIG_MODIFIED_RETENTION_POLICY as config_modified_rp_duration, +) +from ...check.utils import CONFIG_MODIFIED_RP as config_modified_rp_name from ...monitoring.utils import get_db from .. import settings as app_settings from ..signals import health_status_changed @@ -306,11 +311,16 @@ def test_init(self): def test_retention_policy(self): rp = get_db().get_list_retention_policies() - self.assertEqual(len(rp), 2) - self.assertEqual(rp[1]['name'], SHORT_RP) - self.assertEqual(rp[1]['default'], False) - duration = app_settings.SHORT_RETENTION_POLICY - self.assertEqual(rp[1]['duration'], duration) + 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 = app_settings.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) def test_device_deleted(self): d = self._create_device() @@ -646,3 +656,18 @@ def test_is_working_true_false(self, mocked_call): dc.is_working = False dc.save() mocked_call.assert_called_once() + + @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='Config 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 ac9896199..d224724bd 100644 --- a/openwisp_monitoring/device/tests/test_recovery.py +++ b/openwisp_monitoring/device/tests/test_recovery.py @@ -33,7 +33,7 @@ def test_trigger_device_recovery_task(self): d.save() data = self._data() 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/device/utils.py b/openwisp_monitoring/device/utils.py index be63538e4..e5fb00949 100644 --- a/openwisp_monitoring/device/utils.py +++ b/openwisp_monitoring/device/utils.py @@ -1,4 +1,4 @@ -from ..monitoring.utils import get_db +from ..monitoring.utils import create_retention_policy from . import settings as app_settings SHORT_RP = 'short' @@ -12,17 +12,5 @@ def manage_short_retention_policy(): """ creates or updates the "short" retention policy """ - db = get_db() duration = app_settings.SHORT_RETENTION_POLICY - retention_policies = db.get_list_retention_policies() - exists = False - duration_changed = False - for policy in retention_policies: - if policy['name'] == SHORT_RP: - exists = True - duration_changed = policy['duration'] - break - if not exists: - db.create_retention_policy(name=SHORT_RP, duration=duration, replication=1) - elif exists and duration_changed: - db.alter_retention_policy(name=SHORT_RP, duration=duration) + create_retention_policy(SHORT_RP, duration) diff --git a/openwisp_monitoring/monitoring/base/models.py b/openwisp_monitoring/monitoring/base/models.py index 38d79c419..14f6bb127 100644 --- a/openwisp_monitoring/monitoring/base/models.py +++ b/openwisp_monitoring/monitoring/base/models.py @@ -149,7 +149,15 @@ def check_threshold(self, value, time=None): ) 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): @@ -162,6 +170,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/openwisp_monitoring/monitoring/utils.py b/openwisp_monitoring/monitoring/utils.py index a7cebb4a2..68db40674 100644 --- a/openwisp_monitoring/monitoring/utils.py +++ b/openwisp_monitoring/monitoring/utils.py @@ -60,3 +60,20 @@ def create_database(): if settings.INFLUXDB_DATABASE not in databases: db.create_database(settings.INFLUXDB_DATABASE) logger.info(f'Created influxdb database {settings.INFLUXDB_DATABASE}') + + +def create_retention_policy(name, duration): + """ creates or alters existing retention policy if necessary """ + db = get_db() + retention_policies = db.get_list_retention_policies() + exists = False + duration_changed = False + for policy in retention_policies: + if policy['name'] == name: + exists = True + duration_changed = policy['duration'] + break + if not exists: + db.create_retention_policy(name=name, duration=duration, replication=1) + elif exists and duration_changed: + db.alter_retention_policy(name=name, duration=duration) 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,