From 58539d66c643d624b86389ff02473a898b17bc82 Mon Sep 17 00:00:00 2001 From: nepython Date: Thu, 11 Jun 2020 01:41:24 +0000 Subject: [PATCH] [feature] Add a check which inspects device configuration status periodically #54 --- README.rst | 41 +++++++ openwisp_monitoring/check/apps.py | 5 + openwisp_monitoring/check/base/models.py | 63 +++++++---- openwisp_monitoring/check/classes/__init__.py | 1 + openwisp_monitoring/check/classes/base.py | 39 +++++++ .../check/classes/config_status.py | 69 +++++++++++ openwisp_monitoring/check/classes/ping.py | 34 +----- .../check/migrations/0001_initial.py | 4 +- .../check/migrations/0003_create_ping.py | 1 - .../migrations/0004_create_config_status.py | 33 ++++++ openwisp_monitoring/check/settings.py | 14 ++- openwisp_monitoring/check/tasks.py | 29 ++++- .../check/tests/test_models.py | 107 ++++++++++++++---- openwisp_monitoring/check/tests/test_ping.py | 1 + openwisp_monitoring/check/utils.py | 30 ++++- openwisp_monitoring/device/apps.py | 28 ++++- openwisp_monitoring/device/tests/__init__.py | 2 + .../device/tests/test_models.py | 33 +++++- .../device/tests/test_recovery.py | 2 +- openwisp_monitoring/monitoring/base/models.py | 4 +- .../sample_check/migrations/0001_initial.py | 4 +- 21 files changed, 455 insertions(+), 89 deletions(-) create mode 100644 openwisp_monitoring/check/classes/base.py create mode 100644 openwisp_monitoring/check/classes/config_status.py create mode 100644 openwisp_monitoring/check/migrations/0004_create_config_status.py diff --git a/README.rst b/README.rst index ebacbdabf..c187e9942 100644 --- a/README.rst +++ b/README.rst @@ -242,6 +242,47 @@ in terms of disk space. Whether ping checks are created automatically for devices. +``OPENWISP_MONITORING_AUTO_CONFIG_STATUS`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + ++--------------+-------------+ +| **type**: | ``bool`` | ++--------------+-------------+ +| **default**: | ``True`` | ++--------------+-------------+ + +Whether config_status checks are created automatically for devices. + +``OPENWISP_MONITORING_CONFIG_MODIFIED_MAX_TIME`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + ++--------------+-----------+ +| **type**: | ``float`` | ++--------------+-----------+ +| **default**: | ``5`` | ++--------------+-----------+ + +After ``config`` is ``modified``, if the ``modified`` status does not change after a +fixed **duration** then ``device`` health status changes to ``problem``. +This **duration** can be set with the help of this setting. The input represents the duration in minutes. +Thus, by default the health status of the device changes to ``problem`` after 5 minutes of ``config_modified`` status. + +**Note**: If the setting ``AUTO_CONFIG_STATUS`` is disabled then this setting need not be declared. + +``OPENWISP_MONITORING_CONFIG_STATUS_RETENTION_POLICY`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + ++--------------+-------------+ +| **type**: | ``time`` | ++--------------+-------------+ +| **default**: | ``48h0m0s`` | ++--------------+-------------+ + +This setting allows to modify the duration for which the metric data generated +by ``modified_status`` check is to be retained. + +**Note**: If the setting ``AUTO_CONFIG_STATUS`` is disabled then this setting need not be declared. + ``OPENWISP_MONITORING_AUTO_GRAPHS`` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/openwisp_monitoring/check/apps.py b/openwisp_monitoring/check/apps.py index 4bffb7915..a747cab2a 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_status_retention_policy + class CheckConfig(AppConfig): name = 'openwisp_monitoring.check' label = 'check' verbose_name = _('Network Monitoring Checks') + + def ready(self): + manage_config_status_retention_policy() diff --git a/openwisp_monitoring/check/base/models.py b/openwisp_monitoring/check/base/models.py index 92f1aade1..6900bfd4f 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_status') +def auto_config_status_receiver(sender, instance, created, **kwargs): + """ + Implements OPENWISP_MONITORING_AUTO_CONFIG_STATUS + 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_status + + if not app_settings.AUTO_CONFIG_STATUS or not created: + return + with transaction.atomic(): + transaction.on_commit( + lambda: auto_create_config_status.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..d8502ff27 100644 --- a/openwisp_monitoring/check/classes/__init__.py +++ b/openwisp_monitoring/check/classes/__init__.py @@ -1 +1,2 @@ +from .config_status import ConfigStatus # 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_status.py b/openwisp_monitoring/check/classes/config_status.py new file mode 100644 index 000000000..e782653e2 --- /dev/null +++ b/openwisp_monitoring/check/classes/config_status.py @@ -0,0 +1,69 @@ +from time import time + +from swapper import load_model + +from ...monitoring.utils import write +from ..settings import CONFIG_MODIFIED_MAX_TIME +from ..utils import CONFIG_STATUS_RP +from .base import BaseCheck + +Graph = load_model('monitoring', 'Graph') +Threshold = load_model('monitoring', 'Threshold') + + +class ConfigStatus(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 result and self._check_modified_status_crossed_max_time(): + # TODO: When is the status supposed to be made critical? + dm = self.related_object.monitoring + if dm.status in ['ok', 'unknown']: + dm.update_status('problem') + if store: + metric = self.get_metric() + # TODO: Find why doing the same by adding rp as an arg + # in metric write changes status to problem + write( + name=metric.key, + values={metric.field_name: result}, + tags=metric.tags, + retention_policy=CONFIG_STATUS_RP, + ) + return result + + # TODO: Check if this can be done using threshold + def _check_modified_status_crossed_max_time(self): + # A small margin is kept to take care of border cases + # TODO: Find why `m` --> minute is not working correctly! + since = f'now() - {int(CONFIG_MODIFIED_MAX_TIME*63)}s' + measurement_list = self.get_metric().read( + since=since, limit=None, order='time DESC' + ) + for measurement in measurement_list: + if not measurement['config_status']: + break + minutes_difference = (time() - measurement['time']) / 60 + if minutes_difference >= CONFIG_MODIFIED_MAX_TIME: + return True + return False + + def get_metric(self): + metric, created = self._get_or_create_metric(field_name='config_status') + if created: + self._create_threshold(metric) + return metric + + def _create_threshold(self, metric): + t = Threshold(metric=metric, operator='>', value=0, seconds=0) + t.full_clean() + t.save() diff --git a/openwisp_monitoring/check/classes/ping.py b/openwisp_monitoring/check/classes/ping.py index 94cbb167f..8cbb56c1d 100644 --- a/openwisp_monitoring/check/classes/ping.py +++ b/openwisp_monitoring/check/classes/ping.py @@ -1,25 +1,22 @@ 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 Graph = load_model('monitoring', 'Graph') -Metric = load_model('monitoring', 'Metric') Threshold = load_model('monitoring', 'Threshold') -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 +54,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 +155,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_threshold(metric) self._create_graphs(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_status.py b/openwisp_monitoring/check/migrations/0004_create_config_status.py new file mode 100644 index 000000000..6e8a2974b --- /dev/null +++ b/openwisp_monitoring/check/migrations/0004_create_config_status.py @@ -0,0 +1,33 @@ +from django.db import migrations +from openwisp_monitoring.check.settings import AUTO_CONFIG_STATUS +from openwisp_monitoring.check.tasks import auto_create_config_status + + +def add_config_status_checks(apps, schema_editor): + if not AUTO_CONFIG_STATUS: + return + Device = apps.get_model('config', 'Device') + for device in Device.objects.all(): + auto_create_config_status.delay( + model=Device.__name__.lower(), + app_label=Device._meta.app_label, + object_id=str(device.pk), + ) + + +def remove_config_status_checks(apps, schema_editor): + Check = apps.get_model('config', 'Device') + Check.objects.filter(name='Config Status').delete() + + +class Migration(migrations.Migration): + + dependencies = [ + ('check', '0003_create_ping'), + ] + + operations = [ + migrations.RunPython( + add_config_status_checks, reverse_code=remove_config_status_checks + ), + ] diff --git a/openwisp_monitoring/check/settings.py b/openwisp_monitoring/check/settings.py index ba36fff2c..a2f01e30a 100644 --- a/openwisp_monitoring/check/settings.py +++ b/openwisp_monitoring/check/settings.py @@ -3,7 +3,19 @@ CHECK_CLASSES = getattr( settings, 'OPENWISP_MONITORING_CHECK_CLASSES', - (('openwisp_monitoring.check.classes.Ping', 'Ping'),), + ( + ('openwisp_monitoring.check.classes.Ping', 'Ping'), + ('openwisp_monitoring.check.classes.ConfigStatus', 'Config Status'), + ), ) AUTO_PING = getattr(settings, 'OPENWISP_MONITORING_AUTO_PING', True) +AUTO_CONFIG_STATUS = getattr(settings, 'OPENWISP_MONITORING_AUTO_CONFIG_STATUS', True) +# Input in minutes +CONFIG_MODIFIED_MAX_TIME = getattr( + settings, 'OPENWISP_MONITORING_CONFIG_MODIFIED_MAXIMUM_TIME', 5 +) +# Input in days +CONFIG_STATUS_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..624057197 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,30 @@ 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_status(model, app_label, object_id): + """ + Called by openwisp_monitoring.check.models.auto_config_status_receiver + """ + Check = load_model('check', 'Check') + config_status_path = 'openwisp_monitoring.check.classes.ConfigStatus' + has_check = ( + Check.objects.filter( + object_id=object_id, content_type__model='device', check=config_status_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 Status', + check=config_status_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..f8782bb19 100644 --- a/openwisp_monitoring/check/tests/test_models.py +++ b/openwisp_monitoring/check/tests/test_models.py @@ -1,16 +1,24 @@ +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 ConfigStatus, Ping Check = load_model('check', 'Check') +DeviceData = load_model('device_monitoring', 'DeviceData') +Metric = load_model('monitoring', 'Metric') +Threshold = load_model('monitoring', 'Threshold') class TestModels(TestDeviceMonitoringMixin, TransactionTestCase): - _PING = CHECK_CLASSES[0][0] + _PING = app_settings.CHECK_CLASSES[0][0] + _CONFIG_STATUS = app_settings.CHECK_CLASSES[1][0] def test_check_str(self): c = Check(name='Test check') @@ -23,28 +31,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 Status check Class'): + c = Check(name='Config Status class check', check=self._CONFIG_STATUS) + self.assertEqual(c.check_class, ConfigStatus) 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 Status check instance'): + c = Check( + name='Config Status class check', + check=self._CONFIG_STATUS, + content_object=obj, + params={}, + ) + i = c.check_instance + self.assertIsInstance(i, ConfigStatus) + 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 Status check validation'): + check = Check(name='Ping check', check=self._CONFIG_STATUS, 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_STATUS', False) def test_auto_ping(self): self.assertEqual(Check.objects.count(), 0) d = self._create_device(organization=self._create_org()) @@ -53,9 +87,42 @@ 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_status(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('ConfigStatus', 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_status_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_status + self.assertEqual(Check.objects.count(), 1) + self.assertEqual(Metric.objects.count(), 0) + self.assertEqual(Threshold.objects.count(), 0) + check = Check.objects.first() + check.perform_check() + self.assertEqual(Metric.objects.count(), 1) + self.assertEqual(Threshold.objects.count(), 1) + m = Metric.objects.first() + self.assertEqual(m.content_object, dd) + self.assertEqual(m.key, 'configstatus') + dm = dd.monitoring + # Health status should not change as threshold time not crossed + self.assertEqual(dm.status, 'unknown') + m.write(1, time=now() - timedelta(minutes=5)) + check.perform_check() + dm.refresh_from_db() + self.assertEqual(dm.status, 'problem') diff --git a/openwisp_monitoring/check/tests/test_ping.py b/openwisp_monitoring/check/tests/test_ping.py index 7bb33288a..522cf9d82 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_STATUS', 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..716067578 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 get_db +from . import settings as app_settings -from .tasks import run_checks +CONFIG_STATUS_RP = 'config_status' def run_checks_async(): @@ -8,4 +9,29 @@ def run_checks_async(): Calls celery task run_checks is run in a background worker """ + from .tasks import run_checks + run_checks.delay() + + +# TODO: Avoid duplication, create a method to create rp (args: {duration, name}) +def manage_config_status_retention_policy(): + """ + creates or updates the ``config_status`` retention policy + """ + db = get_db() + duration = app_settings.CONFIG_STATUS_RETENTION_POLICY + retention_policies = db.get_list_retention_policies() + exists = False + duration_changed = False + for policy in retention_policies: + if policy['name'] == CONFIG_STATUS_RP: + exists = True + duration_changed = policy['duration'] + break + if not exists: + db.create_retention_policy( + name=CONFIG_STATUS_RP, duration=duration, replication=1 + ) + elif exists and duration_changed: + db.alter_retention_policy(name=CONFIG_STATUS_RP, duration=duration) diff --git a/openwisp_monitoring/device/apps.py b/openwisp_monitoring/device/apps.py index 930d72611..9ba572c47 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_STATUS: + config_modified.connect( + cls.config_status_modified_receiver, + sender=Config, + dispatch_uid='config_modified', + ) + + @classmethod + def config_status_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 Status') + perform_check.delay(check.id) diff --git a/openwisp_monitoring/device/tests/__init__.py b/openwisp_monitoring/device/tests/__init__.py index c16a4d5a6..471a45dba 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_status_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_status_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 d0f13d738..e11d6475c 100644 --- a/openwisp_monitoring/device/tests/test_models.py +++ b/openwisp_monitoring/device/tests/test_models.py @@ -3,11 +3,14 @@ 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_STATUS_RETENTION_POLICY +from ...check.utils import CONFIG_STATUS_RP from ...monitoring.utils import get_db from .. import settings as app_settings from ..signals import health_status_changed @@ -306,11 +309,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 Status retention policy'): + self.assertEqual(rp[2]['name'], CONFIG_STATUS_RP) + self.assertEqual(rp[2]['default'], False) + self.assertEqual(rp[2]['duration'], CONFIG_STATUS_RETENTION_POLICY) def test_device_deleted(self): d = self._create_device() @@ -644,3 +652,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_status_path = 'openwisp_monitoring.check.classes.ConfigStatus' + Check.objects.create( + name='Config Status', content_object=c.device, check=config_status_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/monitoring/base/models.py b/openwisp_monitoring/monitoring/base/models.py index 9736cfaa8..3612e27f0 100644 --- a/openwisp_monitoring/monitoring/base/models.py +++ b/openwisp_monitoring/monitoring/base/models.py @@ -146,7 +146,9 @@ def check_threshold(self, value, time=None): ) self._notify_users(notification_type, threshold) - 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, + ): """ write timeseries data """ values = {self.field_name: value} if extra_values and isinstance(extra_values, dict): 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,