Re: [pdm-devel] [PATCH proxmox-datacenter-manager 04/16] api: sdn: add create_zone endpoint

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

one comment inline

On 8/27/25 1:34 PM, Stefan Hanreich wrote:
> This endpoint is used for creating a new EVPN zone on multiple
> remotes. It utilizes the newly introduced LockSdnClients helper for
> performing the action simultaneously across all remotes and rolling
> back in case of failure.
> 
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> ---
>   lib/pdm-api-types/src/sdn.rs |  61 ++++++++++++++++++++
>   lib/pdm-client/src/lib.rs    |   7 +++
>   server/src/api/sdn/zones.rs  | 108 +++++++++++++++++++++++++++++++++--
>   3 files changed, 170 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/pdm-api-types/src/sdn.rs b/lib/pdm-api-types/src/sdn.rs
> index 28b20c5..2fd988f 100644
> --- a/lib/pdm-api-types/src/sdn.rs
> +++ b/lib/pdm-api-types/src/sdn.rs
> @@ -4,6 +4,67 @@ use serde::{Deserialize, Serialize};
>   
>   use crate::remotes::REMOTE_ID_SCHEMA;
>   
> +pub const VXLAN_ID_SCHEMA: Schema = IntegerSchema::new("VXLAN VNI")
> +    .minimum(1)
> +    .maximum(16777215)
> +    .schema();
> +
> +const_regex! {
> +    SDN_ID_FORMAT = "[a-zA-Z][a-zA-Z0-9]*[a-zA-Z0-9]";
> +}
> +
> +pub const SDN_ID_SCHEMA: Schema = StringSchema::new("The name for an SDN object.")
> +    .min_length(2)
> +    .max_length(8)
> +    .format(&ApiStringFormat::Pattern(&SDN_ID_FORMAT))
> +    .schema();
> +
> +#[api(
> +    properties: {
> +        remote: {
> +            schema: REMOTE_ID_SCHEMA,
> +        },
> +        controller: {
> +            schema: SDN_ID_SCHEMA,
> +        },
> +    }
> +)]
> +/// Describes the remote-specific informations for creating a new zone.
> +#[derive(Clone, Debug, Serialize, Deserialize)]
> +#[serde(rename_all = "kebab-case")]
> +pub struct CreateZoneRemote {
> +    pub remote: String,
> +    pub controller: String,
> +}
> +
> +#[api(
> +    properties: {
> +        "vrf-vxlan": {
> +            schema: VXLAN_ID_SCHEMA,
> +            optional: true,
> +        },
> +        remotes: {
> +            type: Array,
> +            description: "List of remotes and the controllers with which the zone should get created.",
> +            items: {
> +                type: CreateZoneRemote,
> +            }
> +        },
> +        zone: {
> +            schema: SDN_ID_SCHEMA,
> +        },
> +    }
> +)]
> +/// Contains the information for creating a new zone as well as information about the remotes where
> +/// the zone should get created.
> +#[derive(Clone, Debug, Serialize, Deserialize)]
> +#[serde(rename_all = "kebab-case")]
> +pub struct CreateZoneParams {
> +    pub zone: String,
> +    pub vrf_vxlan: Option<u32>,
> +    pub remotes: Vec<CreateZoneRemote>,
> +}
> +
>   #[api(
>       properties: {
>           remote: {
> diff --git a/lib/pdm-client/src/lib.rs b/lib/pdm-client/src/lib.rs
> index 7489cac..ec48250 100644
> --- a/lib/pdm-client/src/lib.rs
> +++ b/lib/pdm-client/src/lib.rs
> @@ -59,6 +59,7 @@ pub mod types {
>   
>       pub use pve_api_types::PveUpid;
>   
> +    pub use pdm_api_types::sdn::{CreateZoneParams, ListZone};
>       pub use pve_api_types::ListZonesType;
>   }
>   
> @@ -928,6 +929,12 @@ impl<T: HttpApiClient> PdmClient<T> {
>   
>           Ok(self.0.get(&path).await?.expect_json()?.data)
>       }
> +
> +    pub async fn pve_sdn_create_zone(&self, params: CreateZoneParams) -> Result<String, Error> {
> +        let path = "/api2/extjs/sdn/zones";
> +
> +        Ok(self.0.post(path, &params).await?.expect_json()?.data)
> +    }
>   }
>   
>   /// Builder for migration parameters.
> diff --git a/server/src/api/sdn/zones.rs b/server/src/api/sdn/zones.rs
> index 4b08736..a0227d3 100644
> --- a/server/src/api/sdn/zones.rs
> +++ b/server/src/api/sdn/zones.rs
> @@ -1,13 +1,23 @@
> -use anyhow::Error;
> +use anyhow::{format_err, Error};
>   use pbs_api_types::REMOTE_ID_SCHEMA;
> -use pdm_api_types::{remotes::RemoteType, sdn::ListZone};
> -use proxmox_router::Router;
> +use pdm_api_types::{
> +    remotes::RemoteType,
> +    sdn::{CreateZoneRemote, ListZone, SDN_ID_SCHEMA, VXLAN_ID_SCHEMA},
> +    Authid,
> +};
> +use proxmox_rest_server::WorkerTask;
> +use proxmox_router::{Router, RpcEnvironment};
>   use proxmox_schema::api;
> -use pve_api_types::ListZonesType;
> +use pve_api_types::{CreateZone, ListZonesType};
>   
> -use crate::api::pve::{connect, get_remote};
> +use crate::{
> +    api::pve::{connect, get_remote},
> +    sdn_client::LockedSdnClients,
> +};
>   
> -pub const ROUTER: Router = Router::new().get(&API_METHOD_LIST_ZONES);
> +pub const ROUTER: Router = Router::new()
> +    .get(&API_METHOD_LIST_ZONES)
> +    .post(&API_METHOD_CREATE_ZONE);
>   
>   #[api(
>       input: {
> @@ -76,3 +86,89 @@ pub async fn list_zones(
>   
>       Ok(result)
>   }
> +
> +#[api(
> +    input: {
> +        properties: {
> +            zone: { schema: SDN_ID_SCHEMA },
> +            "vrf-vxlan": {
> +                schema: VXLAN_ID_SCHEMA,
> +                optional: true,
> +            },
> +            remotes: {
> +                type: Array,
> +                description: "List of remotes with their controller where zone should get created.",
> +                items: {
> +                    type: CreateZoneRemote
> +                }
> +            },
> +        },
> +    },
> +    returns: { type: String, description: "Worker UPID" },
> +)]
> +/// Create a zone across multiple remotes
> +async fn create_zone(
> +    zone: String,
> +    vrf_vxlan: Option<u32>,
> +    remotes: Vec<CreateZoneRemote>,
> +    rpcenv: &mut dyn RpcEnvironment,
> +) -> Result<String, Error> {
> +    let auth_id: Authid = rpcenv
> +        .get_auth_id()
> +        .ok_or_else(|| format_err!("no authid available"))?
> +        .parse()?;
> +
> +    let upid = WorkerTask::spawn(
> +        "create_zone",
> +        None,
> +        auth_id.to_string(),
> +        false,
> +        move |_worker| async move {
> +            LockedSdnClients::from_remote_names(
> +                remotes
> +                    .into_iter()
> +                    .map(|remote| (remote.remote.clone(), remote)),
> +                false,
> +            )
> +            .await?
> +            .for_each(async move |client, ctx| {
> +                let params = CreateZone {
> +                    zone: zone.clone(),
> +                    vrf_vxlan,
> +                    controller: Some(ctx.data().controller.clone()),
> +                    ty: ListZonesType::Evpn,
> +                    advertise_subnets: None,
> +                    bridge: None,
> +                    bridge_disable_mac_learning: None,
> +                    dhcp: None,
> +                    disable_arp_nd_suppression: None,
> +                    dns: None,
> +                    dnszone: None,
> +                    dp_id: None,
> +                    exitnodes: None,
> +                    exitnodes_local_routing: None,
> +                    exitnodes_primary: None,
> +                    ipam: None,
> +                    mac: None,
> +                    mtu: None,
> +                    nodes: None,
> +                    peers: None,
> +                    reversedns: None,
> +                    rt_import: None,
> +                    tag: None,
> +                    vlan_protocol: None,
> +                    vxlan_port: None,
> +                    lock_token: None,
> +                    fabric: None,
> +                };

not really your fault, but maybe we could (in the future) have a
'Default' implementation (with all set to None?) and just set
the options we want? would make the code a bit nicer

> +
> +                client.create_zone(params).await
> +            })
> +            .await?
> +            .apply_and_release()
> +            .await
> +        },
> +    )?;
> +
> +    Ok(upid)
> +}



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