On 13.04.21 14:16, Fabian Grünbichler wrote:
> Signed-off-by: Fabian Grünbichler <f.gruenbich...@proxmox.com>
> ---
>  PVE/API2/Nodes.pm | 70 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 70 insertions(+)
> 
> diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm
> index ba6621c6..b30d0739 100644
> --- a/PVE/API2/Nodes.pm
> +++ b/PVE/API2/Nodes.pm
> @@ -222,6 +222,7 @@ __PACKAGE__->register_method ({
>       my ($param) = @_;
>  
>       my $result = [
> +         { name => 'addr' },
>           { name => 'aplinfo' },
>           { name => 'apt' },
>           { name => 'capabilities' },
> @@ -2183,6 +2184,75 @@ __PACKAGE__->register_method ({
>       return undef;
>      }});
>  
> +__PACKAGE__->register_method ({
> +    name => 'get_node_addr',
> +    path => 'addr',

hmm, not so sure if this the best path choice, if network wouldn't be templated 
by
the {iface} param that would be a better choice to avoid crowding here.

I pondered over moving that one a level deeper to network/if/{name} for a 
major releas,
and mode some things like netstat, possible even dns, under there - but it's 
work and
the gain was rather small - with this call which could fit there ROI would be 
slightly
bigger, but not too sure if worth it, just throwing out the idea there.

Besides that, the name could be slightly more descriptive `ip-addr` or even 
ip-addresses`?

> +    method => 'GET',
> +    proxyto => 'node',
> +    permissions => {
> +     check => ['perm', '/', [ 'Sys.Audit' ]],

why not `/nodes/{node}` ?

> +    },
> +    description => "Get the content of /etc/hosts.",

above is wrong? probably left-over from copying?

> +    parameters => {
> +     additionalProperties => 0,
> +     properties => {
> +         node => get_standard_option('pve-node'),
> +         cidr => {
> +             type => 'string',
> +             format => 'CIDR',
> +             format_description => 'CIDR',
> +             description => 'Extra network for which to retrieve local 
> address(es).',
> +             optional => 1,
> +         },
> +     },
> +    },
> +    returns => {
> +     type => 'object',
> +     properties => {
> +         default => {
> +             type => 'string',
> +             description => 'Default node IP.',
> +             format => 'ip',
> +         },
> +         migration => {
> +             type => 'array',
> +             items => {
> +                 type => 'string',
> +                 description => 'Migration network IP(s).',
> +                 format => 'ip',
> +             },
> +             optional => 1,
> +         },
> +         extra => {
> +             type => 'array',
> +             items => {
> +                 type => 'string',
> +                 description => 'Extra network IP(s).',
> +                 format => 'ip',
> +             },
> +             optional => 1,
> +         },
> +     },
> +    },
> +    code => sub {
> +     my ($param) = @_;
> +
> +     my $data = {};
> +
> +     my $default = PVE::Cluster::remote_node_ip($param->{node});
> +
> +     my $dc_conf = cfs_read_file('datacenter.cfg');
> +     my $migration = $dc_conf->{migration}->{network};
> +
> +     $data->{default} = $default if defined($default);
> +     $data->{migration} = PVE::Network::get_local_ip_from_cidr($migration, 1)
> +         if $migration;

style nit, probably opinionated and thus really no hard feelings, but maybe the
following is lightly more legible due to declaration being nearer to usage:

if (my $default = PVE::Cluster::remote_node_ip($param->{node}) {
    $data->{default} = $default;
}

my $dc_conf = cfs_read_file('datacenter.cfg');
if (my $migration = $dc_conf->{migration}->{network}) {
    $data->{migration} = PVE::Network::get_local_ip_from_cidr($migration, 1);
}

if (my $cidr = $param->{cidr}) {
    $data->{extra} = PVE::Network::get_local_ip_from_cidr($param->{cidr}, 1)
}

> +     $data->{extra} = PVE::Network::get_local_ip_from_cidr($param->{cidr}, 1)
> +         if $param->{cidr};
> +
> +     return $data;
> +    }});
> +
>  # bash completion helper
>  
>  sub complete_templet_repo {
> 



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Reply via email to