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

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

[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.

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');
+                            run_command('chown ceph:ceph /var/log/ceph/*.log*');
+                        };
+                        warn "$@" if $@;
                     };
                     my $err = $@;
                     unlink $monmap;
-- 
2.47.3



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

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

[Maximiliano Sandoval]

Dominik Rusovac <d.rusovac@proxmox.com> writes:

> 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.
>
> 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');
> +                            run_command('chown ceph:ceph /var/log/ceph/*.log*');
> +                        };
> +                        warn "$@" if $@;
>                      };
>                      my $err = $@;
>                      unlink $monmap;

When we call ceph-mon --mkfs a few lines above the file
/var/log/ceph/ceph-mon.$monid.log gets created with owner root:ceph. I
tried using --setuser=ceph instead, but that did not work out while this
approach does work.

It should be noted however that only the file mentioned above has the
wrong owner and that the chown calls above are a bit too greedy, however
this matches ceph-common post-install script better.

Tested-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
Reviewed-by: Maximiliano Sandoval <m.sandoval@proxmox.com>

-- 
Maximiliano


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

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

[Thomas Lamprecht]

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

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

[Maximiliano Sandoval]

Thomas Lamprecht <t.lamprecht@proxmox.com> writes:

> 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

The directories are created with the right permissions and owner, the
issue here is that the monitor logs generated when we create the monitor
(the command above the call introduced by the patch) are created with
root as the owner.

-- 
Maximiliano


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

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

[Thomas Lamprecht]

Am 16.12.25 um 13:06 schrieb Maximiliano Sandoval:
> Thomas Lamprecht <t.lamprecht@proxmox.com> writes:
> 
>> 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
> 
> The directories are created with the right permissions and owner, the
> issue here is that the monitor logs generated when we create the monitor
> (the command above the call introduced by the patch) are created with
> root as the owner.
> 

Ok, thanks for your input, I missed your other reply due to searching
explicitly for Dominik's patch due to talking with him in the morning.

Anyhow, then I'd favor addressing the actual root cause in the
"ceph-mon --mkfs" command over this approach here, might not be that
complicated - I'm sure Max might have some pointers or could help, having
wrestled with the ceph tooling in the past.

Again, something like that here can still be fine as stop-gap, but then
I really would use chown function inside a call to dir_glob_regex from
PVE::Tools.


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

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

[Maximiliano Sandoval]

Thomas Lamprecht <t.lamprecht@proxmox.com> writes:

> Am 16.12.25 um 13:06 schrieb Maximiliano Sandoval:
>> Thomas Lamprecht <t.lamprecht@proxmox.com> writes:
>> 
>>> 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
>> 
>> The directories are created with the right permissions and owner, the
>> issue here is that the monitor logs generated when we create the monitor
>> (the command above the call introduced by the patch) are created with
>> root as the owner.
>> 
>
> Ok, thanks for your input, I missed your other reply due to searching
> explicitly for Dominik's patch due to talking with him in the morning.
>
> Anyhow, then I'd favor addressing the actual root cause in the
> "ceph-mon --mkfs" command over this approach here, might not be that
> complicated - I'm sure Max might have some pointers or could help, having
> wrestled with the ceph tooling in the past.
>
> Again, something like that here can still be fine as stop-gap, but then
> I really would use chown function inside a call to dir_glob_regex from
> PVE::Tools.

For the sake of documenting my findings: the problem when giving
ceph-mon the right ceph:ceph user (via the --set{user,group} optiosn) is
that our keyring is at /etc/pve and while this fixes the permissions on
the log file, the command (and task) would fail and the logs will end
in:

```
2025-12-16T13:48:47.307+0100 7282faa52cc0 -1 mon.c0-pve-101@-1(???) e0 unable to find a keyring on /etc/pve/priv/ceph.mon.keyring: (13) Permission denied
```

since the keyring has 600 permissions.

I think that one could simplify the proposed patch here to only chown
/var/log/ceph/ceph-mon.$monid.log instead of using any glob.

-- 
Maximiliano


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

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

[Thomas Lamprecht]

Am 16.12.25 um 13:54 schrieb Maximiliano Sandoval:
> For the sake of documenting my findings: the problem when giving
> ceph-mon the right ceph:ceph user (via the --set{user,group} optiosn) is
> that our keyring is at /etc/pve and while this fixes the permissions on
> the log file, the command (and task) would fail and the logs will end
> in:
> 
> ```
> 2025-12-16T13:48:47.307+0100 7282faa52cc0 -1 mon.c0-pve-101@-1(???) e0 unable to find a keyring on /etc/pve/priv/ceph.mon.keyring: (13) Permission denied
> ```
> 
> since the keyring has 600 permissions.

Ack, still sounds like it can be fixed in ceph-mon, but might be a bit
more involved; lets put that to the backlog for now.

> 
> I think that one could simplify the proposed patch here to only chown
> /var/log/ceph/ceph-mon.$monid.log instead of using any glob.

In that case I'd be OK with a specific chown to just that file, ideally
accompanied with a comment that includes a short variant of the basic
reasoning, like:

# fix-up initial log file from freshly created monitor here, as currently
# we cannot instruct ceph-mon to create it with the correct ownership while
# not losing access to the mon keyring inside pmxcfs.

might need polishing language/grammar wise though.




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