[pve-devel] [PATCH v2 manager 2/2] gui/cluster: add structured peerLinks to join info

[Dominik Csapak]

some nits inline

On 3/17/20 12:11 PM, Stefan Reiter wrote:
> Instead of the old 'ring_addr' property (which is kept for
> compatibility), we also encode the link numbers into the new peerLinks
> structure. This allows us to display which IP is assigned to which link
> on the cluster in the join dialog, helping a user identify which link
> should receive which interface on the new node.
> 
> Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
> ---
>   www/manager6/dc/Cluster.js          | 13 +++++++++----
>   www/manager6/dc/ClusterEdit.js      | 10 ++++++++--
>   www/manager6/dc/CorosyncLinkEdit.js | 18 ++++++++++++++++--
>   3 files changed, 33 insertions(+), 8 deletions(-)
> 
> diff --git a/www/manager6/dc/Cluster.js b/www/manager6/dc/Cluster.js
> index 963da098..2407ab15 100644
> --- a/www/manager6/dc/Cluster.js
> +++ b/www/manager6/dc/Cluster.js
> @@ -85,14 +85,18 @@ Ext.define('PVE.ClusterAdministration', {
>   			return el.name === data.preferred_node;
>   		    });
>   
> -		    var links = [];
> -		    PVE.Utils.forEachCorosyncLink(nodeinfo,
> -			(num, link) => links.push(link));
> +		    var links = {};
> +		    var ring_addr = [];
> +		    PVE.Utils.forEachCorosyncLink(nodeinfo, (num, link) => {
> +			links[num] = link;
> +			ring_addr.push(link);
> +		    });
>   
>   		    vm.set('preferred_node', {
>   			name: data.preferred_node,
>   			addr: nodeinfo.pve_addr,
> -			ring_addr: links,
> +			peerLinks: links,
> +			ring_addr: ring_addr,
>   			fp: nodeinfo.pve_fp
>   		    });
>   		},
> @@ -116,6 +120,7 @@ Ext.define('PVE.ClusterAdministration', {
>   			joinInfo: {
>   			    ipAddress: vm.get('preferred_node.addr'),
>   			    fingerprint: vm.get('preferred_node.fp'),
> +			    peerLinks: vm.get('preferred_node.peerLinks'),
>   			    ring_addr: vm.get('preferred_node.ring_addr'),
>   			    totem: vm.get('totem')
>   			}
> diff --git a/www/manager6/dc/ClusterEdit.js b/www/manager6/dc/ClusterEdit.js
> index 0820e4a0..4dd977c6 100644
> --- a/www/manager6/dc/ClusterEdit.js
> +++ b/www/manager6/dc/ClusterEdit.js
> @@ -222,10 +222,16 @@ Ext.define('PVE.ClusterJoinNodeWindow', {
>   		var links = [];
>   		var interfaces = joinInfo.totem['interface'];
>   		Ext.Array.each(Object.keys(interfaces), iface => {
> +		    var linkNumber = interfaces[iface]['linknumber'];
> +		    var peerLink;
> +		    if (joinInfo.peerLinks) {
> +			peerLink = joinInfo.peerLinks[linkNumber];

i did not mention this in the last patch, but here its very obvious,
please use either style for object access:

foo['bar'] or foo.bar

i would tend to the latter, since we more often use it already,
but i am ok with both if its consistent

especially for things like
interfaces[iface]['linknumber'], it is not immediatly obvious what
is a variable and what is 'static' (especially so
since you use the result in a variable named linknumber
and use that again as property access :S)

> +		    }
> +
>   		    links.push({
> -			number: interfaces[iface]['linknumber'],
> +			number: linkNumber,
>   			value: '',
> -			text: '',
> +			text: peerLink ? Ext.String.format(gettext("peer's link address: {0}"), peerLink) : '',
>   			allowBlank: false
>   		    });
>   		});
> diff --git a/www/manager6/dc/CorosyncLinkEdit.js b/www/manager6/dc/CorosyncLinkEdit.js
> index 6fb35a0e..c1492453 100644
> --- a/www/manager6/dc/CorosyncLinkEdit.js
> +++ b/www/manager6/dc/CorosyncLinkEdit.js
> @@ -14,7 +14,7 @@ Ext.define('PVE.form.CorosyncLinkEditorController', {
>   	this.addLink();
>       },
>   
> -    addLink: function(number, value, allowBlank) {
> +    addLink: function(number, value, allowBlank, text) {
>   	let me = this;
>   	let view = me.getView();
>   	let vm = view.getViewModel();
> @@ -37,6 +37,7 @@ Ext.define('PVE.form.CorosyncLinkEditorController', {
>   	    allowBlankNetwork: allowBlank,
>   	    initNumber: number,
>   	    initNetwork: value,
> +	    text: text,
>   
>   	    // needs to be set here, because we need to update the viewmodel
>   	    removeBtnHandler: function() {
> @@ -130,6 +131,7 @@ Ext.define('PVE.form.CorosyncLinkSelector', {
>       // values
>       initNumber: 0,
>       initNetwork: '',
> +    text: '',

weird indentation

>   
>       layout: 'hbox',
>       bodyPadding: 5,
> @@ -190,6 +192,17 @@ Ext.define('PVE.form.CorosyncLinkSelector', {
>   		    parent.removeBtnHandler();
>   		}
>   	    }
> +	},
> +	{
> +	    xtype: 'label',
> +	    margin: '-1px 0 0 5px',
> +
> +	    // for muted effect
> +	    cls: 'x-form-item-label-default',
> +
> +	    cbind: {
> +		text: '{text}'
> +	    }
>   	}
>       ],
>   
> @@ -327,7 +340,8 @@ Ext.define('PVE.form.CorosyncLinkEditor', {
>   	vm.set('linkCount', 0);
>   
>   	Ext.Array.each(links, link => {
> -	    controller.addLink(link['number'], link['value'], link['allowBlank']);
> +	    controller.addLink(link['number'], link['value'],
> +		link['allowBlank'], link['text']);
>   	});
>       },
>   
>