[pve-devel] [PATCH widget-toolkit/manager v2 0/4] Ceph OSD: add detail infos

[pve-devel] [PATCH widget-toolkit/manager v2 0/4] Ceph OSD: add detail infos

[Aaron Lauterer]

This patch series adds 2 new API endpoints for OSDs to fetch more
detailed information about a single OSD. One for overall information and
one for a single volume (block, db, wal).

Because the api endpoint for the volume infos can take a few moments, we
most likely want to show that loading state. For this, a small patch in
the widget-toolkit->ObjectGrid is added that makes it possible to enable
this behavior.

More in the actual patches.

Changes since v1:
squashed API commits into one
all new API endpoints are below {osdid} and {osdid} returns an index
incorporate other code improvements

widget-toolkit: Aaron Lauterer (1):
  ObjectGrid: optionally show loading mask on load

 src/grid/ObjectGrid.js | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

manager: Aaron Lauterer (3):
  api ceph osd: add OSD index, metadata and lv-info
  ui utils: add renderer for ceph osd addresses
  ui: osd: add details window

 PVE/API2/Ceph/OSD.pm            | 315 ++++++++++++++++++++++++++++++++
 www/manager6/Makefile           |   1 +
 www/manager6/Utils.js           |  12 ++
 www/manager6/ceph/OSD.js        |  26 +++
 www/manager6/ceph/OSDDetails.js | 289 +++++++++++++++++++++++++++++
 5 files changed, 643 insertions(+)
 create mode 100644 www/manager6/ceph/OSDDetails.js

-- 
2.30.2

[pve-devel] [PATCH widget-toolkit v2 1/4] ObjectGrid: optionally show loading mask on load

[Aaron Lauterer]

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

changes since v1:
- added description in the documention at the top
- renamed paramter to "maskOnLoad" as I couldn't come up with something
more fitting ;)

 src/grid/ObjectGrid.js | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/grid/ObjectGrid.js b/src/grid/ObjectGrid.js
index b355d6d..065a009 100644
--- a/src/grid/ObjectGrid.js
+++ b/src/grid/ObjectGrid.js
@@ -39,6 +39,8 @@ disabled:: setting this parameter to true will disable selection and focus on
   the proxmoxObjectGrid as well as greying out input elements. Useful for a
   readonly tabular display
 
+maskOnLoad: if set to true, will show a "Loading..." while the store is loading data
+
 */
 Ext.define('Proxmox.grid.ObjectGrid', {
     extend: 'Ext.grid.GridPanel',
@@ -48,6 +50,8 @@ Ext.define('Proxmox.grid.ObjectGrid', {
     // see top-level doc-comment above for details/example
     gridRows: [],
 
+    maskOnLoad: false,
+
     disabled: false,
     hideHeaders: true,
 
@@ -221,7 +225,10 @@ Ext.define('Proxmox.grid.ObjectGrid', {
 
     reload: function() {
 	let me = this;
-	me.rstore.load();
+	if (me.maskOnLoad) {
+	    me.setLoading();
+	}
+	me.rstore.load({ callback: () => me.setLoading(false) });
     },
 
     getObjectValue: function(key, defaultValue) {
-- 
2.30.2

[pve-devel] [PATCH manager v2 2/4] api ceph osd: add OSD index, metadata and lv-info

[Aaron Lauterer]

To get more details for a single OSD, we add two new endpoints:
* nodes/{node}/ceph/osd/{osdid}/metadata
* nodes/{node}/ceph/osd/{osdid}/lv-info

The {osdid} endpoint itself gets a new GET handler to return the index.

The metadata one provides various metadata regarding the OSD.

Such as
* process id
* memory usage
* info about devices used (bdev/block, db, wal)
    * size
    * disks used (sdX)
    ...
* network addresses and ports used
...

Memory usage and PID are retrieved from systemd while the rest can be
retrieved from the metadata provided by Ceph.

The second one (lv-info) returns the following infos for a logical
volume:
* creation time
* lv name
* lv path
* lv size
* lv uuid
* vg name

Possible volumes are:
* block (default value if not provided)
* db
* wal

'ceph-volume' is used to gather the infos, except for the creation time
of the LV which is retrieved via 'lvs'.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
changes since v1:
- squashed all API commits into one
- moved all new API endpoints into sub endpoints to {osdid}
- {osdid} itself returns the necessary index
- incorporated other code improvements

 PVE/API2/Ceph/OSD.pm | 315 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 315 insertions(+)

diff --git a/PVE/API2/Ceph/OSD.pm b/PVE/API2/Ceph/OSD.pm
index 93433b3a..170c4fd3 100644
--- a/PVE/API2/Ceph/OSD.pm
+++ b/PVE/API2/Ceph/OSD.pm
@@ -5,6 +5,7 @@ use warnings;
 
 use Cwd qw(abs_path);
 use IO::File;
+use JSON;
 use UUID;
 
 use PVE::Ceph::Tools;
@@ -516,6 +517,320 @@ __PACKAGE__->register_method ({
 	return $rpcenv->fork_worker('cephcreateosd', $devs->{dev}->{name},  $authuser, $worker);
     }});
 
+my $OSD_DEV_RETURN_PROPS = {
+    dev_node => {
+	type => 'string',
+	description => 'Device node',
+    },
+    devices => {
+	type => 'string',
+	description => 'Physical disks used',
+    },
+    size => {
+	type => 'integer',
+	description => 'Size in bytes',
+    },
+    support_discard => {
+	type => 'boolean',
+	description => 'Discard support of the physical device',
+    },
+    type => {
+	type => 'string',
+	description => 'Type of device. For example, hdd or ssd',
+    },
+};
+
+__PACKAGE__->register_method ({
+    name => 'osdindex',
+    path => '{osdid}',
+    method => 'GET',
+    permissions => { user => 'all' },
+    description => "OSD index.",
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    node => get_standard_option('pve-node'),
+	    osdid => {
+		description => 'OSD ID',
+		type => 'integer',
+	    },
+	},
+    },
+    returns => {
+	type => 'array',
+	items => {
+	    type => "object",
+	    properties => {},
+	},
+	links => [ { rel => 'child', href => "{name}" } ],
+    },
+    code => sub {
+	my ($param) = @_;
+
+	my $result = [
+	    { name => 'metadata' },
+	    { name => 'lv-info' },
+	];
+
+	return $result;
+    }});
+
+__PACKAGE__->register_method ({
+    name => 'osddetails',
+    path => '{osdid}/metadata',
+    method => 'GET',
+    description => "Get OSD details",
+    proxyto => 'node',
+    protected => 1,
+    permissions => {
+	check => ['perm', '/', [ 'Sys.Audit' ], any => 1],
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    node => get_standard_option('pve-node'),
+	    osdid => {
+		description => 'OSD ID',
+		type => 'integer',
+	    },
+	},
+    },
+    returns => {
+	type => 'object',
+	properties => {
+	    osd => {
+		type => 'object',
+		description => 'General information about the OSD',
+		properties => {
+		    hostname => {
+			type => 'string',
+			description => 'Name of the host containing the OSD.',
+		    },
+		    id => {
+			type => 'integer',
+			description => 'ID of the OSD.',
+		    },
+		    mem_usage => {
+			type => 'integer',
+			description => 'Memory usage of the OSD service.',
+		    },
+		    osd_data => {
+			type => 'string',
+			description => "Path to the OSD's data directory.",
+		    },
+		    osd_objectstore => {
+			type => 'string',
+			description => 'The type of object store used.',
+		    },
+		    pid => {
+			type => 'integer',
+			description => 'OSD process ID.',
+		    },
+		    version => {
+			type => 'string',
+			description => 'Ceph version of the OSD service.',
+		    },
+		    front_addr => {
+			type => 'string',
+			description => 'Address and port used to talk to clients and monitors.',
+		    },
+		    back_addr => {
+			type => 'string',
+			description => 'Address and port used to talk to other OSDs.',
+		    },
+		    hb_front_addr => {
+			type => 'string',
+			description => 'Heartbeat address and port for clients and monitors.',
+		    },
+		    hb_back_addr => {
+			type => 'string',
+			description => 'Heartbeat address and port for other OSDs.',
+		    },
+		},
+	    },
+	    bdev => {
+		type => 'object',
+		description => 'Data about the OSD block device',
+		properties => $OSD_DEV_RETURN_PROPS,
+	    },
+	    db => {
+		type => 'object',
+		description => 'Data about the DB device (optional)',
+		properties => $OSD_DEV_RETURN_PROPS,
+		optional => 1,
+	    },
+	    wal => {
+		type => 'object',
+		description => 'Data about the WAL device (optional)',
+		properties => $OSD_DEV_RETURN_PROPS,
+		optional => 1,
+	    },
+	}
+    },
+    code => sub {
+	my ($param) = @_;
+
+	PVE::Ceph::Tools::check_ceph_inited();
+
+	my $osdid = $param->{osdid};
+	my $rados = PVE::RADOS->new();
+	my $metadata = $rados->mon_command({ prefix => 'osd metadata', id => int($osdid) });
+
+	die "OSD '${osdid}' does not exists on host '${nodename}'\n"
+	    if $nodename ne $metadata->{hostname};
+
+	my $raw = '';
+	my $pid;
+	my $memory;
+	my $parser = sub { $raw .= shift };
+	my $cmd = [
+	    '/bin/systemctl',
+	    'show',
+	    "ceph-osd\@${osdid}.service",
+	    '--property=MainPID,MemoryCurrent'
+	];
+	run_command($cmd, errmsg => 'systemctl show error', outfunc => $parser);
+
+	if ($raw =~ m/^MainPID=([0-9]*)MemoryCurrent=([0-9]*|\[not set\])$/s) { #untaint
+	    $pid = $1;
+	    $memory = $2 eq "[not set]" ? 0 : $2;
+	} else {
+	    die "got unexpected data from systemctl: '${raw}'\n";
+	}
+
+	my $data = {
+	    osd => {
+		hostname => $metadata->{hostname},
+		id => $metadata->{id},
+		mem_usage => int($memory),
+		osd_data => $metadata->{osd_data},
+		osd_objectstore => $metadata->{osd_objectstore},
+		pid => int($pid),
+		version => "$metadata->{ceph_version_short} ($metadata->{ceph_release})",
+		front_addr => $metadata->{front_addr},
+		back_addr => $metadata->{back_addr},
+		hb_front_addr => $metadata->{hb_front_addr},
+		hb_back_addr => $metadata->{hb_back_addr},
+	    },
+	};
+
+	my $get_data = sub {
+	    my ($dev, $prefix) = @_;
+	    $data->{$dev} = {
+		dev_node => $metadata->{"${prefix}_${dev}_dev_node"},
+		devices => $metadata->{"${prefix}_${dev}_devices"},
+		size => int($metadata->{"${prefix}_${dev}_size"}),
+		support_discard => int($metadata->{"${prefix}_${dev}_support_discard"}),
+		type => $metadata->{"${prefix}_${dev}_type"},
+	    };
+	};
+
+	$get_data->("bdev", "bluestore");
+	$get_data->("db", "bluefs") if $metadata->{bluefs_dedicated_db};
+	$get_data->("wal", "bluefs") if $metadata->{bluefs_dedicated_wal};
+
+	return $data;
+    }});
+
+__PACKAGE__->register_method ({
+    name => 'osdvolume',
+    path => '{osdid}/lv-info',
+    method => 'GET',
+    description => "Get OSD volume details",
+    proxyto => 'node',
+    protected => 1,
+    permissions => {
+	check => ['perm', '/', [ 'Sys.Audit' ], any => 1],
+    },
+    parameters => {
+	additionalProperties => 0,
+	properties => {
+	    node => get_standard_option('pve-node'),
+	    osdid => {
+		description => 'OSD ID',
+		type => 'integer',
+	    },
+	    type => {
+		description => 'OSD device type',
+		type => 'string',
+		enum => ['block', 'db', 'wal'],
+		default => 'block',
+		optional => 1,
+	    },
+	},
+    },
+    returns => {
+	type => 'object',
+	properties => {
+	    creation_time => {
+		type => 'string',
+		description => "Creation time as reported by `lvs`.",
+	    },
+	    lv_name => {
+		type => 'string',
+		description => 'Name of the logical volume (LV).',
+	    },
+	    lv_path => {
+		type => 'string',
+		description => 'Path to the logical volume (LV).',
+	    },
+	    lv_size => {
+		type => 'integer',
+		description => 'Size of the logical volume (LV).',
+	    },
+	    lv_uuid => {
+		type => 'string',
+		description => 'UUID of the logical volume (LV).',
+	    },
+	    vg_name => {
+		type => 'string',
+		description => 'Name of the volume group (VG).',
+	    },
+	},
+    },
+    code => sub {
+	my ($param) = @_;
+
+	PVE::Ceph::Tools::check_ceph_inited();
+
+	my $osdid = $param->{osdid};
+	my $type = $param->{type} // 'block';
+
+	my $raw = '';
+	my $parser = sub { $raw .= shift };
+	my $cmd = ['/usr/sbin/ceph-volume', 'lvm', 'list', '--format', 'json'];
+	run_command($cmd, errmsg => 'ceph volume error', outfunc => $parser);
+
+	my $result;
+	if ($raw =~ m/^(\{.*\})$/s) { #untaint
+	    $result = JSON::decode_json($1);
+	} else {
+	    die "got unexpected data from ceph-volume: '${raw}'\n";
+	}
+	die "OSD '${osdid}' not found in 'ceph-volume lvm list' on node '${nodename}'\n"
+	    if !$result->{$osdid};
+
+	my $lv_data = { map { $_->{type} => $_ } @{$result->{$osdid}} };
+	my $volume = $lv_data->{$type} || die "volume type '${type}' not found for OSD ${osdid}\n";
+
+	$raw = '';
+	$cmd = ['/sbin/lvs', $volume->{lv_path}, '--reportformat', 'json', '-o', 'lv_time'];
+	run_command($cmd, errmsg => 'lvs error', outfunc => $parser);
+
+	if ($raw =~ m/(\{.*\})$/s) { #untaint, lvs has whitespace at beginning
+	    $result = JSON::decode_json($1);
+	} else {
+	    die "got unexpected data from lvs: '${raw}'\n";
+	}
+
+	my $data = { map { $_ => $volume->{$_} } qw(lv_name lv_path lv_uuid vg_name) };
+	$data->{lv_size} = int($volume->{lv_size});
+
+	$data->{creation_time} = @{$result->{report}}[0]->{lv}[0]->{lv_time};
+
+	return $data;
+    }});
+
 # Check if $osdid belongs to $nodename
 # $tree ... rados osd tree (passing the tree makes it easy to test)
 sub osd_belongs_to_node {
-- 
2.30.2

[pve-devel] [PATCH manager v2 3/4] ui utils: add renderer for ceph osd addresses

[Aaron Lauterer]

Render the OSD listening addresses a bit nicer and one per line.

Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
---
I tried to keep it as simple as possible to get each address into its
own line for easier reading.

changes since v1: none

 www/manager6/Utils.js | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index 7ca6a271..6499712f 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -1278,6 +1278,18 @@ Ext.define('PVE.Utils', {
 	return Ext.htmlEncode(first + " " + last);
     },
 
+    // expecting the following format:
+    // [v2:10.10.10.1:6802/2008,v1:10.10.10.1:6803/2008]
+    render_ceph_osd_addr: function(value) {
+	value = value.match(/\[(.*)\]/)[1];
+	value = value.replaceAll(',', '\n');
+	let retVal = '';
+	for (const i of value.matchAll(/^(v[0-9]):(.*):([0-9]*)\/([0-9]*)$/gm)) {
+	    retVal += `${i[1]}: ${i[2]}:${i[3]}<br>`;
+	}
+	return retVal;
+    },
+
     windowHostname: function() {
 	return window.location.hostname.replace(Proxmox.Utils.IP6_bracket_match,
             function(m, addr, offset, original) { return addr; });
-- 
2.30.2

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

[Aaron Lauterer]

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);
+		},
+		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);
+		    d.bdev.device = 'block';
+		    let devData = [d.bdev];
+		    if (d.db) {
+			d.db.device = 'db';
+			devData.push(d.db);
+		    }
+		    if (d.wal) {
+			d.wal.device = 'wal';
+			devData.push(d.wal);
+		    }
+		    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;
+
+	    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();
+    },
+});
-- 
2.30.2

Re: [pve-devel] [PATCH widget-toolkit/manager v2 0/4] Ceph OSD: add detail infos

[Aaron Lauterer]

ping?

patches still apply cleanly

On 7/6/22 15:01, Aaron Lauterer wrote:
> This patch series adds 2 new API endpoints for OSDs to fetch more
> detailed information about a single OSD. One for overall information and
> one for a single volume (block, db, wal).
> 
> Because the api endpoint for the volume infos can take a few moments, we
> most likely want to show that loading state. For this, a small patch in
> the widget-toolkit->ObjectGrid is added that makes it possible to enable
> this behavior.
> 
> More in the actual patches.
> 
> Changes since v1:
> squashed API commits into one
> all new API endpoints are below {osdid} and {osdid} returns an index
> incorporate other code improvements
> 
> widget-toolkit: Aaron Lauterer (1):
>    ObjectGrid: optionally show loading mask on load
> 
>   src/grid/ObjectGrid.js | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
> 
> manager: Aaron Lauterer (3):
>    api ceph osd: add OSD index, metadata and lv-info
>    ui utils: add renderer for ceph osd addresses
>    ui: osd: add details window
> 
>   PVE/API2/Ceph/OSD.pm            | 315 ++++++++++++++++++++++++++++++++
>   www/manager6/Makefile           |   1 +
>   www/manager6/Utils.js           |  12 ++
>   www/manager6/ceph/OSD.js        |  26 +++
>   www/manager6/ceph/OSDDetails.js | 289 +++++++++++++++++++++++++++++
>   5 files changed, 643 insertions(+)
>   create mode 100644 www/manager6/ceph/OSDDetails.js
> 

Re: [pve-devel] [PATCH widget-toolkit/manager v2 0/4] Ceph OSD: add detail infos

[Dominik Csapak]

high level looks mostly good, a small question:

is there a special reason why we ignore pre-lvm osds here?
AFAICS, we simply error out for osds that don't live on lvm
(though we can add additional types later i guess)

comments in the individual patches

Re: [pve-devel] [PATCH widget-toolkit v2 1/4] ObjectGrid: optionally show loading mask on load

[Dominik Csapak]

as talked off-list, we can omit the whole patch
by using the rstore.load()'s callback directly

(we already have to go into the internals of the objectgrid there,
so we can use 'rstore.load()' directly too instead of 'reload')

On 7/6/22 15:01, Aaron Lauterer wrote:
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
> 
> changes since v1:
> - added description in the documention at the top
> - renamed paramter to "maskOnLoad" as I couldn't come up with something
> more fitting ;)
> 
>   src/grid/ObjectGrid.js | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/src/grid/ObjectGrid.js b/src/grid/ObjectGrid.js
> index b355d6d..065a009 100644
> --- a/src/grid/ObjectGrid.js
> +++ b/src/grid/ObjectGrid.js
> @@ -39,6 +39,8 @@ disabled:: setting this parameter to true will disable selection and focus on
>     the proxmoxObjectGrid as well as greying out input elements. Useful for a
>     readonly tabular display
>   
> +maskOnLoad: if set to true, will show a "Loading..." while the store is loading data
> +
>   */
>   Ext.define('Proxmox.grid.ObjectGrid', {
>       extend: 'Ext.grid.GridPanel',
> @@ -48,6 +50,8 @@ Ext.define('Proxmox.grid.ObjectGrid', {
>       // see top-level doc-comment above for details/example
>       gridRows: [],
>   
> +    maskOnLoad: false,
> +
>       disabled: false,
>       hideHeaders: true,
>   
> @@ -221,7 +225,10 @@ Ext.define('Proxmox.grid.ObjectGrid', {
>   
>       reload: function() {
>   	let me = this;
> -	me.rstore.load();
> +	if (me.maskOnLoad) {
> +	    me.setLoading();
> +	}
> +	me.rstore.load({ callback: () => me.setLoading(false) });
>       },
>   
>       getObjectValue: function(key, defaultValue) {

Re: [pve-devel] [PATCH manager v2 2/4] api ceph osd: add OSD index, metadata and lv-info

[Dominik Csapak]

LGTM, some comments inline (mostly nitpicks/qestions)

On 7/6/22 15:01, Aaron Lauterer wrote:
> To get more details for a single OSD, we add two new endpoints:
> * nodes/{node}/ceph/osd/{osdid}/metadata
> * nodes/{node}/ceph/osd/{osdid}/lv-info
> 
> The {osdid} endpoint itself gets a new GET handler to return the index.
> 
> The metadata one provides various metadata regarding the OSD.
> 
> Such as
> * process id
> * memory usage
> * info about devices used (bdev/block, db, wal)
>      * size
>      * disks used (sdX)
>      ...
> * network addresses and ports used
> ...
> 
> Memory usage and PID are retrieved from systemd while the rest can be
> retrieved from the metadata provided by Ceph.
> 
> The second one (lv-info) returns the following infos for a logical
> volume:
> * creation time
> * lv name
> * lv path
> * lv size
> * lv uuid
> * vg name
> 
> Possible volumes are:
> * block (default value if not provided)
> * db
> * wal
> 
> 'ceph-volume' is used to gather the infos, except for the creation time
> of the LV which is retrieved via 'lvs'.
> 
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
> changes since v1:
> - squashed all API commits into one
> - moved all new API endpoints into sub endpoints to {osdid}
> - {osdid} itself returns the necessary index
> - incorporated other code improvements
> 
>   PVE/API2/Ceph/OSD.pm | 315 +++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 315 insertions(+)
> 
> diff --git a/PVE/API2/Ceph/OSD.pm b/PVE/API2/Ceph/OSD.pm
> index 93433b3a..170c4fd3 100644
> --- a/PVE/API2/Ceph/OSD.pm
> +++ b/PVE/API2/Ceph/OSD.pm
> @@ -5,6 +5,7 @@ use warnings;
>   
>   use Cwd qw(abs_path);
>   use IO::File;
> +use JSON;
>   use UUID;
>   
>   use PVE::Ceph::Tools;
> @@ -516,6 +517,320 @@ __PACKAGE__->register_method ({
>   	return $rpcenv->fork_worker('cephcreateosd', $devs->{dev}->{name},  $authuser, $worker);
>       }});
>   
> +my $OSD_DEV_RETURN_PROPS = {
> +    dev_node => {
> +	type => 'string',
> +	description => 'Device node',
> +    },
> +    devices => {
> +	type => 'string',
> +	description => 'Physical disks used',
> +    },
> +    size => {
> +	type => 'integer',
> +	description => 'Size in bytes',
> +    },
> +    support_discard => {
> +	type => 'boolean',
> +	description => 'Discard support of the physical device',
> +    },
> +    type => {
> +	type => 'string',
> +	description => 'Type of device. For example, hdd or ssd',
> +    },
> +};
> +
> +__PACKAGE__->register_method ({
> +    name => 'osdindex',
> +    path => '{osdid}',
> +    method => 'GET',
> +    permissions => { user => 'all' },
> +    description => "OSD index.",
> +    parameters => {
> +	additionalProperties => 0,
> +	properties => {
> +	    node => get_standard_option('pve-node'),
> +	    osdid => {
> +		description => 'OSD ID',
> +		type => 'integer',
> +	    },
> +	},
> +    },
> +    returns => {
> +	type => 'array',
> +	items => {
> +	    type => "object",
> +	    properties => {},
> +	},
> +	links => [ { rel => 'child', href => "{name}" } ],
> +    },
> +    code => sub {
> +	my ($param) = @_;
> +
> +	my $result = [
> +	    { name => 'metadata' },
> +	    { name => 'lv-info' },
> +	];
> +
> +	return $result;
> +    }});
> +
> +__PACKAGE__->register_method ({
> +    name => 'osddetails',
> +    path => '{osdid}/metadata',
> +    method => 'GET',
> +    description => "Get OSD details",
> +    proxyto => 'node',
> +    protected => 1,
> +    permissions => {
> +	check => ['perm', '/', [ 'Sys.Audit' ], any => 1],
> +    },
> +    parameters => {
> +	additionalProperties => 0,
> +	properties => {
> +	    node => get_standard_option('pve-node'),
> +	    osdid => {
> +		description => 'OSD ID',
> +		type => 'integer',
> +	    },
> +	},
> +    },
> +    returns => {
> +	type => 'object',
> +	properties => {
> +	    osd => {
> +		type => 'object',
> +		description => 'General information about the OSD',
> +		properties => {
> +		    hostname => {
> +			type => 'string',
> +			description => 'Name of the host containing the OSD.',
> +		    },
> +		    id => {
> +			type => 'integer',
> +			description => 'ID of the OSD.',
> +		    },
> +		    mem_usage => {
> +			type => 'integer',
> +			description => 'Memory usage of the OSD service.',
> +		    },
> +		    osd_data => {
> +			type => 'string',
> +			description => "Path to the OSD's data directory.",
> +		    },
> +		    osd_objectstore => {
> +			type => 'string',
> +			description => 'The type of object store used.',
> +		    },
> +		    pid => {
> +			type => 'integer',
> +			description => 'OSD process ID.',
> +		    },
> +		    version => {
> +			type => 'string',
> +			description => 'Ceph version of the OSD service.',
> +		    },
> +		    front_addr => {
> +			type => 'string',
> +			description => 'Address and port used to talk to clients and monitors.',
> +		    },
> +		    back_addr => {
> +			type => 'string',
> +			description => 'Address and port used to talk to other OSDs.',
> +		    },
> +		    hb_front_addr => {
> +			type => 'string',
> +			description => 'Heartbeat address and port for clients and monitors.',
> +		    },
> +		    hb_back_addr => {
> +			type => 'string',
> +			description => 'Heartbeat address and port for other OSDs.',
> +		    },
> +		},
> +	    },
> +	    bdev => {
> +		type => 'object',
> +		description => 'Data about the OSD block device',
> +		properties => $OSD_DEV_RETURN_PROPS,
> +	    },
> +	    db => {
> +		type => 'object',
> +		description => 'Data about the DB device (optional)',
> +		properties => $OSD_DEV_RETURN_PROPS,
> +		optional => 1,
> +	    },
> +	    wal => {
> +		type => 'object',
> +		description => 'Data about the WAL device (optional)',
> +		properties => $OSD_DEV_RETURN_PROPS,
> +		optional => 1,
> +	    },
> +	}
> +    },
> +    code => sub {
> +	my ($param) = @_;
> +
> +	PVE::Ceph::Tools::check_ceph_inited();
> +
> +	my $osdid = $param->{osdid};
> +	my $rados = PVE::RADOS->new();
> +	my $metadata = $rados->mon_command({ prefix => 'osd metadata', id => int($osdid) });
> +
> +	die "OSD '${osdid}' does not exists on host '${nodename}'\n"
> +	    if $nodename ne $metadata->{hostname};
> +
> +	my $raw = '';
> +	my $pid;
> +	my $memory;
> +	my $parser = sub { $raw .= shift };
> +	my $cmd = [
> +	    '/bin/systemctl',
> +	    'show',
> +	    "ceph-osd\@${osdid}.service",
> +	    '--property=MainPID,MemoryCurrent'
> +	];

nit: personally i'd prefer to have the value of the parameter on it's own
i.e. :

'--property', 'MainPID,MemoryCurrent'

but it's not really a blocker for me


> +	run_command($cmd, errmsg => 'systemctl show error', outfunc => $parser);
> +
> +	if ($raw =~ m/^MainPID=([0-9]*)MemoryCurrent=([0-9]*|\[not set\])$/s) { #untaint
> +	    $pid = $1;
> +	    $memory = $2 eq "[not set]" ? 0 : $2;
> +	} else {
> +	    die "got unexpected data from systemctl: '${raw}'\n";
> +	}

it works, but wouldn't it also be possible to parse those two
things on their own line in the parser? that way you'd not need the intermediate
$raw and would (IMHO) make the code a bit cleaner

> +
> +	my $data = {
> +	    osd => {
> +		hostname => $metadata->{hostname},
> +		id => $metadata->{id},
> +		mem_usage => int($memory),
> +		osd_data => $metadata->{osd_data},
> +		osd_objectstore => $metadata->{osd_objectstore},
> +		pid => int($pid),
> +		version => "$metadata->{ceph_version_short} ($metadata->{ceph_release})",
> +		front_addr => $metadata->{front_addr},
> +		back_addr => $metadata->{back_addr},
> +		hb_front_addr => $metadata->{hb_front_addr},
> +		hb_back_addr => $metadata->{hb_back_addr},
> +	    },
> +	};
> +
> +	my $get_data = sub {
> +	    my ($dev, $prefix) = @_;
> +	    $data->{$dev} = {
> +		dev_node => $metadata->{"${prefix}_${dev}_dev_node"},
> +		devices => $metadata->{"${prefix}_${dev}_devices"},
> +		size => int($metadata->{"${prefix}_${dev}_size"}),
> +		support_discard => int($metadata->{"${prefix}_${dev}_support_discard"}),
> +		type => $metadata->{"${prefix}_${dev}_type"},
> +	    };
> +	};
> +
> +	$get_data->("bdev", "bluestore");
> +	$get_data->("db", "bluefs") if $metadata->{bluefs_dedicated_db};
> +	$get_data->("wal", "bluefs") if $metadata->{bluefs_dedicated_wal};
> +
> +	return $data;
> +    }});
> +
> +__PACKAGE__->register_method ({
> +    name => 'osdvolume',
> +    path => '{osdid}/lv-info',
> +    method => 'GET',
> +    description => "Get OSD volume details",
> +    proxyto => 'node',
> +    protected => 1,
> +    permissions => {
> +	check => ['perm', '/', [ 'Sys.Audit' ], any => 1],
> +    },
> +    parameters => {
> +	additionalProperties => 0,
> +	properties => {
> +	    node => get_standard_option('pve-node'),
> +	    osdid => {
> +		description => 'OSD ID',
> +		type => 'integer',
> +	    },
> +	    type => {
> +		description => 'OSD device type',
> +		type => 'string',
> +		enum => ['block', 'db', 'wal'],
> +		default => 'block',
> +		optional => 1,
> +	    },
> +	},
> +    },
> +    returns => {
> +	type => 'object',
> +	properties => {
> +	    creation_time => {
> +		type => 'string',
> +		description => "Creation time as reported by `lvs`.",
> +	    },
> +	    lv_name => {
> +		type => 'string',
> +		description => 'Name of the logical volume (LV).',
> +	    },
> +	    lv_path => {
> +		type => 'string',
> +		description => 'Path to the logical volume (LV).',
> +	    },
> +	    lv_size => {
> +		type => 'integer',
> +		description => 'Size of the logical volume (LV).',
> +	    },
> +	    lv_uuid => {
> +		type => 'string',
> +		description => 'UUID of the logical volume (LV).',
> +	    },
> +	    vg_name => {
> +		type => 'string',
> +		description => 'Name of the volume group (VG).',
> +	    },
> +	},
> +    },
> +    code => sub {
> +	my ($param) = @_;
> +
> +	PVE::Ceph::Tools::check_ceph_inited();
> +
> +	my $osdid = $param->{osdid};
> +	my $type = $param->{type} // 'block';
> +
> +	my $raw = '';
> +	my $parser = sub { $raw .= shift };
> +	my $cmd = ['/usr/sbin/ceph-volume', 'lvm', 'list', '--format', 'json'];
> +	run_command($cmd, errmsg => 'ceph volume error', outfunc => $parser);
> +
> +	my $result;
> +	if ($raw =~ m/^(\{.*\})$/s) { #untaint
> +	    $result = JSON::decode_json($1);
> +	} else {
> +	    die "got unexpected data from ceph-volume: '${raw}'\n";
> +	}
> +	die "OSD '${osdid}' not found in 'ceph-volume lvm list' on node '${nodename}'\n"
> +	    if !$result->{$osdid};
> +
> +	my $lv_data = { map { $_->{type} => $_ } @{$result->{$osdid}} };
> +	my $volume = $lv_data->{$type} || die "volume type '${type}' not found for OSD ${osdid}\n";
> +
> +	$raw = '';
> +	$cmd = ['/sbin/lvs', $volume->{lv_path}, '--reportformat', 'json', '-o', 'lv_time'];
> +	run_command($cmd, errmsg => 'lvs error', outfunc => $parser);
> +
> +	if ($raw =~ m/(\{.*\})$/s) { #untaint, lvs has whitespace at beginning
> +	    $result = JSON::decode_json($1);
> +	} else {
> +	    die "got unexpected data from lvs: '${raw}'\n";
> +	}
> +
> +	my $data = { map { $_ => $volume->{$_} } qw(lv_name lv_path lv_uuid vg_name) };
> +	$data->{lv_size} = int($volume->{lv_size});
> +
> +	$data->{creation_time} = @{$result->{report}}[0]->{lv}[0]->{lv_time};
> +
> +	return $data;
> +    }});
> +
>   # Check if $osdid belongs to $nodename
>   # $tree ... rados osd tree (passing the tree makes it easy to test)
>   sub osd_belongs_to_node {

Re: [pve-devel] [PATCH manager v2 3/4] ui utils: add renderer for ceph osd addresses

[Dominik Csapak]

overall, i'd like the renderer to be a bit more robust.
a small change in output results in nothing showing at all.

i'd try to parse it as good as possible, but fallback to the 'raw' value
in case it fails. that way the user can at least see what ceph returned

On 7/6/22 15:01, Aaron Lauterer wrote:
> Render the OSD listening addresses a bit nicer and one per line.
> 
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
> I tried to keep it as simple as possible to get each address into its
> own line for easier reading.
> 
> changes since v1: none
> 
>   www/manager6/Utils.js | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
> 
> diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
> index 7ca6a271..6499712f 100644
> --- a/www/manager6/Utils.js
> +++ b/www/manager6/Utils.js
> @@ -1278,6 +1278,18 @@ Ext.define('PVE.Utils', {
>   	return Ext.htmlEncode(first + " " + last);
>       },
>   
> +    // expecting the following format:
> +    // [v2:10.10.10.1:6802/2008,v1:10.10.10.1:6803/2008]
> +    render_ceph_osd_addr: function(value) {
> +	value = value.match(/\[(.*)\]/)[1];
> +	value = value.replaceAll(',', '\n');
> +	let retVal = '';
> +	for (const i of value.matchAll(/^(v[0-9]):(.*):([0-9]*)\/([0-9]*)$/gm)) {
> +	    retVal += `${i[1]}: ${i[2]}:${i[3]}<br>`;
> +	}
> +	return retVal;
> +    },
> +
>       windowHostname: function() {
>   	return window.location.hostname.replace(Proxmox.Utils.IP6_bracket_match,
>               function(m, addr, offset, original) { return addr; });

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

[Dominik Csapak]

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();
> +    },
> +});

Re: [pve-devel] [PATCH widget-toolkit/manager v2 0/4] Ceph OSD: add detail infos

[Aaron Lauterer]

On 10/17/22 16:29, Dominik Csapak wrote:
> high level looks mostly good, a small question:
> 
> is there a special reason why we ignore pre-lvm osds here?
> AFAICS, we simply error out for osds that don't live on lvm
> (though we can add additional types later i guess)

Mainly because with a recent Ceph version, you shouldn't have any pre Bluestore 
OSDs anymore. With Quincy, they have officially been deprecated. And even 
before, for quite a few versions, every new OSD would be a bluestore one.

So I did not want to do the work for the old filestore OSDs. Once a new 
generation of OSDs becomes available, we will need to handle them as well though.

> 
> comments in the individual patches
> 

Re: [pve-devel] [PATCH widget-toolkit/manager v2 0/4] Ceph OSD: add detail infos

[Dominik Csapak]

On 10/18/22 11:14, Aaron Lauterer wrote:
> 
> 
> On 10/17/22 16:29, Dominik Csapak wrote:
>> high level looks mostly good, a small question:
>>
>> is there a special reason why we ignore pre-lvm osds here?
>> AFAICS, we simply error out for osds that don't live on lvm
>> (though we can add additional types later i guess)
> 
> Mainly because with a recent Ceph version, you shouldn't have any pre Bluestore OSDs anymore. With 
> Quincy, they have officially been deprecated. And even before, for quite a few versions, every new 
> OSD would be a bluestore one.
> 
> So I did not want to do the work for the old filestore OSDs. Once a new generation of OSDs becomes 
> available, we will need to handle them as well though.
> 

just to have record about what we talked off-list yesterday:

i didn't mean 'pre-bluestore' osds, but bluestore osds that don't use lvm
iow. created with ceph-disk instead of ceph-volume (from luminous/pve5)

also imho we don't have to show any useful infos there (if we even had some)
but maybe show a little less 'scary' warning like
'cannot show lvm info for osd's created before pve 6/ceph nautilus' or similar