icinga2_feature: fix module in check_mode (#39853)

* icinga2_feature: fix module in check_mode

* modify check_mode logic

Fixes: #39830

Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>

lib/ansible/modules/monitoring/icinga2_feature.py

@@ -1,18 +1,22 @@
 #!/usr/bin/python
 # -*- coding: utf-8 -*-
 
-# (c) 2016, Loic Blot <loic.blot@unix-experience.fr>
+# Copyright (c) 2016, Loic Blot <loic.blot@unix-experience.fr>
+# Copyright (c) 2018, Ansible Project
 # Sponsored by Infopro Digital. http://www.infopro-digital.com/
 # Sponsored by E.T.A.I. http://www.etai.fr/
+#
 # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 
 from __future__ import absolute_import, division, print_function
 __metaclass__ = type
 
 
-ANSIBLE_METADATA = {'metadata_version': '1.1',
+ANSIBLE_METADATA = {
-                    'status': ['preview'],
+    'metadata_version': '1.1',
-                    'supported_by': 'community'}
+    'status': ['preview'],
+    'supported_by': 'community'
+}
 
 
 DOCUMENTATION = '''
@@ -21,20 +25,22 @@ module: icinga2_feature
 
 short_description: Manage Icinga2 feature
 description:
-    - Enable or disable an Icinga2 feature
+    - This module can be used to enable or disable an Icinga2 feature.
 version_added: "2.3"
 author: "Loic Blot (@nerzhul)"
 options:
     name:
-        description:
+      description:
-            - This is the feature name to enable or disable.
+      - This is the feature name to enable or disable.
-        required: True
+      required: True
     state:
-        description:
+      description:
-            - Apply feature state.
+      - If set to C(present) and feature is disabled, then feature is enabled.
-        required: false
+      - If set to C(present) and feature is already enabled, then nothing is changed.
-        choices: [ "present", "absent" ]
+      - If set to C(absent) and feature is enabled, then feature is disabled.
-        default: present
+      - If set to C(absent) and feature is already disabled, then nothing is changed.
+      choices: [ "present", "absent" ]
+      default: present
 '''
 
 EXAMPLES = '''
@@ -42,6 +48,11 @@ EXAMPLES = '''
   icinga2_feature:
     name: ido-pgsql
     state: present
+
+- name: Disable api feature
+  icinga2_feature:
+    name: api
+    state: absent
 '''
 
 RETURN = '''
@@ -49,7 +60,6 @@ RETURN = '''
 '''
 
 import re
-
 from ansible.module_utils.basic import AnsibleModule
 
 
@@ -57,13 +67,13 @@ class Icinga2FeatureHelper:
     def __init__(self, module):
         self.module = module
         self._icinga2 = module.get_bin_path('icinga2', True)
+        self.feature_name = self.module.params['name']
+        self.state = self.module.params['state']
 
     def _exec(self, args):
-        if not self.module.check_mode:
+        cmd = [self._icinga2, 'feature']
-            cmd = [self._icinga2, 'feature']
+        rc, out, err = self.module.run_command(cmd + args, check_rc=True)
-            rc, out, err = self.module.run_command(cmd + args, check_rc=True)
+        return rc, out
-            return rc, out
-        return 0, list()
 
     def manage(self):
         rc, out = self._exec(["list"])
@@ -71,42 +81,36 @@ class Icinga2FeatureHelper:
             self.module.fail_json(msg="Unable to list icinga2 features. "
                                       "Ensure icinga2 is installed and present in binary path.")
 
-        else:
+        # If feature is already in good state, just exit
-            # If feature is already in good state, just exit
+        if (re.search("Disabled features:.* %s[ \n]" % self.feature_name, out) and self.state == "absent") or \
-            if re.search("Disabled features:.* %s[ \n]" % self.module.params["name"], out) \
+                (re.search("Enabled features:.* %s[ \n]" % self.feature_name, out) and self.state == "present"):
-                    and self.module.params["state"] == "absent" or \
+            self.module.exit_json(changed=False)
-                    re.search("Enabled features:.* %s[ \n]" % self.module.params["name"], out) \
-                    and self.module.params["state"] == "present":
-                self.module.exit_json(changed=False)
 
-            if self.module.check_mode:
+        if self.module.check_mode:
-                self.module.exit_json(changed=True)
+            self.module.exit_json(changed=True)
 
-        if self.module.params["state"] == "present":
+        feature_enable_str = "enable" if self.state == "present" else "disable"
-            feature_enable_str = "enable"
-        else:
-            feature_enable_str = "disable"
 
-        rc, out = self._exec([feature_enable_str, self.module.params["name"]])
+        rc, out = self._exec([feature_enable_str, self.feature_name])
 
-        if self.module.params["state"] == "present":
+        change_applied = False
+        if self.state == "present":
             if rc != 0:
-                self.module.fail_json(msg="Fail to %s feature %s. icinga2 command returned %s"
+                self.module.fail_json(msg="Failed to %s feature %s."
-                                      % (feature_enable_str, self.module.params["name"], out))
+                                          " icinga2 command returned %s" % (feature_enable_str,
+                                                                            self.feature_name,
+                                                                            out))
 
             if re.search("already enabled", out) is None:
                 change_applied = True
-            else:
-                change_applied = False
         else:
             if rc == 0:
                 change_applied = True
             # RC is not 0 for this already disabled feature, handle it as no change applied
-            elif re.search("Cannot disable feature '%s'. Target file .* does not exist"
+            elif re.search("Cannot disable feature '%s'. Target file .* does not exist" % self.feature_name, out):
-                           % self.module.params["name"]):
                 change_applied = False
             else:
-                self.module.fail_json(msg="Fail to disable feature. Command returns %s" % out)
+                self.module.fail_json(msg="Failed to disable feature. Command returns %s" % out)
 
         self.module.exit_json(changed=change_applied)
 
@@ -114,8 +118,8 @@ class Icinga2FeatureHelper:
 def main():
     module = AnsibleModule(
         argument_spec=dict(
-            name=dict(required=True, type='str'),
+            name=dict(type='str', required=True),
-            state=dict(required=False, type='str', choices=["present", "absent"], default="present")
+            state=dict(type='str', choices=["present", "absent"], default="present")
         ),
         supports_check_mode=True
     )
@@ -123,5 +127,6 @@ def main():
     module.run_command_environ_update = dict(LANG='C', LC_ALL='C', LC_MESSAGES='C', LC_CTYPE='C')
     Icinga2FeatureHelper(module).manage()
 
+
 if __name__ == '__main__':
     main()

test/units/modules/monitoring/test_icinga2_feature.py

@@ -0,0 +1,96 @@
+# -*- coding: utf-8 -*-
+
+# Copyright (c) 2018, Ansible Project
+# Copyright (c) 2018, Abhijeet Kasurde <akasurde@redhat.com>
+#
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
+
+from ansible.modules.monitoring import icinga2_feature
+from units.modules.utils import AnsibleExitJson, AnsibleFailJson, ModuleTestCase, set_module_args
+from ansible.compat.tests.mock import patch
+from ansible.module_utils import basic
+
+
+def get_bin_path(*args, **kwargs):
+    """Function to return path of icinga2 binary."""
+    return "/bin/icinga2"
+
+
+class TestIcinga2Feature(ModuleTestCase):
+    """Main class for testing icinga2_feature module."""
+
+    def setUp(self):
+        """Setup."""
+        super(TestIcinga2Feature, self).setUp()
+        self.module = icinga2_feature
+        self.mock_get_bin_path = patch.object(basic.AnsibleModule, 'get_bin_path', get_bin_path)
+        self.mock_get_bin_path.start()
+        self.addCleanup(self.mock_get_bin_path.stop)  # ensure that the patching is 'undone'
+
+    def tearDown(self):
+        """Teardown."""
+        super(TestIcinga2Feature, self).tearDown()
+
+    def test_without_required_parameters(self):
+        """Failure must occurs when all parameters are missing."""
+        with self.assertRaises(AnsibleFailJson):
+            set_module_args({})
+            self.module.main()
+
+    def test_enable_feature(self):
+        """Check that result is changed."""
+        set_module_args({
+            'name': 'api',
+        })
+        with patch.object(basic.AnsibleModule, 'run_command') as run_command:
+            run_command.return_value = 0, '', ''  # successful execution, no output
+            with self.assertRaises(AnsibleExitJson) as result:
+                icinga2_feature.main()
+                self.assertTrue(result.exception.args[0]['changed'])
+
+        self.assertEqual(run_command.call_count, 2)
+        self.assertEqual(run_command.call_args[0][0][-1], 'api')
+
+    def test_enable_feature_with_check_mode(self):
+        """Check that result is changed in check mode."""
+        set_module_args({
+            'name': 'api',
+            '_ansible_check_mode': True,
+        })
+        with patch.object(basic.AnsibleModule, 'run_command') as run_command:
+            run_command.return_value = 0, '', ''  # successful execution, no output
+            with self.assertRaises(AnsibleExitJson) as result:
+                icinga2_feature.main()
+                self.assertTrue(result.exception.args[0]['changed'])
+
+        self.assertEqual(run_command.call_count, 1)
+
+    def test_disable_feature(self):
+        """Check that result is changed."""
+        set_module_args({
+            'name': 'api',
+            'state': 'absent'
+        })
+        with patch.object(basic.AnsibleModule, 'run_command') as run_command:
+            run_command.return_value = 0, '', ''  # successful execution, no output
+            with self.assertRaises(AnsibleExitJson) as result:
+                icinga2_feature.main()
+                self.assertTrue(result.exception.args[0]['changed'])
+
+        self.assertEqual(run_command.call_count, 2)
+        self.assertEqual(run_command.call_args[0][0][-1], 'api')
+
+    def test_disable_feature_with_check_mode(self):
+        """Check that result is changed in check mode."""
+        set_module_args({
+            'name': 'api',
+            'state': 'absent',
+            '_ansible_check_mode': True,
+        })
+        with patch.object(basic.AnsibleModule, 'run_command') as run_command:
+            run_command.return_value = 0, '', ''  # successful execution, no output
+            with self.assertRaises(AnsibleExitJson) as result:
+                icinga2_feature.main()
+                self.assertTrue(result.exception.args[0]['changed'])
+
+        self.assertEqual(run_command.call_count, 1)