libcontainer: Use first cgroup subsystem found (#1792)

libcontainer: Use first cgroup subsystem found

container/libcontainer/helpers.go

@@ -55,19 +55,36 @@ func GetCgroupSubsystems() (CgroupSubsystems, error) {
 	if err != nil {
 		return CgroupSubsystems{}, err
 	}
+
+	return getCgroupSubsystemsHelper(allCgroups)
+}
+
+func getCgroupSubsystemsHelper(allCgroups []cgroups.Mount) (CgroupSubsystems, error) {
 	if len(allCgroups) == 0 {
 		return CgroupSubsystems{}, fmt.Errorf("failed to find cgroup mounts")
 	}
 
 	// Trim the mounts to only the subsystems we care about.
 	supportedCgroups := make([]cgroups.Mount, 0, len(allCgroups))
+	recordedMountpoints := make(map[string]struct{}, len(allCgroups))
 	mountPoints := make(map[string]string, len(allCgroups))
 	for _, mount := range allCgroups {
 		for _, subsystem := range mount.Subsystems {
-			if _, ok := supportedSubsystems[subsystem]; ok {
-				supportedCgroups = append(supportedCgroups, mount)
-				mountPoints[subsystem] = mount.Mountpoint
+			if _, ok := supportedSubsystems[subsystem]; !ok {
+				// Unsupported subsystem
+				continue
 			}
+			if _, ok := mountPoints[subsystem]; ok {
+				// duplicate mount for this subsystem; use the first one we saw
+				glog.V(5).Infof("skipping %s, already using mount at %s", mount.Mountpoint, mountPoints[subsystem])
+				continue
+			}
+			if _, ok := recordedMountpoints[mount.Mountpoint]; !ok {
+				// avoid appending the same mount twice in e.g. `cpu,cpuacct` case
+				supportedCgroups = append(supportedCgroups, mount)
+				recordedMountpoints[mount.Mountpoint] = struct{}{}
+			}
+			mountPoints[subsystem] = mount.Mountpoint
 		}
 	}
 

container/libcontainer/helpers_test.go

@@ -15,7 +15,12 @@
 package libcontainer
 
 import (
+	"fmt"
 	"os"
+	"path/filepath"
+	"reflect"
+	"sort"
+	"strings"
 	"testing"
 
 	info "github.com/google/cadvisor/info/v1"
@@ -134,3 +139,105 @@ func TestMorePossibleCPUs(t *testing.T) {
 		t.Fatalf("expected %+v == %+v", ret, expected)
 	}
 }
+
+var defaultCgroupSubsystems = []string{
+	"systemd", "freezer", "memory", "blkio", "hugetlb", "net_cls,net_prio", "pids", "cpu,cpuacct", "devices", "cpuset", "perf_events",
+}
+
+func cgroupMountsAt(path string, subsystems []string) []cgroups.Mount {
+	res := []cgroups.Mount{}
+	for _, subsystem := range subsystems {
+		res = append(res, cgroups.Mount{
+			Root:       "/",
+			Subsystems: strings.Split(subsystem, ","),
+			Mountpoint: filepath.Join(path, subsystem),
+		})
+	}
+	return res
+}
+
+func TestGetCgroupSubsystems(t *testing.T) {
+	ourSubsystems := []string{"cpu,cpuacct", "devices", "memory", "cpuset", "blkio"}
+
+	testCases := []struct {
+		mounts   []cgroups.Mount
+		expected CgroupSubsystems
+		err      bool
+	}{
+		{
+			mounts: []cgroups.Mount{},
+			err:    true,
+		},
+		{
+			// normal case
+			mounts: cgroupMountsAt("/sys/fs/cgroup", defaultCgroupSubsystems),
+			expected: CgroupSubsystems{
+				MountPoints: map[string]string{
+					"blkio":   "/sys/fs/cgroup/blkio",
+					"cpu":     "/sys/fs/cgroup/cpu,cpuacct",
+					"cpuacct": "/sys/fs/cgroup/cpu,cpuacct",
+					"cpuset":  "/sys/fs/cgroup/cpuset",
+					"devices": "/sys/fs/cgroup/devices",
+					"memory":  "/sys/fs/cgroup/memory",
+				},
+				Mounts: cgroupMountsAt("/sys/fs/cgroup", ourSubsystems),
+			},
+		},
+		{
+			// multiple croup subsystems, should ignore second one
+			mounts: append(cgroupMountsAt("/sys/fs/cgroup", defaultCgroupSubsystems),
+				cgroupMountsAt("/var/lib/rkt/pods/run/ccdd4e36-2d4c-49fd-8b94-4fb06133913d/stage1/rootfs/opt/stage2/flannel/rootfs/sys/fs/cgroup", defaultCgroupSubsystems)...),
+			expected: CgroupSubsystems{
+				MountPoints: map[string]string{
+					"blkio":   "/sys/fs/cgroup/blkio",
+					"cpu":     "/sys/fs/cgroup/cpu,cpuacct",
+					"cpuacct": "/sys/fs/cgroup/cpu,cpuacct",
+					"cpuset":  "/sys/fs/cgroup/cpuset",
+					"devices": "/sys/fs/cgroup/devices",
+					"memory":  "/sys/fs/cgroup/memory",
+				},
+				Mounts: cgroupMountsAt("/sys/fs/cgroup", ourSubsystems),
+			},
+		},
+		{
+			// most subsystems not mounted
+			mounts: append(cgroupMountsAt("/sys/fs/cgroup", []string{"cpu"})),
+			expected: CgroupSubsystems{
+				MountPoints: map[string]string{
+					"cpu": "/sys/fs/cgroup/cpu",
+				},
+				Mounts: cgroupMountsAt("/sys/fs/cgroup", []string{"cpu"}),
+			},
+		},
+	}
+
+	for i, testCase := range testCases {
+		subSystems, err := getCgroupSubsystemsHelper(testCase.mounts)
+		if testCase.err {
+			if err == nil {
+				t.Fatalf("[case %d] Expected error but didn't get one", i)
+			}
+			continue
+		}
+		if err != nil {
+			t.Fatalf("[case %d] Expected no error, but got %v", i, err)
+		}
+		assertCgroupSubsystemsEqual(t, testCase.expected, subSystems, fmt.Sprintf("[case %d]", i))
+	}
+}
+
+func assertCgroupSubsystemsEqual(t *testing.T, expected, actual CgroupSubsystems, message string) {
+	if !reflect.DeepEqual(expected.MountPoints, actual.MountPoints) {
+		t.Fatalf("%s Expected %v == %v", message, expected.MountPoints, actual.MountPoints)
+	}
+
+	sort.Slice(expected.Mounts, func(i, j int) bool {
+		return expected.Mounts[i].Mountpoint < expected.Mounts[j].Mountpoint
+	})
+	sort.Slice(actual.Mounts, func(i, j int) bool {
+		return actual.Mounts[i].Mountpoint < actual.Mounts[j].Mountpoint
+	})
+	if !reflect.DeepEqual(expected.Mounts, actual.Mounts) {
+		t.Fatalf("%s Expected %v == %v", message, expected.Mounts, actual.Mounts)
+	}
+}