Skip to content
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

Use dmidecode to check available RAM in preflight checks #730

Merged
merged 1 commit into from
May 20, 2024

Conversation

tserong
Copy link
Contributor

@tserong tserong commented May 15, 2024

Problem:
The current implementation of the preflight memory check uses MemTotal from /proc/meminfo to try to figure out how much RAM there is. This number will always be a little bit low, which is confusing ("Why is it reporting only 60GiB when I know I have 64GiB?!?") and means we have to allow a certain amount of wiggle room when determining whether or not the system meets the minimum production requirements.

Solution:
This commit makes the following changes:

  1. We use dmidecode -t 19 to get the exact amount of memory installed in the system. No weird low values, no need for wiggle room.
  2. In case the above fails and we have to fall back to /proc/meminfo, increase the wiggle room from 5% to 10%.
  3. For really low RAM scenarios (e.g. tiny test VMs with <1GiB RAM), report available memory in MiB rather than KiB (nobody thinks in KiB anymore, right?)

Related Issue:
harvester/harvester#5599

Test plan:

  • Boot a system with <64GiB RAM. Verify that the warning message gives the exact amount of RAM you actually have (e.g. 16GiB, 32GiB), vs. undercounting like it used to do (e.g. 7GiB on an 8GiB system).

@tserong tserong requested review from Vicente-Cheng and bk201 May 15, 2024 08:53
Copy link
Contributor

@Vicente-Cheng Vicente-Cheng left a comment

Choose a reason for hiding this comment

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

LGTM! just a nit.

Thanks for the enhancement!

pkg/preflight/checks.go Show resolved Hide resolved
The current implementation of the preflight memory check uses `MemTotal`
from `/proc/meminfo` to try to figure out how much RAM there is.  This
number will always be a little bit low, which is confusing ("Why is it
reporting only 60GiB when I know I have 64GiB?!?") and means we have to
allow a certain amount of wiggle room when determining whether or not
the system meets the minimum production requirements.

This commit makes the following changes:

1. We use `dmidecode -t 19` to get the exact amount of memory installed
   in the system.  No weird low values, no need for wiggle room.
2. In case the above fails and we have to fall back to `/proc/meminfo`,
   increase the wiggle room from 5% to 10%.
3. For really low RAM scenarios (e.g. tiny test VMs with <1GiB RAM),
   report available memory in MiB rather than KiB (nobody thinks in
   KiB anymore, right?)

Related issue: harvester/harvester#5599

Signed-off-by: Tim Serong <[email protected]>
@tserong tserong force-pushed the wip-preflight-memory-dmidecode branch from 39f2b7e to 6a2d80c Compare May 20, 2024 04:47
@tserong tserong merged commit c76086f into harvester:master May 20, 2024
5 checks passed
@tserong tserong deleted the wip-preflight-memory-dmidecode branch May 20, 2024 09:46
@tserong
Copy link
Contributor Author

tserong commented May 20, 2024

@Mergifyio backport v1.3

Copy link

mergify bot commented May 20, 2024

backport v1.3

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request May 20, 2024
The current implementation of the preflight memory check uses `MemTotal`
from `/proc/meminfo` to try to figure out how much RAM there is.  This
number will always be a little bit low, which is confusing ("Why is it
reporting only 60GiB when I know I have 64GiB?!?") and means we have to
allow a certain amount of wiggle room when determining whether or not
the system meets the minimum production requirements.

This commit makes the following changes:

1. We use `dmidecode -t 19` to get the exact amount of memory installed
   in the system.  No weird low values, no need for wiggle room.
2. In case the above fails and we have to fall back to `/proc/meminfo`,
   increase the wiggle room from 5% to 10%.
3. For really low RAM scenarios (e.g. tiny test VMs with <1GiB RAM),
   report available memory in MiB rather than KiB (nobody thinks in
   KiB anymore, right?)

Related issue: harvester/harvester#5599

Signed-off-by: Tim Serong <[email protected]>
(cherry picked from commit c76086f)
bk201 pushed a commit that referenced this pull request May 21, 2024
The current implementation of the preflight memory check uses `MemTotal`
from `/proc/meminfo` to try to figure out how much RAM there is.  This
number will always be a little bit low, which is confusing ("Why is it
reporting only 60GiB when I know I have 64GiB?!?") and means we have to
allow a certain amount of wiggle room when determining whether or not
the system meets the minimum production requirements.

This commit makes the following changes:

1. We use `dmidecode -t 19` to get the exact amount of memory installed
   in the system.  No weird low values, no need for wiggle room.
2. In case the above fails and we have to fall back to `/proc/meminfo`,
   increase the wiggle room from 5% to 10%.
3. For really low RAM scenarios (e.g. tiny test VMs with <1GiB RAM),
   report available memory in MiB rather than KiB (nobody thinks in
   KiB anymore, right?)

Related issue: harvester/harvester#5599

Signed-off-by: Tim Serong <[email protected]>
(cherry picked from commit c76086f)
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.

3 participants