Skip to content

Refactor kvm resource capacity kpi to use effective capacity #585

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
SoWieMarkus merged 2 commits into from
Mar 17, 2026
Merged

Refactor kvm resource capacity kpi to use effective capacity #585

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8b6871b
Refactor kvm resource capacity kpi to use effective capacity
SoWieMarkus Mar 17, 2026
ce1fbf9
Feedback
SoWieMarkus Mar 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions internal/knowledge/kpis/plugins/compute/resource_capacity_kvm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,20 +154,30 @@ func (k *KVMResourceCapacityKPI) Collect(ch chan<- prometheus.Metric) {
}

for _, hypervisor := range hvs.Items {
cpuTotal, hasCPUTotal := hypervisor.Status.Capacity["cpu"]
ramTotal, hasRAMTotal := hypervisor.Status.Capacity["memory"]
if hypervisor.Status.EffectiveCapacity == nil {
slog.Warn("hypervisor with nil effective capacity, skipping", "host", hypervisor.Name)
continue
}

cpuTotal, hasCPUTotal := hypervisor.Status.EffectiveCapacity[hv1.ResourceCPU]
ramTotal, hasRAMTotal := hypervisor.Status.EffectiveCapacity[hv1.ResourceMemory]

if !hasCPUTotal || !hasRAMTotal {
slog.Error("hypervisor missing cpu or ram total capacity", "hypervisor", hypervisor.Name)
continue
}

cpuUsed, hasCPUUtilized := hypervisor.Status.Allocation["cpu"]
if cpuTotal.IsZero() || ramTotal.IsZero() {
slog.Warn("hypervisor with zero cpu or ram total capacity, skipping", "host", hypervisor.Name)
continue
}

cpuUsed, hasCPUUtilized := hypervisor.Status.Allocation[hv1.ResourceCPU]
if !hasCPUUtilized {
cpuUsed = resource.MustParse("0")
}

ramUsed, hasRAMUtilized := hypervisor.Status.Allocation["memory"]
ramUsed, hasRAMUtilized := hypervisor.Status.Allocation[hv1.ResourceMemory]
if !hasRAMUtilized {
ramUsed = resource.MustParse("0")
}
Expand Down
61 changes: 55 additions & 6 deletions internal/knowledge/kpis/plugins/compute/resource_capacity_kvm_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,55 @@ func TestKVMResourceCapacityKPI_Collect(t *testing.T) {
hypervisors []hv1.Hypervisor
expectedMetrics map[string][]expectedMetric // metric_name -> []expectedMetric
}{
{
name: "single hypervisor with nil effective capacity",
hypervisors: []hv1.Hypervisor{
{
ObjectMeta: v1.ObjectMeta{
Name: "node001-bb088",
Labels: map[string]string{
"topology.kubernetes.io/zone": "qa-1a",
},
},
Status: hv1.HypervisorStatus{
EffectiveCapacity: nil, // Simulate nil effective capacity
Allocation: map[hv1.ResourceName]resource.Quantity{
hv1.ResourceCPU: resource.MustParse("64"),
hv1.ResourceMemory: resource.MustParse("256Gi"),
},
Traits: []string{},
},
},
},
// No metrics should be emitted for this hypervisor since effective capacity is nil
expectedMetrics: map[string][]expectedMetric{},
},
Comment on lines +50 to +72
Copy link

@coderabbitai coderabbitai bot Mar 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

expectedMetrics: {} does not actually assert “no metrics emitted”.

Line 71 and Line 97 define empty expectations, but the verifier only iterates expected entries. These cases pass even if metrics are emitted.

Suggested fix 
@@
-			// Verify expected metrics
+			// Verify expected metrics
+			if len(tt.expectedMetrics) == 0 {
+				if len(actualMetrics) != 0 {
+					t.Errorf("expected no metrics, got %v", actualMetrics)
+				}
+				return
+			}
+
+			for metricName := range actualMetrics {
+				if _, ok := tt.expectedMetrics[metricName]; !ok {
+					t.Errorf("unexpected metric %q found in actual metrics", metricName)
+				}
+			}
+
 			for metricName, expectedList := range tt.expectedMetrics {

Also applies to: 73-98

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@internal/knowledge/kpis/plugins/compute/resource_capacity_kvm_test.go` around
lines 50 - 72, The test uses an empty expectedMetrics map which doesn't fail
when metrics are unexpectedly emitted because the verifier only iterates
expected entries; update the tests (cases with name "single hypervisor with nil
effective capacity" and the other empty-expectation case) to explicitly assert
no metrics are emitted: either set a sentinel (e.g., expectedMetrics = nil or
add expectNoMetrics: true) and after collecting metrics assert
len(emittedMetrics) == 0, or update the verifier (the test helper that currently
iterates expectedMetrics) to treat an empty/nil expectedMetrics as a negative
assertion by scanning actual emitted metrics and failing if any metric is
present. Reference the test's expectedMetrics variable and the verifier helper
used in resource_capacity_kvm_test.go when making the change.

{
name: "single hypervisor with zero total capacity",
hypervisors: []hv1.Hypervisor{
{
ObjectMeta: v1.ObjectMeta{
Name: "node001-bb088",
Labels: map[string]string{
"topology.kubernetes.io/zone": "qa-1a",
},
},
Status: hv1.HypervisorStatus{
EffectiveCapacity: map[hv1.ResourceName]resource.Quantity{
hv1.ResourceCPU: resource.MustParse("0"), // Simulate zero CPU capacity
hv1.ResourceMemory: resource.MustParse("0"), // Simulate zero RAM capacity
},
Allocation: map[hv1.ResourceName]resource.Quantity{
hv1.ResourceCPU: resource.MustParse("0"),
hv1.ResourceMemory: resource.MustParse("0"),
},
Traits: []string{},
},
},
},
// No metrics should be emitted for this hypervisor since total capacity is zero
expectedMetrics: map[string][]expectedMetric{},
},
{
name: "single hypervisor with default traits",
hypervisors: []hv1.Hypervisor{
Expand All @@ -58,7 +107,7 @@ func TestKVMResourceCapacityKPI_Collect(t *testing.T) {
},
},
Status: hv1.HypervisorStatus{
Capacity: map[hv1.ResourceName]resource.Quantity{
EffectiveCapacity: map[hv1.ResourceName]resource.Quantity{
hv1.ResourceCPU: resource.MustParse("128"),
hv1.ResourceMemory: resource.MustParse("512Gi"),
},
Expand Down Expand Up @@ -148,7 +197,7 @@ func TestKVMResourceCapacityKPI_Collect(t *testing.T) {
},
},
Status: hv1.HypervisorStatus{
Capacity: map[hv1.ResourceName]resource.Quantity{
EffectiveCapacity: map[hv1.ResourceName]resource.Quantity{
hv1.ResourceCPU: resource.MustParse("256"),
hv1.ResourceMemory: resource.MustParse("1Ti"),
},
Expand Down Expand Up @@ -209,7 +258,7 @@ func TestKVMResourceCapacityKPI_Collect(t *testing.T) {
},
},
Status: hv1.HypervisorStatus{
Capacity: map[hv1.ResourceName]resource.Quantity{
EffectiveCapacity: map[hv1.ResourceName]resource.Quantity{
hv1.ResourceCPU: resource.MustParse("64"),
hv1.ResourceMemory: resource.MustParse("256Gi"),
},
Expand Down Expand Up @@ -255,7 +304,7 @@ func TestKVMResourceCapacityKPI_Collect(t *testing.T) {
},
},
Status: hv1.HypervisorStatus{
Capacity: map[hv1.ResourceName]resource.Quantity{
EffectiveCapacity: map[hv1.ResourceName]resource.Quantity{
hv1.ResourceCPU: resource.MustParse("100"),
hv1.ResourceMemory: resource.MustParse("200Gi"),
},
Expand All @@ -274,7 +323,7 @@ func TestKVMResourceCapacityKPI_Collect(t *testing.T) {
},
},
Status: hv1.HypervisorStatus{
Capacity: map[hv1.ResourceName]resource.Quantity{
EffectiveCapacity: map[hv1.ResourceName]resource.Quantity{
hv1.ResourceCPU: resource.MustParse("200"),
hv1.ResourceMemory: resource.MustParse("400Gi"),
},
Expand Down Expand Up @@ -332,7 +381,7 @@ func TestKVMResourceCapacityKPI_Collect(t *testing.T) {
},
},
Status: hv1.HypervisorStatus{
Capacity: map[hv1.ResourceName]resource.Quantity{
EffectiveCapacity: map[hv1.ResourceName]resource.Quantity{
hv1.ResourceCPU: resource.MustParse("96"),
hv1.ResourceMemory: resource.MustParse("384Gi"),
},
Expand Down
Loading