Skip to content

fix(detect): workload memory requests add up in MiB, so requests under 1Gi count - #395

Merged
bmscomp merged 1 commit into
mainfrom
fix/detect-memory-requests-mib
Oct 18, 2026
Merged

bmscomp merged 1 commit into
mainfrom
fix/detect-memory-requests-mib

Conversation

@bmscomp

@bmscomp bmscomp commented Oct 18, 2026

Copy link
Copy Markdown
Owner

Item 1.7b of the CLI performance and stability plan (finding F18, second half). Independent of the other Wave 1 PRs; it merges cleanly with 1.7a (#394), which touches the same two files elsewhere.

Why

Detect's sizing reads are wrong for memory, and detect feeds kates auto and the values kates deploy generates. On main (6f4cf61), cli/pkg/detect/collector.go:1688-1704 converts each container's memory request to whole GiB with integer division before adding it up:

  • 256Mi counts as 0, so 100 pods with two 256Mi containers each report TotalMemRequests=0 instead of 50 GiB.
  • The capacity check (:889) then says Sufficient, and valuesgen.go sizes brokers on memory other pods hold.
  • Decimal suffixes (500M, 1G), fractions (0.5Gi) and plain bytes also read as 0.

Changes

cli/pkg/detect/collector.go:

  • Requests add up in MiB, cluster-wide and per node, and become GiB once, so TotalMemRequests and PerNode[].MemRequested keep their GiB unit and every reader stays as it is.
  • New memoryMiB reads a request with any Kubernetes memory suffix (Ki, Mi, Gi, Ti, k, M, G, T) or none, as bytes. Anything else, a missing request included, still reads as 0.

Tests

Added:

  • TestGetWorkloadPressure_SumsMemoryBeforeRounding: 100 pods with two 256Mi containers each, on two nodes, request 50 GiB, 25 per node. On main's collector.go it reports 100 pods requesting 0 GiB.
  • TestMemoryMiB: 256Mi, 2Gi, 0.5Gi, 524288Ki, plain bytes, 1Ti, 1G, 500M, empty, and an unparsable value.

Run locally, passing:

  • cd cli && go test ./..., go test -race ./pkg/detect, go vet ./....
  • golangci-lint run --new-from-rev=origin/main ./...: 0 issues.
  • scripts/check-cli-style.sh and scripts/check-cli-compat.sh: OK.
  • The backend suite, which cli/** triggers (Build & Test): this PR touches nothing under kates/. ./mvnw test on main's kates/ tree ran 1,254 tests with 0 failures and 0 errors, once for all of Wave 1.

Skipped locally: govulncheck, as go.mod is unchanged.

CI only: ./mvnw verify with the *IT tests, the Kind and Docker jobs, CodeQL and Trivy.

Deliberately not done

CPU requests on the lines above still read a fraction of a core (0.5) as 0. The plan does not list it, and it is left for a separate change.

…r 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.
@bmscomp
bmscomp merged commit d21c397 into main Oct 18, 2026
55 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant