[pdm-devel] [PATCH datacenter-manager 3/7] server: api: collect failed remotes list while getting status

[Dominik Csapak]

On 10/14/25 2:41 PM, Christian Ebner wrote:
> On 10/14/25 10:37 AM, Dominik Csapak wrote:
>> comments inline
>>
>> On 10/13/25 10:56 AM, Christian Ebner wrote:
>>> Include name, remote type and error message for failed remotes when
>>> gathering status information, in order to be able to discriminate
>>> errors by remote type for the dashboard. To get the per-remote-type
>>> resources, utilize the now previously exposed remote type filtering
>>> for resources.
>>>
>>> Signed-off-by: Christian Ebner <c.ebner at proxmox.com>
>>> ---
>>>   server/src/api/resources.rs | 105 ++++++++++++++++++++----------------
>>>   1 file changed, 58 insertions(+), 47 deletions(-)
>>>
>>> diff --git a/server/src/api/resources.rs b/server/src/api/resources.rs
>>> index 029106f..33da5c2 100644
>>> --- a/server/src/api/resources.rs
>>> +++ b/server/src/api/resources.rs
>>> @@ -9,9 +9,9 @@ use futures::FutureExt;
>>>   use pbs_api_types::{DataStoreStatusListItem, NodeStatus};
>>>   use pdm_api_types::remotes::{Remote, RemoteType};
>>>   use pdm_api_types::resource::{
>>> -    PbsDatastoreResource, PbsNodeResource, PveLxcResource, 
>>> PveNodeResource, PveQemuResource,
>>> -    PveSdnResource, PveStorageResource, RemoteResources, Resource, 
>>> ResourceType, ResourcesStatus,
>>> -    SdnStatus, SdnZoneResource, TopEntities,
>>> +    FailedRemote, PbsDatastoreResource, PbsNodeResource, 
>>> PveLxcResource, PveNodeResource,
>>> +    PveQemuResource, PveSdnResource, PveStorageResource, 
>>> RemoteResources, Resource, ResourceType,
>>> +    ResourcesStatus, SdnStatus, SdnZoneResource, TopEntities,
>>>   };
>>>   use pdm_api_types::subscription::{
>>>       NodeSubscriptionInfo, RemoteSubscriptionState, 
>>> RemoteSubscriptions, SubscriptionLevel,
>>> @@ -373,55 +373,66 @@ pub async fn get_status(
>>>       max_age: u64,
>>>       rpcenv: &mut dyn RpcEnvironment,
>>>   ) -> Result<ResourcesStatus, Error> {
>>> -    let remotes = get_resources(max_age, None, None, rpcenv).await?;
>>>       let mut counts = ResourcesStatus::default();
>>> -    for remote in remotes {
>>> -        if remote.error.is_some() {
>>> -            counts.failed_remotes += 1;
>>> -        } else {
>>> -            counts.remotes += 1;
>>> -        }
>>> -        for resource in remote.resources {
>>> -            match resource {
>>> -                Resource::PveStorage(r) => match r.status.as_str() {
>>> -                    "available" => counts.storages.available += 1,
>>> -                    _ => counts.storages.unknown += 1,
>>> -                },
>>> -                Resource::PveQemu(r) => match r.status.as_str() {
>>> -                    _ if r.template => counts.qemu.template += 1,
>>> -                    "running" => counts.qemu.running += 1,
>>> -                    "stopped" => counts.qemu.stopped += 1,
>>> -                    _ => counts.qemu.unknown += 1,
>>> -                },
>>> -                Resource::PveLxc(r) => match r.status.as_str() {
>>> -                    _ if r.template => counts.lxc.template += 1,
>>> -                    "running" => counts.lxc.running += 1,
>>> -                    "stopped" => counts.lxc.stopped += 1,
>>> -                    _ => counts.lxc.unknown += 1,
>>> -                },
>>> -                Resource::PveNode(r) => match r.status.as_str() {
>>> -                    "online" => counts.pve_nodes.online += 1,
>>> -                    "offline" => counts.pve_nodes.offline += 1,
>>> -                    _ => counts.pve_nodes.unknown += 1,
>>> -                },
>>> -                Resource::PveSdn(r) => {
>>> -                    if let PveSdnResource::Zone(_) = &r {
>>> -                        match r.status() {
>>> -                            SdnStatus::Available => {
>>> -                                counts.sdn_zones.available += 1;
>>> -                            }
>>> -                            SdnStatus::Error => {
>>> -                                counts.sdn_zones.error += 1;
>>> -                            }
>>> -                            SdnStatus::Unknown => {
>>> -                                counts.sdn_zones.unknown += 1;
>>> +    for remote_type in [RemoteType::Pve, RemoteType::Pbs] {
>>> +        let remote_type_search =
>>> + SearchTerm::new(remote_type.to_string()).category(Some("remote- 
>>> type"));
>>> +        let remote_type_search = remote_type_search.to_string();
>>> +        let remotes =
>>> +            get_resources_impl(max_age, Some(remote_type_search), 
>>> None, Some(rpcenv)).await?;
>>
>>
>> if the `RemoteResources` struct had the complete (or at least more than
>> the name) struct of the remote, we could keep the current structure of 
>> just looping over that result instead of manually crafting a search
>> for each type
>>
>> which would also make the patch diff itself a lot smaller i think
> 
> That will not work though? The whole point of looping over the remote 
> types here is to have it in case of an error state, allowing to add the 
> failed remote including it's type.
> 
> So adding that additional information to the RemoteResouces does not 
> solve that issue, and I would rather opt to keep the iteration here.

i meant that in `get_resources_impl` we already iterate over all remotes
to collect the info.

instead of just saving the name per remote we could add more info
(like the type) so

instead of
---8<---
  RemoteResources {
      remote: remote_name,
      resources,
      error,
  }
--->8---

we could do

---8<---
  RemoteResources {
      remote,
      resources,
      error,
  }
--->8---

where `remote` is some struct that also holds the remote-type info?

then we can simply loop over these and extract the type directly from
that


or am I missing something here?