From: "Lukas Wagner" <l.wagner@proxmox.com>
To: "Proxmox Datacenter Manager development discussion"
<pdm-devel@lists.proxmox.com>
Cc: "pdm-devel" <pdm-devel-bounces@lists.proxmox.com>
Subject: Re: [pdm-devel] [RFC PATCH datacenter-manager 3/3] api/ui: pve: get cluster resources from cache if remote is unreachable
Date: Thu, 16 Oct 2025 11:09:24 +0200 [thread overview]
Message-ID: <DDJMNMFP4110.2P9D4T707RWH8@proxmox.com> (raw)
In-Reply-To: <20251013083806.1068917-4-d.csapak@proxmox.com>
On Mon Oct 13, 2025 at 10:33 AM CEST, Dominik Csapak wrote:
> so that the last cached data can be shown instead of an error.
> This now means that we'll always use the full api call here and filter
> on the PDM side to get a fresh version in the cache.
>
> The 'kind' parameter is not sent to the pve backend anymore but it's
> filtered on the pdm side.
>
> To show a notice in the ui that it was loaded from the cache, we need to
> add a marker property in the result, and need to extract that in the
> pdm-client so the gui has access to that.
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> sending as rfc because
> * it changes how we handle the 'kind' parameter
> * we may want to opt in to that behaviour?
> * not sure if a result side channel is the best way to convey the
> 'from-cache' state.
I think think this should be part of the API response type, the
'attribs' mechanic feels a bit too magic for me.
So instead of return a Vec<PveResource>, maybe a PveResourceList
#[api]
struct PveResourceList {
resources: Vec<PveResource>,
cached: bool,
cache_timestamp: ... // Could be useful as well
}
Recently I've started to avoid returning a bare Vec<...> from API
endpoints in general, basically exactly for cases like these, where we
want to augment the endpoint with additional data later on.
>
> lib/pdm-client/src/lib.rs | 10 ++++++--
> server/src/api/pve/mod.rs | 48 ++++++++++++++++++++++++-------------
> server/src/api/resources.rs | 20 ++++++++++++++++
> ui/src/pve/mod.rs | 14 +++++++----
> 4 files changed, 70 insertions(+), 22 deletions(-)
>
> diff --git a/lib/pdm-client/src/lib.rs b/lib/pdm-client/src/lib.rs
> index 2f36fab1..3ebb1c35 100644
> --- a/lib/pdm-client/src/lib.rs
> +++ b/lib/pdm-client/src/lib.rs
> @@ -408,11 +408,17 @@ impl<T: HttpApiClient> PdmClient<T> {
> &self,
> remote: &str,
> kind: Option<pve_api_types::ClusterResourceKind>,
> - ) -> Result<Vec<PveResource>, Error> {
> + ) -> Result<(bool, Vec<PveResource>), Error> {
> let query = ApiPathBuilder::new(format!("/api2/extjs/pve/remotes/{remote}/resources"))
> .maybe_arg("kind", &kind)
> .build();
> - Ok(self.0.get(&query).await?.expect_json()?.data)
> + let api_response_data = self.0.get(&query).await?.expect_json()?;
> + let from_cache = api_response_data
> + .attribs
> + .get("from-cache")
> + .and_then(|val| val.as_bool())
> + .unwrap_or(false);
> + Ok((from_cache, api_response_data.data))
> }
>
> pub async fn pve_cluster_status(
> diff --git a/server/src/api/pve/mod.rs b/server/src/api/pve/mod.rs
> index 0de82323..c16d6de5 100644
> --- a/server/src/api/pve/mod.rs
> +++ b/server/src/api/pve/mod.rs
> @@ -16,19 +16,18 @@ use proxmox_sortable_macro::sortable;
> use pdm_api_types::remotes::{
> NodeUrl, Remote, RemoteListEntry, RemoteType, TlsProbeOutcome, REMOTE_ID_SCHEMA,
> };
> -use pdm_api_types::resource::PveResource;
> +use pdm_api_types::resource::{PveResource, Resource};
> use pdm_api_types::{
> Authid, RemoteUpid, HOST_OPTIONAL_PORT_FORMAT, PRIV_RESOURCE_AUDIT, PRIV_RESOURCE_DELETE,
> PRIV_SYS_MODIFY,
> };
>
> use pve_api_types::ClusterNodeStatus;
> +use pve_api_types::ClusterResourceKind;
> use pve_api_types::ListRealm;
> use pve_api_types::PveUpid;
> -use pve_api_types::{ClusterResourceKind, ClusterResourceType};
> -
> -use super::resources::{map_pve_lxc, map_pve_node, map_pve_qemu, map_pve_storage};
>
> +use crate::api::resources::get_updated_resources_or_cache;
> use crate::connection::PveClient;
> use crate::connection::{self, probe_tls_connection};
> use crate::remote_tasks;
> @@ -202,13 +201,29 @@ pub async fn cluster_resources(
> http_bail!(UNAUTHORIZED, "user has no access to resource list");
> }
>
> - let cluster_resources = connect_to_remote(&remotes, &remote)?
> - .cluster_resources(kind)
> - .await?
> + let remote = get_remote(&remotes, &remote)?;
> +
> + let (from_cache, cluster_resources) = get_updated_resources_or_cache(remote).await?;
> +
> + rpcenv.result_attrib_mut()["from-cache"] = from_cache.into();
> +
> + let cluster_resources = cluster_resources
> .into_iter()
> - .filter_map(|r| map_pve_resource(&remote, r));
> + .filter(|res| match kind {
> + Some(kind) => matches!(
> + (kind, res),
> + (ClusterResourceKind::Vm, Resource::PveQemu(_))
> + | (ClusterResourceKind::Vm, Resource::PveLxc(_))
> + | (ClusterResourceKind::Storage, Resource::PveStorage(_))
> + | (ClusterResourceKind::Node, Resource::PveNode(_))
> + | (ClusterResourceKind::Sdn, Resource::PveSdn(_))
> + ),
> + None => true,
> + })
> + .filter_map(map_pve_resource)
> + .collect();
>
> - Ok(cluster_resources.collect())
> + Ok(cluster_resources)
> }
>
> #[api(
> @@ -245,13 +260,14 @@ pub async fn cluster_status(
> Ok(status)
> }
>
> -fn map_pve_resource(remote: &str, resource: pve_api_types::ClusterResource) -> Option<PveResource> {
> - match resource.ty {
> - ClusterResourceType::Node => map_pve_node(remote, resource).map(PveResource::Node),
> - ClusterResourceType::Lxc => map_pve_lxc(remote, resource).map(PveResource::Lxc),
> - ClusterResourceType::Qemu => map_pve_qemu(remote, resource).map(PveResource::Qemu),
> - ClusterResourceType::Storage => map_pve_storage(remote, resource).map(PveResource::Storage),
> - _ => None,
> +fn map_pve_resource(resource: Resource) -> Option<PveResource> {
> + match resource {
> + Resource::PveStorage(storage) => Some(PveResource::Storage(storage)),
> + Resource::PveQemu(qemu) => Some(PveResource::Qemu(qemu)),
> + Resource::PveLxc(lxc) => Some(PveResource::Lxc(lxc)),
> + Resource::PveNode(node) => Some(PveResource::Node(node)),
> + Resource::PveSdn(sdn) => Some(PveResource::Sdn(sdn)),
> + Resource::PbsNode(_) | Resource::PbsDatastore(_) => None,
> }
> }
>
> diff --git a/server/src/api/resources.rs b/server/src/api/resources.rs
> index 5b7ac524..5b876998 100644
> --- a/server/src/api/resources.rs
> +++ b/server/src/api/resources.rs
> @@ -645,6 +645,26 @@ async fn get_resources_for_remote(remote: &Remote, max_age: u64) -> Result<Vec<R
> .await
> }
>
> +/// Tries to update the resources and returns data from the cache if it was not
> +/// possible to reach the remote. This is indacted in the return value when
> +/// `(true, data)` is returned.
> +pub async fn get_updated_resources_or_cache(
> + remote: &Remote,
> +) -> Result<(bool, Vec<Resource>), Error> {
> + match CACHE
> + .get_updated_value(&remote.id, 0, |_| async move {
> + fetch_remote_resource(remote).await
> + })
> + .await
> + {
> + Ok(data) => Ok((false, data)),
> + Err(err) => match CACHE.get_cached_value(&remote.id, i64::MAX) {
> + Some(data) => Ok((true, data)),
> + None => Err(err.context("cache was empty and could not update")),
> + },
> + }
> +}
> +
> /// Read cached resource data from the cache
> pub fn get_cached_resources(remote: &str, max_age: u64) -> Option<Vec<Resource>> {
> CACHE.get_cached_value(remote, max_age as i64)
> diff --git a/ui/src/pve/mod.rs b/ui/src/pve/mod.rs
> index 058424a9..2cdc99f7 100644
> --- a/ui/src/pve/mod.rs
> +++ b/ui/src/pve/mod.rs
> @@ -11,7 +11,7 @@ use yew::{
> use pwt::{
> css::AlignItems,
> state::NavigationContainer,
> - widget::{Button, Container, Fa},
> + widget::{error_message, Button, Container, Fa},
> };
> use pwt::{
> css::FlexFit,
> @@ -122,13 +122,14 @@ impl GuestInfo {
>
> pub enum Msg {
> SelectedView(tree::PveTreeNode),
> - ResourcesList(Result<Vec<PveResource>, Error>),
> + ResourcesList(Result<(bool, Vec<PveResource>), Error>),
> }
>
> pub struct PveRemoteComp {
> view: tree::PveTreeNode,
> resources: Rc<Vec<PveResource>>,
> last_error: Option<String>,
> + from_cache: bool,
> }
>
> impl LoadableComponent for PveRemoteComp {
> @@ -142,6 +143,7 @@ impl LoadableComponent for PveRemoteComp {
> view: PveTreeNode::Root,
> resources: Rc::new(Vec::new()),
> last_error: None,
> + from_cache: false,
> }
> }
>
> @@ -151,8 +153,9 @@ impl LoadableComponent for PveRemoteComp {
> self.view = node;
> }
> Msg::ResourcesList(res) => match res {
> - Ok(res) => {
> + Ok((from_cache, res)) => {
> self.last_error = None;
> + self.from_cache = from_cache;
> self.resources = Rc::new(res);
> }
> Err(err) => {
> @@ -245,7 +248,10 @@ impl LoadableComponent for PveRemoteComp {
> let link = link.clone();
> move |_| link.send_reload()
> },
> - )),
> + ))
> + .with_optional_child(self.from_cache.then_some(error_message(&tr!(
> + "Could not reach remote, data shown from cache."
> + )))),
> ),
> )
> .with_child(
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
prev parent reply other threads:[~2025-10-16 9:09 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-10-13 8:33 [pdm-devel] [PATCH datacenter-manager 0/3] refactor api caching & try to reuse the cache Dominik Csapak
2025-10-13 8:33 ` [pdm-devel] [PATCH datacenter-manager 1/3] server: api: resources: change use of remote to reference Dominik Csapak
2025-10-13 8:33 ` [pdm-devel] [PATCH datacenter-manager 2/3] server: introduce ApiCache abstraction Dominik Csapak
2025-10-16 9:09 ` Lukas Wagner
2025-10-13 8:33 ` [pdm-devel] [RFC PATCH datacenter-manager 3/3] api/ui: pve: get cluster resources from cache if remote is unreachable Dominik Csapak
2025-10-16 9:09 ` Lukas Wagner [this message]
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=DDJMNMFP4110.2P9D4T707RWH8@proxmox.com \
--to=l.wagner@proxmox.com \
--cc=pdm-devel-bounces@lists.proxmox.com \
--cc=pdm-devel@lists.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