public inbox for pdm-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Dominik Csapak <d.csapak@proxmox.com>
To: Proxmox Datacenter Manager development discussion
	<pdm-devel@lists.proxmox.com>,
	Stefan Hanreich <s.hanreich@proxmox.com>
Subject: Re: [pdm-devel] [PATCH network/proxmox{, -backup, -api-types, -datacenter-manager} v2 00/30] Add initial SDN / EVPN integration
Date: Tue, 2 Sep 2025 13:10:29 +0200	[thread overview]
Message-ID: <8e3d101a-cb70-4f20-b36a-c6632d34b421@proxmox.com> (raw)
In-Reply-To: <20250829145313.329114-1-s.hanreich@proxmox.com>

setup a few remotes created a few zones/vnets, series tested mostly fine
(did not do too extensive testing though)

noticed one thing that can probably be improved (i think on the pve 
side?): when i try to create e.g. a vnet with an id that already exists

i get
---8<---
2025-09-02T13:04:14+02:00: failed to execute transaction on remote 
pve-ceph-cluster: api error (status = 500: create sdn vnet object 
failed: sdn vnet object ID 'foo' already defined
)
2025-09-02T13:04:14+02:00: could not rollback and unlock configuration 
for remote pve-ceph-cluster - configuration needs to be manually 
unlocked via 'pvesh delete /cluster/sdn/lock --force 1'
--->8---

two things here:
* there is a stray newline in the error message
* detecting duplicates should already be detected before applying
   and we should be able to roll them back?

aside from my two minor comments (see the relevant patches) and
iff the error i got here has to be fixed on the pve side (not sure about 
the interaction) consider this series

Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
Tested-by: Dominik Csapak <d.csapak@proxmox.com>

there are maybe some things that could be polished in the ui in the
future (edit window default width for example, emptytext in the grids,
etc.) but nothing urgent or blocking IMHO,


On 8/29/25 4:53 PM, Stefan Hanreich wrote:
> ## Introduction
> 
> This patch series adds a new panel to the PDM that shows an overview of the
> current state of all EVPN zones across all remotes. It includes two different
> tree views:
> 
> * IP-VRFs: that shows the contents of all IP-VRFs (identified by their Route
>    Target = ASN:VNI) across all remotes.
> * Zones: that shows the contents of a specific zone on a specific remote.
> 
> For more information on the two tree views, consult the respective commits that
> introduce the components.
> 
> The panel also allows users to create new Zones / VNets on multiple remotes
> simultaneously by utilizing the new SDN locking functionality.
> 
> I have provided prebuilt packages on the share in the folder pdm-evpn.
> 
> This patch series requires the ParallelFetcher patch series from Lukas in order
> to work.
> 
> ## API
> 
> It introduces the following API endpoints on PDM:
> 
> /sdn
>      GET /controllers - list the controllers of all remotes
>      GET /zones - list the zones of all remotes
>      POST /zones - create a zone on multiple remotes
>      GET /vnets - list the vnets of all remotes
>      POST /vnets - create a vnet on multiple remotes
> 
> 
> ## Additional remarks
> 
> This patch series contains some preparatory patches that are not directly
> related to the implemented functionality:
> 
> * One fix for proxmox-schema so values that are larger than i32 can be used in
>    the integer schema definition (required for e.g. 64-bit ASNs)
> * Add JSONSchema to a lot of SDN API endpoints that were previously undocumented
> 
> I have sent them initially as separate patch series, but since they are a hard
> requirement for this patch series I have merged all of them into one patch
> series now. This way it is easier to keep track of the requirements.
> 
> 
> ## Open questions for reviewers
> 
> * The LockedSdnClient(s) are abstractions for locked SDN remotes. I'm still a
> bit unsure about the design / implementation but for future features I will be
> making more complex changes across multiple remotes so I figured an abstraction
> for this will come in handy in the future.
> 
> I'd love some inputs / opinions on the API design as well as the general concept
> of locking config -> making changes -> rolling back / applying.
> 
> I will work on a more sophisticated implementation utilizing tokio-specific
> functions in the following days, but I wanted to get the patch series out now
> and validate the API / general idea.
> 
> * We might wanna move the EvpnRouteTarget type out of the UI, even though it is
> currently only used there.
> 
> * Should we introduce a caching mechanism for the SDN API calls?
> 
> I have shortly talked about this with @Lukas, but we decided against
> implementing such a mechanism for now after some deliberation.
> 
> Showing outdated information is particularly problematic with configuration,
> especially because the create dialogues rely on that information.
> 
> After creating a new zone / vnet we would have to hit the remotes anyway, in
> order to be able to show the updated data immediately.
> 
> The downside is of course a long load time for the EVPN panel, as well as a long
> load if even one of the remotes is not available.
> 
> For an initial release I think it is fine to go forward without caching and see
> how it works out in practice based on reports from our users. Any input on this
> matter would be greatly appreciated!
> 
> 
> ## Future Work
> * show the output of the new status API calls created by Gabriel in the views.
> * add a functionality for grouping remotes together, instead of implicitly
>    grouping them based on ASN:VNI
> * introduce a caching mechanism for the SDN API calls (?)
> * integration tests with mocked SDN clients
> * add some QoL to the UI (e.g expand/collapse all)
> 
> 
> Huge thanks to @Lukas and @Dominik for helping me greatly on moving this patch
> series forward the last few days!
> 
> ## Changelog
> 
> Changes from v1:
> * detect legacy PVE remotes without SDN locking API capability
> * remove already applied patch
> * parallelize list endpoints via Lukas' ParallelFetcher
> * reversed toolbar / grid order in EVPN panel
> * updated and improved commit messages
> * added missing translation macro invocations
> * replaced thread_local in components
> * store columns in component to avoid re-creating them on update
> * add better error message in add_zone/vnet dialogues if there is no
>    controller / zone
> * remove unused message from vrf/remote tree components
> * use update_root_tree for restoring tree state
> * moved EVPN above remotes in the main menu
> * added instructions on how to unlock SDN configuration in cases of errors
> 
> Changes from RFC v2:
> * rebased on top of current master
> * improved error handling for the yew components considerably
> * tinkered with column sizes in the remote view
> * preserve collapsed state on refresh
> * fix SDN ID schema definition
> * improved EVPN icon
> * moved task descriptions from yew-comp to pdm
> * improved default sorting order for the remote view
> 
> Changes from RFC v1:
> * overhauled the structure of the trees completely
>    * split the initial tree view into two distinct tree views
>    * changed the grouping of elements
>    * improved and unified the terms used across all UI elements
> * improved toolbar design
> * removed the controller data table, since the tree views should now include
>    that information
> * improved locked SDN client and added a collection type for locked SDN clients
> * improved error handling and logging considerably for the worker tasks
> 
> 
> ## Dependencies:
> pbs-api-types depends on proxmox-schema
> proxmox-api-types depends on proxmox-schema
> proxmox-backup depends on proxmox-schema
> proxmox-datacenter-manager depends on proxmox-schema
> 
> proxmox-api-types depends on pve-network
> proxmox-datacenter-manager depends on proxmox-api-types
> proxmox-datacenter-manager depends on pve-network
> 
> proxmox:
> 
> Stefan Hanreich (2):
>    schema: use i64 for minimum / maximum / default integer values
>    pbs-api-types: fix values for integer schemas
> 
>   pbs-api-types/src/datastore.rs  |  6 +++---
>   proxmox-schema/src/de/mod.rs    |  3 +--
>   proxmox-schema/src/de/verify.rs | 13 ++++++++-----
>   proxmox-schema/src/schema.rs    | 18 +++++++++---------
>   4 files changed, 21 insertions(+), 19 deletions(-)
> 
> 
> proxmox-backup:
> 
> Stefan Hanreich (1):
>    api: change integer schema parameters to i64
> 
>   pbs-tape/src/bin/pmt.rs           |  6 +++---
>   proxmox-backup-client/src/main.rs |  2 +-
>   pxar-bin/src/main.rs              |  6 +++---
>   src/api2/backup/upload_chunk.rs   | 15 ++++++---------
>   4 files changed, 13 insertions(+), 16 deletions(-)
> 
> 
> pve-network:
> 
> Stefan Hanreich (6):
>    sdn: api: return null for rollback / lock endpoints
>    controllers: fix maximum value for ASN
>    api: add state standard option
>    api: controllers: update schema of endpoints
>    api: vnets: update schema of endpoints
>    api: zones: update schema of endpoints
> 
>   src/PVE/API2/Network/SDN.pm                   |   4 +
>   src/PVE/API2/Network/SDN/Controllers.pm       | 116 +++++++++-
>   src/PVE/API2/Network/SDN/Vnets.pm             |  92 +++++++-
>   src/PVE/API2/Network/SDN/Zones.pm             | 203 ++++++++++++++++--
>   src/PVE/Network/SDN.pm                        |  10 +
>   src/PVE/Network/SDN/Controllers/BgpPlugin.pm  |   7 +-
>   src/PVE/Network/SDN/Controllers/EvpnPlugin.pm |   2 +-
>   src/PVE/Network/SDN/Controllers/IsisPlugin.pm |   6 +-
>   src/PVE/Network/SDN/VnetPlugin.pm             |  21 +-
>   src/PVE/Network/SDN/Zones/EvpnPlugin.pm       |  22 +-
>   src/PVE/Network/SDN/Zones/QinQPlugin.pm       |   6 +-
>   src/PVE/Network/SDN/Zones/VlanPlugin.pm       |   1 +
>   src/PVE/Network/SDN/Zones/VxlanPlugin.pm      |  15 +-
>   13 files changed, 457 insertions(+), 48 deletions(-)
> 
> 
> proxmox-api-types:
> 
> Stefan Hanreich (6):
>    sdn: add list/create zone endpoints
>    sdn: add list/create vnet endpoints
>    sdn: add list/create controller endpoints
>    sdn: add sdn configuration locking endpoints
>    tasks: add helper for querying successfully finished tasks
>    sdn: add helpers for pending values
> 
>   pve-api-types/generate.pl      | 38 ++++++++++++++++++++++++++++++++++
>   pve-api-types/src/lib.rs       |  1 +
>   pve-api-types/src/sdn.rs       | 33 +++++++++++++++++++++++++++++
>   pve-api-types/src/types/mod.rs |  4 ++++
>   4 files changed, 76 insertions(+)
>   create mode 100644 pve-api-types/src/sdn.rs
> 
> 
> proxmox-datacenter-manager:
> 
> Stefan Hanreich (15):
>    server: add locked sdn client helpers
>    ui: pve: sdn: add descriptions for sdn tasks
>    api: sdn: add list_zones endpoint
>    api: sdn: add create_zone endpoint
>    api: sdn: add list_vnets endpoint
>    api: sdn: add create_vnet endpoint
>    api: sdn: add list_controllers endpoint
>    ui: sdn: add EvpnRouteTarget type
>    ui: sdn: add vnet icon
>    ui: sdn: add view for showing evpn zones
>    ui: sdn: add view for showing ip vrfs
>    ui: sdn: add component for creating evpn vnets
>    ui: sdn: add component for creatin evpn zones
>    ui: sdn: add evpn overview panel
>    ui: sdn: add evpn panel to main menu
> 
>   lib/pdm-api-types/Cargo.toml      |   2 +
>   lib/pdm-api-types/src/lib.rs      |   2 +
>   lib/pdm-api-types/src/sdn.rs      | 168 +++++++++++
>   lib/pdm-client/src/lib.rs         |  61 ++++
>   server/src/api/mod.rs             |   2 +
>   server/src/api/sdn/controllers.rs | 112 +++++++
>   server/src/api/sdn/mod.rs         |  17 ++
>   server/src/api/sdn/vnets.rs       | 180 +++++++++++
>   server/src/api/sdn/zones.rs       | 204 +++++++++++++
>   server/src/lib.rs                 |   1 +
>   server/src/sdn_client.rs          | 427 ++++++++++++++++++++++++++
>   ui/css/pdm.scss                   |  14 +-
>   ui/images/icon-sdn-vnet.svg       |   6 +
>   ui/src/lib.rs                     |   2 +
>   ui/src/main_menu.rs               |  10 +
>   ui/src/sdn/evpn/add_vnet.rs       | 313 +++++++++++++++++++
>   ui/src/sdn/evpn/add_zone.rs       | 328 ++++++++++++++++++++
>   ui/src/sdn/evpn/evpn_panel.rs     | 275 +++++++++++++++++
>   ui/src/sdn/evpn/mod.rs            |  41 +++
>   ui/src/sdn/evpn/remote_tree.rs    | 480 ++++++++++++++++++++++++++++++
>   ui/src/sdn/evpn/vrf_tree.rs       | 409 +++++++++++++++++++++++++
>   ui/src/sdn/mod.rs                 |   1 +
>   ui/src/tasks.rs                   |   4 +
>   23 files changed, 3058 insertions(+), 1 deletion(-)
>   create mode 100644 lib/pdm-api-types/src/sdn.rs
>   create mode 100644 server/src/api/sdn/controllers.rs
>   create mode 100644 server/src/api/sdn/mod.rs
>   create mode 100644 server/src/api/sdn/vnets.rs
>   create mode 100644 server/src/api/sdn/zones.rs
>   create mode 100644 server/src/sdn_client.rs
>   create mode 100644 ui/images/icon-sdn-vnet.svg
>   create mode 100644 ui/src/sdn/evpn/add_vnet.rs
>   create mode 100644 ui/src/sdn/evpn/add_zone.rs
>   create mode 100644 ui/src/sdn/evpn/evpn_panel.rs
>   create mode 100644 ui/src/sdn/evpn/mod.rs
>   create mode 100644 ui/src/sdn/evpn/remote_tree.rs
>   create mode 100644 ui/src/sdn/evpn/vrf_tree.rs
>   create mode 100644 ui/src/sdn/mod.rs
> 
> 
> Summary over all repositories:
>    48 files changed, 3625 insertions(+), 84 deletions(-)
> 



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


  parent reply	other threads:[~2025-09-02 11:10 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-29 14:52 Stefan Hanreich
2025-08-29 14:52 ` [pdm-devel] [PATCH proxmox v2 1/2] schema: use i64 for minimum / maximum / default integer values Stefan Hanreich
2025-08-29 14:52 ` [pdm-devel] [PATCH proxmox v2 2/2] pbs-api-types: fix values for integer schemas Stefan Hanreich
2025-08-29 14:52 ` [pdm-devel] [PATCH proxmox-backup v2 1/1] api: change integer schema parameters to i64 Stefan Hanreich
2025-08-29 14:52 ` [pdm-devel] [PATCH pve-network v2 1/6] sdn: api: return null for rollback / lock endpoints Stefan Hanreich
2025-08-29 14:52 ` [pdm-devel] [PATCH pve-network v2 2/6] controllers: fix maximum value for ASN Stefan Hanreich
2025-08-29 14:52 ` [pdm-devel] [PATCH pve-network v2 3/6] api: add state standard option Stefan Hanreich
2025-08-29 14:52 ` [pdm-devel] [PATCH pve-network v2 4/6] api: controllers: update schema of endpoints Stefan Hanreich
2025-08-29 14:52 ` [pdm-devel] [PATCH pve-network v2 5/6] api: vnets: " Stefan Hanreich
2025-08-29 14:52 ` [pdm-devel] [PATCH pve-network v2 6/6] api: zones: " Stefan Hanreich
2025-08-29 14:52 ` [pdm-devel] [PATCH proxmox-api-types v2 1/6] sdn: add list/create zone endpoints Stefan Hanreich
2025-08-29 14:52 ` [pdm-devel] [PATCH proxmox-api-types v2 2/6] sdn: add list/create vnet endpoints Stefan Hanreich
2025-08-29 14:52 ` [pdm-devel] [PATCH proxmox-api-types v2 3/6] sdn: add list/create controller endpoints Stefan Hanreich
2025-08-29 14:52 ` [pdm-devel] [PATCH proxmox-api-types v2 4/6] sdn: add sdn configuration locking endpoints Stefan Hanreich
2025-08-29 14:52 ` [pdm-devel] [PATCH proxmox-api-types v2 5/6] tasks: add helper for querying successfully finished tasks Stefan Hanreich
2025-08-29 14:52 ` [pdm-devel] [PATCH proxmox-api-types v2 6/6] sdn: add helpers for pending values Stefan Hanreich
2025-08-29 14:52 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 01/15] server: add locked sdn client helpers Stefan Hanreich
2025-08-29 14:52 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 02/15] ui: pve: sdn: add descriptions for sdn tasks Stefan Hanreich
2025-08-29 14:52 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 03/15] api: sdn: add list_zones endpoint Stefan Hanreich
2025-09-01 13:36   ` Dominik Csapak
2025-08-29 14:53 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 04/15] api: sdn: add create_zone endpoint Stefan Hanreich
2025-08-29 14:53 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 05/15] api: sdn: add list_vnets endpoint Stefan Hanreich
2025-08-29 14:53 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 06/15] api: sdn: add create_vnet endpoint Stefan Hanreich
2025-08-29 14:53 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 07/15] api: sdn: add list_controllers endpoint Stefan Hanreich
2025-08-29 14:53 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 08/15] ui: sdn: add EvpnRouteTarget type Stefan Hanreich
2025-08-29 14:53 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 09/15] ui: sdn: add vnet icon Stefan Hanreich
2025-08-29 14:53 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 10/15] ui: sdn: add view for showing evpn zones Stefan Hanreich
2025-08-29 14:53 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 11/15] ui: sdn: add view for showing ip vrfs Stefan Hanreich
2025-08-29 14:53 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 12/15] ui: sdn: add component for creating evpn vnets Stefan Hanreich
2025-08-29 14:53 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 13/15] ui: sdn: add component for creatin evpn zones Stefan Hanreich
2025-08-29 14:53 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 14/15] ui: sdn: add evpn overview panel Stefan Hanreich
2025-09-01 13:44   ` Dominik Csapak
2025-09-01 15:02     ` Stefan Hanreich
2025-09-02  6:34       ` Dominik Csapak
2025-09-02  9:30         ` Dominik Csapak
2025-09-02 12:22           ` Stefan Hanreich
2025-08-29 14:53 ` [pdm-devel] [PATCH proxmox-datacenter-manager v2 15/15] ui: sdn: add evpn panel to main menu Stefan Hanreich
2025-09-02 11:10 ` Dominik Csapak [this message]
2025-09-02 12:03   ` [pdm-devel] [PATCH network/proxmox{, -backup, -api-types, -datacenter-manager} v2 00/30] Add initial SDN / EVPN integration Stefan Hanreich
2025-09-02 12:29     ` Dominik Csapak
2025-09-02 12:54       ` Stefan Hanreich
2025-09-02 14:10 ` [pdm-devel] superseded: " Stefan Hanreich

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=8e3d101a-cb70-4f20-b36a-c6632d34b421@proxmox.com \
    --to=d.csapak@proxmox.com \
    --cc=pdm-devel@lists.proxmox.com \
    --cc=s.hanreich@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal