Merge remote-tracking branch 'origin/master' into issues/1049-send-generic-message

Author: stktyagi

.github/workflows/ci.yml

@@ -79,9 +79,9 @@ jobs:
           pip install -U -e .
           pip install ${{ matrix.django-version }}
 
-      - name: Start postgres and redis
+      - name: Start redis
         if: ${{ !cancelled() && steps.deps.conclusion == 'success' }}
-        run: docker compose up -d postgres redis
+        run: docker compose up -d redis
 
       - name: QA checks
         run: ./run-qa-checks

docker-compose.yml

@@ -17,12 +17,3 @@ services:
     ports:
       - "6379:6379"
     entrypoint: redis-server --appendonly yes
-
-  postgres:
-    image: postgis/postgis:17-3.5-alpine
-    environment:
-      POSTGRES_PASSWORD: openwisp2
-      POSTGRES_USER: openwisp2
-      POSTGRES_DB: openwisp2
-    ports:
-      - 5432:5432

docs/developer/installation.rst

@@ -37,11 +37,11 @@ Navigate into the cloned repository:
 
     cd openwisp-controller/
 
-Launch Redis and PostgreSQL:
+Launch Redis:
 
 .. code-block:: shell
 
-    docker compose up -d redis postgres
+    docker compose up -d redis
 
 Setup and activate a virtual-environment (we'll be using `virtualenv
 <https://pypi.org/project/virtualenv/>`_):
@@ -99,18 +99,6 @@ Run tests with (make sure you have the :ref:`selenium dependencies
 
     ./runtests
 
-Some tests, such as the Selenium UI tests, require a PostgreSQL database
-to run. If you don't have a PostgreSQL database running on your system,
-you can use :ref:`the Docker Compose configuration provided in this
-repository <controller_dev_docker>`. Once set up, you can run these
-specific tests as follows:
-
-.. code-block:: shell
-
-    # Run only specific selenium tests classes
-    cd tests/
-    DJANGO_SETTINGS_MODULE=openwisp2.postgresql_settings ./manage.py test openwisp_controller.config.tests.test_selenium.TestDeviceAdmin
-
 Run quality assurance tests with:
 
 .. code-block:: shell

openwisp_controller/config/tasks.py

@@ -119,6 +119,10 @@ def invalidate_devicegroup_cache_delete(instance_id, model_name, **kwargs):
         )
 
 
+
+# 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')

openwisp_controller/connection/connectors/exceptions.py

@@ -4,3 +4,11 @@ class CommandFailedException(Exception):
     """
 
     pass
+
+
+class CommandTimeoutException(Exception):
+    """
+    raised when a command times out
+    """
+
+    pass

openwisp_controller/connection/connectors/ssh.py

@@ -1,5 +1,6 @@
 import logging
 import socket
+import time
 from io import BytesIO, StringIO
 
 import paramiko
@@ -10,7 +11,7 @@
 from scp import SCPClient
 
 from .. import settings as app_settings
-from .exceptions import CommandFailedException
+from .exceptions import CommandFailedException, CommandTimeoutException
 
 logger = logging.getLogger(__name__)
 
@@ -148,7 +149,7 @@ def _connect(self, address):
                     auth_timeout=app_settings.SSH_AUTH_TIMEOUT,
                     banner_timeout=app_settings.SSH_BANNER_TIMEOUT,
                     timeout=app_settings.SSH_CONNECTION_TIMEOUT,
-                    **params
+                    **params,
                 )
             except paramiko.ssh_exception.AuthenticationException as e:
                 # the authentication failure may be caused by the issue
@@ -183,9 +184,12 @@ def exec_command(
         - aborts on exceptions
         - raises socket.timeout exceptions
         """
+        # paramiko expects timeout as a float
+        timeout = float(timeout)
         logger.info("Executing command: {0}".format(command))
         # execute commmand
         try:
+            start_cmd = time.perf_counter()
             stdin, stdout, stderr = self.shell.exec_command(command, timeout=timeout)
         # re-raise socket.timeout to avoid being catched
         # by the subsequent `except Exception as e` block
@@ -195,8 +199,16 @@ def exec_command(
         except Exception as e:
             logger.exception(e)
             raise e
-        # store command exit status
-        exit_status = stdout.channel.recv_exit_status()
+        # workaround https://github.com/paramiko/paramiko/issues/1815
+        # workaround https://github.com/paramiko/paramiko/issues/1787
+        # Ref. https://docs.paramiko.org/en/stable/api/channel.html#paramiko.channel.Channel.recv_exit_status  # noqa
+        if not stdout.channel.status_event.wait(
+            timeout=timeout - (time.perf_counter() - start_cmd)
+        ):
+            log_message = f"Command timed out after {timeout} seconds."
+            logger.info(log_message)
+            raise CommandTimeoutException(log_message)
+        exit_status = stdout.channel.exit_status
         # log standard output
         # try to decode to UTF-8, ignoring unconvertible characters
         # https://docs.python.org/3/howto/unicode.html#the-string-type

openwisp_controller/connection/tasks.py

@@ -9,6 +9,7 @@
 from swapper import load_model
 
 from . import settings as app_settings
+from .connectors.exceptions import CommandTimeoutException
 from .exceptions import NoWorkingDeviceConnectionError
 
 logger = logging.getLogger(__name__)
@@ -76,6 +77,10 @@ def launch_command(command_id):
         command.status = "failed"
         command._add_output(_("Background task time limit exceeded."))
         command.save()
+    except CommandTimeoutException as e:
+        command.status = "failed"
+        command._add_output(_(f"The command took longer than expected: {e}"))
+        command.save()
     except Exception as e:
         logger.exception(
             f"An exception was raised while executing command {command_id}"

openwisp_controller/connection/tests/test_models.py

@@ -1,5 +1,6 @@
 import socket
 from unittest import mock
+from unittest.mock import PropertyMock
 
 import paramiko
 from django.contrib.auth.models import ContentType
@@ -46,7 +47,7 @@ def _exec_command_return_value(
         stderr_ = mock.Mock()
         stdin_.read().decode.return_value = stdin
         stdout_.read().decode.return_value = stdout
-        stdout_.channel.recv_exit_status.return_value = exit_code
+        type(stdout_.channel).exit_status = PropertyMock(return_value=exit_code)
         stderr_.read().decode.return_value = stderr
         return (stdin_, stdout_, stderr_)
 
@@ -1009,7 +1010,7 @@ def _assert_applying_conf_test_command(mocked_exec):
                 # 1. Checking openwisp_config returns with 0
                 # 2. Testing presence of /tmp/openwisp/applying_conf returns with 1
                 # 3. Restarting openwisp_config returns with 0 exit code
-                stdout.channel.recv_exit_status.side_effect = [0, 1, 1]
+                type(stdout.channel).exit_status = PropertyMock(side_effect=[0, 1, 1])
                 mocked_exec_command.return_value = (stdin, stdout, stderr)
                 conf.save()
                 self.assertEqual(mocked_exec_command.call_count, 3)

openwisp_controller/connection/tests/test_ssh.py

@@ -70,6 +70,19 @@ def test_connection_failed_command(self, mocked_debug, mocked_info):
             ]
         )
 
+    @mock.patch.object(ssh_logger, "info")
+    @mock.patch.object(ssh_logger, "debug")
+    def test_connection_command_timeout(self, mocked_debug, mocked_info):
+        ckey = self._create_credentials_with_key(port=self.ssh_server.port)
+        dc = self._create_device_connection(credentials=ckey)
+        dc.connector_instance.connect()
+        with self.assertRaises(Exception) as ctx:
+            # timeout of 0.0 is a special case in paramiko -> we check for 0.01 instead
+            dc.connector_instance.exec_command("sleep 1", timeout=0.01)
+        log_message = "Command timed out after 0.01 seconds."
+        mocked_info.assert_has_calls([mock.call(log_message)])
+        self.assertEqual(str(ctx.exception), log_message)
+
     @mock.patch.object(ssh_logger, "info")
     @mock.patch.object(ssh_logger, "debug")
     def test_connection_failed_command_suppressed_output(

openwisp_controller/geo/tests/test_selenium.py

@@ -1,6 +1,6 @@
+from channels.testing import ChannelsLiveServerTestCase
 from django.contrib.auth import get_user_model
 from django.contrib.auth.models import Permission
-from django.contrib.staticfiles.testing import StaticLiveServerTestCase
 from django.test import tag
 from django.urls.base import reverse
 from django_loci.tests import TestAdminMixin
@@ -22,7 +22,10 @@
 # these tests are for geo elements on device admin
 @tag("selenium_tests")
 class TestDeviceAdminGeoSelenium(
-    BaseTestDeviceAdminSelenium, TestOrganizationMixin, StaticLiveServerTestCase
+    TestGeoMixin,
+    BaseTestDeviceAdminSelenium,
+    TestOrganizationMixin,
+    ChannelsLiveServerTestCase,
 ):
     app_label = "geo"
     object_model = Device
@@ -66,7 +69,7 @@ class TestDeviceAdminReadonly(
     TestGeoMixin,
     TestAdminMixin,
     SeleniumTestMixin,
-    StaticLiveServerTestCase,
+    ChannelsLiveServerTestCase,
 ):
     browser = "chrome"
     app_label = "geo"