all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
	Gabriel Goller <g.goller@proxmox.com>
Subject: Re: [pve-devel] [PATCH container v2 3/3] api: return all addresses of an interface
Date: Tue, 10 Dec 2024 18:58:07 +0100	[thread overview]
Message-ID: <5ea7a6e0-d522-4de7-9609-bb98858a5da6@proxmox.com> (raw)
In-Reply-To: <20241210151130.321984-4-g.goller@proxmox.com>

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 <g.goller@proxmox.com>
> ---
>  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


  reply	other threads:[~2024-12-10 17:58 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-10 15:11 [pve-devel] [PATCH container/manager v2 0/3] Show container ip in summary and network tab Gabriel Goller
2024-12-10 15:11 ` [pve-devel] [PATCH manager v2 1/3] lxc: show dynamically assigned IPs in " Gabriel Goller
2024-12-10 15:11 ` [pve-devel] [PATCH manager v2 2/3] guest: refactor and reuse AgentIPView for containers Gabriel Goller
2024-12-10 15:11 ` [pve-devel] [PATCH container v2 3/3] api: return all addresses of an interface Gabriel Goller
2024-12-10 17:58   ` Thomas Lamprecht [this message]
2025-01-07 14:14     ` Gabriel Goller
2024-12-11 14:57   ` Maximiliano Sandoval
2025-01-07 14:14     ` Gabriel Goller
2024-12-12  8:02 ` [pve-devel] [PATCH container/manager v2 0/3] Show container ip in summary and network tab Fabian Grünbichler
2025-01-08  9:32 ` Gabriel Goller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5ea7a6e0-d522-4de7-9609-bb98858a5da6@proxmox.com \
    --to=t.lamprecht@proxmox.com \
    --cc=g.goller@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal