From ff9d314305400aeab764310c44bfab95beb64f04 Mon Sep 17 00:00:00 2001 From: Dimitri Savineau Date: Thu, 21 Jan 2021 11:19:44 -0500 Subject: [PATCH] library: make cephadm_adopt module idempotent Rerunning the cephadm_adopt module on an already adopted daemon will fail because the cephadm adopt command isn't idempotent. Closes: https://bugzilla.redhat.com/show_bug.cgi?id=1918424 Signed-off-by: Dimitri Savineau --- library/cephadm_adopt.py | 61 +++++++++++++++++++---------- tests/library/test_cephadm_adopt.py | 61 +++++++++++++++++++++++------ 2 files changed, 88 insertions(+), 34 deletions(-) diff --git a/library/cephadm_adopt.py b/library/cephadm_adopt.py index 776213d5d..88277162a 100644 --- a/library/cephadm_adopt.py +++ b/library/cephadm_adopt.py @@ -21,6 +21,7 @@ try: except ImportError: from module_utils.ca_common import exit_module import datetime +import json ANSIBLE_METADATA = { @@ -122,6 +123,35 @@ def main(): startd = datetime.datetime.now() + cmd = ['cephadm', 'ls', '--no-detail'] + + if module.check_mode: + exit_module( + module=module, + out='', + rc=0, + cmd=cmd, + err='', + startd=startd, + changed=False + ) + else: + rc, out, err = module.run_command(cmd) + + if rc == 0: + if name in [x["name"] for x in json.loads(out) if x["style"] == "cephadm:v1"]: + exit_module( + module=module, + out='{} is already adopted'.format(name), + rc=0, + cmd=cmd, + err='', + startd=startd, + changed=False + ) + else: + module.fail_json(msg=err, rc=rc) + cmd = ['cephadm'] if docker: @@ -138,27 +168,16 @@ def main(): if not firewalld: cmd.append('--skip-firewalld') - if module.check_mode: - exit_module( - module=module, - out='', - rc=0, - cmd=cmd, - err='', - startd=startd, - changed=False - ) - else: - rc, out, err = module.run_command(cmd) - exit_module( - module=module, - out=out, - rc=rc, - cmd=cmd, - err=err, - startd=startd, - changed=True - ) + rc, out, err = module.run_command(cmd) + exit_module( + module=module, + out=out, + rc=rc, + cmd=cmd, + err=err, + startd=startd, + changed=True + ) if __name__ == '__main__': diff --git a/tests/library/test_cephadm_adopt.py b/tests/library/test_cephadm_adopt.py index 7dac2a30c..4f091ee3c 100644 --- a/tests/library/test_cephadm_adopt.py +++ b/tests/library/test_cephadm_adopt.py @@ -34,31 +34,29 @@ class TestCephadmAdoptModule(object): result = result.value.args[0] assert not result['changed'] - assert result['cmd'] == ['cephadm', 'adopt', '--cluster', fake_cluster, '--name', fake_name, '--style', 'legacy'] + assert result['cmd'] == ['cephadm', 'ls', '--no-detail'] assert result['rc'] == 0 assert not result['stdout'] assert not result['stderr'] - @patch('ansible.module_utils.basic.AnsibleModule.exit_json') + @patch('ansible.module_utils.basic.AnsibleModule.fail_json') @patch('ansible.module_utils.basic.AnsibleModule.run_command') - def test_with_failure(self, m_run_command, m_exit_json): + def test_with_failure(self, m_run_command, m_fail_json): ca_test_common.set_module_args({ 'name': fake_name }) - m_exit_json.side_effect = ca_test_common.exit_json + m_fail_json.side_effect = ca_test_common.fail_json stdout = '' stderr = 'ERROR: cephadm should be run as root' rc = 1 m_run_command.return_value = rc, stdout, stderr - with pytest.raises(ca_test_common.AnsibleExitJson) as result: + with pytest.raises(ca_test_common.AnsibleFailJson) as result: cephadm_adopt.main() result = result.value.args[0] - assert result['changed'] - assert result['cmd'] == ['cephadm', 'adopt', '--cluster', fake_cluster, '--name', fake_name, '--style', 'legacy'] assert result['rc'] == 1 - assert result['stderr'] == 'ERROR: cephadm should be run as root' + assert result['msg'] == 'ERROR: cephadm should be run as root' @patch('ansible.module_utils.basic.AnsibleModule.exit_json') @patch('ansible.module_utils.basic.AnsibleModule.run_command') @@ -76,7 +74,10 @@ class TestCephadmAdoptModule(object): 'firewalld ready'.format(fake_name, fake_name) stderr = '' rc = 0 - m_run_command.return_value = rc, stdout, stderr + m_run_command.side_effect = [ + (0, '[{{"style":"legacy","name":"{}"}}]'.format(fake_name), ''), + (rc, stdout, stderr) + ] with pytest.raises(ca_test_common.AnsibleExitJson) as result: cephadm_adopt.main() @@ -88,6 +89,28 @@ class TestCephadmAdoptModule(object): assert result['stderr'] == stderr assert result['stdout'] == stdout + @patch('ansible.module_utils.basic.AnsibleModule.exit_json') + @patch('ansible.module_utils.basic.AnsibleModule.run_command') + def test_already_adopted(self, m_run_command, m_exit_json): + ca_test_common.set_module_args({ + 'name': fake_name + }) + m_exit_json.side_effect = ca_test_common.exit_json + stderr = '' + stdout = '[{{"style":"cephadm:v1","name":"{}"}}]'.format(fake_name) + rc = 0 + m_run_command.return_value = rc, stdout, stderr + + with pytest.raises(ca_test_common.AnsibleExitJson) as result: + cephadm_adopt.main() + + result = result.value.args[0] + assert not result['changed'] + assert result['cmd'] == ['cephadm', 'ls', '--no-detail'] + assert result['rc'] == 0 + assert result['stderr'] == stderr + assert result['stdout'] == '{} is already adopted'.format(fake_name) + @patch('ansible.module_utils.basic.AnsibleModule.exit_json') @patch('ansible.module_utils.basic.AnsibleModule.run_command') def test_with_docker(self, m_run_command, m_exit_json): @@ -99,7 +122,10 @@ class TestCephadmAdoptModule(object): stdout = '' stderr = '' rc = 0 - m_run_command.return_value = rc, stdout, stderr + m_run_command.side_effect = [ + (0, '[{{"style":"legacy","name":"{}"}}]'.format(fake_name), ''), + (rc, stdout, stderr) + ] with pytest.raises(ca_test_common.AnsibleExitJson) as result: cephadm_adopt.main() @@ -120,7 +146,10 @@ class TestCephadmAdoptModule(object): stdout = '' stderr = '' rc = 0 - m_run_command.return_value = rc, stdout, stderr + m_run_command.side_effect = [ + (0, '[{{"style":"legacy","name":"{}"}}]'.format(fake_name), ''), + (rc, stdout, stderr) + ] with pytest.raises(ca_test_common.AnsibleExitJson) as result: cephadm_adopt.main() @@ -141,7 +170,10 @@ class TestCephadmAdoptModule(object): stdout = '' stderr = '' rc = 0 - m_run_command.return_value = rc, stdout, stderr + m_run_command.side_effect = [ + (0, '[{{"style":"legacy","name":"{}"}}]'.format(fake_name), ''), + (rc, stdout, stderr) + ] with pytest.raises(ca_test_common.AnsibleExitJson) as result: cephadm_adopt.main() @@ -162,7 +194,10 @@ class TestCephadmAdoptModule(object): stdout = '' stderr = '' rc = 0 - m_run_command.return_value = rc, stdout, stderr + m_run_command.side_effect = [ + (0, '[{{"style":"legacy","name":"{}"}}]'.format(fake_name), ''), + (rc, stdout, stderr) + ] with pytest.raises(ca_test_common.AnsibleExitJson) as result: cephadm_adopt.main() -- 2.39.5