[pve-devel] [PATCH manager] pveceph: add ecpool create wrapper for the CLI

[pve-devel] [PATCH manager] pveceph: add ecpool create wrapper for the CLI

[Dominik Csapak]

that exposes the ec options as seperate parameters instead of a format string
(for convenience). I made the ceph_pool_common_options and ec_format
public so that we can reuse them for that

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
@Thomas, because you suggested something like that, but it seems a bit
out of place to me, and giving the options as property-string is not too
bad, e.g.

pveceph pool create poolname --erasure-coding k=2,m2

vs

pveceph ecpool create poolname 2 2

 PVE/API2/Ceph/Pools.pm | 14 +++++++++-----
 PVE/CLI/pveceph.pm     | 39 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 47 insertions(+), 6 deletions(-)

diff --git a/PVE/API2/Ceph/Pools.pm b/PVE/API2/Ceph/Pools.pm
index eeb81d65..7cf16b39 100644
--- a/PVE/API2/Ceph/Pools.pm
+++ b/PVE/API2/Ceph/Pools.pm
@@ -193,7 +193,7 @@ __PACKAGE__->register_method ({
     }});
 
 
-my $ceph_pool_common_options = sub {
+sub ceph_pool_common_options {
     my ($nodefault) = shift;
     my $options = {
 	name => {
@@ -276,7 +276,7 @@ my $ceph_pool_common_options = sub {
 	delete $options->{$_}->{default} for keys %$options;
     }
     return $options;
-};
+}
 
 
 my $add_storage = sub {
@@ -352,6 +352,10 @@ my $ec_format = {
     },
 };
 
+sub get_ec_format {
+    return $ec_format;
+}
+
 sub ec_parse_and_check {
     my ($property, $rados) = @_;
     return if !$property;
@@ -390,7 +394,7 @@ __PACKAGE__->register_method ({
 		format => $ec_format,
 		optional => 1,
 	    },
-	    %{ $ceph_pool_common_options->() },
+	    %{ ceph_pool_common_options() },
 	},
     },
     returns => { type => 'string' },
@@ -598,7 +602,7 @@ __PACKAGE__->register_method ({
 	additionalProperties => 0,
 	properties => {
 	    node => get_standard_option('pve-node'),
-	    %{ $ceph_pool_common_options->('nodefault') },
+	    %{ ceph_pool_common_options('nodefault') },
 	},
     },
     returns => { type => 'string' },
@@ -671,7 +675,7 @@ __PACKAGE__->register_method ({
 	    application_list       => { type => 'array', title => 'Application', optional => 1 },
 	    statistics             => { type => 'object', title => 'Statistics', optional => 1 },
 	    autoscale_status       => { type => 'object',  title => 'Autoscale Status', optional => 1 },
-	    %{ $ceph_pool_common_options->() },
+	    %{ ceph_pool_common_options() },
 	},
     },
     code => sub {
diff --git a/PVE/CLI/pveceph.pm b/PVE/CLI/pveceph.pm
index 995cfcd5..7fd110fd 100755
--- a/PVE/CLI/pveceph.pm
+++ b/PVE/CLI/pveceph.pm
@@ -15,7 +15,7 @@ use PVE::Cluster;
 use PVE::INotify;
 use PVE::RPCEnvironment;
 use PVE::Storage;
-use PVE::Tools qw(run_command);
+use PVE::Tools qw(run_command extract_param);
 use PVE::JSONSchema qw(get_standard_option);
 use PVE::Ceph::Tools;
 use PVE::Ceph::Services;
@@ -340,8 +340,45 @@ __PACKAGE__->register_method ({
 	return $rpcenv->fork_worker('cephdestroyfs', $fs_name,  $user, $worker);
     }});
 
+__PACKAGE__->register_method ({
+    name => 'ecpoolcreate',
+    path => 'ecpoolcreate',
+    method => 'POST',
+    description => "Create an erasure-coded pool",
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    node => get_standard_option('pve-node'),
+	    add_storages => {
+		description => "Configure VM and CT storage using the new pool. ".
+				"Always enabled for erasure coded pools.",
+		type => 'boolean',
+		optional => 1,
+	    },
+	    %{ PVE::API2::Ceph::Pools::ceph_pool_common_options() },
+	    %{ PVE::API2::Ceph::Pools::get_ec_format() },
+	},
+    },
+    returns => { type => 'string' },
+    code => sub {
+	my ($param) = @_;
+
+	my $ec_format = PVE::API2::Ceph::Pools::get_ec_format();
+	my $ec_params = {};
+	for my $k (keys %$ec_format) {
+	    $ec_params->{$k} = extract_param($param, $k);
+	}
+
+	$param->{'erasure-coding'} = PVE::JSONSchema::print_property_string($ec_params, $ec_format);
+
+	return PVE::API2::Ceph::Pools->createpool($param);
+    }});
+
 our $cmddef = {
     init => [ 'PVE::API2::Ceph', 'init', [], { node => $nodename } ],
+    ecpool => {
+	create => [ __PACKAGE__, 'ecpoolcreate', ['name', 'k', 'm'], { node => $nodename }, $upid_exit],
+    },
     pool => {
 	ls => [ 'PVE::API2::Ceph::Pools', 'lspools', [], { node => $nodename }, sub {
 	    my ($data, $schema, $options) = @_;
-- 
2.30.2

Re: [pve-devel] [PATCH manager] pveceph: add ecpool create wrapper for the CLI

[Thomas Lamprecht]

On 29.04.22 09:39, Dominik Csapak wrote:
> that exposes the ec options as seperate parameters instead of a format string
> (for convenience). I made the ceph_pool_common_options and ec_format
> public so that we can reuse them for that
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> @Thomas, because you suggested something like that, but it seems a bit
> out of place to me, and giving the options as property-string is not too
> bad, e.g.
> 
> pveceph pool create poolname --erasure-coding k=2,m2
> 
> vs
> 
> pveceph ecpool create poolname 2 2


I mean, I'd have done:

pveceph ec-pool create poolname -k 2 -m 2

which is a bit more telling than using fixed options for k/m but anyhow, you may be
right in that the format string isn't to bad after all for the CLI too, and avoiding
extra commands can have its merits as it reduces complexity in some way too.

So its fine for me as is, minus the missing docs naturally ;-P