lib/cgroup: follow-up after b9bf3cbe3e

docs/CHANGELOG.md

@@ -5,6 +5,7 @@
 * FEATURE: single-node VictoriaMetrics now accepts requests to handlers with `/prometheus` and `/graphite` prefixes such as `/prometheus/api/v1/query`. This improves compatibility with [handlers from VictoriaMetrics cluster](https://victoriametrics.github.io/Cluster-VictoriaMetrics.html#url-format).
 * FEATURE: expose `process_open_fds` and `process_max_fds` metrics. These metrics can be used for alerting when `process_open_fds` reaches `process_max_fds`. See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/402 and https://github.com/VictoriaMetrics/VictoriaMetrics/issues/1037
 * FEATURE: vmalert: add `-datasource.appendTypePrefix` command-line option for querying both Prometheus and Graphite datasource in cluster version of VictoriaMetrics. See [these docs](https://victoriametrics.github.io/vmalert.html#graphite) for details.
+* FEATURE: remove dependency on external programs such as `cat`, `grep` and `cut` when detecting cpu and memory limits inside Docker or LXC container.
 
 * BUGFIX: do not spam error logs when discovering Docker Swarm targets without dedicated IP. See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/1028 .
 

lib/cgroup/cpu.go

@@ -3,7 +3,6 @@ package cgroup
 import (
 	"io/ioutil"
 	"os"
-	"path"
 	"runtime"
 	"strconv"
 	"strings"
@@ -41,20 +40,8 @@ func updateGOMAXPROCSToCPUQuota() {
 	runtime.GOMAXPROCS(gomaxprocs)
 }
 
-func getCPUStat(sysPath, cgroupPath, statName string) (int64, error) {
-	n, err := readInt64(path.Join(sysPath, statName))
-	if err == nil {
-		return n, nil
-	}
-	subPath, err := grepFirstMatch(cgroupPath, "cpu,", 2, ":")
-	if err != nil {
-		return 0, err
-	}
-	return readInt64(path.Join(sysPath, subPath, statName))
-}
-
 func getCPUQuota() float64 {
-	quotaUS, err := getCPUStat("/sys/fs/cgroup/cpu", "/proc/self/cgroup", "cpu.cfs_quota_us")
+	quotaUS, err := getCPUStat("cpu.cfs_quota_us")
 	if err != nil {
 		return 0
 	}
@@ -63,13 +50,17 @@ func getCPUQuota() float64 {
 		// See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/685#issuecomment-674423728
 		return getOnlineCPUCount()
 	}
-	periodUS, err := getCPUStat("/sys/fs/cgroup/cpu", "/proc/self/cgroup", "cpu.cfs_period_us")
+	periodUS, err := getCPUStat("cpu.cfs_period_us")
 	if err != nil {
 		return 0
 	}
 	return float64(quotaUS) / float64(periodUS)
 }
 
+func getCPUStat(statName string) (int64, error) {
+	return getStatGeneric(statName, "/sys/fs/cgroup/cpu", "/proc/self/cgroup", "cpu,")
+}
+
 func getOnlineCPUCount() float64 {
 	// See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/685#issuecomment-674423728
 	data, err := ioutil.ReadFile("/sys/devices/system/cpu/online")

lib/cgroup/cpu_test.go

@@ -22,35 +22,3 @@ func TestCountCPUs(t *testing.T) {
 	f("0-3", 4)
 	f("0-6", 7)
 }
-
-func TestGetCPUStatQuota(t *testing.T) {
-	f := func(sysPath, cgroupPath string, want int64, wantErr bool) {
-		t.Helper()
-		got, err := getCPUStat(sysPath, cgroupPath, "cpu.cfs_quota_us")
-		if (err != nil && !wantErr) || (err == nil && wantErr) {
-			t.Fatalf("unxpected error value: %v, want err: %v", err, wantErr)
-		}
-		if got != want {
-			t.Fatalf("unxpected result, got: %d, want %d", got, want)
-		}
-	}
-	f("testdata/", "testdata/self/cgroup", -1, false)
-	f("testdata/cgroup", "testdata/self/cgroup", 10, false)
-	f("testdata/", "testdata/missing_folder", 0, true)
-}
-
-func TestGetCPUStatPeriod(t *testing.T) {
-	f := func(sysPath, cgroupPath string, want int64, wantErr bool) {
-		t.Helper()
-		got, err := getCPUStat(sysPath, cgroupPath, "cpu.cfs_period_us")
-		if (err != nil && !wantErr) || (err == nil && wantErr) {
-			t.Fatalf("unxpected error value: %v, want err: %v", err, wantErr)
-		}
-		if got != want {
-			t.Fatalf("unxpected result, got: %d, want %d", got, want)
-		}
-	}
-	f("testdata/", "testdata/self/cgroup", 100000, false)
-	f("testdata/cgroup", "testdata/self/cgroup", 500000, false)
-	f("testdata/", "testdata/missing_folder", 0, true)
-}

lib/cgroup/mem.go

@@ -1,7 +1,6 @@
 package cgroup
 
 import (
-	"path"
 	"strconv"
 )
 
@@ -13,23 +12,15 @@ func GetMemoryLimit() int64 {
 	// Read memory limit according to https://unix.stackexchange.com/questions/242718/how-to-find-out-how-much-memory-lxc-container-is-allowed-to-consume
 	// This should properly determine the limit inside lxc container.
 	// See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/84
-	n, err := getMemLimit("/sys/fs/cgroup/", "/proc/self/cgroup")
+	n, err := getMemStat("memory.limit_in_bytes")
 	if err != nil {
 		return 0
 	}
 	return n
 }
 
-func getMemLimit(sysPath, cgroupPath string) (int64, error) {
-	n, err := readInt64(path.Join(sysPath, "memory.limit_in_bytes"))
-	if err == nil {
-		return n, nil
-	}
-	subPath, err := grepFirstMatch(cgroupPath, "memory", 2, ":")
-	if err != nil {
-		return 0, err
-	}
-	return readInt64(path.Join(sysPath, subPath, "memory.limit_in_bytes"))
+func getMemStat(statName string) (int64, error) {
+	return getStatGeneric(statName, "/sys/fs/cgroup/memory", "/proc/self/cgroup", "memory")
 }
 
 // GetHierarchicalMemoryLimit returns hierarchical memory limit
@@ -43,28 +34,12 @@ func GetHierarchicalMemoryLimit() int64 {
 	return n
 }
 
-func getHierarchicalMemoryLimit(sysPath, cgroupPath string) (int64, error) {
-	n, err := getMemStatDirect(sysPath)
-	if err == nil {
-		return n, nil
+func getHierarchicalMemoryLimit(sysfsPrefix, cgroupPath string) (int64, error) {
+	data, err := getFileContents("memory.stat", sysfsPrefix, cgroupPath, "memory")
+	if err != nil {
+		return 0, err
 	}
-	return getMemStatSubPath(sysPath, cgroupPath)
-}
-
-func getMemStatDirect(sysPath string) (int64, error) {
-	memStat, err := grepFirstMatch(path.Join(sysPath, "memory.stat"), "hierarchical_memory_limit", 1, " ")
-	if err != nil {
-		return 0, err
-	}
-	return strconv.ParseInt(memStat, 10, 64)
-}
-
-func getMemStatSubPath(sysPath, cgroupPath string) (int64, error) {
-	cgrps, err := grepFirstMatch(cgroupPath, "memory", 2, ":")
-	if err != nil {
-		return 0, err
-	}
-	memStat, err := grepFirstMatch(path.Join(sysPath, cgrps, "memory.stat"), "hierarchical_memory_limit", 1, " ")
+	memStat, err := grepFirstMatch(data, "hierarchical_memory_limit", 1, " ")
 	if err != nil {
 		return 0, err
 	}

lib/cgroup/mem_test.go

@@ -1,35 +1,34 @@
 package cgroup
 
-import "testing"
+import (
+	"testing"
+)
 
-func TestGetMemLimit(t *testing.T) {
-	f := func(sysPath, cgroupPath string, want int64, wantErr bool) {
-		t.Helper()
-		got, err := getMemLimit(sysPath, cgroupPath)
-		if (err != nil && !wantErr) || (err == nil && wantErr) {
-			t.Fatalf("unxpected error: %v, wantErr: %v", err, wantErr)
-		}
-		if got != want {
-			t.Fatalf("unxpected result, got: %d, want %d", got, want)
-		}
-	}
-	f("testdata/", "testdata/self/cgroup", 9223372036854771712, false)
-	f("testdata/cgroup", "testdata/self/cgroup", 523372036854771712, false)
-	f("testdata/", "testdata/none_existing_folder", 0, true)
-}
-
-func TestGetMemHierarchical(t *testing.T) {
-	f := func(sysPath, cgroupPath string, want int64, wantErr bool) {
+func TestGetHierarchicalMemoryLimitSuccess(t *testing.T) {
+	f := func(sysPath, cgroupPath string, want int64) {
 		t.Helper()
 		got, err := getHierarchicalMemoryLimit(sysPath, cgroupPath)
-		if (err != nil && !wantErr) || (err == nil && wantErr) {
-			t.Fatalf("unxpected error: %v, wantErr: %v", err, wantErr)
+		if err != nil {
+			t.Fatalf("unexpected error: %s", err)
 		}
 		if got != want {
-			t.Fatalf("unxpected result, got: %d, want %d", got, want)
+			t.Fatalf("unexpected result, got: %d, want %d", got, want)
 		}
 	}
-	f("testdata/", "testdata/self/cgroup", 16, false)
-	f("testdata/cgroup", "testdata/self/cgroup", 120, false)
-	f("testdata/", "testdata/none_existing_folder", 0, true)
+	f("testdata/", "testdata/self/cgroup", 16)
+	f("testdata/cgroup", "testdata/self/cgroup", 120)
+}
+
+func TestGetHierarchicalMemoryLimitFailure(t *testing.T) {
+	f := func(sysPath, cgroupPath string) {
+		t.Helper()
+		got, err := getHierarchicalMemoryLimit(sysPath, cgroupPath)
+		if err == nil {
+			t.Fatalf("expecting non-nil error")
+		}
+		if got != 0 {
+			t.Fatalf("unexpected result, got: %d, want 0", got)
+		}
+	}
+	f("testdata/", "testdata/none_existing_folder")
 }

lib/cgroup/testdata/memory.stat

@@ -1 +0,0 @@
-9223372036854771712

lib/cgroup/util.go

@@ -1,42 +1,58 @@
 package cgroup
 
 import (
-	"bufio"
-	"bytes"
 	"fmt"
 	"io/ioutil"
-	"os"
+	"path"
 	"strconv"
 	"strings"
 )
 
-// grepFirstMatch search match line at file and returns item from it by index with given delimiter.
-func grepFirstMatch(sourcePath string, match string, index int, delimiter string) (string, error) {
-	f, err := os.Open(sourcePath)
+func getStatGeneric(statName, sysfsPrefix, cgroupPath, cgroupGrepLine string) (int64, error) {
+	data, err := getFileContents(statName, sysfsPrefix, cgroupPath, cgroupGrepLine)
+	if err != nil {
+		return 0, err
+	}
+	n, err := strconv.ParseInt(data, 10, 64)
+	if err != nil {
+		return 0, err
+	}
+	return n, nil
+}
+
+func getFileContents(statName, sysfsPrefix, cgroupPath, cgroupGrepLine string) (string, error) {
+	filepath := path.Join(sysfsPrefix, statName)
+	data, err := ioutil.ReadFile(filepath)
+	if err == nil {
+		return string(data), nil
+	}
+	cgroupData, err := ioutil.ReadFile(cgroupPath)
 	if err != nil {
 		return "", err
 	}
-	defer func() { _ = f.Close() }()
-	scanner := bufio.NewScanner(f)
-	for scanner.Scan() {
-		text := scanner.Text()
-		if !strings.Contains(text, match) {
-			continue
+	subPath, err := grepFirstMatch(string(cgroupData), cgroupGrepLine, 2, ":")
+	if err != nil {
+		return "", err
 	}
-		split := strings.Split(text, delimiter)
-		if len(split) < index {
-			return "", fmt.Errorf("needed index line: %d, wasn't found at line: %q at file: %q", index, text, sourcePath)
+	filepath = path.Join(sysfsPrefix, subPath, statName)
+	data, err = ioutil.ReadFile(filepath)
+	if err != nil {
+		return "", err
 	}
-		return strings.TrimSpace(split[index]), nil
-	}
-	return "", fmt.Errorf("stat: %q, wasn't found at file: %q", match, sourcePath)
+	return string(data), nil
 }
 
-func readInt64(path string) (int64, error) {
-	data, err := ioutil.ReadFile(path)
-	if err == nil {
-		data = bytes.TrimSpace(data)
-		return strconv.ParseInt(string(data), 10, 64)
+// grepFirstMatch searches match line at data and returns item from it by index with given delimiter.
+func grepFirstMatch(data string, match string, index int, delimiter string) (string, error) {
+	lines := strings.Split(string(data), "\n")
+	for _, s := range lines {
+		if !strings.Contains(s, match) {
+			continue
 		}
-	return 0, err
+		parts := strings.Split(s, delimiter)
+		if index < len(parts) {
+			return strings.TrimSpace(parts[index]), nil
+		}
+	}
+	return "", fmt.Errorf("cannot find %q in %q", match, data)
 }

lib/cgroup/util_test.go

@@ -0,0 +1,40 @@
+package cgroup
+
+import (
+	"testing"
+)
+
+func TestGetStatGenericSuccess(t *testing.T) {
+	f := func(statName, sysfsPrefix, cgroupPath, cgroupGrepLine string, want int64) {
+		t.Helper()
+		got, err := getStatGeneric(statName, sysfsPrefix, cgroupPath, cgroupGrepLine)
+		if err != nil {
+			t.Fatalf("unexpected error: %s", err)
+		}
+		if got != want {
+			t.Fatalf("unexpected result, got: %d, want %d", got, want)
+		}
+	}
+	f("cpu.cfs_quota_us", "testdata/", "testdata/self/cgroup", "cpu,", -1)
+	f("cpu.cfs_quota_us", "testdata/cgroup", "testdata/self/cgroup", "cpu,", 10)
+	f("cpu.cfs_period_us", "testdata/", "testdata/self/cgroup", "cpu,", 100000)
+	f("cpu.cfs_period_us", "testdata/cgroup", "testdata/self/cgroup", "cpu,", 500000)
+	f("memory.limit_in_bytes", "testdata/", "testdata/self/cgroup", "memory", 9223372036854771712)
+	f("memory.limit_in_bytes", "testdata/cgroup", "testdata/self/cgroup", "memory", 523372036854771712)
+}
+
+func TestGetStatGenericFailure(t *testing.T) {
+	f := func(statName, sysfsPrefix, cgroupPath, cgroupGrepLine string) {
+		t.Helper()
+		got, err := getStatGeneric(statName, sysfsPrefix, cgroupPath, cgroupGrepLine)
+		if err == nil {
+			t.Fatalf("expecting non-nil error")
+		}
+		if got != 0 {
+			t.Fatalf("unexpected result, got: %d, want 0", got)
+		}
+	}
+	f("cpu.cfs_quota_us", "testdata/", "testdata/missing_folder", "cpu,")
+	f("cpu.cfs_period_us", "testdata/", "testdata/missing_folder", "cpu,")
+	f("memory.limit_in_bytes", "testdata/", "testdata/none_existing_folder", "memory")
+}