ceph_key: refact the code and minor fixes

This commit refactors the code to remove a duplicate condition and it
makes the `state: absent` code idempotent

Signed-off-by: Guillaume Abrioux <gabrioux@redhat.com>
(cherry picked from commit 13e2311cbe78b7b51930a3a1210629bf036a20c5)

diff --git a/library/ceph_key.py b/library/ceph_key.py
index bc92e14949fec9bd0bdbd76f19aba3c05c6e8626..59318fa8d446d209377406b56ffad25ed44e5c13 100644 (file)
--- a/library/ceph_key.py
+++ b/library/ceph_key.py
@@ -538,7 +538,6 @@ def run_module():
 
     if module.check_mode:
         return result
-
     startd = datetime.datetime.now()
 
     # will return either the image name or None
@@ -547,10 +546,16 @@ 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"]):
+    key_exist = 1
+
+    user = "client.admin"
+    keyring_filename = cluster + '.' + user + '.keyring'
+    user_key = os.path.join("/etc/ceph/", keyring_filename)
+    output_format = "json"
+
+    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):
@@ -566,51 +571,41 @@ 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
-
-                        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
+        if import_key:
+            _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 not caps and key_exist != 0:
+                fatal("Capabilities must be provided when state is 'present'", module)  # noqa E501
+            if key_exist != 0 and secret is None and caps is None:
+                fatal("Keyring doesn't exist, you must provide 'secret' and 'caps'", module)  # noqa E501
+            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)
-
-    # "update" is here only for backward compatibility
-    if state in ["present", "update"]:
-        if not caps and import_key and rc != 0:
-            fatal("Capabilities must be provided when state is 'present'", module)  # noqa E501
-        if import_key and key_exist != 0 and secret is None and caps is None:
-            fatal("Keyring doesn't exist, you must provide 'secret' and 'caps'", module)  # noqa E501
-
-        # There's no need to run create_key() if neither secret nor caps have changed
+        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)
         if (key_exist == 0 and (secret != _secret or caps != _caps)) or key_exist != 0:
             rc, cmd, out, err = exec_commands(module, create_key(
                 module, result, cluster, name, secret, caps, import_key, file_path, container_image))  # noqa E501
@@ -618,19 +613,19 @@ def run_module():
                 result["stdout"] = "Couldn't create or update {0}".format(name)
                 result["stderr"] = err
                 module.exit_json(**result)
+            module.set_fs_attributes_if_different(file_args, False)
             changed = True
 
-        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"
-        keyring_filename = cluster + '.' + user + '.keyring'
-        user_key = os.path.join("/etc/ceph/", keyring_filename)
-        output_format = "json"
         rc, cmd, out, err = exec_commands(
             module, info_key(cluster, name, user, user_key, output_format, container_image))  # noqa E501
         if rc != 0:
@@ -639,9 +634,6 @@ def run_module():
             module.exit_json(**result)
 
     elif state == "list":
-        user = "client.admin"
-        keyring_filename = cluster + '.' + user + '.keyring'
-        user_key = os.path.join("/etc/ceph/", keyring_filename)
         rc, cmd, out, err = exec_commands(
             module, list_keys(cluster, user, user_key, container_image))