[pve-devel] [RFC manager 2/2] ha/groups: allow editing node priorities through UI

[Dominik Csapak]

comments inline

On 09/22/2016 06:14 PM, Thomas Lamprecht wrote:
> This allows the setting of node priorities in HA groups.
>
> Use a grid panel like in the dc.BackupEdit window with an widget
> column which adds a number field per row to set and edit the nodes
> priority.
>
> Also fixes the bug where the priorities of an existing group (e.g.
> set through the CLI) where deleted when editing said group through
> the GUI.
>
> Signed-off-by: Thomas Lamprecht <t.lamprecht at proxmox.com>
> ---
>
> This is loosely based on the VMID selection of 'PVE.dc.BackupEdit'
> idea/code.
>
> I used initially an nodefield.on('change') event callback to call
> the update_node_selection() function, but as it only one call is
> needed when editing a existing group, I call it just once in the
> overwritten setValue method.
>
>  www/manager6/ha/GroupEdit.js | 166 +++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 153 insertions(+), 13 deletions(-)
>
> diff --git a/www/manager6/ha/GroupEdit.js b/www/manager6/ha/GroupEdit.js
> index 0d9c3c5..b9f9d61 100644
> --- a/www/manager6/ha/GroupEdit.js
> +++ b/www/manager6/ha/GroupEdit.js
> @@ -13,9 +13,159 @@ Ext.define('PVE.ha.GroupInputPanel', {
>  	return values;
>      },
>
> +    setValues: function(values) {
> +	var me = this;
> +
> +	me.callParent([values]);
> +
> +	if (values.nodes) {
> +	    me.update_node_selection(values.nodes)
> +	}
> +    },
> +
>      initComponent : function() {
>  	var me = this;
>
> +	var sm = Ext.create('Ext.selection.CheckboxModel', {
> +	    mode: 'SIMPLE',
> +	    listeners: {
> +		selectionchange: function(model, selected) {
> +		    update_nodefield(selected);
> +		}
> +	    }
> +	});
> +
> +	// use already cached data to avoid an API call
> +	var data = PVE.data.ResourceStore.getNodes();
> +
> +	var store = Ext.create('Ext.data.Store', {
> +	    fields: [ 'node', 'mem', 'cpu', 'prio' ],
> +	    data: data,
> +	    proxy: {
> +		type: 'memory',
> +		reader: {type: 'json'}
> +	    },
> +	    sorters: [
> +		{
> +		    property : 'node',
> +		    direction: 'ASC'
> +		}
> +	    ]
> +	});
> +
> +	nodegrid = Ext.createWidget('grid', {
> +	    store: store,
> +	    border: true,
> +	    height: 300,
> +	    selModel: sm,
> +	    columns: [
> +		{
> +		    header: gettext('Node'),
> +		    flex: 1,
> +		    dataIndex: 'node'
> +		},
> +		{
> +		    header: gettext('Memory usage') + " %",
> +		    renderer: PVE.Utils.render_mem_usage_percent,
> +		    sortable: true,
> +		    width: 150,
> +		    dataIndex: 'mem'
> +		},
> +		{
> +		    header: gettext('CPU usage'),
> +		    renderer: PVE.Utils.render_cpu,
> +		    sortable: true,
> +		    width: 150,
> +		    dataIndex: 'cpu'
> +		},
> +		{
> +		    header: 'Priority',
> +		    xtype: 'widgetcolumn',
> +		    dataIndex: 'prio',
> +		    sortable: true,
> +		    stopSelection: true,
> +		    widget: {
> +			xtype: 'numberfield',
> +			minValue: 0,

i think a sensible maxValue would also be good here
(what does the backend accept as maximum?)

> +			listeners: {
> +			    change: function(numberfield, value, old_value) {
> +				if (value === old_value) {
> +					return;
> +				}
> +				var rowIndex = numberfield.up('gridview').indexOf(numberfield.el.up('table'));
> +				var record = numberfield.up('gridview').getStore().getAt(rowIndex);
> +				if (record) {
> +				    record.set('prio', value);
> +
> +				    update_nodefield(sm.getSelection());
> +				}

instead of using the el.up syntax, you could simply use the
numberfield.getWidgetRecord() method to get the record

> +			    }
> +			}
> +		    }
> +		}
> +	    ]
> +	});
> +
> +	var nodefield = Ext.create('Ext.form.field.Hidden', {
> +	    name: 'nodes',
> +	    value: '',
> +	    isValid: function () {
> +		var value = nodefield.getValue();
> +		return (value && 0 !== value.length);
> +	    }
> +	});

is this necessary?
couldn't we assemble the field in getValues()?

this would eliminate the whole insideUpdate thing?

> +
> +	var insideUpdate = false;
> +
> +	// only call once when editing a group
> +	me.update_node_selection = function(string) {
> +	    if (insideUpdate) { // just to be sure
> +		return;
> +	    }
> +	    insideUpdate = true;
> +
> +	    sm.deselectAll(true);
> +
> +	    string.split(',').forEach(function (e, idx, array) {
> +		res = e.split(':');
> +
> +		store.each(function(record) {
> +		    node = record.get('node');
> +
> +		    if (node == res[0]) {
> +			sm.select(record, true);
> +			   record.set('prio', res[1]);
> +			   record.commit();
> +		    }
> +		});
> +	    });
> +	    nodegrid.reconfigure(store);
> +
> +	    insideUpdate = false;
> +	};
> +
> +	var update_nodefield = function(selected) {
> +	    if (!insideUpdate) { // avoid endless loop
> +		var sel = '';
> +		first_iter = true;
> +		Ext.Array.each(selected, function(record) {
> +		    if (!first_iter) {
> +			sel += ',';
> +		    }
> +		    first_iter = false;
> +
> +		    sel += record.data.node;
> +		    if (record.data.prio) {
> +			sel += ':' + record.data.prio;
> +		    }
> +		});
> +
> +		insideUpdate = true;
> +		nodefield.setValue(sel);
> +		insideUpdate = false;
> +	    }
> +	}
> +
>  	me.column1 = [
>  	    {
>  		xtype: me.create ? 'textfield' : 'displayfield',
> @@ -25,14 +175,7 @@ Ext.define('PVE.ha.GroupInputPanel', {
>  		vtype: 'StorageId',
>  		allowBlank: false
>  	    },
> -	    {
> -		xtype: 'pveNodeSelector',
> -		name: 'nodes',
> -		fieldLabel: gettext('Nodes'),
> -		allowBlank: false,
> -		multiSelect: true,
> -		autoSelect: false
> -	    }
> +	    nodefield
>  	];
>
>  	me.column2 = [
> @@ -55,7 +198,8 @@ Ext.define('PVE.ha.GroupInputPanel', {
>  		xtype: 'textfield',
>  		name: 'comment',
>  		fieldLabel: gettext('Comment')
> -	    }
> +	    },
> +	    nodegrid
>  	];
>  	
>  	me.callParent();
> @@ -97,10 +241,6 @@ Ext.define('PVE.ha.GroupEdit', {
>  		success:  function(response, options) {
>  		    var values = response.result.data;
>
> -		    if (values.nodes) {
> -			values.nodes = values.nodes.split(',');
> -		    }
> -
>  		    ipanel.setValues(values);
>  		}
>  	    });
>