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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.