Skip to content

Fix bind if ceph_ip_address is set #54

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

breml
Copy link
Contributor

@breml breml commented May 27, 2025

Fixes #44

@gregoryguillou you might want to have a look at a077db6. Since you have refactored the ansible playbook to roles and added the support to use it as ansible collection, the repository was no longer self-sufficient in the sense, that one could easily alter the code and just run it. After changing an existing role, it was always required to re-install the collection in order to make ansible pick up the changes.

@gregoryguillou
Copy link
Contributor

gregoryguillou commented May 27, 2025

1- Thank @breml for pointing me that parameter! You are right, I am reinstalling the collection after each change with ansible-galaxy. I'll check if that fixes my issue and detect the changes

2- I was thinking from #44 that the IP you were using for Ceph was IPv4 and I would have expected to use ansible.utils.ipv4 at some point to change the line you are changing. I know there are other issues and my understanding is that tests somehow assume we are running IPv6. For instance, you can check the [\\1] below which I assume should not work with IPv4:

@breml breml force-pushed the fix-bind-with-ceph-ip-address branch from 39df0e5 to 2eb50e1 Compare May 28, 2025 10:09
@stgraber
Copy link
Member

@breml @gregoryguillou what's going on with this one?

@gregoryguillou
Copy link
Contributor

@breml @gregoryguillou what's going on with this one?

This PR introduces a default value (empty string) for ceph_ip_address as pointed in the comments above. This leads to an error when ceph_ip_address is not set. When it is the case, this template truns into - ip: instead of ip: <default_ip>. The simplest fix to me is to leave ceph_ip_address undefined and remove the , true) from the default syntax.

In addition, the roles_path is not relevant anymore because roles are now prefixed with the collection as recommended by the linter.

@breml did 90% of the work so I was thinking he would be interested to fix the issue. If needed, I can fix the PR and pass the tests later this week...

@breml
Copy link
Contributor Author

breml commented Jun 19, 2025

@gregoryguillou Feel free to fix the PR. I have not had the time to look at this for the past weeks, I was chasing other priorities, sorry.

breml added 2 commits July 11, 2025 21:37
Make the ansible playbook work without the need
to install it as collection with Ansible Galaxy.

Signed-off-by: Lucas Bremgartner <[email protected]>
Fixes: lxc#44

Signed-off-by: Lucas Bremgartner <[email protected]>
@breml breml force-pushed the fix-bind-with-ceph-ip-address branch from 2eb50e1 to 8ff6b11 Compare July 11, 2025 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

ceph fails to bind with ceph_ip_address set explicitly to IPv4 address
3 participants