[pve-devel] [PATCH v2 pve-manager 09/11] fix #4759: ceph: configure keyring for ceph-crash.service
Fabian Grünbichler
f.gruenbichler at proxmox.com
Mon Feb 12 14:34:05 CET 2024
On February 5, 2024 6:54 pm, Max Carrara wrote:
> when creating the cluster's first monitor.
>
> Signed-off-by: Max Carrara <m.carrara at proxmox.com>
> ---
> Changes v1 --> v2:
> * do not enable/restart `ceph-crash` anymore when creating first mon
> * drop changes to function `ceph_service_cmd` as they are no longer
> needed
> * create keyring for `ceph-crash` before modifying 'ceph.conf'
> * always set keyring for 'client.crash' section instead of only
> if section doesn't exist already
> * only modify the keyring file in `get_or_create_crash_keyring()`
> if the content differs from the output of `ceph auth get-or-create`
you kinda ignored my comment about this adding yet another create keyring helper ;)
> PVE/API2/Ceph/MON.pm | 17 ++++++++++++++++-
> PVE/Ceph/Tools.pm | 39 +++++++++++++++++++++++++++++++++++++++
> 2 files changed, 55 insertions(+), 1 deletion(-)
>
> diff --git a/PVE/API2/Ceph/MON.pm b/PVE/API2/Ceph/MON.pm
> index 1e959ef3..ae12a2d3 100644
> --- a/PVE/API2/Ceph/MON.pm
> +++ b/PVE/API2/Ceph/MON.pm
> @@ -459,11 +459,26 @@ __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}
> - })
> + });
> +
> + eval {
> + PVE::Ceph::Tools::get_or_create_crash_keyring();
this is called get_or_create, but it actually returns the path to, not
the key(ring).. nothing uses the return value anyway, so it could also
be called differently I guess and not return anything, but just from the
name, I'd expect the key to be returned.
> + };
> + warn "Unable to configure keyring for ceph-crash.service: $@" if $@;
> +
> + PVE::Cluster::cfs_lock_file('ceph.conf', undef, sub {
> + my $cfg = cfs_read_file('ceph.conf');
> +
> + $cfg->{'client.crash'}->{keyring} = '/etc/pve/ceph/$cluster.$name.keyring';
I guess this doesn't make anything worse (since we starting from
"ceph-crash is totally broken"), but if querying or creating the keyring
failed, does it make sense to reference it in the config?
> +
> + cfs_write_file('ceph.conf', $cfg);
> + });
> + die $@ if $@;
we could move this whole part up to where we do the monitor changes, and
only lock and read and write the config once..
> }
> };
>
> diff --git a/PVE/Ceph/Tools.pm b/PVE/Ceph/Tools.pm
> index 273a3eb6..02a932e3 100644
> --- a/PVE/Ceph/Tools.pm
> +++ b/PVE/Ceph/Tools.pm
> @@ -18,7 +18,9 @@ my $ccname = 'ceph'; # ceph cluster name
> my $ceph_cfgdir = "/etc/ceph";
> my $pve_ceph_cfgpath = "/etc/pve/$ccname.conf";
> my $ceph_cfgpath = "$ceph_cfgdir/$ccname.conf";
> +my $pve_ceph_cfgdir = "/etc/pve/ceph";
>
> +my $pve_ceph_crash_key_path = "$pve_ceph_cfgdir/$ccname.client.crash.keyring";
> my $pve_mon_key_path = "/etc/pve/priv/$ccname.mon.keyring";
> my $pve_ckeyring_path = "/etc/pve/priv/$ccname.client.admin.keyring";
> my $ckeyring_path = "/etc/ceph/ceph.client.admin.keyring";
> @@ -37,12 +39,14 @@ my $ceph_service = {
>
> my $config_values = {
> ccname => $ccname,
> + pve_ceph_cfgdir => $pve_ceph_cfgdir,
> ceph_mds_data_dir => $ceph_mds_data_dir,
> long_rados_timeout => 60,
> };
>
> my $config_files = {
> pve_ceph_cfgpath => $pve_ceph_cfgpath,
> + pve_ceph_crash_key_path => $pve_ceph_crash_key_path,
> pve_mon_key_path => $pve_mon_key_path,
> pve_ckeyring_path => $pve_ckeyring_path,
> ceph_bootstrap_osd_keyring => $ceph_bootstrap_osd_keyring,
> @@ -415,6 +419,41 @@ sub get_or_create_admin_keyring {
> return $pve_ckeyring_path;
> }
>
> +# requires connection to existing monitor
> +sub get_or_create_crash_keyring {
> + my ($rados) = @_;
> +
> + if (!defined($rados)) {
> + $rados = PVE::RADOS->new();
> + }
> +
> + 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) {
> + File::Path::make_path($pve_ceph_cfgdir);
> + }
> +
> + if (-f $pve_ceph_crash_key_path) {
> + my $contents = PVE::Tools::file_get_contents($pve_ceph_crash_key_path);
> +
> + if ($contents ne $output) {
> + PVE::Tools::file_set_contents($pve_ceph_crash_key_path, $output);
> + }
> + } else {
> + PVE::Tools::file_set_contents($pve_ceph_crash_key_path, $output);
> + }
> +
> + return $pve_ceph_crash_key_path;
> +}
> +
> # get ceph-volume managed osds
> sub ceph_volume_list {
> my $result = {};
> --
> 2.39.2
>
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel at lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>
>
>
More information about the pve-devel
mailing list