Re: [pve-devel] [PATCH pve-manager v3 3/9] pvestatd: add network resource to status reporting

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

and why is this subject prefixed with pvestatd?

It's mostly for the "api: cluster:" subsystem. Could be broken into
two though, first one adding the broadcasting to pvestatd, second
one actually using that in the cluster resource API.

Am 07.11.25 um 15:38 schrieb Stefan Hanreich:
> From: Gabriel Goller <g.goller@proxmox.com>
> 
> The new network resource will act as the top-level resource for all
> networking entities (including SDN entities). The network resource
> contains a network_type field, which indicates the type of networking
> resource - similar to how the storage plugin handles different types
> of storages. For now, it contains SDN fabrics and the SDN zones have
> been copied over as well.
> 
> The main reason for moving over to a new resource type is the current
> ID schema of the SDN resource, which is 'sdn/{zone_id}'. This makes it
> hard to extend without the possibility of ID collisions. Additionally,
> since the ID is used in several places throughout the backend / UI,
> changing the schema would break compatibility with nodes that are on
> an earlier version and would be an API break as well.
> 
> Nodes will still broadcast the old format for backwards-compatibility
> and nodes with this patch applied support handling both formats. With
> this patch, nodes will check whether a node is sending both formats or
> only the old one, and parse the resources based on that information.
> Older nodes will drop the new network resource type, but will still be
> able to show zones, because the old format still gets broadcast. Newer
> nodes will take the information from the network store, if available,
> otherwise fall back to the SDN store.
> 
> Another reason for keeping the old format around is so we do not break
> older clients, that rely on the old SDN format - removing it would be
> a breaking API change.
> 
> Co-authored-by: Stefan Hanreich <s.hanreich@proxmox.com>
> Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> ---
>  PVE/API2/Cluster.pm     | 101 +++++++++++++++++++++++++++++++++-------
>  PVE/Service/pvestatd.pm |  27 +++++++++++
>  2 files changed, 110 insertions(+), 18 deletions(-)
> 
> diff --git a/PVE/API2/Cluster.pm b/PVE/API2/Cluster.pm
> index 479803960..0c779bf9b 100644
> --- a/PVE/API2/Cluster.pm
> +++ b/PVE/API2/Cluster.pm
> @@ -251,7 +251,8 @@ __PACKAGE__->register_method({
>                  type => {
>                      description => "Resource type.",
>                      type => 'string',
> -                    enum => ['node', 'storage', 'pool', 'qemu', 'lxc', 'openvz', 'sdn'],
> +                    enum =>
> +                        ['node', 'storage', 'pool', 'qemu', 'lxc', 'openvz', 'sdn', 'network'],
>                  },
>                  status => {
>                      description => "Resource type dependent status.",
> @@ -431,6 +432,23 @@ __PACKAGE__->register_method({
>                      optional => 1,
>                      default => 0,
>                  },
> +                network => {
> +                    description => "The name of a Network entity (for type 'network').",
> +                    type => "string",
> +                    optional => 1,
> +                },
> +                network_type => {
> +                    description => "The type of network resource (for type 'network').",
> +                    type => "string",
> +                    enum => ["fabric", "zone"],
> +                    optional => 1,
> +                },
> +                protocol => {
> +                    description =>
> +                        "The protocol of a fabric (for type 'network', network_type 'fabric').",
> +                    type => "string",
> +                    optional => 1,
> +                },
>              },
>          },
>      },
> @@ -584,25 +602,15 @@ __PACKAGE__->register_method({
>          }
>  
>          if (!$param->{type} || $param->{type} eq 'sdn') {
> -            #add default "localnetwork" zone
> -            if ($rpcenv->check($authuser, "/sdn/zones/localnetwork", ['SDN.Audit'], 1)) {
> -                foreach my $node (@$nodelist) {
> -                    my $local_sdn = {
> -                        id => "sdn/$node/localnetwork",
> -                        sdn => 'localnetwork',
> -                        node => $node,
> -                        type => 'sdn',
> -                        status => 'ok',
> -                    };
> -                    push @$res, $local_sdn;
> -                }
> -            }
> +            my $nodes = PVE::Cluster::get_node_kv("sdn");
> +            my $network_nodes = PVE::Cluster::get_node_kv("network");
>  
> -            if ($have_sdn) {
> -                my $nodes = PVE::Cluster::get_node_kv("sdn");
> +            for my $node (sort keys %{$nodes}) {
> +                # host is already sending the new network resource, so ignore
> +                # its sdn resources
> +                next if defined $network_nodes->{$node};
>  
> -                for my $node (sort keys %{$nodes}) {
> -                    my $sdns = decode_json($nodes->{$node});
> +                my $sdns = decode_json($nodes->{$node});
>  
>                      for my $id (sort keys %{$sdns}) {
>                          next if !$rpcenv->check($authuser, "/sdn/zones/$id", ['SDN.Audit'], 1);
> @@ -620,6 +628,63 @@ __PACKAGE__->register_method({
>              }
>          }
>  
> +        if (!$param->{type} || $param->{type} eq 'network') {
> +            my $nodes = PVE::Cluster::get_node_kv("network");
> +
> +            # add default "localnetwork" zone
> +            if ($rpcenv->check($authuser, "/sdn/zones/localnetwork", ['SDN.Audit'], 1)) {
> +                foreach my $node (@$nodelist) {
> +                    my $local_sdn = {
> +                        id => "network/$node/zone/localnetwork",
> +                        type => 'network',
> +                        network_type => 'zone',
> +                        network => 'localnetwork',
> +                        node => $node,
> +                        status => 'ok',
> +                    };
> +                    push $res->@*, $local_sdn;
> +                }
> +            }
> +
> +            for my $node (sort keys $nodes->%*) {
> +                my $node_config = decode_json($nodes->{$node});
> +
> +                for my $id (sort keys $node_config->%*) {
> +                    my $entry = $node_config->{$id};
> +
> +                    if ($entry->{network_type} eq 'fabric') {
> +                        next
> +                            if !$rpcenv->check_any(
> +                                $authuser,
> +                                "/sdn/fabrics/$entry->{network}",
> +                                ['SDN.Audit', 'SDN.Allocate'],
> +                                1,
> +                            );
> +                    } elsif ($entry->{network_type} eq 'zone') {
> +                        next
> +                            if !$rpcenv->check(
> +                                $authuser,
> +                                "/sdn/zones/$entry->{network}",
> +                                ['SDN.Audit'],
> +                                1,
> +                            );
> +                    } else {
> +                        # unknown type, so most likely introduced in a newer
> +                        # version - avoid leaking information by suppressing any
> +                        # unknown sdn types in the returned array.
> +                        next;
> +                    }

Might want to add a local helper looking something like

my sub can_access_network {
    my ($rpcenv, $type, $network) = @_;

    if ($type eq 'fabric') {
        return rpcenv->check_any($authuser, "/sdn/fabrics/${network}",  ['SDN.Audit', 'SDN.Allocate'], 1);
    } elsif ($entry->{network_type} eq 'zone') {
        return rpcenv->check($authuser, "/sdn/zones/${network}",  ['SDN.Audit'], 1);
    }
    return 0;
}


And use that here to make it clearer what mainly happens here.

> +
> +                    push $res->@*,
> +                        {
> +                            "id" => "network/$node/$entry->{network_type}/$entry->{network}",
> +                            "node" => $node,
> +                            $entry->%*,
> +                        };
> +                }
> +            }
> +        }
> +
>          return $res;
>      },
>  });
> diff --git a/PVE/Service/pvestatd.pm b/PVE/Service/pvestatd.pm
> index 618d6139a..085bf9d61 100755
> --- a/PVE/Service/pvestatd.pm
> +++ b/PVE/Service/pvestatd.pm
> @@ -15,6 +15,7 @@ use PVE::CpuSet;
>  use Filesys::Df;
>  use PVE::INotify;
>  use PVE::Network;
> +use PVE::RS::SDN::Fabrics;
>  use PVE::NodeConfig;
>  use PVE::Cluster qw(cfs_read_file);
>  use PVE::Storage;
> @@ -775,6 +776,28 @@ sub update_sdn_status {
>      }
>  }
>  
> +sub update_network_status {
> +    my $network_status = {};
> +
> +    my ($fabric_status) = PVE::RS::SDN::Fabrics::status();
> +    for my $fabric (values $fabric_status->%*) {
> +        $network_status->{"fabric/$fabric->{network}"} = $fabric;
> +    }
> +
> +    my ($zone_status, $vnet_status) = PVE::Network::SDN::Zones::status();
> +    for my $id (sort keys $zone_status->%*) {
> +        my $zone = $zone_status->{$id};
> +
> +        $zone->{network_type} = 'zone';
> +        $zone->{network} = $id;
> +        $zone->{type} = 'network';
> +
> +        $network_status->{"zone/$id"} = $zone;
> +    }
> +
> +    PVE::Cluster::broadcast_node_kv("network", encode_json($network_status));
> +}
> +
>  my $broadcast_version_info_done = 0;
>  my sub broadcast_version_info : prototype() {
>      if (
> @@ -840,6 +863,10 @@ sub update_status {
>      $err = $@;
>      syslog('err', "sdn status update error: $err") if $err;
>  
> +    eval { update_network_status(); };
> +    $err = $@;
> +    syslog('err', "network status update error: $err") if $err;
> +
>      eval { broadcast_version_info(); };
>      $err = $@;
>      syslog('err', "version info update error: $err") if $err;



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