Re: [pve-devel] [PATCH pve-manager] fix #7011: ceph monitor: set ownership of monitor logs

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

Am 12.12.25 um 14:05 schrieb Dominik Rusovac:
> Ownership of ceph logs is now set to ceph:ceph after the creation of a
> new monitor and before the new monitor starts. Hence, effective ceph
> monitor logging on freshly set up ceph clusters no longer depends on the
> first upgrade of ceph-common.

Might it be a better fix to then change the postinst script of
ceph-common, or whatever packages postinst script creates those
directories, to chown them to ceph:ceph? That way it would also work
if one installs ceph directly, circumventing pveceph. While that is
not exactly something we promote, but it's not really hard, and
packaging is often a good place to take care of such things like
directory ownership

> 
> For setups (still) affected by #7011 it is required that ownership of
> ceph logs is set to ceph:ceph (either manually or due to some
> ceph-common upgrade), followed by a monitor restart.


> 
> Signed-off-by: Dominik Rusovac <d.rusovac@proxmox.com>
> ---
>  PVE/API2/Ceph/MON.pm | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/PVE/API2/Ceph/MON.pm b/PVE/API2/Ceph/MON.pm
> index 70fc158d..047337ea 100644
> --- a/PVE/API2/Ceph/MON.pm
> +++ b/PVE/API2/Ceph/MON.pm
> @@ -428,6 +428,12 @@ __PACKAGE__->register_method({
>                              $mon_keyring,
>                          ]);
>                          run_command(['chown', 'ceph:ceph', '-R', $mondir]);
> +
> +                        eval {
> +                            run_command('chown ceph:ceph /var/log/ceph');


For new usage of run_command it's nicer to be explicit and pass the command
directly as array, above can be transformed as is, but...

> +                            run_command('chown ceph:ceph /var/log/ceph/*.log*');

the one above here would need to be either wrapped in a shell for the GLOB
to still work, which is effectively happening if you pass commands as simple
string to run_command anyway, or alternatively:

- dir_glob_regex + the chown function [0] (syscall), which is much cheaper 
  than executing a new process. While speed/efficiency might no matter much
  here, it's IMO still good to try to leverage some relatively cheap
  approaches to avoid basic overhead (it can add up).

- Execute chown -R for the whole log directory - but is that actually
  sensible? If logs where written by a process that did neither run as
  ceph:ceph and thus created the logs as that user implicitly, nor run as
  root and created the log files with ceph:ceph explicitly, what point is
  there in "fixing them up once on a fresh create"?

As written at the top of my reply, I'd slightly prefer to fix this in ceph
packaging; that said, we do can create a workaround here, as a pve-manager
update is much quicker to roll out and backport to PVE 8, but ideally we
could remove doing that sometimes in a next release again, once all supported
ceph versions correctly create directories from their packaging itself.
For the workaround I currently wouldn't have any strong preference which
option to use, but, as mentioned, I'm not a fan of implicitly relying on
run_command wrapping strings in a "sh -c", and that being the only reason that
the glob above works.

[0]: https://perldoc.perl.org/functions/chown


> +                        };
> +                        warn "$@" if $@;
>                      };
>                      my $err = $@;
>                      unlink $monmap;



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel