ceph_key: refact the code and minor fixes

wip

Signed-off-by: Guillaume Abrioux <gabrioux@redhat.com>

library/ceph_key.py

@@ -545,10 +545,19 @@ def run_module():
     # Test if the key exists, if it does we skip its creation
     # We only want to run this check when a key needs to be added
     # There is no guarantee that any cluster is running and we don't need one
-    key_exist = 1
     _secret = secret
     _caps = caps
-    if (state in ["present", "update", "info"]):
+
+    user = "client.admin"
+    user_key = os.path.join(
+        "/etc/ceph/" + cluster + ".client.admin.keyring")
+    output_format = "json"
+    _info_key = []
+    rc, cmd, out, err = exec_commands(
+        module, info_key(cluster, name, user, user_key, output_format, container_image))  # noqa E501
+    key_exist = rc
+
+    if (state in ["present", "update"]):
         # if dest is not a directory, the user wants to change the file's name
         # (e,g: /etc/ceph/ceph.mgr.ceph-mon2.keyring)
         if not os.path.isdir(dest):
@@ -564,42 +573,33 @@ def run_module():
 
         file_args['path'] = file_path
 
-        if state != 'info':
-            if import_key:
-                user = "client.admin"
-                user_key = os.path.join(
-                    "/etc/ceph/" + cluster + ".client.admin.keyring")
-                output_format = "json"
-                _info_key = []
-                rc, cmd, out, err = exec_commands(
-                    module, info_key(cluster, name, user, user_key, output_format, container_image))  # noqa E501
-                key_exist = rc
-                if key_exist == 0:
-                    _info_key = json.loads(out)
-                    if not secret:
-                        secret = _info_key[0]['key']
-                    _secret = _info_key[0]['key']
-                    if not caps:
-                        caps = _info_key[0]['caps']
-                    _caps = _info_key[0]['caps']
-                    if secret == _secret and caps == _caps:
-                        if not os.path.isfile(file_path):
-                            rc, cmd, out, err = exec_commands(module, get_key(cluster, name, file_path, container_image))  # noqa E501
-                            result["rc"] = rc
-                            if rc != 0:
-                                result["stdout"] = "Couldn't fetch the key {0} at {1}.".format(name, file_path) # noqa E501
-                                module.exit_json(**result)
-                            result["stdout"] = "fetched the key {0} at {1}.".format(name, file_path) # noqa E501
+        if import_key:
+            if key_exist == 0:
+                _info_key = json.loads(out)
+                if not secret:
+                    secret = _info_key[0]['key']
+                _secret = _info_key[0]['key']
+                if not caps:
+                    caps = _info_key[0]['caps']
+                _caps = _info_key[0]['caps']
+                if secret == _secret and caps == _caps:
+                    if not os.path.isfile(file_path):
+                        rc, cmd, out, err = exec_commands(module, get_key(cluster, name, file_path, container_image))  # noqa E501
+                        result["rc"] = rc
+                        if rc != 0:
+                            result["stdout"] = "Couldn't fetch the key {0} at {1}.".format(name, file_path) # noqa E501
+                            module.exit_json(**result)
+                        result["stdout"] = "fetched the key {0} at {1}.".format(name, file_path) # noqa E501
 
-                        result["stdout"] = "{0} already exists and doesn't need to be updated.".format(name) # noqa E501
-                        result["rc"] = 0
-                        module.set_fs_attributes_if_different(file_args, False)
-                        module.exit_json(**result)
-            else:
-                if os.path.isfile(file_path) and not secret or not caps:
-                    result["stdout"] = "{0} already exists in {1} you must provide secret *and* caps when import_key is {2}".format(name, dest, import_key) # noqa E501
+                    result["stdout"] = "{0} already exists and doesn't need to be updated.".format(name) # noqa E501
                     result["rc"] = 0
+                    module.set_fs_attributes_if_different(file_args, False)
                     module.exit_json(**result)
+        else:
+            if os.path.isfile(file_path) and not secret or not caps:
+                result["stdout"] = "{0} already exists in {1} you must provide secret *and* caps when import_key is {2}".format(name, dest, import_key) # noqa E501
+                result["rc"] = 0
+                module.exit_json(**result)
 
     # "update" is here only for backward compatibility
     if state in ["present", "update"]:
@@ -621,8 +621,13 @@ def run_module():
         module.set_fs_attributes_if_different(file_args, False)
 
     elif state == "absent":
-        rc, cmd, out, err = exec_commands(
-            module, delete_key(cluster, name, container_image))
+        if key_exist == 0:
+            rc, cmd, out, err = exec_commands(
+                module, delete_key(cluster, name, container_image))
+            if rc == 0:
+                changed = True
+        else:
+            rc = 0
 
     elif state == "info":
         user = "client.admin"

tests/library/test_ceph_key.py

@@ -555,4 +555,72 @@ class TestCephKeyModule(object):
         assert result['changed'] == False
         assert result['stdout'] == '[{"entity":"client.admin","key":"AQC1tw5fF156GhAAoJCvHGX/jl/k7/N4VZm8iQ==","caps":{"mds":"allow *","mgr":"allow *","mon":"allow *","osd":"allow *"}}]'
         assert result['stderr'] == 'exported keyring for client.admin'
-        assert result['rc'] == 0
+        assert result['rc'] == 0
+
+    @mock.patch('ansible.module_utils.basic.AnsibleModule.exit_json')
+    @mock.patch('ceph_key.exec_commands')
+    def test_state_absent_existing_keyring(self, m_exec_commands, m_exit_json):
+        set_module_args({"state": "absent",
+                         "cluster": "ceph",
+                         "name": "client.foo"
+        })
+        m_exit_json.side_effect = exit_json
+        m_exec_commands.return_value = (0,
+                                        ['ceph', 'auth', 'rm', 'client.foo'] ,
+                                        '', 'updated'
+                                        )
+
+        with pytest.raises(AnsibleExitJson) as result:
+            ceph_key.run_module()
+
+        result = result.value.args[0]
+        assert result['changed'] == True
+        assert result['stdout'] == ''
+        assert result['stderr'] == 'updated'
+        assert result['rc'] == 0
+
+    @mock.patch('ansible.module_utils.basic.AnsibleModule.exit_json')
+    @mock.patch('ceph_key.exec_commands')
+    def test_state_absent_non_existing_keyring(self, m_exec_commands, m_exit_json):
+        set_module_args({"state": "absent",
+                         "cluster": "ceph",
+                         "name": "client.foo"
+        })
+        m_exit_json.side_effect = exit_json
+        m_exec_commands.return_value = (1,
+                                        ['ceph', 'auth', 'get', 'client.foo'] ,
+                                        '', 'Error ENOENT: failed to find client.foo in keyring'
+                                        )
+
+        with pytest.raises(AnsibleExitJson) as result:
+            ceph_key.run_module()
+
+        result = result.value.args[0]
+        assert result['changed'] == False
+        assert result['stdout'] == ''
+        assert result['stderr'] == 'Error ENOENT: failed to find client.foo in keyring'
+        assert result['rc'] == 0
+
+    @mock.patch('ansible.module_utils.basic.AnsibleModule.exit_json')
+    @mock.patch('ceph_key.exec_commands')
+    def test_state_absent_non_existing_keyring(self, m_exec_commands, m_exit_json):
+        set_module_args({"state": "absent",
+                         "cluster": "ceph",
+                         "name": "client.foo"
+        })
+        m_exit_json.side_effect = exit_json
+        m_exec_commands.return_value = (1,
+                                        ['ceph', 'auth', 'get', 'client.foo'] ,
+                                        '', 'Error ENOENT: failed to find client.foo in keyring'
+                                        )
+
+        with pytest.raises(AnsibleExitJson) as result:
+            ceph_key.run_module()
+
+        result = result.value.args[0]
+        assert result['changed'] == False
+        assert result['stdout'] == ''
+        assert result['stderr'] == 'Error ENOENT: failed to find client.foo in keyring'
+        assert result['rc'] == 0
+
+# entity client.test2 does not exist