generally looks good but i have one high level comment/question
(and some nits inline)

one thing i'd like to see here is to retain the info what is configured,
so previously the info was either 'dhcp'/'auto' (slaac) or an ip address

now we only show the ip adress

what i mean is something like

'x.y.z.w (dhcp)'

or

'xx00::1 (static)'

etc. so one can still see what mode is configured

On 12/2/24 11:46, Gabriel Goller wrote:
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.nun...@proxmox.com>
Signed-off-by: Gabriel Goller <g.gol...@proxmox.com>
---
  www/manager6/lxc/Network.js | 57 +++++++++++++++++++++++++++----------
  1 file changed, 42 insertions(+), 15 deletions(-)

diff --git a/www/manager6/lxc/Network.js b/www/manager6/lxc/Network.js
index b2cd94109485..41de72f43646 100644
--- a/www/manager6/lxc/Network.js
+++ b/www/manager6/lxc/Network.js
@@ -356,25 +356,52 @@ 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.hwaddr] = iface;
+                       }
+
+                       let result = Ext.decode(confResponse.responseText);

i know it's pre-existing, but when touching the code we could directly use
confResponse.result.data, no? AFAICS this is the already decoded info from there
(no clue why that wasn't used before though...)

+                       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;
+
+                               let iface;
+                               if ((iface = 
interfaces[net.hwaddr.toLowerCase()])) {
+                                   net.name = iface.name;
+                                   net.ip = iface.inet;
+                                   net.ip6 = iface.inet6;
+                               }

this reads a bit odd with the if condition
i'd rather use something like

let iface = interfaces[net....];
if (iface) {
...
}

this should do the same, but is much easier to read

+
+                               records.push(net);
+                           }
+                       }
+
+                       me.store.loadData(records);
+                       
me.down('button[name=addButton]').setDisabled(records.length >= 32);
+                   },
+               });
            },
        });
      },



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

Reply via email to