Re: [pve-devel] [PATCH container v2 3/3] api: return all addresses of an interface
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
---
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
On 11.12.2024 15:57, Maximiliano Sandoval wrote:
Gabriel Goller 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 v2 3/3] api: return all addresses of an interface
Gabriel Goller 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
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
> ---
> 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
