From: "Lukas Wagner" <l.wagner@proxmox.com>
To: "Dominik Csapak" <d.csapak@proxmox.com>, <pdm-devel@lists.proxmox.com>
Subject: Re: [PATCH datacenter-manager v2 3/4] ui: dashboard: add new gauge panels widget type
Date: Wed, 01 Apr 2026 13:34:41 +0200 [thread overview]
Message-ID: <DHHSBU0IVU0E.1VOS16BE5W555@proxmox.com> (raw)
In-Reply-To: <20260330131044.693709-4-d.csapak@proxmox.com>
Hi Dominik,
thanks for the patch!
Some notes inline, along with a proposed patch that you can squash in if
you like at the end.
On Mon Mar 30, 2026 at 3:07 PM CEST, Dominik Csapak wrote:
> This adds gauge chart panels for cpu/memory/storage. Either individually
> or combined all three. For either PVE hosts, PBS hosts, or combined.
>
> We use the new pie chart component for this.
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> lib/pdm-api-types/src/views.rs | 15 +++
> ui/src/dashboard/gauge_panel.rs | 160 ++++++++++++++++++++++++++++++
> ui/src/dashboard/mod.rs | 3 +
> ui/src/dashboard/view.rs | 9 +-
> ui/src/dashboard/view/row_view.rs | 43 +++++++-
> 5 files changed, 227 insertions(+), 3 deletions(-)
> create mode 100644 ui/src/dashboard/gauge_panel.rs
>
> diff --git a/lib/pdm-api-types/src/views.rs b/lib/pdm-api-types/src/views.rs
> index 5e9b4b31..50181e72 100644
> --- a/lib/pdm-api-types/src/views.rs
> +++ b/lib/pdm-api-types/src/views.rs
> @@ -266,6 +266,14 @@ pub struct RowWidget {
> pub r#type: WidgetType,
> }
>
> +#[derive(Serialize, Deserialize, PartialEq, Clone, Copy)]
> +#[serde(rename_all = "kebab-case")]
> +pub enum NodeResourceType {
> + Cpu,
> + Memory,
> + Storage,
> +}
nit: missing doc comments for a public type.
> +
> #[derive(Serialize, Deserialize, PartialEq, Clone)]
> #[serde(rename_all = "kebab-case")]
> #[serde(tag = "widget-type")]
> @@ -295,6 +303,13 @@ pub enum WidgetType {
> grouping: TaskSummaryGrouping,
> },
> ResourceTree,
> + #[serde(rename_all = "kebab-case")]
nit: missing doc-comments for this public enum variant
> + NodeResourceGauge {
> + #[serde(skip_serializing_if = "Option::is_none")]
> + resource: Option<NodeResourceType>,
> + #[serde(skip_serializing_if = "Option::is_none")]
> + remote_type: Option<RemoteType>,
> + },
> }
>
> #[derive(Serialize, Deserialize, PartialEq, Clone, Copy)]
> diff --git a/ui/src/dashboard/gauge_panel.rs b/ui/src/dashboard/gauge_panel.rs
> new file mode 100644
> index 00000000..8b30eb0f
> --- /dev/null
> +++ b/ui/src/dashboard/gauge_panel.rs
> @@ -0,0 +1,160 @@
> +use anyhow::Error;
> +
> +use proxmox_human_byte::HumanByte;
> +use pwt::css;
> +use pwt::prelude::*;
> +use pwt::state::SharedState;
> +use pwt::widget::Fa;
> +use pwt::widget::{charts::PieChart, Panel};
> +use pwt::widget::{error_message, Column, Container, Row};
> +
> +use pdm_api_types::remotes::RemoteType;
> +use pdm_api_types::{resource::ResourcesStatus, views::NodeResourceType};
> +
> +use crate::dashboard::{create_title_with_icon, loading_column};
> +use crate::LoadResult;
> +
This is a pub fn and should therefore have at least some basic documentation.
> +pub fn create_gauge_panel(
> + resource_type: Option<NodeResourceType>,
> + remote_type: Option<RemoteType>,
> + status: SharedState<LoadResult<ResourcesStatus, Error>>,
> +) -> Panel {
> + let status = status.read();
> + let (show_cpu, show_mem, show_storage, title, subtitle, icon) = match resource_type {
> + Some(NodeResourceType::Cpu) => (true, false, false, tr!("CPU Usage"), false, "cpu"),
> + Some(NodeResourceType::Memory) => {
> + (false, true, false, tr!("Memory Usage"), false, "memory")
> + }
> + Some(NodeResourceType::Storage) => {
> + (false, false, true, tr!("Storage Usage"), false, "database")
> + }
> + None => (true, true, true, tr!("Resource Usage"), true, "tachometer"),
> + };
I find this tuple-based approach a bit hard to read, one has jump around
between the 'let (....)' and the returned tuple to see what is going on.
Having 4 different booleans in the assignment could also lead to subtle
logic bugs if one is not careful when changing this code.
I think it could be a nice idea to move these local variables into a struct.
See the end of this message for a full patch.
It's quite a bit more verbose, but personally I think its nicer than
tuple destructuring with such a large amount of parameters - plus it
makes the actual create_gauge_panel function a bit shorter.
Feel free to squash it in if you agree.
> +
> + let suffix = match remote_type {
> + Some(RemoteType::Pve) => " - Virtual Environment",
> + Some(RemoteType::Pbs) => " - Backup Server",
> + None => "",
> + };
> +
> + let is_loading = !status.has_data();
> +
> + Panel::new()
> + .title(create_title_with_icon(icon, format!("{title}{suffix}")))
> + .border(true)
> + .with_optional_child(status.data.as_ref().map(|data| {
> + let (cpu, mem, storage) = match remote_type {
> + Some(RemoteType::Pve) => (
> + show_cpu.then_some((data.pve_cpu_stats.used, data.pve_cpu_stats.max)),
> + show_mem.then_some((data.pve_memory_stats.used, data.pve_memory_stats.total)),
> + show_storage
> + .then_some((data.pve_storage_stats.used, data.pve_storage_stats.total)),
> + ),
> + Some(RemoteType::Pbs) => (
> + show_cpu.then_some((data.pbs_cpu_stats.used, data.pbs_cpu_stats.max)),
> + show_mem.then_some((data.pbs_memory_stats.used, data.pbs_memory_stats.total)),
> + show_storage
> + .then_some((data.pbs_storage_stats.used, data.pbs_storage_stats.total)),
> + ),
> + None => (
> + show_cpu.then_some((
> + data.pve_cpu_stats.used + data.pbs_cpu_stats.used,
> + data.pve_cpu_stats.max + data.pbs_cpu_stats.max,
> + )),
> + show_mem.then_some((
> + data.pve_memory_stats.used + data.pbs_memory_stats.used,
> + data.pve_memory_stats.total + data.pbs_memory_stats.total,
> + )),
> + show_storage.then_some((
> + data.pve_storage_stats.used + data.pbs_storage_stats.used,
> + data.pve_storage_stats.total + data.pbs_storage_stats.total,
> + )),
> + ),
> + };
> +
> + let chart = |percentage: f64, icon: Fa, title: String, extra_text: String| -> Column {
> + let subtitle = subtitle.then_some(
> + Row::new()
> + .gap(1)
> + .class(css::AlignItems::Center)
> + .class(css::JustifyContent::Center)
> + .with_child(icon)
> + .with_child(&title),
> + );
> + Column::new()
> + .flex(1.0)
> + .width("0") // correct flex base size for calculation
> + .max_height(250)
> + .with_child(
> + PieChart::new(title, percentage)
> + .class(css::Overflow::Auto)
> + .text(format!("{:.0}%", percentage * 100.))
> + .angle_start(75.0)
> + .angle_end(285.0)
> + .show_tooltip(false),
> + )
> + .with_optional_child(subtitle)
> + .with_child(
> + Container::new()
> + .padding_top(1)
> + .class(css::TextAlign::Center)
> + .with_child(extra_text),
> + )
> + };
> +
> + Row::new()
> + .padding(4)
> + .with_optional_child(cpu.map(|(used, total)| {
> + let pct = if total == 0.0 { 0.0 } else { used / total };
> + let extra_text = match remote_type {
> + Some(RemoteType::Pve) => {
> + tr!(
> + "{0} of {1} cores ({2} allocated)",
> + format!("{used:.2}"),
> + format!("{total:.0}"),
> + format!("{:.0}", data.pve_cpu_stats.allocated.unwrap_or(0.0)),
> + )
> + }
> + _ => {
> + tr!(
> + "{0} of {1} cores",
> + format!("{used:.2}"),
> + format!("{total:.0}")
> + )
> + }
> + };
> + chart(pct, Fa::new("cpu"), tr!("CPU"), extra_text)
> + }))
> + .with_optional_child(mem.map(|(used, total)| {
> + chart(
> + if total == 0 {
> + 0.0
> + } else {
> + used as f64 / total as f64
> + },
> + Fa::new("memory"),
> + tr!("Memory"),
> + tr!("{0} of {1}", HumanByte::from(used), HumanByte::from(total)),
> + )
> + }))
> + .with_optional_child(storage.map(|(used, total)| {
> + chart(
> + if total == 0 {
> + 0.0
> + } else {
> + used as f64 / total as f64
> + },
> + Fa::new("database"),
> + tr!("Storage"),
> + tr!("{0} of {1}", HumanByte::from(used), HumanByte::from(total)),
> + )
> + }))
> + }))
> + .with_optional_child(is_loading.then_some(loading_column()))
> + .with_optional_child(
> + status
> + .error
> + .as_ref()
> + .map(|err| error_message(&err.to_string())),
> + )
> +}
> diff --git a/ui/src/dashboard/mod.rs b/ui/src/dashboard/mod.rs
> index 17e5ccd3..194000c2 100644
> --- a/ui/src/dashboard/mod.rs
> +++ b/ui/src/dashboard/mod.rs
> @@ -14,6 +14,9 @@ pub use subscriptions_list::SubscriptionsList;
> mod remote_panel;
> pub use remote_panel::create_remote_panel;
>
> +mod gauge_panel;
> +pub use gauge_panel::create_gauge_panel;
> +
> mod guest_panel;
> pub use guest_panel::create_guest_panel;
>
> diff --git a/ui/src/dashboard/view.rs b/ui/src/dashboard/view.rs
> index 4a0400ac..eb1a348e 100644
> --- a/ui/src/dashboard/view.rs
> +++ b/ui/src/dashboard/view.rs
> @@ -22,7 +22,7 @@ use crate::dashboard::refresh_config_edit::{
> use crate::dashboard::subscription_info::create_subscriptions_dialog;
> use crate::dashboard::tasks::get_task_options;
> use crate::dashboard::{
> - create_guest_panel, create_node_panel, create_pbs_datastores_panel,
> + create_gauge_panel, create_guest_panel, create_node_panel, create_pbs_datastores_panel,
> create_refresh_config_edit_window, create_remote_panel, create_resource_tree, create_sdn_panel,
> create_subscription_panel, create_task_summary_panel, create_top_entities_panel,
> DashboardStatusRow,
> @@ -167,6 +167,10 @@ fn render_widget(
> create_task_summary_panel(statistics, remotes, hours, since)
> }
> WidgetType::ResourceTree => create_resource_tree(redraw_controller),
> + WidgetType::NodeResourceGauge {
> + resource,
> + remote_type,
> + } => create_gauge_panel(*resource, *remote_type, status),
> };
>
> if let Some(title) = &item.title {
> @@ -268,7 +272,8 @@ fn required_api_calls(layout: &ViewLayout) -> (bool, bool, bool) {
> | WidgetType::Guests { .. }
> | WidgetType::Remotes { .. }
> | WidgetType::Sdn
> - | WidgetType::PbsDatastores => {
> + | WidgetType::PbsDatastores
> + | WidgetType::NodeResourceGauge { .. } => {
> status = true;
> }
> WidgetType::Subscription => {
> diff --git a/ui/src/dashboard/view/row_view.rs b/ui/src/dashboard/view/row_view.rs
> index 673b4627..f220f954 100644
> --- a/ui/src/dashboard/view/row_view.rs
> +++ b/ui/src/dashboard/view/row_view.rs
> @@ -21,7 +21,7 @@ use crate::dashboard::view::EditingMessage;
>
> use pdm_api_types::remotes::RemoteType;
> use pdm_api_types::views::{
> - LeaderboardType, RowWidget, TaskSummaryGrouping, ViewLayout, WidgetType,
> + LeaderboardType, NodeResourceType, RowWidget, TaskSummaryGrouping, ViewLayout, WidgetType,
> };
>
> #[derive(Properties, PartialEq)]
> @@ -539,6 +539,35 @@ fn create_menu(ctx: &yew::Context<RowViewComp>, new_coords: Position) -> Menu {
> ctx.link()
> .callback(move |_| Msg::AddWidget(new_coords, widget.clone()))
> };
> + let create_gauge_menu = |remote_type: Option<RemoteType>| -> Menu {
> + Menu::new()
> + .with_item(
> + MenuItem::new(tr!("All Resources")).on_select(create_callback(
> + WidgetType::NodeResourceGauge {
> + resource: None,
> + remote_type,
> + },
> + )),
> + )
> + .with_item(MenuItem::new(tr!("CPU")).on_select(create_callback(
> + WidgetType::NodeResourceGauge {
> + resource: Some(NodeResourceType::Cpu),
> + remote_type,
> + },
> + )))
> + .with_item(MenuItem::new(tr!("Memory")).on_select(create_callback(
> + WidgetType::NodeResourceGauge {
> + resource: Some(NodeResourceType::Memory),
> + remote_type,
> + },
> + )))
> + .with_item(MenuItem::new(tr!("Storage")).on_select(create_callback(
> + WidgetType::NodeResourceGauge {
> + resource: Some(NodeResourceType::Storage),
> + remote_type,
> + },
> + )))
> + };
> Menu::new()
> .with_item(
> MenuItem::new(tr!("Remote Panel"))
> @@ -642,4 +671,16 @@ fn create_menu(ctx: &yew::Context<RowViewComp>, new_coords: Position) -> Menu {
> MenuItem::new(tr!("Resource Tree"))
> .on_select(create_callback(WidgetType::ResourceTree)),
> )
> + .with_item(
> + MenuItem::new(tr!("Resource Usage")).menu(
> + Menu::new()
> + .with_item(MenuItem::new(tr!("All")).menu(create_gauge_menu(None)))
> + .with_item(
> + MenuItem::new(tr!("PVE")).menu(create_gauge_menu(Some(RemoteType::Pve))),
> + )
> + .with_item(
> + MenuItem::new(tr!("PBS")).menu(create_gauge_menu(Some(RemoteType::Pbs))),
> + ),
> + ),
> + )
> }
>From f8e9e10c8a12dfd2cc03450c1ccb6cf6d978d94e Mon Sep 17 00:00:00 2001
From: Lukas Wagner <l.wagner@proxmox.com>
Date: Wed, 1 Apr 2026 13:12:15 +0200
Subject: [PATCH datacenter-manager] extract local vars to struct
Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
ui/src/dashboard/gauge_panel.rs | 94 +++++++++++++++++++++++++--------
1 file changed, 73 insertions(+), 21 deletions(-)
diff --git a/ui/src/dashboard/gauge_panel.rs b/ui/src/dashboard/gauge_panel.rs
index 8b30eb0f..a88f56cb 100644
--- a/ui/src/dashboard/gauge_panel.rs
+++ b/ui/src/dashboard/gauge_panel.rs
@@ -14,22 +14,61 @@ use pdm_api_types::{resource::ResourcesStatus, views::NodeResourceType};
use crate::dashboard::{create_title_with_icon, loading_column};
use crate::LoadResult;
+struct PanelConfig {
+ show_cpu: bool,
+ show_mem: bool,
+ show_storage: bool,
+ title: String,
+ subtitle: bool,
+ icon: &'static str,
+}
+
+impl PanelConfig {
+ fn new(resource_type: Option<NodeResourceType>) -> Self {
+ match resource_type {
+ Some(NodeResourceType::Cpu) => PanelConfig {
+ show_cpu: true,
+ show_mem: false,
+ show_storage: false,
+ title: tr!("CPU Usage"),
+ subtitle: false,
+ icon: "cpu",
+ },
+ Some(NodeResourceType::Memory) => PanelConfig {
+ show_cpu: false,
+ show_mem: true,
+ show_storage: false,
+ title: tr!("Memory Usage"),
+ subtitle: false,
+ icon: "memory",
+ },
+ Some(NodeResourceType::Storage) => PanelConfig {
+ show_cpu: false,
+ show_mem: false,
+ show_storage: true,
+ title: tr!("Storage Usage"),
+ subtitle: false,
+ icon: "database",
+ },
+ None => PanelConfig {
+ show_cpu: true,
+ show_mem: true,
+ show_storage: true,
+ title: tr!("Resource Usage"),
+ subtitle: true,
+ icon: "tachometer",
+ },
+ }
+ }
+}
+
pub fn create_gauge_panel(
resource_type: Option<NodeResourceType>,
remote_type: Option<RemoteType>,
status: SharedState<LoadResult<ResourcesStatus, Error>>,
) -> Panel {
let status = status.read();
- let (show_cpu, show_mem, show_storage, title, subtitle, icon) = match resource_type {
- Some(NodeResourceType::Cpu) => (true, false, false, tr!("CPU Usage"), false, "cpu"),
- Some(NodeResourceType::Memory) => {
- (false, true, false, tr!("Memory Usage"), false, "memory")
- }
- Some(NodeResourceType::Storage) => {
- (false, false, true, tr!("Storage Usage"), false, "database")
- }
- None => (true, true, true, tr!("Resource Usage"), true, "tachometer"),
- };
+ let panel_config = PanelConfig::new(resource_type);
let suffix = match remote_type {
Some(RemoteType::Pve) => " - Virtual Environment",
@@ -40,32 +79,45 @@ pub fn create_gauge_panel(
let is_loading = !status.has_data();
Panel::new()
- .title(create_title_with_icon(icon, format!("{title}{suffix}")))
+ .title(create_title_with_icon(
+ panel_config.icon,
+ format!("{}{suffix}", panel_config.title),
+ ))
.border(true)
.with_optional_child(status.data.as_ref().map(|data| {
let (cpu, mem, storage) = match remote_type {
Some(RemoteType::Pve) => (
- show_cpu.then_some((data.pve_cpu_stats.used, data.pve_cpu_stats.max)),
- show_mem.then_some((data.pve_memory_stats.used, data.pve_memory_stats.total)),
- show_storage
+ panel_config
+ .show_cpu
+ .then_some((data.pve_cpu_stats.used, data.pve_cpu_stats.max)),
+ panel_config
+ .show_mem
+ .then_some((data.pve_memory_stats.used, data.pve_memory_stats.total)),
+ panel_config
+ .show_storage
.then_some((data.pve_storage_stats.used, data.pve_storage_stats.total)),
),
Some(RemoteType::Pbs) => (
- show_cpu.then_some((data.pbs_cpu_stats.used, data.pbs_cpu_stats.max)),
- show_mem.then_some((data.pbs_memory_stats.used, data.pbs_memory_stats.total)),
- show_storage
+ panel_config
+ .show_cpu
+ .then_some((data.pbs_cpu_stats.used, data.pbs_cpu_stats.max)),
+ panel_config
+ .show_mem
+ .then_some((data.pbs_memory_stats.used, data.pbs_memory_stats.total)),
+ panel_config
+ .show_storage
.then_some((data.pbs_storage_stats.used, data.pbs_storage_stats.total)),
),
None => (
- show_cpu.then_some((
+ panel_config.show_cpu.then_some((
data.pve_cpu_stats.used + data.pbs_cpu_stats.used,
data.pve_cpu_stats.max + data.pbs_cpu_stats.max,
)),
- show_mem.then_some((
+ panel_config.show_mem.then_some((
data.pve_memory_stats.used + data.pbs_memory_stats.used,
data.pve_memory_stats.total + data.pbs_memory_stats.total,
)),
- show_storage.then_some((
+ panel_config.show_storage.then_some((
data.pve_storage_stats.used + data.pbs_storage_stats.used,
data.pve_storage_stats.total + data.pbs_storage_stats.total,
)),
@@ -73,7 +125,7 @@ pub fn create_gauge_panel(
};
let chart = |percentage: f64, icon: Fa, title: String, extra_text: String| -> Column {
- let subtitle = subtitle.then_some(
+ let subtitle = panel_config.subtitle.then_some(
Row::new()
.gap(1)
.class(css::AlignItems::Center)
--
2.47.3
next prev parent reply other threads:[~2026-04-01 11:34 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-30 13:07 [PATCH datacenter-manager v2 0/4] add resource gauge panels to dashboard/views Dominik Csapak
2026-03-30 13:07 ` [PATCH datacenter-manager v2 1/4] api: return global cpu/memory/storage statistics Dominik Csapak
2026-04-01 11:34 ` Lukas Wagner
2026-04-02 13:02 ` Dominik Csapak
2026-04-02 13:48 ` Lukas Wagner
2026-03-30 13:07 ` [PATCH datacenter-manager v2 2/4] ui: css: use mask for svg icons Dominik Csapak
2026-03-30 13:07 ` [PATCH datacenter-manager v2 3/4] ui: dashboard: add new gauge panels widget type Dominik Csapak
2026-04-01 11:34 ` Lukas Wagner [this message]
2026-04-01 11:36 ` Lukas Wagner
2026-03-30 13:07 ` [PATCH datacenter-manager v2 4/4] ui: dashboard: add resource gauges to default dashboard Dominik Csapak
2026-04-01 11:34 ` [PATCH datacenter-manager v2 0/4] add resource gauge panels to dashboard/views Lukas Wagner
2026-04-02 13:24 ` superseded: " Dominik Csapak
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=DHHSBU0IVU0E.1VOS16BE5W555@proxmox.com \
--to=l.wagner@proxmox.com \
--cc=d.csapak@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