Merge pull request #2839 from giuseppe/cpu-fixes-cgroupv2

helpers: fix reading cpu stats on cgroup v2

container/common/helpers.go

@@ -61,6 +61,10 @@ var bootTime = func() time.Time {
 }()
 
 func GetSpec(cgroupPaths map[string]string, machineInfoFactory info.MachineInfoFactory, hasNetwork, hasFilesystem bool) (info.ContainerSpec, error) {
+	return getSpecInternal(cgroupPaths, machineInfoFactory, hasNetwork, hasFilesystem, cgroups.IsCgroup2UnifiedMode())
+}
+
+func getSpecInternal(cgroupPaths map[string]string, machineInfoFactory info.MachineInfoFactory, hasNetwork, hasFilesystem, cgroup2UnifiedMode bool) (info.ContainerSpec, error) {
 	var spec info.ContainerSpec
 
 	// Assume unified hierarchy containers.
@@ -77,7 +81,7 @@ func GetSpec(cgroupPaths map[string]string, machineInfoFactory info.MachineInfoF
 		// Use clone_children/events as a workaround as it isn't usually modified. It is only likely changed
 		// immediately after creating a container. If the directory modified time is lower, we use that.
 		cgroupPathFile := path.Join(cgroupPathDir, "cgroup.clone_children")
-		if cgroups.IsCgroup2UnifiedMode() {
+		if cgroup2UnifiedMode {
 			cgroupPathFile = path.Join(cgroupPathDir, "cgroup.events")
 		}
 		fi, err := os.Stat(cgroupPathFile)
@@ -103,17 +107,43 @@ func GetSpec(cgroupPaths map[string]string, machineInfoFactory info.MachineInfoF
 	cpuRoot, ok := cgroupPaths["cpu"]
 	if ok {
 		if utils.FileExists(cpuRoot) {
-			spec.HasCpu = true
-			spec.Cpu.Limit = readUInt64(cpuRoot, "cpu.shares")
-			spec.Cpu.Period = readUInt64(cpuRoot, "cpu.cfs_period_us")
-			quota := readString(cpuRoot, "cpu.cfs_quota_us")
-
-			if quota != "" && quota != "-1" {
-				val, err := strconv.ParseUint(quota, 10, 64)
-				if err != nil {
-					klog.Errorf("GetSpec: Failed to parse CPUQuota from %q: %s", path.Join(cpuRoot, "cpu.cfs_quota_us"), err)
-				} else {
-					spec.Cpu.Quota = val
+			if cgroup2UnifiedMode {
+				spec.HasCpu = true
+
+				weight := readUInt64(cpuRoot, "cpu.weight")
+				if weight > 0 {
+					limit, err := convertCPUWeightToCPULimit(weight)
+					if err != nil {
+						klog.Errorf("GetSpec: Failed to read CPULimit from %q: %s", path.Join(cpuRoot, "cpu.weight"), err)
+					} else {
+						spec.Cpu.Limit = limit
+					}
+				}
+				max := readString(cpuRoot, "cpu.max")
+				if max != "" {
+					splits := strings.SplitN(max, " ", 2)
+					if len(splits) != 2 {
+						klog.Errorf("GetSpec: Failed to parse CPUmax from %q", path.Join(cpuRoot, "cpu.max"))
+					} else {
+						if splits[0] != "max" {
+							spec.Cpu.Quota = parseUint64String(splits[0])
+						}
+						spec.Cpu.Period = parseUint64String(splits[1])
+					}
+				}
+			} else {
+				spec.HasCpu = true
+				spec.Cpu.Limit = readUInt64(cpuRoot, "cpu.shares")
+				spec.Cpu.Period = readUInt64(cpuRoot, "cpu.cfs_period_us")
+				quota := readString(cpuRoot, "cpu.cfs_quota_us")
+
+				if quota != "" && quota != "-1" {
+					val, err := strconv.ParseUint(quota, 10, 64)
+					if err != nil {
+						klog.Errorf("GetSpec: Failed to parse CPUQuota from %q: %s", path.Join(cpuRoot, "cpu.cfs_quota_us"), err)
+					} else {
+						spec.Cpu.Quota = val
+					}
 				}
 			}
 		}
@@ -126,7 +156,7 @@ func GetSpec(cgroupPaths map[string]string, machineInfoFactory info.MachineInfoF
 		if utils.FileExists(cpusetRoot) {
 			spec.HasCpu = true
 			mask := ""
-			if cgroups.IsCgroup2UnifiedMode() {
+			if cgroup2UnifiedMode {
 				mask = readString(cpusetRoot, "cpuset.cpus.effective")
 			} else {
 				mask = readString(cpusetRoot, "cpuset.cpus")
@@ -138,20 +168,20 @@ func GetSpec(cgroupPaths map[string]string, machineInfoFactory info.MachineInfoF
 	// Memory
 	memoryRoot, ok := cgroupPaths["memory"]
 	if ok {
-		if !cgroups.IsCgroup2UnifiedMode() {
-			if utils.FileExists(memoryRoot) {
-				spec.HasMemory = true
-				spec.Memory.Limit = readUInt64(memoryRoot, "memory.limit_in_bytes")
-				spec.Memory.SwapLimit = readUInt64(memoryRoot, "memory.memsw.limit_in_bytes")
-				spec.Memory.Reservation = readUInt64(memoryRoot, "memory.soft_limit_in_bytes")
-			}
-		} else {
+		if cgroup2UnifiedMode {
 			if utils.FileExists(path.Join(memoryRoot, "memory.max")) {
 				spec.HasMemory = true
 				spec.Memory.Reservation = readUInt64(memoryRoot, "memory.high")
 				spec.Memory.Limit = readUInt64(memoryRoot, "memory.max")
 				spec.Memory.SwapLimit = readUInt64(memoryRoot, "memory.swap.max")
 			}
+		} else {
+			if utils.FileExists(memoryRoot) {
+				spec.HasMemory = true
+				spec.Memory.Limit = readUInt64(memoryRoot, "memory.limit_in_bytes")
+				spec.Memory.SwapLimit = readUInt64(memoryRoot, "memory.memsw.limit_in_bytes")
+				spec.Memory.Reservation = readUInt64(memoryRoot, "memory.soft_limit_in_bytes")
+			}
 		}
 	}
 
@@ -176,7 +206,7 @@ func GetSpec(cgroupPaths map[string]string, machineInfoFactory info.MachineInfoF
 	spec.HasFilesystem = hasFilesystem
 
 	ioControllerName := "blkio"
-	if cgroups.IsCgroup2UnifiedMode() {
+	if cgroup2UnifiedMode {
 		ioControllerName = "io"
 	}
 	if blkioRoot, ok := cgroupPaths[ioControllerName]; ok && utils.FileExists(blkioRoot) {
@@ -201,6 +231,37 @@ func readString(dirpath string, file string) string {
 	return strings.TrimSpace(string(out))
 }
 
+// Convert from [1-10000] to [2-262144]
+func convertCPUWeightToCPULimit(weight uint64) (uint64, error) {
+	const (
+		// minWeight is the lowest value possible for cpu.weight
+		minWeight = 1
+		// maxWeight is the highest value possible for cpu.weight
+		maxWeight = 10000
+	)
+	if weight < minWeight || weight > maxWeight {
+		return 0, fmt.Errorf("convertCPUWeightToCPULimit: invalid cpu weight: %v", weight)
+	}
+	return 2 + ((weight-1)*262142)/9999, nil
+}
+
+func parseUint64String(strValue string) uint64 {
+	if strValue == "max" {
+		return math.MaxUint64
+	}
+	if strValue == "" {
+		return 0
+	}
+
+	val, err := strconv.ParseUint(strValue, 10, 64)
+	if err != nil {
+		klog.Errorf("parseUint64String: Failed to parse int %q: %s", strValue, err)
+		return 0
+	}
+
+	return val
+}
+
 func readUInt64(dirpath string, file string) uint64 {
 	out := readString(dirpath, file)
 	if out == "max" {

container/common/helpers_test.go

@@ -15,7 +15,15 @@
 package common
 
 import (
+	"errors"
+	"math"
+	"os"
+	"path/filepath"
 	"testing"
+
+	info "github.com/google/cadvisor/info/v1"
+	v2 "github.com/google/cadvisor/info/v2"
+	"github.com/stretchr/testify/assert"
 )
 
 func BenchmarkListDirectories(b *testing.B) {
@@ -26,3 +34,166 @@ func BenchmarkListDirectories(b *testing.B) {
 		}
 	}
 }
+
+func TestConvertCpuWeightToCpuLimit(t *testing.T) {
+	limit, err := convertCPUWeightToCPULimit(1)
+	if err != nil {
+		t.Fatalf("Error in convertCPUWeightToCPULimit: %s", err)
+	}
+	if limit != 2 {
+		t.Fatalf("convertCPUWeightToCPULimit(1) != 2")
+	}
+	limit, err = convertCPUWeightToCPULimit(10000)
+	if err != nil {
+		t.Fatalf("Error in convertCPUWeightToCPULimit: %s", err)
+	}
+	if limit != 262144 {
+		t.Fatalf("convertCPUWeightToCPULimit(10000) != 262144")
+	}
+	_, err = convertCPUWeightToCPULimit(0)
+	if err == nil {
+		t.Fatalf("convertCPUWeightToCPULimit(0) must raise an error")
+	}
+	_, err = convertCPUWeightToCPULimit(10001)
+	if err == nil {
+		t.Fatalf("convertCPUWeightToCPULimit(10001) must raise an error")
+	}
+}
+
+func TestParseUint64String(t *testing.T) {
+	if parseUint64String("1000") != 1000 {
+		t.Fatalf("parseUint64String(\"1000\") != 1000")
+	}
+	if parseUint64String("-1") != 0 {
+		t.Fatalf("parseUint64String(\"-1\") != 0")
+	}
+	if parseUint64String("0") != 0 {
+		t.Fatalf("parseUint64String(\"0\") != 0")
+	}
+	if parseUint64String("not-a-number") != 0 {
+		t.Fatalf("parseUint64String(\"not-a-number\") != 0")
+	}
+	if parseUint64String(" 1000 ") != 0 {
+		t.Fatalf("parseUint64String(\" 1000 \") != 0")
+	}
+	if parseUint64String("18446744073709551615") != 18446744073709551615 {
+		t.Fatalf("parseUint64String(\"18446744073709551615\") != 18446744073709551615")
+	}
+}
+
+type mockInfoProvider struct {
+	options v2.RequestOptions
+}
+
+func (m *mockInfoProvider) GetRequestedContainersInfo(containerName string, options v2.RequestOptions) (map[string]*info.ContainerInfo, error) {
+	m.options = options
+	return map[string]*info.ContainerInfo{}, nil
+}
+
+func (m *mockInfoProvider) GetVersionInfo() (*info.VersionInfo, error) {
+	return nil, errors.New("not supported")
+}
+
+func (m *mockInfoProvider) GetMachineInfo() (*info.MachineInfo, error) {
+	return &info.MachineInfo{
+		NumCores: 7,
+	}, nil
+}
+
+func TestGetSpecCgroupV1(t *testing.T) {
+	root, err := os.Getwd()
+	if err != nil {
+		t.Fatalf("getwd: %s", err)
+	}
+
+	cgroupPaths := map[string]string{
+		"memory": filepath.Join(root, "test_resources/cgroup_v1/test1/memory"),
+		"cpu":    filepath.Join(root, "test_resources/cgroup_v1/test1/cpu"),
+		"cpuset": filepath.Join(root, "test_resources/cgroup_v1/test1/cpuset"),
+		"pids":   filepath.Join(root, "test_resources/cgroup_v1/test1/pids"),
+	}
+
+	spec, err := getSpecInternal(cgroupPaths, &mockInfoProvider{}, false, false, false)
+	assert.Nil(t, err)
+
+	assert.True(t, spec.HasMemory)
+	assert.EqualValues(t, spec.Memory.Limit, 123456789)
+	assert.EqualValues(t, spec.Memory.SwapLimit, 13579)
+	assert.EqualValues(t, spec.Memory.Reservation, 24680)
+
+	assert.True(t, spec.HasCpu)
+	assert.EqualValues(t, spec.Cpu.Limit, 1025)
+	assert.EqualValues(t, spec.Cpu.Period, 100010)
+	assert.EqualValues(t, spec.Cpu.Quota, 20000)
+
+	assert.EqualValues(t, spec.Cpu.Mask, "0-5")
+
+	assert.True(t, spec.HasProcesses)
+	assert.EqualValues(t, spec.Processes.Limit, 1027)
+
+	assert.False(t, spec.HasHugetlb)
+	assert.False(t, spec.HasDiskIo)
+}
+
+func TestGetSpecCgroupV2(t *testing.T) {
+	root, err := os.Getwd()
+	if err != nil {
+		t.Fatalf("getwd: %s", err)
+	}
+
+	cgroupPaths := map[string]string{
+		"memory": filepath.Join(root, "test_resources/cgroup_v2/test1"),
+		"cpu":    filepath.Join(root, "test_resources/cgroup_v2/test1"),
+		"cpuset": filepath.Join(root, "test_resources/cgroup_v2/test1"),
+		"pids":   filepath.Join(root, "test_resources/cgroup_v2/test1"),
+	}
+
+	spec, err := getSpecInternal(cgroupPaths, &mockInfoProvider{}, false, false, true)
+	assert.Nil(t, err)
+
+	assert.True(t, spec.HasMemory)
+	assert.EqualValues(t, spec.Memory.Limit, 123456789)
+	assert.EqualValues(t, spec.Memory.SwapLimit, 13579)
+	assert.EqualValues(t, spec.Memory.Reservation, 24680)
+
+	assert.True(t, spec.HasCpu)
+	assert.EqualValues(t, spec.Cpu.Limit, 1286)
+	assert.EqualValues(t, spec.Cpu.Period, 100010)
+	assert.EqualValues(t, spec.Cpu.Quota, 20000)
+
+	assert.EqualValues(t, spec.Cpu.Mask, "0-5")
+
+	assert.True(t, spec.HasProcesses)
+	assert.EqualValues(t, spec.Processes.Limit, 1027)
+
+	assert.False(t, spec.HasHugetlb)
+	assert.False(t, spec.HasDiskIo)
+}
+
+func TestGetSpecCgroupV2Max(t *testing.T) {
+	root, err := os.Getwd()
+	assert.Nil(t, err)
+
+	cgroupPaths := map[string]string{
+		"memory": filepath.Join(root, "test_resources/cgroup_v2/test2"),
+		"cpu":    filepath.Join(root, "test_resources/cgroup_v2/test2"),
+		"pids":   filepath.Join(root, "test_resources/cgroup_v2/test2"),
+	}
+
+	spec, err := getSpecInternal(cgroupPaths, &mockInfoProvider{}, false, false, true)
+	assert.Nil(t, err)
+
+	max := uint64(math.MaxUint64)
+
+	assert.True(t, spec.HasMemory)
+	assert.EqualValues(t, spec.Memory.Limit, max)
+	assert.EqualValues(t, spec.Memory.SwapLimit, max)
+	assert.EqualValues(t, spec.Memory.Reservation, max)
+
+	assert.True(t, spec.HasCpu)
+	assert.EqualValues(t, spec.Cpu.Limit, 1286)
+	assert.EqualValues(t, spec.Cpu.Period, 100010)
+	assert.EqualValues(t, spec.Cpu.Quota, 0)
+
+	assert.EqualValues(t, spec.Processes.Limit, max)
+}

container/common/test_resources/cgroup_v1/test1/cpu/cpu.cfs_period_us

@@ -0,0 +1 @@
+100010

container/common/test_resources/cgroup_v1/test1/cpu/cpu.cfs_quota_us

@@ -0,0 +1 @@
+20000

container/common/test_resources/cgroup_v1/test1/cpu/cpu.shares

@@ -0,0 +1 @@
+1025

container/common/test_resources/cgroup_v1/test1/cpuset/cpuset.cpus

@@ -0,0 +1 @@
+0-5

container/common/test_resources/cgroup_v1/test1/memory/memory.limit_in_bytes

@@ -0,0 +1 @@
+123456789

container/common/test_resources/cgroup_v1/test1/memory/memory.memsw.limit_in_bytes

@@ -0,0 +1 @@
+13579

container/common/test_resources/cgroup_v1/test1/memory/memory.soft_limit_in_bytes

@@ -0,0 +1 @@
+24680

container/common/test_resources/cgroup_v1/test1/pids/pids.max

@@ -0,0 +1 @@
+1027

container/common/test_resources/cgroup_v2/test1/cpu.max

@@ -0,0 +1 @@
+20000 100010

container/common/test_resources/cgroup_v2/test1/cpu.weight

@@ -0,0 +1 @@
+50

container/common/test_resources/cgroup_v2/test1/cpuset.cpus.effective

@@ -0,0 +1 @@
+0-5

container/common/test_resources/cgroup_v2/test1/memory.high

@@ -0,0 +1 @@
+24680

container/common/test_resources/cgroup_v2/test1/memory.max

@@ -0,0 +1 @@
+123456789

container/common/test_resources/cgroup_v2/test1/memory.swap.max

@@ -0,0 +1 @@
+13579

container/common/test_resources/cgroup_v2/test1/pids.max

@@ -0,0 +1 @@
+1027

container/common/test_resources/cgroup_v2/test2/cpu.max

@@ -0,0 +1 @@
+max 100010

container/common/test_resources/cgroup_v2/test2/cpu.weight

@@ -0,0 +1 @@
+50

container/common/test_resources/cgroup_v2/test2/memory.high

@@ -0,0 +1 @@
+max

container/common/test_resources/cgroup_v2/test2/memory.max

@@ -0,0 +1 @@
+max

container/common/test_resources/cgroup_v2/test2/memory.swap.max

@@ -0,0 +1 @@
+max

container/common/test_resources/cgroup_v2/test2/pids.max

@@ -0,0 +1 @@
+max