From 1800af10fcd13c7d057d9ac0712b9fc996ef3e9e Mon Sep 17 00:00:00 2001 From: Andreas Kupries Date: Wed, 7 Oct 2020 11:53:45 -0700 Subject: [PATCH 1/2] fix: Query cgroup memory data and fuse with /proc data for more reliable information. The standard API for memory limits and usage under /proc are not containerized, and return host data when used in a container. This is a source of trouble for everything trying to configure itself based on available memory, causing it to work with bogus data. Inside containers the cgroup information rules instead. Trying to detect when running inside a container or not looks to have many approaches, which are either unreliable, or non-portable, or both. The approach here is to query both sets of data, and then fuse them for accuracy. As we ask for the available memory taking the min of cgroup and proc data is the safe choice for this fusion. It assumes that inside a container the cgroup data is likely less than the proc data, due to the container getting limited. And outside the container the cgroup data should not be less than proc. (That said, in the local box, having 36GiB memory cgroup reports a limit of nearly 8EiB (yes, Exa; Exact value is 8EiB-4KiB; As far as is known the absolute limit for Linux virtual memory management). As proc reports the 36GiB correctly the min is again the proper choice for such systems where cgroup on the host is that way out of whack). As for containers without limits set by docker or other runtime, cgroup data seems to be that of the host. The min-fusion again gives the best possible result, i.e. the host memory. Note that getting the cgroup information is convoluted. - We have v1 and v2 controllers. Both can be mounted, some mixture, or neither. - The location of the mounts is arbitrary. - The process the library is part of is likely not managed by the root cgroup. The actual controlling group is needed. - Swap usage is not present if swap accounting is not active. The gory details are noted in code comments. Added tests. Added clean up of the test's temp directory. Added for most of the mock file setup for more readable tests. --- sigar_linux.go | 276 ++++++++++++++++ sigar_linux_test.go | 770 ++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 990 insertions(+), 56 deletions(-) diff --git a/sigar_linux.go b/sigar_linux.go index 8cc3fbd38..0975d0991 100644 --- a/sigar_linux.go +++ b/sigar_linux.go @@ -3,6 +3,7 @@ package sigar import ( "bufio" "bytes" + "errors" "io" "io/ioutil" "os" @@ -19,11 +20,47 @@ var system struct { } var Procd string +var Sysd1 string +var Sysd2 string + +// Files in system directories used here +// - Procd +// - /stat +// - /meminfo +// - /self/cgroup | 'grep :memory:' | split ':' | last => cgroup +// - /self/cgroup | 'grep ::' | split ':' | last => cgroup/fallback +// - /self/mounts +// - Sysd1 (cgroup v1) +// - memory//memory.limit_in_bytes +// - memory//memory.stat +// - Sysd2 (cgroup v2) +// - /memory.high +// - /memory.current +// - /memory.swap.current +// +// While Procd is fixed `/proc` the `Sysd*` directories are +// dynamic. I.e. while there are semi-standard mount points for the +// cgroup controllers, this is just convention. They can be mounted +// anywhere. The file `/proc/self/mounts` contains the information we +// need. func init() { system.ticks = 100 // C.sysconf(C._SC_CLK_TCK) Procd = "/proc" + Sysd1 = "" + Sysd2 = "" + + determineControllerMounts(&Sysd1, &Sysd2) + + // Fallbacks for cgroup controller mount points if nothing was + // found in /proc/self/mounts + if Sysd1 == "" { + Sysd1 = "/sys/fs/cgroup/memory" + } + if Sysd2 == "" { + Sysd2 = "/sys/fs/cgroup/unified" + } // grab system boot time readFile(Procd+"/stat", func(line string) bool { @@ -86,6 +123,70 @@ func (self *Mem) Get() error { self.Used = self.Total - self.Free self.ActualUsed = self.Total - self.ActualFree + // Instead of detecting if this code is run within a container + // or not (*), we simply attempt to retrieve the cgroup + // information about memory limits and usage and if present + // incorporate them into the results. + // + // 0. If we are unable to determine the Cgroup for the process + // we ignore it and stay with the host data. + // + // 1. If the cgroup limit is not available we ignore it and + // stay with the host data. + // + // 2. Note that we are taking the smaller of host total and + // cgroup limit, as the safer value for the total. The + // reason here is that there are Linux systems which report + // something like 8 EiB (Exa!) (**) as the cgroup limit, on + // systems which have only 64 GiB (Giga) of physical RAM. + // + // (*) There does not seem to be a truly reliable and portable + // means of detecting execution inside a container vs + // outside. Between all the platforms (macos, linux, + // windows), and container runtimes (docker, lxc, oci, ...). + // + // (**) The exact value actually is 2^63 - 4096, i.e + // 8 EiB - 4 KiB. This is, as far as is known, the + // maximum limit of the Linux virtual memory system. + + var cgroup string + if err := determineSelfCgroup(&cgroup); err != nil { + // Unable to determine process' Cgroup + return nil + } + + cgroupLimit, err := determineMemoryLimit(cgroup) + // (x) If the limit is not available or bogus we keep the host data as limit. + + if err == nil && cgroupLimit < self.Total { + // See (2) above why only a cgroup limit less than the + // host total is accepted as the new total available + // memory in the cgroup. + self.Total = cgroupLimit + } + + rss, err := determineMemoryUsage(cgroup) + + if err != nil { + return nil + } + + swap, err := determineSwapUsage(cgroup) + if err != nil { + // Swap information is optional. I.e. the kernel may + // have swap accounting disabled. Because of this any + // kind of trouble determining the swap usage is + // mapped to `no swap used`. This allows us to limp + // on with some inaccuracies, instead of aborting. + swap = 0 + } + + self.Used = rss + swap + self.Free = self.Total - self.Used + + self.ActualUsed = self.Used + self.ActualFree = self.Free + return nil } @@ -316,6 +417,119 @@ func (self *ProcExe) Get(pid int) error { return nil } +func determineSwapUsage(cgroup string) (uint64, error) { + // Check v2 over v1 + usageAsString, err := ioutil.ReadFile(Sysd2 + cgroup + "/memory.swap.current") + if err == nil { + return strtoull(strings.Split(string(usageAsString), "\n")[0]) + } + + var swap uint64 + table := map[string]*uint64{ + "swap": &swap, + } + + err, found := parseCgroupMeminfo(Sysd1+cgroup, table) + if err == nil { + if !found { + // If no data was found, simply claim `zero swap used`. + return 0, errors.New("no data found") + } + return swap, nil + } + + return 0, err +} + +func determineMemoryUsage(cgroup string) (uint64, error) { + // Check v2 over v1 + usageAsString, err := ioutil.ReadFile(Sysd2 + cgroup + "/memory.current") + if err == nil { + return strtoull(strings.Split(string(usageAsString), "\n")[0]) + } + + var rss uint64 + table := map[string]*uint64{ + "total_rss": &rss, + } + + err, found := parseCgroupMeminfo(Sysd1+cgroup, table) + if err == nil { + if !found { + return 0, errors.New("no data found") + } + return rss, nil + } + + return 0, err +} + +func determineMemoryLimit(cgroup string) (uint64, error) { + // Check v2 over v1 + limitAsString, err := ioutil.ReadFile(Sysd2 + cgroup + "/memory.high") + if err == nil { + val := strings.Split(string(limitAsString), "\n")[0] + if val == "max" { + return 0, errors.New("no limit") + // See (x) in the caller where this keeps the host's self.Total. + } + return strtoull(val) + } + + limitAsString, err = ioutil.ReadFile(Sysd1 + cgroup + "/memory.limit_in_bytes") + if err == nil { + return strtoull(strings.Split(string(limitAsString), "\n")[0]) + } + + return 0, err +} + +func determineSelfCgroup(cgroup *string) error { + // - /proc/self/cgroup + // Expected line syntax - id:tag:path + // Three fields required in each line. + + // Look for a cgroup v1 memory controller first + err := readFile(Procd+"/self/cgroup", func(line string) bool { + fields := strings.Split(line, ":") + // Match: `*:memory:/path` + if len(fields) < 3 { + return true + } + if fields[1] == "memory" { + *cgroup = strings.Trim(fields[len(fields)-1], " ") + } + return true + }) + if err != nil { + return err + } + if *cgroup != "" { + return nil + } + + // Fall back to a cgroup v2 memory controller + err = readFile(Procd+"/self/cgroup", func(line string) bool { + fields := strings.Split(line, ":") + // Match: `0::/path` + if len(fields) < 3 { + return true + } + if (fields[0] == "0") && (fields[1] == "") { + *cgroup = strings.Trim(fields[len(fields)-1], " ") + } + return true + }) + if err != nil { + return err + } + if *cgroup != "" { + return nil + } + + return errors.New("unable to determine control group") +} + func parseMeminfo(table map[string]*uint64) error { return readFile(Procd+"/meminfo", func(line string) bool { fields := strings.Split(line, ":") @@ -332,6 +546,27 @@ func parseMeminfo(table map[string]*uint64) error { }) } +func parseCgroupMeminfo(cgroupDir string, table map[string]*uint64) (error, bool) { + var found bool + err := readFile(cgroupDir+"/memory.stat", func(line string) bool { + fields := strings.Split(line, " ") + if ptr := table[fields[0]]; ptr != nil { + num := strings.TrimLeft(fields[1], " ") + val, err := strtoull(strings.Fields(num)[0]) + if err == nil { + *ptr = val + found = true + } + } + + return true + }) + if err != nil { + return err, false + } + return nil, found +} + func parseCpuStat(self *Cpu, line string) error { fields := strings.Fields(line) @@ -390,3 +625,44 @@ func readProcFile(pid int, name string) ([]byte, error) { return contents, err } + +func determineControllerMounts(sysd1, sysd2 *string) { + // grab cgroup controller mount points + readFile(Procd+"/self/mounts", func(line string) bool { + + // Entries have the form `device path type options`. + // The elements are separated by single spaces. + // + // v2: `path` element of entry fulfilling `type == "cgroup2"`. + // v1: `path` element of entry fulfilling `type == "cgroup" && options ~ "memory"` + // + // NOTE: The `device` column can be anything. It + // cannot be used to pare down the set of entries + // going into the full check. + + fields := strings.Split(line, " ") + if len(fields) < 4 { + return true + } + + mpath := fields[1] + mtype := fields[2] + moptions := fields[3] + + if mtype == "cgroup2" { + if *sysd2 != "" { + panic("Multiple cgroup v2 mount points") + } + *sysd2 = mpath + return true + } + if mtype == "cgroup" && strings.Contains(moptions, "memory") { + if *sysd1 != "" { + panic("Multiple cgroup v1 mount points") + } + *sysd1 = mpath + return true + } + return true + }) +} diff --git a/sigar_linux_test.go b/sigar_linux_test.go index 95b70b683..240b9900a 100644 --- a/sigar_linux_test.go +++ b/sigar_linux_test.go @@ -2,24 +2,154 @@ package sigar import ( "io/ioutil" + "os" + "path/filepath" "time" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" ) -var _ = Describe("sigarLinux", func() { - var procd string +// Helpers. Create various system information files +var procd string + +func setupFile(path, contents string) { + _ = os.MkdirAll(filepath.Dir(path), 0755) + err := ioutil.WriteFile(path, []byte(contents), 0444) + Expect(err).ToNot(HaveOccurred()) +} + +func cgroupSetup(contents string) { + setupFile(procd+"/self/cgroup", contents+"\n") +} + +func memLimitSetup1(cg, contents string) { + setupFile(procd+"/memory"+cg+"/memory.limit_in_bytes", contents+"\n") +} + +func memLimitSetup2(cg, contents string) { + setupFile(procd+cg+"/memory.high", contents+"\n") +} + +func memStatSetup(cg, contents string) { + setupFile(procd+"/memory"+cg+"/memory.stat", contents+"\n") +} + +func memStatWithSwap(cg string) { + memStatSetup(cg, `total_rss 14108536832 +swap 290564089`) +} + +func memStatWithoutSwap(cg string) { + memStatSetup(cg, `total_rss 14108536832`) +} + +func memUsageSetup2(cg, contents string) { + setupFile(procd+cg+"/memory.current", contents+"\n") +} + +func swapUsageSetup1(cg, contents string) { + setupFile(procd+"/memory"+cg+"/memory.stat", contents+"\n") +} + +func swapUsageSetup2(cg, contents string) { + setupFile(procd+cg+"/memory.swap.current", contents+"\n") +} +func memUsageWithSwap(cg string) { + memUsageSetup2(cg, `14108536832`) + swapUsageSetup2(cg, `290564089`) +} + +func memUsageWithoutSwap(cg string) { + memUsageSetup2(cg, `14108536832`) +} + +func memInfoSetup(contents string) { + setupFile(procd+"/meminfo", contents+"\n") +} + +func memInfoWithMemAvailable() { + memInfoSetup(` +MemTotal: 35008180 kB +MemFree: 487816 kB +MemAvailable: 20913400 kB +Buffers: 249244 kB +Cached: 5064684 kB +`) +} + +func memInfoWithoutMemAvailable() { + memInfoSetup(` +MemTotal: 35008180 kB +MemFree: 487816 kB +Buffers: 249244 kB +Cached: 5064684 kB +`) +} + +var _ = Describe("sigarLinux", func() { BeforeEach(func() { var err error procd, err = ioutil.TempDir("", "sigarTests") Expect(err).ToNot(HaveOccurred()) + // Can share the directory, no overlap in files used Procd = procd + Sysd1 = procd + "/memory" + Sysd2 = procd }) AfterEach(func() { Procd = "/proc" + Sysd1 = "/sys/fs/cgroup/unified" + Sysd2 = "/sys/fs/cgroup/memory" + + err := os.RemoveAll(procd) + Expect(err).ToNot(HaveOccurred()) + }) + + Describe("Initialization subset: Cgroup Controller Mountpoints", func() { + Describe("No mounts", func() { + var sys1, sys2 string + + BeforeEach(func() { + setupFile(procd+"/self/mounts", ` +device path type options +alpha /fox cgroup options +beta /dog jumping memory +cgroup /cat sleeping irrelevant +delta /carp +`) + }) + + It("it is a no-op", func() { + determineControllerMounts(&sys1, &sys2) + Expect(sys1).To(Equal("")) + Expect(sys2).To(Equal("")) + }) + }) + + Describe("Mounts", func() { + var sys1, sys2 string + + BeforeEach(func() { + setupFile(procd+"/self/mounts", ` +device path type options +memory1 /somewhere/over/the/rainbow cgroup dummy,memory,and,other +memory2 /smart/fox/jumped/by/lazy/dog cgroup2 irrelevant,options +`) + }) + + It("it extracts the mounts", func() { + determineControllerMounts(&sys1, &sys2) + Expect(sys1).To(Equal("/somewhere/over/the/rainbow")) + Expect(sys2).To(Equal("/smart/fox/jumped/by/lazy/dog")) + }) + }) + + // Note: We cannot test that the system panics when + // the file contains multiple entries for any of the + // controllers. Because, well, panic. }) Describe("CPU", func() { @@ -95,12 +225,209 @@ var _ = Describe("sigarLinux", func() { }) }) - Describe("Mem without MemAvailable", func() { - var meminfoFile string - BeforeEach(func() { - meminfoFile = procd + "/meminfo" + Describe("Memory", func() { + Describe("determineSelfCgroup", func() { + It("fails for missing file", func() { + var cg string + err := determineSelfCgroup(&cg) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal("open " + procd + "/self/cgroup: no such file or directory")) + Expect(cg).To(Equal("")) + }) + It("fails for empty file", func() { + cgroupSetup(``) + var cg string + err := determineSelfCgroup(&cg) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal("unable to determine control group")) + Expect(cg).To(Equal("")) + }) + It("fails for missing data", func() { + cgroupSetup(`12:freezer:/`) + var cg string + err := determineSelfCgroup(&cg) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal("unable to determine control group")) + Expect(cg).To(Equal("")) + }) + It("finds *:memory: over 0::", func() { + cgroupSetup(`4:memory:/user +0::/bogus`) + var cg string + err := determineSelfCgroup(&cg) + Expect(err).ToNot(HaveOccurred()) + Expect(cg).To(Equal("/user")) + }) + It("find 0:: without *:memory:", func() { + cgroupSetup(`0::/user`) + var cg string + err := determineSelfCgroup(&cg) + Expect(err).ToNot(HaveOccurred()) + Expect(cg).To(Equal("/user")) + }) + }) + + Describe("determineMemoryLimit", func() { + It("fails for missing files", func() { + limit, err := determineMemoryLimit(``) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal("open " + procd + "/memory/memory.limit_in_bytes: no such file or directory")) + Expect(limit).To(BeNumerically("==", 0)) + }) + It("fails for missing data in v1 file", func() { + memLimitSetup1(``, ``) + limit, err := determineMemoryLimit(``) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal(`strconv.ParseUint: parsing "": invalid syntax`)) + Expect(limit).To(BeNumerically("==", 0)) + }) + It("fails for missing data in v2 file", func() { + memLimitSetup2(``, ``) + limit, err := determineMemoryLimit(``) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal(`strconv.ParseUint: parsing "": invalid syntax`)) + Expect(limit).To(BeNumerically("==", 0)) + }) + It("fails for bogus data in v1 file", func() { + memLimitSetup1(``, `bogus`) + limit, err := determineMemoryLimit(``) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal(`strconv.ParseUint: parsing "bogus": invalid syntax`)) + Expect(limit).To(BeNumerically("==", 0)) + }) + It("fails for bogus data in v2 file", func() { + memLimitSetup2(``, `bogus`) + limit, err := determineMemoryLimit(``) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal(`strconv.ParseUint: parsing "bogus": invalid syntax`)) + Expect(limit).To(BeNumerically("==", 0)) + }) + It("returns v2 data over v1", func() { + memLimitSetup1(``, `1111`) + memLimitSetup2(``, `2222`) + limit, err := determineMemoryLimit(``) + Expect(err).ToNot(HaveOccurred()) + Expect(limit).To(BeNumerically("==", 2222)) + }) + It("returns v1 data when v2 not available", func() { + memLimitSetup1(``, `1111`) + limit, err := determineMemoryLimit(``) + Expect(err).ToNot(HaveOccurred()) + Expect(limit).To(BeNumerically("==", 1111)) + }) + It("signals v2 no limit with failure", func() { + memLimitSetup2(``, `max`) + limit, err := determineMemoryLimit(``) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal(`no limit`)) + Expect(limit).To(BeNumerically("==", 0)) + }) + }) + + Describe("determineMemoryUsage", func() { + It("fails for missing files", func() { + limit, err := determineMemoryUsage(``) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal("open " + procd + "/memory/memory.stat: no such file or directory")) + Expect(limit).To(BeNumerically("==", 0)) + }) + It("fails for missing data in v1 file", func() { + memStatSetup(``, ``) + limit, err := determineMemoryUsage(``) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal(`no data found`)) + Expect(limit).To(BeNumerically("==", 0)) + }) + It("fails for missing data in v2 file", func() { + memUsageSetup2(``, ``) + limit, err := determineMemoryUsage(``) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal(`strconv.ParseUint: parsing "": invalid syntax`)) + Expect(limit).To(BeNumerically("==", 0)) + }) + It("fails for bogus data in v1 file", func() { + memStatSetup(``, `total_rss bogus`) + limit, err := determineMemoryUsage(``) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal(`no data found`)) + Expect(limit).To(BeNumerically("==", 0)) + }) + It("fails for bogus data in v2 file", func() { + memUsageSetup2(``, `bogus`) + limit, err := determineMemoryUsage(``) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal(`strconv.ParseUint: parsing "bogus": invalid syntax`)) + Expect(limit).To(BeNumerically("==", 0)) + }) + It("returns v2 data over v1", func() { + memStatSetup(``, `total_rss 1111`) + memUsageSetup2(``, `2222`) + limit, err := determineMemoryUsage(``) + Expect(err).ToNot(HaveOccurred()) + Expect(limit).To(BeNumerically("==", 2222)) + }) + It("returns v1 data when v2 not available", func() { + memStatSetup(``, `total_rss 1111`) + limit, err := determineMemoryUsage(``) + Expect(err).ToNot(HaveOccurred()) + Expect(limit).To(BeNumerically("==", 1111)) + }) + }) + + Describe("determineSwapUsage", func() { + It("fails for missing files", func() { + limit, err := determineSwapUsage(``) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal("open " + procd + "/memory/memory.stat: no such file or directory")) + Expect(limit).To(BeNumerically("==", 0)) + }) + It("fails for missing data in v1 file", func() { + swapUsageSetup1(``, ``) + limit, err := determineSwapUsage(``) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal(`no data found`)) + Expect(limit).To(BeNumerically("==", 0)) + }) + It("fails for missing data in v2 file", func() { + swapUsageSetup2(``, ``) + limit, err := determineSwapUsage(``) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal(`strconv.ParseUint: parsing "": invalid syntax`)) + Expect(limit).To(BeNumerically("==", 0)) + }) + It("fails for bogus data in v1 file", func() { + swapUsageSetup1(``, `swap bogus`) + limit, err := determineSwapUsage(``) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal(`no data found`)) + Expect(limit).To(BeNumerically("==", 0)) + }) + It("fails for bogus data in v2 file", func() { + swapUsageSetup2(``, `bogus`) + limit, err := determineSwapUsage(``) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal(`strconv.ParseUint: parsing "bogus": invalid syntax`)) + Expect(limit).To(BeNumerically("==", 0)) + }) + It("returns v2 data over v1", func() { + swapUsageSetup1(``, `swap 1111`) + swapUsageSetup2(``, `2222`) + limit, err := determineSwapUsage(``) + Expect(err).ToNot(HaveOccurred()) + Expect(limit).To(BeNumerically("==", 2222)) + }) + It("returns v1 data when v2 not available", func() { + swapUsageSetup1(``, `swap 1111`) + limit, err := determineSwapUsage(``) + Expect(err).ToNot(HaveOccurred()) + Expect(limit).To(BeNumerically("==", 1111)) + }) + }) - meminfoContents := ` + Describe("Mem", func() { + Describe("Without MemAvailable", func() { + BeforeEach(func() { + memInfoSetup(` MemTotal: 374256 kB MemFree: 274460 kB Buffers: 9764 kB @@ -142,30 +469,24 @@ HugePages_Rsvd: 0 HugePages_Surp: 0 Hugepagesize: 2048 kB DirectMap4k: 59328 kB -DirectMap2M: 333824 kB -` - err := ioutil.WriteFile(meminfoFile, []byte(meminfoContents), 0444) - Expect(err).ToNot(HaveOccurred()) - }) +DirectMap2M: 333824 kB`) + }) - It("returns correct memory info", func() { - mem := Mem{} - err := mem.Get() - Expect(err).ToNot(HaveOccurred()) - - Expect(mem.Total).To(BeNumerically("==", 374256*1024)) - Expect(mem.Free).To(BeNumerically("==", 274460*1024)) - Expect(mem.ActualFree).To(BeNumerically("==", 322872*1024)) - Expect(mem.ActualUsed).To(BeNumerically("==", 51384*1024)) - }) - }) + It("returns correct memory info", func() { + mem := Mem{} + err := mem.Get() + Expect(err).ToNot(HaveOccurred()) - Describe("Mem with MemAvailable", func() { - var meminfoFile string - BeforeEach(func() { - meminfoFile = procd + "/meminfo" + Expect(mem.Total).To(BeNumerically("==", 374256*1024)) + Expect(mem.Free).To(BeNumerically("==", 274460*1024)) + Expect(mem.ActualFree).To(BeNumerically("==", 322872*1024)) + Expect(mem.ActualUsed).To(BeNumerically("==", 51384*1024)) + }) + }) - meminfoContents := ` + Describe("With MemAvailable", func() { + BeforeEach(func() { + memInfoSetup(` MemTotal: 35008180 kB MemFree: 487816 kB MemAvailable: 20913400 kB @@ -210,30 +531,366 @@ HugePages_Rsvd: 0 HugePages_Surp: 0 Hugepagesize: 2048 kB DirectMap4k: 667520 kB -DirectMap2M: 34983936 kB -` - err := ioutil.WriteFile(meminfoFile, []byte(meminfoContents), 0444) - Expect(err).ToNot(HaveOccurred()) - }) +DirectMap2M: 34983936 kB`) + }) + + It("returns correct memory info", func() { + mem := Mem{} + err := mem.Get() + Expect(err).ToNot(HaveOccurred()) + + Expect(mem.Total).To(BeNumerically("==", 35008180*1024)) + Expect(mem.Free).To(BeNumerically("==", 487816*1024)) + Expect(mem.ActualFree).To(BeNumerically("==", 20913400*1024)) + Expect(mem.ActualUsed).To(BeNumerically("==", 14094780*1024)) + }) + }) + + // Three toggles: + // - MemAvailable present yes/no + // - Cgroup limit valid&sensible yes/no + // - Cgroup swap data present yes/no + // Times two, for cgroup v1 and v2/ + // + // Total of 16 tests. + // + // Note that `MemAvailable present yes/no` does not matter in + // the results, as the cgroup derived results will write over + // them. Thus we have 2 groups a 4 tests, with identical + // results for the equivalent tests of each group. + + Describe("With MemAvailable. With v1 cgroup limit. With v1 cgroup swap", func() { + // cgroup = '/user' + // The other tests use cgroup = '/'. This reduces the amount of changes needed + BeforeEach(func() { + cgroupSetup(`4:memory:/user`) + memInfoWithMemAvailable() + memLimitSetup1(`/user`, `21390950400`) + memStatWithSwap(`/user`) + }) + + It("returns correct memory info", func() { + mem := Mem{} + err := mem.Get() + Expect(err).ToNot(HaveOccurred()) + + Expect(mem.Total).To(BeNumerically("==", 21390950400)) + Expect(mem.Free).To(BeNumerically("==", 21390950400-14108536832-290564089)) + Expect(mem.ActualFree).To(BeNumerically("==", 21390950400-14108536832-290564089)) + Expect(mem.ActualUsed).To(BeNumerically("==", 14108536832+290564089)) + }) + }) + + Describe("With MemAvailable. With v1 cgroup limit. Without cgroup swap", func() { + BeforeEach(func() { + cgroupSetup(`4:memory:/`) + memInfoWithMemAvailable() + memLimitSetup1(``, `21390950400`) + memStatWithoutSwap(``) + }) + + It("returns correct memory info", func() { + mem := Mem{} + err := mem.Get() + Expect(err).ToNot(HaveOccurred()) + + Expect(mem.Total).To(BeNumerically("==", 21390950400)) + Expect(mem.Free).To(BeNumerically("==", 21390950400-14108536832)) + Expect(mem.ActualFree).To(BeNumerically("==", 21390950400-14108536832)) + Expect(mem.ActualUsed).To(BeNumerically("==", 14108536832)) + }) + }) + + Describe("With MemAvailable. Overlarge v1 cgroup limit. With v1 cgroup swap", func() { + BeforeEach(func() { + cgroupSetup(`4:memory:/`) + memInfoWithMemAvailable() + memLimitSetup1(``, `213909504000000000000`) + memStatWithSwap(``) + }) + + It("returns correct memory info", func() { + mem := Mem{} + err := mem.Get() + Expect(err).ToNot(HaveOccurred()) + + Expect(mem.Total).To(BeNumerically("==", 35008180*1024)) + Expect(mem.Free).To(BeNumerically("==", 35008180*1024-14108536832-290564089)) + Expect(mem.ActualFree).To(BeNumerically("==", 35008180*1024-14108536832-290564089)) + Expect(mem.ActualUsed).To(BeNumerically("==", 14108536832+290564089)) + }) + }) + + Describe("With MemAvailable. Overlarge v1 cgroup limit. Without cgroup swap", func() { + BeforeEach(func() { + cgroupSetup(`4:memory:/`) + memInfoWithMemAvailable() + memLimitSetup1(``, `213909504000000000000`) + memStatWithoutSwap(``) + }) + + It("returns correct memory info", func() { + mem := Mem{} + err := mem.Get() + Expect(err).ToNot(HaveOccurred()) + + Expect(mem.Total).To(BeNumerically("==", 35008180*1024)) + Expect(mem.Free).To(BeNumerically("==", 35008180*1024-14108536832)) + Expect(mem.ActualFree).To(BeNumerically("==", 35008180*1024-14108536832)) + Expect(mem.ActualUsed).To(BeNumerically("==", 14108536832)) + }) + }) + + Describe("Without MemAvailable. With v1 cgroup limit. With v1 cgroup swap", func() { + BeforeEach(func() { + cgroupSetup(`4:memory:/`) + memInfoWithoutMemAvailable() + memLimitSetup1(``, `21390950400`) + memStatWithSwap(``) + }) + + It("returns correct memory info", func() { + mem := Mem{} + err := mem.Get() + Expect(err).ToNot(HaveOccurred()) + + Expect(mem.Total).To(BeNumerically("==", 21390950400)) + Expect(mem.Free).To(BeNumerically("==", 21390950400-14108536832-290564089)) + Expect(mem.ActualFree).To(BeNumerically("==", 21390950400-14108536832-290564089)) + Expect(mem.ActualUsed).To(BeNumerically("==", 14108536832+290564089)) + }) + }) + + Describe("Without MemAvailable. With v1 cgroup limit. Without cgroup swap", func() { + BeforeEach(func() { + cgroupSetup(`4:memory:/`) + memInfoWithoutMemAvailable() + memLimitSetup1(``, `21390950400`) + memStatWithoutSwap(``) + }) + + It("returns correct memory info", func() { + mem := Mem{} + err := mem.Get() + Expect(err).ToNot(HaveOccurred()) + + Expect(mem.Total).To(BeNumerically("==", 21390950400)) + Expect(mem.Free).To(BeNumerically("==", 21390950400-14108536832)) + Expect(mem.ActualFree).To(BeNumerically("==", 21390950400-14108536832)) + Expect(mem.ActualUsed).To(BeNumerically("==", 14108536832)) + }) + }) + + Describe("Without MemAvailable. Overlarge v1 cgroup limit. With v1 cgroup swap", func() { + BeforeEach(func() { + cgroupSetup(`4:memory:/`) + memInfoWithoutMemAvailable() + memLimitSetup1(``, `213909504000000000000`) + memStatWithSwap(``) + }) + + It("returns correct memory info", func() { + mem := Mem{} + err := mem.Get() + Expect(err).ToNot(HaveOccurred()) + + Expect(mem.Total).To(BeNumerically("==", 35008180*1024)) + Expect(mem.Free).To(BeNumerically("==", 35008180*1024-14108536832-290564089)) + Expect(mem.ActualFree).To(BeNumerically("==", 35008180*1024-14108536832-290564089)) + Expect(mem.ActualUsed).To(BeNumerically("==", 14108536832+290564089)) + }) + }) - It("returns correct memory info", func() { - mem := Mem{} - err := mem.Get() - Expect(err).ToNot(HaveOccurred()) + Describe("Without MemAvailable. Overlarge v1 cgroup limit. Without cgroup swap", func() { + BeforeEach(func() { + cgroupSetup(`4:memory:/`) + memInfoWithoutMemAvailable() + memLimitSetup1(``, `213909504000000000000`) + memStatWithoutSwap(``) + }) + + It("returns correct memory info", func() { + mem := Mem{} + err := mem.Get() + Expect(err).ToNot(HaveOccurred()) + + Expect(mem.Total).To(BeNumerically("==", 35008180*1024)) + Expect(mem.Free).To(BeNumerically("==", 35008180*1024-14108536832)) + Expect(mem.ActualFree).To(BeNumerically("==", 35008180*1024-14108536832)) + Expect(mem.ActualUsed).To(BeNumerically("==", 14108536832)) + }) + }) + + Describe("With MemAvailable. With v2 cgroup limit. With v2 cgroup swap", func() { + // cgroup = '/user' + // The other tests use cgroup = '/'. This reduces the amount of changes needed + BeforeEach(func() { + cgroupSetup(`4:memory:/user`) + memInfoWithMemAvailable() + memLimitSetup2(`/user`, `21390950400`) + memUsageWithSwap(`/user`) + }) + + It("returns correct memory info", func() { + mem := Mem{} + err := mem.Get() + Expect(err).ToNot(HaveOccurred()) + + Expect(mem.Total).To(BeNumerically("==", 21390950400)) + Expect(mem.Free).To(BeNumerically("==", 21390950400-14108536832-290564089)) + Expect(mem.ActualFree).To(BeNumerically("==", 21390950400-14108536832-290564089)) + Expect(mem.ActualUsed).To(BeNumerically("==", 14108536832+290564089)) + }) + }) + + Describe("With MemAvailable. With v2 cgroup limit. Without cgroup swap", func() { + BeforeEach(func() { + cgroupSetup(`4:memory:/`) + memInfoWithMemAvailable() + memLimitSetup2(``, `21390950400`) + memUsageWithoutSwap(``) + }) + + It("returns correct memory info", func() { + mem := Mem{} + err := mem.Get() + Expect(err).ToNot(HaveOccurred()) + + Expect(mem.Total).To(BeNumerically("==", 21390950400)) + Expect(mem.Free).To(BeNumerically("==", 21390950400-14108536832)) + Expect(mem.ActualFree).To(BeNumerically("==", 21390950400-14108536832)) + Expect(mem.ActualUsed).To(BeNumerically("==", 14108536832)) + }) + }) + + Describe("With MemAvailable. Overlarge v2 cgroup limit. With v2 cgroup swap", func() { + BeforeEach(func() { + cgroupSetup(`4:memory:/`) + memInfoWithMemAvailable() + memLimitSetup2(``, `213909504000000000000`) + memUsageWithSwap(``) + }) + + It("returns correct memory info", func() { + mem := Mem{} + err := mem.Get() + Expect(err).ToNot(HaveOccurred()) + + Expect(mem.Total).To(BeNumerically("==", 35008180*1024)) + Expect(mem.Free).To(BeNumerically("==", 35008180*1024-14108536832-290564089)) + Expect(mem.ActualFree).To(BeNumerically("==", 35008180*1024-14108536832-290564089)) + Expect(mem.ActualUsed).To(BeNumerically("==", 14108536832+290564089)) + }) + }) + + Describe("With MemAvailable. Overlarge v2 cgroup limit. Without cgroup swap", func() { + BeforeEach(func() { + cgroupSetup(`4:memory:/`) + memInfoWithMemAvailable() + memLimitSetup2(``, `213909504000000000000`) + memUsageWithoutSwap(``) + }) + + It("returns correct memory info", func() { + mem := Mem{} + err := mem.Get() + Expect(err).ToNot(HaveOccurred()) + + Expect(mem.Total).To(BeNumerically("==", 35008180*1024)) + Expect(mem.Free).To(BeNumerically("==", 35008180*1024-14108536832)) + Expect(mem.ActualFree).To(BeNumerically("==", 35008180*1024-14108536832)) + Expect(mem.ActualUsed).To(BeNumerically("==", 14108536832)) + }) + }) + + Describe("Without MemAvailable. With v2 cgroup limit. With v2 cgroup swap", func() { + BeforeEach(func() { + cgroupSetup(`4:memory:/`) + memInfoWithoutMemAvailable() + memLimitSetup2(``, `21390950400`) + memUsageWithSwap(``) + }) + + It("returns correct memory info", func() { + mem := Mem{} + err := mem.Get() + Expect(err).ToNot(HaveOccurred()) + + Expect(mem.Total).To(BeNumerically("==", 21390950400)) + Expect(mem.Free).To(BeNumerically("==", 21390950400-14108536832-290564089)) + Expect(mem.ActualFree).To(BeNumerically("==", 21390950400-14108536832-290564089)) + Expect(mem.ActualUsed).To(BeNumerically("==", 14108536832+290564089)) + }) + }) + + Describe("Without MemAvailable. With v2 cgroup limit. Without cgroup swap", func() { + BeforeEach(func() { + cgroupSetup(`4:memory:/`) + memInfoWithoutMemAvailable() + memLimitSetup2(``, `21390950400`) + memUsageWithoutSwap(``) + }) + + It("returns correct memory info", func() { + mem := Mem{} + err := mem.Get() + Expect(err).ToNot(HaveOccurred()) + + Expect(mem.Total).To(BeNumerically("==", 21390950400)) + Expect(mem.Free).To(BeNumerically("==", 21390950400-14108536832)) + Expect(mem.ActualFree).To(BeNumerically("==", 21390950400-14108536832)) + Expect(mem.ActualUsed).To(BeNumerically("==", 14108536832)) + }) + }) + + Describe("Without MemAvailable. Overlarge v2 cgroup limit. With v2 cgroup swap", func() { + BeforeEach(func() { + cgroupSetup(`4:memory:/`) + memInfoWithoutMemAvailable() + memLimitSetup2(``, `213909504000000000000`) + memUsageWithSwap(``) + }) + + It("returns correct memory info", func() { + mem := Mem{} + err := mem.Get() + Expect(err).ToNot(HaveOccurred()) + + Expect(mem.Total).To(BeNumerically("==", 35008180*1024)) + Expect(mem.Free).To(BeNumerically("==", 35008180*1024-14108536832-290564089)) + Expect(mem.ActualFree).To(BeNumerically("==", 35008180*1024-14108536832-290564089)) + Expect(mem.ActualUsed).To(BeNumerically("==", 14108536832+290564089)) + }) + }) + + Describe("Without MemAvailable. Overlarge v2 cgroup limit. Without cgroup swap", func() { + BeforeEach(func() { + cgroupSetup(`4:memory:/`) + memInfoWithoutMemAvailable() + memLimitSetup2(``, `213909504000000000000`) + memUsageWithoutSwap(``) + }) + + It("returns correct memory info", func() { + mem := Mem{} + err := mem.Get() + Expect(err).ToNot(HaveOccurred()) + + Expect(mem.Total).To(BeNumerically("==", 35008180*1024)) + Expect(mem.Free).To(BeNumerically("==", 35008180*1024-14108536832)) + Expect(mem.ActualFree).To(BeNumerically("==", 35008180*1024-14108536832)) + Expect(mem.ActualUsed).To(BeNumerically("==", 14108536832)) + }) + }) - Expect(mem.Total).To(BeNumerically("==", 35008180*1024)) - Expect(mem.Free).To(BeNumerically("==", 487816*1024)) - Expect(mem.ActualFree).To(BeNumerically("==", 20913400*1024)) - Expect(mem.ActualUsed).To(BeNumerically("==", 14094780*1024)) }) - }) - Describe("Swap", func() { - var meminfoFile string - BeforeEach(func() { - meminfoFile = procd + "/meminfo" + Describe("Swap", func() { + var meminfoFile string + BeforeEach(func() { + meminfoFile = procd + "/meminfo" - meminfoContents := ` + meminfoContents := ` MemTotal: 374256 kB MemFree: 274460 kB Buffers: 9764 kB @@ -277,17 +934,18 @@ Hugepagesize: 2048 kB DirectMap4k: 59328 kB DirectMap2M: 333824 kB ` - err := ioutil.WriteFile(meminfoFile, []byte(meminfoContents), 0444) - Expect(err).ToNot(HaveOccurred()) - }) + err := ioutil.WriteFile(meminfoFile, []byte(meminfoContents), 0444) + Expect(err).ToNot(HaveOccurred()) + }) - It("returns correct memory info", func() { - swap := Swap{} - err := swap.Get() - Expect(err).ToNot(HaveOccurred()) + It("returns correct memory info", func() { + swap := Swap{} + err := swap.Get() + Expect(err).ToNot(HaveOccurred()) - Expect(swap.Total).To(BeNumerically("==", 786428*1024)) - Expect(swap.Free).To(BeNumerically("==", 786428*1024)) + Expect(swap.Total).To(BeNumerically("==", 786428*1024)) + Expect(swap.Free).To(BeNumerically("==", 786428*1024)) + }) }) }) }) From 34f6fb963477d6d121a5e5543ed18696cf8a56dd Mon Sep 17 00:00:00 2001 From: Andreas Kupries Date: Thu, 29 Apr 2021 14:26:28 +0200 Subject: [PATCH 2/2] fix: proper option search Avoid trigger on larger strings having the target as substring --- sigar_linux.go | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/sigar_linux.go b/sigar_linux.go index 0975d0991..b3d0b2ccc 100644 --- a/sigar_linux.go +++ b/sigar_linux.go @@ -656,13 +656,25 @@ func determineControllerMounts(sysd1, sysd2 *string) { *sysd2 = mpath return true } - if mtype == "cgroup" && strings.Contains(moptions, "memory") { - if *sysd1 != "" { - panic("Multiple cgroup v1 mount points") + if mtype == "cgroup" { + options := strings.Split(moptions, ",") + if stringSliceContains(options, "memory") { + if *sysd1 != "" { + panic("Multiple cgroup v1 mount points") + } + *sysd1 = mpath + return true } - *sysd1 = mpath - return true } return true }) } + +func stringSliceContains(a []string, x string) bool { + for _, n := range a { + if x == n { + return true + } + } + return false +}