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

[Dominik Csapak <d.csapak@proxmox.com>]

On 8/26/25 4:07 PM, Stefan Hanreich wrote:
> 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?
> 

for the root node of a tree, there are
`extract_expanded_state()` and `apply_expanded_state()` so
the idea here is you extract the state, refresh, and then apply the old
state again. assuming the keys did not change, this should keep the
expansion state


>>   * 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!
> 
> 
> 
> _______________________________________________
> pdm-devel mailing list
> pdm-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel



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