-
Notifications
You must be signed in to change notification settings - Fork 4k
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.
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
feat: humanize memory quantities using binary SI units #7428
Conversation
e60a57b
to
dad1519
Compare
In general I like this and will likely turn it on in production. |
Thanks! If the approvers are satisfied, I’ll be happy to document everything. :) |
/retest |
Passed , thanks 😀 |
dad1519
to
217d7f2
Compare
Jotting down some thoughts that I haven't had the time to dig into:
|
|
My gut feel here is that may be we allow 2 digits after the decimal place only? (ie: not configurable). For example, 1.5Gi is actually easy for a human and more precise than rounding up to 2Gi |
Fine with me. I can adjust :) |
217d7f2
to
0d5591b
Compare
PTAL @adrianmoisey |
Signed-off-by: Omer Aplatony <omerap12@gmail.com>
0d5591b
to
ab6d986
Compare
I like this, but am also curious what others think. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally lgtm, just a nit about the log
Co-authored-by: Adrian Moisey <adrian@changeover.za.net>
Done :) |
/lgtm Let's see if @kwiesmueller agrees with the log level |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kwiesmueller, omerap12 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Was this not released with 1.2.2 for some reason?
Maybe its missing from https://github.com/kubernetes/autoscaler/blob/master/vertical-pod-autoscaler/pkg/recommender/main.go#L58-L115? Or did I get it wrong in regard of how to use it? |
There is a change log here: https://github.com/kubernetes/autoscaler/releases/tag/vertical-pod-autoscaler-1.2.2 Patch releases don't get new features, only bug fixes or regressions. |
What type of PR is this?
/kind feature
What this PR does / why we need it:
Improves resource reporting by converting raw memory values (bytes) into human-readable binary units (KiB, MiB, GiB, TiB). Allowing users to quickly understand memory quantities without interpreting large byte numbers.
Which issue(s) this PR fixes:
Fixes #7417
Special notes for your reviewer:
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: