diff --git a/lib/charms/operator_libs_linux/v1/systemd.py b/lib/charms/operator_libs_linux/v1/systemd.py index d75ade1..cdcbad6 100644 --- a/lib/charms/operator_libs_linux/v1/systemd.py +++ b/lib/charms/operator_libs_linux/v1/systemd.py @@ -23,6 +23,7 @@ service_resume with run the mask/unmask and enable/disable invocations. Example usage: + ```python from charms.operator_libs_linux.v0.systemd import service_running, service_reload @@ -33,13 +34,14 @@ # Attempt to reload a service, restarting if necessary success = service_reload("nginx", restart_on_failure=True) ``` - """ -import logging -import subprocess - __all__ = [ # Don't export `_systemctl`. (It's not the intended way of using this lib.) + "SystemdError", + "daemon_reload", + "service_disable", + "service_enable", + "service_failed", "service_pause", "service_reload", "service_restart", @@ -47,9 +49,11 @@ "service_running", "service_start", "service_stop", - "daemon_reload", ] +import logging +import subprocess + logger = logging.getLogger(__name__) # The unique Charmhub library identifier, never change it @@ -60,133 +64,168 @@ # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 3 +LIBPATCH = 4 class SystemdError(Exception): """Custom exception for SystemD related errors.""" - pass +def _systemctl(*args: str, check: bool = False) -> int: + """Control a system service using systemctl. -def _popen_kwargs(): - return { - "stdout": subprocess.PIPE, - "stderr": subprocess.STDOUT, - "bufsize": 1, - "universal_newlines": True, - "encoding": "utf-8", - } - + Args: + *args: Arguments to pass to systemctl. + check: Check the output of the systemctl command. Default: False. -def _systemctl( - sub_cmd: str, service_name: str = None, now: bool = None, quiet: bool = None -) -> bool: - """Control a system service. + Returns: + Returncode of systemctl command execution. - Args: - sub_cmd: the systemctl subcommand to issue - service_name: the name of the service to perform the action on - now: passes the --now flag to the shell invocation. - quiet: passes the --quiet flag to the shell invocation. + Raises: + SystemdError: Raised if calling systemctl returns a non-zero returncode and check is True. """ - cmd = ["systemctl", sub_cmd] - - if service_name is not None: - cmd.append(service_name) - if now is not None: - cmd.append("--now") - if quiet is not None: - cmd.append("--quiet") - if sub_cmd != "is-active": - logger.debug("Attempting to {} '{}' with command {}.".format(cmd, service_name, cmd)) - else: - logger.debug("Checking if '{}' is active".format(service_name)) - - proc = subprocess.Popen(cmd, **_popen_kwargs()) - last_line = "" - for line in iter(proc.stdout.readline, ""): - last_line = line - logger.debug(line) - - proc.wait() - - if proc.returncode < 1: - return True - - # If we are just checking whether a service is running, return True/False, rather - # than raising an error. - if sub_cmd == "is-active" and proc.returncode == 3: # Code returned when service not active. - return False - - if sub_cmd == "is-failed": - return False - - raise SystemdError( - "Could not {}{}: systemd output: {}".format( - sub_cmd, " {}".format(service_name) if service_name else "", last_line + cmd = ["systemctl", *args] + logger.debug(f"Executing command: {cmd}") + try: + proc = subprocess.run( + cmd, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + text=True, + bufsize=1, + encoding="utf-8", + check=check, + ) + logger.debug( + f"Command {cmd} exit code: {proc.returncode}. systemctl output:\n{proc.stdout}" + ) + return proc.returncode + except subprocess.CalledProcessError as e: + raise SystemdError( + f"Command {cmd} failed with returncode {e.returncode}. systemctl output:\n{e.stdout}" ) - ) def service_running(service_name: str) -> bool: - """Determine whether a system service is running. + """Report whether a system service is running. Args: - service_name: the name of the service to check + service_name: The name of the service to check. + + Return: + True if service is running/active; False if not. """ - return _systemctl("is-active", service_name, quiet=True) + # If returncode is 0, this means that is service is active. + return _systemctl("--quiet", "is-active", service_name) == 0 def service_failed(service_name: str) -> bool: - """Determine whether a system service has failed. + """Report whether a system service has failed. Args: - service_name: the name of the service to check + service_name: The name of the service to check. + + Returns: + True if service is marked as failed; False if not. """ - return _systemctl("is-failed", service_name, quiet=True) + # If returncode is 0, this means that the service has failed. + return _systemctl("--quiet", "is-failed", service_name) == 0 -def service_start(service_name: str) -> bool: +def service_start(*args: str) -> bool: """Start a system service. Args: - service_name: the name of the service to start + *args: Arguments to pass to `systemctl start` (normally the service name). + + Returns: + On success, this function returns True for historical reasons. + + Raises: + SystemdError: Raised if `systemctl start ...` returns a non-zero returncode. """ - return _systemctl("start", service_name) + return _systemctl("start", *args, check=True) == 0 -def service_stop(service_name: str) -> bool: +def service_stop(*args: str) -> bool: """Stop a system service. Args: - service_name: the name of the service to stop + *args: Arguments to pass to `systemctl stop` (normally the service name). + + Returns: + On success, this function returns True for historical reasons. + + Raises: + SystemdError: Raised if `systemctl stop ...` returns a non-zero returncode. """ - return _systemctl("stop", service_name) + return _systemctl("stop", *args, check=True) == 0 -def service_restart(service_name: str) -> bool: +def service_restart(*args: str) -> bool: """Restart a system service. Args: - service_name: the name of the service to restart + *args: Arguments to pass to `systemctl restart` (normally the service name). + + Returns: + On success, this function returns True for historical reasons. + + Raises: + SystemdError: Raised if `systemctl restart ...` returns a non-zero returncode. """ - return _systemctl("restart", service_name) + return _systemctl("restart", *args, check=True) == 0 + + +def service_enable(*args: str) -> bool: + """Enable a system service. + + Args: + *args: Arguments to pass to `systemctl enable` (normally the service name). + + Returns: + On success, this function returns True for historical reasons. + + Raises: + SystemdError: Raised if `systemctl enable ...` returns a non-zero returncode. + """ + return _systemctl("enable", *args, check=True) == 0 + + +def service_disable(*args: str) -> bool: + """Disable a system service. + + Args: + *args: Arguments to pass to `systemctl disable` (normally the service name). + + Returns: + On success, this function returns True for historical reasons. + + Raises: + SystemdError: Raised if `systemctl disable ...` returns a non-zero returncode. + """ + return _systemctl("disable", *args, check=True) == 0 def service_reload(service_name: str, restart_on_failure: bool = False) -> bool: """Reload a system service, optionally falling back to restart if reload fails. Args: - service_name: the name of the service to reload - restart_on_failure: boolean indicating whether to fallback to a restart if the - reload fails. + service_name: The name of the service to reload. + restart_on_failure: + Boolean indicating whether to fall back to a restart if the reload fails. + + Returns: + On success, this function returns True for historical reasons. + + Raises: + SystemdError: Raised if `systemctl reload|restart ...` returns a non-zero returncode. """ try: - return _systemctl("reload", service_name) + return _systemctl("reload", service_name, check=True) == 0 except SystemdError: if restart_on_failure: - return _systemctl("restart", service_name) + return service_restart(service_name) else: raise @@ -194,37 +233,56 @@ def service_reload(service_name: str, restart_on_failure: bool = False) -> bool: def service_pause(service_name: str) -> bool: """Pause a system service. - Stop it, and prevent it from starting again at boot. + Stops the service and prevents the service from starting again at boot. Args: - service_name: the name of the service to pause + service_name: The name of the service to pause. + + Returns: + On success, this function returns True for historical reasons. + + Raises: + SystemdError: Raised if service is still running after being paused by systemctl. """ - _systemctl("disable", service_name, now=True) + _systemctl("disable", "--now", service_name) _systemctl("mask", service_name) - if not service_running(service_name): - return True + if service_running(service_name): + raise SystemdError(f"Attempted to pause {service_name!r}, but it is still running.") - raise SystemdError("Attempted to pause '{}', but it is still running.".format(service_name)) + return True def service_resume(service_name: str) -> bool: """Resume a system service. - Re-enable starting again at boot. Start the service. + Re-enable starting the service again at boot. Start the service. Args: - service_name: the name of the service to resume + service_name: The name of the service to resume. + + Returns: + On success, this function returns True for historical reasons. + + Raises: + SystemdError: Raised if service is not running after being resumed by systemctl. """ _systemctl("unmask", service_name) - _systemctl("enable", service_name, now=True) + _systemctl("enable", "--now", service_name) - if service_running(service_name): - return True + if not service_running(service_name): + raise SystemdError(f"Attempted to resume {service_name!r}, but it is not running.") - raise SystemdError("Attempted to resume '{}', but it is not running.".format(service_name)) + return True def daemon_reload() -> bool: - """Reload systemd manager configuration.""" - return _systemctl("daemon-reload") + """Reload systemd manager configuration. + + Returns: + On success, this function returns True for historical reasons. + + Raises: + SystemdError: Raised if `systemctl daemon-reload` returns a non-zero returncode. + """ + return _systemctl("daemon-reload", check=True) == 0 diff --git a/tests/unit/test_systemd.py b/tests/unit/test_systemd.py index f051903..469c5fc 100644 --- a/tests/unit/test_systemd.py +++ b/tests/unit/test_systemd.py @@ -1,6 +1,7 @@ # Copyright 2021 Canonical Ltd. # See LICENSE file for licensing details. +import subprocess import unittest from typing import List from unittest.mock import MagicMock, call, patch @@ -9,40 +10,43 @@ def with_mock_subp(func): - """Set up a Popen mock. + """Set up a `subprocess.run(...)` mock. Any function that uses this decorator should take a function as an argument. When - called with a series of return codes, that function will mock out subprocess.Popen, + called with a series of return codes, that function will mock out `subprocess.run(...)`, and set it to return those error codes, in order. - The function returns the mock Popen object, so that routines such as - assert_called_with can be called upon it, along with the return from - systemd._popen_kwargs, for convenience when composing call objects. - + The function returns the mock `subprocess.run(...)` object, so that routines such as + assert_called_with can be called upon it. """ - @patch("charms.operator_libs_linux.v1.systemd.subprocess") + @patch("charms.operator_libs_linux.v1.systemd.subprocess.run") def make_mocks_and_run(cls, mock_subp): - def make_mock_popen(returncodes: List[int], lines: List[str] = None, stdout: str = None): - lines = lines if lines is not None else ("", "") - - mock_subp.PIPE = mock_subp.STDOUT = stdout or "" - + def make_mock_run(returncodes: List[int], check: bool = False): side_effects = [] - for code in returncodes: - mock_proc = MagicMock() - mock_proc.wait.return_value = None - mock_proc.stdout.readline.side_effect = lines - mock_proc.returncode = code - side_effects.append(mock_proc) - - mock_popen = mock_subp.Popen - mock_popen.side_effect = tuple(side_effects) - - return mock_popen, systemd._popen_kwargs() - - func(cls, make_mock_popen) + if code != 0 and check: + side_effects.append(subprocess.CalledProcessError(code, "systemctl fail")) + else: + mock_proc = MagicMock() + mock_proc.returncode = code + mock_proc.stdout = (subprocess.PIPE,) + mock_proc.stderr = (subprocess.STDOUT,) + mock_proc.check = check + side_effects.append(mock_proc) + + mock_subp.side_effect = tuple(side_effects) + + return mock_subp, { + "stdout": subprocess.PIPE, + "stderr": subprocess.STDOUT, + "text": True, + "bufsize": 1, + "encoding": "utf-8", + "check": check, + } + + func(cls, make_mock_run) return make_mocks_and_run @@ -50,13 +54,17 @@ def make_mock_popen(returncodes: List[int], lines: List[str] = None, stdout: str class TestSystemD(unittest.TestCase): @with_mock_subp def test_service(self, make_mock): - mockp, kw = make_mock([0, 1]) + mockp, kw = make_mock([0]) success = systemd._systemctl("is-active", "mysql") mockp.assert_called_with(["systemctl", "is-active", "mysql"], **kw) - self.assertTrue(success) + self.assertEqual(success, 0) - self.assertRaises(systemd.SystemdError, systemd._systemctl, "is-active", "mysql") + mockp, kw = make_mock([1], check=True) + + self.assertRaises( + systemd.SystemdError, systemd._systemctl, "is-active", "mysql", check=True + ) mockp.assert_called_with(["systemctl", "is-active", "mysql"], **kw) @with_mock_subp @@ -64,11 +72,11 @@ def test_service_running(self, make_mock): mockp, kw = make_mock([0, 3]) is_running = systemd.service_running("mysql") - mockp.assert_called_with(["systemctl", "is-active", "mysql", "--quiet"], **kw) + mockp.assert_called_with(["systemctl", "--quiet", "is-active", "mysql"], **kw) self.assertTrue(is_running) is_running = systemd.service_running("mysql") - mockp.assert_called_with(["systemctl", "is-active", "mysql", "--quiet"], **kw) + mockp.assert_called_with(["systemctl", "--quiet", "is-active", "mysql"], **kw) self.assertFalse(is_running) @with_mock_subp @@ -76,72 +84,95 @@ def test_service_failed(self, make_mock): mockp, kw = make_mock([0, 1]) is_failed = systemd.service_failed("mysql") - mockp.assert_called_with(["systemctl", "is-failed", "mysql", "--quiet"], **kw) + mockp.assert_called_with(["systemctl", "--quiet", "is-failed", "mysql"], **kw) self.assertTrue(is_failed) is_failed = systemd.service_failed("mysql") - mockp.assert_called_with(["systemctl", "is-failed", "mysql", "--quiet"], **kw) + mockp.assert_called_with( + [ + "systemctl", + "--quiet", + "is-failed", + "mysql", + ], + **kw, + ) self.assertFalse(is_failed) @with_mock_subp def test_service_start(self, make_mock): - mockp, kw = make_mock([0, 1]) + mockp, kw = make_mock([0, 1], check=True) - started = systemd.service_start("mysql") + systemd.service_start("mysql") mockp.assert_called_with(["systemctl", "start", "mysql"], **kw) - self.assertTrue(started) self.assertRaises(systemd.SystemdError, systemd.service_start, "mysql") mockp.assert_called_with(["systemctl", "start", "mysql"], **kw) @with_mock_subp def test_service_stop(self, make_mock): - mockp, kw = make_mock([0, 1]) + mockp, kw = make_mock([0, 1], check=True) - stopped = systemd.service_stop("mysql") + systemd.service_stop("mysql") mockp.assert_called_with(["systemctl", "stop", "mysql"], **kw) - self.assertTrue(stopped) self.assertRaises(systemd.SystemdError, systemd.service_stop, "mysql") mockp.assert_called_with(["systemctl", "stop", "mysql"], **kw) @with_mock_subp def test_service_restart(self, make_mock): - mockp, kw = make_mock([0, 1]) + mockp, kw = make_mock([0, 1], check=True) - restarted = systemd.service_restart("mysql") + systemd.service_restart("mysql") mockp.assert_called_with(["systemctl", "restart", "mysql"], **kw) - self.assertTrue(restarted) self.assertRaises(systemd.SystemdError, systemd.service_restart, "mysql") mockp.assert_called_with(["systemctl", "restart", "mysql"], **kw) + @with_mock_subp + def test_service_enable(self, make_mock): + mockp, kw = make_mock([0, 1], check=True) + + systemd.service_enable("slurmd") + mockp.assert_called_with(["systemctl", "enable", "slurmd"], **kw) + + self.assertRaises(systemd.SystemdError, systemd.service_enable, "slurmd") + mockp.assert_called_with(["systemctl", "enable", "slurmd"], **kw) + + @with_mock_subp + def test_service_disable(self, make_mock): + mockp, kw = make_mock([0, 1], check=True) + + systemd.service_disable("slurmd") + mockp.assert_called_with(["systemctl", "disable", "slurmd"], **kw) + + self.assertRaises(systemd.SystemdError, systemd.service_disable, "slurmd") + mockp.assert_called_with(["systemctl", "disable", "slurmd"], **kw) + @with_mock_subp def test_service_reload(self, make_mock): # We reload successfully. - mockp, kw = make_mock([0]) - reloaded = systemd.service_reload("mysql") + mockp, kw = make_mock([0], check=True) + systemd.service_reload("mysql") mockp.assert_called_with(["systemctl", "reload", "mysql"], **kw) - self.assertTrue(reloaded) # We can't reload, so we restart - mockp, kw = make_mock([1, 0]) - reloaded = systemd.service_reload("mysql", restart_on_failure=True) + mockp, kw = make_mock([1, 0], check=True) + systemd.service_reload("mysql", restart_on_failure=True) mockp.assert_has_calls( [ call(["systemctl", "reload", "mysql"], **kw), call(["systemctl", "restart", "mysql"], **kw), ] ) - self.assertTrue(reloaded) # We should only restart if requested. - mockp, kw = make_mock([1, 0]) + mockp, kw = make_mock([1, 0], check=True) self.assertRaises(systemd.SystemdError, systemd.service_reload, "mysql") mockp.assert_called_with(["systemctl", "reload", "mysql"], **kw) # ... and if we fail at both, we should fail. - mockp, kw = make_mock([1, 1]) + mockp, kw = make_mock([1, 1], check=True) self.assertRaises( systemd.SystemdError, systemd.service_reload, "mysql", restart_on_failure=True ) @@ -157,24 +188,23 @@ def test_service_pause(self, make_mock): # Test pause mockp, kw = make_mock([0, 0, 3]) - paused = systemd.service_pause("mysql") + systemd.service_pause("mysql") mockp.assert_has_calls( [ - call(["systemctl", "disable", "mysql", "--now"], **kw), + call(["systemctl", "disable", "--now", "mysql"], **kw), call(["systemctl", "mask", "mysql"], **kw), - call(["systemctl", "is-active", "mysql", "--quiet"], **kw), + call(["systemctl", "--quiet", "is-active", "mysql"], **kw), ] ) - self.assertTrue(paused) # Could not stop service! mockp, kw = make_mock([0, 0, 0]) self.assertRaises(systemd.SystemdError, systemd.service_pause, "mysql") mockp.assert_has_calls( [ - call(["systemctl", "disable", "mysql", "--now"], **kw), + call(["systemctl", "disable", "--now", "mysql"], **kw), call(["systemctl", "mask", "mysql"], **kw), - call(["systemctl", "is-active", "mysql", "--quiet"], **kw), + call(["systemctl", "--quiet", "is-active", "mysql"], **kw), ] ) @@ -182,27 +212,25 @@ def test_service_pause(self, make_mock): def test_service_resume(self, make_mock): # Service is already running mockp, kw = make_mock([0, 0, 0]) - resumed = systemd.service_resume("mysql") + systemd.service_resume("mysql") mockp.assert_has_calls( [ call(["systemctl", "unmask", "mysql"], **kw), - call(["systemctl", "enable", "mysql", "--now"], **kw), - call(["systemctl", "is-active", "mysql", "--quiet"], **kw), + call(["systemctl", "enable", "--now", "mysql"], **kw), + call(["systemctl", "--quiet", "is-active", "mysql"], **kw), ] ) - self.assertTrue(resumed) # Service was stopped mockp, kw = make_mock([0, 0, 0]) - resumed = systemd.service_resume("mysql") + systemd.service_resume("mysql") mockp.assert_has_calls( [ call(["systemctl", "unmask", "mysql"], **kw), - call(["systemctl", "enable", "mysql", "--now"], **kw), - call(["systemctl", "is-active", "mysql", "--quiet"], **kw), + call(["systemctl", "enable", "--now", "mysql"], **kw), + call(["systemctl", "--quiet", "is-active", "mysql"], **kw), ] ) - self.assertTrue(resumed) # Could not start service! mockp, kw = make_mock([0, 0, 3]) @@ -210,18 +238,18 @@ def test_service_resume(self, make_mock): mockp.assert_has_calls( [ call(["systemctl", "unmask", "mysql"], **kw), - call(["systemctl", "enable", "mysql", "--now"], **kw), - call(["systemctl", "is-active", "mysql", "--quiet"], **kw), + call(["systemctl", "enable", "--now", "mysql"], **kw), + call(["systemctl", "--quiet", "is-active", "mysql"], **kw), ] ) @with_mock_subp def test_daemon_reload(self, make_mock): - mockp, kw = make_mock([0, 1]) + mockp, kw = make_mock([0, 1], check=True) - reloaded = systemd.daemon_reload() + systemd.daemon_reload() mockp.assert_called_with(["systemctl", "daemon-reload"], **kw) - self.assertTrue(reloaded) + # Failed to reload systemd configuration. self.assertRaises(systemd.SystemdError, systemd.daemon_reload) mockp.assert_called_with(["systemctl", "daemon-reload"], **kw)