Repository navigation
fix(detect): workload memory requests add up in MiB, so requests under 1Gi count - #395
Merged
Merged
Conversation
…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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 autoand the valueskates deploygenerates. On main (6f4cf61),cli/pkg/detect/collector.go:1688-1704converts each container's memory request to whole GiB with integer division before adding it up:TotalMemRequests=0instead of 50 GiB.:889) then says Sufficient, andvaluesgen.gosizes brokers on memory other pods hold.500M,1G), fractions (0.5Gi) and plain bytes also read as 0.Changes
cli/pkg/detect/collector.go:TotalMemRequestsandPerNode[].MemRequestedkeep their GiB unit and every reader stays as it is.memoryMiBreads 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'scollector.goit reports100 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.shandscripts/check-cli-compat.sh: OK.cli/**triggers (Build & Test): this PR touches nothing underkates/../mvnw teston main'skates/tree ran 1,254 tests with 0 failures and 0 errors, once for all of Wave 1.Skipped locally:
govulncheck, asgo.modis unchanged.CI only:
./mvnw verifywith the*ITtests, 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.