[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.antreich at 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;
>