Re: [pve-devel] [PATCH manager v2 4/4] ui: osd: add details window

[Dominik Csapak <d.csapak@proxmox.com>]

comments inline:

On 7/6/22 15:01, Aaron Lauterer wrote:
> This new windows provides more detailes about an OSD such as:
> * PID
> * Memory usage
> * various metadata that could be of interest
> * list of phyiscal disks used for the main disk, db and wal with
>    additional infos about the volumes for each
> 
> A new 'Details' button is added to the OSD overview and a double click
> on an OSD will also open this new window.
> 
> The componend defines the items in the initComponent instead of
> following a fully declarative approach. This is because we need to pass
> the same store to multiple Proxmox.ObjectGrids.
> 
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
> changes since v2:
> - adapting API urls
> - renaming me.url to me.baseUrl
> 
>   www/manager6/Makefile           |   1 +
>   www/manager6/ceph/OSD.js        |  26 +++
>   www/manager6/ceph/OSDDetails.js | 289 ++++++++++++++++++++++++++++++++
>   3 files changed, 316 insertions(+)
>   create mode 100644 www/manager6/ceph/OSDDetails.js
> 
> diff --git a/www/manager6/Makefile b/www/manager6/Makefile
> index d16770b1..2535aaea 100644
> --- a/www/manager6/Makefile
> +++ b/www/manager6/Makefile
> @@ -179,6 +179,7 @@ JSSRC= 							\
>   	ceph/Log.js					\
>   	ceph/Monitor.js					\
>   	ceph/OSD.js					\
> +	ceph/OSDDetails.js				\
>   	ceph/Pool.js					\
>   	ceph/ServiceList.js				\
>   	ceph/Services.js				\
> diff --git a/www/manager6/ceph/OSD.js b/www/manager6/ceph/OSD.js
> index 78f226ff..75855f95 100644
> --- a/www/manager6/ceph/OSD.js
> +++ b/www/manager6/ceph/OSD.js
> @@ -481,6 +481,20 @@ Ext.define('PVE.node.CephOsdTree', {
>   	    });
>   	},
>   
> +	run_details: function(view, rec) {
> +	    if (rec.data.host && rec.data.type === 'osd' && rec.data.id >= 0) {
> +		this.details();
> +	    }
> +	},
> +
> +	details: function() {
> +	    let vm = this.getViewModel();
> +	    Ext.create('PVE.CephOsdDetails', {
> +		nodename: vm.get('osdhost'),
> +		osdid: vm.get('osdid'),
> +	    }).show();
> +	},
> +
>   	set_selection_status: function(tp, selection) {
>   	    if (selection.length < 1) {
>   		return;
> @@ -593,6 +607,9 @@ Ext.define('PVE.node.CephOsdTree', {
>       stateId: 'grid-ceph-osd',
>       rootVisible: false,
>       useArrows: true,
> +    listeners: {
> +	itemdblclick: 'run_details',
> +    },
>   
>       columns: [
>   	{
> @@ -733,6 +750,15 @@ Ext.define('PVE.node.CephOsdTree', {
>   		    '</tpl>',
>   		],
>   	    },
> +	    {
> +		text: gettext('Details'),
> +		iconCls: 'fa fa-info-circle',
> +		disabled: true,
> +		bind: {
> +		    disabled: '{!isOsd}',
> +		},
> +		handler: 'details',
> +	    },
>   	    {
>   		text: gettext('Start'),
>   		iconCls: 'fa fa-play',
> diff --git a/www/manager6/ceph/OSDDetails.js b/www/manager6/ceph/OSDDetails.js
> new file mode 100644
> index 00000000..738aa227
> --- /dev/null
> +++ b/www/manager6/ceph/OSDDetails.js
> @@ -0,0 +1,289 @@
> +Ext.define('pve-osd-details-devices', {
> +    extend: 'Ext.data.Model',
> +    fields: ['device', 'type', 'devices', 'size', 'support_discard', 'dev_node'],
> +    idProperty: 'device',
> +});
> +
> +Ext.define('PVE.CephOsdDetails', {
> +    extend: 'Ext.window.Window',
> +    alias: ['widget.pveCephOsdDetails'],
> +
> +    mixins: ['Proxmox.Mixin.CBind'],
> +
> +    cbindData: function() {
> +	let me = this;
> +	me.baseUrl = `/nodes/${me.nodename}/ceph/osd/${me.osdid}`;
> +	return {
> +	    title: `${gettext('Details')}: OSD ${me.osdid}`,
> +	};
> +    },
> +
> +    viewModel: {
> +	data: {
> +	    device: '',
> +	},
> +    },
> +
> +    modal: true,
> +    width: 650,
> +    minHeight: 250,
> +    resizable: true,
> +    cbind: {
> +	title: '{title}',
> +    },
> +
> +    layout: {
> +	type: 'vbox',
> +	align: 'stretch',
> +    },
> +    defaults: {
> +	layout: 'fit',
> +	border: false,
> +    },
> +
> +    controller: {
> +	xclass: 'Ext.app.ViewController',
> +
> +	reload: function() {
> +	    let view = this.getView();
> +
> +	    Proxmox.Utils.API2Request({
> +		url: `${view.baseUrl}/metadata`,
> +		waitMsgTarget: view,
> +		method: 'GET',
> +		failure: function(response, opts) {
> +		    Proxmox.Utils.setErrorMask(view, response.htmlStatus);

the 'view' here is the whole window, in case of an error, we even mask the
'close' button. better use the tabpanel, or wrap the toolbar + tabpanel in
a panel which you can then use

> +		},
> +		success: function(response, opts) {
> +		    let d = response.result.data;
> +		    let map_data = function(data) {
> +			return Object.keys(data).sort().map(x => ({ key: x, value: data[x] }));
> +		    };
> +		    let osdData = map_data(d.osd);

map_data is only used once, so it could be done inline?

> +		    d.bdev.device = 'block';

wouldn't it make sense to do that in the api?

> +		    let devData = [d.bdev];
> +		    if (d.db) {
> +			d.db.device = 'db';

same here?

> +			devData.push(d.db);
> +		    }
> +		    if (d.wal) {
> +			d.wal.device = 'wal';

and here?

> +			devData.push(d.wal);

also seeing this, wouldn't it make sense to just return a 'devices' array
from the api instead of building it here in the ui?

i know it's not very likely that we get another device type, but since
we are now inventing it, shouldn't it match with what we expect in the gui?
(one could argue that having it the current way makes for a nicer api, since
i can directly access the properties... mhmm)

> +		    }
> +		    view.osdStore.loadData(osdData);
> +		    let devices = view.lookup('devices');
> +		    let deviceStore = devices.getStore();
> +		    deviceStore.loadData(devData);
> +
> +		    view.lookup('osdGeneral').rstore.fireEvent('load', view.osdStore, osdData, true);
> +		    view.lookup('osdNetwork').rstore.fireEvent('load', view.osdStore, osdData, true);
> +
> +		    // select 'block' device automatically on first load
> +		    if (devices.getSelection().length === 0) {
> +			devices.setSelection(deviceStore.findRecord('device', 'block'));
> +		    }
> +		},
> +	    });
> +	},
> +
> +	showDevInfo: function(grid, selected) {
> +	    let view = this.getView();
> +	    if (selected[0]) {
> +		let device = selected[0].data.device;
> +		this.getViewModel().set('device', device);
> +
> +		let detailStore = view.lookup('volumeDetails');
> +		detailStore.rstore.getProxy().setUrl(`api2/json${view.baseUrl}/lv-info`);
> +		detailStore.rstore.getProxy().setExtraParams({ 'type': device });
> +		detailStore.reload();
> +	    }
> +	},
> +
> +	init: function() {
> +	    let me = this;
> +	    let view = me.getView();
> +	    view.lookup('osdGeneral').down('tableview').enableTextSelection=true;
> +	    view.lookup('osdNetwork').down('tableview').enableTextSelection=true;
> +	    view.lookup('volumeDetails').down('tableview').enableTextSelection=true;

AFAIR, these can be specified on the grid with 'viewConfig', e.g.
having the following in the grid config:

viewConfig: {
	enableTextSelection: true,
},

> +
> +	    me.reload();
> +	},
> +
> +	control: {
> +	    'grid[reference=devices]': {
> +		selectionchange: 'showDevInfo',
> +	    },
> +	},
> +    },
> +    tbar: [
> +	{
> +	    text: gettext('Reload'),
> +	    iconCls: 'fa fa-refresh',
> +	    handler: 'reload',
> +	},
> +    ],
> +    initComponent: function() {
> +        let me = this;
> +
> +	me.osdStore = Ext.create('Proxmox.data.ObjectStore');
> +
> +	Ext.applyIf(me, {
> +	    items: [
> +		{
> +		    xtype: 'tabpanel',
> +		    reference: 'detailsTabs',
> +		    items: [
> +			{
> +			    xtype: 'proxmoxObjectGrid',
> +			    reference: 'osdGeneral',
> +			    tooltip: gettext('Various information about the OSD'),
> +			    rstore: me.osdStore,
> +			    title: gettext('General'),
> +			    gridRows: [
> +				{
> +				    xtype: 'text',
> +				    name: 'version',
> +				    text: gettext('Version'),
> +				},
> +				{
> +				    xtype: 'text',
> +				    name: 'hostname',
> +				    text: gettext('Hostname'),
> +				},
> +				{
> +				    xtype: 'text',
> +				    name: 'osd_data',
> +				    text: gettext('OSD data path'),
> +				},
> +				{
> +				    xtype: 'text',
> +				    name: 'osd_objectstore',
> +				    text: gettext('OSD object store'),
> +				},
> +				{
> +				    xtype: 'text',
> +				    name: 'mem_usage',
> +				    text: gettext('Memory usage'),
> +				    renderer: Proxmox.Utils.render_size,
> +				},
> +				{
> +				    xtype: 'text',
> +				    name: 'pid',
> +				    text: `${gettext('Process ID')} (PID)`,
> +				},
> +			    ],
> +			},
> +			{
> +			    xtype: 'proxmoxObjectGrid',
> +			    reference: 'osdNetwork',
> +			    tooltip: gettext('Addresses and ports used by the OSD service'),
> +			    rstore: me.osdStore,
> +			    title: gettext('Network'),
> +			    gridRows: [
> +				{
> +				    xtype: 'text',
> +				    name: 'front_addr',
> +				    text: `${gettext('Front Address')}<br>(Client & Monitor)`,
> +				    renderer: PVE.Utils.render_ceph_osd_addr,
> +				},
> +				{
> +				    xtype: 'text',
> +				    name: 'hb_front_addr',
> +				    text: gettext('Heartbeat Front Address'),
> +				    renderer: PVE.Utils.render_ceph_osd_addr,
> +				},
> +				{
> +				    xtype: 'text',
> +				    name: 'back_addr',
> +				    text: `${gettext('Back Address')}<br>(OSD)`,
> +				    renderer: PVE.Utils.render_ceph_osd_addr,
> +				},
> +				{
> +				    xtype: 'text',
> +				    name: 'hb_back_addr',
> +				    text: gettext('Heartbeat Back Address'),
> +				    renderer: PVE.Utils.render_ceph_osd_addr,
> +				},
> +			    ],
> +			},
> +			{
> +			    xtype: 'panel',
> +			    title: 'Devices',
> +			    tooltip: gettext('Physical devices used by the OSD'),
> +			    items: [
> +				{
> +				    xtype: 'grid',
> +				    border: false,
> +				    reference: 'devices',
> +				    store: {
> +					model: 'pve-osd-details-devices',
> +				    },
> +				    columns: {
> +					items: [
> +					    { text: gettext('Name'), dataIndex: 'device' },
> +					    { text: gettext('Type'), dataIndex: 'type' },
> +					    {
> +						text: gettext('Physical Devices'),
> +						dataIndex: 'devices',
> +					    },
> +					    {
> +						text: gettext('Size'),
> +						dataIndex: 'size',
> +						renderer: Proxmox.Utils.render_size,
> +					    },
> +					    {
> +						text: 'Discard',
> +						dataIndex: 'support_discard',
> +						hidden: true,
> +					    },
> +					    {
> +						text: gettext('Device node'),
> +						dataIndex: 'dev_node',
> +						hidden: true,
> +					    },
> +					],
> +					defaults: {
> +					    tdCls: 'pointer',
> +					    flex: 1,
> +					},
> +				    },
> +				},
> +				{
> +				    xtype: 'proxmoxObjectGrid',
> +				    reference: 'volumeDetails',
> +				    maskOnLoad: true,
> +				    bind: {
> +					title: Ext.String.format(
> +						gettext('Volume Details for {0}'),
> +						'{device}',
> +					),
> +				    },
> +				    rows: {
> +					creation_time: {
> +					    header: gettext('Creation time'),
> +					},
> +					lv_name: {
> +					    header: gettext('LV Name'),
> +					},
> +					lv_path: {
> +					    header: gettext('LV Path'),
> +					},
> +					lv_uuid: {
> +					    header: gettext('LV UUID'),
> +					},
> +					vg_name: {
> +					    header: gettext('VG Name'),
> +					},
> +				    },
> +				    url: 'nodes/', //placeholder will be set when device is selected
> +				},
> +			    ],
> +			},
> +		    ],
> +		},
> +	    ],
> +	});
> +
> +	me.callParent();
> +    },
> +});