Re: [pdm-devel] [PATCH datacenter-manager 2/3] ui: make NodeStatusPanel available as a widget

["Lukas Wagner" <l.wagner@proxmox.com>]

On Thu Nov 6, 2025 at 10:10 PM CET, Thomas Lamprecht wrote:
> Am 06.11.25 um 13:43 schrieb Shannon Sterz:
>> Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
>> ---
>>  ui/src/administration/mod.rs |  4 +++-
>>  ui/src/dashboard/types.rs    |  1 +
>>  ui/src/dashboard/view.rs     | 15 ++++++++++++++-
>>  3 files changed, 18 insertions(+), 2 deletions(-)
>> 
>> diff --git a/ui/src/administration/mod.rs b/ui/src/administration/mod.rs
>> index a9f7ac6..72a4ffa 100644
>> --- a/ui/src/administration/mod.rs
>> +++ b/ui/src/administration/mod.rs
>> @@ -16,7 +16,9 @@ use pwt_macros::builder;
>>  //mod services;
>>  //pub use services::Services;
>> 
>> -use proxmox_yew_comp::{AptPackageManager, AptRepositories, ExistingProduct, Syslog, Tasks};
>> +use proxmox_yew_comp::{
>> +    AptPackageManager, AptRepositories, ExistingProduct, NodeStatusPanel, Syslog, Tasks,
>> +};
>> 
>>  #[derive(Clone, PartialEq, Properties)]
>>  #[builder]
>> diff --git a/ui/src/dashboard/types.rs b/ui/src/dashboard/types.rs
>> index c79c38a..ce4bbb7 100644
>> --- a/ui/src/dashboard/types.rs
>> +++ b/ui/src/dashboard/types.rs
>> @@ -61,6 +61,7 @@ pub enum WidgetType {
>>      TaskSummary {
>>          grouping: TaskSummaryGrouping,
>>      },
>> +    NodeStatus,
>>  }
>> 
>>  #[derive(Serialize, Deserialize, PartialEq, Clone, Copy)]
>> diff --git a/ui/src/dashboard/view.rs b/ui/src/dashboard/view.rs
>> index c781d99..b5cd722 100644
>> --- a/ui/src/dashboard/view.rs
>> +++ b/ui/src/dashboard/view.rs
>> @@ -3,6 +3,7 @@ use std::rc::Rc;
>>  use anyhow::Error;
>>  use futures::join;
>>  use js_sys::Date;
>> +use proxmox_yew_comp::NodeStatusPanel;
>>  use serde_json::json;
>>  use yew::virtual_dom::{VComp, VNode};
>> 
>> @@ -123,6 +124,11 @@ fn render_widget(
>>              let (hours, since) = get_task_options(refresh_config.task_last_hours);
>>              create_task_summary_panel(statistics, remotes, hours, since)
>>          }
>> +        WidgetType::NodeStatus => {
>> +            return NodeStatusPanel::new()
>> +                .status_base_url("/nodes/localhost/status")
>> +                .into()
>> +        }
>>      };
>> 
>
> While it might be OK to have as widget for remote node status in the dashboard, I
> would not add the one from PDM here, that's IMO confusing, especially if it's
> nowhere stated that it is in fact the one from PDM itself, given that all other
> widgets show the status of remote resources.
> I.e., the PBS dashboard is for the PBS instance itself, the PDM one is mainly for
> the remote resources, they need to be handled a bit differently when deciding what
> to add where, or at least ensure that it's clear that some info is from PDM itself.
>
> But rather lets add a new tab to the Administration panel, there we can then also
> add metrics graphs (memory, cpu, ...) for the local PDM itself in the future.
> Could be still nice to implement a PDM specific overview for the dashboard, but
> that should be probably something new with the specifics of PDM in mind, and can
> be done separately.
> Reboot/shutdown could then go in the toolbar there.
>

+1 

I quickly tried this patch series out yesterday before going home, my
findings/thoughts were

- As you mentioned, the fact that this widget is about the PDM host
  itself is not super clear (no mention of it in the title, and all the
  other widgets are about remotes)

- The widget appears a bit too dense/crowded, at least compared to the
  other widgets that we already have. I think this mostly due to the
  three buttons, as well as the long kernel version string, on my PDM
  dev host this is

	  Linux 6.14.8-2-pve #1 SMP PREEMPT_DYNAMIC PMX 6.14.8-2 (2025-07-22T10:04Z)

  which appears to be quite a bit longer than what we show in PVE/PBS:

	  Linux 6.14.11-4-pve (2025-10-10T08:04Z)

- Having the potentially disruptive Reboot/Shutdown buttons so prominent
  in the dashboard can be a bit 'dangerous', I think - especially since
  it's not 100% clear at the moment that this is the PDM host itself.


I fully agree with you proposal, I think putting this into a new tab in
the Administration panel definitely makes a lot of sense.

> It also might be more useful to not show the "Show Fingerprint" button by default.
> Adding that in this card was a bit of a stop-gap for PBS on it's own that I
> implemented only because knowing the fingerprint is quite often a must-have for
> being able to add a PBS as storage to a PVE cluster. As we do not can add a PDM
> somewhere (in our projects) at all for now, there is really not such a need there.
>
> FWIW, for PBS we have reduced the need for having this in the node status directly
> too, as there we now got the "connection info" button/window available on the
> Datastore summary page, which also includes the full datastore repository string,
> and is thus more convenient than the dashboard one.
> That's also why I'd not show this button by default at all, at least in the newer
> Yew UI.
>
>
> _______________________________________________
> pdm-devel mailing list
> pdm-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel



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