From 7cd062d7be8a6089381e965a06c50be932b0d0d7 Mon Sep 17 00:00:00 2001
From: Kir Kolyshkin <kolyshkin@gmail.com>
Date: Mon, 23 Nov 2020 16:05:49 -0800
Subject: [PATCH] libct/cgroup/utils: fix GetCgroupMounts(all=true)

The `all` argument was introduced by commit f5579964014 specifically
for use by cAdvisor (see [1]), but there were no test cases added,
so it was later broken by 5ee0648bfbe037bcf4b which started incrementing
numFound unconditionally.

Fix this (by not checking numFound in case all is true), and add a
simple test case to avoid future regressions.

[1] https://github.com/google/cadvisor/pull/1476

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
---
 libcontainer/cgroups/utils_test.go | 31 ++++++++++++++++++++++++++++--
 libcontainer/cgroups/v1_utils.go   |  2 +-
 2 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/libcontainer/cgroups/utils_test.go b/libcontainer/cgroups/utils_test.go
index 4a5d95eb6..1645ae7b0 100644
--- a/libcontainer/cgroups/utils_test.go
+++ b/libcontainer/cgroups/utils_test.go
@@ -181,8 +181,11 @@ const cgroup2Mountinfo = `18 64 0:18 / /sys rw,nosuid,nodev,noexec,relatime shar
 
 func TestGetCgroupMounts(t *testing.T) {
 	type testData struct {
-		mountInfo  string
-		root       string
+		mountInfo string
+		root      string
+		// all is the total number of records expected with all=true,
+		// or 0 for no extra records expected (most cases).
+		all        int
 		subsystems map[string]bool
 	}
 	testTable := []testData{
@@ -223,6 +226,7 @@ func TestGetCgroupMounts(t *testing.T) {
 		{
 			mountInfo: bedrockMountinfo,
 			root:      "/",
+			all:       50,
 			subsystems: map[string]bool{
 				"name=systemd": false,
 				"cpuset":       false,
@@ -274,6 +278,29 @@ func TestGetCgroupMounts(t *testing.T) {
 				t.Fatalf("subsystem %s not found in Subsystems field %v", ss, m.Subsystems)
 			}
 		}
+		// Test the all=true case.
+
+		// Reset the test input.
+		mi = bytes.NewBufferString(td.mountInfo)
+		for k := range td.subsystems {
+			td.subsystems[k] = false
+		}
+		cgMountsAll, err := getCgroupMountsHelper(td.subsystems, mi, true)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if td.all == 0 {
+			// Results with and without "all" should be the same.
+			if len(cgMounts) != len(cgMountsAll) || !reflect.DeepEqual(cgMounts, cgMountsAll) {
+				t.Errorf("expected same results, got (all=false) %v, (all=true) %v", cgMounts, cgMountsAll)
+			}
+		} else {
+			// Make sure we got all records.
+			if len(cgMountsAll) != td.all {
+				t.Errorf("expected %d records, got %d (%+v)", td.all, len(cgMountsAll), cgMountsAll)
+			}
+		}
+
 	}
 }
 
diff --git a/libcontainer/cgroups/v1_utils.go b/libcontainer/cgroups/v1_utils.go
index 8b9275fb9..f610ed8c4 100644
--- a/libcontainer/cgroups/v1_utils.go
+++ b/libcontainer/cgroups/v1_utils.go
@@ -173,7 +173,7 @@ func getCgroupMountsHelper(ss map[string]bool, mi io.Reader, all bool) ([]Mount,
 	res := make([]Mount, 0, len(ss))
 	scanner := bufio.NewScanner(mi)
 	numFound := 0
-	for scanner.Scan() && numFound < len(ss) {
+	for scanner.Scan() && (all || numFound < len(ss)) {
 		txt := scanner.Text()
 		sepIdx := strings.Index(txt, " - ")
 		if sepIdx == -1 {