[pdm-devel] [RFC network/proxmox{, -backup, -api-types, -yew-comp, -datacenter-manager} v2 00/32] Add initial SDN / EVPN integration

[Stefan Hanreich]

On 8/26/25 2:21 PM, Gabriel Goller wrote:
> Don't have much experience with PDM, so I haven't looked at the code.
> 
> The patches don't apply cleanly anymore, but after a bit of manual
> tinkering it works. As we found out together filtering vnets doesn't
> work so we need to filter in the frontend for vnets which have a evpn
> zone. The following patch fixes it:

will rebase in a v3

> diff --git a/ui/src/sdn/evpn/remote_tree.rs b/ui/src/sdn/evpn/
> remote_tree.rs
> index e4b0fe46a121..4077693d29df 100644
> --- a/ui/src/sdn/evpn/remote_tree.rs
> +++ b/ui/src/sdn/evpn/remote_tree.rs
> @@ -207,13 +207,12 @@ fn zones_to_vrf_view(
>      for vnet in vnets {
>          let vnet_data = &vnet.vnet;
> 
> -        let zone = zones
> -            .iter()
> -            .find(|zone| {
> -                zone.remote == vnet.remote
> -                    && vnet_data.zone.as_ref().expect("vnet has zone")
> == &zone.zone.zone
> -            })
> -            .expect("vnet has zone");
> +        let Some(zone) = zones.iter().find(|zone| {
> +            zone.remote == vnet.remote
> +                && vnet_data.zone.as_ref().expect("vnet has zone") ==
> &zone.zone.zone
> +        }) else {
> +            continue;
> +        };
> 
>          let controller = controllers
>              .iter()
> diff --git a/ui/src/sdn/evpn/vrf_tree.rs b/ui/src/sdn/evpn/vrf_tree.rs
> index 8b01b00eba26..2980d2379b0a 100644
> --- a/ui/src/sdn/evpn/vrf_tree.rs
> +++ b/ui/src/sdn/evpn/vrf_tree.rs
> @@ -155,13 +155,12 @@ fn zones_to_vrf_view(
>      for vnet in vnets {
>          let vnet_data = &vnet.vnet;
> 
> -        let zone = zones
> -            .iter()
> -            .find(|zone| {
> -                zone.remote == vnet.remote
> -                    && vnet_data.zone.as_ref().expect("vnet has zone")
> == &zone.zone.zone
> -            })
> -            .expect("zone of vnet exists");
> +        let Some(zone) = zones.iter().find(|zone| {
> +            zone.remote == vnet.remote
> +                && vnet_data.zone.as_ref().expect("vnet has zone") ==
> &zone.zone.zone
> +        }) else {
> +            continue;
> +        };
> 
>          let controller = controllers
>              .iter()
> 
> 
> A few small UI nits:
>  * In the "Remotes" view, widen the "Name" column a bit, it's too narrow
>  * In the "IP-VRFs" view, when fully expanding all the VNets, the "VNet"
>    level text is indented more than the last child. So the "VNet" text
>    is more to the left than the actual VNet name below. (I think this is
>    the VNet icon missing.)
>  * Clicking the refresh button should IMO not collapse the tree.

Not sure how I can prevent that, since I rebuild the tree after every
request so it will 'reset' to the collapsed state. Maybe @Dominik has an
idea?

>  * When clicking on "Add" the VNet icon is missing (The zone icon is
>    there).

Weird, the icon should be included in the patch series and showed
correctly on my machine. I'll check it out!

>  * What about deleting zones and vnets? Would that be complex? If a
>    remote fails to delete a vnet/zone, we could roll-back all the other
>    ones using the lock thingy-right?

shouldn't be too hard to add in a future patch series, but I had to make
a scope cut somewhere for now since I was already running quite late
with the series.

>  * I think there is a min-character limit missing on the vnet name, I
>    get:
>      2025-08-26T14:16:38+02:00: failed to execute transaction on remote
> andiknowbangers: api error (status = 400: Parameter verification failed.
>      vnet: invalid format - vnet ID 't' contains illegal characters
>    when creating a vnet named "t".
>  * IDK about the EVPN icon being a key :) Maybe we should already create
> a SDN
>    "folder" and then an EVPN entry (like in PVE) so that we are ready>
   adding more stuff afterwards.
>  * Do we really want to auto-apply everything immediately? Should we
>    maybe introduce a SDN "apply" thingy in PDM as well?

In the future we could certainly implement that, but currently we are
only using the running configuration as source for the PDM tree.

If we want to render things correctly on PDM side after adding Zones /
VNets, we'd have to read the pending configuration and then special case
new / changed / deleted entities and respect that state while merging -
which is more complex than this current approach so I've left it out for
now. Certainly something for the future though.


Thanks for the review, I'll incorporate your suggestions in a v3!