Skip to content

Conversation

jovial
Copy link
Contributor

@jovial jovial commented Jan 9, 2023

Support for this feature was added in #290. This adds a variable and some documentation to raise awareness of its existence.

Support for this feature was added in #290. This adds a variable
and some documentation to raise awareness of its existence.
@jovial jovial requested a review from a team as a code owner January 9, 2023 15:02
# A list of commands to pass to cephadm shell -- ceph. See stackhpc.cephadm.commands
# for format.
cephadm_commands:
- "fs new cephfs cephfs_metadata cephfs_data"
Copy link
Member

Choose a reason for hiding this comment

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

I would argue that's a good example, changing placement should rather be done using cephadm role, not the commands role.

Copy link
Contributor Author

@jovial jovial Jan 11, 2023

Choose a reason for hiding this comment

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

That is the example from the docs in the collection. I was going to use this approach to do:

ceph config set mgr mgr/prometheus/server_addr 10.0.0.1

So it wasn't bound to all interfaces. Is that a better example?

EDIT: Assuming you meant not a good example

Copy link
Member

Choose a reason for hiding this comment

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

Yes, an mgr var is a good example :-) I meant the old one is NOT a good example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, makes sense to not encourage a bad pattern of usage.

jovial and others added 2 commits January 11, 2023 14:33
Co-authored-by: Mark Goddard <mark@stackhpc.com>
Updating the example based on feedback from the code review. The previous example applied configuration that would have been better applied using the cephadm role.  We don't want to encourage such usage.
@jovial jovial mentioned this pull request Jan 11, 2023
@markgoddard markgoddard merged commit 5ec350b into stackhpc/xena Jan 12, 2023
@markgoddard markgoddard deleted the docs/xena/cephadm-commands branch January 12, 2023 09:27
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.

3 participants