From 12ddfaf1676cbe07c5142949981eb041462c7ea2 Mon Sep 17 00:00:00 2001 From: Robert Schweikert Date: Thu, 17 Feb 2022 12:21:51 -0500 Subject: [PATCH 1/4] It is reasonable to expect that "systemctl" is found in one of the locations in the PATH + Using the '/bin' prefix is very distribution specific. A number of distributions are moving all executables from '/' to '/usr'. --- cloudinit/analyze/show.py | 2 +- cloudinit/config/cc_puppet.py | 4 ++-- cloudinit/sources/helpers/azure.py | 4 ++-- tests/unittests/config/test_cc_puppet.py | 8 +++----- 4 files changed, 8 insertions(+), 10 deletions(-) diff --git a/cloudinit/analyze/show.py b/cloudinit/analyze/show.py index 5fd9cdfd5df..e1e340ce9a7 100644 --- a/cloudinit/analyze/show.py +++ b/cloudinit/analyze/show.py @@ -139,7 +139,7 @@ class SystemctlReader(object): def __init__(self, property, parameter=None): self.epoch = None - self.args = ["/bin/systemctl", "show"] + self.args = [subp.which("systemctl"), "show"] if parameter: self.args.append(parameter) self.args.extend(["-p", property]) diff --git a/cloudinit/config/cc_puppet.py b/cloudinit/config/cc_puppet.py index f51f49bcc21..0483c67e8d9 100644 --- a/cloudinit/config/cc_puppet.py +++ b/cloudinit/config/cc_puppet.py @@ -142,9 +142,9 @@ def _autostart_puppet(log): ], capture=False, ) - elif os.path.exists("/bin/systemctl"): + elif subp.which("systemctl"): subp.subp( - ["/bin/systemctl", "enable", "puppet.service"], capture=False + ["systemctl", "enable", "puppet.service"], capture=False ) elif os.path.exists("/sbin/chkconfig"): subp.subp(["/sbin/chkconfig", "puppet", "on"], capture=False) diff --git a/cloudinit/sources/helpers/azure.py b/cloudinit/sources/helpers/azure.py index d07dc3c055c..0a9f1f459e2 100755 --- a/cloudinit/sources/helpers/azure.py +++ b/cloudinit/sources/helpers/azure.py @@ -115,7 +115,7 @@ def get_boot_telemetry(): try: out, _ = subp.subp( - ["/bin/systemctl", "show", "-p", "UserspaceTimestampMonotonic"], + ["systemctl", "show", "-p", "UserspaceTimestampMonotonic"], capture=True, ) tsm = None @@ -140,7 +140,7 @@ def get_boot_telemetry(): try: out, _ = subp.subp( [ - "/bin/systemctl", + "systemctl", "show", "cloud-init-local", "-p", diff --git a/tests/unittests/config/test_cc_puppet.py b/tests/unittests/config/test_cc_puppet.py index 2c4481da7f9..b3d748ffc6b 100644 --- a/tests/unittests/config/test_cc_puppet.py +++ b/tests/unittests/config/test_cc_puppet.py @@ -40,14 +40,11 @@ def _fake_exists(path): def test_wb_autostart_pupppet_enables_puppet_systemctl(self, m_os, m_subp): """If systemctl is present, enable puppet via systemctl.""" - def _fake_exists(path): - return path == "/bin/systemctl" - - m_os.path.exists.side_effect = _fake_exists + m_subp.which.side_effect = '/usr/bin/systemctl' cc_puppet._autostart_puppet(LOG) expected_calls = [ mock.call( - ["/bin/systemctl", "enable", "puppet.service"], capture=False + ["systemctl", "enable", "puppet.service"], capture=False ) ] self.assertEqual(expected_calls, m_subp.call_args_list) @@ -58,6 +55,7 @@ def test_wb_autostart_pupppet_enables_puppet_chkconfig(self, m_os, m_subp): def _fake_exists(path): return path == "/sbin/chkconfig" + m_subp.which.side_effect = None m_os.path.exists.side_effect = _fake_exists cc_puppet._autostart_puppet(LOG) expected_calls = [ From 88e1afd8c5342d683b596f8c8eaacaa46fd99423 Mon Sep 17 00:00:00 2001 From: Robert Schweikert Date: Sat, 26 Feb 2022 07:21:29 -0500 Subject: [PATCH 2/4] Fix unit test for updated check using systemctl form system path --- tests/unittests/config/test_cc_puppet.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/tests/unittests/config/test_cc_puppet.py b/tests/unittests/config/test_cc_puppet.py index b3d748ffc6b..783b00b4d05 100644 --- a/tests/unittests/config/test_cc_puppet.py +++ b/tests/unittests/config/test_cc_puppet.py @@ -9,11 +9,12 @@ LOG = logging.getLogger(__name__) - +@mock.patch("cloudinit.config.cc_puppet.subp.which") @mock.patch("cloudinit.config.cc_puppet.subp.subp") @mock.patch("cloudinit.config.cc_puppet.os") class TestAutostartPuppet(CiTestCase): - def test_wb_autostart_puppet_updates_puppet_default(self, m_os, m_subp): + def test_wb_autostart_puppet_updates_puppet_default( + self, m_os, m_subp, m_subpw): """Update /etc/default/puppet to autostart if it exists.""" def _fake_exists(path): @@ -37,10 +38,12 @@ def _fake_exists(path): m_subp.call_args_list, ) - def test_wb_autostart_pupppet_enables_puppet_systemctl(self, m_os, m_subp): + def test_wb_autostart_pupppet_enables_puppet_systemctl( + self, m_os, m_subp, m_subpw): """If systemctl is present, enable puppet via systemctl.""" - m_subp.which.side_effect = '/usr/bin/systemctl' + m_os.path.exists.return_value = False + m_subpw.return_value = '/usr/bin/systemctl' cc_puppet._autostart_puppet(LOG) expected_calls = [ mock.call( @@ -49,13 +52,14 @@ def test_wb_autostart_pupppet_enables_puppet_systemctl(self, m_os, m_subp): ] self.assertEqual(expected_calls, m_subp.call_args_list) - def test_wb_autostart_pupppet_enables_puppet_chkconfig(self, m_os, m_subp): + def test_wb_autostart_pupppet_enables_puppet_chkconfig( + self, m_os, m_subp, m_subpw): """If chkconfig is present, enable puppet via checkcfg.""" def _fake_exists(path): return path == "/sbin/chkconfig" - m_subp.which.side_effect = None + m_subpw.return_value = None m_os.path.exists.side_effect = _fake_exists cc_puppet._autostart_puppet(LOG) expected_calls = [ From 51d17846400eec1ae1806be4b5f107524fdde019 Mon Sep 17 00:00:00 2001 From: Robert Schweikert Date: Mon, 28 Feb 2022 09:49:47 -0500 Subject: [PATCH 3/4] Fix formatting issues --- tests/unittests/config/test_cc_puppet.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/tests/unittests/config/test_cc_puppet.py b/tests/unittests/config/test_cc_puppet.py index 783b00b4d05..d070ab41146 100644 --- a/tests/unittests/config/test_cc_puppet.py +++ b/tests/unittests/config/test_cc_puppet.py @@ -9,12 +9,14 @@ LOG = logging.getLogger(__name__) + @mock.patch("cloudinit.config.cc_puppet.subp.which") @mock.patch("cloudinit.config.cc_puppet.subp.subp") @mock.patch("cloudinit.config.cc_puppet.os") class TestAutostartPuppet(CiTestCase): def test_wb_autostart_puppet_updates_puppet_default( - self, m_os, m_subp, m_subpw): + self, m_os, m_subp, m_subpw + ): """Update /etc/default/puppet to autostart if it exists.""" def _fake_exists(path): @@ -39,21 +41,21 @@ def _fake_exists(path): ) def test_wb_autostart_pupppet_enables_puppet_systemctl( - self, m_os, m_subp, m_subpw): + self, m_os, m_subp, m_subpw + ): """If systemctl is present, enable puppet via systemctl.""" m_os.path.exists.return_value = False - m_subpw.return_value = '/usr/bin/systemctl' + m_subpw.return_value = "/usr/bin/systemctl" cc_puppet._autostart_puppet(LOG) expected_calls = [ - mock.call( - ["systemctl", "enable", "puppet.service"], capture=False - ) + mock.call(["systemctl", "enable", "puppet.service"], capture=False) ] self.assertEqual(expected_calls, m_subp.call_args_list) def test_wb_autostart_pupppet_enables_puppet_chkconfig( - self, m_os, m_subp, m_subpw): + self, m_os, m_subp, m_subpw + ): """If chkconfig is present, enable puppet via checkcfg.""" def _fake_exists(path): From d8ec8c3c4740a0dae7a387dca2357ea803692166 Mon Sep 17 00:00:00 2001 From: Robert Schweikert Date: Mon, 28 Feb 2022 12:01:26 -0500 Subject: [PATCH 4/4] Fix another style issue --- cloudinit/config/cc_puppet.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/cloudinit/config/cc_puppet.py b/cloudinit/config/cc_puppet.py index 0483c67e8d9..ad6acafe1e3 100644 --- a/cloudinit/config/cc_puppet.py +++ b/cloudinit/config/cc_puppet.py @@ -143,9 +143,7 @@ def _autostart_puppet(log): capture=False, ) elif subp.which("systemctl"): - subp.subp( - ["systemctl", "enable", "puppet.service"], capture=False - ) + subp.subp(["systemctl", "enable", "puppet.service"], capture=False) elif os.path.exists("/sbin/chkconfig"): subp.subp(["/sbin/chkconfig", "puppet", "on"], capture=False) else: