diff --git a/libcontainer/cgroups/utils.go b/libcontainer/cgroups/utils.go index 1577c41ef..13ebf52ab 100644 --- a/libcontainer/cgroups/utils.go +++ b/libcontainer/cgroups/utils.go @@ -319,7 +319,10 @@ func HugePageSizes() []string { return } - hugePageSizes, _ = getHugePageSizeFromFilenames(files) + hugePageSizes, err = getHugePageSizeFromFilenames(files) + if err != nil { + logrus.Warn("HugePageSizes: ", err) + } }) return hugePageSizes @@ -327,24 +330,33 @@ func HugePageSizes() []string { func getHugePageSizeFromFilenames(fileNames []string) ([]string, error) { pageSizes := make([]string, 0, len(fileNames)) + var warn error for _, file := range fileNames { // example: hugepages-1048576kB val := strings.TrimPrefix(file, "hugepages-") if len(val) == len(file) { - // unexpected file name: no prefix found + // Unexpected file name: no prefix found, ignore it. continue } - // The suffix is always "kB" (as of Linux 5.9) + // The suffix is always "kB" (as of Linux 5.13). If we find + // something else, produce an error but keep going. eLen := len(val) - 2 val = strings.TrimSuffix(val, "kB") if len(val) != eLen { - logrus.Warnf("HugePageSizes: %s: invalid filename suffix (expected \"kB\")", file) + // Highly unlikely. + if warn == nil { + warn = errors.New(file + `: invalid suffix (expected "kB")`) + } continue } size, err := strconv.Atoi(val) if err != nil { - return nil, err + // Highly unlikely. + if warn == nil { + warn = fmt.Errorf("%s: %w", file, err) + } + continue } // Model after https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/mm/hugetlb_cgroup.c?id=eff48ddeab782e35e58ccc8853f7386bbae9dec4#n574 // but in our case the size is in KB already. @@ -358,7 +370,7 @@ func getHugePageSizeFromFilenames(fileNames []string) ([]string, error) { pageSizes = append(pageSizes, val) } - return pageSizes, nil + return pageSizes, warn } // GetPids returns all pids, that were added to cgroup at path. diff --git a/libcontainer/cgroups/utils_test.go b/libcontainer/cgroups/utils_test.go index 708015d44..259313ba5 100644 --- a/libcontainer/cgroups/utils_test.go +++ b/libcontainer/cgroups/utils_test.go @@ -8,7 +8,6 @@ import ( "testing" "github.com/moby/sys/mountinfo" - "github.com/sirupsen/logrus" ) const fedoraMountinfo = `15 35 0:3 / /proc rw,nosuid,nodev,noexec,relatime shared:5 - proc proc rw @@ -467,7 +466,6 @@ func TestGetHugePageSizeImpl(t *testing.T) { input []string output []string isErr bool - isWarn bool }{ { input: []string{"hugepages-1048576kB", "hugepages-2048kB", "hugepages-32768kB", "hugepages-64kB"}, @@ -487,25 +485,29 @@ func TestGetHugePageSizeImpl(t *testing.T) { { // invalid prefix (silently skipped) input: []string{"whatever-1024kB"}, }, - { // invalid suffix (skipped with a warning) - input: []string{"hugepages-1024gB"}, - isWarn: true, + { // invalid suffix + input: []string{"hugepages-1024gB"}, + isErr: true, }, - { // no suffix (skipped with a warning) - input: []string{"hugepages-1024"}, - isWarn: true, + { // no suffix + input: []string{"hugepages-1024"}, + isErr: true, + }, + { // mixed valid and invalid entries + input: []string{"hugepages-4194304kB", "hugepages-2048kB", "hugepages-akB", "hugepages-64kB"}, + output: []string{"4GB", "2MB", "64KB"}, + isErr: true, + }, + { // mixed valid and invalid entries + input: []string{"hugepages-2048kB", "hugepages-kB", "hugepages-64kB"}, + output: []string{"2MB", "64KB"}, + isErr: true, }, } - // Need to catch warnings. - savedOut := logrus.StandardLogger().Out - defer logrus.SetOutput(savedOut) - warns := new(bytes.Buffer) - logrus.SetOutput(warns) - for _, c := range testCases { - warns.Reset() output, err := getHugePageSizeFromFilenames(c.input) + t.Log("input:", c.input, "; output:", output, "; err:", err) if err != nil { if !c.isErr { t.Errorf("input %v, expected nil, got error: %v", c.input, err) @@ -515,15 +517,6 @@ func TestGetHugePageSizeImpl(t *testing.T) { } if c.isErr { t.Errorf("input %v, expected error, got error: nil, output: %v", c.input, output) - // no more checks - continue - } - // check for warnings - if c.isWarn && warns.Len() == 0 { - t.Errorf("input %v, expected a warning, got none", c.input) - } - if !c.isWarn && warns.Len() > 0 { - t.Errorf("input %v, unexpected warning: %s", c.input, warns.String()) } // check output if len(output) != len(c.output) || (len(output) > 0 && !reflect.DeepEqual(output, c.output)) {