Skip to content

Commit

Permalink
Use dmidecode to check available RAM in preflight checks (#730)
Browse files Browse the repository at this point in the history 
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)
  • Loading branch information
@tserong @bk201
tserong authored and bk201 committed May 21, 2024
1 parent 8b2b6c4 commit d5c2abc
Show file tree
Hide file tree
Showing 3 changed files with 209 additions and 43 deletions.
2 changes: 1 addition & 1 deletion Vagrantfile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ Vagrant.configure("2") do |config|
config.vm.provision "shell", inline: <<-SHELL
zypper ar --no-gpgcheck https://download.opensuse.org/repositories/home:/vcheng:/Packages/15.4/home:vcheng:Packages.repo
zypper --gpg-auto-import-keys refresh
zypper --non-interactive in yip
zypper --non-interactive in yip dmidecode
echo -e '#!/bin/sh\necho "fake $0"' > /usr/local/bin/fake
chmod a+x /usr/local/bin/fake
for f in /usr/sbin/harv-install /usr/sbin/cos-installer-shutdown ; do
Expand Down
161 changes: 126 additions & 35 deletions pkg/preflight/checks.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
"os/exec"
"strconv"
"strings"

"github.com/sirupsen/logrus"
)

const (
Expand Down Expand Up @@ -62,51 +64,140 @@ func (c CPUCheck) Run() (msg string, err error) {
return
}

func (c MemoryCheck) Run() (msg string, err error) {
meminfo, err := os.Open(procMemInfo)
if err != nil {
return
}
defer meminfo.Close()
scanner := bufio.NewScanner(meminfo)
var memTotalKiB int
for scanner.Scan() {
if n, _ := fmt.Sscanf(scanner.Text(), "MemTotal: %d kB", &memTotalKiB); n == 1 {
break
func (c MemoryCheck) Run() (string, error) {
// We're working in KiB because that's what the fallback /proc/meminfo uses
var memTotalKiB uint
var wiggleRoom float32 = 1.0

// dmidecode is part of sle-micro-rancher, see e.g.
// https://build.opensuse.org/projects/SUSE:SLE-15-SP4:Update:Products:Micro54/packages/SLE-Micro-Rancher/files/SLE-Micro-Rancher.kiwi?expand=1
//
// The output of `dmidecode -t 19` will include one or more
// Memory Array Mapped Address blocks, for example on a system
// with 512GiB RAM, we might see this:
//
// # dmidecode 3.5
// Getting SMBIOS data from sysfs.
// SMBIOS 2.8 present.
//
// Handle 0x0024, DMI type 19, 31 bytes
// Memory Array Mapped Address
// Starting Address: 0x00000000000
// Ending Address: 0x0007FFFFFFF
// Range Size: 2 GB
// Physical Array Handle: 0x000A
// Partition Width: 1
//
// Handle 0x0025, DMI type 19, 31 bytes
// Memory Array Mapped Address
// Starting Address: 0x0000000100000000k
// Ending Address: 0x000000807FFFFFFFk
// Range Size: 510 GB
// Physical Array Handle: 0x000B
// Partition Width: 1
//
// By adding together all the "Range Size" lines we can determine
// the amount of physical RAM installed. Note that it's possible
// for units to be specified in any of "bytes", "kB", "MB", "GB",
// "TB", "PB", "EB", "ZB", so we have to handle all of them...
// (see http://git.savannah.nongnu.org/cgit/dmidecode.git/tree/dmidecode.c#n283)
out, err := execCommand("/usr/sbin/dmidecode", "-t", "19").Output()
if err == nil {
rangeSizeToKiB := func(rangeSize uint, unit string) uint {
switch unit {
case "GB":
// We're probably usually going to see GB
return rangeSize << 20
case "MB":
// This seems unlikely
return rangeSize << 10
case "kB":
// This seems even more unlikely
return rangeSize
case "bytes":
// Seriously, are you kidding me?
return rangeSize >> 10
}
return 0
}

for _, line := range strings.Split(string(out), "\n") {
var rangeSize uint
var unit string
if n, _ := fmt.Sscanf(strings.TrimSpace(line), "Range Size: %d %s", &rangeSize, &unit); n == 2 {
if unit == "TB" || unit == "PB" || unit == "EB" || unit == "ZB" {
// If we've somehow got a Memory Array Mapped Address
// with one of these enormous units, let's just pretend
// we've got a terabyte of RAM and be done with it ;-)
logrus.Infof("Found Memory Array Mapped Address with Range Size %d %s, assuming 1 TiB RAM for preflight check", rangeSize, unit)
memTotalKiB = 1 << 30
break
}
memTotalKiB += rangeSizeToKiB(rangeSize, unit)
}
}
}

if memTotalKiB == 0 {
err = errors.New("unable to extract MemTotal from /proc/cpuinfo")
return
// Somehow, we didn't get anything out of dmidecode, fall back to
// parsing /proc/meminfo

meminfo, err := os.Open(procMemInfo)

if err != nil {
return "", err
}

defer meminfo.Close()
scanner := bufio.NewScanner(meminfo)

for scanner.Scan() {
if n, _ := fmt.Sscanf(scanner.Text(), "MemTotal: %d kB", &memTotalKiB); n == 1 {
break
}
}

if memTotalKiB == 0 {
return "", errors.New("unable to extract MemTotal from /proc/meminfo")
}

// MemTotal from /proc/cpuinfo is a bit less than the actual physical
// memory in the system, due to reserved RAM not being included, so
// we can't actually do a trivial check of MemTotalGiB < MinMemoryTest,
// because it will fail. For example:
// - A host with 32GiB RAM may report MemTotal 32856636 = 31.11GiB
// - A host with 64GiB RAM may report MemTotal 65758888 = 62.71GiB
// - A host with 128GiB RAM may report MemTotal 131841120 = 125.73GiB
// This means we have to test against a slightly lower number. Knocking
// 10% off is somewhat arbitrary but probably not unreasonable (e.g. for
// 32GB we're actually allowing anything over 28.8GB, and for 64GB we're
// allowing anything over 57.6GB).

wiggleRoom = 0.9

// Note that the above also means the warning messages below will be a
// bit off (e.g. something like "System reports 31GiB RAM" on a 32GiB
// system).
}
// MemTotal from /proc/cpuinfo is a bit less than the actual physical
// memory in the system, due to reserved RAM not being included, so
// we can't actually do a trivial check of MemTotalGiB < MinMemoryTest,
// because it will fail. For example:
// - A host with 32GiB RAM may report MemTotal 32856636 = 31.11GiB
// - A host with 64GiB RAM may report MemTotal 65758888 = 62.71GiB
// - A host with 128GiB RAM may report MemTotal 131841120 = 125.73GiB
// This means we have to test against a slightly lower number. Knocking
// 5% off is somewhat arbitrary but probably not unreasonable (e.g. for
// 32GB we're actually allowing anything over 30.4GB, and for 64GB we're
// allowing anything over 60.8GB).
// Note that the above also means the warning messages below will be a
// bit off (e.g. something like "System reports 31GiB RAM" on a 32GiB
// system).

memTotalMiB := memTotalKiB / (1 << 10)
memTotalGiB := memTotalKiB / (1 << 20)
memReported := fmt.Sprintf("%dGiB", memTotalGiB)

if memTotalGiB < 1 {
// Just in case someone runs it on a really tiny VM...
memReported = fmt.Sprintf("%dKiB", memTotalKiB)
memReported = fmt.Sprintf("%dMiB", memTotalMiB)
}
if float32(memTotalGiB) < (MinMemoryTest * 0.95) {
msg = fmt.Sprintf("Only %s RAM detected. Harvester requires at least %dGiB for testing and %dGiB for production use.",
memReported, MinMemoryTest, MinMemoryProd)
} else if float32(memTotalGiB) < (MinMemoryProd * 0.95) {
msg = fmt.Sprintf("%s RAM detected. Harvester requires at least %dGiB for production use.",
memReported, MinMemoryProd)

if float32(memTotalGiB) < (MinMemoryTest * wiggleRoom) {
return fmt.Sprintf("Only %s RAM detected. Harvester requires at least %dGiB for testing and %dGiB for production use.",
memReported, MinMemoryTest, MinMemoryProd), nil
} else if float32(memTotalGiB) < (MinMemoryProd * wiggleRoom) {
return fmt.Sprintf("%s RAM detected. Harvester requires at least %dGiB for production use.",
memReported, MinMemoryProd), nil
}
return

return "", nil
}

func (c VirtCheck) Run() (msg string, err error) {
Expand Down
89 changes: 82 additions & 7 deletions pkg/preflight/checks_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,61 @@ type fakeOutput struct {

var (
execOutputs = map[string]fakeOutput{
"nproc 4": {"4\n", 0},
"nproc 8": {"8\n", 0},
"nproc 16": {"16\n", 0},
"kvm": {"kvm\n", 0},
"metal": {"none\n", 1},
"nproc 4": {"4\n", 0},
"nproc 8": {"8\n", 0},
"nproc 16": {"16\n", 0},
"kvm": {"kvm\n", 0},
"metal": {"none\n", 1},
"dmidecode-fail": {"", 1},
"dmidecode-8GiB": {`# dmidecode 3.4
Getting SMBIOS data from sysfs.
SMBIOS 3.0.0 present.
Handle 0x1300, DMI type 19, 31 bytes
Memory Array Mapped Address
Starting Address: 0x00000000000
Ending Address: 0x0007FFFFFFF
Range Size: 2 GB
Physical Array Handle: 0x1000
Partition Width: 1
Handle 0x1301, DMI type 19, 31 bytes
Memory Array Mapped Address
Starting Address: 0x00100000000
Ending Address: 0x0027FFFFFFF
Range Size: 6 GB
Physical Array Handle: 0x1000
Partition Width: 1`, 0},
"dmidecode-32GiB": {`# dmidecode 3.4
Getting SMBIOS data from sysfs.
SMBIOS 3.0.0 present.
Handle 0x1300, DMI type 19, 31 bytes
Memory Array Mapped Address
Starting Address: 0x00000000000
Ending Address: 0x0007FFFFFFF
Range Size: 2 GB
Physical Array Handle: 0x1000
Partition Width: 1
Handle 0x1301, DMI type 19, 31 bytes
Memory Array Mapped Address
Starting Address: 0x00100000000
Ending Address: 0x0087FFFFFFF
Range Size: 30 GB
Physical Array Handle: 0x1000
Partition Width: 1`, 0},
"dmidecode-64GiB": {`# dmidecode 3.5
Getting SMBIOS data from sysfs.
SMBIOS 2.8 present.
Handle 0x0034, DMI type 19, 31 bytes
Memory Array Mapped Address
Starting Address: 0x00000000000
Ending Address: 0x00FFFFFFFFF
Range Size: 64 GB
Physical Array Handle: 0x002F
Partition Width: 8`, 0},
}
)

Expand Down Expand Up @@ -111,12 +161,37 @@ func TestVirtCheck(t *testing.T) {

}

func TestMemoryCheck(t *testing.T) {
func TestMemoryCheckDmiDecode(t *testing.T) {
defer func() { execCommand = exec.Command }()

expectedOutputs := map[string]string{
"dmidecode-8GiB": "Only 8GiB RAM detected. Harvester requires at least 32GiB for testing and 64GiB for production use.",
"dmidecode-32GiB": "32GiB RAM detected. Harvester requires at least 64GiB for production use.",
"dmidecode-64GiB": "",
}

check := MemoryCheck{}
for key, expectedOutput := range expectedOutputs {
execCommand = func(_ string, _ ...string) *exec.Cmd {
return fakeExecCommand(key)
}
msg, err := check.Run()
assert.Nil(t, err)
assert.Equal(t, expectedOutput, msg)
}
}

func TestMemoryCheckProcMemInfo(t *testing.T) {
defaultMemInfo := procMemInfo
defer func() { procMemInfo = defaultMemInfo }()
defer func() { execCommand = exec.Command }()

execCommand = func(_ string, _ ...string) *exec.Cmd {
return fakeExecCommand("dmidecode-fail")
}

expectedOutputs := map[string]string{
"./testdata/meminfo-512MiB": "Only 458112KiB RAM detected. Harvester requires at least 32GiB for testing and 64GiB for production use.",
"./testdata/meminfo-512MiB": "Only 447MiB RAM detected. Harvester requires at least 32GiB for testing and 64GiB for production use.",
"./testdata/meminfo-32GiB": "31GiB RAM detected. Harvester requires at least 64GiB for production use.",
"./testdata/meminfo-64GiB": "",
}
Expand Down

0 comments on commit d5c2abc

Please sign in to comment.