Re: [pdm-devel] [PATCH proxmox-datacenter-manager v2 03/15] api: sdn: add list_zones endpoint

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

tiny nit inline

On 8/29/25 4:53 PM, Stefan Hanreich wrote:
> Add an endpoint for listing the zones of all configured PVE remotes.
> They can be filtered by type / remote and it exposes options for
> querying the pending / running configuration.
> 
> This call is quite expensive, since it makes a GET call to every
> configured PVE remote, which can take awhile depending on the network
> connection. For the future we might want to introduce a caching
> mechanism for this call, but we've decided against it for the time
> being.
> 
> Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> ---
>   lib/pdm-api-types/Cargo.toml |   2 +
>   lib/pdm-api-types/src/lib.rs |   2 +
>   lib/pdm-api-types/src/sdn.rs |  24 ++++++++
>   lib/pdm-client/src/lib.rs    |  18 ++++++
>   server/src/api/mod.rs        |   2 +
>   server/src/api/sdn/mod.rs    |  13 ++++
>   server/src/api/sdn/zones.rs  | 111 +++++++++++++++++++++++++++++++++++
>   7 files changed, 172 insertions(+)
>   create mode 100644 lib/pdm-api-types/src/sdn.rs
>   create mode 100644 server/src/api/sdn/mod.rs
>   create mode 100644 server/src/api/sdn/zones.rs
> 
> diff --git a/lib/pdm-api-types/Cargo.toml b/lib/pdm-api-types/Cargo.toml
> index 26f7227..015df51 100644
> --- a/lib/pdm-api-types/Cargo.toml
> +++ b/lib/pdm-api-types/Cargo.toml
> @@ -23,3 +23,5 @@ proxmox-dns-api.workspace = true
>   proxmox-time.workspace = true
>   proxmox-serde.workspace = true
>   proxmox-subscription = { workspace = true, features = ["api-types"], default-features = false }
> +
> +pve-api-types = { workspace = true }
> diff --git a/lib/pdm-api-types/src/lib.rs b/lib/pdm-api-types/src/lib.rs
> index 8dbacba..07d18a9 100644
> --- a/lib/pdm-api-types/src/lib.rs
> +++ b/lib/pdm-api-types/src/lib.rs
> @@ -101,6 +101,8 @@ pub mod rrddata;
>   
>   pub mod subscription;
>   
> +pub mod sdn;
> +
>   const_regex! {
>       // just a rough check - dummy acceptor is used before persisting
>       pub OPENSSL_CIPHERS_REGEX = r"^[0-9A-Za-z_:, +!\-@=.]+$";
> diff --git a/lib/pdm-api-types/src/sdn.rs b/lib/pdm-api-types/src/sdn.rs
> new file mode 100644
> index 0000000..28b20c5
> --- /dev/null
> +++ b/lib/pdm-api-types/src/sdn.rs
> @@ -0,0 +1,24 @@
> +use proxmox_schema::{api, const_regex, ApiStringFormat, IntegerSchema, Schema, StringSchema};
> +use pve_api_types::SdnZone;
> +use serde::{Deserialize, Serialize};
> +
> +use crate::remotes::REMOTE_ID_SCHEMA;
> +
> +#[api(
> +    properties: {
> +        remote: {
> +            schema: REMOTE_ID_SCHEMA,
> +        },
> +        zone: {
> +            type: SdnZone,
> +            flatten: true,
> +        }
> +    }
> +)]
> +/// SDN controller with additional information about which remote it belongs to
> +#[derive(Serialize, Deserialize, Clone, PartialEq)]
> +pub struct ListZone {
> +    pub remote: String,
> +    #[serde(flatten)]
> +    pub zone: SdnZone,
> +}
> diff --git a/lib/pdm-client/src/lib.rs b/lib/pdm-client/src/lib.rs
> index f2bb546..5f7f18c 100644
> --- a/lib/pdm-client/src/lib.rs
> +++ b/lib/pdm-client/src/lib.rs
> @@ -8,6 +8,7 @@ use pdm_api_types::resource::{PveResource, RemoteResources, TopEntities};
>   use pdm_api_types::rrddata::{
>       LxcDataPoint, NodeDataPoint, PbsDatastoreDataPoint, PbsNodeDataPoint, QemuDataPoint,
>   };
> +use pdm_api_types::sdn::ListZone;
>   use pdm_api_types::BasicRealmInfo;
>   use pve_api_types::StartQemuMigrationType;
>   use serde::{Deserialize, Serialize};
> @@ -57,6 +58,8 @@ pub mod types {
>       pub use pve_api_types::ClusterNodeStatus;
>   
>       pub use pve_api_types::PveUpid;
> +
> +    pub use pve_api_types::ListZonesType;
>   }
>   
>   pub struct PdmClient<T: HttpApiClient>(pub T);
> @@ -966,6 +969,21 @@ impl<T: HttpApiClient> PdmClient<T> {
>               .expect_json()?
>               .data)
>       }
> +
> +    pub async fn pve_sdn_list_zones(
> +        &self,
> +        pending: impl Into<Option<bool>>,
> +        running: impl Into<Option<bool>>,
> +        ty: impl Into<Option<ListZonesType>>,
> +    ) -> Result<Vec<ListZone>, Error> {
> +        let path = ApiPathBuilder::new("/api2/extjs/sdn/zones".to_string())
> +            .maybe_arg("pending", &pending.into())
> +            .maybe_arg("running", &running.into())
> +            .maybe_arg("ty", &ty.into())
> +            .build();
> +
> +        Ok(self.0.get(&path).await?.expect_json()?.data)
> +    }
>   }
>   
>   /// Builder for migration parameters.
> diff --git a/server/src/api/mod.rs b/server/src/api/mod.rs
> index ff875fc..02ee0ec 100644
> --- a/server/src/api/mod.rs
> +++ b/server/src/api/mod.rs
> @@ -17,6 +17,7 @@ pub mod remote_tasks;
>   pub mod remotes;
>   pub mod resources;
>   mod rrd_common;
> +pub mod sdn;
>   
>   #[sortable]
>   const SUBDIRS: SubdirMap = &sorted!([
> @@ -30,6 +31,7 @@ const SUBDIRS: SubdirMap = &sorted!([
>       ("resources", &resources::ROUTER),
>       ("nodes", &nodes::ROUTER),
>       ("remote-tasks", &remote_tasks::ROUTER),
> +    ("sdn", &sdn::ROUTER),
>       ("version", &Router::new().get(&API_METHOD_VERSION)),
>   ]);
>   
> diff --git a/server/src/api/sdn/mod.rs b/server/src/api/sdn/mod.rs
> new file mode 100644
> index 0000000..2abdaf6
> --- /dev/null
> +++ b/server/src/api/sdn/mod.rs
> @@ -0,0 +1,13 @@
> +use proxmox_router::{list_subdirs_api_method, Router, SubdirMap};
> +use proxmox_sortable_macro::sortable;
> +
> +pub mod zones;
> +
> +#[sortable]
> +pub const SUBDIRS: SubdirMap = &sorted!([
> +    ("zones", &zones::ROUTER),
> +]);
> +
> +pub const ROUTER: Router = Router::new()
> +    .get(&list_subdirs_api_method!(SUBDIRS))
> +    .subdirs(SUBDIRS);
> diff --git a/server/src/api/sdn/zones.rs b/server/src/api/sdn/zones.rs
> new file mode 100644
> index 0000000..7d02076
> --- /dev/null
> +++ b/server/src/api/sdn/zones.rs
> @@ -0,0 +1,111 @@
> +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 proxmox_schema::api;
> +use pve_api_types::ListZonesType;
> +
> +use crate::{
> +    api::pve,
> +    parallel_fetcher::{NodeResults, ParallelFetcher},
> +    sdn_client::LockedSdnClients,
> +};
> +
> +pub const ROUTER: Router = Router::new().get(&API_METHOD_LIST_ZONES);
> +
> +#[api(
> +    input: {
> +        properties: {
> +            pending: {
> +                type: Boolean,
> +                optional: true,
> +                description: "Include a list of attributes whose changes are currently pending.",
> +            },
> +            running: {
> +                type: Boolean,
> +                optional: true,
> +                description: "If true shows the running configuration, otherwise the pending configuration.",
> +            },
> +            ty: {
> +                type: ListZonesType,
> +                optional: true,
> +            },
> +            remotes: {
> +                type: Array,
> +                optional: true,
> +                description: "Only return controllers from the specified remotes.",
> +                items: {
> +                    schema: REMOTE_ID_SCHEMA,
> +                }
> +            },
> +        }
> +    },
> +    returns: {
> +        type: Array,
> +        description: "Get a list of zones fitting the filtering criteria.",
> +        items: {
> +            type: ListZone,
> +        },
> +    },
> +)]
> +/// Query zones of remotes with optional filtering options
> +pub async fn list_zones(
> +    pending: Option<bool>,
> +    running: Option<bool>,
> +    ty: Option<ListZonesType>,
> +    remotes: Option<Vec<String>>,

later in the `list_vnet` patch, you use a `Option<HashSet<String>>` for 
the remotes, which i find a bit nicer, because it does deduplicate the 
remotes automatically.

Either way is fine, but we probably should be consistent

(can be fixed/changed as a follow up though)

> +) -> Result<Vec<ListZone>, Error> {
> +    let (remote_config, _) = pdm_config::remotes::config()?;
> +
> +    let filtered_remotes = remote_config.into_iter().filter_map(|(_, remote)| {
> +        if remote.ty == RemoteType::Pve
> +            && remotes
> +                .as_ref()
> +                .map(|remotes| remotes.contains(&remote.id))
> +                .unwrap_or(true)
> +        {
> +            return Some(remote);
> +        }
> +
> +        None
> +    });
> +
> +    let mut vnets = Vec::new();
> +    let fetcher = ParallelFetcher::new((pending, running, ty));
> +
> +    let results = fetcher
> +        .do_for_all_remotes(filtered_remotes, async |ctx, r, _| {
> +            Ok(pve::connect(&r)?.list_zones(ctx.0, ctx.1, ctx.2).await?)
> +        })
> +        .await;
> +
> +    for (remote, remote_result) in results.remote_results.into_iter() {
> +        match remote_result {
> +            Ok(remote_result) => {
> +                for (node, node_result) in remote_result.node_results.into_iter() {
> +                    match node_result {
> +                        Ok(NodeResults { data, .. }) => {
> +                            vnets.extend(data.into_iter().map(|zone| ListZone {
> +                                remote: remote.clone(),
> +                                zone,
> +                            }))
> +                        }
> +                        Err(error) => {
> +                            log::error!(
> +                                "could not fetch vnets from remote {} node {}: {error:#}",
> +                                remote,
> +                                node
> +                            );
> +                        }
> +                    }
> +                }
> +            }
> +            Err(error) => {
> +                log::error!("could not fetch vnets from remote {}: {error:#}", remote)
> +            }
> +        }
> +    }
> +
> +    Ok(vnets)
> +}



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