[pve-devel] [PATCH manager] gui: lxc/Network: add a virtual 'none' option

[Thomas Lamprecht]

Am 2/5/20 um 9:37 AM schrieb Dominik Csapak:
> sometimes, if users do not want a ipv4/6 address, they set the network mode
> to 'dhcp' instead of 'static' (with no ip set), in believe
> this will result in no ip
> 
> in reality, often no ipv6 dhcp server exists in the environment, and
> the container start stalls for ~5min trying to get a ipv6
> 
> to improve ux, add the option 'none', which functionally is the same
> as having selected 'static' with no ip set, and simultaniously
> make the ip required
> 
> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
> ---
> this panel is a good candidate for a rewrite using viemodel/controller,
> but this change is small and the gains of a rewrite not big enough,
> so i left it as is for now, but if someone wants to get some extjs experience,
> this could be a good exercise
> 

The basic idea is OK, but do not make none the new default! CTs normally do
want a network, our  main use case is system CTs after all, no network no fun

>  www/manager6/lxc/Network.js | 51 ++++++++++++++++++++++++++-----------
>  1 file changed, 36 insertions(+), 15 deletions(-)
> 
> diff --git a/www/manager6/lxc/Network.js b/www/manager6/lxc/Network.js
> index be40a8fa..4afcd09b 100644
> --- a/www/manager6/lxc/Network.js
> +++ b/www/manager6/lxc/Network.js
> @@ -36,10 +36,10 @@ Ext.define('PVE.lxc.NetworkInputPanel', {
>  
>  	var newdata = {};
>  
> -	if (values.ipv6mode !== 'static') {
> +	if (values.ipv6mode !== 'static' && values.ipv6mode !== 'none') {
>  	    values.ip6 = values.ipv6mode;
>  	}
> -	if (values.ipv4mode !== 'static') {
> +	if (values.ipv4mode !== 'static' && values.ipv4mode !== 'none') {
>  	    values.ip = values.ipv4mode;
>  	}
>  	newdata[id] = PVE.Parser.printLxcNetwork(values);
> @@ -161,6 +161,7 @@ Ext.define('PVE.lxc.NetworkInputPanel', {
>  	    cdata.ip = '';
>  	    cdata.gw = '';
>  	}
> +	var none4 = (!cdata.ip && !cdata.gw && !dhcp4);
>  
>  	var auto6 = (cdata.ip6 === 'auto');
>  	var dhcp6 = (cdata.ip6 === 'dhcp');
> @@ -168,26 +169,36 @@ Ext.define('PVE.lxc.NetworkInputPanel', {
>  	    cdata.ip6 = '';
>  	    cdata.gw6 = '';
>  	}
> +
> +	var none6 = (!cdata.ip6 && !cdata.gw6 && !auto6 && !dhcp6);

something more telling could be nice, we do not need to exten the ip4/ip6
namescheme to all related variables, especially if they're not API params
(and thus fixed). Maybe:

noIPv6addr
noIPv4addr 

>  	
>  	me.column2 = [
> +	    {
> +		xtype: 'label',
> +		text: 'IPv4:', // do not localize
> +	    },
>  	    {
>  		layout: {
>  		    type: 'hbox',
>  		    align: 'middle'
>  		},
>  		border: false,
> -		margin: '0 0 5 0',
> +		margin: '5 0 5 0',
>  		items: [
>  		    {
> -			xtype: 'label',
> -			text: 'IPv4:' // do not localize
useless comment
> +			xtype: 'radiofield',
> +			boxLabel: Proxmox.Utils.noneText,
> +			name: 'ipv4mode',
> +			inputValue: 'none',
> +			checked: none4,
> +			margin: '0 0 0 10',
>  		    },
>  		    {
>  			xtype: 'radiofield',
>  			boxLabel: gettext('Static'),
>  			name: 'ipv4mode',
>  			inputValue: 'static',
> -			checked: !dhcp4,
> +			checked: !(dhcp4 || none4),
>  			margin: '0 0 0 10',
>  			listeners: {
>  			    change: function(cb, value) {
> @@ -211,7 +222,8 @@ Ext.define('PVE.lxc.NetworkInputPanel', {
>  		name: 'ip',
>  		vtype: 'IPCIDRAddress',
>  		value: cdata.ip,
> -		disabled: dhcp4,
> +		allowBlank: false,
> +		disabled: dhcp4 || none4,
>  		fieldLabel: 'IPv4/CIDR' // do not localize
>  	    },
>  	    {
> @@ -219,14 +231,18 @@ Ext.define('PVE.lxc.NetworkInputPanel', {
>  		name: 'gw',
>  		value: cdata.gw,
>  		vtype: 'IPAddress',
> -		disabled: dhcp4,
> +		disabled: dhcp4 || none4,
>  		fieldLabel: gettext('Gateway') + ' (IPv4)',
>  		margin: '0 0 3 0' // override bottom margin to account for the menuseparator
>  	    },
>  	    {
>  		xtype: 'menuseparator',
>  		height: '3',
> -		margin: '0'
> +		margin: '0 0 5 0',
> +	    },
> +	    {
> +		xtype: 'label',
> +		text: 'IPv6:' // do not localize
>  	    },
>  	    {
>  		layout: {
> @@ -234,18 +250,22 @@ Ext.define('PVE.lxc.NetworkInputPanel', {
>  		    align: 'middle'
>  		},
>  		border: false,
> -		margin: '0 0 5 0',
> +		margin: '5 0 5 0',
>  		items: [
>  		    {
> -			xtype: 'label',
> -			text: 'IPv6:' // do not localize
nit: useless comment
> +			xtype: 'radiofield',
> +			boxLabel: Proxmox.Utils.noneText,

noneText is not capitalized, stands out in a ugly way.

> +			name: 'ipv6mode',
> +			inputValue: 'none',
> +			checked: none6,
> +			margin: '0 0 0 10',
>  		    },
>  		    {
>  			xtype: 'radiofield',
>  			boxLabel: gettext('Static'),
>  			name: 'ipv6mode',
>  			inputValue: 'static',
> -			checked: !(auto6 || dhcp6),
> +			checked: !(auto6 || dhcp6 || none6),
>  			margin: '0 0 0 10',
>  			listeners: {
>  			    change: function(cb, value) {
> @@ -277,7 +297,8 @@ Ext.define('PVE.lxc.NetworkInputPanel', {
>  		name: 'ip6',
>  		value: cdata.ip6,
>  		vtype: 'IP6CIDRAddress',
> -		disabled: (dhcp6 || auto6),
> +		allowBlank: false,
> +		disabled: (dhcp6 || auto6 || none6),
>  		fieldLabel: 'IPv6/CIDR' // do not localize
>  	    },
>  	    {
> @@ -285,7 +306,7 @@ Ext.define('PVE.lxc.NetworkInputPanel', {
>  		name: 'gw6',
>  		vtype: 'IP6Address',
>  		value: cdata.gw6,
> -		disabled: (dhcp6 || auto6),
> +		disabled: (dhcp6 || auto6 || none6),
>  		fieldLabel: gettext('Gateway') + ' (IPv6)'
>  	    }
>  	];
>