From 28a40a2650215179434edfd362a540a82fd9cf21 Mon Sep 17 00:00:00 2001 From: Carsten Date: Wed, 22 Apr 2026 18:06:17 +0200 Subject: [PATCH] chore(ui,agent): harden collector parsing, drop dead CSS, resilver label Addresses final code review: - to_int/1 now returns 0 on nil or unparseable strings instead of crashing - remove unused .pool-row CSS (superseded by .pool-block) - clamp capacity bar width to [0, 100] to prevent visual overflow - pool_scrub_line/1 uses scan_function so resilver shows as "resilver..." --- agent/lib/proxmox_agent/collectors/zfs.ex | 10 ++++- .../proxmox_agent/collectors/zfs_test.exs | 37 +++++++++++++++++++ server/assets/css/app.css | 11 ------ .../lib/server_web/live/host_detail_live.ex | 26 +++++++++---- .../server_web/live/host_detail_live_test.exs | 4 +- 5 files changed, 67 insertions(+), 21 deletions(-) diff --git a/agent/lib/proxmox_agent/collectors/zfs.ex b/agent/lib/proxmox_agent/collectors/zfs.ex index 4e8b4da..89dfdd2 100644 --- a/agent/lib/proxmox_agent/collectors/zfs.ex +++ b/agent/lib/proxmox_agent/collectors/zfs.ex @@ -176,7 +176,15 @@ defmodule ProxmoxAgent.Collectors.Zfs do end defp to_int(v) when is_integer(v), do: v - defp to_int(v) when is_binary(v), do: String.to_integer(v) + + defp to_int(v) when is_binary(v) do + case Integer.parse(v) do + {n, _} -> n + :error -> 0 + end + end + + defp to_int(_), do: 0 defp max_or_nil([]), do: nil defp max_or_nil(list), do: Enum.max(list) diff --git a/agent/test/proxmox_agent/collectors/zfs_test.exs b/agent/test/proxmox_agent/collectors/zfs_test.exs index 122d48c..cceda66 100644 --- a/agent/test/proxmox_agent/collectors/zfs_test.exs +++ b/agent/test/proxmox_agent/collectors/zfs_test.exs @@ -104,6 +104,43 @@ defmodule ProxmoxAgent.Collectors.ZfsTest do # log vdev is retained in the per-pool vdevs list even though it's ignored for layout classification assert Enum.any?(by_name["mirror_with_log"].vdevs, &(&1.type == "log")) end + + test "tolerates nil and non-integer error counters" do + list_json = + Jason.encode!(%{ + "pools" => %{ + "weird" => %{"name" => "weird", "size" => 1, "alloc" => 0, "free" => 1, + "frag" => 0, "cap" => 0, "health" => "ONLINE"} + } + }) + + status_json = + Jason.encode!(%{ + "pools" => %{ + "weird" => %{ + "name" => "weird", "state" => "ONLINE", "error_count" => nil, + "vdevs" => %{ + "disk-0" => %{"name" => "disk-0", "vdev_type" => "disk", "state" => "ONLINE", + "read_errors" => nil, "write_errors" => "abc", + "checksum_errors" => "0"} + } + } + } + }) + + runner = fn + "zpool", ["list" | _] -> {:ok, list_json} + "zpool", ["status" | _] -> {:ok, status_json} + end + + sample = Zfs.collect_pools(runner: runner) + [pool] = sample.pools + assert pool.error_count == 0 + [vdev] = pool.vdevs + assert vdev.read_errors == 0 + assert vdev.write_errors == 0 + assert vdev.checksum_errors == 0 + end end describe "collect_datasets/1" do diff --git a/server/assets/css/app.css b/server/assets/css/app.css index 75d9155..3af2171 100644 --- a/server/assets/css/app.css +++ b/server/assets/css/app.css @@ -246,17 +246,6 @@ hr { border: 0; border-top: 1px solid var(--border); margin: 0; } .kv dt { color: var(--muted); } .kv dd { margin: 0; font-family: var(--mono); color: var(--fg-bright); } -.pool-row { - display: flex; - justify-content: space-between; - padding: 0.45rem 0; - border-bottom: 1px solid var(--border); - gap: 0.8rem; - font-size: 0.85rem; -} -.pool-row:last-child { border-bottom: none; } -.pool-row .details { color: var(--muted); font-family: var(--mono); font-size: 0.78rem; } - .capbar { height: 4px; background: var(--panel-2); diff --git a/server/lib/server_web/live/host_detail_live.ex b/server/lib/server_web/live/host_detail_live.ex index 1b032a8..4a8b5e2 100644 --- a/server/lib/server_web/live/host_detail_live.ex +++ b/server/lib/server_web/live/host_detail_live.ex @@ -76,7 +76,7 @@ defmodule ServerWeb.HostDetailLive do
- +
@@ -181,17 +181,29 @@ defmodule ServerWeb.HostDetailLive do defp capbar_level(cap) when is_number(cap) and cap >= 80, do: "warn" defp capbar_level(_), do: "ok" - defp pool_scrub_line(%{"scan_state" => "SCANNING"}), do: "scrub scanning" + defp capbar_width(cap) when is_number(cap), do: cap |> max(0) |> min(100) + defp capbar_width(_), do: 0 - defp pool_scrub_line(%{"scan_state" => "FINISHED", "last_scrub_end" => end_time}) - when is_binary(end_time) and end_time != "", - do: "scrub finished #{end_time}" + defp pool_scrub_line(%{"scan_state" => "SCANNING"} = pool) do + "#{scan_verb(pool)} scanning" + end - defp pool_scrub_line(%{"last_scrub_end" => end_time}) when is_binary(end_time) and end_time != "", - do: "scrub #{end_time}" + defp pool_scrub_line(%{"last_scrub_end" => end_time} = pool) + when is_binary(end_time) and end_time != "" do + prefix = + case pool["scan_state"] do + "FINISHED" -> "#{scan_verb(pool)} finished " + _ -> "#{scan_verb(pool)} " + end + + prefix <> end_time + end defp pool_scrub_line(_), do: "scrub never" + defp scan_verb(%{"scan_function" => "resilver"}), do: "resilver" + defp scan_verb(_), do: "scrub" + defp degraded_vdevs(pool) do (pool["vdevs"] || []) |> Enum.filter(fn v -> Map.get(v, "state") not in [nil, "ONLINE"] end) diff --git a/server/test/server_web/live/host_detail_live_test.exs b/server/test/server_web/live/host_detail_live_test.exs index bedd022..da88932 100644 --- a/server/test/server_web/live/host_detail_live_test.exs +++ b/server/test/server_web/live/host_detail_live_test.exs @@ -46,7 +46,7 @@ defmodule ServerWeb.HostDetailLiveTest do "error_count" => 2, "vdev_count" => 1, "degraded_vdev_count" => 1, - "scan_function" => "scrub", + "scan_function" => "resilver", "scan_state" => "SCANNING", "last_scrub_end" => nil, "vdevs" => [ @@ -117,7 +117,7 @@ defmodule ServerWeb.HostDetailLiveTest do assert html =~ "tank" assert html =~ "DEGRADED" assert html =~ "raidz2" - assert html =~ "scrub scanning" + assert html =~ "resilver scanning" assert html =~ "raidz2-0 DEGRADED" assert html =~ "cksum=2" assert html =~ "callout err"