Repository navigation
feat(monitoring): add KubeVirt VM dashboard - #2198
Conversation
dosubot
Bot
added
size/XXL
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds two new Grafana dashboards for KubeVirt VM monitoring and registers them in the system monitoring dashboards list. The Console Logs dashboard provides a logs view of VM console output, while the Virtual Machines dashboard offers comprehensive Prometheus-backed metrics across CPU, memory, network, filesystem, and disk I/O with stat panels, an overview table, and detailed timeseries visualizations. ChangesVM Monitoring Dashboards
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
dashboards/vm/virtual-machines.json (1)
120-145: Clarify whether this is CPU % or CPU cores.These panels plot raw
rate(..._seconds_total)values, so they scale with the number of busy vCPUs. Rendering them aspercentunitmakes the result ambiguous for multi-vCPU guests. Either normalize by vCPU count before formatting as%, or rename/unit the panels as cores.Also applies to: 375-414
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dashboards/vm/virtual-machines.json` around lines 120 - 145, The "Average CPU" stat panel is using avg(rate(kubevirt_vmi_cpu_usage_seconds_total...)) but is labeled/unit'd as "percentunit", which is incorrect for raw CPU-seconds (it yields cores * time); update the panel to either (A) normalize the expression by the vCPU count metric (e.g., divide by a vCPU count metric such as kubevirt_vmi_vcpu_count or another available label) and keep the unit as percentunit if you convert to usage fraction*100, or (B) change the unit from "percentunit" to a core-based unit and rename the "Average CPU" title to "Average vCPUs" (and apply the same change to the other similar panels referenced at lines ~375-414) so the legend/unit matches the raw avg(rate(...)) output.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@dashboards/vm/console-logs.json`:
- Around line 21-30: The "namespace" textbox field currently named "namespace"
with type "textbox" and default/current value "*" allows users to override
tenant scope and causes cluster-wide log queries; fix by removing or converting
this field to a non-editable fixed value (e.g., change type from "textbox" to
"const"/hidden and set a tenant-specific namespace value) or remove it and
enforce namespace via the data source or RBAC so the query referenced (the
namespace filter used in the panel query) cannot be set to "*" by users; update
the "namespace" field definition accordingly and ensure any panel queries use
the enforced namespace variable rather than the editable textbox.
In `@dashboards/vm/virtual-machines.json`:
- Around line 194-220: The panel queries (refId net_rx, net_tx, iops_r, iops_w)
produce one series per interface/drive and must be aggregated before the
downstream table merge: wrap each rate(...) expression with sum by (name)(...)
so the PromQL becomes sum by
(name)(rate(kubevirt_vmi_network_receive_bytes_total{...}[$__rate_interval]))
(and similarly for net_tx and for iops_r/iops_w while keeping the
drive!="cloudinitdisk" filter), leaving legendFormat, refId and instant settings
unchanged.
---
Nitpick comments:
In `@dashboards/vm/virtual-machines.json`:
- Around line 120-145: The "Average CPU" stat panel is using
avg(rate(kubevirt_vmi_cpu_usage_seconds_total...)) but is labeled/unit'd as
"percentunit", which is incorrect for raw CPU-seconds (it yields cores * time);
update the panel to either (A) normalize the expression by the vCPU count metric
(e.g., divide by a vCPU count metric such as kubevirt_vmi_vcpu_count or another
available label) and keep the unit as percentunit if you convert to usage
fraction*100, or (B) change the unit from "percentunit" to a core-based unit and
rename the "Average CPU" title to "Average vCPUs" (and apply the same change to
the other similar panels referenced at lines ~375-414) so the legend/unit
matches the raw avg(rate(...)) output.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a55ed75d-9122-4948-bb8c-1deedc257c76
📒 Files selected for processing (3)
dashboards/vm/console-logs.jsondashboards/vm/virtual-machines.jsonpackages/system/monitoring/dashboards.list
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
dashboards/vm/console-logs.json (1)
29-40:⚠️ Potential issue | 🟠 MajorNamespace filtering still isn't an isolation boundary.
The textbox wildcard is gone, but
namespaceis still user-selectable vialabel_values(..., namespace). The filter on Line 74 only constrains the query to the chosen namespace; it does not stop a user from switching to another namespace that the datasource can see. If tenant isolation is part of this dashboard's contract, scope still needs to be enforced outside this variable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dashboards/vm/console-logs.json` around lines 29 - 40, The "namespace" dashboard variable (query "label_values(kubevirt_vmi_memory_available_bytes, namespace)") is still user-selectable and thus does not enforce tenant isolation; replace this dynamic label_values source with a tenant-scoped constant (or remove the variable entirely) and make all panels use the injected tenant namespace instead of the "namespace" variable. Concretely: stop populating "namespace" from label_values(kubevirt_vmi_memory_available_bytes, namespace), either hardcode the tenant namespace or bind it to a secure injected variable like ${tenant_namespace}, and update every panel/filter that currently references the "namespace" variable (the panel filter mentioned around the existing query on Line 74) to use the tenant-scoped value so users cannot switch to other namespaces. Ensure datasource permission-based scoping or external enforcement is used if per-request injection is required.
🧹 Nitpick comments (1)
dashboards/vm/console-logs.json (1)
29-57: Use a selector source that survives VM shutdown.Lines 36 and 50 drive both dropdowns from
kubevirt_vmi_memory_available_bytes, so a VM disappears from the selector as soon as it stops exporting that metric even if its recent console logs still exist. For a log dashboard, it's safer to back these variables with a source that outlives the VM instead of a live memory series.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dashboards/vm/console-logs.json` around lines 29 - 57, The namespace and vm template variables ("namespace" and "vm") use the ephemeral metric kubevirt_vmi_memory_available_bytes so VMs drop out when that metric stops; change both queries to use a stable selector such as kubevirt_vmi_info or kubevirt_vmi_labels (e.g. label_values(kubevirt_vmi_info, namespace) and label_values(kubevirt_vmi_info{namespace="$namespace"}, name)) instead of kubevirt_vmi_memory_available_bytes so selectors persist after VM shutdown.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@dashboards/vm/virtual-machines.json`:
- Around line 577-655: The network charts use per-interface Prometheus metrics
(e.g., kubevirt_vmi_network_receive_bytes_total,
kubevirt_vmi_network_transmit_bytes_total,
kubevirt_vmi_network_receive_packets_total,
kubevirt_vmi_network_transmit_packets_total,
kubevirt_vmi_network_receive_errors_total,
kubevirt_vmi_network_transmit_errors_total,
kubevirt_vmi_network_receive_packets_dropped_total,
kubevirt_vmi_network_transmit_packets_dropped_total) but their legendFormat only
shows {{ name }}, collapsing multiple NIC series; either aggregate each query
(wrap with sum by (name)(...)) to show per-VM totals, or preserve per-interface
visibility by updating the legendFormat for all affected targets (Inbound
Throughput, Outbound Throughput (TX), Packets/s, Errors & Drops) to include the
interface label (e.g., change "{{ name }}" to "{{ name }} {{ interface }}" or
"{{ name }} {{ interface }} RX/TX" as appropriate) and apply the same change to
the errors/drops targets.
---
Duplicate comments:
In `@dashboards/vm/console-logs.json`:
- Around line 29-40: The "namespace" dashboard variable (query
"label_values(kubevirt_vmi_memory_available_bytes, namespace)") is still
user-selectable and thus does not enforce tenant isolation; replace this dynamic
label_values source with a tenant-scoped constant (or remove the variable
entirely) and make all panels use the injected tenant namespace instead of the
"namespace" variable. Concretely: stop populating "namespace" from
label_values(kubevirt_vmi_memory_available_bytes, namespace), either hardcode
the tenant namespace or bind it to a secure injected variable like
${tenant_namespace}, and update every panel/filter that currently references the
"namespace" variable (the panel filter mentioned around the existing query on
Line 74) to use the tenant-scoped value so users cannot switch to other
namespaces. Ensure datasource permission-based scoping or external enforcement
is used if per-request injection is required.
---
Nitpick comments:
In `@dashboards/vm/console-logs.json`:
- Around line 29-57: The namespace and vm template variables ("namespace" and
"vm") use the ephemeral metric kubevirt_vmi_memory_available_bytes so VMs drop
out when that metric stops; change both queries to use a stable selector such as
kubevirt_vmi_info or kubevirt_vmi_labels (e.g. label_values(kubevirt_vmi_info,
namespace) and label_values(kubevirt_vmi_info{namespace="$namespace"}, name))
instead of kubevirt_vmi_memory_available_bytes so selectors persist after VM
shutdown.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 74986ee1-b4b3-4f5d-bfca-4b09402aab36
📒 Files selected for processing (2)
dashboards/vm/console-logs.jsondashboards/vm/virtual-machines.json
60d258b to
bcf42b3
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
dashboards/vm/virtual-machines.json (1)
577-655:⚠️ Potential issue | 🟡 MinorNetwork legends still collapse multi-NIC series.
These targets are still per-interface, but the legends only show the VM and direction. On multi-NIC VMs Grafana will plot multiple indistinguishable lines. Either aggregate to per-VM totals with
sum by (name)or include{{ interface }}in every RX/TX/error/drop legend.Example fix if you want to keep per-interface visibility
- "legendFormat": "{{ name }}" + "legendFormat": "{{ name }} ({{ interface }})" - "legendFormat": "{{ name }}" + "legendFormat": "{{ name }} ({{ interface }})" - "legendFormat": "{{ name }} RX" + "legendFormat": "{{ name }} RX ({{ interface }})" - "legendFormat": "{{ name }} TX" + "legendFormat": "{{ name }} TX ({{ interface }})" - "legendFormat": "{{ name }} RX errors" + "legendFormat": "{{ name }} RX errors ({{ interface }})" - "legendFormat": "{{ name }} TX errors" + "legendFormat": "{{ name }} TX errors ({{ interface }})" - "legendFormat": "{{ name }} RX dropped" + "legendFormat": "{{ name }} RX dropped ({{ interface }})" - "legendFormat": "{{ name }} TX dropped" + "legendFormat": "{{ name }} TX dropped ({{ interface }})"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dashboards/vm/virtual-machines.json` around lines 577 - 655, The panel legends for network panels ("Outbound Throughput (TX)", "Packets/s", "Errors & Drops") collapse multi-NIC series because legendFormat uses only "{{ name }}"/direction; update each target in those panels to either aggregate per-VM (wrap the PromQL in sum by (name) e.g. sum by(name)(rate(...))) or keep per-interface visibility by adding the interface label to the legend (change legendFormat to include "{{ interface }}" such as "{{ name }} {{ interface }} TX"/"RX"/"{{ interface }} RX errors" etc); modify the target expressions and/or legendFormat in the targets for kubevirt_vmi_network_receive_*, kubevirt_vmi_network_transmit_*, kubevirt_vmi_network_*_packets_dropped_total, and kubevirt_vmi_network_*_errors_total accordingly.
🧹 Nitpick comments (1)
dashboards/vm/virtual-machines.json (1)
132-143: Revisit CPU thresholds now that these panels show cores.The dashboard now renders CPU as
short, but the cutoffs stayed at0.7/0.9and0.5/0.8. Those values behave like leftover percentage thresholds and will color ordinary load as saturation on larger VMs. If the intent is utilization, normalize by allocated vCPUs; otherwise drop the color thresholds.Also applies to: 278-294
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dashboards/vm/virtual-machines.json` around lines 132 - 143, The CPU panels set fieldConfig.defaults.unit to "short" but keep thresholds.steps values like 0.7/0.9 and 0.5/0.8 (percentage-style), which will miscolor load when panels show cores; update the panels that define fieldConfig.defaults.decimals and fieldConfig.defaults.thresholds.steps (the entries with values 0.7/0.9 and 0.5/0.8, also present at the other occurrence around the 278-294 region) to either (A) normalize the threshold values by allocated vCPUs (compute threshold = desired utilization * vCPU count so thresholds become absolute core counts), (B) change fieldConfig.defaults.unit to "percent" and keep the fractional thresholds, or (C) remove/disable thresholds.steps entirely if no coloring is desired; apply the chosen fix consistently to both occurrences and ensure decimals remain appropriate for the selected unit.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@dashboards/vm/virtual-machines.json`:
- Around line 423-453: The Grafana panels using
rate(kubevirt_vmi_vcpu_wait_seconds_total[...] ) and
rate(kubevirt_vmi_vcpu_delay_seconds_total[...] ) are labeled with unit "s"
which is incorrect because rate() yields seconds/second (dimensionless); update
the panels (identified by title "vCPU Wait" and "vCPU Delay (Steal Time)" or the
target expressions rate(kubevirt_vmi_vcpu_wait_seconds_total{...}) and
rate(kubevirt_vmi_vcpu_delay_seconds_total{...})) to use a dimensionless unit
such as "short" or an explicit "s/s" (or alternatively change the query to
increase(...)/interval if you really want elapsed seconds over the range), and
adjust the fieldConfig.defaults.unit from "s" to the chosen unit so axis and
tooltips display correctly.
---
Duplicate comments:
In `@dashboards/vm/virtual-machines.json`:
- Around line 577-655: The panel legends for network panels ("Outbound
Throughput (TX)", "Packets/s", "Errors & Drops") collapse multi-NIC series
because legendFormat uses only "{{ name }}"/direction; update each target in
those panels to either aggregate per-VM (wrap the PromQL in sum by (name) e.g.
sum by(name)(rate(...))) or keep per-interface visibility by adding the
interface label to the legend (change legendFormat to include "{{ interface }}"
such as "{{ name }} {{ interface }} TX"/"RX"/"{{ interface }} RX errors" etc);
modify the target expressions and/or legendFormat in the targets for
kubevirt_vmi_network_receive_*, kubevirt_vmi_network_transmit_*,
kubevirt_vmi_network_*_packets_dropped_total, and
kubevirt_vmi_network_*_errors_total accordingly.
---
Nitpick comments:
In `@dashboards/vm/virtual-machines.json`:
- Around line 132-143: The CPU panels set fieldConfig.defaults.unit to "short"
but keep thresholds.steps values like 0.7/0.9 and 0.5/0.8 (percentage-style),
which will miscolor load when panels show cores; update the panels that define
fieldConfig.defaults.decimals and fieldConfig.defaults.thresholds.steps (the
entries with values 0.7/0.9 and 0.5/0.8, also present at the other occurrence
around the 278-294 region) to either (A) normalize the threshold values by
allocated vCPUs (compute threshold = desired utilization * vCPU count so
thresholds become absolute core counts), (B) change fieldConfig.defaults.unit to
"percent" and keep the fractional thresholds, or (C) remove/disable
thresholds.steps entirely if no coloring is desired; apply the chosen fix
consistently to both occurrences and ensure decimals remain appropriate for the
selected unit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dd64ca1f-3810-462b-b6fc-ef2a53608e7d
📒 Files selected for processing (3)
dashboards/vm/console-logs.jsondashboards/vm/virtual-machines.jsonpackages/system/monitoring/dashboards.list
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/system/monitoring/dashboards.list
- dashboards/vm/console-logs.json
|
Hey mattia-eleuteri could you please fix conflict after merging |
bcf42b3 to
46104de
Compare
|
Rebased on upstream/main — conflict with #2197 in |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@dashboards/vm/virtual-machines.json`:
- Around line 127-134: The panel is using unit "percentunit" while the PromQL
expr returns CPU cores (seconds/second) via
kubevirt_vmi_cpu_usage_seconds_total, which makes multi-vCPU VMs show >100%;
update the panel to either change the fieldConfig.defaults.unit from
"percentunit" to a generic unit like "short" and update the legend/text to
indicate "cores", or normalize the query to produce a 0–1 percentage by dividing
rate(kubevirt_vmi_cpu_usage_seconds_total{...}[$__rate_interval]) by a vCPU
total metric such as kubevirt_vmi_vcpu_seconds_total{state="total"} (and keep
percentunit only if you perform that normalization).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 38339b5d-6bf4-4f6e-a2f8-30b96e01da1c
📒 Files selected for processing (3)
dashboards/vm/console-logs.jsondashboards/vm/virtual-machines.jsonpackages/system/monitoring/dashboards.list
🚧 Files skipped from review as they are similar to previous changes (1)
- dashboards/vm/console-logs.json
| "expr": "avg(rate(kubevirt_vmi_cpu_usage_seconds_total{namespace=\"$namespace\", name=~\"$vm\"}[$__rate_interval]))", | ||
| "legendFormat": "" | ||
| } | ||
| ], | ||
| "options": { "colorMode": "value", "graphMode": "area", "textMode": "value" }, | ||
| "fieldConfig": { | ||
| "defaults": { | ||
| "unit": "percentunit", |
There was a problem hiding this comment.
CPU percentunit may be misleading for multi-vCPU VMs.
rate(kubevirt_vmi_cpu_usage_seconds_total) returns CPU cores consumed (seconds/second), not a 0-1 ratio. For a VM with 4 vCPUs at full utilization, this returns ~4.0, which Grafana displays as "400%" with percentunit.
Consider using short with a label like "cores" or normalize by vCPU count if you truly want percentage:
rate(kubevirt_vmi_cpu_usage_seconds_total{...}[$__rate_interval])
/ on(name, namespace) kubevirt_vmi_vcpu_seconds_total{state="total"}
Alternatively, adjust thresholds to reflect core counts rather than percentages.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@dashboards/vm/virtual-machines.json` around lines 127 - 134, The panel is
using unit "percentunit" while the PromQL expr returns CPU cores
(seconds/second) via kubevirt_vmi_cpu_usage_seconds_total, which makes
multi-vCPU VMs show >100%; update the panel to either change the
fieldConfig.defaults.unit from "percentunit" to a generic unit like "short" and
update the legend/text to indicate "cores", or normalize the query to produce a
0–1 percentage by dividing
rate(kubevirt_vmi_cpu_usage_seconds_total{...}[$__rate_interval]) by a vCPU
total metric such as kubevirt_vmi_vcpu_seconds_total{state="total"} (and keep
percentunit only if you perform that normalization).
|
Hey mattia-eleuteri, I reviewed the dashboards and found one issue with the CPU panels.
I pushed a fix here: matthieu-robin/cozystack@0f9a2c0b Changes:
Feel free to cherry-pick if you agree: |
Aleksei Sviridkin (lexfrei)
left a comment
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
NOT LGTM — the dashboards.list change reverts the tenant-scoping fix from #2197, and the CPU-panel formatting issue raised in review is still unaddressed.
Business context: add two KubeVirt VM dashboards (per-tenant VM metrics + console logs) to Grafana.
Blockers
B1: dashboards.list re-adds 16 infra dashboards that #2197 moved to dashboards-infra.list (regression + duplicates)
File: packages/system/monitoring/dashboards.list
Only 2 of the 18 added entries are this PR's new dashboards (vm/virtual-machines, vm/console-logs). The other 16 (main/*, control-plane/*, kubevirt/kubevirt-control-plane, flux/*) already live in dashboards-infra.list, where #2197 ("Scope infrastructure dashboards to tenant-root only") deliberately moved them. The template renders dashboards.list in every namespace but guards dashboards-infra.list behind {{- if eq .Release.Namespace "tenant-root" }}. Re-adding these to dashboards.list: (a) leaks infra dashboards back into every tenant's Grafana — the exact regression #2197 fixed; (b) produces 16 duplicate GrafanaDashboard resources in tenant-root (same <folder>-<name> rendered from both lists) → server-side-apply / ownership conflict.
Evidence: on origin/main, main/controller is absent from dashboards.list and present in dashboards-infra.list; this branch re-adds it. The branch's merge-base predates #2197, so this looks like a stale-branch artifact.
Fix: the dashboards.list diff must contain only vm/virtual-machines and vm/console-logs; leave the infra entries untouched (a rebase onto current main should drop the 16 lines).
B2: CPU panels still format a cores-valued metric as a percentage (review finding still open)
File: dashboards/vm/virtual-machines.json (panels 4, 10, 50)
rate(kubevirt_vmi_cpu_usage_seconds_total[...]) returns CPU cores consumed (seconds/second), not a 0–1 ratio, but these panels still use "unit": "percentunit" with thresholds 0.5/0.8/0.9. A 4-vCPU VM at full load renders as "400%", and the thresholds are meaningless for multi-vCPU VMs. This was already raised in review with a proposed fix (percentunit → short with "(cores)" in titles, thresholds adjusted to core counts); the branch is still a single commit and the panels are unchanged, so it remains open. The lgtm label predates this and is stale.
Fix: apply the proposed cores-based formatting to the CPU stat/timeseries/table panels.
Notes (verified, non-blocking)
- Delivery path is correct — the JSONs bake into the
grafana-dashboardsimage (CI rebuilds and rewrites the digest), so no manual tag bump is needed. console-logs.jsondepends on the logs datasource and onguest-console-logcontainer logs being ingested with thevm.kubevirt.io/namelabel — verify the panel populates on a live cluster.- Both dashboard JSONs are valid; no secrets.
- A
helm templateassertion that a non-tenant-root namespace renders zeromain-*/control-plane-*/flux-*/kubevirt-control-planeGrafanaDashboards would pin B1 against re-introduction.
| db/maria-db | ||
| db/redis | ||
| main/controller | ||
| main/namespaces |
There was a problem hiding this comment.
Only vm/virtual-machines and vm/console-logs are new here. The other 16 entries are infra dashboards that #2197 moved into dashboards-infra.list (tenant-root-guarded). Re-adding them to dashboards.list leaks them into every tenant's Grafana and duplicates the GrafanaDashboard resources in tenant-root. Rebase onto current main and keep only the two vm/* lines.
There was a problem hiding this comment.
Done — rebased onto current main and the dashboards.list diff now contains only vm/virtual-machines and vm/console-logs. The 16 infra entries are gone (they stay in dashboards-infra.list), so no more leak into tenant Grafana and no duplicate GrafanaDashboard resources in tenant-root.
| "options": { "colorMode": "value", "graphMode": "area", "textMode": "value" }, | ||
| "fieldConfig": { | ||
| "defaults": { | ||
| "unit": "percentunit", |
There was a problem hiding this comment.
rate(kubevirt_vmi_cpu_usage_seconds_total) returns cores consumed (sec/sec), not a 0–1 ratio, so percentunit shows e.g. "400%" for a 4-vCPU VM and the 0.5/0.8/0.9 thresholds don't fit multi-vCPU VMs. Use short with "(cores)" and core-count thresholds (this was already proposed in review and isn't applied yet).
There was a problem hiding this comment.
Fixed. The CPU panels that plot rate(kubevirt_vmi_cpu_*_usage_seconds_total) — "Average CPU", "CPU per VM", "CPU User / System", and the CPU column of the VM Overview table — now use unit: short with "(cores)" in the titles instead of percentunit, so a 4-vCPU VM at full load reads 4 rather than 400%.
On thresholds: rather than convert the 0.5/0.8/0.9 fractions to fixed core counts, I dropped the color steps (green-only). Absolute core thresholds are arbitrary across heterogeneous VM sizes (and meaningless on the averaged/aggregated stat), so coloring would be misleading either way. Happy to add explicit core cutoffs if you'd prefer a specific pair.
Head branch was pushed to by a user without write access
46104de to
58b917e
Compare
github-actions
Bot
added
the
area/monitoring
|
Both blockers addressed (force-pushed after a rebase onto B1 — B2 — CPU panels formatted as percentage. All panels plotting Also collapsed the duplicate On the non-blocking notes: delivery via the |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
dashboards/vm/console-logs.json (1)
21-40:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEnforce non-editable namespace scoping to preserve tenant isolation.
namespace/vmare editable textbox variables with*defaults (Lines 24-27, 34-37), and Line 56 injects them directly into the log query. That enables cluster-wide wildcard queries and undermines the stated cross-tenant protection objective. Replace textbox vars with constrained query variables (or constant/hidden scoped values) so wildcards cannot be user-selected for namespace scope.As per coding guidelines, review comments should be grounded in supplied context; the PR objective explicitly requires namespace filtering to prevent cross-tenant log exposure, which this editable wildcard setup violates.
Also applies to: 56-56
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dashboards/vm/console-logs.json` around lines 21 - 40, The namespace and vm variables are currently configured as editable textbox fields with wildcard defaults, which allows users to bypass tenant isolation by querying across all namespaces and VMs. Replace the textbox type configuration for both the namespace and vm variables with constrained query variables or constant/hidden scoped values that prevent wildcard selection, ensuring users cannot override the namespace filter that is injected into the log query. This will enforce tenant isolation as required by the cross-tenant protection objective.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@dashboards/vm/console-logs.json`:
- Around line 21-40: The namespace and vm variables are currently configured as
editable textbox fields with wildcard defaults, which allows users to bypass
tenant isolation by querying across all namespaces and VMs. Replace the textbox
type configuration for both the namespace and vm variables with constrained
query variables or constant/hidden scoped values that prevent wildcard
selection, ensuring users cannot override the namespace filter that is injected
into the log query. This will enforce tenant isolation as required by the
cross-tenant protection objective.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9d9a5d0d-d182-4fa6-83f1-9106d275b2db
📒 Files selected for processing (2)
dashboards/vm/console-logs.jsondashboards/vm/virtual-machines.json
💤 Files with no reviewable changes (1)
- dashboards/vm/virtual-machines.json
Aleksei Sviridkin (lexfrei)
added
area/virtualization
Aleksei Sviridkin (lexfrei)
left a comment
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
Both prior blockers are resolved in code. B1: the dashboards.list diff now adds only vm/virtual-machines and vm/console-logs; the 16 infra entries that belong in dashboards-infra.list are no longer re-introduced, so there is no tenant leak and no duplicate GrafanaDashboard resources. B2: every CPU panel plotting rate(kubevirt_vmi_cpu_*_usage_seconds_total) now uses short with "(cores)" — the Average CPU stat, the per-VM timeseries, the User/System breakdown, and the table's CPU column — with the percentage-style thresholds dropped; the remaining percentunit fields are the genuine 0–1 ratios (RAM %, Disk %), which is correct.
The one thing left before I can LGTM is visual: this adds two Grafana dashboards with no rendered screenshots. Please attach screenshots of both against a live cluster — virtual-machines populated with KubeVirt VM metrics, and console-logs showing guest-console-log output filtered by namespace — so the panels and queries can be confirmed to render.
a3d3d13 to
1dd789d
Compare
mattia-eleuteri
changed the title
Add a Grafana dashboard for KubeVirt virtual machines (CPU, memory, network, disk I/O per VM) to the tenant-visible dashboards list. Signed-off-by: Mattia Eleuteri <[email protected]>
1dd789d to
20d4375
Compare
mattia-eleuteri
changed the title
github-actions
Bot
added
size/XL
|
Rendered screenshots, as requested — captured against a live Cozystack cluster (our lab, 6 running VMs: 3 vm-instance VMs + CAPK tenant-Kubernetes workers), Grafana 11.4 with the dashboard imported from this branch as-is. Virtual Machines dashboard: Overview (active VMs, memory, CPU, network + per-VM table): CPU saturation / steal and Memory: Network: Disk I/O (throughput, latency, flush): One honest caveat: the Filesystem section panels ( Console-logs dashboard: dropped from this PR (last push). Reason: Cozystack ships KubeVirt with |
Aleksei Sviridkin (lexfrei)
left a comment
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
LGTM — both prior blockers (dashboards.list scoping, CPU cores units) are resolved, and the rendered screenshots confirm the dashboard populates against a live cluster.
Business context update: this now ships a single dashboard (vm/virtual-machines); the console-logs dashboard was dropped because Cozystack sets disableSerialConsoleLog, so it would render empty on a stock install (previously: two dashboards — VM metrics + console logs).
Non-blocking follow-ups
- VM Overview table —
net_rx/net_tx/iops_r/iops_waren't aggregated per VM.kubevirt_vmi_network_*carries aninterfacelabel andkubevirt_vmi_storage_iops_*adrivelabel (thedrive!="cloudinitdisk"filter confirms it); themergetransform joins on those labels beforeorganizedrops them, so a VM with more than one interface or more than one non-cloudinit disk renders as several rows with partial cells. It renders cleanly here because the lab VMs each have a single interface and disk. Wrapping the four expressions insum by (name)(...)collapses them to one row per VM. The CPU/RAM/Disk% columns are single-series and unaffected. - Filesystem panels (
kubevirt_vmi_filesystem_*) depend on qemu-guest-agent and show No data without it — fine as an opt-in section; worth a line in the row description so the empty panels aren't mistaken for a bug.
5 participants
Summary
dashboards/vm/virtual-machines.json— KubeVirt VM resource metrics (CPU, memory, network, storage) with namespace and VM template variablesdashboards/vm/console-logs.json— VM serial console log viewer via VictoriaLogs with namespace filteringvm/virtual-machinesandvm/console-logsentries todashboards.listProblem
dashboards.listreferencesvm/virtual-machinesandvm/console-logsbut the corresponding JSON files don't exist.The console-logs dashboard includes
kubernetes_namespace_namefiltering to prevent cross-tenant log exposure.Relates to #2194
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit