diff options
author | d1r3ct0r <calvin.mwadime@canonical.com> | 2023-02-01 22:36:07 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-02-01 12:36:07 -0700 |
commit | f447bab61d02e78e1a10becde38219a7537d548c (patch) | |
tree | 89baae9e098f1acf1d1d6a24ef732b813ed77fbd | |
parent | e1e1abbb08575c3deb4c9ef83d1d21c76638f2dc (diff) | |
download | cloud-init-git-f447bab61d02e78e1a10becde38219a7537d548c.tar.gz |
cc_puppet: Update puppet service name (#1970)
cc_puppet: Update puppet service name to puppet-agent
From Lunar, we see that the default puppet version is 7.20
which replaces `puppet.service` with `puppet-agent.service`.
Thus, we need to have a way of calling the appropriate
service depending on the distribution of puppet installed.
Attempt to install, start or enable puppet-agent first and fallback
to puppet.
Log warnings if neither preferred package names exist or if the
package_name in user-data is not able to be configured.
LP: #2002969
-rw-r--r-- | cloudinit/config/cc_puppet.py | 59 | ||||
-rw-r--r-- | tests/integration_tests/modules/test_puppet.py | 7 | ||||
-rw-r--r-- | tests/unittests/config/test_cc_puppet.py | 194 |
3 files changed, 157 insertions, 103 deletions
diff --git a/cloudinit/config/cc_puppet.py b/cloudinit/config/cc_puppet.py index b8a9fe17..38c2cc99 100644 --- a/cloudinit/config/cc_puppet.py +++ b/cloudinit/config/cc_puppet.py @@ -25,6 +25,7 @@ from cloudinit.settings import PER_INSTANCE AIO_INSTALL_URL = "https://raw.githubusercontent.com/puppetlabs/install-puppet/main/install.sh" # noqa: E501 PUPPET_AGENT_DEFAULT_ARGS = ["--test"] +PUPPET_PACKAGE_NAMES = ("puppet-agent", "puppet") MODULE_DESCRIPTION = """\ This module handles puppet installation and configuration. If the ``puppet`` @@ -118,26 +119,21 @@ class PuppetConstants: self.csr_attributes_path = csr_attributes_path -def _autostart_puppet(log): - # Set puppet to automatically start - if os.path.exists("/etc/default/puppet"): - subp.subp( - [ - "sed", - "-i", - "-e", - "s/^START=.*/START=yes/", - "/etc/default/puppet", - ], - capture=False, - ) - elif subp.which("systemctl"): - subp.subp(["systemctl", "enable", "puppet.service"], capture=False) - elif os.path.exists("/sbin/chkconfig"): - subp.subp(["/sbin/chkconfig", "puppet", "on"], capture=False) - else: +def _manage_puppet_services(log, cloud: Cloud, action: str): + """Attempts to perform action on one of the puppet services""" + service_managed: str = "" + for puppet_name in PUPPET_PACKAGE_NAMES: + try: + cloud.distro.manage_service(action, f"{puppet_name}.service") + service_managed = puppet_name + break + except subp.ProcessExecutionError: + pass + if not service_managed: log.warning( - "Sorry we do not know how to enable puppet services on this system" + "Could not '%s' any of the following services: %s", + action, + ", ".join(PUPPET_PACKAGE_NAMES), ) @@ -221,7 +217,7 @@ def handle( else: # default to 'packages' puppet_user = "puppet" puppet_bin = "puppet" - puppet_package = "puppet" + puppet_package = None # changes with distro package_name = util.get_cfg_option_str( puppet_cfg, "package_name", puppet_package @@ -238,7 +234,22 @@ def handle( ) if install_type == "packages": - cloud.distro.install_packages((package_name, version)) + if package_name is None: # conf has no package_nam + for puppet_name in PUPPET_PACKAGE_NAMES: + try: + cloud.distro.install_packages((puppet_name, version)) + package_name = puppet_name + break + except subp.ProcessExecutionError: + pass + if not package_name: + log.warning( + "No installable puppet package in any of: %s", + ", ".join(PUPPET_PACKAGE_NAMES), + ) + else: + cloud.distro.install_packages((package_name, version)) + elif install_type == "aio": install_puppet_aio( cloud.distro, aio_install_url, version, collection, cleanup @@ -316,9 +327,9 @@ def handle( yaml.dump(puppet_cfg["csr_attributes"], default_flow_style=False), ) - # Set it up so it autostarts if start_puppetd: - _autostart_puppet(log) + # Enables the services + _manage_puppet_services(log, cloud, "enable") # Run the agent if needed if run: @@ -344,7 +355,7 @@ def handle( if start_puppetd: # Start puppetd - subp.subp(["service", "puppet", "start"], capture=False) + _manage_puppet_services(log, cloud, "start") # vi: ts=4 expandtab diff --git a/tests/integration_tests/modules/test_puppet.py b/tests/integration_tests/modules/test_puppet.py index 1bd9cee4..b8613866 100644 --- a/tests/integration_tests/modules/test_puppet.py +++ b/tests/integration_tests/modules/test_puppet.py @@ -17,7 +17,12 @@ def test_puppet_service(client: IntegrationInstance): """Basic test that puppet gets installed and runs.""" log = client.read_from_file("/var/log/cloud-init.log") verify_clean_log(log) - assert client.execute("systemctl is-active puppet").ok + puppet_ok = client.execute("systemctl is-active puppet.service").ok + puppet_agent_ok = client.execute( + "systemctl is-active puppet-agent.service" + ).ok + assert True in [puppet_ok, puppet_agent_ok] + assert False in [puppet_ok, puppet_agent_ok] assert "Running command ['puppet', 'agent'" not in log diff --git a/tests/unittests/config/test_cc_puppet.py b/tests/unittests/config/test_cc_puppet.py index 27a49722..23461c2b 100644 --- a/tests/unittests/config/test_cc_puppet.py +++ b/tests/unittests/config/test_cc_puppet.py @@ -12,6 +12,7 @@ from cloudinit.config.schema import ( get_schema, validate_cloudconfig_schema, ) +from cloudinit.subp import ProcessExecutionError from tests.unittests.helpers import CiTestCase, mock, skipUnlessJsonSchema from tests.unittests.util import get_cloud @@ -25,67 +26,55 @@ def fake_tempdir(mocker, tmpdir): ).return_value.__enter__.return_value = str(tmpdir) -@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 - ): - """Update /etc/default/puppet to autostart if it exists.""" - - def _fake_exists(path): - return path == "/etc/default/puppet" - - m_os.path.exists.side_effect = _fake_exists - cc_puppet._autostart_puppet(LOG) - self.assertEqual( - [ - mock.call( - [ - "sed", - "-i", - "-e", - "s/^START=.*/START=yes/", - "/etc/default/puppet", - ], - capture=False, - ) - ], - m_subp.call_args_list, - ) +class TestManagePuppetServices(CiTestCase): + def setUp(self): + super(TestManagePuppetServices, self).setUp() + self.cloud = get_cloud() - def test_wb_autostart_pupppet_enables_puppet_systemctl( - self, m_os, m_subp, m_subpw + def test_wb_manage_puppet_services_enables_puppet_systemctl( + self, + m_subp, ): - """If systemctl is present, enable puppet via systemctl.""" - - m_os.path.exists.return_value = False - m_subpw.return_value = "/usr/bin/systemctl" - cc_puppet._autostart_puppet(LOG) + cc_puppet._manage_puppet_services(LOG, self.cloud, "enable") expected_calls = [ - mock.call(["systemctl", "enable", "puppet.service"], capture=False) + mock.call( + ["systemctl", "enable", "puppet-agent.service"], + capture=True, + ) ] - self.assertEqual(expected_calls, m_subp.call_args_list) + self.assertIn(expected_calls, m_subp.call_args_list) - def test_wb_autostart_pupppet_enables_puppet_chkconfig( - self, m_os, m_subp, m_subpw + def test_wb_manage_puppet_services_starts_puppet_systemctl( + self, + m_subp, ): - """If chkconfig is present, enable puppet via checkcfg.""" - - def _fake_exists(path): - return path == "/sbin/chkconfig" + cc_puppet._manage_puppet_services(LOG, self.cloud, "start") + expected_calls = [ + mock.call( + ["systemctl", "start", "puppet-agent.service"], + capture=True, + ) + ] + self.assertIn(expected_calls, m_subp.call_args_list) - m_subpw.return_value = None - m_os.path.exists.side_effect = _fake_exists - cc_puppet._autostart_puppet(LOG) + def test_enable_fallback_on_failure(self, m_subp): + m_subp.side_effect = (ProcessExecutionError, 0) + cc_puppet._manage_puppet_services(LOG, self.cloud, "enable") expected_calls = [ - mock.call(["/sbin/chkconfig", "puppet", "on"], capture=False) + mock.call( + ["systemctl", "enable", "puppet-agent.service"], + capture=True, + ), + mock.call( + ["systemctl", "enable", "puppet.service"], + capture=True, + ), ] self.assertEqual(expected_calls, m_subp.call_args_list) -@mock.patch("cloudinit.config.cc_puppet._autostart_puppet") +@mock.patch("cloudinit.config.cc_puppet._manage_puppet_services") class TestPuppetHandle(CiTestCase): with_logs = True @@ -97,35 +86,36 @@ class TestPuppetHandle(CiTestCase): self.csr_attributes_path = self.tmp_path("csr_attributes.yaml") self.cloud = get_cloud() - def test_skips_missing_puppet_key_in_cloudconfig(self, m_auto): + def test_skips_missing_puppet_key_in_cloudconfig(self, m_man_puppet): """Cloud-config containing no 'puppet' key is skipped.""" cfg = {} cc_puppet.handle("notimportant", cfg, self.cloud, LOG, None) self.assertIn("no 'puppet' configuration found", self.logs.getvalue()) - self.assertEqual(0, m_auto.call_count) + self.assertEqual(0, m_man_puppet.call_count) @mock.patch("cloudinit.config.cc_puppet.subp.subp", return_value=("", "")) - def test_puppet_config_starts_puppet_service(self, m_subp, m_auto): + def test_puppet_config_starts_puppet_service(self, m_subp, m_man_puppet): """Cloud-config 'puppet' configuration starts puppet.""" cfg = {"puppet": {"install": False}} cc_puppet.handle("notimportant", cfg, self.cloud, LOG, None) - self.assertEqual(1, m_auto.call_count) - self.assertIn( - [mock.call(["service", "puppet", "start"], capture=False)], - m_subp.call_args_list, - ) + self.assertEqual(2, m_man_puppet.call_count) + expected_calls = [ + mock.call(LOG, self.cloud, "enable"), + mock.call(LOG, self.cloud, "start"), + ] + self.assertEqual(expected_calls, m_man_puppet.call_args_list) @mock.patch("cloudinit.config.cc_puppet.subp.subp", return_value=("", "")) - def test_empty_puppet_config_installs_puppet(self, m_subp, m_auto): + def test_empty_puppet_config_installs_puppet(self, m_subp, m_man_puppet): """Cloud-config empty 'puppet' configuration installs latest puppet.""" self.cloud.distro = mock.MagicMock() cfg = {"puppet": {}} cc_puppet.handle("notimportant", cfg, self.cloud, LOG, None) self.assertEqual( - [mock.call(("puppet", None))], + [mock.call(("puppet-agent", None))], self.cloud.distro.install_packages.call_args_list, ) @@ -136,8 +126,8 @@ class TestPuppetHandle(CiTestCase): self.cloud.distro = mock.MagicMock() cfg = {"puppet": {"install": True}} cc_puppet.handle("notimportant", cfg, self.cloud, LOG, None) - self.assertEqual( - [mock.call(("puppet", None))], + self.assertIn( + [mock.call(("puppet-agent", None))], self.cloud.distro.install_packages.call_args_list, ) @@ -246,14 +236,14 @@ class TestPuppetHandle(CiTestCase): cfg = {"puppet": {"version": "3.8"}} cc_puppet.handle("notimportant", cfg, self.cloud, LOG, None) self.assertEqual( - [mock.call(("puppet", "3.8"))], + [mock.call(("puppet-agent", "3.8"))], self.cloud.distro.install_packages.call_args_list, ) @mock.patch("cloudinit.config.cc_puppet.get_config_value") @mock.patch("cloudinit.config.cc_puppet.subp.subp", return_value=("", "")) def test_puppet_config_updates_puppet_conf( - self, m_subp, m_default, m_auto + self, m_subp, m_default, m_man_puppet ): """When 'conf' is provided update values in PUPPET_CONF_PATH.""" @@ -277,7 +267,7 @@ class TestPuppetHandle(CiTestCase): @mock.patch("cloudinit.config.cc_puppet.get_config_value") @mock.patch("cloudinit.config.cc_puppet.subp.subp") def test_puppet_writes_csr_attributes_file( - self, m_subp, m_default, m_auto + self, m_subp, m_default, m_man_puppet ): """When csr_attributes is provided creates file in PUPPET_CSR_ATTRIBUTES_PATH.""" @@ -321,44 +311,50 @@ class TestPuppetHandle(CiTestCase): self.assertEqual(expected, content) @mock.patch("cloudinit.config.cc_puppet.subp.subp", return_value=("", "")) - def test_puppet_runs_puppet_if_requested(self, m_subp, m_auto): + def test_puppet_runs_puppet_if_requested(self, m_subp, m_man_puppet): """Run puppet with default args if 'exec' is set to True.""" cfg = {"puppet": {"exec": True}} cc_puppet.handle("notimportant", cfg, self.cloud, LOG, None) - self.assertEqual(1, m_auto.call_count) + self.assertEqual(2, m_man_puppet.call_count) + expected_calls = [ + mock.call(LOG, self.cloud, "enable"), + mock.call(LOG, self.cloud, "start"), + ] + self.assertEqual(expected_calls, m_man_puppet.call_args_list) self.assertIn( [mock.call(["puppet", "agent", "--test"], capture=False)], m_subp.call_args_list, ) @mock.patch("cloudinit.config.cc_puppet.subp.subp", return_value=("", "")) - def test_puppet_starts_puppetd(self, m_subp, m_auto): + def test_puppet_starts_puppetd(self, m_subp, m_man_puppet): """Run puppet with default args if 'exec' is set to True.""" cfg = {"puppet": {}} cc_puppet.handle("notimportant", cfg, self.cloud, LOG, None) - self.assertEqual(1, m_auto.call_count) - self.assertIn( - [mock.call(["service", "puppet", "start"], capture=False)], - m_subp.call_args_list, - ) + self.assertEqual(2, m_man_puppet.call_count) + expected_calls = [ + mock.call(LOG, self.cloud, "enable"), + mock.call(LOG, self.cloud, "start"), + ] + self.assertEqual(expected_calls, m_man_puppet.call_args_list) @mock.patch("cloudinit.config.cc_puppet.subp.subp", return_value=("", "")) - def test_puppet_skips_puppetd(self, m_subp, m_auto): + def test_puppet_skips_puppetd(self, m_subp, m_man_puppet): """Run puppet with default args if 'exec' is set to True.""" cfg = {"puppet": {"start_service": False}} cc_puppet.handle("notimportant", cfg, self.cloud, LOG, None) - self.assertEqual(0, m_auto.call_count) + self.assertEqual(0, m_man_puppet.call_count) self.assertNotIn( - [mock.call(["service", "puppet", "start"], capture=False)], + [mock.call(["systemctl", "start", "puppet-agent"], capture=False)], m_subp.call_args_list, ) @mock.patch("cloudinit.config.cc_puppet.subp.subp", return_value=("", "")) def test_puppet_runs_puppet_with_args_list_if_requested( - self, m_subp, m_auto + self, m_subp, m_man_puppet ): """Run puppet with 'exec_args' list if 'exec' is set to True.""" @@ -369,7 +365,7 @@ class TestPuppetHandle(CiTestCase): } } cc_puppet.handle("notimportant", cfg, self.cloud, LOG, None) - self.assertEqual(1, m_auto.call_count) + self.assertEqual(2, m_man_puppet.call_count) self.assertIn( [ mock.call( @@ -382,7 +378,7 @@ class TestPuppetHandle(CiTestCase): @mock.patch("cloudinit.config.cc_puppet.subp.subp", return_value=("", "")) def test_puppet_runs_puppet_with_args_string_if_requested( - self, m_subp, m_auto + self, m_subp, m_man_puppet ): """Run puppet with 'exec_args' string if 'exec' is set to True.""" @@ -393,7 +389,7 @@ class TestPuppetHandle(CiTestCase): } } cc_puppet.handle("notimportant", cfg, self.cloud, LOG, None) - self.assertEqual(1, m_auto.call_count) + self.assertEqual(2, m_man_puppet.call_count) self.assertIn( [ mock.call( @@ -404,6 +400,48 @@ class TestPuppetHandle(CiTestCase): m_subp.call_args_list, ) + @mock.patch("cloudinit.config.cc_puppet.subp.subp", return_value=("", "")) + def test_puppet_falls_back_to_older_name(self, m_subp, m_man_puppet): + cfg = {"puppet": {}} + with mock.patch( + "tests.unittests.util.MockDistro.install_packages" + ) as install_pkg: + # puppet-agent not installed, but puppet is + install_pkg.side_effect = (ProcessExecutionError, 0) + + cc_puppet.handle("notimportant", cfg, self.cloud, LOG, None) + expected_calls = [ + mock.call(LOG, self.cloud, "enable"), + mock.call(LOG, self.cloud, "start"), + ] + self.assertEqual(expected_calls, m_man_puppet.call_args_list) + + @mock.patch("cloudinit.config.cc_puppet.subp.subp", return_value=("", "")) + def test_puppet_with_conf_package_name_fails(self, m_subp, m_man_puppet): + cfg = {"puppet": {"package_name": "puppet"}} + with mock.patch( + "tests.unittests.util.MockDistro.install_packages" + ) as install_pkg: + # puppet-agent not installed, but puppet is + install_pkg.side_effect = (ProcessExecutionError, 0) + with pytest.raises(ProcessExecutionError): + cc_puppet.handle("notimportant", cfg, self.cloud, LOG, None) + self.assertEqual(0, m_man_puppet.call_count) + self.assertNotIn( + [ + mock.call( + ["systemctl", "start", "puppet-agent"], capture=True + ) + ], + m_subp.call_args_list, + ) + + @mock.patch("cloudinit.config.cc_puppet.subp.subp", return_value=("", "")) + def test_puppet_with_conf_package_name_success(self, m_subp, m_man_puppet): + cfg = {"puppet": {"package_name": "puppet"}} + cc_puppet.handle("notimportant", cfg, self.cloud, LOG, None) + self.assertEqual(2, m_man_puppet.call_count) + URL_MOCK = mock.Mock() URL_MOCK.contents = b'#!/bin/bash\necho "Hi Mom"' |