From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 2189E1FF161 for ; Wed, 4 Dec 2024 10:53:00 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 3A97D4559; Wed, 4 Dec 2024 10:52:57 +0100 (CET) Date: Wed, 4 Dec 2024 10:52:23 +0100 From: Gabriel Goller To: Dominik Csapak Message-ID: <5kqws66tg5bi2efn4mtbya25avhs6gkvypd3xukdorme57hiav@cub27f5gv4os> References: <20241202104626.166056-1-g.goller@proxmox.com> <20241202104626.166056-2-g.goller@proxmox.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20241002-35-39f9a6 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.188 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment POISEN_SPAM_PILL 0.1 Meta: its spam POISEN_SPAM_PILL_1 0.1 random spam to be learned in bayes POISEN_SPAM_PILL_3 0.1 random spam to be learned in bayes RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [net.name, net.id, result.data] Subject: Re: [pve-devel] [PATCH manager 1/2] lxc: show dynamically assigned IPs in network tab X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Cc: Proxmox VE development discussion Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" On 04.12.2024 10:17, Dominik Csapak wrote: >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 This is a nice idea, but it could be a bit tricky. To get the ip info we execute `ip a` in the container's netns, but for some reason I can't see the 'dynamic' option which is usually shown on a dynamically acquired address. I could use `ip route` and check if the route was inserted by 'dhcp' or 'kernel', but no idea how foolproof this is... >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 >>Signed-off-by: Gabriel Goller >>--- >> 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...) Ack. >>+ 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 I agree. Thanks for the review! _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel