[chores:refactor] Improved auto check logic

- Makes it easy to create new checks without having to write
  the auto check logic
- Improves consistency

Author: nemesifier

openwisp_monitoring/check/base/models.py

@@ -8,11 +8,11 @@
 from django.utils.translation import gettext_lazy as _
 from jsonfield import JSONField
 
-from openwisp_monitoring.check import settings as app_settings
-from openwisp_monitoring.check.tasks import auto_create_check
 from openwisp_utils.base import TimeStampedEditableModel
 
 from ...utils import transaction_on_commit
+from .. import settings as app_settings
+from ..tasks import auto_create_check
 
 
 class AbstractCheck(TimeStampedEditableModel):
@@ -33,7 +33,7 @@ class AbstractCheck(TimeStampedEditableModel):
     content_object = GenericForeignKey('content_type', 'object_id')
     check_type = models.CharField(
         _('check type'),
-        choices=app_settings.CHECK_CLASSES,
+        choices=app_settings.CHECK_CHOICES,
         db_index=True,
         max_length=128,
     )
@@ -114,43 +114,14 @@ def _auto_check_receiver(sender, instance, **kwargs):
     model = sender.__name__.lower()
     app_label = sender._meta.app_label
     object_id = str(instance.pk)
-    if app_settings.AUTO_PING:
-        auto_create_check.delay(
-            model=model,
-            app_label=app_label,
-            object_id=object_id,
-            check_type='openwisp_monitoring.check.classes.Ping',
-            check_name='Ping',
-        )
-    if app_settings.AUTO_CONFIG_CHECK:
-        auto_create_check.delay(
-            model=model,
-            app_label=app_label,
-            object_id=object_id,
-            check_type='openwisp_monitoring.check.classes.ConfigApplied',
-            check_name='Configuration Applied',
-        )
-    if app_settings.AUTO_IPERF3:
-        auto_create_check.delay(
-            model=model,
-            app_label=app_label,
-            object_id=object_id,
-            check_type='openwisp_monitoring.check.classes.Iperf3',
-            check_name='Iperf3',
-        )
-    if app_settings.AUTO_WIFI_CLIENTS_CHECK:
-        auto_create_check.delay(
-            model=model,
-            app_label=app_label,
-            object_id=object_id,
-            check_type='openwisp_monitoring.check.classes.WifiClients',
-            check_name='WiFi Clients',
-        )
-    if app_settings.AUTO_DATA_COLLECTED_CHECK:
+
+    for class_string, name, auto_create_setting in app_settings.CHECK_CLASSES:
+        if not getattr(app_settings, auto_create_setting):
+            continue
         auto_create_check.delay(
             model=model,
             app_label=app_label,
             object_id=object_id,
-            check_type='openwisp_monitoring.check.classes.DataCollected',
-            check_name='Monitoring Data Collected',
+            check_type=class_string,
+            check_name=name,
         )

openwisp_monitoring/check/migrations/0001_initial_squashed_0002_check_unique_together.py

@@ -10,7 +10,7 @@
 import swapper
 from django.db import migrations, models
 
-from ..settings import CHECK_CLASSES
+from ..settings import CHECK_CHOICES
 
 
 class Migration(migrations.Migration):
@@ -58,7 +58,7 @@ class Migration(migrations.Migration):
                 (
                     'check',
                     models.CharField(
-                        choices=CHECK_CLASSES,
+                        choices=CHECK_CHOICES,
                         db_index=True,
                         help_text='Select check type',
                         max_length=128,

openwisp_monitoring/check/migrations/0004_rename_active_to_is_active.py

@@ -2,7 +2,7 @@
 
 from django.db import migrations, models
 
-from ..settings import CHECK_CLASSES
+from ..settings import CHECK_CHOICES
 
 
 class Migration(migrations.Migration):
@@ -26,7 +26,7 @@ class Migration(migrations.Migration):
             model_name='check',
             name='check',
             field=models.CharField(
-                choices=CHECK_CLASSES,
+                choices=CHECK_CHOICES,
                 db_index=True,
                 max_length=128,
                 verbose_name='check type',

openwisp_monitoring/check/settings.py

@@ -1,20 +1,37 @@
 from django.conf import settings
+from django.utils.translation import gettext_lazy as _
 
 from ..settings import get_settings_value
 
 CHECK_CLASSES = get_settings_value(
     'CHECK_CLASSES',
     (
-        ('openwisp_monitoring.check.classes.Ping', 'Ping'),
-        ('openwisp_monitoring.check.classes.ConfigApplied', 'Configuration Applied'),
-        ('openwisp_monitoring.check.classes.Iperf3', 'Iperf3'),
-        ('openwisp_monitoring.check.classes.WifiClients', 'WiFi Clients'),
+        ('openwisp_monitoring.check.classes.Ping', _('Ping'), 'AUTO_PING'),
+        (
+            'openwisp_monitoring.check.classes.ConfigApplied',
+            _('Configuration Applied'),
+            'AUTO_CONFIG_CHECK',
+        ),
+        ('openwisp_monitoring.check.classes.Iperf3', 'Iperf3', 'AUTO_IPERF3'),
+        (
+            'openwisp_monitoring.check.classes.WifiClients',
+            'WiFi Clients',
+            'AUTO_WIFI_CLIENTS_CHECK',
+        ),
         (
             'openwisp_monitoring.check.classes.DataCollected',
             'Monitoring Data Collected',
+            'AUTO_DATA_COLLECTED_CHECK',
         ),
     ),
 )
+
+CHECK_CHOICES = []
+CHECK_LIST = []
+for class_string, name, setting_name in CHECK_CLASSES:
+    CHECK_CHOICES.append((class_string, name))
+    CHECK_LIST.append(class_string)
+
 AUTO_PING = get_settings_value('AUTO_PING', True)
 AUTO_CONFIG_CHECK = get_settings_value('AUTO_DEVICE_CONFIG_CHECK', True)
 # If OPENWISP_MONITORING_MANAGEMENT_IP_ONLY is not configured, use
@@ -44,7 +61,6 @@
     'IPERF3_CHECK_LOCK_EXPIRE', 10 * 60
 )  # 10 minutes arbitrarily chosen (must be longer than TCP + UDP test time)
 IPERF3_CHECK_DELETE_RSA_KEY = get_settings_value('IPERF3_CHECK_DELETE_RSA_KEY', True)
-CHECKS_LIST = get_settings_value('CHECK_LIST', list(dict(CHECK_CLASSES).keys()))
 CONFIG_CHECK_INTERVAL = int(
     get_settings_value('CONFIG_CHECK_INTERVAL', 5)
 )  # in minutes

openwisp_monitoring/check/tasks.py

@@ -10,7 +10,7 @@
 
 from openwisp_utils.tasks import OpenwispCeleryTask
 
-from .settings import CHECKS_LIST
+from . import settings as app_settings
 
 logger = logging.getLogger(__name__)
 
@@ -31,15 +31,15 @@ def run_checks(checks=None):
     """
     # If checks is None, We should execute all the checks
     if checks is None:
-        checks = CHECKS_LIST
+        checks = app_settings.CHECK_LIST
 
     if not isinstance(checks, list):
         raise ImproperlyConfigured(
             f'Check path {checks} should be of type "list"'
         )  # pragma: no cover
-    if not all(check_path in CHECKS_LIST for check_path in checks):
+    if not all(check_path in app_settings.CHECK_LIST for check_path in checks):
         raise ImproperlyConfigured(
-            f'Check path {checks} should be in {CHECKS_LIST}'
+            f'Check path {checks} should be listed in CHECK_CLASSES'
         )  # pragma: no cover
 
     runnable_checks = []

openwisp_monitoring/check/tests/test_ping.py

@@ -7,7 +7,7 @@
 from ... import settings as monitoring_settings
 from ...device import settings as device_settings
 from ...device.tests import TestDeviceMonitoringMixin
-from .. import settings
+from .. import settings as app_settings
 from ..classes import Ping
 from ..classes.ping import get_ping_schema
 from ..exceptions import OperationalError
@@ -20,7 +20,7 @@
 
 
 class TestPing(TestDeviceMonitoringMixin, TransactionTestCase):
-    _PING = settings.CHECK_CLASSES[0][0]
+    _PING = app_settings.CHECK_CLASSES[0][0]
     _RESULT_KEYS = ['reachable', 'loss', 'rtt_min', 'rtt_avg', 'rtt_max']
     _RTT_KEYS = _RESULT_KEYS[-3:]
     _UNRECOGNIZED_OUTPUT = (
@@ -63,7 +63,7 @@ def test_check_ping_params(self):
             self.assertTrue(result[key] < 1)
 
     @patch.object(
-        settings,
+        app_settings,
         'PING_CHECK_CONFIG',
         {
             'timeout': {'default': '10000'},

openwisp_monitoring/check/tests/test_utils.py

@@ -9,7 +9,6 @@
 from .. import settings as app_settings
 from ..checks import check_wifi_clients_snooze_schedule
 from ..classes import Ping
-from ..settings import CHECK_CLASSES
 from ..tasks import perform_check
 from ..utils import run_checks_async
 from . import _FPING_REACHABLE
@@ -18,7 +17,7 @@
 
 
 class TestUtils(TestDeviceMonitoringMixin, TransactionTestCase):
-    _PING = CHECK_CLASSES[0][0]
+    _PING = app_settings.CHECK_CLASSES[0][0]
 
     def _create_check(self):
         device = self._create_device(organization=self._create_org())

openwisp_monitoring/check/tests/test_wifi_client.py

@@ -8,7 +8,6 @@
 from swapper import load_model
 
 from ...device.tests import TestDeviceMonitoringMixin
-from .. import settings
 from .. import settings as app_settings
 from .. import tasks
 from ..classes import WifiClients
@@ -26,7 +25,7 @@ class TestWifiClient(
     TestDeviceMonitoringMixin,
     TransactionTestCase,
 ):
-    _WIFI_CLIENTS = settings.CHECK_CLASSES[3][0]
+    _WIFI_CLIENTS = app_settings.CHECK_CLASSES[3][0]
 
     def _run_wifi_clients_checks(self):
         tasks.run_checks(checks=[self._WIFI_CLIENTS])

openwisp_monitoring/device/base/models.py

@@ -25,7 +25,7 @@
 from openwisp_controller.config.validators import mac_address_validator
 from openwisp_utils.base import TimeStampedEditableModel
 
-from ...check.settings import CHECKS_LIST
+from ...check import settings as check_settings
 from ...db import device_data_query, timeseries_db
 from ...monitoring.signals import threshold_crossed
 from ...monitoring.tasks import _timeseries_write
@@ -392,7 +392,7 @@ def get_active_metrics(cls):
         """
         if not hasattr(cls, '_active_metrics'):
             active_metrics = []
-            for check in CHECKS_LIST:
+            for check in check_settings.CHECK_LIST:
                 Check = import_string(check)
                 active_metrics.extend(Check.get_related_metrics())
             cls._active_metrics = active_metrics

tests/openwisp2/sample_check/migrations/0001_initial.py

@@ -9,7 +9,7 @@
 import model_utils.fields
 from django.db import migrations, models
 
-from openwisp_monitoring.check.settings import CHECK_CLASSES
+from openwisp_monitoring.check.settings import CHECK_CHOICES
 
 
 class Migration(migrations.Migration):
@@ -66,7 +66,7 @@ class Migration(migrations.Migration):
                 (
                     'check_type',
                     models.CharField(
-                        choices=CHECK_CLASSES,
+                        choices=CHECK_CHOICES,
                         db_index=True,
                         max_length=128,
                         verbose_name='check type',