Skip to content

Commit

Permalink
library: make cephadm_adopt module idempotent
Browse files Browse the repository at this point in the history
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 <dsavinea@redhat.com>
  • Loading branch information
dsavineau authored and guits committed Jan 29, 2021
1 parent 6886700 commit ff9d314
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 34 deletions.
61 changes: 40 additions & 21 deletions library/cephadm_adopt.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
except ImportError:
from module_utils.ca_common import exit_module
import datetime
import json


ANSIBLE_METADATA = {
Expand Down Expand Up @@ -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:
Expand All @@ -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__':
Expand Down
61 changes: 48 additions & 13 deletions tests/library/test_cephadm_adopt.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,31 +34,29 @@ def test_with_check_mode(self, m_exit_json):

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')
Expand All @@ -76,7 +74,10 @@ def test_with_default_values(self, m_run_command, m_exit_json):
'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()
Expand All @@ -88,6 +89,28 @@ def test_with_default_values(self, m_run_command, m_exit_json):
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):
Expand All @@ -99,7 +122,10 @@ def test_with_docker(self, m_run_command, m_exit_json):
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()
Expand All @@ -120,7 +146,10 @@ def test_with_custom_image(self, m_run_command, m_exit_json):
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()
Expand All @@ -141,7 +170,10 @@ def test_without_pull(self, m_run_command, m_exit_json):
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()
Expand All @@ -162,7 +194,10 @@ def test_without_firewalld(self, m_run_command, m_exit_json):
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()
Expand Down

0 comments on commit ff9d314

Please sign in to comment.