nagios: some refactoring (#5239) (#5246)

* nagios: some refactoring

* rollback one change

* add changelog fragment

* Update changelogs/fragments/5239-nagios-refactor.yaml

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update plugins/modules/monitoring/nagios.py

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update plugins/modules/monitoring/nagios.py

Co-authored-by: Felix Fontein <felix@fontein.de>

Co-authored-by: Felix Fontein <felix@fontein.de>
(cherry picked from commit 570445adc4)

Co-authored-by: Alexei Znamensky <103110+russoz@users.noreply.github.com>

changelogs/fragments/5239-nagios-refactor.yaml

@@ -0,0 +1,2 @@
+minor_changes:
+  - nagios - minor refactoring on parameter validation for different actions (https://github.com//pull/5239).

plugins/modules/monitoring/nagios.py

@@ -252,8 +252,6 @@ import stat
 from ansible.module_utils.basic import AnsibleModule
 
 
-######################################################################
-
 def which_cmdfile():
     locations = [
         # rhel
@@ -287,8 +285,6 @@ def which_cmdfile():
 
     return None
 
-######################################################################
-
 
 def main():
     ACTION_CHOICES = [
@@ -309,95 +305,42 @@ def main():
 
     module = AnsibleModule(
         argument_spec=dict(
-            action=dict(required=True, choices=ACTION_CHOICES),
+            action=dict(type='str', required=True, choices=ACTION_CHOICES),
-            author=dict(default='Ansible'),
+            author=dict(type='str', default='Ansible'),
-            comment=dict(default='Scheduling downtime'),
+            comment=dict(type='str', default='Scheduling downtime'),
-            host=dict(required=False, default=None),
+            host=dict(type='str'),
-            servicegroup=dict(required=False, default=None),
+            servicegroup=dict(type='str'),
-            start=dict(required=False, default=None),
+            start=dict(type='str'),
-            minutes=dict(default=30, type='int'),
+            minutes=dict(type='int', default=30),
-            cmdfile=dict(default=which_cmdfile()),
+            cmdfile=dict(type='str', default=which_cmdfile()),
-            services=dict(default=None, aliases=['service']),
+            services=dict(type='str', aliases=['service']),
-            command=dict(required=False, default=None),
+            command=dict(type='str'),
-        )
+        ),
+        required_if=[
+            ('action', 'downtime', ['host', 'services']),
+            ('action', 'delete_downtime', ['host', 'services']),
+            ('action', 'silence', ['host']),
+            ('action', 'unsilence', ['host']),
+            ('action', 'enable_alerts', ['host', 'services']),
+            ('action', 'disable_alerts', ['host', 'services']),
+            ('action', 'command', ['command']),
+            ('action', 'servicegroup_host_downtime', ['host', 'servicegroup']),
+            ('action', 'servicegroup_service_downtime', ['host', 'servicegroup']),
+            ('action', 'acknowledge', ['host', 'services']),
+            ('action', 'forced_check', ['host', 'services']),
+        ],
     )
 
-    action = module.params['action']
+    if not module.params['cmdfile']:
-    host = module.params['host']
-    servicegroup = module.params['servicegroup']
-    start = module.params['start']
-    services = module.params['services']
-    cmdfile = module.params['cmdfile']
-    command = module.params['command']
-
-    ##################################################################
-    # Required args per action:
-    # downtime = (minutes, service, host)
-    # acknowledge = (service, host)
-    # (un)silence = (host)
-    # (enable/disable)_alerts = (service, host)
-    # command = command
-    #
-    # AnsibleModule will verify most stuff, we need to verify
-    # 'service' manually.
-
-    ##################################################################
-    if action not in ['command', 'silence_nagios', 'unsilence_nagios']:
-        if not host:
-            module.fail_json(msg='no host specified for action requiring one')
-    ######################################################################
-    if action == 'downtime':
-        # Make sure there's an actual service selected
-        if not services:
-            module.fail_json(msg='no service selected to set downtime for')
-
-    ######################################################################
-    if action == 'delete_downtime':
-        # Make sure there's an actual service selected
-        if not services:
-            module.fail_json(msg='no service selected to set downtime for')
-
-    ######################################################################
-
-    if action in ['servicegroup_service_downtime', 'servicegroup_host_downtime']:
-        # Make sure there's an actual servicegroup selected
-        if not servicegroup:
-            module.fail_json(msg='no servicegroup selected to set downtime for')
-
-    ##################################################################
-    if action in ['enable_alerts', 'disable_alerts']:
-        if not services:
-            module.fail_json(msg='a service is required when setting alerts')
-
-    if action in ['command']:
-        if not command:
-            module.fail_json(msg='no command passed for command action')
-    ######################################################################
-    if action == 'acknowledge':
-        # Make sure there's an actual service selected
-        if not services:
-            module.fail_json(msg='no service selected to acknowledge')
-
-    ##################################################################
-    if action == 'forced_check':
-        # Make sure there's an actual service selected
-        if not services:
-            module.fail_json(msg='no service selected to check')
-
-    ##################################################################
-    if not cmdfile:
         module.fail_json(msg='unable to locate nagios.cfg')
 
-    ##################################################################
     ansible_nagios = Nagios(module, **module.params)
     if module.check_mode:
         module.exit_json(changed=True)
     else:
         ansible_nagios.act()
-    ##################################################################
 
 
-######################################################################
 class Nagios(object):
     """
     Perform common tasks in Nagios related to downtime and
@@ -454,10 +397,9 @@ class Nagios(object):
             self.module.fail_json(msg='nagios command file is not a fifo file',
                                   cmdfile=self.cmdfile)
         try:
-            fp = open(self.cmdfile, 'w')
+            with open(self.cmdfile, 'w') as fp:
-            fp.write(cmd)
+                fp.write(cmd)
-            fp.flush()
+                fp.flush()
-            fp.close()
             self.command_results.append(cmd.strip())
         except IOError:
             self.module.fail_json(msg='unable to write to nagios command file',