diff --git a/changelog/62942.fixed b/changelog/62942.fixed new file mode 100644 index 00000000000..f11a010d588 --- /dev/null +++ b/changelog/62942.fixed @@ -0,0 +1 @@ +Fix systemd_service.* functions hard code relative command name diff --git a/salt/modules/systemd_service.py b/salt/modules/systemd_service.py index df20de29a1c..52c17ba6074 100644 --- a/salt/modules/systemd_service.py +++ b/salt/modules/systemd_service.py @@ -305,7 +305,9 @@ def _runlevel(): contextkey = "systemd._runlevel" if contextkey in __context__: return __context__[contextkey] - out = __salt__["cmd.run"]("runlevel", python_shell=False, ignore_retcode=True) + out = __salt__["cmd.run"]( + salt.utils.path.which("runlevel"), python_shell=False, ignore_retcode=True + ) try: ret = out.split()[1] except IndexError: @@ -338,8 +340,8 @@ def _systemctl_cmd(action, name=None, systemd_scope=False, no_block=False, root= and salt.utils.systemd.has_scope(__context__) and __salt__["config.get"]("systemd.scope", True) ): - ret.extend(["systemd-run", "--scope"]) - ret.append("systemctl") + ret.extend([salt.utils.path.which("systemd-run"), "--scope"]) + ret.append(salt.utils.path.which("systemctl")) if no_block: ret.append("--no-block") if root: @@ -1440,7 +1442,7 @@ def firstboot( salt '*' service.firstboot keymap=jp locale=en_US.UTF-8 """ - cmd = ["systemd-firstboot"] + cmd = [salt.utils.path.which("systemd-firstboot")] parameters = [ ("locale", locale), ("locale-message", locale_message), diff --git a/tests/unit/modules/test_systemd_service.py b/tests/unit/modules/test_systemd_service.py index e6c8a73a105..f07773925fe 100644 --- a/tests/unit/modules/test_systemd_service.py +++ b/tests/unit/modules/test_systemd_service.py @@ -361,116 +361,72 @@ class SystemdScopeTestCase(TestCase, LoaderModuleMockMixin): func = getattr(systemd, action) # Remove trailing _ in "reload_" action = action.rstrip("_").replace("_", "-") - systemctl_command = ["systemctl"] + systemctl_command = ["/bin/systemctl"] if no_block: systemctl_command.append("--no-block") systemctl_command.extend([action, self.unit_name + ".service"]) - scope_prefix = ["systemd-run", "--scope"] + scope_prefix = ["/bin/systemd-run", "--scope"] assert_kwargs = {"python_shell": False} if action in ("enable", "disable"): assert_kwargs["ignore_retcode"] = True - with patch.object(systemd, "_check_for_unit_changes", self.mock_none): - with patch.object(systemd, "_unit_file_changed", self.mock_none): - with patch.object(systemd, "_check_unmask", self.mock_none): - with patch.object( - systemd, "_get_sysv_services", self.mock_empty_list - ): - - # Has scopes available + with patch("salt.utils.path.which", lambda x: "/bin/" + x): + with patch.object(systemd, "_check_for_unit_changes", self.mock_none): + with patch.object(systemd, "_unit_file_changed", self.mock_none): + with patch.object(systemd, "_check_unmask", self.mock_none): with patch.object( - salt.utils.systemd, "has_scope", self.mock_true + systemd, "_get_sysv_services", self.mock_empty_list ): - # Scope enabled, successful - with patch.dict( - systemd.__salt__, - { - "config.get": self.mock_true, - "cmd.run_all": self.mock_run_all_success, - }, + # Has scopes available + with patch.object( + salt.utils.systemd, "has_scope", self.mock_true ): - ret = func(self.unit_name, no_block=no_block) - self.assertTrue(ret) - self.mock_run_all_success.assert_called_with( - scope_prefix + systemctl_command, **assert_kwargs - ) - # Scope enabled, failed - with patch.dict( - systemd.__salt__, - { - "config.get": self.mock_true, - "cmd.run_all": self.mock_run_all_failure, - }, - ): - if action in ("stop", "disable"): - ret = func(self.unit_name, no_block=no_block) - self.assertFalse(ret) - else: - self.assertRaises( - CommandExecutionError, - func, - self.unit_name, - no_block=no_block, - ) - self.mock_run_all_failure.assert_called_with( - scope_prefix + systemctl_command, **assert_kwargs - ) - - # Scope disabled, successful - with patch.dict( - systemd.__salt__, - { - "config.get": self.mock_false, - "cmd.run_all": self.mock_run_all_success, - }, - ): - ret = func(self.unit_name, no_block=no_block) - self.assertTrue(ret) - self.mock_run_all_success.assert_called_with( - systemctl_command, **assert_kwargs - ) - - # Scope disabled, failed - with patch.dict( - systemd.__salt__, - { - "config.get": self.mock_false, - "cmd.run_all": self.mock_run_all_failure, - }, - ): - if action in ("stop", "disable"): - ret = func(self.unit_name, no_block=no_block) - self.assertFalse(ret) - else: - self.assertRaises( - CommandExecutionError, - func, - self.unit_name, - no_block=no_block, - ) - self.mock_run_all_failure.assert_called_with( - systemctl_command, **assert_kwargs - ) - - # Does not have scopes available - with patch.object( - salt.utils.systemd, "has_scope", self.mock_false - ): - - # The results should be the same irrespective of - # whether or not scope is enabled, since scope is not - # available, so we repeat the below tests with it both - # enabled and disabled. - for scope_mock in (self.mock_true, self.mock_false): - - # Successful + # Scope enabled, successful with patch.dict( systemd.__salt__, { - "config.get": scope_mock, + "config.get": self.mock_true, + "cmd.run_all": self.mock_run_all_success, + }, + ): + ret = func(self.unit_name, no_block=no_block) + self.assertTrue(ret) + self.mock_run_all_success.assert_called_with( + scope_prefix + systemctl_command, + **assert_kwargs + ) + + # Scope enabled, failed + with patch.dict( + systemd.__salt__, + { + "config.get": self.mock_true, + "cmd.run_all": self.mock_run_all_failure, + }, + ): + if action in ("stop", "disable"): + ret = func(self.unit_name, no_block=no_block) + self.assertFalse(ret) + else: + self.assertRaises( + CommandExecutionError, + func, + self.unit_name, + no_block=no_block, + ) + self.mock_run_all_failure.assert_called_with( + scope_prefix + systemctl_command, + **assert_kwargs + ) + + # Scope disabled, successful + with patch.dict( + systemd.__salt__, + { + "config.get": self.mock_false, "cmd.run_all": self.mock_run_all_success, }, ): @@ -480,11 +436,11 @@ class SystemdScopeTestCase(TestCase, LoaderModuleMockMixin): systemctl_command, **assert_kwargs ) - # Failed + # Scope disabled, failed with patch.dict( systemd.__salt__, { - "config.get": scope_mock, + "config.get": self.mock_false, "cmd.run_all": self.mock_run_all_failure, }, ): @@ -502,6 +458,55 @@ class SystemdScopeTestCase(TestCase, LoaderModuleMockMixin): systemctl_command, **assert_kwargs ) + # Does not have scopes available + with patch.object( + salt.utils.systemd, "has_scope", self.mock_false + ): + + # The results should be the same irrespective of + # whether or not scope is enabled, since scope is not + # available, so we repeat the below tests with it both + # enabled and disabled. + for scope_mock in (self.mock_true, self.mock_false): + + # Successful + with patch.dict( + systemd.__salt__, + { + "config.get": scope_mock, + "cmd.run_all": self.mock_run_all_success, + }, + ): + ret = func(self.unit_name, no_block=no_block) + self.assertTrue(ret) + self.mock_run_all_success.assert_called_with( + systemctl_command, **assert_kwargs + ) + + # Failed + with patch.dict( + systemd.__salt__, + { + "config.get": scope_mock, + "cmd.run_all": self.mock_run_all_failure, + }, + ): + if action in ("stop", "disable"): + ret = func( + self.unit_name, no_block=no_block + ) + self.assertFalse(ret) + else: + self.assertRaises( + CommandExecutionError, + func, + self.unit_name, + no_block=no_block, + ) + self.mock_run_all_failure.assert_called_with( + systemctl_command, **assert_kwargs + ) + def _mask_unmask(self, action, runtime): """ Common code for mask/unmask tests @@ -512,110 +517,75 @@ class SystemdScopeTestCase(TestCase, LoaderModuleMockMixin): func = getattr(systemd, action) # Remove trailing _ in "unmask_" action = action.rstrip("_").replace("_", "-") - systemctl_command = ["systemctl", action] + systemctl_command = ["/bin/systemctl", action] if runtime: systemctl_command.append("--runtime") systemctl_command.append(self.unit_name + ".service") - scope_prefix = ["systemd-run", "--scope"] + scope_prefix = ["/bin/systemd-run", "--scope"] args = [self.unit_name, runtime] masked_mock = self.mock_true if action == "unmask" else self.mock_false - with patch.object(systemd, "_check_for_unit_changes", self.mock_none): - if action == "unmask": - mock_not_run = MagicMock( - return_value={ - "retcode": 0, - "stdout": "", - "stderr": "", - "pid": 12345, - } - ) - with patch.dict(systemd.__salt__, {"cmd.run_all": mock_not_run}): - with patch.object(systemd, "masked", self.mock_false): - # Test not masked (should take no action and return True) - self.assertTrue(systemd.unmask_(self.unit_name)) - # Also should not have called cmd.run_all - self.assertTrue(mock_not_run.call_count == 0) + with patch("salt.utils.path.which", lambda x: "/bin/" + x): + with patch.object(systemd, "_check_for_unit_changes", self.mock_none): + if action == "unmask": + mock_not_run = MagicMock( + return_value={ + "retcode": 0, + "stdout": "", + "stderr": "", + "pid": 12345, + } + ) + with patch.dict(systemd.__salt__, {"cmd.run_all": mock_not_run}): + with patch.object(systemd, "masked", self.mock_false): + # Test not masked (should take no action and return True) + self.assertTrue(systemd.unmask_(self.unit_name)) + # Also should not have called cmd.run_all + self.assertTrue(mock_not_run.call_count == 0) - with patch.object(systemd, "masked", masked_mock): + with patch.object(systemd, "masked", masked_mock): - # Has scopes available - with patch.object(salt.utils.systemd, "has_scope", self.mock_true): + # Has scopes available + with patch.object(salt.utils.systemd, "has_scope", self.mock_true): - # Scope enabled, successful - with patch.dict( - systemd.__salt__, - { - "config.get": self.mock_true, - "cmd.run_all": self.mock_run_all_success, - }, - ): - ret = func(*args) - self.assertTrue(ret) - self.mock_run_all_success.assert_called_with( - scope_prefix + systemctl_command, - python_shell=False, - redirect_stderr=True, - ) - - # Scope enabled, failed - with patch.dict( - systemd.__salt__, - { - "config.get": self.mock_true, - "cmd.run_all": self.mock_run_all_failure, - }, - ): - self.assertRaises(CommandExecutionError, func, *args) - self.mock_run_all_failure.assert_called_with( - scope_prefix + systemctl_command, - python_shell=False, - redirect_stderr=True, - ) - - # Scope disabled, successful - with patch.dict( - systemd.__salt__, - { - "config.get": self.mock_false, - "cmd.run_all": self.mock_run_all_success, - }, - ): - ret = func(*args) - self.assertTrue(ret) - self.mock_run_all_success.assert_called_with( - systemctl_command, python_shell=False, redirect_stderr=True - ) - - # Scope disabled, failed - with patch.dict( - systemd.__salt__, - { - "config.get": self.mock_false, - "cmd.run_all": self.mock_run_all_failure, - }, - ): - self.assertRaises(CommandExecutionError, func, *args) - self.mock_run_all_failure.assert_called_with( - systemctl_command, python_shell=False, redirect_stderr=True - ) - - # Does not have scopes available - with patch.object(salt.utils.systemd, "has_scope", self.mock_false): - - # The results should be the same irrespective of - # whether or not scope is enabled, since scope is not - # available, so we repeat the below tests with it both - # enabled and disabled. - for scope_mock in (self.mock_true, self.mock_false): - - # Successful + # Scope enabled, successful with patch.dict( systemd.__salt__, { - "config.get": scope_mock, + "config.get": self.mock_true, + "cmd.run_all": self.mock_run_all_success, + }, + ): + ret = func(*args) + self.assertTrue(ret) + self.mock_run_all_success.assert_called_with( + scope_prefix + systemctl_command, + python_shell=False, + redirect_stderr=True, + ) + + # Scope enabled, failed + with patch.dict( + systemd.__salt__, + { + "config.get": self.mock_true, + "cmd.run_all": self.mock_run_all_failure, + }, + ): + self.assertRaises(CommandExecutionError, func, *args) + self.mock_run_all_failure.assert_called_with( + scope_prefix + systemctl_command, + python_shell=False, + redirect_stderr=True, + ) + + # Scope disabled, successful + with patch.dict( + systemd.__salt__, + { + "config.get": self.mock_false, "cmd.run_all": self.mock_run_all_success, }, ): @@ -627,11 +597,11 @@ class SystemdScopeTestCase(TestCase, LoaderModuleMockMixin): redirect_stderr=True, ) - # Failed + # Scope disabled, failed with patch.dict( systemd.__salt__, { - "config.get": scope_mock, + "config.get": self.mock_false, "cmd.run_all": self.mock_run_all_failure, }, ): @@ -642,6 +612,46 @@ class SystemdScopeTestCase(TestCase, LoaderModuleMockMixin): redirect_stderr=True, ) + # Does not have scopes available + with patch.object(salt.utils.systemd, "has_scope", self.mock_false): + + # The results should be the same irrespective of + # whether or not scope is enabled, since scope is not + # available, so we repeat the below tests with it both + # enabled and disabled. + for scope_mock in (self.mock_true, self.mock_false): + + # Successful + with patch.dict( + systemd.__salt__, + { + "config.get": scope_mock, + "cmd.run_all": self.mock_run_all_success, + }, + ): + ret = func(*args) + self.assertTrue(ret) + self.mock_run_all_success.assert_called_with( + systemctl_command, + python_shell=False, + redirect_stderr=True, + ) + + # Failed + with patch.dict( + systemd.__salt__, + { + "config.get": scope_mock, + "cmd.run_all": self.mock_run_all_failure, + }, + ): + self.assertRaises(CommandExecutionError, func, *args) + self.mock_run_all_failure.assert_called_with( + systemctl_command, + python_shell=False, + redirect_stderr=True, + ) + def test_start(self): self._change_state("start", no_block=False) self._change_state("start", no_block=True) @@ -690,9 +700,10 @@ class SystemdScopeTestCase(TestCase, LoaderModuleMockMixin): salt_mock = { "cmd.run_all": MagicMock(return_value=result), } - with patch.dict(systemd.__salt__, salt_mock): - assert systemd.firstboot() - salt_mock["cmd.run_all"].assert_called_with(["systemd-firstboot"]) + with patch("salt.utils.path.which", lambda x: "/bin/" + x): + with patch.dict(systemd.__salt__, salt_mock): + assert systemd.firstboot() + salt_mock["cmd.run_all"].assert_called_with(["/bin/systemd-firstboot"]) def test_firstboot_params(self): """ @@ -702,35 +713,36 @@ class SystemdScopeTestCase(TestCase, LoaderModuleMockMixin): salt_mock = { "cmd.run_all": MagicMock(return_value=result), } - with patch.dict(systemd.__salt__, salt_mock): - assert systemd.firstboot( - locale="en_US.UTF-8", - locale_message="en_US.UTF-8", - keymap="jp", - timezone="Europe/Berlin", - hostname="node-001", - machine_id="1234567890abcdef", - root="/mnt", - ) - salt_mock["cmd.run_all"].assert_called_with( - [ - "systemd-firstboot", - "--locale", - "en_US.UTF-8", - "--locale-message", - "en_US.UTF-8", - "--keymap", - "jp", - "--timezone", - "Europe/Berlin", - "--hostname", - "node-001", - "--machine-ID", - "1234567890abcdef", - "--root", - "/mnt", - ] - ) + with patch("salt.utils.path.which", lambda x: "/bin/" + x): + with patch.dict(systemd.__salt__, salt_mock): + assert systemd.firstboot( + locale="en_US.UTF-8", + locale_message="en_US.UTF-8", + keymap="jp", + timezone="Europe/Berlin", + hostname="node-001", + machine_id="1234567890abcdef", + root="/mnt", + ) + salt_mock["cmd.run_all"].assert_called_with( + [ + "/bin/systemd-firstboot", + "--locale", + "en_US.UTF-8", + "--locale-message", + "en_US.UTF-8", + "--keymap", + "jp", + "--timezone", + "Europe/Berlin", + "--hostname", + "node-001", + "--machine-ID", + "1234567890abcdef", + "--root", + "/mnt", + ] + ) def test_firstboot_error(self): """