[pve-devel] [PATCH container/manager v2 0/3] Show container ip in summary and network tab

[pve-devel] [PATCH container/manager v2 0/3] Show container ip in summary and network tab

[Gabriel Goller]

Show the ip/hwaddress of the network interfaces of containers in the summary
tab of the container and in the network tab on a per-interface basis.

This series was originally by Leo Nunner: 
https://lore.proxmox.com/pve-devel/20230615094333.66179-1-l.nunner@proxmox.com/

v2, thanks @Dominik:
 - show if ip is static or dynamic (dhcp) 
 - show multiple ips per interface
 - refactor/reuse AgentIPView instead of adding ContainerIPView component
 - various other small improvements

manager:

Gabriel Goller (2):
  lxc: show dynamically assigned IPs in network tab
  guest: refactor and reuse AgentIPView for containers

 www/manager6/Makefile                         |   2 +-
 www/manager6/lxc/Network.js                   |  85 +++++++----
 www/manager6/panel/GuestStatusView.js         | 139 +++++++++++++++++-
 www/manager6/panel/GuestSummary.js            |   2 +-
 .../{qemu/AgentIPView.js => panel/IPView.js}  |  78 ++--------
 5 files changed, 213 insertions(+), 93 deletions(-)
 rename www/manager6/{qemu/AgentIPView.js => panel/IPView.js} (59%)


container:

Gabriel Goller (1):
  api: return all addresses of an interface

 src/PVE/API2/LXC.pm | 35 +++++++++++++++++++++++++----------
 src/PVE/LXC.pm      |  7 +++++--
 2 files changed, 30 insertions(+), 12 deletions(-)


Summary over all repositories:
  7 files changed, 243 insertions(+), 105 deletions(-)

-- 
Generated by git-murpp 0.7.1


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH manager v2 1/3] lxc: show dynamically assigned IPs in network tab

[Gabriel Goller]

adds a call to /nodes/{node}/lxc/{vmid}/interfaces and merges the
returned data with the existing configuration. This will update the
IPv4 and IPv6 address, as well as the interface name (in case the
container changed it).

Originally-by: Leo Nunner <l.nunner@proxmox.com>
Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 www/manager6/lxc/Network.js | 85 ++++++++++++++++++++++++++-----------
 1 file changed, 60 insertions(+), 25 deletions(-)

diff --git a/www/manager6/lxc/Network.js b/www/manager6/lxc/Network.js
index b2cd94109485..d50274739400 100644
--- a/www/manager6/lxc/Network.js
+++ b/www/manager6/lxc/Network.js
@@ -356,25 +356,66 @@ Ext.define('PVE.lxc.NetworkView', {
 
 	Proxmox.Utils.setErrorMask(me, true);
 
+	let nodename = me.pveSelNode.data.node;
+	let vmid = me.pveSelNode.data.vmid;
+
 	Proxmox.Utils.API2Request({
-	    url: me.url,
+	    url: `/nodes/${nodename}/lxc/${vmid}/interfaces`,
+	    method: 'GET',
 	    failure: function(response, opts) {
 		Proxmox.Utils.setErrorMask(me, gettext('Error') + ': ' + response.htmlStatus);
 	    },
-	    success: function(response, opts) {
-		Proxmox.Utils.setErrorMask(me, false);
-		let result = Ext.decode(response.responseText);
-		me.dataCache = result.data || {};
-		let records = [];
-		for (const [key, value] of Object.entries(me.dataCache)) {
-		    if (key.match(/^net\d+/)) {
-			let net = PVE.Parser.parseLxcNetwork(value);
-			net.id = key;
-			records.push(net);
-		    }
-		}
-		me.store.loadData(records);
-		me.down('button[name=addButton]').setDisabled(records.length >= 32);
+	    success: function(ifResponse, ifOpts) {
+		Proxmox.Utils.API2Request({
+		    url: me.url,
+		    failure: function(response, opts) {
+			Proxmox.Utils.setErrorMask(me, gettext('Error') + ': ' + response.htmlStatus);
+		    },
+		    success: function(confResponse, confOpts) {
+			Proxmox.Utils.setErrorMask(me, false);
+
+			let interfaces = [];
+			for (const [, iface] of Object.entries(ifResponse?.result?.data || {})) {
+			    interfaces[iface['hardware-address']] = iface;
+			}
+
+			me.dataCache = confResponse.result.data || {};
+			let records = [];
+			for (const [key, value] of Object.entries(me.dataCache)) {
+			    if (key.match(/^net\d+/)) {
+				let config = PVE.Parser.parseLxcNetwork(value);
+				let net = structuredClone(config);
+				net.id = key;
+
+				let iface = interfaces[config.hwaddr.toLowerCase()];
+				if (iface) {
+				    net.name = iface.name;
+				    net.ip = [];
+				    for (const i of iface['ip-addresses']) {
+					let ip_with_prefix = `${i['ip-address']}/${i.prefix}`;
+					if (i['ip-address-type'] === "inet") {
+					    if (config.ip === ip_with_prefix) {
+						net.ip.push(`${ip_with_prefix} (static)`);
+					    } else {
+						net.ip.push(`${ip_with_prefix} (dhcp)`);
+					    }
+					} else if (i['ip-address-type'] === "inet6") {
+					    if (config.ip6 === ip_with_prefix) {
+						net.ip.push(`${ip_with_prefix} (static)`);
+					    } else {
+						net.ip.push(`${ip_with_prefix} (dhcp)`);
+					    }
+					}
+				    }
+				}
+				records.push(net);
+			    }
+			}
+
+			me.store.loadData(records);
+			me.down('button[name=addButton]').setDisabled(records.length >= 32);
+		    },
+		});
 	    },
 	});
     },
@@ -504,7 +545,7 @@ Ext.define('PVE.lxc.NetworkView', {
 		},
 		{
 		    header: gettext('VLAN Tag'),
-		    width: 80,
+		    width: 70,
 		    dataIndex: 'tag',
 		},
 		{
@@ -514,16 +555,10 @@ Ext.define('PVE.lxc.NetworkView', {
 		},
 		{
 		    header: gettext('IP address'),
-		    width: 150,
+		    width: 200,
 		    dataIndex: 'ip',
-		    renderer: function(value, metaData, rec) {
-			if (rec.data.ip && rec.data.ip6) {
-			    return rec.data.ip + "<br>" + rec.data.ip6;
-			} else if (rec.data.ip6) {
-			    return rec.data.ip6;
-			} else {
-			    return rec.data.ip;
-			}
+		    renderer: function(_value, _metaData, rec) {
+			return rec.data.ip.join("<br>");
 		    },
 		},
 		{
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH manager v2 2/3] guest: refactor and reuse AgentIPView for containers

[Gabriel Goller]

Refactor AgentIPView to be generic over container and vms. Reuse it in
the Container Summary to show the ip addresses of the container.

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 www/manager6/Makefile                         |   2 +-
 www/manager6/panel/GuestStatusView.js         | 139 +++++++++++++++++-
 www/manager6/panel/GuestSummary.js            |   2 +-
 .../{qemu/AgentIPView.js => panel/IPView.js}  |  78 ++--------
 4 files changed, 153 insertions(+), 68 deletions(-)
 rename www/manager6/{qemu/AgentIPView.js => panel/IPView.js} (59%)

diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index c94a5cdfbf70..aa10eb420c5b 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -102,6 +102,7 @@ JSSRC= 							\
 	panel/BackupJobPrune.js				\
 	panel/HealthWidget.js				\
 	panel/IPSet.js					\
+	panel/IPView.js					\
 	panel/RunningChart.js				\
 	panel/StatusPanel.js				\
 	panel/GuestStatusView.js			\
@@ -231,7 +232,6 @@ JSSRC= 							\
 	pool/Config.js					\
 	pool/StatusView.js				\
 	pool/Summary.js					\
-	qemu/AgentIPView.js				\
 	qemu/AudioEdit.js				\
 	qemu/BootOrderEdit.js				\
 	qemu/CDEdit.js					\
diff --git a/www/manager6/panel/GuestStatusView.js b/www/manager6/panel/GuestStatusView.js
index 6401811c73bb..c9fb7badc4f1 100644
--- a/www/manager6/panel/GuestStatusView.js
+++ b/www/manager6/panel/GuestStatusView.js
@@ -146,14 +146,149 @@ Ext.define('PVE.panel.GuestStatusView', {
 	    height: 15,
 	},
 	{
-	    itemId: 'ips',
-	    xtype: 'pveAgentIPView',
+	    itemId: 'agentIPs',
+	    xtype: 'pveIPView',
 	    cbind: {
 		rstore: '{rstore}',
 		pveSelNode: '{pveSelNode}',
 		hidden: '{isLxc}',
 		disabled: '{isLxc}',
 	    },
+	    createUpdateStoreCallback: function(ipview, nodename, vmid) {
+		ipview.ipStore = Ext.create('Proxmox.data.UpdateStore', {
+		    interval: 10000,
+		    storeid: 'pve-qemu-agent-' + vmid,
+		    method: 'POST',
+		    proxy: {
+			type: 'proxmox',
+			url: '/api2/json/nodes/' + nodename + '/qemu/' + vmid + '/agent/network-get-interfaces',
+		    },
+		});
+		ipview.callParent();
+
+		ipview.mon(ipview.ipStore, 'load', function(_store, records, success) {
+		    if (records && records.length) {
+			ipview.nics = records[0].data.result;
+		    } else {
+			ipview.nics = undefined;
+		    }
+		    ipview.updateStatus(!success);
+		});
+	    },
+	    updateStatusCallback: function(ipview, unsuccessful, defaulttext) {
+		var text = defaulttext || gettext('No network information');
+		var more = false;
+		if (unsuccessful) {
+		    text = gettext('Guest Agent not running');
+		} else if (ipview.agent && ipview.running) {
+		    if (Ext.isArray(ipview.nics) && ipview.nics.length) {
+			more = true;
+			var ips = ipview.getDefaultIps(ipview.nics);
+			if (ips.length !== 0) {
+			    text = ips.join('<br>');
+			}
+		    } else if (ipview.nics && ipview.nics.error) {
+			text = Ext.String.format(text, ipview.nics.error.desc);
+		    }
+		} else if (ipview.agent) {
+		    text = gettext('Guest Agent not running');
+		} else {
+		    text = gettext('No Guest Agent configured');
+		}
+
+		var ipBox = ipview.down('#ipBox');
+		ipBox.update(text);
+
+		var moreBtn = ipview.down('#moreBtn');
+		moreBtn.setVisible(more);
+	    },
+	    startIPStoreCallback: function(ipview, store) {
+		let agentRec = store.getById('agent');
+		let state = store.getById('status');
+
+		ipview.agent = agentRec && agentRec.data.value === 1;
+		ipview.running = state && state.data.value === 'running';
+
+		var caps = Ext.state.Manager.get('GuiCap');
+
+		if (!caps.vms['VM.Monitor']) {
+		    var errorText = gettext("Requires '{0}' Privileges");
+		    ipview.updateStatus(false, Ext.String.format(errorText, 'VM.Monitor'));
+		    return;
+		}
+
+		if (ipview.agent && ipview.running && ipview.ipStore.isStopped) {
+		    ipview.ipStore.startUpdate();
+		} else if (ipview.ipStore.isStopped) {
+		    ipview.updateStatus();
+		}
+	    },
+	},
+	{
+	    itemId: 'ctIPS',
+	    xtype: 'pveIPView',
+	    cbind: {
+		rstore: '{rstore}',
+		pveSelNode: '{pveSelNode}',
+		hidden: '{!isLxc}',
+		disabled: '{!isLxc}',
+	    },
+	    createUpdateStoreCallback: function(ipview, nodename, vmid) {
+		ipview.ipStore = Ext.create('Proxmox.data.UpdateStore', {
+		    interval: 10000,
+		    storeid: 'lxc-interfaces-' + vmid,
+		    method: 'GET',
+		    proxy: {
+			type: 'proxmox',
+			url: '/api2/json/nodes/' + nodename + '/lxc/' + vmid + '/interfaces',
+		    },
+		});
+		ipview.callParent();
+
+		ipview.mon(ipview.ipStore, 'load', function(_store, records, success) {
+		    if (records && records.length) {
+			ipview.nics = records.map(r => r.data);
+		    } else {
+			ipview.nics = undefined;
+		    }
+		    ipview.updateStatus(!success);
+		});
+	    },
+	    updateStatusCallback: function(ipview, _unsuccessful, defaulttext) {
+		var text = defaulttext || gettext('No network information');
+		var more = false;
+		if (Ext.isArray(ipview.nics) && ipview.nics.length) {
+		    more = true;
+		    var ips = ipview.getDefaultIps(ipview.nics);
+		    if (ips.length !== 0) {
+			text = ips.join('<br>');
+		    }
+		}
+		var ipBox = ipview.down('#ipBox');
+		ipBox.update(text);
+
+		var moreBtn = ipview.down('#moreBtn');
+		moreBtn.setVisible(more);
+	    },
+	    startIPStoreCallback: function(ipview, store) {
+		let state = store.getById('status');
+
+		ipview.running = state && state.data.value === 'running';
+
+		var caps = Ext.state.Manager.get('GuiCap');
+
+		if (!caps.vms['VM.Audit']) {
+		    var errorText = gettext("Requires '{0}' Privileges");
+		    ipview.updateStatus(false, Ext.String.format(errorText, 'VM.Audit'));
+		    return;
+		}
+
+		if (ipview.running && ipview.ipStore.isStopped) {
+		    ipview.ipStore.startUpdate();
+		} else if (ipview.ipStore.isStopped) {
+		    ipview.updateStatus();
+		}
+	    },
 	},
     ],
 
diff --git a/www/manager6/panel/GuestSummary.js b/www/manager6/panel/GuestSummary.js
index 1565db3f658d..2186967f62da 100644
--- a/www/manager6/panel/GuestSummary.js
+++ b/www/manager6/panel/GuestSummary.js
@@ -54,7 +54,7 @@ Ext.define('PVE.guest.Summary', {
 	    items = [
 		{
 		    xtype: 'container',
-		    height: 300,
+		    height: 370,
 		    layout: {
 			type: 'hbox',
 			align: 'stretch',
diff --git a/www/manager6/qemu/AgentIPView.js b/www/manager6/panel/IPView.js
similarity index 59%
rename from www/manager6/qemu/AgentIPView.js
rename to www/manager6/panel/IPView.js
index 829e55960b23..ba58b372a0e4 100644
--- a/www/manager6/qemu/AgentIPView.js
+++ b/www/manager6/panel/IPView.js
@@ -1,7 +1,7 @@
 Ext.define('PVE.window.IPInfo', {
     extend: 'Ext.window.Window',
     width: 600,
-    title: gettext('Guest Agent Network Information'),
+    title: gettext('Network Information'),
     height: 300,
     layout: {
 	type: 'fit',
@@ -51,9 +51,9 @@ Ext.define('PVE.window.IPInfo', {
     ],
 });
 
-Ext.define('PVE.qemu.AgentIPView', {
+Ext.define('PVE.IPView', {
     extend: 'Ext.container.Container',
-    xtype: 'pveAgentIPView',
+    xtype: 'pveIPView',
 
     layout: {
 	type: 'hbox',
@@ -61,6 +61,9 @@ Ext.define('PVE.qemu.AgentIPView', {
     },
 
     nics: [],
+    startIPStoreCallback: undefined,
+    updateStatusCallback: undefined,
+    createUpdateStoreCallback: undefined,
 
     items: [
 	{
@@ -90,7 +93,7 @@ Ext.define('PVE.qemu.AgentIPView', {
 		    hidden: true,
 		    ui: 'default-toolbar',
 		    handler: function(btn) {
-			let view = this.up('pveAgentIPView');
+			let view = this.up('pveIPView');
 
 			var win = Ext.create('PVE.window.IPInfo');
 			win.down('grid').getStore().setData(view.nics);
@@ -125,53 +128,19 @@ Ext.define('PVE.qemu.AgentIPView', {
 
     startIPStore: function(store, records, success) {
 	var me = this;
-	let agentRec = store.getById('agent');
-	let state = store.getById('status');
 
-	me.agent = agentRec && agentRec.data.value === 1;
-	me.running = state && state.data.value === 'running';
-
-	var caps = Ext.state.Manager.get('GuiCap');
-
-	if (!caps.vms['VM.Monitor']) {
-	    var errorText = gettext("Requires '{0}' Privileges");
-	    me.updateStatus(false, Ext.String.format(errorText, 'VM.Monitor'));
-	    return;
-	}
-
-	if (me.agent && me.running && me.ipStore.isStopped) {
-	    me.ipStore.startUpdate();
-	} else if (me.ipStore.isStopped) {
-	    me.updateStatus();
-	}
+	me.startIPStoreCallback(me, store);
     },
 
     updateStatus: function(unsuccessful, defaulttext) {
 	var me = this;
-	var text = defaulttext || gettext('No network information');
-	var more = false;
-	if (unsuccessful) {
-	    text = gettext('Guest Agent not running');
-	} else if (me.agent && me.running) {
-	    if (Ext.isArray(me.nics) && me.nics.length) {
-		more = true;
-		var ips = me.getDefaultIps(me.nics);
-		if (ips.length !== 0) {
-		    text = ips.join('<br>');
-		}
-	    } else if (me.nics && me.nics.error) {
-		text = Ext.String.format(text, me.nics.error.desc);
-	    }
-	} else if (me.agent) {
-	    text = gettext('Guest Agent not running');
-	} else {
-	    text = gettext('No Guest Agent configured');
-	}
 
-	var ipBox = me.down('#ipBox');
+	me.updateStatusCallback(me, unsuccessful, defaulttext);
+
+	var ipBox = ipview.down('#ipBox');
 	ipBox.update(text);
 
-	var moreBtn = me.down('#moreBtn');
+	var moreBtn = ipview.down('#moreBtn');
 	moreBtn.setVisible(more);
     },
 
@@ -189,26 +158,7 @@ Ext.define('PVE.qemu.AgentIPView', {
 	var nodename = me.pveSelNode.data.node;
 	var vmid = me.pveSelNode.data.vmid;
 
-	me.ipStore = Ext.create('Proxmox.data.UpdateStore', {
-	    interval: 10000,
-	    storeid: 'pve-qemu-agent-' + vmid,
-	    method: 'POST',
-	    proxy: {
-		type: 'proxmox',
-		url: '/api2/json/nodes/' + nodename + '/qemu/' + vmid + '/agent/network-get-interfaces',
-	    },
-	});
-
-	me.callParent();
-
-	me.mon(me.ipStore, 'load', function(store, records, success) {
-	    if (records && records.length) {
-		me.nics = records[0].data.result;
-	    } else {
-		me.nics = undefined;
-	    }
-	    me.updateStatus(!success);
-	});
+	me.createUpdateStoreCallback(me, nodename, vmid);
 
 	me.on('destroy', me.ipStore.stopUpdate, me.ipStore);
 
@@ -217,7 +167,7 @@ Ext.define('PVE.qemu.AgentIPView', {
 	    me.startIPStore(me.rstore, me.rstore.getData(), false);
 	}
 
-	// check if the guest agent is there on every statusstore load
 	me.mon(me.rstore, 'load', me.startIPStore, me);
     },
 });
+
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH container v2 3/3] api: return all addresses of an interface

[Gabriel Goller]

Return all ip-addresses of an interface, not only the first one. Change
return schema to resemble the 'agent/network-get-interfaces' qemu call.
This helps us making the AgentIPView more generic and display the ip on
both containers and vms.

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 src/PVE/API2/LXC.pm | 35 +++++++++++++++++++++++++----------
 src/PVE/LXC.pm      |  7 +++++--
 2 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
index 213e518a7b62..b14e88a7baf2 100644
--- a/src/PVE/API2/LXC.pm
+++ b/src/PVE/API2/LXC.pm
@@ -2546,20 +2546,35 @@ __PACKAGE__->register_method({
 		    description => 'The name of the interface',
 		    optional => 0,
 		},
-		hwaddr => {
+		"hardware-address" => {
 		    type => 'string',
 		    description => 'The MAC address of the interface',
 		    optional => 0,
 		},
-		inet => {
-		    type => 'string',
-		    description => 'The IPv4 address of the interface',
-		    optional => 1,
-		},
-		inet6 => {
-		    type => 'string',
-		    description => 'The IPv6 address of the interface',
-		    optional => 1,
+		"ip-addresses" => {
+		    type => 'array',
+		    description => 'All the Addresses of the interface',
+		    optional => 0,
+		    items => {
+			type => 'object',
+			properties => {
+			    prefix => {
+				type => 'integer',
+				description => 'IP-Prefix',
+				optional => 1,
+			    },
+			    "ip-address" => {
+				type => 'string',
+				description => 'IP-Address',
+				optional => 1,
+			    },
+			    "ip-address-type" => {
+				type => 'string',
+				description => 'IP-Family',
+				optional => 1,
+			    },
+			}
+		    }
 		},
 	    }
 	},
diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index e78e36576fc3..60d92d69abcc 100644
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -1141,10 +1141,13 @@ sub get_interfaces {
     my $res;
     for my $interface ($config->@*) {
 	my $obj = { name => $interface->{ifname} };
+	my @list = ();
 	for my $ip ($interface->{addr_info}->@*) {
-	    $obj->{$ip->{family}} = $ip->{local} . "/" . $ip->{prefixlen};
+	    my $ip = {'ip-address-type' => $ip->{family}, 'ip-address' => $ip->{local}, 'prefix' => $ip->{prefixlen}};
+	    push(@list, $ip);
 	}
-	$obj->{hwaddr} = $interface->{address};
+	$obj->{'ip-addresses'} = \@list;
+	$obj->{'hardware-address'} = $interface->{address};
 	push @$res, $obj
     }
 
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH container v2 3/3] api: return all addresses of an interface

[Thomas Lamprecht]

Am 10.12.24 um 16:11 schrieb Gabriel Goller:
> Return all ip-addresses of an interface, not only the first one. Change
> return schema to resemble the 'agent/network-get-interfaces' qemu call.
> This helps us making the AgentIPView more generic and display the ip on
> both containers and vms.
> 
> Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
> ---
>  src/PVE/API2/LXC.pm | 35 +++++++++++++++++++++++++----------
>  src/PVE/LXC.pm      |  7 +++++--
>  2 files changed, 30 insertions(+), 12 deletions(-)
> 
> diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
> index 213e518a7b62..b14e88a7baf2 100644
> --- a/src/PVE/API2/LXC.pm
> +++ b/src/PVE/API2/LXC.pm
> @@ -2546,20 +2546,35 @@ __PACKAGE__->register_method({
>  		    description => 'The name of the interface',
>  		    optional => 0,
>  		},
> -		hwaddr => {
> +		"hardware-address" => {

Can we keep the old properties for backward compatibility and add a TODO comment
stating that they should be removed for the next major release?

Otherwise, this breaks any tooling, automation, UI, ... that is using this API
endpoint.

>  		    type => 'string',
>  		    description => 'The MAC address of the interface',
>  		    optional => 0,
>  		},
> -		inet => {
> -		    type => 'string',
> -		    description => 'The IPv4 address of the interface',
> -		    optional => 1,
> -		},
> -		inet6 => {
> -		    type => 'string',
> -		    description => 'The IPv6 address of the interface',
> -		    optional => 1,
> +		"ip-addresses" => {
> +		    type => 'array',
> +		    description => 'All the Addresses of the interface',
> +		    optional => 0,
> +		    items => {
> +			type => 'object',
> +			properties => {
> +			    prefix => {
> +				type => 'integer',
> +				description => 'IP-Prefix',
> +				optional => 1,
> +			    },
> +			    "ip-address" => {
> +				type => 'string',
> +				description => 'IP-Address',
> +				optional => 1,
> +			    },
> +			    "ip-address-type" => {

This is already inside a 'ip-addresses' property, so I think it would be fine to
drop prefix here. E.g., just 'ip' or just 'address' for the one above and just
'type' here. No hard feelings though, just seems a bit redundant here to me.

> +				type => 'string',
> +				description => 'IP-Family',
> +				optional => 1,
> +			    },
> +			}
> +		    }
>  		},
>  	    }
>  	},
> diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
> index e78e36576fc3..60d92d69abcc 100644
> --- a/src/PVE/LXC.pm
> +++ b/src/PVE/LXC.pm
> @@ -1141,10 +1141,13 @@ sub get_interfaces {
>      my $res;
>      for my $interface ($config->@*) {
>  	my $obj = { name => $interface->{ifname} };
> +	my @list = ();

You can directly use an array reference here, i.e.:

my $list = [];

>  	for my $ip ($interface->{addr_info}->@*) {
> -	    $obj->{$ip->{family}} = $ip->{local} . "/" . $ip->{prefixlen};
> +	    my $ip = {'ip-address-type' => $ip->{family}, 'ip-address' => $ip->{local}, 'prefix' => $ip->{prefixlen}};
> +	    push(@list, $ip);

FWIW, this can also be pushed directly, e.g., something like:

push(@$list, {
    type => $ip->{family},
    address => $ip->{local},
    prefix => $ip->{prefixlen},
});

>  	}
> -	$obj->{hwaddr} = $interface->{address};
> +	$obj->{'ip-addresses'} = \@list;
> +	$obj->{'hardware-address'} = $interface->{address};
>  	push @$res, $obj
>      }
>  



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH container v2 3/3] api: return all addresses of an interface

[Gabriel Goller]

On 10.12.2024 18:58, Thomas Lamprecht wrote:
>Am 10.12.24 um 16:11 schrieb Gabriel Goller:
>> Return all ip-addresses of an interface, not only the first one. Change
>> return schema to resemble the 'agent/network-get-interfaces' qemu call.
>> This helps us making the AgentIPView more generic and display the ip on
>> both containers and vms.
>>
>> Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
>> ---
>>  src/PVE/API2/LXC.pm | 35 +++++++++++++++++++++++++----------
>>  src/PVE/LXC.pm      |  7 +++++--
>>  2 files changed, 30 insertions(+), 12 deletions(-)
>>
>> diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
>> index 213e518a7b62..b14e88a7baf2 100644
>> --- a/src/PVE/API2/LXC.pm
>> +++ b/src/PVE/API2/LXC.pm
>> @@ -2546,20 +2546,35 @@ __PACKAGE__->register_method({
>>  		    description => 'The name of the interface',
>>  		    optional => 0,
>>  		},
>> -		hwaddr => {
>> +		"hardware-address" => {
>
>Can we keep the old properties for backward compatibility and add a TODO comment
>stating that they should be removed for the next major release?
>
>Otherwise, this breaks any tooling, automation, UI, ... that is using this API
>endpoint.

True, this endpoint is already merged and present in the latest version.
Fixed it, thanks!

>>  		    type => 'string',
>>  		    description => 'The MAC address of the interface',
>>  		    optional => 0,
>>  		},
>> -		inet => {
>> -		    type => 'string',
>> -		    description => 'The IPv4 address of the interface',
>> -		    optional => 1,
>> -		},
>> -		inet6 => {
>> -		    type => 'string',
>> -		    description => 'The IPv6 address of the interface',
>> -		    optional => 1,
>> +		"ip-addresses" => {
>> +		    type => 'array',
>> +		    description => 'All the Addresses of the interface',
>> +		    optional => 0,
>> +		    items => {
>> +			type => 'object',
>> +			properties => {
>> +			    prefix => {
>> +				type => 'integer',
>> +				description => 'IP-Prefix',
>> +				optional => 1,
>> +			    },
>> +			    "ip-address" => {
>> +				type => 'string',
>> +				description => 'IP-Address',
>> +				optional => 1,
>> +			    },
>> +			    "ip-address-type" => {
>
>This is already inside a 'ip-addresses' property, so I think it would be fine to
>drop prefix here. E.g., just 'ip' or just 'address' for the one above and just
>'type' here. No hard feelings though, just seems a bit redundant here to me.

I agree, but I wanted the schema to be the same as the qemu agent
`network-get-interfaces` response so that we can have a common frontend
component for vm's and container.

>> +				type => 'string',
>> +				description => 'IP-Family',
>> +				optional => 1,
>> +			    },
>> +			}
>> +		    }
>>  		},
>>  	    }
>>  	},
>> diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
>> index e78e36576fc3..60d92d69abcc 100644
>> --- a/src/PVE/LXC.pm
>> +++ b/src/PVE/LXC.pm
>> @@ -1141,10 +1141,13 @@ sub get_interfaces {
>>      my $res;
>>      for my $interface ($config->@*) {
>>  	my $obj = { name => $interface->{ifname} };
>> +	my @list = ();
>
>You can directly use an array reference here, i.e.:
>
>my $list = [];
>
>>  	for my $ip ($interface->{addr_info}->@*) {
>> -	    $obj->{$ip->{family}} = $ip->{local} . "/" . $ip->{prefixlen};
>> +	    my $ip = {'ip-address-type' => $ip->{family}, 'ip-address' => $ip->{local}, 'prefix' => $ip->{prefixlen}};
>> +	    push(@list, $ip);
>
>FWIW, this can also be pushed directly, e.g., something like:
>
>push(@$list, {
>    type => $ip->{family},
>    address => $ip->{local},
>    prefix => $ip->{prefixlen},
>});
>

Fixed this.

Thanks for the review!


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH container v2 3/3] api: return all addresses of an interface

[Maximiliano Sandoval]

Gabriel Goller <g.goller@proxmox.com> writes:

> +		"ip-addresses" => {
> +		    type => 'array',
> +		    description => 'All the Addresses of the interface',
> +		    optional => 0,
> +		    items => {
> +			type => 'object',

I feel 'The addresses of the interface' sounds a bit better. But no
strong opinion.

Is it intentional that `Addresses` is capitalized? If this is Header
Capitalization then the word "interface" should be capitalized too.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH container v2 3/3] api: return all addresses of an interface

[Gabriel Goller]

On 11.12.2024 15:57, Maximiliano Sandoval wrote:
>
>Gabriel Goller <g.goller@proxmox.com> writes:
>
>> +		"ip-addresses" => {
>> +		    type => 'array',
>> +		    description => 'All the Addresses of the interface',
>> +		    optional => 0,
>> +		    items => {
>> +			type => 'object',
>
>I feel 'The addresses of the interface' sounds a bit better. But no
>strong opinion.
>
>Is it intentional that `Addresses` is capitalized? If this is Header
>Capitalization then the word "interface" should be capitalized too.

I agree, thanks for the heads up!


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH container/manager v2 0/3] Show container ip in summary and network tab

[Fabian Grünbichler]

there is both a bugzilla entry for this, and a patch as well:

https://lore.proxmox.com/pve-devel/mailman.710.1713473387.450.pve-devel@lists.proxmox.com/

granted, both happened after Leo's initial patch below, but please still
coordinate/communicate with Johannes as well ;)

On December 10, 2024 4:11 pm, Gabriel Goller wrote:
> Show the ip/hwaddress of the network interfaces of containers in the summary
> tab of the container and in the network tab on a per-interface basis.
> 
> This series was originally by Leo Nunner: 
> https://lore.proxmox.com/pve-devel/20230615094333.66179-1-l.nunner@proxmox.com/
> 
> v2, thanks @Dominik:
>  - show if ip is static or dynamic (dhcp) 
>  - show multiple ips per interface
>  - refactor/reuse AgentIPView instead of adding ContainerIPView component
>  - various other small improvements
> 
> manager:
> 
> Gabriel Goller (2):
>   lxc: show dynamically assigned IPs in network tab
>   guest: refactor and reuse AgentIPView for containers
> 
>  www/manager6/Makefile                         |   2 +-
>  www/manager6/lxc/Network.js                   |  85 +++++++----
>  www/manager6/panel/GuestStatusView.js         | 139 +++++++++++++++++-
>  www/manager6/panel/GuestSummary.js            |   2 +-
>  .../{qemu/AgentIPView.js => panel/IPView.js}  |  78 ++--------
>  5 files changed, 213 insertions(+), 93 deletions(-)
>  rename www/manager6/{qemu/AgentIPView.js => panel/IPView.js} (59%)
> 
> 
> container:
> 
> Gabriel Goller (1):
>   api: return all addresses of an interface
> 
>  src/PVE/API2/LXC.pm | 35 +++++++++++++++++++++++++----------
>  src/PVE/LXC.pm      |  7 +++++--
>  2 files changed, 30 insertions(+), 12 deletions(-)
> 
> 
> Summary over all repositories:
>   7 files changed, 243 insertions(+), 105 deletions(-)
> 
> -- 
> Generated by git-murpp 0.7.1
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH container/manager v2 0/3] Show container ip in summary and network tab

[Gabriel Goller]

Submitted a new version!


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH container/manager v2 0/3] Show container ip in summary and network tab

[Gabriel Goller]

Show the ip/hwaddress of the network interfaces of containers in the summary
tab of the container and in the network tab on a per-interface basis.

This series was originally by Leo Nunner: 
https://lore.proxmox.com/pve-devel/20230615094333.66179-1-l.nunner@proxmox.com/

v2, thanks @Dominik:
 - show if ip is static or dynamic (dhcp) 
 - show multiple ips per interface
 - refactor/reuse AgentIPView instead of adding ContainerIPView component
 - various other small improvements

manager:

Gabriel Goller (2):
  lxc: show dynamically assigned IPs in network tab
  guest: refactor and reuse AgentIPView for containers

 www/manager6/Makefile                         |   2 +-
 www/manager6/lxc/Network.js                   |  85 +++++++----
 www/manager6/panel/GuestStatusView.js         | 139 +++++++++++++++++-
 www/manager6/panel/GuestSummary.js            |   2 +-
 .../{qemu/AgentIPView.js => panel/IPView.js}  |  78 ++--------
 5 files changed, 213 insertions(+), 93 deletions(-)
 rename www/manager6/{qemu/AgentIPView.js => panel/IPView.js} (59%)


container:

Gabriel Goller (1):
  api: return all addresses of an interface

 src/PVE/API2/LXC.pm | 35 +++++++++++++++++++++++++----------
 src/PVE/LXC.pm      |  7 +++++--
 2 files changed, 30 insertions(+), 12 deletions(-)


Summary over all repositories:
  7 files changed, 243 insertions(+), 105 deletions(-)

-- 
Generated by git-murpp 0.7.1


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH container/manager v2 0/3] Show container ip in summary and network tab

[Gabriel Goller]

Ignore this one please, forgot the --suppress-cc again :(
Sorry for the noise.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel