* [pve-devel] [PATCH container] vmstatus: include detected IP address of running containers @ 2021-07-27 13:37 Oguz Bektas 2021-07-27 14:03 ` Thomas Lamprecht 0 siblings, 1 reply; 4+ messages in thread From: Oguz Bektas @ 2021-07-27 13:37 UTC (permalink / raw) To: pve-devel 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 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; + + 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 { -- 2.30.2 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [pve-devel] [PATCH container] vmstatus: include detected IP address of running containers 2021-07-27 13:37 [pve-devel] [PATCH container] vmstatus: include detected IP address of running containers Oguz Bektas @ 2021-07-27 14:03 ` Thomas Lamprecht 2021-07-27 14:17 ` Oguz Bektas 0 siblings, 1 reply; 4+ messages in thread From: Thomas Lamprecht @ 2021-07-27 14:03 UTC (permalink / raw) To: Proxmox VE development discussion, Oguz Bektas 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 > what about multiple, you only print the last match which is quite confusing... 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. > 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); 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... > + > 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 { > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [pve-devel] [PATCH container] vmstatus: include detected IP address of running containers 2021-07-27 14:03 ` Thomas Lamprecht @ 2021-07-27 14:17 ` Oguz Bektas 2021-07-27 15:02 ` Thomas Lamprecht 0 siblings, 1 reply; 4+ messages in thread From: Oguz Bektas @ 2021-07-27 14:17 UTC (permalink / raw) To: Thomas Lamprecht; +Cc: Proxmox VE development discussion 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 { > > > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [pve-devel] [PATCH container] vmstatus: include detected IP address of running containers 2021-07-27 14:17 ` Oguz Bektas @ 2021-07-27 15:02 ` Thomas Lamprecht 0 siblings, 0 replies; 4+ messages in thread From: Thomas Lamprecht @ 2021-07-27 15:02 UTC (permalink / raw) To: Oguz Bektas, Proxmox VE development discussion On 27.07.21 16:17, Oguz Bektas wrote: > 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]). Does not have to be hacky if done nicely and we can do that in rust and use only the bindings in perl.. Mira already did quite some work for the contrack stuff, that could possibly be re-used/shaped into something for that use case. But we cannot really do away the fork their either, as unshare into another NS is only valid for the current's process future children... > > [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': sure, would be useful as it actually gets you the full CIDR and possibly any other detail. > $ lxc-info --pid 1000 > PID: 34897 > $ ip netns attach 1000 34897 > $ ip -n 1000 a show three commands isn't exactly better than the "no command" I wished for ;-) > 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. yeah no, that seems icky. > > 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. the latter is adding a lot of complexity, the former can work out but would need to be in /run or the like, else every worker would just cache 1000s of entries eventually, and that's not really that what a simple and clean solution would look like either. We could replicate what we do for VMs, adding a `$full` boolean flag and only query that info if it's true, that'd be at least dead simple. > >> 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 > and also: On 27.07.21 16:03, Thomas Lamprecht wrote: > 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... ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-07-27 15:03 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-07-27 13:37 [pve-devel] [PATCH container] vmstatus: include detected IP address of running containers Oguz Bektas 2021-07-27 14:03 ` Thomas Lamprecht 2021-07-27 14:17 ` Oguz Bektas 2021-07-27 15:02 ` Thomas Lamprecht
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox