From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <o.bektas@proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits))
 (No client certificate requested)
 by lists.proxmox.com (Postfix) with ESMTPS id 5C0B4698A1
 for <pve-devel@lists.proxmox.com>; Tue, 27 Jul 2021 16:18:55 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
 by firstgate.proxmox.com (Proxmox) with ESMTP id 467691AAF3
 for <pve-devel@lists.proxmox.com>; Tue, 27 Jul 2021 16:18:25 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com
 [94.136.29.106])
 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
 key-exchange X25519 server-signature RSA-PSS (2048 bits))
 (No client certificate requested)
 by firstgate.proxmox.com (Proxmox) with ESMTPS id 7F0D91AAE6
 for <pve-devel@lists.proxmox.com>; Tue, 27 Jul 2021 16:18:23 +0200 (CEST)
Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1])
 by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 4A07B416D1
 for <pve-devel@lists.proxmox.com>; Tue, 27 Jul 2021 16:18:23 +0200 (CEST)
Date: Tue, 27 Jul 2021 16:17:54 +0200
From: Oguz Bektas <o.bektas@proxmox.com>
To: Thomas Lamprecht <t.lamprecht@proxmox.com>
Cc: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Message-ID: <YQAVku4oDXjsF9UG@gaia>
Mail-Followup-To: Oguz Bektas <o.bektas@proxmox.com>,
 Thomas Lamprecht <t.lamprecht@proxmox.com>,
 Proxmox VE development discussion <pve-devel@lists.proxmox.com>
References: <20210727133709.1311314-1-o.bektas@proxmox.com>
 <166e8460-05ff-5791-6103-7e2a5fc796d4@proxmox.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <166e8460-05ff-5791-6103-7e2a5fc796d4@proxmox.com>
X-SPAM-LEVEL: Spam detection results:  0
 AWL 1.364 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 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. [lxc.pm]
Subject: Re: [pve-devel] [PATCH container] vmstatus: include detected IP
 address of running containers
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>, 
 <mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
X-List-Received-Date: Tue, 27 Jul 2021 14:18:55 -0000

On Tue, Jul 27, 2021 at 04:03:17PM +0200, Thomas Lamprecht wrote:
> On 27.07.21 15:37, Oguz Bektas wrote:
> > add a helper 'find_lxc_ip_address' to fetch IP address of container from
> > its network namespace using lxc-info.
> > 
> > for the moment it can be queried with the pct tool:
> > $ pct status 1000 --verbose
> > cpu: 0
> > cpus: 1
> > disk: 6422528
> > diskread: 368640
> > diskwrite: 0
> > ipaddress: 192.168.31.83        <----
> > maxdisk: 4294967296
> > maxmem: 536870912
> > maxswap: 536870912
> > mem: 864256
> > name: CT1000
> > netin: 3281265
> > netout: 15794
> > pid: 34897
> > status: running
> > swap: 94208
> > type: lxc
> > uptime: 11088
> > vmid: 1000
> > 
> 
> 
> And why use `lxc-info`, this is called very often and we know about setups with
> 1500+ CTs on a single host, so it'd be good to check if adding 1000s forks every
> status call could be avoided.

lxc-info already queries this information from netlink, otherwise switching
network namespace and communicating to netlink in perl would be hacky (at least
that's the impression i got from looking at the relevant lxc code [0][1]).

[0]: https://github.com/lxc/lxc/blob/f1c64634c40a7218165538b89aca320fa258b3c1/src/lxc/lxccontainer.c#L2235
[1]: https://github.com/lxc/lxc/blob/f1c64634c40a7218165538b89aca320fa258b3c1/src/lxc/lxccontainer.c#L2429

fabian also mentioned we can use 'iproute2':
$ lxc-info --pid 1000
PID:            34897
$ ip netns attach 1000 34897
$ ip -n 1000 a show
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN group default qlen 1000
    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
    inet 127.0.0.1/8 scope host lo
       valid_lft forever preferred_lft forever
    inet6 ::1/128 scope host
       valid_lft forever preferred_lft forever
2: eth0@if84: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default qlen 1000
    link/ether fa:2e:e1:b6:a4:b0 brd ff:ff:ff:ff:ff:ff link-netns 1000
    inet 192.168.31.83/20 scope global eth0
       valid_lft forever preferred_lft forever
    inet6 fe80::f82e:e1ff:feb6:a4b0/64 scope link
       valid_lft forever preferred_lft forever

but that leaves the namespace file in /run/netns/ which requires extra cleanup
and handling when container is rebooting or shutting down, so this way seemed
like it would be cleaner.

for avoiding the forks in bigger setups it might make sense to call it once and
cache the result until there's a network hotplug or similar, or only poll every N minutes.

> what about multiple, you only print the last match which is quite confusing...
> You forgot to add this to the API/CLI return schema in $PVE::LXC::vmstatus_return_properties,
> which may highly probably actually want to be an array...

okay will do that in v2 with an array to account for multiple addresses



> 
> > Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
> > ---
> >  src/PVE/LXC.pm | 16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> > 
> > diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
> > index 139f901..e7804e0 100644
> > --- a/src/PVE/LXC.pm
> > +++ b/src/PVE/LXC.pm
> > @@ -247,6 +247,8 @@ sub vmstatus {
> >  
> >  	next if !$pid; # skip stopped CTs
> >  
> > +	$d->{ipaddress} = find_lxc_ip_address($vmid);
> 
> 
> 
> > +
> >  	my $proc_pid_stat = PVE::ProcFSTools::read_proc_pid_stat($pid);
> >  	$d->{uptime} = int(($uptime - $proc_pid_stat->{starttime}) / $clock_ticks); # the method lxcfs uses
> >  
> > @@ -397,6 +399,20 @@ sub open_ppid {
> >      return ($ppid, $fd);
> >  }
> >  
> > +sub find_lxc_ip_address {
> > +    my ($vmid) = @_;
> > +
> > +    my $ip = undef;
> 
> as said above, needs to be an array..
> 
> > +
> > +    my $parser = sub {
> > +	my $line = shift;
> > +	$ip = $1 if $line =~ m/^IP:\s+(.*)$/;
> > +    };
> > +
> > +    PVE::Tools::run_command(['lxc-info', '-n', $vmid, '--ips'], outfunc => $parser, errfunc => sub {});
> > +    return $ip;
> > +}
> > +
> >  # Note: we cannot use Net:IP, because that only allows strict
> >  # CIDR networks
> >  sub parse_ipv4_cidr {
> > 
>