[pve-devel] [PATCH manager v2 1/2] api: ceph: add applications of each pool to the lspool endpoint

[pve-devel] [PATCH manager v2 1/2] api: ceph: add applications of each pool to the lspool endpoint

[Stefan Sterz]

since ceph luminous (ceph 12) pools need to be associated with at
least one applicaton expose this information here too so that clients
of this endpoint can use that information

Signed-off-by: Stefan Sterz <s.sterz@proxmox.com>
---
even though an application needs to be defined for a pool since
luminous, i tried to make this code fail gracefully in the case the
rados mon command fails or there is no application for a given pool.

thanks @ alwin antreich for pointing out that pools have applications!

 PVE/API2/Ceph/Pools.pm | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/PVE/API2/Ceph/Pools.pm b/PVE/API2/Ceph/Pools.pm
index 6c05250e..607b6c17 100644
--- a/PVE/API2/Ceph/Pools.pm
+++ b/PVE/API2/Ceph/Pools.pm
@@ -167,6 +167,8 @@ __PACKAGE__->register_method ({
 	# pg_autoscaler module is not enabled in Nautilus
 	my $autoscale = eval { $get_autoscale_status->($rados) };
 
+	my $apps = eval { $rados->mon_command({ prefix => 'osd pool application get' }) };
+
 	foreach my $e (@{$res->{pools}}) {
 	    my $d = {};
 	    foreach my $attr (@$attr_list) {
@@ -191,6 +193,10 @@ __PACKAGE__->register_method ({
 		$d->{percent_used} = $s->{percent_used};
 	    }
 
+	    if ($apps) {
+		$d->{applications} = $apps->{$d->{pool_name}} if $apps->{$d->{pool_name}};
+	    }
+
 	    # Cephs numerical pool types are barely documented. Found the following in the Ceph
 	    # codebase: https://github.com/ceph/ceph/blob/ff144995a849407c258bcb763daa3e03cfce5059/src/osd/osd_types.h#L1221-L1233
 	    if ($e->{type} == 1) {
-- 
2.30.2

[pve-devel] [PATCH manager v2 2/2] ui: remove ceph-mgr pools from rbd pool selection

[Stefan Sterz]

when using a hyper-converged cluster it was previously possible to add
the pool used by the ceph-mgr modules (".mgr" since quincy or
"device_health_metrics" previously) as an RBD storage. this would lead
to all kinds of errors when that storage was used (e.g.: VMs missing
their disks after a migration). hence, filter these pools from the
list of available pools.

Signed-off-by: Stefan Sterz <s.sterz@proxmox.com>
---
similar to the previous api change this tries to fail gracefully if no
applications are defined for a pool.

 www/manager6/form/CephPoolSelector.js | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/www/manager6/form/CephPoolSelector.js b/www/manager6/form/CephPoolSelector.js
index 5b96398d..4dd77269 100644
--- a/www/manager6/form/CephPoolSelector.js
+++ b/www/manager6/form/CephPoolSelector.js
@@ -15,9 +15,17 @@ Ext.define('PVE.form.CephPoolSelector', {
 	    throw "no nodename given";
 	}
 
+	let filterCephMgrPools = (item) => {
+	    let apps = item.data?.applications;
+	    return apps === undefined || apps?.rbd !== undefined;
+	};
+
 	var store = Ext.create('Ext.data.Store', {
 	    fields: ['name'],
 	    sorters: 'name',
+	    filters: [
+		filterCephMgrPools,
+	    ],
 	    proxy: {
 		type: 'proxmox',
 		url: '/api2/json/nodes/' + me.nodename + '/ceph/pools',
@@ -32,8 +40,10 @@ Ext.define('PVE.form.CephPoolSelector', {
 
 	store.load({
 	    callback: function(rec, op, success) {
-		if (success && rec.length > 0) {
-		    me.select(rec[0]);
+		let filteredRec = rec.filter(filterCephMgrPools);
+
+		if (success && filteredRec.length > 0) {
+		    me.select(filteredRec[0]);
 		}
 	    },
 	});
-- 
2.30.2

Re: [pve-devel] [PATCH manager v2 2/2] ui: remove ceph-mgr pools from rbd pool selection

[Stefan Sterz]

On 10/19/22 14:16, Stefan Sterz wrote:
> when using a hyper-converged cluster it was previously possible to add
> the pool used by the ceph-mgr modules (".mgr" since quincy or
> "device_health_metrics" previously) as an RBD storage. this would lead
> to all kinds of errors when that storage was used (e.g.: VMs missing
> their disks after a migration). hence, filter these pools from the
> list of available pools.

just realized that this commit message doesn't really make sense
anymore, sorry for the inconvenience. i'd suggest changing it to the
following (if necessary i can also send the patch again):

ui: only allow rbd pools to be added as rbd storage

previously the ui would allow adding all pools (even the default
ceph-mon pools) as storage. this could lead to issues when users did
use these pools as storage (e.g.: vms missing their disks after a
migration). hence, restrict the pool selector to rbd pools.

fails gracefully be reverting to the previous behavior if a pool has
no application assigned to it.

>> Signed-off-by: Stefan Sterz <s.sterz@proxmox.com> ---
> similar to the previous api change this tries to fail gracefully if no
> applications are defined for a pool.
> 
>  www/manager6/form/CephPoolSelector.js | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/www/manager6/form/CephPoolSelector.js b/www/manager6/form/CephPoolSelector.js
> index 5b96398d..4dd77269 100644
> --- a/www/manager6/form/CephPoolSelector.js
> +++ b/www/manager6/form/CephPoolSelector.js
> @@ -15,9 +15,17 @@ Ext.define('PVE.form.CephPoolSelector', {
>  	    throw "no nodename given";
>  	}
>  
> +	let filterCephMgrPools = (item) => {
> +	    let apps = item.data?.applications;
> +	    return apps === undefined || apps?.rbd !== undefined;
> +	};
> +
>  	var store = Ext.create('Ext.data.Store', {
>  	    fields: ['name'],
>  	    sorters: 'name',
> +	    filters: [
> +		filterCephMgrPools,
> +	    ],
>  	    proxy: {
>  		type: 'proxmox',
>  		url: '/api2/json/nodes/' + me.nodename + '/ceph/pools',
> @@ -32,8 +40,10 @@ Ext.define('PVE.form.CephPoolSelector', {
>  
>  	store.load({
>  	    callback: function(rec, op, success) {
> -		if (success && rec.length > 0) {
> -		    me.select(rec[0]);
> +		let filteredRec = rec.filter(filterCephMgrPools);
> +
> +		if (success && filteredRec.length > 0) {
> +		    me.select(filteredRec[0]);
>  		}
>  	    },
>  	});

Re: [pve-devel] [PATCH manager v2 2/2] ui: remove ceph-mgr pools from rbd pool selection

[Stefan Sterz]

On 10/20/22 06:07, Alwin Antreich wrote:
> On October 19, 2022 2:16:44 PM GMT+02:00, Stefan Sterz <s.sterz@proxmox.com> wrote:
>> when using a hyper-converged cluster it was previously possible to add
>> the pool used by the ceph-mgr modules (".mgr" since quincy or
>> "device_health_metrics" previously) as an RBD storage. this would lead
>> to all kinds of errors when that storage was used (e.g.: VMs missing
>> their disks after a migration). hence, filter these pools from the
>> list of available pools.
>>
>> Signed-off-by: Stefan Sterz <s.sterz@proxmox.com>
>> ---
>> similar to the previous api change this tries to fail gracefully if no
>> applications are defined for a pool.
>>
>> www/manager6/form/CephPoolSelector.js | 14 ++++++++++++--
>> 1 file changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/www/manager6/form/CephPoolSelector.js b/www/manager6/form/CephPoolSelector.js
>> index 5b96398d..4dd77269 100644
>> --- a/www/manager6/form/CephPoolSelector.js
>> +++ b/www/manager6/form/CephPoolSelector.js
>> @@ -15,9 +15,17 @@ Ext.define('PVE.form.CephPoolSelector', {
>> 	    throw "no nodename given";
>> 	}
>>
>> +	let filterCephMgrPools = (item) => {
>> +	    let apps = item.data?.applications;
>> +	    return apps === undefined || apps?.rbd !== undefined;
>> +	};
> I think the variable should be called filterCephRBDPools or onlyCephRBDPools. As you only want RBD nothing else.
> 
> Ceph has lots of other pools that shouldn't be used, eg. .rgw.root (or anything else aside from RBD for that matter).
> 
yes that makes sense, thanks! sent a v3 already with this change and i
also added another small improvement that i forgot previously.

>> +
>> 	var store = Ext.create('Ext.data.Store', {
>> 	    fields: ['name'],
>> 	    sorters: 'name',
>> +	    filters: [
>> +		filterCephMgrPools,
>> +	    ],
>> 	    proxy: {
>> 		type: 'proxmox',
>> 		url: '/api2/json/nodes/' + me.nodename + '/ceph/pools',
>> @@ -32,8 +40,10 @@ Ext.define('PVE.form.CephPoolSelector', {
>>
>> 	store.load({
>> 	    callback: function(rec, op, success) {
>> -		if (success && rec.length > 0) {
>> -		    me.select(rec[0]);
>> +		let filteredRec = rec.filter(filterCephMgrPools);
>> +
>> +		if (success && filteredRec.length > 0) {
>> +		    me.select(filteredRec[0]);
>> 		}
>> 	    },
>> 	});
> 
> Cheers,
> Alwin 
> 
> Hi again :)
>