[fix] Avoided re-populating VPN peers cache in post_save

Previously, ``VpnClient.post_save`` re-populated the peer cache immediately.
With a large number of peers, this could make uWSGI requests slow or even
timeout.

Now, the method only invalidates the peer cache. The cache will be
re-populated later by the ``trigger_vpn_server_endpoint`` task.

Additionally, the ``trigger_vpn_server_endpoint`` task now has a higher
soft time limit (300s) to accommodate generating large configurations
for many peers, which could exceed the default
`OpenwispCeleryTask.soft_time_limit`.

Author: pandafy

openwisp_controller/config/base/vpn.py

@@ -904,7 +904,7 @@ def _get_common_name(self):
     @classmethod
     def post_save(cls, instance, **kwargs):
         def _post_save():
-            instance.vpn._invalidate_peer_cache(update=True)
+            instance.vpn._invalidate_peer_cache()
 
         transaction.on_commit(_post_save)
         # ZT network member should be authorized and assigned

openwisp_controller/config/tasks.py

@@ -97,7 +97,9 @@ def invalidate_devicegroup_cache_delete(instance_id, model_name, **kwargs):
         )
 
 
-@shared_task(base=OpenwispCeleryTask)
+# Generating large configurations can be time-consuming; hence,
+# a custom soft time limit is applied here.
+@shared_task(soft_time_limit=300)
 def trigger_vpn_server_endpoint(endpoint, auth_token, vpn_id):
     Vpn = load_model("config", "Vpn")
     try:

openwisp_controller/config/tests/test_vpn.py

@@ -728,9 +728,18 @@ def test_trigger_vpn_server_endpoint_invalid_vpn_id(self):
 
 
 class TestWireguardTransaction(BaseTestVpn, TestWireguardVpnMixin, TransactionTestCase):
-    def test_auto_peer_configuration(self):
+    @mock.patch(
+        "openwisp_controller.config.tasks.requests.post",
+        return_value=HttpResponse(status=200),
+    )
+    def test_auto_peer_configuration(self, *args):
         self.assertEqual(IpAddress.objects.count(), 0)
-        device, vpn, template = self._create_wireguard_vpn_template()
+        device, vpn, template = self._create_wireguard_vpn_template(
+            vpn_options={
+                "webhook_endpoint": "https://example.com:8080/trigger-update",
+                "auth_token": "openwisp",
+            }
+        )
         vpnclient_qs = device.config.vpnclient_set
         self.assertEqual(vpnclient_qs.count(), 1)
         self.assertEqual(IpAddress.objects.count(), 2)
@@ -770,16 +779,12 @@ def test_auto_peer_configuration(self):
                 Vpn,
                 "invalidate_checksum_cache",
                 return_value=vpn.invalidate_checksum_cache(),
-            ) as mocked_invalidate_checksum_cache, mock.patch.object(
-                Vpn,
-                "get_cached_configuration",
-                return_value=vpn.get_cached_configuration(),
-            ) as mocked_cached_configuration:
+            ) as mocked_invalidate_checksum_cache:
                 device2.config.templates.add(template)
                 # The Vpn configuration cache is invalidated and re-populated
                 mocked_invalidate_checksum_cache.assert_called_once()
-                mocked_cached_configuration.assert_called_once()
-            # cache is invalidated and updated, hence no queries expected
+            # cache is invalidated and updated (by trigger_vpn_server_endpoint task),
+            # hence no queries expected
             with self.assertNumQueries(0):
                 vpn_config = vpn.get_config()["wireguard"][0]
             self.assertEqual(len(vpn_config.get("peers", [])), 2)
@@ -789,17 +794,12 @@ def test_auto_peer_configuration(self):
                 Vpn,
                 "invalidate_checksum_cache",
                 return_value=vpn.invalidate_checksum_cache(),
-            ) as mocked_invalidate_checksum_cache, mock.patch.object(
-                Vpn,
-                "get_cached_configuration",
-                return_value=vpn.get_cached_configuration(),
-            ) as mocked_cached_configuration:
+            ) as mocked_invalidate_checksum_cache:
                 device2.delete(check_deactivated=False)
                 mocked_invalidate_checksum_cache.assert_called_once()
-                mocked_cached_configuration.assert_not_called()
-            # cache is invalidated but not updated
-            # hence we expect queries to be generated
-            with self.assertNumQueries(1):
+            # cache is invalidated and is updated by the
+            # trigger_vpn_server_endpoint task
+            with self.assertNumQueries(0):
                 vpn_config = vpn.get_config()["wireguard"][0]
             self.assertEqual(len(vpn_config.get("peers", [])), 1)
 
@@ -985,9 +985,18 @@ def test_auto_client(self):
 class TestVxlanTransaction(
     BaseTestVpn, TestVxlanWireguardVpnMixin, TransactionTestCase
 ):
-    def test_auto_peer_configuration(self):
+    @mock.patch(
+        "openwisp_controller.config.tasks.requests.post",
+        return_value=HttpResponse(status=200),
+    )
+    def test_auto_peer_configuration(self, *args):
         self.assertEqual(IpAddress.objects.count(), 0)
-        device, vpn, template = self._create_vxlan_vpn_template()
+        device, vpn, template = self._create_vxlan_vpn_template(
+            vpn_options={
+                "webhook_endpoint": "https://example.com:8080/trigger-update",
+                "auth_token": "openwisp",
+            }
+        )
         vpnclient_qs = device.config.vpnclient_set
         self.assertEqual(vpnclient_qs.count(), 1)
         self.assertEqual(IpAddress.objects.count(), 2)
@@ -1014,17 +1023,17 @@ def test_auto_peer_configuration(self):
                 }
             )
             device2.config.templates.add(template)
-            # cache is invalidated and updated, hence no queries expected
+            # cache is invalidated and updated (by trigger_vpn_server_endpoint task),
+            # hence no queries expected
             with self.assertNumQueries(0):
                 config = vpn.get_config()
             peers = json.loads(config["files"][0]["contents"])
             self.assertEqual(len(peers), 2)
 
         with self.subTest("cache updated when a new peer is deleted"):
             device2.delete(check_deactivated=False)
-            # cache is invalidated but not updated
-            # hence we expect queries to be generated
-            with self.assertNumQueries(2):
+            # cache is invalidated and updated (by trigger_vpn_server_endpoint task)
+            with self.assertNumQueries(0):
                 config = vpn.get_config()
             peers = json.loads(config["files"][0]["contents"])
             self.assertEqual(len(peers), 1)

openwisp_controller/config/tests/utils.py

@@ -164,8 +164,11 @@ def _create_wireguard_vpn(self, config=None, **kwargs):
         vpn.save()
         return vpn
 
-    def _create_wireguard_vpn_template(self, auto_cert=True, **kwargs):
-        vpn = self._create_wireguard_vpn()
+    def _create_wireguard_vpn_template(
+        self, auto_cert=True, vpn_options=None, **kwargs
+    ):
+        vpn_options = vpn_options or {}
+        vpn = self._create_wireguard_vpn(**vpn_options)
         org1 = kwargs.get("organization", vpn.organization)
         template = self._create_template(
             name="wireguard",
@@ -180,7 +183,7 @@ def _create_wireguard_vpn_template(self, auto_cert=True, **kwargs):
 
 
 class TestVxlanWireguardVpnMixin(CreateIpamModelsMixin):
-    def _create_vxlan_tunnel(self, config=None):
+    def _create_vxlan_tunnel(self, config=None, **kwargs):
         if config is None:
             config = {"wireguard": [{"name": "wg0", "port": 51820}]}
         org = self._get_org()
@@ -193,11 +196,13 @@ def _create_vxlan_tunnel(self, config=None):
             config=config,
             subnet=subnet,
             ca=None,
+            **kwargs,
         )
         return tunnel, subnet
 
-    def _create_vxlan_vpn_template(self):
-        vpn, subnet = self._create_vxlan_tunnel()
+    def _create_vxlan_vpn_template(self, vpn_options=None):
+        vpn_options = vpn_options or {}
+        vpn, subnet = self._create_vxlan_tunnel(**vpn_options)
         org1 = vpn.organization
         template = self._create_template(
             name="vxlan-wireguard",