radicale, bepasty: Fix issue with failed diagnostic test

Fixes: #2571.

- During backup, a service related to backup is stopped and then started back
again after the backup. In case of .socket unit, the .service unit is not being
stopped and it continues to listen on the socket path. When the .socket unit is
started back again, it tries to listen on the socket path and fails.

- To fix the issue, when running stop, restart, etc. operations on a .socket
file, try to perform that operations that we actually intend.

Tests:

- Unit tests pass

- Functional tests for bepasty and radicale work.

- After taking a radicale backup uwsgi-app@radicale.socket does not become
inactive (works when service is running or stopped). uwsgi-app@radicale.service
stops if it is running, backup doesn't fail if service is not running.

- After taking a radicale backup uwsgi-app@bepasty-freedombox.socket does not
become inactive (works when service is running or stopped).
uwsgi-app@bepasty-freedombox.service stops if it is running, backup doesn't fail
if service is not running.

Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>

plinth/action_utils.py

@@ -120,14 +120,6 @@ def service_disable(service_name: str, check: bool = False):
     except subprocess.CalledProcessError:
         pass
 
-    if service_name.endswith('.socket'):
-        # Instead, may need to query the unit for associated .service file.
-        base_name = service_name.rpartition('.')[0]
-        try:
-            service_stop(f'{base_name}.service', check=check)
-        except subprocess.CalledProcessError:
-            pass
-
 
 def service_mask(service_name: str, check: bool = False):
     """Mask a service"""
@@ -143,25 +135,62 @@ def service_start(service_name: str, check: bool = False):
     """Start a service with systemd."""
     service_action(service_name, 'start', check=check)
 
+    # When starting a .socket unit, there is not need to start the .service
+    # unit as it will be automatically started when a request is received on
+    # the socket.
+
+
+def _get_service_unit(socket_name: str) -> str:
+    """Return the .service unit name for a .socket unit."""
+    # Instead, may need to query the unit for associated .service file.
+    base_name = socket_name.rpartition('.')[0]
+    return f'{base_name}.service'
+
 
 def service_stop(service_name: str, check: bool = False):
     """Stop a service with systemd."""
     service_action(service_name, 'stop', check=check)
 
+    # When stopping a .socket unit, most of the time, we must also stop
+    # .service unit. This frees up resources when disabling the app. It also
+    # stops using resources that are being backed up.
+    if service_name.endswith('.socket'):
+        service_action(_get_service_unit(service_name), 'stop', check=check)
+
 
 def service_restart(service_name: str, check: bool = False):
     """Restart a service with systemd."""
-    service_action(service_name, 'restart', check=check)
+    if not service_name.endswith('.socket'):
+        service_action(service_name, 'restart', check=check)
+    else:
+        # When restarting a .socket unit, most of the time, we actually want to
+        # restart the corresponding .service unit. This reloads the
+        # configuration changes as needed. To restart, all we need to do stop
+        # the service. It will be automatically started again by .socket unit.
+        service_action(_get_service_unit(service_name), 'stop', check=check)
 
 
 def service_try_restart(service_name: str, check: bool = False):
     """Try to restart a service with systemd."""
-    service_action(service_name, 'try-restart', check=check)
+    if not service_name.endswith('.socket'):
+        service_action(service_name, 'try-restart', check=check)
+    else:
+        # When try-restarting a .socket unit, most of the time, we actually
+        # want to restart the corresponding .service unit. This reloads the
+        # configuration changes as needed. To restart, all we need to do stop
+        # the service. It will be automatically started again by .socket unit.
+        service_action(_get_service_unit(service_name), 'stop', check=check)
 
 
 def service_reload(service_name: str, check: bool = False):
     """Reload a service with systemd."""
-    service_action(service_name, 'reload', check=check)
+    if not service_name.endswith('.socket'):
+        service_action(service_name, 'reload', check=check)
+    else:
+        # When reloading a .socket unit, most of the time, we actually want to
+        # reload the corresponding .service unit. This reloads the
+        # configuration changes as needed.
+        service_action(_get_service_unit(service_name), 'reload', check=check)
 
 
 def service_try_reload_or_restart(service_name: str, check: bool = False):
@@ -169,7 +198,14 @@ def service_try_reload_or_restart(service_name: str, check: bool = False):
 
     Do nothing if service is not running.
     """
-    service_action(service_name, 'try-reload-or-restart', check=check)
+    if not service_name.endswith('.socket'):
+        service_action(service_name, 'try-reload-or-restart', check=check)
+    else:
+        # When reloading a .socket unit, most of the time, we actually want to
+        # reload the corresponding .service unit. This reloads the
+        # configuration changes as needed.
+        service_action(_get_service_unit(service_name),
+                       'try-reload-or-restart', check=check)
 
 
 def service_reset_failed(service_name: str, check: bool = False):

plinth/tests/test_action_utils.py

@@ -20,7 +20,8 @@ from plinth.action_utils import (get_addresses, get_hostname,
                                  service_try_reload_or_restart,
                                  service_try_restart, service_unmask, umask)
 
-UNKNOWN = 'unknowndeamon'
+UNKNOWN = 'unknowndeamon.service'
+UNKNOWN_SOCKET = 'unknowndeamon.socket'
 
 systemctl_path = pathlib.Path('/usr/bin/systemctl')
 systemd_installed = pytest.mark.skipif(not systemctl_path.exists(),
@@ -76,20 +77,101 @@ def test_service_enable_and_disable():
 
 @patch('plinth.action_utils.service_action')
 @systemd_installed
-def test_service_actions(mock):
-    """Trivial white box test for trivial implementations."""
+def test_service_start(mock):
+    """Test staring a service."""
     service_start(UNKNOWN)
-    mock.assert_called_with(UNKNOWN, 'start', check=False)
+    mock.mock_calls = [call(UNKNOWN, 'start', check=False)]
+
+    mock.reset_mock()
+    service_start(UNKNOWN, check=True)
+    mock.mock_calls = [call(UNKNOWN, 'start', check=True)]
+
+
+@patch('plinth.action_utils.service_action')
+@systemd_installed
+def test_service_stop(mock):
+    """Test stopping a service."""
     service_stop(UNKNOWN)
-    mock.assert_called_with(UNKNOWN, 'stop', check=False)
+    assert mock.mock_calls == [call(UNKNOWN, 'stop', check=False)]
+
+    mock.reset_mock()
+    service_stop(UNKNOWN, check=True)
+    mock.mock_calls = [call(UNKNOWN, 'stop', check=True)]
+
+    mock.reset_mock()
+    service_stop(UNKNOWN_SOCKET)
+    assert mock.mock_calls == [
+        call(UNKNOWN_SOCKET, 'stop', check=False),
+        call(UNKNOWN, 'stop', check=False)
+    ]
+
+
+@patch('plinth.action_utils.service_action')
+@systemd_installed
+def test_service_restart(mock):
+    """Test restaring a service."""
     service_restart(UNKNOWN)
-    mock.assert_called_with(UNKNOWN, 'restart', check=False)
+    assert mock.mock_calls == [call(UNKNOWN, 'restart', check=False)]
+
+    mock.reset_mock()
+    service_restart(UNKNOWN, check=True)
+    mock.mock_calls = [call(UNKNOWN, 'restart', check=True)]
+
+    mock.reset_mock()
+    service_restart(UNKNOWN_SOCKET)
+    assert mock.mock_calls == [call(UNKNOWN, 'stop', check=False)]
+
+
+@patch('plinth.action_utils.service_action')
+@systemd_installed
+def test_service_try_restart(mock):
+    """Test try-restaring a service."""
     service_try_restart(UNKNOWN)
-    mock.assert_called_with(UNKNOWN, 'try-restart', check=False)
+    assert mock.mock_calls == [call(UNKNOWN, 'try-restart', check=False)]
+
+    mock.reset_mock()
+    service_try_restart(UNKNOWN, check=True)
+    mock.mock_calls = [call(UNKNOWN, 'try-restart', check=True)]
+
+    mock.reset_mock()
+    service_restart(UNKNOWN_SOCKET)
+    assert mock.mock_calls == [call(UNKNOWN, 'stop', check=False)]
+
+
+@patch('plinth.action_utils.service_action')
+@systemd_installed
+def test_service_reload(mock):
+    """Test reloading a service."""
     service_reload(UNKNOWN)
-    mock.assert_called_with(UNKNOWN, 'reload', check=False)
+    assert mock.mock_calls == [call(UNKNOWN, 'reload', check=False)]
+
+    mock.reset_mock()
+    service_reload(UNKNOWN, check=True)
+    mock.mock_calls = [call(UNKNOWN, 'reload', check=True)]
+
+    mock.reset_mock()
+    service_reload(UNKNOWN_SOCKET)
+    assert mock.mock_calls == [call(UNKNOWN, 'reload', check=False)]
+
+
+@patch('plinth.action_utils.service_action')
+@systemd_installed
+def test_service_try_reload_or_restart(mock):
+    """Test try-reload-or-restart on a service."""
     service_try_reload_or_restart(UNKNOWN)
-    mock.assert_called_with(UNKNOWN, 'try-reload-or-restart', check=False)
+    assert mock.mock_calls == [
+        call(UNKNOWN, 'try-reload-or-restart', check=False)
+    ]
+
+    mock.reset_mock()
+    service_try_reload_or_restart(UNKNOWN, check=True)
+    mock.mock_calls = [call(UNKNOWN, 'try-reload-or-restart', check=True)]
+
+    mock.reset_mock()
+    service_try_reload_or_restart(UNKNOWN_SOCKET)
+    assert mock.mock_calls == [
+        call(UNKNOWN, 'try-reload-or-restart', check=False)
+    ]
 
 
 @pytest.mark.usefixtures('needs_root')