From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] [PATCH pve-manager 5/8] fix #4759: ceph: configure keyring for ceph-crash.service
Date: Wed, 31 Jan 2024 14:17:53 +0100 [thread overview]
Message-ID: <1706704585.2yd7g33cz4.astroid@yuna.none> (raw)
In-Reply-To: <20240130184041.1125674-6-m.carrara@proxmox.com>
On January 30, 2024 7:40 pm, Max Carrara wrote:
> when creating the cluster's first monitor.
>
> Signed-off-by: Max Carrara <m.carrara@proxmox.com>
> ---
> PVE/API2/Ceph/MON.pm | 28 +++++++++++++++++++++++++++-
> PVE/Ceph/Services.pm | 12 ++++++++++--
> PVE/Ceph/Tools.pm | 38 ++++++++++++++++++++++++++++++++++++++
> 3 files changed, 75 insertions(+), 3 deletions(-)
>
> diff --git a/PVE/API2/Ceph/MON.pm b/PVE/API2/Ceph/MON.pm
> index 1e959ef3..8d75f5d1 100644
> --- a/PVE/API2/Ceph/MON.pm
> +++ b/PVE/API2/Ceph/MON.pm
> @@ -459,11 +459,37 @@ __PACKAGE__->register_method ({
> });
> die $@ if $@;
> # automatically create manager after the first monitor is created
> + # and set up keyring and config for ceph-crash.service
> if ($is_first_monitor) {
> PVE::API2::Ceph::MGR->createmgr({
> node => $param->{node},
> id => $param->{node}
> - })
> + });
> +
> + PVE::Cluster::cfs_lock_file('ceph.conf', undef, sub {
> + my $cfg = cfs_read_file('ceph.conf');
> +
> + if ($cfg->{'client.crash'}) {
> + return undef;
> + }
> +
> + $cfg->{'client.crash'}->{keyring} = '/etc/pve/ceph/$cluster.$name.keyring';
> +
> + cfs_write_file('ceph.conf', $cfg);
> + });
> + die $@ if $@;
> +
> + eval {
> + PVE::Ceph::Tools::get_or_create_crash_keyring();
> + };
> + warn "Unable to configure keyring for ceph-crash.service: $@" if $@;
the order here should maybe be switched around? first handle the
keyring, then put it in the config?
> +
> + print "enabling service 'ceph-crash.service'\n";
> + PVE::Ceph::Services::ceph_service_cmd('enable', 'crash');
shouldn't this already be handled by default?
> + print "starting service 'ceph-crash.service'\n";
> + # ceph-crash already runs by default,
> + # this makes sure the keyring is used
> + PVE::Ceph::Services::ceph_service_cmd('restart', 'crash');
this should probably be a try-restart to avoid starting it if the admin
explicitly disabled and/or stopped it..
but - AFAICT, the ceph-crash script that is executed by the service
boils down to (as forked process!) "ceph -n XXX ..." where XXX is (in sequence)
client.crash.$HOST, client.crash, client.admin, so a service restart
shouldn't even be needed, since a fresh ceph (client) process will pick
up the config changes anyway?
> }
> };
>
> diff --git a/PVE/Ceph/Services.pm b/PVE/Ceph/Services.pm
> index e0f31e8e..5f5986f9 100644
> --- a/PVE/Ceph/Services.pm
> +++ b/PVE/Ceph/Services.pm
> [..]
> diff --git a/PVE/Ceph/Tools.pm b/PVE/Ceph/Tools.pm
> index 3acef11b..cf9f2ed4 100644
> --- a/PVE/Ceph/Tools.pm
> +++ b/PVE/Ceph/Tools.pm
> [..]
> + my $output = $rados->mon_command({
> + prefix => 'auth get-or-create',
> + entity => 'client.crash',
> + caps => [
> + mon => 'profile crash',
> + mgr => 'profile crash',
> + ],
> + format => 'plain',
> + });
> +
> + if (! -d $pve_ceph_cfgdir) {
> + mkdir $pve_ceph_cfgdir;
> + }
> +
> + PVE::Tools::file_set_contents($pve_ceph_crash_key_path, $output);
> +
> + return $pve_ceph_crash_key_path;
> +}
> +
we have another helper for creating a keyring (and another inline call
to ceph-authtool when creating a monitor), should we unify them?
next prev parent reply other threads:[~2024-01-31 13:18 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-30 18:40 [pve-devel] [PATCH master ceph, quincy-stable-8 ceph, pve-storage, pve-manager 0/8] Fix #4759: Configure Permissions " Max Carrara
2024-01-30 18:40 ` [pve-devel] [PATCH master ceph 1/8] debian: add patch to fix ceph crash dir permissions in postinst hook Max Carrara
2024-01-31 13:18 ` Fabian Grünbichler
2024-02-01 13:28 ` Max Carrara
2024-01-30 18:40 ` [pve-devel] [PATCH quincy-stable-8 ceph 2/8] " Max Carrara
2024-01-30 18:40 ` [pve-devel] [PATCH pve-storage 3/8] cephconfig: support sections in the format of [client.$NAME] Max Carrara
2024-01-31 13:18 ` Fabian Grünbichler
2024-02-01 13:40 ` Max Carrara
2024-01-30 18:40 ` [pve-devel] [PATCH pve-manager 4/8] ceph: fix edge case of wrong files being deleted on purge Max Carrara
2024-01-31 13:18 ` Fabian Grünbichler
2024-02-01 13:59 ` Max Carrara
2024-01-30 18:40 ` [pve-devel] [PATCH pve-manager 5/8] fix #4759: ceph: configure keyring for ceph-crash.service Max Carrara
2024-01-31 13:17 ` Fabian Grünbichler [this message]
2024-02-05 11:57 ` Max Carrara
2024-02-12 13:41 ` Fabian Grünbichler
2024-01-30 18:40 ` [pve-devel] [PATCH pve-manager 6/8] ceph: create '/etc/pve/ceph' during `pveceph init` Max Carrara
2024-01-30 18:40 ` [pve-devel] [PATCH pve-manager 7/8] debian/postinst: fix shellcheck warning Max Carrara
2024-01-31 13:16 ` [pve-devel] applied-partially: " Fabian Grünbichler
2024-02-01 13:40 ` Max Carrara
2024-01-30 18:40 ` [pve-devel] [PATCH pve-manager 8/8] fix #4759: debian/postinst: configure ceph-crash.service and its key Max Carrara
2024-01-31 13:15 ` Fabian Grünbichler
2024-02-01 13:54 ` Max Carrara
2024-01-31 13:25 ` [pve-devel] [PATCH master ceph, quincy-stable-8 ceph, pve-storage, pve-manager 0/8] Fix #4759: Configure Permissions for ceph-crash.service Fabian Grünbichler
2024-01-31 14:22 ` Friedrich Weber
2024-02-01 13:35 ` Fabian Grünbichler
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1706704585.2yd7g33cz4.astroid@yuna.none \
--to=f.gruenbichler@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox