Repository: ansible/ansible
Pull request: Add module parameter to control Puppet agent noop switch
@emonty 

Current module overrides puppet.conf noop setting by forcing '--no-noop' switch which may not be expected or desired.

 Adds parameter to allow user to control noop mode, or honour local puppet agent configuration.

`noop: None` - Use agent configuration

`noop: true` - Add --noop switch to cmd
`noop: false` - Add --no-noop switch to cmd

*Note:* Ansible check mode always runs with --noop switch

Fixes: #43686


Problem statement:
Add module parameter to control Puppet agent noop switch 
##### SUMMARY
Current puppet module overrides `puppet.conf` noop setting by forcing '--no-noop' switch which may not be expected or desired.

Add a parameter to allow user to control noop mode, or honour local puppet agent configuration.

noop: Undefined - Use agent configuration
noop: true - Add --noop switch to cmd
noop: false - Add --no-noop switch to cmd

Note: Ansible check mode always runs with --noop switch

##### ISSUE TYPE
 - Bug Report

##### COMPONENT NAME
puppet module

##### ANSIBLE VERSION
<!--- Paste, BELOW THIS COMMENT, verbatim output from "ansible --version" between quotes below -->
```
ansible 2.6.2
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/home/matonb/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python2.7/site-packages/ansible
  executable location = /usr/bin/ansible
  python version = 2.7.5 (default, Jul 13 2018, 13:06:57) [GCC 4.8.5 20150623 (Red Hat 4.8.5-28)]
```

##### CONFIGURATION
<!--- If using Ansible 2.4 or above, paste, BELOW THIS COMMENT, the results of "ansible-config dump --only-changed"
Otherwise, mention any settings you have changed/added/removed in ansible.cfg
(or using the ANSIBLE_* environment variables).-->
COLOR_VERBOSE(/etc/ansible/ansible.cfg) = bright blue
DEFAULT_ACTION_PLUGIN_PATH(/etc/ansible/ansible.cfg) = [u'/usr/share/ansible/plu
RETRY_FILES_ENABLED(/etc/ansible/ansible.cfg) = False

##### OS / ENVIRONMENT
<!--- Mention, BELOW THIS COMMENT, the OS you are running Ansible from, and the OS you are
managing, or say "N/A" for anything that is not platform-specific.
Also mention the specific version of what you are trying to control,
e.g. if this is a network bug the version of firmware on the network device.-->
N/A

##### STEPS TO REPRODUCE
<!--- For bugs, show exactly how to reproduce the problem, using a minimal test-case.
For new features, show how the feature would be used. -->
Execute puppet on any node with noop enabled in `puppet.conf`.

<!--- Paste example playbooks or commands between quotes below -->
Run puppet agent in noop mode
```yaml
puppet:
  noop: true
```
Run puppet force agent to no-noop mode
```yaml
puppet:
  noop: false
```
Run puppet agent in using host's puppet.conf noop mode
```yaml
puppet:
```

<!--- You can also paste gist.github.com links for larger files -->

##### EXPECTED RESULTS
```yaml
puppet:
```

puppet.conf
```bash
[agent]
noop = true
```

Expected result, no puppet agent runs no configuration changes applied.

<!--- What did you expect to happen when running the steps above? -->

##### ACTUAL RESULTS
<!--- What actually happened? If possible run with extra verbosity (-vvvv) -->

puppet is executed with `--no-noop` switch and configuration changes are applied.

Hints from issue discussion:

Commit message: Puppet agent noop switch

    Add module parameter to control noop switch

    Current module overrides puppet.conf noop setting by forcing '--no-noop' switch  which may not be expected or desired.

    Add parameter to allow user control.

    noop: Undefined - Use agent configuration
    noop: true - Add --noop switch to cmd
    noop: false - Add --no-noop switch to cmd

Ansible check mode always runs with --noop switch

Patch to review:
diff --git a/lib/ansible/modules/system/puppet.py b/lib/ansible/modules/system/puppet.py
index ddd0dd7c4cb36a..60d3bd262ef5f0 100644
--- a/lib/ansible/modules/system/puppet.py
+++ b/lib/ansible/modules/system/puppet.py
@@ -32,6 +32,13 @@
   manifest:
     description:
       - Path to the manifest file to run puppet apply on.
+  noop:
+    description:
+      - Override puppet.conf noop mode.
+      - Undefined, use default or puppet.conf value if defined.
+      - true, Run puppet agent with --noop switch set.
+      - false, Run puppet agent with --no-noop switch set.
+    version_added: "2.7"
   facts:
     description:
       - A dict of values to pass in as persistent external facter facts.
@@ -104,6 +111,10 @@
   puppet:
     tags: update,nginx
 
+- name: Run puppet agent in noop mode
+  puppet:
+    noop: true
+
 - name: Run a manifest with debug, log to both syslog and stdout, specify module path
   puppet:
     modulepath: /etc/puppet/modules:/opt/stack/puppet-modules:/usr/share/openstack-puppet/modules
@@ -148,6 +159,7 @@ def main():
             puppetmaster=dict(type='str'),
             modulepath=dict(type='str'),
             manifest=dict(type='str'),
+            noop=dict(required=False, type='bool'),
             logdest=dict(type='str', default='stdout', choices=['stdout',
                                                                 'syslog',
                                                                 'all']),
@@ -230,8 +242,11 @@ def main():
             cmd += " --certname='%s'" % p['certname']
         if module.check_mode:
             cmd += " --noop"
-        else:
-            cmd += " --no-noop"
+        elif 'noop' in p:
+            if p['noop']:
+                cmd += " --noop"
+            else:
+                cmd += " --no-noop"
     else:
         cmd = "%s apply --detailed-exitcodes " % base_cmd
         if p['logdest'] == 'syslog':
@@ -248,8 +263,11 @@ def main():
             cmd += " --tags '%s'" % ','.join(p['tags'])
         if module.check_mode:
             cmd += "--noop "
-        else:
-            cmd += "--no-noop "
+        elif 'noop' in p:
+            if p['noop']:
+                cmd += " --noop"
+            else:
+                cmd += " --no-noop"
         if p['execute']:
             cmd += " --execute '%s'" % p['execute']
         else:


Code review comments:
diff_hunk: @@ -32,6 +32,13 @@
   manifest:
     description:
       - Path to the manifest file to run puppet apply on.
+  noop:
+    description:
+      - Override puppet.conf noop mode.
+      - Undefined, use default or puppet.conf value if defined.
+      - true, Run puppet agent with --noop switch set.
+      - false, Run puppet agent with --no-noop switch set.
+    version_added: "2.7"
path: lib/ansible/modules/system/puppet.py
text: @user1:
```suggestion
    version_added: "2.8"
```

diff_hunk: @@ -32,6 +32,13 @@
   manifest:
     description:
       - Path to the manifest file to run puppet apply on.
+  noop:
+    description:
+      - Override puppet.conf noop mode.
+      - Undefined, use default or puppet.conf value if defined.
+      - true, Run puppet agent with --noop switch set.
+      - false, Run puppet agent with --no-noop switch set.
path: lib/ansible/modules/system/puppet.py
text: @user1:
```suggestion
      - false, Run Puppet agent with C(--no-noop) switch set.
```