[pve-devel] [PATCH v2 manager] fix #3451 ui: ceph create osd: show custom device classes

[pve-devel] [PATCH v2 manager] fix #3451 ui: ceph create osd: show custom device classes

[Aaron Lauterer]

Showing already configured custom device classes makes it easier to
create new OSDs with custom device classes.

The Crush map contains a list of all OSDs in the cluster, including
their device class.
This means we can create a list of used device classes from it, avoiding
adding another API endpoint.

Fetching the crushmap should also be quite a bit less data that needs to
be transferred, compared to the other possible nodes/<node>/ceph/osd
endpoint, especially in larger clusters.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---

changes:
- template string for API call
- API failure now throws an alert
- used Thomas idea on how to make the list creation more concise and
only update the combo items if we do have custom device classes
- reverted default `comboItems` to the element definition

@Thomas by using `Array.from` we can iterate over the results returned
from matchAll and also map element [1] from the returned array as it
actually holds the matched item.
The unshift needed the `...` deconstruction operator as comboItems is an
array of arrays.

 www/manager6/ceph/OSD.js | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/www/manager6/ceph/OSD.js b/www/manager6/ceph/OSD.js
index 30671dc4..e578026c 100644
--- a/www/manager6/ceph/OSD.js
+++ b/www/manager6/ceph/OSD.js
@@ -17,6 +17,27 @@ Ext.define('PVE.CephCreateOsd', {
 
 	me.isCreate = true;
 
+	Proxmox.Utils.API2Request({
+	    url: `/nodes/${me.nodename}/ceph/crush`,
+	    method: 'GET',
+	    failure: response => Ext.Msg.alert(gettext('Error'), response.htmlStatus),
+	    success: function({ result: { data } }) {
+		let classes = [...new Set(
+		    Array.from(
+			data.matchAll(/^device\s[0-9]*\sosd\.[0-9]*\sclass\s(.*)$/gim),
+			m => m[1]
+		    ).filter(v => !['hdd', 'ssd', 'nvme'].includes(v))
+		)].map(v => [v, v]);
+
+		if (classes.length) {
+		    let kvField = me.down('field[name=crush-device-class]');
+		    classes.unshift(...kvField.comboItems);
+
+		    kvField.setComboItems(classes);
+		}
+	    },
+	});
+
         Ext.applyIf(me, {
 	    url: "/nodes/" + me.nodename + "/ceph/osd",
 	    method: 'POST',
-- 
2.30.2

[pve-devel] [PATCHi v3 manager] fix #3451 ui: ceph create osd: show custom device classes

[Aaron Lauterer]

Showing already configured custom device classes makes it easier to
create new OSDs with custom device classes.

The Crush map contains a list of all OSDs in the cluster, including
their device class.
This means we can create a list of used device classes from it, avoiding
adding another API endpoint.

Fetching the crushmap should also be quite a bit less data that needs to
be transferred, compared to the other possible nodes/<node>/ceph/osd
endpoint, especially in larger clusters.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
changes from v2:
- check against eslint warnings, adding two commas

changes from v1:
- template string for API call
- API failure now throws an alert
- used Thomas idea on how to make the list creation more concise and
only update the combo items if we do have custom device classes
- reverted default `comboItems` to the element definition

@Thomas by using `Array.from` we can iterate over the results returned
from matchAll and also map element [1] from the returned array as it
actually holds the matched item.
The unshift needed the `...` deconstruction operator as comboItems is an
array of arrays.

 www/manager6/ceph/OSD.js | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/www/manager6/ceph/OSD.js b/www/manager6/ceph/OSD.js
index 30671dc4..9651de1b 100644
--- a/www/manager6/ceph/OSD.js
+++ b/www/manager6/ceph/OSD.js
@@ -17,6 +17,27 @@ Ext.define('PVE.CephCreateOsd', {
 
 	me.isCreate = true;
 
+	Proxmox.Utils.API2Request({
+	    url: `/nodes/${me.nodename}/ceph/crush`,
+	    method: 'GET',
+	    failure: response => Ext.Msg.alert(gettext('Error'), response.htmlStatus),
+	    success: function({ result: { data } }) {
+		let classes = [...new Set(
+		    Array.from(
+			data.matchAll(/^device\s[0-9]*\sosd\.[0-9]*\sclass\s(.*)$/gim),
+			m => m[1],
+		    ).filter(v => !['hdd', 'ssd', 'nvme'].includes(v)),
+		)].map(v => [v, v]);
+
+		if (classes.length) {
+		    let kvField = me.down('field[name=crush-device-class]');
+		    classes.unshift(...kvField.comboItems);
+
+		    kvField.setComboItems(classes);
+		}
+	    },
+	});
+
         Ext.applyIf(me, {
 	    url: "/nodes/" + me.nodename + "/ceph/osd",
 	    method: 'POST',
-- 
2.30.2

[pve-devel] applied: [PATCHi v3 manager] fix #3451 ui: ceph create osd: show custom device classes

[Thomas Lamprecht]

On 25.01.22 09:51, Aaron Lauterer wrote:
> Showing already configured custom device classes makes it easier to
> create new OSDs with custom device classes.
> 
> The Crush map contains a list of all OSDs in the cluster, including
> their device class.
> This means we can create a list of used device classes from it, avoiding
> adding another API endpoint.
> 
> Fetching the crushmap should also be quite a bit less data that needs to
> be transferred, compared to the other possible nodes/<node>/ceph/osd
> endpoint, especially in larger clusters.
> 
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
> changes from v2:
> - check against eslint warnings, adding two commas
> 
> changes from v1:
> - template string for API call
> - API failure now throws an alert
> - used Thomas idea on how to make the list creation more concise and
> only update the combo items if we do have custom device classes
> - reverted default `comboItems` to the element definition
> 
> @Thomas by using `Array.from` we can iterate over the results returned
> from matchAll and also map element [1] from the returned array as it
> actually holds the matched item.
> The unshift needed the `...` deconstruction operator as comboItems is an
> array of arrays.

I actually dropped the unshift and expanded directly:

kvField.setComboItems([...kvField.comboItems, ...classes]);

In theory this could allow a minimally slightly more efficient use, not that
it'd matter here much, but hey a line less code ;-P

> 
>  www/manager6/ceph/OSD.js | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
>

applied, thanks!