From: Friedrich Weber <f.weber@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
Max Carrara <m.carrara@proxmox.com>
Subject: Re: [pve-devel] [PATCH v3 ceph master, ceph quincy-stable 8, pve-storage, pve-manager 00/13] Fix #4759: Configure Permissions for ceph-crash.service
Date: Wed, 21 Feb 2024 12:55:50 +0100 [thread overview]
Message-ID: <4fa0d7e0-c428-48e0-85ee-422aa8d26e99@proxmox.com> (raw)
In-Reply-To: <20240216145615.2301594-1-m.carrara@proxmox.com>
Quickly tested the patch series on my existing Ceph Quincy cluster, did
not encounter major issues -- the keyring was created and the Ceph
config was rewritten accordingly. After a restart of `ceph-crash`, it
correctly posts crashes (produced with `kill -n11 $(pidof ceph-osd)`)
again and does not write any potentially misleading messages to the
journal. Nice!
Didn't have time yet to test these patches when setting up a new
cluster, but I'll try to do so this week and report back.
Two minor things I've noticed so far:
- the `ceph-crash` service does not restart after installing the patched
ceph-base package, so the reordering done by patches 02+04 does not take
effect immediately: ceph-crash posts crash logs just fine, but logs to
the journal that it can't find a keyring. After a restart of ceph-crash,
the patch takes effect, so only a tiny inconvenience, but still: Not
sure if restarting the service is something we'd want to do in a
postinst -- is this an acceptable thing to do in a postinst?
- Might there be issues in a mixed-version cluster scenario, so if some
node A already has an updated pve-storage package (patches 05-10), but
node B doesn't yet? One thing I noticed is that node A will add the
[client.crash] section, but node B may remove it again when it needs to
rewrite the Ceph config (e.g. when creating a monitor). I don't find
this particular issue too concerning, as hopefully node B will be
updated eventually as well and reinstate the [client.crash] section. But
I wonder if there could be other more serious issues?
On 16/02/2024 15:56, Max Carrara wrote:
> This marks version 03 of the patch series "Fix #4759: Configure
> Permissions for ceph-crash.service". Older versions can be found below.
>
> Notable changes since v2
> ------------------------
>
> * The 'ceph.conf' parser in pve-storage is now equivalent to Ceph's
> and even supports continued lines
> * The addition of the '/etc/pve/ceph' directory has been moved into a
> separate patch in order to preserve the context of its purpose in
> the git history
> * The debian `postinst` hook for pve-manager is now version-guarded
> and uses a separate Perl helper script instead of doing everything
> in BASH
>
>
> 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
>
>
>
> 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 (6):
> cephconfig: align our parser more with Ceph's parser
> cephconfig: support line-continuations in parser
> cephconfig: allow writing arbitrary sections
> cephconfig: change code style inside config writer
> cephconfig: change order of written sections
> cephconfig: remove leading whitespace on write to Ceph config
>
> src/PVE/CephConfig.pm | 80 +++++++++++++++++++++++++++++++------------
> 1 file changed, 58 insertions(+), 22 deletions(-)
>
>
> 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 | 104 ++++++++++++++++++++++++++++++++++++++++
> debian/postinst | 16 +++++++
> 6 files changed, 183 insertions(+), 3 deletions(-)
> create mode 100755 bin/pve-init-ceph-crash
>
next prev parent reply other threads:[~2024-02-21 11:55 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-16 14:56 Max Carrara
2024-02-16 14:56 ` [pve-devel] [PATCH v3 master ceph 01/13] debian: add patch to fix ceph crash dir permissions in postinst hook Max Carrara
2024-02-16 14:56 ` [pve-devel] [PATCH v3 master ceph 02/13] patches: add patch that reorders clients used by ceph-crash Max Carrara
2024-02-16 14:56 ` [pve-devel] [PATCH v3 quincy-stable-8 ceph 03/13] debian: add patch to fix ceph crash dir permissions in postinst hook Max Carrara
2024-02-16 14:56 ` [pve-devel] [PATCH v3 quincy-stable-8 ceph 04/13] patches: add patch that reorders clients used by ceph-crash Max Carrara
2024-02-16 14:56 ` [pve-devel] [PATCH v3 pve-storage 05/13] cephconfig: align our parser more with Ceph's parser Max Carrara
2024-02-16 14:56 ` [pve-devel] [PATCH v3 pve-storage 06/13] cephconfig: support line-continuations in parser Max Carrara
2024-02-16 14:56 ` [pve-devel] [PATCH v3 pve-storage 07/13] cephconfig: allow writing arbitrary sections Max Carrara
2024-02-16 14:56 ` [pve-devel] [PATCH v3 pve-storage 08/13] cephconfig: change code style inside config writer Max Carrara
2024-02-16 14:56 ` [pve-devel] [PATCH v3 pve-storage 09/13] cephconfig: change order of written sections Max Carrara
2024-02-16 14:56 ` [pve-devel] [PATCH v3 pve-storage 10/13] cephconfig: remove leading whitespace on write to Ceph config Max Carrara
2024-02-16 14:56 ` [pve-devel] [PATCH v3 pve-manager 11/13] ceph: introduce '/etc/pve/ceph' Max Carrara
2024-02-16 14:56 ` [pve-devel] [PATCH v3 pve-manager 12/13] fix #4759: ceph: configure ceph-crash.service and its key Max Carrara
2024-02-16 14:56 ` [pve-devel] [PATCH v3 pve-manager 13/13] bin/make: gather helper scripts in separate variable Max Carrara
2024-02-21 11:55 ` Friedrich Weber [this message]
2024-02-21 13:15 ` [pve-devel] [PATCH v3 ceph master, ceph quincy-stable 8, pve-storage, pve-manager 00/13] Fix #4759: Configure Permissions for ceph-crash.service Max Carrara
2024-02-23 16:19 ` Friedrich Weber
2024-02-26 9:51 ` Max Carrara
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=4fa0d7e0-c428-48e0-85ee-422aa8d26e99@proxmox.com \
--to=f.weber@proxmox.com \
--cc=m.carrara@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