From abec624a1e043549d84090de8d359fd6b1970e06 Mon Sep 17 00:00:00 2001 From: Said Boudjelda Date: Sun, 18 Oct 2026 15:52:46 +0200 Subject: [PATCH] fix(detect): workload memory requests add up in MiB, so requests under 1Gi count Detect converted each container's memory request to whole GiB with integer division before adding it up, so 256Mi counted as 0: 100 pods with two 256Mi containers each requested 0 GiB instead of 50. The capacity check then said Sufficient, and the values detect generates for kates auto and kates deploy sized brokers on memory other pods hold. Decimal suffixes (M, G), fractions (0.5Gi) and plain bytes read as 0 too. Requests now add up in MiB, cluster-wide and per node, and become GiB once. memoryMiB reads every Kubernetes memory suffix, binary and decimal, and plain bytes; anything else still reads as 0. --- cli/pkg/detect/collector.go | 54 +++++++++++++++++++++----------- cli/pkg/detect/collector_test.go | 47 +++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 18 deletions(-) diff --git a/cli/pkg/detect/collector.go b/cli/pkg/detect/collector.go index 3deb2eee..ddebeace 100644 --- a/cli/pkg/detect/collector.go +++ b/cli/pkg/detect/collector.go @@ -1664,9 +1664,11 @@ func (c *Collector) getWorkloadPressure() WorkloadPressure { return wp } - // Per-node accumulation + // Per-node accumulation. Memory adds up in MiB and becomes GiB once: + // converted per container, every request under 1Gi counted as nothing. nodeCPU := make(map[string]int) - nodeMem := make(map[string]int) + nodeMemMiB := make(map[string]float64) + var totalMemMiB float64 for _, pod := range data.Items { wp.TotalPods++ @@ -1675,7 +1677,7 @@ func (c *Collector) getWorkloadPressure() WorkloadPressure { } for _, c := range pod.Spec.Containers { cpuStr := c.Resources.Requests.CPU - memStr := c.Resources.Requests.Memory + memMiB := memoryMiB(c.Resources.Requests.Memory) // Parse CPU cpuM := 0 @@ -1686,30 +1688,19 @@ func (c *Collector) getWorkloadPressure() WorkloadPressure { cpuM = cores * 1000 } - // Parse Memory (to GiB, approximate) - memGi := 0 - if strings.HasSuffix(memStr, "Gi") { - memGi, _ = strconv.Atoi(strings.TrimSuffix(memStr, "Gi")) - } else if strings.HasSuffix(memStr, "Mi") { - mi, _ := strconv.Atoi(strings.TrimSuffix(memStr, "Mi")) - memGi = mi / 1024 // rough conversion - } else if strings.HasSuffix(memStr, "Ki") { - ki, _ := strconv.Atoi(strings.TrimSuffix(memStr, "Ki")) - memGi = ki / 1048576 - } - wp.TotalCPURequests += cpuM - wp.TotalMemRequests += memGi + totalMemMiB += memMiB nodeCPU[pod.Spec.NodeName] += cpuM - nodeMem[pod.Spec.NodeName] += memGi + nodeMemMiB[pod.Spec.NodeName] += memMiB } } + wp.TotalMemRequests = int(totalMemMiB / 1024) for node, cpu := range nodeCPU { wp.PerNode = append(wp.PerNode, NodePressure{ Name: node, CPURequested: cpu, - MemRequested: nodeMem[node], + MemRequested: int(nodeMemMiB[node] / 1024), }) } sort.Slice(wp.PerNode, func(i, j int) bool { @@ -1719,6 +1710,33 @@ func (c *Collector) getWorkloadPressure() WorkloadPressure { return wp } +// memUnits are the Kubernetes memory quantity suffixes, in bytes. "Mi" and "M" +// cannot both match: a binary suffix ends in "i". +var memUnits = []struct { + suffix string + bytes float64 +}{ + {"Ki", 1 << 10}, {"Mi", 1 << 20}, {"Gi", 1 << 30}, {"Ti", 1 << 40}, + {"k", 1e3}, {"M", 1e6}, {"G", 1e9}, {"T", 1e12}, +} + +// memoryMiB reads a memory request as MiB: a number of bytes, with or without +// one of memUnits. Anything else, a missing request included, reads as 0. +func memoryMiB(quantity string) float64 { + bytes := 1.0 + for _, u := range memUnits { + if n, ok := strings.CutSuffix(quantity, u.suffix); ok { + quantity, bytes = n, u.bytes + break + } + } + n, err := strconv.ParseFloat(quantity, 64) + if err != nil { + return 0 + } + return n * bytes / (1 << 20) +} + func hydrateWorkloadPressure(nodes []NodeInfo, wp WorkloadPressure) WorkloadPressure { requests := map[string]NodePressure{} for _, node := range wp.PerNode { diff --git a/cli/pkg/detect/collector_test.go b/cli/pkg/detect/collector_test.go index 1b6fbb63..93d5d58d 100644 --- a/cli/pkg/detect/collector_test.go +++ b/cli/pkg/detect/collector_test.go @@ -579,3 +579,50 @@ func TestCollect_ReadOnlyIssuesNoWrites(t *testing.T) { t.Errorf("ClusterDomain = %q, want the cluster.local default", report.Network.ClusterDomain) } } + +// Each container's request was cut to whole GiB, so 256Mi counted as 0, and +// 100 pods of two 256Mi containers requested nothing: the capacity check said +// Sufficient, and broker sizing took that memory as free. +func TestGetWorkloadPressure_SumsMemoryBeforeRounding(t *testing.T) { + pod := func(node string) string { + return `{"metadata":{"namespace":"apps"},"spec":{"nodeName":"` + node + `","containers":[` + + `{"resources":{"requests":{"cpu":"100m","memory":"256Mi"}}},` + + `{"resources":{"requests":{"cpu":"100m","memory":"256Mi"}}}]}}` + } + pods := make([]string, 100) + for i := range pods { + pods[i] = pod(fmt.Sprintf("node-%d", i%2)) + } + m := NewMockExecutor() + m.Set(`{"items":[`+strings.Join(pods, ",")+`]}`, + "kubectl", "get", "pods", "-A", "-o", "json", "--field-selector=status.phase=Running") + + wp := NewCollector(m).getWorkloadPressure() + if wp.TotalPods != 100 || wp.TotalMemRequests != 50 { + t.Errorf("%d pods requesting %d GiB, want 100 pods requesting 50 GiB", wp.TotalPods, wp.TotalMemRequests) + } + for _, node := range wp.PerNode { + if node.MemRequested != 25 { + t.Errorf("%s requests %d GiB, want 25", node.Name, node.MemRequested) + } + } +} + +func TestMemoryMiB(t *testing.T) { + for quantity, want := range map[string]float64{ + "256Mi": 256, + "2Gi": 2048, + "0.5Gi": 512, + "524288Ki": 512, + "268435456": 256, + "1Ti": 1 << 20, + "1G": 1e9 / (1 << 20), + "500M": 500e6 / (1 << 20), + "": 0, + "lots": 0, + } { + if got := memoryMiB(quantity); got != want { + t.Errorf("memoryMiB(%q) = %v, want %v", quantity, got, want) + } + } +}