[pdm-devel] [RFC PATCH datacenter-manager 0/2] use streaming content api

[pdm-devel] [RFC PATCH datacenter-manager 0/2] use streaming content api

[Dominik Csapak]

instead of using the snapshot list + a namespace selector, use the
content api call [0] to show the whole tree.

Sending as RFC because there are some crucial parts missing in the ui
(see the commit message of 2/2) but I wanted to get early feedback and
send out some patches so one can test the streaming content api call
in the pdm ui.

0: https://lore.proxmox.com/pbs-devel/20251008134344.3512958-1-d.csapak@proxmox.com/

Dominik Csapak (2):
  server: add new streaming 'content' api call for pbs
  ui: pbs: snapshot list: change to streaming 'content' api call

 server/src/api/pbs/mod.rs   |  40 +++++++++-
 server/src/pbs_client.rs    |  51 +++++++++++++
 ui/src/pbs/snapshot_list.rs | 148 +++++++++++++++++++++++++-----------
 3 files changed, 193 insertions(+), 46 deletions(-)

-- 
2.47.3



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

[pdm-devel] [RFC PATCH datacenter-manager 1/2] server: add new streaming 'content' api call for pbs

[Dominik Csapak]

that makes use of 'application/json-seq' mime type to stream data from
pbs to pdm, removing the need to buffer the data on pdm in the api call.

The pbs-client code is more or less a copy of the current snapshot call
list, but if needed this method can be refactored.

This requires the new 'content' api call for pbs [0]

0: https://lore.proxmox.com/pbs-devel/20251008134344.3512958-1-d.csapak@proxmox.com/

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 server/src/api/pbs/mod.rs | 40 +++++++++++++++++++++++++++++-
 server/src/pbs_client.rs  | 51 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 90 insertions(+), 1 deletion(-)

diff --git a/server/src/api/pbs/mod.rs b/server/src/api/pbs/mod.rs
index dc31f620..6e53ce6f 100644
--- a/server/src/api/pbs/mod.rs
+++ b/server/src/api/pbs/mod.rs
@@ -1,7 +1,7 @@
 use anyhow::{format_err, Error};
 use futures::StreamExt;
 
-use proxmox_router::{list_subdirs_api_method, Permission, Router, SubdirMap};
+use proxmox_router::{list_subdirs_api_method, Permission, Record, Router, SubdirMap};
 use proxmox_schema::api;
 use proxmox_schema::property_string::PropertyString;
 use proxmox_sortable_macro::sortable;
@@ -63,6 +63,7 @@ const DATASTORE_ITEM_SUBDIRS: SubdirMap = &sorted!([
         "snapshots",
         &Router::new().get(&API_METHOD_LIST_SNAPSHOTS_2)
     ),
+    ("content", &Router::new().get(&API_METHOD_LIST_CONTENT)),
 ]);
 
 #[api(
@@ -175,6 +176,43 @@ async fn list_snapshots_2(
     .into())
 }
 
+#[api(
+    stream: true,
+    input: {
+        properties: {
+            remote: { schema: REMOTE_ID_SCHEMA },
+            datastore: { schema: pbs_api_types::DATASTORE_SCHEMA },
+            ns: {
+                schema: pbs_api_types::BACKUP_NAMESPACE_SCHEMA,
+                optional: true,
+            },
+            "max-depth": {
+                schema: pbs_api_types::NS_MAX_DEPTH_SCHEMA,
+                optional: true,
+            }
+        },
+    },
+    returns: pbs_api_types::ADMIN_DATASTORE_LIST_CONTENT_RETURN_TYPE,
+    access: {
+        permission: &Permission::Privilege(&["resource", "{remote}", "datastore", "{datastore}"], PRIV_RESOURCE_AUDIT, false),
+    },
+)]
+/// List the PBS remote's datastores.
+async fn list_content(
+    remote: String,
+    datastore: String,
+    ns: Option<String>,
+    max_depth: Option<usize>,
+) -> Result<proxmox_router::Stream, Error> {
+    let (remotes, _) = pdm_config::remotes::config()?;
+    let remote = get_remote(&remotes, &remote)?;
+    let snapshots = connection::make_pbs_client(remote)?
+        .list_content(&datastore, ns.as_deref(), max_depth)
+        .await?
+        .map(Record::from_result);
+    Ok(snapshots.into())
+}
+
 #[api(
     input: {
         properties: {
diff --git a/server/src/pbs_client.rs b/server/src/pbs_client.rs
index d8278c8a..76041090 100644
--- a/server/src/pbs_client.rs
+++ b/server/src/pbs_client.rs
@@ -190,6 +190,57 @@ impl PbsClient {
         }
     }
 
+    /// List a datastore's content.
+    pub async fn list_content(
+        &self,
+        datastore: &str,
+        namespace: Option<&str>,
+        max_depth: Option<usize>,
+    ) -> Result<JsonRecords<pbs_api_types::DatastoreContent>, anyhow::Error> {
+        let path = ApiPathBuilder::new(format!("/api2/json/admin/datastore/{datastore}/content"))
+            .maybe_arg("ns", &namespace)
+            .maybe_arg("max-depth", &max_depth)
+            .build();
+        let response = self
+            .0
+            .streaming_request(http::Method::GET, &path, None::<()>)
+            .await?;
+
+        let body = response
+            .body
+            .ok_or_else(|| Error::Other("missing response body"))?;
+
+        if response.status == 200 {
+            if response
+                .content_type
+                .is_some_and(|c| c.starts_with("application/json-seq"))
+            {
+                Ok(JsonRecords::from_body(body))
+            } else {
+                let response: JsonData<_> = serde_json::from_slice(
+                    &body
+                        .collect()
+                        .await
+                        .map_err(|err| {
+                            Error::Anyhow(Box::new(err).context("failed to retrieve response body"))
+                        })?
+                        .to_bytes(),
+                )?;
+                Ok(JsonRecords::from_vec(response.data))
+            }
+        } else {
+            let data = body
+                .collect()
+                .await
+                .map_err(|err| {
+                    Error::Anyhow(Box::new(err).context("failed to retrieve response body"))
+                })?
+                .to_bytes();
+            let error = String::from_utf8_lossy(&data).into_owned();
+            Err(anyhow::Error::msg(error))
+        }
+    }
+
     /// create an API-Token on the PBS remote and give the token admin ACL on everything.
     pub async fn create_admin_token(
         &self,
-- 
2.47.3



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

[pdm-devel] [RFC PATCH datacenter-manager 2/2] ui: pbs: snapshot list: change to streaming 'content' api call

[Dominik Csapak]

this changes the logic to add elements to the tree a bit, since we now
also show the namespaces in a true tree fashion.

We also have to adapt the sorting logic a bit, since now groups can sit
alongside namespaces.

We also collect all streaming errors we encounter and show a counter on
the bottom.

There are several things to improve here still:
* show the streaming errors in a popup when clicking on the message
* construct tree nodes on demand if the elements come out of order, or a
  snapshot is listed without it's corresponding group, etc.
* add a 'cancel' button for long running requests
* change the namespace selection, either:
  - add a max-depth selection too with a sensible default
  - make the list load only one level at a time and fetch additional
    levels on expanding a namespace
  - simply remove the namespace selector

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 ui/src/pbs/snapshot_list.rs | 148 +++++++++++++++++++++++++-----------
 1 file changed, 103 insertions(+), 45 deletions(-)

diff --git a/ui/src/pbs/snapshot_list.rs b/ui/src/pbs/snapshot_list.rs
index b82ffd5c..9f82054e 100644
--- a/ui/src/pbs/snapshot_list.rs
+++ b/ui/src/pbs/snapshot_list.rs
@@ -21,9 +21,11 @@ use pwt::widget::data_table::{DataTable, DataTableColumn, DataTableHeader};
 use pwt::widget::{error_message, Column, Container, Fa, Progress, Toolbar, Tooltip};
 use pwt::{css, AsyncPool};
 
-use pbs_api_types::{BackupGroup, BackupNamespace, BackupType, SnapshotListItem, VerifyState};
+use pbs_api_types::{
+    BackupGroup, BackupNamespace, BackupType, DatastoreContent, SnapshotListItem, VerifyState,
+};
 
-use proxmox_yew_comp::http_stream::Stream;
+use proxmox_yew_comp::http_stream::{Record, Stream};
 
 use crate::locale_compare;
 use crate::pbs::namespace_selector::NamespaceSelector;
@@ -59,16 +61,20 @@ struct SnapshotVerifyCount {
 #[derive(PartialEq, Clone)]
 enum SnapshotTreeEntry {
     Root(BackupNamespace),
-    Group(BackupGroup, SnapshotVerifyCount),
-    Snapshot(SnapshotListItem),
+    Group(BackupNamespace, BackupGroup, SnapshotVerifyCount),
+    Snapshot(BackupNamespace, SnapshotListItem),
 }
 
 impl ExtractPrimaryKey for SnapshotTreeEntry {
     fn extract_key(&self) -> Key {
         match self {
             SnapshotTreeEntry::Root(namespace) => Key::from(format!("root+{namespace}")),
-            SnapshotTreeEntry::Group(group, _) => Key::from(format!("group+{group}")),
-            SnapshotTreeEntry::Snapshot(entry) => Key::from(entry.backup.to_string()),
+            SnapshotTreeEntry::Group(namespace, group, _) => {
+                Key::from(format!("ns+{namespace}+group+{group}"))
+            }
+            SnapshotTreeEntry::Snapshot(namespace, entry) => {
+                Key::from(format!("ns+{namespace}+snap+{}", entry.backup))
+            }
         }
     }
 }
@@ -77,7 +83,7 @@ impl ExtractPrimaryKey for SnapshotTreeEntry {
 enum Msg {
     SelectionChange,
     ConsumeBuffer,
-    UpdateBuffer(SnapshotListItem),
+    UpdateBuffer(Record<DatastoreContent>),
     UpdateParentNamespace(Key),
     Reload,
     LoadFinished(Result<(), Error>),
@@ -89,7 +95,8 @@ struct SnapshotListComp {
     _async_pool: AsyncPool,
     columns: Rc<Vec<DataTableHeader<SnapshotTreeEntry>>>,
     load_result: Option<Result<(), Error>>,
-    buffer: Vec<SnapshotListItem>,
+    buffer: Vec<DatastoreContent>,
+    errors: Vec<serde_json::Value>,
     current_namespace: BackupNamespace,
     interval: Option<Interval>,
 }
@@ -109,7 +116,7 @@ impl SnapshotListComp {
                                 ("object-group", tr!("Namespace '{0}'", namespace))
                             }
                         }
-                        SnapshotTreeEntry::Group(group, _) => (
+                        SnapshotTreeEntry::Group(_, group, _) => (
                             match group.ty {
                                 BackupType::Vm => "desktop",
                                 BackupType::Ct => "cube",
@@ -117,7 +124,9 @@ impl SnapshotListComp {
                             },
                             group.to_string(),
                         ),
-                        SnapshotTreeEntry::Snapshot(entry) => ("file-o", entry.backup.to_string()),
+                        SnapshotTreeEntry::Snapshot(_, entry) => {
+                            ("file-o", entry.backup.to_string())
+                        }
                     };
                     render_tree_column(Fa::new(icon).fixed_width().into(), res).into()
                 })
@@ -126,10 +135,10 @@ impl SnapshotListComp {
                 .justify("right")
                 .render(|item: &SnapshotTreeEntry| match item {
                     SnapshotTreeEntry::Root(_) => "".into(),
-                    SnapshotTreeEntry::Group(_group, counts) => {
+                    SnapshotTreeEntry::Group(_ns, _group, counts) => {
                         (counts.ok + counts.failed + counts.none).into()
                     }
-                    SnapshotTreeEntry::Snapshot(_entry) => "".into(),
+                    SnapshotTreeEntry::Snapshot(_ns, _entry) => "".into(),
                 })
                 .into(),
             DataTableColumn::new(tr!("Verify State"))
@@ -145,6 +154,8 @@ impl SnapshotListComp {
             .write()
             .set_root(SnapshotTreeEntry::Root(self.current_namespace.clone()));
         self._async_pool = AsyncPool::new();
+        self.buffer.clear();
+        self.errors.clear();
         self.reload(ctx);
     }
 
@@ -193,6 +204,7 @@ impl Component for SnapshotListComp {
             _async_pool: AsyncPool::new(),
             load_result: None,
             buffer: Vec::new(),
+            errors: Vec::new(),
             current_namespace: BackupNamespace::root(),
             interval: None,
         };
@@ -216,50 +228,90 @@ impl Component for SnapshotListComp {
                 }
                 let now = (Date::now() / 1000.0) as i64;
 
+                // TODO: handle out of order items
+                // we have to create the intermediate tree nodes if items are received out of order
+
                 for item in data {
-                    let group = item.backup.group.to_string();
-                    let mut group = if let Some(group) =
-                        root.find_node_by_key_mut(&Key::from(format!("group+{group}")))
-                    {
-                        group
-                    } else {
-                        root.append(SnapshotTreeEntry::Group(
-                            item.backup.group.clone(),
-                            Default::default(),
-                        ))
-                    };
-                    if let SnapshotTreeEntry::Group(_, verify_state) = group.record_mut() {
-                        match item.verification.as_ref() {
-                            Some(state) => {
-                                match state.state {
-                                    VerifyState::Ok => verify_state.ok += 1,
-                                    VerifyState::Failed => verify_state.failed += 1,
+                    let (ns, item) = match item {
+                        DatastoreContent::NameSpace(ns) => {
+                            let ns = ns.ns;
+                            let ns_entry = SnapshotTreeEntry::Root(ns.clone());
+                            if root.find_node_by_key_mut(&ns_entry.extract_key()).is_some() {
+                                // already inserted
+                                continue;
+                            }
+
+                            let parent = SnapshotTreeEntry::Root(ns.parent());
+                            if let Some(mut parent) =
+                                root.find_node_by_key_mut(&parent.extract_key())
+                            {
+                                parent.append(ns_entry);
+                            }
+                            continue;
+                        }
+                        DatastoreContent::Group(group) => {
+                            let ns = SnapshotTreeEntry::Root(group.ns.clone());
+                            if let Some(mut ns) = root.find_node_by_key_mut(&ns.extract_key()) {
+                                let group_entry = SnapshotTreeEntry::Group(
+                                    group.ns,
+                                    group.group.backup,
+                                    Default::default(),
+                                );
+                                if ns.find_node_by_key(&group_entry.extract_key()).is_none() {
+                                    ns.append(group_entry);
                                 }
+                            }
+
+                            continue;
+                        }
+                        DatastoreContent::Snapshot(snapshot) => (snapshot.ns, snapshot.snapshot),
+                    };
 
-                                let age_days = (now - state.upid.starttime) / (30 * 24 * 60 * 60);
-                                if age_days > 30 {
-                                    verify_state.outdated += 1;
+                    let group = SnapshotTreeEntry::Group(
+                        ns.clone(),
+                        item.backup.group.clone(),
+                        Default::default(),
+                    );
+                    if let Some(mut group) = root.find_node_by_key_mut(&group.extract_key()) {
+                        if let SnapshotTreeEntry::Group(_, _, verify_state) = group.record_mut() {
+                            match item.verification.as_ref() {
+                                Some(state) => {
+                                    match state.state {
+                                        VerifyState::Ok => verify_state.ok += 1,
+                                        VerifyState::Failed => verify_state.failed += 1,
+                                    }
+
+                                    let age_days =
+                                        (now - state.upid.starttime) / (30 * 24 * 60 * 60);
+                                    if age_days > 30 {
+                                        verify_state.outdated += 1;
+                                    }
                                 }
+                                None => verify_state.none += 1,
                             }
-                            None => verify_state.none += 1,
                         }
-                    }
-                    group.append(SnapshotTreeEntry::Snapshot(item));
+                        group.append(SnapshotTreeEntry::Snapshot(ns, item));
+                    };
                 }
 
                 store.sort_by(true, |a, b| match (a, b) {
-                    (SnapshotTreeEntry::Group(a, _), SnapshotTreeEntry::Group(b, _)) => {
+                    (SnapshotTreeEntry::Root(a), SnapshotTreeEntry::Root(b)) => a.cmp(b),
+                    (SnapshotTreeEntry::Root(_), _) => std::cmp::Ordering::Less,
+                    (SnapshotTreeEntry::Group(_, a, _), SnapshotTreeEntry::Group(_, b, _)) => {
                         locale_compare(a.to_string(), &b.to_string(), true)
                     }
-                    (SnapshotTreeEntry::Snapshot(a), SnapshotTreeEntry::Snapshot(b)) => {
+                    (SnapshotTreeEntry::Snapshot(_, a), SnapshotTreeEntry::Snapshot(_, b)) => {
                         a.backup.cmp(&b.backup)
                     }
-                    _ => std::cmp::Ordering::Less,
+                    _ => std::cmp::Ordering::Equal,
                 });
                 true
             }
             Msg::UpdateBuffer(item) => {
-                self.buffer.push(item);
+                match item {
+                    Record::Data(data) => self.buffer.push(data),
+                    Record::Error(err) => self.errors.push(err),
+                }
                 false
             }
             Msg::UpdateParentNamespace(ns_key) => {
@@ -294,6 +346,11 @@ impl Component for SnapshotListComp {
             _ => None,
         };
 
+        let err_count = self.errors.len();
+        let streaming_err = (err_count > 0).then_some(error_message(&tr!(
+            "One error during streaming" | "{n} errors during streaming" % err_count
+        )));
+
         let link = ctx.link();
 
         let props = ctx.props();
@@ -333,6 +390,7 @@ impl Component for SnapshotListComp {
                     .selection(self.selection.clone()),
             )
             .with_optional_child(err.map(|err| error_message(&err.to_string())))
+            .with_optional_child(streaming_err)
             .into()
     }
 }
@@ -341,12 +399,12 @@ async fn list_snapshots(
     remote: String,
     datastore: String,
     namespace: BackupNamespace,
-    callback: yew::Callback<SnapshotListItem>,
+    callback: yew::Callback<Record<DatastoreContent>>,
 ) -> Result<(), Error> {
     let path = if namespace.is_root() {
-        format!("/api2/json/pbs/remotes/{remote}/datastore/{datastore}/snapshots")
+        format!("/api2/json/pbs/remotes/{remote}/datastore/{datastore}/content")
     } else {
-        format!("/api2/json/pbs/remotes/{remote}/datastore/{datastore}/snapshots?ns={namespace}")
+        format!("/api2/json/pbs/remotes/{remote}/datastore/{datastore}/content?ns={namespace}")
     };
 
     // TODO: refactor application/json-seq helper for general purpose use
@@ -368,7 +426,7 @@ async fn list_snapshots(
 
     let mut stream = Stream::try_from(raw_reader)?;
 
-    while let Some(entry) = stream.next::<pbs_api_types::SnapshotListItem>().await? {
+    while let Some(entry) = stream.next::<Record<DatastoreContent>>().await? {
         callback.emit(entry);
     }
 
@@ -379,7 +437,7 @@ fn render_verification(entry: &SnapshotTreeEntry) -> Html {
     let now = (Date::now() / 1000.0) as i64;
     match entry {
         SnapshotTreeEntry::Root(_) => "".into(),
-        SnapshotTreeEntry::Group(_, verify_state) => {
+        SnapshotTreeEntry::Group(_, _, verify_state) => {
             let text;
             let icon_class;
             let tip;
@@ -430,7 +488,7 @@ fn render_verification(entry: &SnapshotTreeEntry) -> Html {
                 .tip(tip)
                 .into()
         }
-        SnapshotTreeEntry::Snapshot(entry) => match &entry.verification {
+        SnapshotTreeEntry::Snapshot(_, entry) => match &entry.verification {
             Some(state) => {
                 let age_days = (now - state.upid.starttime) / (30 * 24 * 60 * 60);
                 let (text, icon_class, class) = match state.state {
-- 
2.47.3



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