From c28247d0f8aba81c2f2536cebdecd7ca7999b870 Mon Sep 17 00:00:00 2001 From: Peter Hunt Date: Wed, 27 Sep 2023 09:55:53 -0400 Subject: [PATCH] libctr/fs2: Initialize MaxUsage field with memory.peak value as well as rework the function to reduce duplication, and update the test for it Signed-off-by: Peter Hunt --- libcontainer/cgroups/fs2/memory.go | 53 ++++++++++++++++--------- libcontainer/cgroups/fs2/memory_test.go | 16 +++++++- 2 files changed, 50 insertions(+), 19 deletions(-) diff --git a/libcontainer/cgroups/fs2/memory.go b/libcontainer/cgroups/fs2/memory.go index 85e96b1ce98..56b36945c5f 100644 --- a/libcontainer/cgroups/fs2/memory.go +++ b/libcontainer/cgroups/fs2/memory.go @@ -136,26 +136,43 @@ func getMemoryDataV2(path, name string) (cgroups.MemoryData, error) { if name != "" { moduleName = "memory." + name } - usage := moduleName + ".current" - limit := moduleName + ".max" - - value, err := fscommon.GetCgroupParamUint(path, usage) - if err != nil { - if name != "" && os.IsNotExist(err) { - // Ignore EEXIST as there's no swap accounting - // if kernel CONFIG_MEMCG_SWAP is not set or - // swapaccount=0 kernel boot parameter is given. - return cgroups.MemoryData{}, nil + files := []struct { + name string + value *uint64 + }{ + { + name: moduleName + ".current", + value: &memoryData.Usage, + }, + { + name: moduleName + ".max", + value: &memoryData.Limit, + }, + { + name: moduleName + ".peak", + value: &memoryData.MaxUsage, + }, + } + + for _, f := range files { + value, err := fscommon.GetCgroupParamUint(path, f.name) + if err != nil { + if os.IsNotExist(err) { + // Peak usage is only available since kernel v5.14, so collect best-effort. + if strings.HasSuffix(f.name, "peak") { + continue + } + // Ignore EEXIST as there's no swap accounting + // if kernel CONFIG_MEMCG_SWAP is not set or + // swapaccount=0 kernel boot parameter is given. + if name == "swap" { + return cgroups.MemoryData{}, nil + } + } + return memoryData, err } - return cgroups.MemoryData{}, err - } - memoryData.Usage = value - - value, err = fscommon.GetCgroupParamUint(path, limit) - if err != nil { - return cgroups.MemoryData{}, err + *f.value = value } - memoryData.Limit = value return memoryData, nil } diff --git a/libcontainer/cgroups/fs2/memory_test.go b/libcontainer/cgroups/fs2/memory_test.go index 2e2713c29ae..fd71e1b58dc 100644 --- a/libcontainer/cgroups/fs2/memory_test.go +++ b/libcontainer/cgroups/fs2/memory_test.go @@ -71,7 +71,7 @@ func TestStatMemoryPodCgroupNotFound(t *testing.T) { t.Errorf("expected error when statting memory for cgroupv2 root, but was nil") } - if !strings.Contains(err.Error(), "memory.current: no such file or directory") { + if !strings.Contains(err.Error(), "memory.peak: no such file or directory") { t.Errorf("expected error to contain 'memory.current: no such file or directory', but was %s", err.Error()) } } @@ -94,6 +94,10 @@ func TestStatMemoryPodCgroup(t *testing.T) { t.Fatal(err) } + if err := os.WriteFile(filepath.Join(fakeCgroupDir, "memory.peak"), []byte("10000000000000"), 0o644); err != nil { + t.Fatal(err) + } + gotStats := cgroups.NewStats() // use a fake root path to trigger the pod cgroup lookup. @@ -107,6 +111,16 @@ func TestStatMemoryPodCgroup(t *testing.T) { if gotStats.MemoryStats.Usage.Usage != expectedUsageBytes { t.Errorf("parsed cgroupv2 memory.stat doesn't match expected result: \ngot %#v\nexpected %#v\n", gotStats.MemoryStats.Usage.Usage, expectedUsageBytes) } + + var expectedLimitBytes uint64 = 999999999 + if gotStats.MemoryStats.Usage.Limit != expectedLimitBytes { + t.Errorf("parsed cgroupv2 memory.stat doesn't match expected result: \ngot %#v\nexpected %#v\n", gotStats.MemoryStats.Usage.Limit, expectedLimitBytes) + } + + var expectedMaxUsageBytes uint64 = 10000000000000 + if gotStats.MemoryStats.Usage.MaxUsage != expectedMaxUsageBytes { + t.Errorf("parsed cgroupv2 memory.stat doesn't match expected result: \ngot %#v\nexpected %#v\n", gotStats.MemoryStats.Usage.MaxUsage, expectedMaxUsageBytes) + } } func TestRootStatsFromMeminfo(t *testing.T) {