[change] Notify users of background subnet division rule errors #837

openwisp_controller/subnet_division/rule_types/base.py

@@ -7,6 +7,7 @@
 from django.dispatch import Signal
 from django.utils.translation import gettext_lazy as _
 from netaddr import IPNetwork
+from openwisp_notifications.signals import notify
 from swapper import load_model
 
 from ..signals import subnet_provisioned
@@ -204,7 +205,22 @@ def create_subnets(config, division_rule, max_subnet, generated_indexes):
 
         for subnet_id in range(1, division_rule.number_of_subnets + 1):
             if not ip_network(str(required_subnet)).subnet_of(master_subnet.subnet):
-                logger.error(f'Cannot create more subnets of {master_subnet}')
+                notify.send(
+                    sender=config,
+                    type='generic_message',
+                    target=config.device,
+                    action_object=master_subnet,
+                    level='error',
+                    message=_(
+                        'Failed to provision subnets for'
+                        ' [{notification.target}]({notification.target_link})'
+                    ),
+                    description=_(
+                        'The [{notification.action_object}]({notification.action_link})'
+                        ' subnet has run out of space.'
+                    ),
+                )
+                logger.info(f'Cannot create more subnets of {master_subnet}')
                 break
             subnet_obj = Subnet(
                 name=f'{division_rule.label}_subnet{subnet_id}',

openwisp_controller/subnet_division/tests/test_models.py

@@ -26,9 +26,13 @@
 VpnClient = load_model('config', 'VpnClient')
 Device = load_model('config', 'Device')
 OrganizationConfigSettings = load_model('config', 'OrganizationConfigSettings')
+Notification = load_model('openwisp_notifications', 'Notification')
 
 
 class BaseSubnetDivisionRule:
+    config_label = 'config'
+    ipam_label = 'openwisp_ipam'
+
     @property
     def ip_query(self):
         return IpAddress.objects.exclude(id=self.vpn_server.ip_id)
@@ -486,8 +490,8 @@ def test_multiple_vpnclient_delete(self):
             ip_query.count(), (rule.number_of_subnets * rule.number_of_ips)
         )
 
-    @patch('logging.Logger.error')
-    def test_subnets_exhausted(self, mocked_logger):
+    @patch('logging.Logger.info')
+    def test_subnets_exhausted(self, mocked_logger, *args):
         subnet = self._get_master_subnet(
             '10.0.0.0/29', master_subnet=self.master_subnet
         )
@@ -504,6 +508,8 @@ def test_subnets_exhausted(self, mocked_logger):
             number_of_ips=2,
             number_of_subnets=2,
         )
+        # A user is required to verify notification is created
+        self._get_admin()
         self.vpn_server.subnet = subnet
         self.vpn_server.save()
         self.config.templates.add(self.template)
@@ -512,7 +518,40 @@ def test_subnets_exhausted(self, mocked_logger):
             device=self._create_device(mac_address='00:11:22:33:44:66')
         )
         config2.templates.add(self.template)
-        mocked_logger.assert_called_with(f'Cannot create more subnets of {subnet}')
+        self.assertEqual(
+            mocked_logger.call_args_list[4][0][0],
+            f'Cannot create more subnets of {subnet}',
+        )
+        notification = Notification.objects.first()
+        self.assertEqual(notification.level, 'error')
+        self.assertEqual(notification.type, 'generic_message')
+        self.assertEqual(notification.target, config2.device)
+        self.assertEqual(notification.action_object, subnet)
+        self.assertEqual(
+            notification.message,
+            (
+                '<p>Failed to provision subnets for '
+                '<a href="https://example.com{device_path}">'
+                '{device_name}</a></p>'
+            ).format(
+                device_path=reverse(
+                    f'admin:{self.config_label}_device_change', args=[config2.device_id]
+                ),
+                device_name=config2.device.name,
+            ),
+        )
+        self.assertEqual(
+            notification.rendered_description,
+            (
+                '<p>The <a href="https://example.com{subnet_path}">'
+                '{subnet_name}</a> subnet has run out of space.</p>'
+            ).format(
+                subnet_path=reverse(
+                    f'admin:{self.ipam_label}_subnet_change', args=[subnet.id]
+                ),
+                subnet_name=subnet,
+            ),
+        )
 
     def test_vpn_subnet_none(self):
         self.vpn_server.subnet = None

tests/openwisp2/sample_subnet_division/tests.py

@@ -21,7 +21,7 @@ class TestSubnetAdmin(BaseTestSubnetAdmin):
 
 
 class TestSubnetDivsionRule(BaseTestSubnetDivisionRule):
-    pass
+    config_label = 'sample_config'
 
 
 class TestIPAdmin(BaseTestIPAdmin):