Repository: opencontainers/runc (go)

Issue:
Capture number of IO operations performed with cgroup v2 hierarchy
In cgroup v1, [GetStats](https://github.com/opencontainers/runc/blob/e005fee9d37f3ee35cfc6d4cfee687043656928f/libcontainer/cgroups/cgroups.go#L23) api captures the number of IO operations performed under `IoServicedRecursive` field of [BlkioStats](https://github.com/opencontainers/runc/blob/e005fee9d37f3ee35cfc6d4cfee687043656928f/libcontainer/cgroups/stats.go#L128-L138) struct. But the same stat is missing with cgroup v2 implementation. As you can see [here](https://github.com/opencontainers/runc/blob/e005fee9d37f3ee35cfc6d4cfee687043656928f/libcontainer/cgroups/fs2/io.go#L140), we never populate the `IoServicedRecursive` field for cgroup v2. This resulted in misleading zero valued metrics in kubelet and cadvisor for cgroup v2. As per the [cgroup v2 documentation](https://www.kernel.org/doc/html/latest/admin-guide/cgroup-v2.html#io-interface-files), io.stat file has `rios` and `wios` fields denoting the number of read and write operations performed on a device. Let me know if I can add these stats for cgroup v2 as well?

Original cAdvisor Issue - https://github.com/google/cadvisor/issues/2881

Pull request description:
cgroup2: map io.stats to v1 blkio.stats correctly
Kubelet and cAdvisor depend on the metrics having the same values as in
cgroupv1, but we didn't correctly map the number of read and write IOs
to the correct cgroupv1 stats table (blkio.io_serviced).

In addition, don't leak any extra stats in our output -- if users need
that information we can always add a new field for it.

Change-log entry:

```
* cgroupv2: correctly convert "number of IOs" statistics in a cgroupv1-compatible way (#2967, #2964)
* cgroupv2: support larger than 32-bit IO statistics on 32-bit architectures
```

Fixes #2967
Reported-by: Yashpal Choudhary <yashpal.c1995@gmail.com>
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>

Patch:
diff --git a/libcontainer/cgroups/fs2/io.go b/libcontainer/cgroups/fs2/io.go
index bd6f2088..77174f43 100644
--- a/libcontainer/cgroups/fs2/io.go
+++ b/libcontainer/cgroups/fs2/io.go
@@ -8,6 +8,8 @@ import (
 	"strconv"
 	"strings"
 
+	"github.com/sirupsen/logrus"
+
 	"github.com/opencontainers/runc/libcontainer/cgroups"
 	"github.com/opencontainers/runc/libcontainer/cgroups/fscommon"
 	"github.com/opencontainers/runc/libcontainer/configs"
@@ -88,22 +90,22 @@ func readCgroup2MapFile(dirPath string, name string) (map[string][]string, error
 }
 
 func statIo(dirPath string, stats *cgroups.Stats) error {
-	// more details on the io.stat file format: https://www.kernel.org/doc/Documentation/cgroup-v2.txt
-	var ioServiceBytesRecursive []cgroups.BlkioStatEntry
 	values, err := readCgroup2MapFile(dirPath, "io.stat")
 	if err != nil {
 		return err
 	}
+	// more details on the io.stat file format: https://www.kernel.org/doc/Documentation/cgroup-v2.txt
+	var parsedStats cgroups.BlkioStats
 	for k, v := range values {
 		d := strings.Split(k, ":")
 		if len(d) != 2 {
 			continue
 		}
-		major, err := strconv.ParseUint(d[0], 10, 0)
+		major, err := strconv.ParseUint(d[0], 10, 64)
 		if err != nil {
 			return err
 		}
-		minor, err := strconv.ParseUint(d[1], 10, 0)
+		minor, err := strconv.ParseUint(d[1], 10, 64)
 		if err != nil {
 			return err
 		}
@@ -115,15 +117,32 @@ func statIo(dirPath string, stats *cgroups.Stats) error {
 			}
 			op := d[0]
 
-			// Accommodate the cgroup v1 naming
+			// Map to the cgroupv1 naming and layout (in separate tables).
+			var targetTable *[]cgroups.BlkioStatEntry
 			switch op {
+			// Equivalent to cgroupv1's blkio.io_service_bytes.
 			case "rbytes":
 				op = "Read"
+				targetTable = &parsedStats.IoServiceBytesRecursive
 			case "wbytes":
 				op = "Write"
+				targetTable = &parsedStats.IoServiceBytesRecursive
+			// Equivalent to cgroupv1's blkio.io_serviced.
+			case "rios":
+				op = "Read"
+				targetTable = &parsedStats.IoServicedRecursive
+			case "wios":
+				op = "Write"
+				targetTable = &parsedStats.IoServicedRecursive
+			default:
+				// Skip over entries we cannot map to cgroupv1 stats for now.
+				// In the future we should expand the stats struct to include
+				// them.
+				logrus.Debugf("cgroupv2 io stats: skipping over unmappable %s entry", item)
+				continue
 			}
 
-			value, err := strconv.ParseUint(d[1], 10, 0)
+			value, err := strconv.ParseUint(d[1], 10, 64)
 			if err != nil {
 				return err
 			}
@@ -134,9 +153,9 @@ func statIo(dirPath string, stats *cgroups.Stats) error {
 				Minor: minor,
 				Value: value,
 			}
-			ioServiceBytesRecursive = append(ioServiceBytesRecursive, entry)
+			*targetTable = append(*targetTable, entry)
 		}
 	}
-	stats.BlkioStats = cgroups.BlkioStats{IoServiceBytesRecursive: ioServiceBytesRecursive}
+	stats.BlkioStats = parsedStats
 	return nil
 }


Test patch:
diff --git a/libcontainer/cgroups/fs2/io_test.go b/libcontainer/cgroups/fs2/io_test.go
new file mode 100644
index 00000000..c6cb7a63
--- /dev/null
+++ b/libcontainer/cgroups/fs2/io_test.go
@@ -0,0 +1,87 @@
+package fs2
+
+import (
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"reflect"
+	"sort"
+	"testing"
+
+	"github.com/opencontainers/runc/libcontainer/cgroups"
+	"github.com/opencontainers/runc/libcontainer/cgroups/fscommon"
+)
+
+const exampleIoStatData = `254:1 rbytes=6901432320 wbytes=14245535744 rios=263278 wios=248603 dbytes=0 dios=0
+254:0 rbytes=2702336 wbytes=0 rios=97 wios=0 dbytes=0 dios=0
+259:0 rbytes=6911345664 wbytes=14245536256 rios=264538 wios=244914 dbytes=530485248 dios=2`
+
+var exampleIoStatsParsed = cgroups.BlkioStats{
+	IoServiceBytesRecursive: []cgroups.BlkioStatEntry{
+		{Major: 254, Minor: 1, Value: 6901432320, Op: "Read"},
+		{Major: 254, Minor: 1, Value: 14245535744, Op: "Write"},
+		{Major: 254, Minor: 0, Value: 2702336, Op: "Read"},
+		{Major: 254, Minor: 0, Value: 0, Op: "Write"},
+		{Major: 259, Minor: 0, Value: 6911345664, Op: "Read"},
+		{Major: 259, Minor: 0, Value: 14245536256, Op: "Write"},
+	},
+	IoServicedRecursive: []cgroups.BlkioStatEntry{
+		{Major: 254, Minor: 1, Value: 263278, Op: "Read"},
+		{Major: 254, Minor: 1, Value: 248603, Op: "Write"},
+		{Major: 254, Minor: 0, Value: 97, Op: "Read"},
+		{Major: 254, Minor: 0, Value: 0, Op: "Write"},
+		{Major: 259, Minor: 0, Value: 264538, Op: "Read"},
+		{Major: 259, Minor: 0, Value: 244914, Op: "Write"},
+	},
+}
+
+func lessBlkioStatEntry(a, b cgroups.BlkioStatEntry) bool {
+	if a.Major != b.Major {
+		return a.Major < b.Major
+	}
+	if a.Minor != b.Minor {
+		return a.Minor < b.Minor
+	}
+	if a.Op != b.Op {
+		return a.Op < b.Op
+	}
+	return a.Value < b.Value
+}
+
+func sortBlkioStats(stats *cgroups.BlkioStats) {
+	for _, table := range []*[]cgroups.BlkioStatEntry{
+		&stats.IoServicedRecursive,
+		&stats.IoServiceBytesRecursive,
+	} {
+		sort.SliceStable(*table, func(i, j int) bool { return lessBlkioStatEntry((*table)[i], (*table)[j]) })
+	}
+}
+
+func TestStatIo(t *testing.T) {
+	// We're using a fake cgroupfs.
+	fscommon.TestMode = true
+
+	fakeCgroupDir, err := ioutil.TempDir("", "runc-stat-io-test.*")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(fakeCgroupDir)
+	statPath := filepath.Join(fakeCgroupDir, "io.stat")
+
+	if err := ioutil.WriteFile(statPath, []byte(exampleIoStatData), 0644); err != nil {
+		t.Fatal(err)
+	}
+
+	var gotStats cgroups.Stats
+	if err := statIo(fakeCgroupDir, &gotStats); err != nil {
+		t.Error(err)
+	}
+
+	// Sort the output since statIo uses a map internally.
+	sortBlkioStats(&gotStats.BlkioStats)
+	sortBlkioStats(&exampleIoStatsParsed)
+
+	if !reflect.DeepEqual(gotStats.BlkioStats, exampleIoStatsParsed) {
+		t.Errorf("parsed cgroupv2 io.stat doesn't match expected result: \ngot %#v\nexpected %#v\n", gotStats.BlkioStats, exampleIoStatsParsed)
+	}
+}