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

[Christian Ebner]

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.