[pve-devel] [PATCH v4 ceph master, ceph quincy-stable-8, pve-storage, pve-manager 0/16] Fix #4759: Configure Permissions for ceph-crash.service

[Friedrich Weber]

Tested setting up a fresh Reef with patched packages, and tested
updating an existing Reef with the new packages. In both cases, crashes
are posted without noise in the journal and without having to manually
restart ceph-crash. Nice!

Also tested the case where [client.crash] already has a `key` (see my
comment on [1]) -- was properly rewritten to a `keyring` for me.

Don't have a Quincy cluster currently, so didn't test the backport.

One thing I noticed and already discussed with Max off-list: When
updating to the patched pve-manager package on a cluster without Ceph, I
get the following message:

> Setting up pve-manager (8.1.4) ...
> Ceph is not initialized. No action required.
> Failed to get unit file state for ceph-crash.service: No such file or
> directory

It's harmless, but might be confusing to users, so might be good to get
rid of.

Unrelated to this patch series, there are also the following message
from ceph-base:

> Setting system user ceph properties..usermod: no changes
> usermod: unlocking the user's password would result in a passwordless
> account.
> You should set a password with usermod -p to unlock this user's >
password.
> ..done
> chown: cannot access '/var/log/ceph/*.log*': No such file or directory
> Fixing /var/run/ceph ownership....done

Both the `usermod` stuff as well as the `chown` would be nice to
silence, but probably out of scope for this patch series.

[1] https://lists.proxmox.com/pipermail/pve-devel/2024-February/061956.html

On 05/03/2024 16:07, Max Carrara wrote:
> Fix #4759: Configure Permissions for ceph-crash.service - Version 4
> ===================================================================
> 
> Notable changes since v3
> ------------------------
> 
>   * Both parser and writer for 'ceph.conf' now have unit tests which run
>     during make targets like e.g. `make deb`, thanks to `dh_auto_test`
>   * The parser for 'ceph.conf' now correctly un-escapes comment literals
>     (found while developing unit tests)
>   * The writer for 'ceph.conf' now correctly escapes comment literals
>     (found while developing unit tests)
>   * The helper script called in 'postinst' of pve-manager for updating
>     'ceph.crash' in 'ceph.conf' now correctly handles an existing key
>     being referenced directly and removes it (thanks Friedrich!)
>   * The aforementioned helper script has more verbose output, showing
>     explicitly what's being done to the configuration
>   * The 'postinst' hook now prints an empty line before and after it
>     runs to make it a little more visible
>   * The 'postinst' hook now also restarts 'ceph-crash.service' if the
>     user hasn't disabled it (thanks Friedrich!)
> 
> For a detailed list of changes, please see the comments in the
> individual patches.
> 
> 
> Older Versions
> --------------
> 
> v1: https://lists.proxmox.com/pipermail/pve-devel/2024-January/061546.html
> v2: https://lists.proxmox.com/pipermail/pve-devel/2024-February/061646.html
> v3: https://lists.proxmox.com/pipermail/pve-devel/2024-February/061802.html
> 
> Summary of Changes
> ------------------
> 
> ceph (master):
> 
> Max Carrara (2):
>   debian: add patch to fix ceph crash dir permissions in postinst hook
>   patches: add patch that reorders clients used by ceph-crash
> 
>  ...ly-adjust-permissions-of-var-lib-cep.patch | 54 +++++++++++++++++++
>  ...h-crash-change-order-of-client-names.patch | 30 +++++++++++
>  patches/series                                |  2 +
>  3 files changed, 86 insertions(+)
>  create mode 100644 patches/0016-debian-recursively-adjust-permissions-of-var-lib-cep.patch
>  create mode 100644 patches/0017-ceph-crash-change-order-of-client-names.patch
> 
> 
> ceph (quincy-stable-8):
> 
> Max Carrara (2):
>   debian: add patch to fix ceph crash dir permissions in postinst hook
>   patches: add patch that reorders clients used by ceph-crash
> 
>  ...ly-adjust-permissions-of-var-lib-cep.patch | 54 +++++++++++++++++++
>  ...h-crash-change-order-of-client-names.patch | 30 +++++++++++
>  patches/series                                |  2 +
>  3 files changed, 86 insertions(+)
>  create mode 100644 patches/0025-debian-recursively-adjust-permissions-of-var-lib-cep.patch
>  create mode 100644 patches/0026-ceph-crash-change-order-of-client-names.patch
> 
> 
> pve-storage:
> 
> Max Carrara (9):
>   cephconfig: align our parser more with Ceph's parser
>   cephconfig: support line-continuations in parser
>   cephconfig: allow writing arbitrary sections
>   cephconfig: support escaped comment literals
>   cephconfig: emit warning for lines that fail to parse
>   cephconfig: change code style inside config writer
>   cephconfig: change order of written sections
>   cephconfig: remove leading whitespace on write to Ceph config
>   test: add tests for 'ceph.conf' parser and writer
> 
>  src/Makefile                               |   1 +
>  src/PVE/CephConfig.pm                      |  95 +++--
>  src/PVE/Makefile                           |   4 +
>  src/PVE/test/Makefile                      |   9 +
>  src/PVE/test/ceph_conf_parse_write_test.pl | 402 +++++++++++++++++++++
>  5 files changed, 490 insertions(+), 21 deletions(-)
>  create mode 100644 src/PVE/test/Makefile
>  create mode 100755 src/PVE/test/ceph_conf_parse_write_test.pl
> 
> 
> pve-manager:
> 
> Max Carrara (3):
>   ceph: introduce '/etc/pve/ceph'
>   fix #4759: ceph: configure ceph-crash.service and its key
>   bin/make: gather helper scripts in separate variable
> 
>  PVE/API2/Ceph.pm        |   5 ++
>  PVE/API2/Ceph/MON.pm    |   8 +++
>  PVE/Ceph/Tools.pm       |  47 ++++++++++++++-
>  bin/Makefile            |   6 +-
>  bin/pve-init-ceph-crash | 129 ++++++++++++++++++++++++++++++++++++++++
>  debian/postinst         |  26 ++++++++
>  6 files changed, 218 insertions(+), 3 deletions(-)
>  create mode 100755 bin/pve-init-ceph-crash
>