From db72c841c2ec0d94d366df7fa623e82e91e2201c Mon Sep 17 00:00:00 2001
From: Andrew Bogott <Andrewbogott@gmail.com>
Date: Wed, 1 Sep 2021 09:50:37 -0500
Subject: puppet config: add the start_agent option (#1002)

The current code starts the puppet agent and also sets autostart
in all cases. This conflicts with a common pattern where puppet
itself manages the agent and autostart state.

For example, in my deploy puppet disables the puppet agent
and replaces it with a cron. This causes various races
both within this cloud-init unit and within puppet itself
while cloud-init and puppet fight over whether or not
to enable the service.
---
 cloudinit/config/cc_puppet.py                      | 23 +++++++++++++++-------
 .../unittests/test_handler/test_handler_puppet.py  | 22 +++++++++++++++++++++
 2 files changed, 38 insertions(+), 7 deletions(-)

diff --git a/cloudinit/config/cc_puppet.py b/cloudinit/config/cc_puppet.py
index a0779eb0..dc20fc44 100644
--- a/cloudinit/config/cc_puppet.py
+++ b/cloudinit/config/cc_puppet.py
@@ -59,10 +59,13 @@ Additionally it's possible to create a ``csr_attributes.yaml`` file for CSR
 attributes and certificate extension requests.
 See https://puppet.com/docs/puppet/latest/config_file_csr_attributes.html
 
-The puppet service will be automatically enabled after installation. A manual
-run can also be triggered by setting ``exec`` to ``true``, and additional
-arguments can be passed to ``puppet agent`` via the ``exec_args`` key (by
-default the agent will execute with the ``--test`` flag).
+By default, the puppet service will be automatically enabled after installation
+and set to automatically start on boot. To override this in favor of manual
+puppet execution set ``start_service`` to ``false``.
+
+A single manual run can be triggered by setting ``exec`` to ``true``, and
+additional arguments can be passed to ``puppet agent`` via the ``exec_args``
+key (by default the agent will execute with the ``--test`` flag).
 
 **Internal name:** ``cc_puppet``
 
@@ -85,6 +88,7 @@ default the agent will execute with the ``--test`` flag).
         package_name: 'puppet'
         exec: <true/false>
         exec_args: ['--test']
+        start_service: <true/false>
         conf:
             agent:
                 server: "puppetserver.example.org"
@@ -197,6 +201,9 @@ def handle(name, cfg, cloud, log, _args):
         puppet_cfg, 'install_type', 'packages')
     cleanup = util.get_cfg_option_bool(puppet_cfg, 'cleanup', True)
     run = util.get_cfg_option_bool(puppet_cfg, 'exec', default=False)
+    start_puppetd = util.get_cfg_option_bool(puppet_cfg,
+                                             'start_service',
+                                             default=True)
     aio_install_url = util.get_cfg_option_str(
         puppet_cfg, 'aio_install_url', default=AIO_INSTALL_URL)
 
@@ -291,7 +298,8 @@ def handle(name, cfg, cloud, log, _args):
                                   default_flow_style=False))
 
     # Set it up so it autostarts
-    _autostart_puppet(log)
+    if start_puppetd:
+        _autostart_puppet(log)
 
     # Run the agent if needed
     if run:
@@ -312,7 +320,8 @@ def handle(name, cfg, cloud, log, _args):
             cmd.extend(PUPPET_AGENT_DEFAULT_ARGS)
         subp.subp(cmd, capture=False)
 
-    # Start puppetd
-    subp.subp(['service', 'puppet', 'start'], capture=False)
+    if start_puppetd:
+        # Start puppetd
+        subp.subp(['service', 'puppet', 'start'], capture=False)
 
 # vi: ts=4 expandtab
diff --git a/tests/unittests/test_handler/test_handler_puppet.py b/tests/unittests/test_handler/test_handler_puppet.py
index b7891ab4..19f72a0c 100644
--- a/tests/unittests/test_handler/test_handler_puppet.py
+++ b/tests/unittests/test_handler/test_handler_puppet.py
@@ -277,6 +277,28 @@ class TestPuppetHandle(CiTestCase):
             [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):
+        """Run puppet with default args if 'exec' is set to True."""
+        mycloud = self._get_cloud('ubuntu')
+        cfg = {'puppet': {}}
+        cc_puppet.handle('notimportant', cfg, mycloud, LOG, None)
+        self.assertEqual(1, m_auto.call_count)
+        self.assertIn(
+            [mock.call(['service', 'puppet', 'start'], capture=False)],
+            m_subp.call_args_list)
+
+    @mock.patch('cloudinit.config.cc_puppet.subp.subp', return_value=("", ""))
+    def test_puppet_skips_puppetd(self, m_subp, m_auto):
+        """Run puppet with default args if 'exec' is set to True."""
+        mycloud = self._get_cloud('ubuntu')
+        cfg = {'puppet': {'start_service': False}}
+        cc_puppet.handle('notimportant', cfg, mycloud, LOG, None)
+        self.assertEqual(0, m_auto.call_count)
+        self.assertNotIn(
+            [mock.call(['service', 'puppet', 'start'], 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):
-- 
cgit v1.2.3