all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: "Max Carrara" <m.carrara@proxmox.com>
To: "Proxmox VE development discussion" <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] [PATCH v4 pve-manager 14/16] ceph: introduce '/etc/pve/ceph'
Date: Tue, 19 Mar 2024 17:01:32 +0100	[thread overview]
Message-ID: <CZXUPDWDJAZD.3OA10BBA87LA8@proxmox.com> (raw)
In-Reply-To: <1710841216.ygaihbiy98.astroid@yuna.none>

On Tue Mar 19, 2024 at 11:04 AM CET, Fabian Grünbichler wrote:
> On March 5, 2024 4:07 pm, Max Carrara wrote:
> > This commit adds the '/etc/pve/ceph' directory to our overall expected
> > Ceph configuration.
> > 
> > This directory is meant to store cluster-wide, non-private
> > configuration files used by Ceph applications and services that are
> > executed with lower privileges, such as 'ceph-crash.service'.
> > 
> > The existence of the directory is now also checked for when checking
> > whether Ceph is configured correctly. This makes it easier for our
> > other tooling to rely on the directory's existence, reducing the
> > number of otherwise needless frequent checking.
> > 
> > For new clusters: `pveceph init` now creates '/etc/pve/ceph' when
> > called.
> > 
> > For existing clusters: The 'postinst' hook this commit adds ensures
> > that '/etc/pve/ceph' is created upon update.
> > 
> > The 'postinst' hook is also version-guarded and does not run when
> > upgrading from pve-manager version 8.1.5 and above.
> > 
> > Signed-off-by: Max Carrara <m.carrara@proxmox.com>
> > ---
> > Changes v2 --> v3:
> >   * new; originally part of patches 09, 10 of series v2 - decided it's
> >     better to move all this into a separate patch to make context +
> >     intention clearer
> > Changes v3 --> v4:
> >   * none
> > 
> >  PVE/API2/Ceph.pm  |  5 +++++
> >  PVE/Ceph/Tools.pm | 12 ++++++++++--
> >  debian/postinst   | 12 ++++++++++++
> >  3 files changed, 27 insertions(+), 2 deletions(-)
> > 
> > diff --git a/PVE/API2/Ceph.pm b/PVE/API2/Ceph.pm
> > index 81c17d6e..7fedb87a 100644
> > --- a/PVE/API2/Ceph.pm
> > +++ b/PVE/API2/Ceph.pm
> > @@ -192,6 +192,11 @@ __PACKAGE__->register_method ({
> >  	    PVE::Ceph::Tools::check_ceph_installed('ceph_bin');
> >  	}
> >  
> > +	my $pve_ceph_cfgdir = PVE::Ceph::Tools::get_config('pve_ceph_cfgdir');
> > +	if (! -d $pve_ceph_cfgdir) {
> > +	    File::Path::make_path($pve_ceph_cfgdir);
> > +	}
> > +
> >  	my $auth = $param->{disable_cephx} ? 'none' : 'cephx';
> >  
> >  	# simply load old config if it already exists
> > diff --git a/PVE/Ceph/Tools.pm b/PVE/Ceph/Tools.pm
> > index ee6c515c..735bb116 100644
> > --- a/PVE/Ceph/Tools.pm
> > +++ b/PVE/Ceph/Tools.pm
> > @@ -18,6 +18,7 @@ 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_mon_key_path = "/etc/pve/priv/$ccname.mon.keyring";
> >  my $pve_ckeyring_path = "/etc/pve/priv/$ccname.client.admin.keyring";
> > @@ -37,6 +38,7 @@ 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,
> >  };
> > @@ -186,8 +188,14 @@ sub check_ceph_inited {
> >  
> >      return undef if !check_ceph_installed('ceph_mon', $noerr);
> >  
> > -    if (! -f $pve_ceph_cfgpath) {
> > -	die "pveceph configuration not initialized\n" if !$noerr;
> > +    my @errors;
> > +
> > +    push(@errors, "missing '$pve_ceph_cfgpath'") if ! -f $pve_ceph_cfgpath;
> > +    push(@errors, "missing '$pve_ceph_cfgdir'") if ! -d $pve_ceph_cfgdir;
> > +
> > +    if (@errors) {
> > +	my $err = 'pveceph configuration not initialized - ' . join(', ', @errors) . "\n";
> > +	die $err if !$noerr;
> >  	return undef;
> >      }
> >  
> > diff --git a/debian/postinst b/debian/postinst
> > index 6138ef6d..61b50f97 100755
> > --- a/debian/postinst
> > +++ b/debian/postinst
> > @@ -80,6 +80,14 @@ EOF
> >      fi
> >  }
> >  
> > +update_ceph_conf() {
> > +    CEPH_CONF_DIR='/etc/pve/ceph'
> > +
> > +    if test ! -d "${CEPH_CONF_DIR}"; then
> > +        mkdir -p "${CEPH_CONF_DIR}"
> > +    fi
> > +}
> > +
> >  migrate_apt_auth_conf() {
> >      output=""
> >      removed=""
> > @@ -190,6 +198,10 @@ case "$1" in
> >  
> >      set_lvm_conf
> >  
> > +    if test -n "$2" && dpkg --compare-versions "$2" 'lt' '8.1.5~'; then
>
> nit: never hard code such versions in patches unless you know they will be
> applied right away ;) instead, have a placeholder here and call it out
> prominently in the patch notes and cover letter.

Thanks, will do!

>
> > +        update_ceph_conf
> > +    fi
> > +
> >      if test ! -e /proxmox_install_mode; then
> >          # modeled after code generated by dh_start
> >          for unit in ${UNITS}; do
> > -- 
> > 2.39.2
> > 
> > 
> > 
> > _______________________________________________
> > pve-devel mailing list
> > pve-devel@lists.proxmox.com
> > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> > 
> > 
> > 
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel





  reply	other threads:[~2024-03-19 16:02 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-05 15:07 [pve-devel] [PATCH v4 ceph master, ceph quincy-stable-8, pve-storage, pve-manager 0/16] Fix #4759: Configure Permissions for ceph-crash.service Max Carrara
2024-03-05 15:07 ` [pve-devel] [PATCH v4 master ceph 1/16] debian: add patch to fix ceph crash dir permissions in postinst hook Max Carrara
2024-03-05 15:07 ` [pve-devel] [PATCH v4 master ceph 2/16] patches: add patch that reorders clients used by ceph-crash Max Carrara
2024-03-05 15:07 ` [pve-devel] [PATCH v4 quincy-stable-8 ceph 3/16] debian: add patch to fix ceph crash dir permissions in postinst hook Max Carrara
2024-03-05 15:07 ` [pve-devel] [PATCH v4 quincy-stable-8 ceph 4/16] patches: add patch that reorders clients used by ceph-crash Max Carrara
2024-03-05 15:07 ` [pve-devel] [PATCH v4 pve-storage 05/16] cephconfig: align our parser more with Ceph's parser Max Carrara
2024-03-19  9:38   ` Fabian Grünbichler
2024-03-19 15:58     ` Max Carrara
2024-03-05 15:07 ` [pve-devel] [PATCH v4 pve-storage 06/16] cephconfig: support line-continuations in parser Max Carrara
2024-03-19  9:37   ` Fabian Grünbichler
2024-03-19 15:59     ` Max Carrara
2024-03-20 16:59       ` Max Carrara
2024-03-05 15:07 ` [pve-devel] [PATCH v4 pve-storage 07/16] cephconfig: allow writing arbitrary sections Max Carrara
2024-03-05 15:07 ` [pve-devel] [PATCH v4 pve-storage 08/16] cephconfig: support escaped comment literals Max Carrara
2024-03-05 15:07 ` [pve-devel] [PATCH v4 pve-storage 09/13] cephconfig: emit warning for lines that fail to parse Max Carrara
2024-03-05 15:07 ` [pve-devel] [PATCH v4 pve-storage 10/16] cephconfig: change code style inside config writer Max Carrara
2024-03-05 15:07 ` [pve-devel] [PATCH v4 pve-storage 11/16] cephconfig: change order of written sections Max Carrara
2024-03-05 15:07 ` [pve-devel] [PATCH v4 pve-storage 12/16] cephconfig: remove leading whitespace on write to Ceph config Max Carrara
2024-03-05 15:07 ` [pve-devel] [PATCH v4 pve-storage 13/16] test: add tests for 'ceph.conf' parser and writer Max Carrara
2024-03-19  9:36   ` Fabian Grünbichler
2024-03-19 16:00     ` Max Carrara
2024-03-05 15:07 ` [pve-devel] [PATCH v4 pve-manager 14/16] ceph: introduce '/etc/pve/ceph' Max Carrara
2024-03-19 10:04   ` Fabian Grünbichler
2024-03-19 16:01     ` Max Carrara [this message]
2024-03-05 15:07 ` [pve-devel] [PATCH v4 pve-manager 15/16] fix #4759: ceph: configure ceph-crash.service and its key Max Carrara
2024-03-19 10:04   ` Fabian Grünbichler
2024-03-19 17:41     ` Max Carrara
2024-03-20  8:05       ` Fabian Grünbichler
2024-03-20  9:25         ` Max Carrara
2024-03-05 15:07 ` [pve-devel] [PATCH v4 pve-manager 16/16] bin/make: gather helper scripts in separate variable Max Carrara
2024-03-08 12:37 ` [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
2024-03-11 16:45 ` [pve-devel] partially-applied-series: " Thomas Lamprecht

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=CZXUPDWDJAZD.3OA10BBA87LA8@proxmox.com \
    --to=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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal