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

ceph-salt-formula: give explicit IP addr when adding host #465

Merged
merged 1 commit into from
Nov 10, 2021

Conversation

smithfarm
Copy link
Contributor

@smithfarm smithfarm commented Nov 8, 2021

The upstream cephadm developers recently added a commit - ceph/ceph@0facfac - which was backported to pacific and was found to be causing our post-bootstrap "ceph orch host add" command to fail because we weren't specifying the IP address explicitly.

While the developers are now discussing whether the command really should fail in this way, it seems prudent to give our IP address here explicitly, since mgr/cephadm is going to try to resolve the hostname, anyway.

Fixes: #463

Signed-off-by: Nathan Cutler [email protected]

@smithfarm smithfarm added bug Something isn't working Add To Changelog Marks the PR to be included in the changelog of the next version release labels Nov 8, 2021
@smithfarm
Copy link
Contributor Author

@trociny

@smithfarm smithfarm requested a review from p-se November 8, 2021 20:41
The upstream cephadm developers recently added a commit
- 0facfac91fd8f71e5a8b869d818e7c2b07b93516 - which was backported to pacific
and was found to be causing our "ceph orch host add" command to fail because we
weren't specifying the IP address explicitly.

While the developers are now discussing whether the command really should fail
in this way, it seems prudent to give our IP address here explicitly, since
mgr/cephadm is going to try to resolve the hostname, anyway.

Fixes: ceph#463

Signed-off-by: Nathan Cutler <[email protected]>
Copy link
Contributor

@tserong tserong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's an unavoidable kink with socket.gethostbyname(socket.gethostname()): if you happen to have specified the system hostname with the loopback address in /etc/hosts, this construct will return 127.0.0.1. I'm almost certain this is unavoidable, and you shouldn't be configuring /etc/hosts like that anyway, but, it might be worth checking if the address returned starts with "127.", and aborting or erroring out in that case. Otherwise looks fine.

@tserong
Copy link
Contributor

tserong commented Nov 9, 2021

...I realise bailing out in this context may be problematic, so don't let my comment stop this PR. Might be worth opening an issue to somehow add a hostname resolution check elsewhere in ceph-salt, as a general sanity check?

@sebastian-philipp
Copy link
Contributor

alternative would be to have the check in ceph/ceph#43856

@smithfarm
Copy link
Contributor Author

smithfarm commented Nov 9, 2021

alternative would be to have the check in ceph/ceph#43856

Well, it's not an "either-or" proposition. It's possible (and probably even desirable) for both ceph-salt and mgr/cephadm to implement this same check.

But ceph-salt has an advantage: it isn't running in a container. So it doesn't get confused about its own IP address like mgr/cephadm does.

@smithfarm
Copy link
Contributor Author

Anyway, let's move the discussion of the IP address sanity check to #466

I'm going to merge this soon if there are no further comments.

@smithfarm smithfarm merged commit 6bc161f into ceph:master Nov 10, 2021
@smithfarm smithfarm deleted the wip-463 branch November 10, 2021 08:42
@tserong tserong removed the Add To Changelog Marks the PR to be included in the changelog of the next version release label Dec 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ceph-salt can no longer bootstrap pacific as of ceph commit 5c9227bbdf987365ae90433ac4e501e1a88b7aca
5 participants