diff --git a/plinth/action_utils.py b/plinth/action_utils.py index 8d50836dc..8d9c91196 100644 --- a/plinth/action_utils.py +++ b/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): diff --git a/plinth/tests/test_action_utils.py b/plinth/tests/test_action_utils.py index 7e6a13c9d..e190a19ba 100644 --- a/plinth/tests/test_action_utils.py +++ b/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')