Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix state=list in ceph_pool module #7

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

cosandr
Copy link

@cosandr cosandr commented Dec 10, 2024

This PR fixes list not being allowed by the ceph_pool module, despite being implemented.

I've also fixed a crash caused by an extra parameter being passed.

Traceback (most recent call last):
File "", line 107, in
File "", line 99, in _ansiballz_main
File "", line 47, in invoke_module
File "", line 226, in run_module
File "", line 98, in _run_module_code
File "", line 88, in _run_code
File "/tmp/ansible_ceph.automation.ceph_pool_payload_9wlbrefz/ansible_ceph.automation.ceph_pool_payload.zip/ansible_collections/ceph/automation/plugins/modules/ceph_pool.py", line 718, in
File "/tmp/ansible_ceph.automation.ceph_pool_payload_9wlbrefz/ansible_ceph.automation.ceph_pool_payload.zip/ansible_collections/ceph/automation/plugins/modules/ceph_pool.py", line 714, in main
File "/tmp/ansible_ceph.automation.ceph_pool_payload_9wlbrefz/ansible_ceph.automation.ceph_pool_payload.zip/ansible_collections/ceph/automation/plugins/modules/ceph_pool.py", line 693, in run_module
File "/tmp/ansible_ceph.automation.ceph_pool_payload_9wlbrefz/ansible_ceph.automation.ceph_pool_payload.zip/ansible_collections/ceph/automation/plugins/module_utils/ceph_common.py", line 102, in exec_command
File "/tmp/ansible_ceph.automation.ceph_pool_payload_9wlbrefz/ansible_ceph.automation.ceph_pool_payload.zip/ansible/module_utils/basic.py", line 1860, in run_command
File "/tmp/ansible_ceph.automation.ceph_pool_payload_9wlbrefz/ansible_ceph.automation.ceph_pool_payload.zip/ansible/module_utils/basic.py", line 1860, in
File "", line 296, in expandvars
TypeError: expected str, bytes or os.PathLike object, not bool

The name parameter isn't required for list (neither is it being used), so only make it required if state is not list.

@asm0deuz
Copy link
Collaborator

asm0deuz commented Jan 2, 2025

@cosandr Hi!

Thx for this PR.

When migrating modules from ceph-ansible project(1), sanity check failed as best practices require to move state: info, list (2) into separate modules called _info.

Looks like I removed list from the ceph_pool module but forgot to create a ceph_pool_info.py module. Either you can do it or just open an issue and I'll do it later.

(1) #3
(2) https://docs.ansible.com/ansible/latest/dev_guide/developing_modules_general.html#creating-an-info-or-a-facts-module

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants