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

mgr/cephadm: reimplement ceph.conf pushing; push client keyrings too #40941

Merged
merged 9 commits into from Apr 28, 2021

Conversation

liewegas
Copy link
Member

@liewegas liewegas commented Apr 20, 2021

  • Add a config option to control which hosts (by default, *) get a ceph.conf (if the bool manage_etc_ceph_ceph_conf option is enabled).
  • Add ceph orch client-keyring {ls,set,rm} commands to manage which client keyrings (and associated configs) to push where
  • Refactor a bit to use a cleaner, safer, more efficient method for writing remote files.

@liewegas liewegas requested a review from a team as a code owner April 20, 2021 16:58
@liewegas liewegas force-pushed the cephadm-conf-placement branch 2 times, most recently from 7da5ab8 to 144da3b Compare April 21, 2021 17:33
@liewegas liewegas changed the title mgr/cephadm: add placementspec for which hosts get ceph.conf mgr/cephadm: reimplement ceph.conf pushing; push client keyrings too Apr 21, 2021
Add a config option to control which hosts (by default, *) get a
ceph.conf (if the bool manage_etc_ceph_ceph_conf option is enabled).

We don't modify the existing option because changing a type makes for a
messy migration: we have to sort out which section the config option is
in to change it.  Also, a simple on/off which is more friendly than
specifying "*" to enable something.

Signed-off-by: Sage Weil <sage@newdream.net>
This is careful is ownership, mode, and fsyncs before renaming into
position.

Signed-off-by: Sage Weil <sage@newdream.net>
Signed-off-by: Sage Weil <sage@newdream.net>
@liewegas liewegas force-pushed the cephadm-conf-placement branch 2 times, most recently from 4ed969e to 34cd3d8 Compare April 22, 2021 12:13
Copy link
Contributor

@adk3798 adk3798 left a comment

Choose a reason for hiding this comment

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

Code mostly looks good. Just some thoughts.

doc/cephadm/operations.rst Outdated Show resolved Hide resolved
doc/cephadm/operations.rst Outdated Show resolved Hide resolved
src/pybind/mgr/cephadm/inventory.py Show resolved Hide resolved
src/pybind/mgr/cephadm/inventory.py Show resolved Hide resolved
raise OrchestratorError('mode must be an octal mode, e.g. "600"')
else:
imode = 0o600
pspec = PlacementSpec.from_string(placement)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should placement specs that just use "count" not be allowed here since they don't make much sense in this context?

src/cephadm/cephadm Outdated Show resolved Hide resolved
@@ -1191,6 +1201,67 @@ def run(h: str) -> str:

return HandleCommandResult(stdout='\n'.join(run(host)))

@orchestrator._cli_read_command('orch client-keyring ls')
Copy link
Contributor

Choose a reason for hiding this comment

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

are we okay with starting to put "ceph orch" commands directly in the cephadm module rather than the orchestrator? Other commands we've done here have been "ceph cephadm"

@github-actions github-actions bot added the core label Apr 22, 2021
@liewegas liewegas force-pushed the cephadm-conf-placement branch 5 times, most recently from d9964f3 to 8eea8c7 Compare April 26, 2021 13:00
@github-actions github-actions bot added the tests label Apr 26, 2021
@liewegas liewegas force-pushed the cephadm-conf-placement branch 3 times, most recently from 4621b2a to dfbaebc Compare April 26, 2021 14:55
Comment on lines -330 to -332
if 'last_etc_ceph_ceph_conf' in j:
self.last_etc_ceph_ceph_conf[host] = str_to_datetime(
j['last_etc_ceph_ceph_conf'])
Copy link
Contributor

Choose a reason for hiding this comment

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

downgrades. Yes it will work, but it will redeploy the ceph.conf on all hosts.

src/pybind/mgr/cephadm/serve.py Outdated Show resolved Hide resolved
Use a more generic inventory map of paths to digests to track what we've
pushed.

Signed-off-by: Sage Weil <sage@newdream.net>
…g files

Teach cephadm to manage keyring files on cluster hosts.  These keys must
already exist in the mon auth database--cephadm does not create them if
they don't exist (and will issue warnings to the log if they do not).

A ceph.conf is pushed implicitly along with the keyring file.

Each keyring added will be pushed to the hosts described by the placement
spec with the appropriate ownership and mode.  If the ownership, mode, or
path are modified, the files are rewritten or removed as need.

If the client-keyring entry is removed, the keyring files are removed.

Signed-off-by: Sage Weil <sage@newdream.net>
Signed-off-by: Sage Weil <sage@newdream.net>
@liewegas liewegas force-pushed the cephadm-conf-placement branch 3 times, most recently from 8326505 to b67186c Compare April 27, 2021 14:08
@sebastian-philipp
Copy link
Contributor

…strap

If we are placing ceph.conf in /etc/ceph (the default), tell the cluster
to continue doing this going forward to hosts with the '_admin' label.

This doesn't induce the user to add the admin label to other hosts too,
unfortunately--e probably want them to add the admin label to other mons,
for instance--but it is a start.

Signed-off-by: Sage Weil <sage@newdream.net>
Except during upgrades, since it is not supported there.

Signed-off-by: Sage Weil <sage@newdream.net>
Signed-off-by: Sage Weil <sage@newdream.net>
@sebastian-philipp
Copy link
Contributor

Do you want to add this to https://docs.ceph.com/en/latest/cephadm/client-setup/ ?

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