Repository: opencontainers/runc
Pull request #2408 (2020-05-15 12:47:59)

Add cpuset statistics
Adds statistics for cpuset cgroup

Signed-off-by: Piotr Wagner <piotr.wagner@intel.com>

kolyshkin: @piowag you need to squash your commits
kolyshkin: @piowag please let us know if you still can / want to work on this one
piowag: Sorry, I was absent a little. Yes I still can and want to work on this one.
kolyshkin: One thing I don't like about this patch is it requires all the cpuset files being present. On my system (kernel 5.6) I do not have, for example, `cpuset.mem_hardwall`. Does that mean that `GetStats()` will now return an error?

For example, `(* BlkioGroup).GetStat` ignores the ENOENT, returning no errors if some files are not present.
piowag: Ignoring ENOENT for all files, but cpuset.cpus and cpuset.mems which are mandatory. 
piowag: @AkihiroSuda Could you take a look at this PR?
AkihiroSuda: @piowag WDYT about multierr?
piowag: Not sure if adding new 3-rd party library just to get rid of few lines is required. As for retaining the error type, it shouldn't be used anymore outside the logic of the `GetStats` function
piowag: What do you think about changes introduced since your review @kolyshkin ?
AkihiroSuda: @kolyshkin PTAL?

AkihiroSuda: Ping @kolyshkin 
AkihiroSuda: Sorry this needs rebase

Creatone: @kolyshkin @AkihiroSuda 
AkihiroSuda: @kolyshkin LGTY?

AkihiroSuda: @piowag Sorry, please rebase to retrigger CI

AkihiroSuda: @opencontainers/runc-maintainers PTAL. This has been open so long.

kolyshkin: @Creatone thanks for working on this! Can you please squash the commits?
AkihiroSuda: GitHub web ui says "Required statuses must pass before merging".

Seems to need to be rebased again to pick the recent CI changes, sorry


AkihiroSuda: @cyphar @kolyshkin Can we merge this to master directly without waiting for CI retrigger?


Creatone: @AkihiroSuda @kolyshkin 

Patch:
diff --git a/events.go b/events.go
index e749d519..0c7d4e3c 100644
--- a/events.go
+++ b/events.go
@@ -132,6 +132,8 @@ func convertLibcontainerStats(ls *libcontainer.Stats) *types.Stats {
 	s.CPU.Throttling.ThrottledPeriods = cg.CpuStats.ThrottlingData.ThrottledPeriods
 	s.CPU.Throttling.ThrottledTime = cg.CpuStats.ThrottlingData.ThrottledTime
 
+	s.CPUSet = types.CPUSet(cg.CPUSetStats)
+
 	s.Memory.Cache = cg.MemoryStats.Cache
 	s.Memory.Kernel = convertMemoryEntry(cg.MemoryStats.KernelUsage)
 	s.Memory.KernelTCP = convertMemoryEntry(cg.MemoryStats.KernelTCPUsage)
diff --git a/libcontainer/cgroups/fs/cpuset.go b/libcontainer/cgroups/fs/cpuset.go
index e7b5cf67..d79ecb11 100644
--- a/libcontainer/cgroups/fs/cpuset.go
+++ b/libcontainer/cgroups/fs/cpuset.go
@@ -3,8 +3,11 @@
 package fs
 
 import (
+	"fmt"
 	"os"
 	"path/filepath"
+	"strconv"
+	"strings"
 
 	"github.com/moby/sys/mountinfo"
 	"github.com/opencontainers/runc/libcontainer/cgroups"
@@ -39,7 +42,106 @@ func (s *CpusetGroup) Set(path string, cgroup *configs.Cgroup) error {
 	return nil
 }
 
+func getCpusetStat(path string, filename string) ([]uint16, error) {
+	var extracted []uint16
+	fileContent, err := fscommon.GetCgroupParamString(path, filename)
+	if err != nil {
+		return extracted, err
+	}
+	if len(fileContent) == 0 {
+		return extracted, fmt.Errorf("%s found to be empty", filepath.Join(path, filename))
+	}
+
+	for _, s := range strings.Split(fileContent, ",") {
+		splitted := strings.SplitN(s, "-", 3)
+		switch len(splitted) {
+		case 3:
+			return extracted, fmt.Errorf("invalid values in %s", filepath.Join(path, filename))
+		case 2:
+			min, err := strconv.ParseUint(splitted[0], 10, 16)
+			if err != nil {
+				return extracted, err
+			}
+			max, err := strconv.ParseUint(splitted[1], 10, 16)
+			if err != nil {
+				return extracted, err
+			}
+			if min > max {
+				return extracted, fmt.Errorf("invalid values in %s", filepath.Join(path, filename))
+			}
+			for i := min; i <= max; i++ {
+				extracted = append(extracted, uint16(i))
+			}
+		case 1:
+			value, err := strconv.ParseUint(s, 10, 16)
+			if err != nil {
+				return extracted, err
+			}
+			extracted = append(extracted, uint16(value))
+		}
+	}
+
+	return extracted, nil
+}
+
 func (s *CpusetGroup) GetStats(path string, stats *cgroups.Stats) error {
+	var err error
+
+	stats.CPUSetStats.CPUs, err = getCpusetStat(path, "cpuset.cpus")
+	if err != nil && !errors.Is(err, os.ErrNotExist) {
+		return err
+	}
+
+	stats.CPUSetStats.CPUExclusive, err = fscommon.GetCgroupParamUint(path, "cpuset.cpu_exclusive")
+	if err != nil && !errors.Is(err, os.ErrNotExist) {
+		return err
+	}
+
+	stats.CPUSetStats.Mems, err = getCpusetStat(path, "cpuset.mems")
+	if err != nil && !errors.Is(err, os.ErrNotExist) {
+		return err
+	}
+
+	stats.CPUSetStats.MemHardwall, err = fscommon.GetCgroupParamUint(path, "cpuset.mem_hardwall")
+	if err != nil && !errors.Is(err, os.ErrNotExist) {
+		return err
+	}
+
+	stats.CPUSetStats.MemExclusive, err = fscommon.GetCgroupParamUint(path, "cpuset.mem_exclusive")
+	if err != nil && !errors.Is(err, os.ErrNotExist) {
+		return err
+	}
+
+	stats.CPUSetStats.MemoryMigrate, err = fscommon.GetCgroupParamUint(path, "cpuset.memory_migrate")
+	if err != nil && !errors.Is(err, os.ErrNotExist) {
+		return err
+	}
+
+	stats.CPUSetStats.MemorySpreadPage, err = fscommon.GetCgroupParamUint(path, "cpuset.memory_spread_page")
+	if err != nil && !errors.Is(err, os.ErrNotExist) {
+		return err
+	}
+
+	stats.CPUSetStats.MemorySpreadSlab, err = fscommon.GetCgroupParamUint(path, "cpuset.memory_spread_slab")
+	if err != nil && !errors.Is(err, os.ErrNotExist) {
+		return err
+	}
+
+	stats.CPUSetStats.MemoryPressure, err = fscommon.GetCgroupParamUint(path, "cpuset.memory_pressure")
+	if err != nil && !errors.Is(err, os.ErrNotExist) {
+		return err
+	}
+
+	stats.CPUSetStats.SchedLoadBalance, err = fscommon.GetCgroupParamUint(path, "cpuset.sched_load_balance")
+	if err != nil && !errors.Is(err, os.ErrNotExist) {
+		return err
+	}
+
+	stats.CPUSetStats.SchedRelaxDomainLevel, err = fscommon.GetCgroupParamInt(path, "cpuset.sched_relax_domain_level")
+	if err != nil && !errors.Is(err, os.ErrNotExist) {
+		return err
+	}
+
 	return nil
 }
 
diff --git a/libcontainer/cgroups/fscommon/utils.go b/libcontainer/cgroups/fscommon/utils.go
index 7c387a8b..2e4e837f 100644
--- a/libcontainer/cgroups/fscommon/utils.go
+++ b/libcontainer/cgroups/fscommon/utils.go
@@ -72,6 +72,25 @@ func GetCgroupParamUint(path, file string) (uint64, error) {
 	return res, nil
 }
 
+// GetCgroupParamInt reads a single int64 value from specified cgroup file.
+// If the value read is "max", the math.MaxInt64 is returned.
+func GetCgroupParamInt(path, file string) (int64, error) {
+	contents, err := ReadFile(path, file)
+	if err != nil {
+		return 0, err
+	}
+	contents = strings.TrimSpace(contents)
+	if contents == "max" {
+		return math.MaxInt64, nil
+	}
+
+	res, err := strconv.ParseInt(contents, 10, 64)
+	if err != nil {
+		return res, fmt.Errorf("unable to parse %q as a int from Cgroup file %q", contents, path+"/"+file)
+	}
+	return res, nil
+}
+
 // GetCgroupParamString reads a string from the specified cgroup file.
 func GetCgroupParamString(path, file string) (string, error) {
 	contents, err := ReadFile(path, file)
diff --git a/libcontainer/cgroups/stats.go b/libcontainer/cgroups/stats.go
index 7ac81660..e7f9c462 100644
--- a/libcontainer/cgroups/stats.go
+++ b/libcontainer/cgroups/stats.go
@@ -39,6 +39,33 @@ type CpuStats struct {
 	ThrottlingData ThrottlingData `json:"throttling_data,omitempty"`
 }
 
+type CPUSetStats struct {
+	// List of the physical numbers of the CPUs on which processes
+	// in that cpuset are allowed to execute
+	CPUs []uint16 `json:"cpus,omitempty"`
+	// cpu_exclusive flag
+	CPUExclusive uint64 `json:"cpu_exclusive"`
+	// List of memory nodes on which processes in that cpuset
+	// are allowed to allocate memory
+	Mems []uint16 `json:"mems,omitempty"`
+	// mem_hardwall flag
+	MemHardwall uint64 `json:"mem_hardwall"`
+	// mem_exclusive flag
+	MemExclusive uint64 `json:"mem_exclusive"`
+	// memory_migrate flag
+	MemoryMigrate uint64 `json:"memory_migrate"`
+	// memory_spread page flag
+	MemorySpreadPage uint64 `json:"memory_spread_page"`
+	// memory_spread slab flag
+	MemorySpreadSlab uint64 `json:"memory_spread_slab"`
+	// memory_pressure
+	MemoryPressure uint64 `json:"memory_pressure"`
+	// sched_load balance flag
+	SchedLoadBalance uint64 `json:"sched_load_balance"`
+	// sched_relax_domain_level
+	SchedRelaxDomainLevel int64 `json:"sched_relax_domain_level"`
+}
+
 type MemoryData struct {
 	Usage    uint64 `json:"usage,omitempty"`
 	MaxUsage uint64 `json:"max_usage,omitempty"`
@@ -121,6 +148,7 @@ type HugetlbStats struct {
 
 type Stats struct {
 	CpuStats    CpuStats    `json:"cpu_stats,omitempty"`
+	CPUSetStats CPUSetStats `json:"cpuset_stats,omitempty"`
 	MemoryStats MemoryStats `json:"memory_stats,omitempty"`
 	PidsStats   PidsStats   `json:"pids_stats,omitempty"`
 	BlkioStats  BlkioStats  `json:"blkio_stats,omitempty"`
diff --git a/types/events.go b/types/events.go
index 6f9a12f1..81bde829 100644
--- a/types/events.go
+++ b/types/events.go
@@ -12,6 +12,7 @@ type Event struct {
 // stats is the runc specific stats structure for stability when encoding and decoding stats.
 type Stats struct {
 	CPU               Cpu                 `json:"cpu"`
+	CPUSet            CPUSet              `json:"cpuset"`
 	Memory            Memory              `json:"memory"`
 	Pids              Pids                `json:"pids"`
 	Blkio             Blkio               `json:"blkio"`
@@ -70,6 +71,20 @@ type Cpu struct {
 	Throttling Throttling `json:"throttling,omitempty"`
 }
 
+type CPUSet struct {
+	CPUs                  []uint16 `json:"cpus,omitempty"`
+	CPUExclusive          uint64   `json:"cpu_exclusive"`
+	Mems                  []uint16 `json:"mems,omitempty"`
+	MemHardwall           uint64   `json:"mem_hardwall"`
+	MemExclusive          uint64   `json:"mem_exclusive"`
+	MemoryMigrate         uint64   `json:"memory_migrate"`
+	MemorySpreadPage      uint64   `json:"memory_spread_page"`
+	MemorySpreadSlab      uint64   `json:"memory_spread_slab"`
+	MemoryPressure        uint64   `json:"memory_pressure"`
+	SchedLoadBalance      uint64   `json:"sched_load_balance"`
+	SchedRelaxDomainLevel int64    `json:"sched_relax_domain_level"`
+}
+
 type MemoryEntry struct {
 	Limit   uint64 `json:"limit"`
 	Usage   uint64 `json:"usage,omitempty"`


Test patch:
diff --git a/libcontainer/cgroups/fs/cpuset_test.go b/libcontainer/cgroups/fs/cpuset_test.go
index 927e6311..8a49e440 100644
--- a/libcontainer/cgroups/fs/cpuset_test.go
+++ b/libcontainer/cgroups/fs/cpuset_test.go
@@ -3,12 +3,42 @@
 package fs
 
 import (
+	"reflect"
 	"testing"
 
+	"github.com/opencontainers/runc/libcontainer/cgroups"
 	"github.com/opencontainers/runc/libcontainer/cgroups/fscommon"
 )
 
-func TestCpusetSetCpus(t *testing.T) {
+const (
+	cpus                  = "0-2,7,12-14\n"
+	cpuExclusive          = "1\n"
+	mems                  = "1-4,6,9\n"
+	memHardwall           = "0\n"
+	memExclusive          = "0\n"
+	memoryMigrate         = "1\n"
+	memorySpreadPage      = "0\n"
+	memorySpeadSlab       = "1\n"
+	memoryPressure        = "34377\n"
+	schedLoadBalance      = "1\n"
+	schedRelaxDomainLevel = "-1\n"
+)
+
+var cpusetTestFiles = map[string]string{
+	"cpuset.cpus":                     cpus,
+	"cpuset.cpu_exclusive":            cpuExclusive,
+	"cpuset.mems":                     mems,
+	"cpuset.mem_hardwall":             memHardwall,
+	"cpuset.mem_exclusive":            memExclusive,
+	"cpuset.memory_migrate":           memoryMigrate,
+	"cpuset.memory_spread_page":       memorySpreadPage,
+	"cpuset.memory_spread_slab":       memorySpeadSlab,
+	"cpuset.memory_pressure":          memoryPressure,
+	"cpuset.sched_load_balance":       schedLoadBalance,
+	"cpuset.sched_relax_domain_level": schedRelaxDomainLevel,
+}
+
+func TestCPUSetSetCpus(t *testing.T) {
 	helper := NewCgroupTestUtil("cpuset", t)
 	defer helper.cleanup()
 
@@ -37,7 +67,7 @@ func TestCpusetSetCpus(t *testing.T) {
 	}
 }
 
-func TestCpusetSetMems(t *testing.T) {
+func TestCPUSetSetMems(t *testing.T) {
 	helper := NewCgroupTestUtil("cpuset", t)
 	defer helper.cleanup()
 
@@ -65,3 +95,152 @@ func TestCpusetSetMems(t *testing.T) {
 		t.Fatal("Got the wrong value, set cpuset.mems failed.")
 	}
 }
+
+func TestCPUSetStatsCorrect(t *testing.T) {
+	helper := NewCgroupTestUtil("cpuset", t)
+	defer helper.cleanup()
+	helper.writeFileContents(cpusetTestFiles)
+
+	cpuset := &CpusetGroup{}
+	actualStats := *cgroups.NewStats()
+	err := cpuset.GetStats(helper.CgroupPath, &actualStats)
+	if err != nil {
+		t.Fatal(err)
+	}
+	expectedStats := cgroups.CPUSetStats{
+		CPUs:                  []uint16{0, 1, 2, 7, 12, 13, 14},
+		CPUExclusive:          1,
+		Mems:                  []uint16{1, 2, 3, 4, 6, 9},
+		MemoryMigrate:         1,
+		MemHardwall:           0,
+		MemExclusive:          0,
+		MemorySpreadPage:      0,
+		MemorySpreadSlab:      1,
+		MemoryPressure:        34377,
+		SchedLoadBalance:      1,
+		SchedRelaxDomainLevel: -1}
+	if !reflect.DeepEqual(expectedStats, actualStats.CPUSetStats) {
+		t.Fatalf("Expected Cpuset stats usage %#v but found %#v",
+			expectedStats, actualStats.CPUSetStats)
+	}
+
+}
+
+func TestCPUSetStatsMissingFiles(t *testing.T) {
+	for _, testCase := range []struct {
+		desc               string
+		filename, contents string
+		removeFile         bool
+	}{
+		{
+			desc:       "empty cpus file",
+			filename:   "cpuset.cpus",
+			contents:   "",
+			removeFile: false,
+		},
+		{
+			desc:       "empty mems file",
+			filename:   "cpuset.mems",
+			contents:   "",
+			removeFile: false,
+		},
+		{
+			desc:       "corrupted cpus file",
+			filename:   "cpuset.cpus",
+			contents:   "0-3,*4^2",
+			removeFile: false,
+		},
+		{
+			desc:       "corrupted mems file",
+			filename:   "cpuset.mems",
+			contents:   "0,1,2-5,8-7",
+			removeFile: false,
+		},
+		{
+			desc:       "missing cpu_exclusive file",
+			filename:   "cpuset.cpu_exclusive",
+			contents:   "",
+			removeFile: true,
+		},
+		{
+			desc:       "missing memory_migrate file",
+			filename:   "cpuset.memory_migrate",
+			contents:   "",
+			removeFile: true,
+		},
+		{
+			desc:       "missing mem_hardwall file",
+			filename:   "cpuset.mem_hardwall",
+			contents:   "",
+			removeFile: true,
+		},
+		{
+			desc:       "missing mem_exclusive file",
+			filename:   "cpuset.mem_exclusive",
+			contents:   "",
+			removeFile: true,
+		},
+		{
+			desc:       "missing memory_spread_page file",
+			filename:   "cpuset.memory_spread_page",
+			contents:   "",
+			removeFile: true,
+		},
+		{
+			desc:       "missing memory_spread_slab file",
+			filename:   "cpuset.memory_spread_slab",
+			contents:   "",
+			removeFile: true,
+		},
+		{
+			desc:       "missing memory_pressure file",
+			filename:   "cpuset.memory_pressure",
+			contents:   "",
+			removeFile: true,
+		},
+		{
+			desc:       "missing sched_load_balance file",
+			filename:   "cpuset.sched_load_balance",
+			contents:   "",
+			removeFile: true,
+		},
+		{
+			desc:       "missing sched_relax_domain_level file",
+			filename:   "cpuset.sched_relax_domain_level",
+			contents:   "",
+			removeFile: true,
+		},
+	} {
+		t.Run(testCase.desc, func(t *testing.T) {
+			helper := NewCgroupTestUtil("cpuset", t)
+			defer helper.cleanup()
+
+			tempCpusetTestFiles := map[string]string{}
+			for i, v := range cpusetTestFiles {
+				tempCpusetTestFiles[i] = v
+			}
+
+			if testCase.removeFile {
+				delete(tempCpusetTestFiles, testCase.filename)
+				helper.writeFileContents(tempCpusetTestFiles)
+				cpuset := &CpusetGroup{}
+				actualStats := *cgroups.NewStats()
+				err := cpuset.GetStats(helper.CgroupPath, &actualStats)
+
+				if err != nil {
+					t.Errorf("failed unexpectedly: %q", err)
+				}
+			} else {
+				tempCpusetTestFiles[testCase.filename] = testCase.contents
+				helper.writeFileContents(tempCpusetTestFiles)
+				cpuset := &CpusetGroup{}
+				actualStats := *cgroups.NewStats()
+				err := cpuset.GetStats(helper.CgroupPath, &actualStats)
+
+				if err == nil {
+					t.Error("failed to return expected error")
+				}
+			}
+		})
+	}
+}