From 13e2311cbe78b7b51930a3a1210629bf036a20c5 Mon Sep 17 00:00:00 2001 From: Guillaume Abrioux Date: Tue, 4 Aug 2020 13:53:24 +0200 Subject: [PATCH] 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 --- library/ceph_key.py | 103 +++++++++++++++++++++----------------------- 1 file changed, 48 insertions(+), 55 deletions(-) diff --git a/library/ceph_key.py b/library/ceph_key.py index bdd74902a..851bee703 100644 --- a/library/ceph_key.py +++ b/library/ceph_key.py @@ -536,7 +536,6 @@ def run_module(): if module.check_mode: return result - startd = datetime.datetime.now() # will return either the image name or None @@ -545,10 +544,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): @@ -564,51 +569,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 + 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) - 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) - - # "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 @@ -616,19 +611,20 @@ 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: @@ -637,9 +633,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))