Re: [pve-devel] [PATCH manager v2] implement checks for ceph version & binaries

[Thomas Lamprecht]

thanks for v2, looks good!

The single thing I would still change is the die-message if the ceph
version is to old. We strongly go from a specific ceph version to
another in PVE, so the "...or higher..." from the "init" API entry point:

+           die "Ceph version luminous or higher is required\n";

line may (or may not, by luck) be wrong. This can be easily fixed up
on/after applying so it does not warrants a v3, IMO.

On 08/24/2017 02:45 PM, Alwin Antreich wrote:

add version check to ceph init to require luminous or higher and
fix #1481: check existence of ceph binaries before use

Signed-off-by: Alwin Antreich <a.antre...@proxmox.com>
---
  PVE/API2/Ceph.pm | 17 ++++++++++++++++-
  PVE/CephTools.pm | 34 +++++++++++++++++++++++++++++-----
  2 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/PVE/API2/Ceph.pm b/PVE/API2/Ceph.pm
index c4d6ffcb..652ab14c 100644
--- a/PVE/API2/Ceph.pm
+++ b/PVE/API2/Ceph.pm
@@ -233,6 +233,8 @@ __PACKAGE__->register_method ({

PVE::CephTools::setup_pve_symlinks(); + PVE::CephTools::check_ceph_installed('ceph_osd');

+
        my $bluestore = $param->{bluestore} // 0;

my $journal_dev;

@@ -827,7 +829,13 @@ __PACKAGE__->register_method ({
      code => sub {
        my ($param) = @_;

- PVE::CephTools::check_ceph_installed();

+       my $version = PVE::CephTools::get_local_version(1);
+
+       if (!$version || $version < 12) {
+           die "Ceph version luminous or higher is required\n";
+       } else {
+           PVE::CephTools::check_ceph_installed('ceph_bin');
+       }

# simply load old config if it already exists

        my $cfg = PVE::CephTools::parse_ceph_config();
@@ -982,6 +990,11 @@ __PACKAGE__->register_method ({
      code => sub {
        my ($param) = @_;

+ PVE::CephTools::check_ceph_installed('ceph_mon');

+
+       PVE::CephTools::check_ceph_installed('ceph_mgr')
+           if (!$param->{'exclude-manager'});
+
        PVE::CephTools::check_ceph_inited();

PVE::CephTools::setup_pve_symlinks();

@@ -1220,6 +1233,8 @@ __PACKAGE__->register_method ({
      code => sub {

my ($param) = @_> + PVE::CephTools::check_ceph_installed('ceph_mgr');

+
        PVE::CephTools::check_ceph_inited();

my $rpcenv = PVE::RPCEnvironment::get();

diff --git a/PVE/CephTools.pm b/PVE/CephTools.pm
index 0c0d7c18..b104b5e8 100644
--- a/PVE/CephTools.pm
+++ b/PVE/CephTools.pm
@@ -20,7 +20,12 @@ my $pve_ckeyring_path = 
"/etc/pve/priv/$ccname.client.admin.keyring";
  my $ceph_bootstrap_osd_keyring = 
"/var/lib/ceph/bootstrap-osd/$ccname.keyring";
  my $ceph_bootstrap_mds_keyring = 
"/var/lib/ceph/bootstrap-mds/$ccname.keyring";

-my $ceph_bin = "/usr/bin/ceph";

+my $ceph_service = {
+    ceph_bin => "/usr/bin/ceph",
+    ceph_mon => "/usr/bin/ceph-mon",
+    ceph_mgr => "/usr/bin/ceph-mgr",
+    ceph_osd => "/usr/bin/ceph-osd"
+};

my $config_hash = {

      ccname => $ccname,
@@ -32,6 +37,23 @@ my $config_hash = {
      long_rados_timeout => 60,
  };

+sub get_local_version {

+    my ($noerr) = @_;
+
+    if (PVE::CephTools::check_ceph_installed('ceph_bin', $noerr)) {
+       my $ceph_version;
+       run_command([$ceph_service->{ceph_bin}, '--version'],
+                   noerr => $noerr,
+                   outfunc => sub { $ceph_version = shift; });
+       if ($ceph_version && $ceph_version =~ /^ceph.*\s((\d+)\.(\d+)\.(\d+))/) 
{
+           # return (version, major, minor, patch) : major;
+           return wantarray ? ($1, $2, $3, $4) : $2;
+       }
+    }
+
+    return undef;
+}
+
  sub get_config {
      my $key = shift;

@@ -60,10 +82,12 @@ sub purge_all_ceph_files {

  }

sub check_ceph_installed {

-    my ($noerr) = @_;
+    my ($service, $noerr) = @_;
+
+    $service = 'ceph_bin' if !defined($service);

- if (! -x $ceph_bin) {

-       die "ceph binaries not installed\n" if !$noerr;
+    if (! -x $ceph_service->{$service}) {
+       die "binary not installed: $ceph_service->{$service}\n" if !$noerr;
        return undef;
      }

@@ -73,7 +97,7 @@ sub check_ceph_installed {

  sub check_ceph_inited {
      my ($noerr) = @_;

- return undef if !check_ceph_installed($noerr);

+    return undef if !check_ceph_installed('ceph_bin', $noerr);

if (! -f $pve_ceph_cfgpath) {

        die "pveceph configuration not initialized\n" if !$noerr;

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