[pve-devel] [PATCH manager 1/2] gui/ceph: show versions in osd overview

[Dominik Csapak]

On 5/31/19 11:28 AM, Thomas Lamprecht wrote:
> Am 5/29/19 um 3:33 PM schrieb Dominik Csapak:
>> and highlight the not current osds
>>
>> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
>> ---
>>   PVE/API2/Ceph/OSD.pm     |  4 ++++
>>   www/manager6/ceph/OSD.js | 34 +++++++++++++++++++++++++++++++---
>>   2 files changed, 35 insertions(+), 3 deletions(-)
>>
>> diff --git a/PVE/API2/Ceph/OSD.pm b/PVE/API2/Ceph/OSD.pm
>> index 5f781e27..adb0025c 100644
>> --- a/PVE/API2/Ceph/OSD.pm
>> +++ b/PVE/API2/Ceph/OSD.pm
>> @@ -139,6 +139,9 @@ __PACKAGE__->register_method ({
>>   		} else {
>>   		    $new->{osdtype} = 'filestore';
>>   		}
>> +		for my $field (qw(ceph_version ceph_version_short)) {
>> +		    $new->{$field} = $osdmd->{$field} if $osdmd->{$field};
>> +		}
>>   	    }
>>   
>>   	    $newnodes->{$e->{id}} = $new;
>> @@ -175,6 +178,7 @@ __PACKAGE__->register_method ({
>>   
>>   	# we want this for the noout flag
>>   	$data->{flags} = $flags if $flags;
>> +	$data->{versions} = PVE::Cluster::get_node_kv("ceph-version");
>>   
>>   	return $data;
>>       }});
>> diff --git a/www/manager6/ceph/OSD.js b/www/manager6/ceph/OSD.js
>> index cb3d5f0d..bf4034aa 100644
>> --- a/www/manager6/ceph/OSD.js
>> +++ b/www/manager6/ceph/OSD.js
>> @@ -177,6 +177,22 @@ Ext.define('PVE.node.CephOsdTree', {
>>   	    width: 80
>>   	},
>>   	{
>> +	    text: gettext('Version'),
>> +	    dataIndex: 'version',
>> +	    renderer: function(value, metadata, rec) {
>> +		var me = this;
>> +		if (value < me.maxversion) {
> 
> Yeah, nice try, but it isn't as easy as that, e.g. 14.2.10 is newer than
> 14.2.9, but my Firefox JS console tells me different:
> # print("14.2.10" > "14.2.9")
> "false"
> 
> And, AFAIS, the versions are not 0 padded to a "00.0.00" format, even then
> on a release with many patch bugfix releases reaching versions > 100 isn't
> completely unthinkable..
> 
> I _really_ do not want to have false positives here, so let's to this
> in a bit more defined and understandable way, e.g., split on ".", convert
> to integer and to the comparisons on that level until you found a value
> difference (or one has a additional version level which the other hasn't,
> as for this to happen in checking all previous version must have been equal
> we know that the one with more level is newer (there are no negative versions,
> maybe with SUSE but not here ;P)
> 
> Another way would be split, then to multiply add, each level with 10^x,
> where 'x' decreases per level (starts with max count of version levels)
> then simple integer comparisons can be made, not sure what's nicer to do
> "performance" is here completely irrelevant, to a reasonable level ;)

yeah, here it would be easy to do 'version != maxversion' (as long as 
maxversion is really the highest version)

> 
>> +		    return PVE.Utils.get_ceph_icon_html('HEALTH_OLD') + value;
>> +		}
>> +
>> +		if (!value && rec.data.type == 'host') {
>> +		    return '<i class="fa faded fa-cube"></i> ' + me.versions[rec.data.name];
>> +		}
>> +
>> +		return value;
>> +	    }
>> +	},
>> +	{
>>   	    text: 'weight',
>>   	    dataIndex: 'crush_weight',
>>   	    align: 'right',
>> @@ -265,6 +281,7 @@ Ext.define('PVE.node.CephOsdTree', {
>>   
>>   	// we expect noout to be not set by default
>>   	var noout = false;
>> +	me.maxversion = "00.0.00";
>>   
>>   	var nodename = me.pveSelNode.data.node;
>>   	if (!nodename) {
>> @@ -291,16 +308,24 @@ Ext.define('PVE.node.CephOsdTree', {
>>   		    );
>>   		},
>>   		success: function(response, opts) {
>> +		    var data = response.result.data;
>>   		    sm.deselectAll();
>> -		    me.setRootNode(response.result.data.root);
>> +		    me.setRootNode(data.root);
>>   		    me.expandAll();
>>   		    // extract noout flag
>> -		    if (response.result.data.flags &&
>> -			response.result.data.flags.search(/noout/) !== -1) {
>> +		    if (data.flags && data.flags.search(/noout/) !== -1) {
>>   			noout = true;
>>   		    } else {
>>   			noout = false;
>>   		    }
>> +
>> +		    me.versions = data.versions;
>> +		    // extract max version
>> +		    Object.values(data.versions || {}).forEach(function(version) {
>> +			if (version > me.maxversion) {
> 
> same here.

you are right ofc, this is the same logic as in 'services' and 
'statusdetail'

will send an extra patch that fixes that there and a v2 of this

> 
>> +			    me.maxversion = version;
>> +			}
>> +		    });
>>   		    set_button_status();
>>   		}
>>   	    });
>> @@ -484,6 +509,9 @@ Ext.define('PVE.node.CephOsdTree', {
>>   		     { type: 'string', name: 'blfsdev' },
>>   		     { type: 'string', name: 'dbdev' },
>>   		     { type: 'string', name: 'waldev' },
>> +		     { type: 'string', name: 'version', calculate: function(data) {
>> +			 return PVE.Utils.parse_ceph_version(data);
>> +		     } },
>>   		     { type: 'string', name: 'iconCls', calculate: function(data) {
>>   			 var iconCls = 'fa x-fa-tree fa-';
>>   			 switch (data.type) {
>>
>