Convert CgroupPaths from struct to map

kolyshkin · Paweł Szulik · commit 4d1fb1cc9690 · 2021-10-21T19:43:19.000+02:00
As this structure only have 1 element now, it does not make sense to
have it as a structure.

Signed-off-by: Kir Kolyshkin &lt;kolyshkin@gmail.com&gt;
diff --git a/cmd/internal/container/mesos/factory.go b/cmd/internal/container/mesos/factory.go
@@ -68,7 +68,7 @@ func (f *mesosFactory) NewContainerHandler(name string, metadataEnvAllowList []s
 
 	return newMesosContainerHandler(
 		name,
-		&f.cgroupSubsystems,
+		f.cgroupSubsystems,
 		f.machineInfoFactory,
 		f.fsInfo,
 		f.includedMetrics,
diff --git a/cmd/internal/container/mesos/factory_test.go b/cmd/internal/container/mesos/factory_test.go
@@ -59,7 +59,7 @@ func TestCanHandleAndAccept(t *testing.T) {
 
 	f := &mesosFactory{
 		machineInfoFactory: nil,
-		cgroupSubsystems:   containerlibcontainer.CgroupSubsystems{},
+		cgroupSubsystems:   nil,
 		fsInfo:             nil,
 		includedMetrics:    nil,
 		client:             fakeMesosAgentClient(testContainers, nil),
diff --git a/cmd/internal/container/mesos/handler.go b/cmd/internal/container/mesos/handler.go
@@ -52,15 +52,15 @@ type mesosContainerHandler struct {
 
 func newMesosContainerHandler(
 	name string,
-	cgroupSubsystems *containerlibcontainer.CgroupSubsystems,
+	cgroupSubsystems containerlibcontainer.CgroupSubsystems,
 	machineInfoFactory info.MachineInfoFactory,
 	fsInfo fs.FsInfo,
 	includedMetrics container.MetricSet,
 	inHostNamespace bool,
 	metadataEnvAllowList []string,
 	client mesosAgentClient,
 ) (container.ContainerHandler, error) {
-	cgroupPaths := common.MakeCgroupPaths(cgroupSubsystems.MountPoints, name)
+	cgroupPaths := common.MakeCgroupPaths(cgroupSubsystems, name)
 
 	// Generate the equivalent cgroup manager for this container.
 	cgroupManager, err := containerlibcontainer.NewCgroupManager(name, cgroupPaths)
diff --git a/cmd/internal/container/mesos/handler_test.go b/cmd/internal/container/mesos/handler_test.go
@@ -42,7 +42,7 @@ func TestContainerReference(t *testing.T) {
 		name                 string
 		machineInfoFactory   info.MachineInfoFactory
 		fsInfo               fs.FsInfo
-		cgroupSubsystems     *containerlibcontainer.CgroupSubsystems
+		cgroupSubsystems     containerlibcontainer.CgroupSubsystems
 		inHostNamespace      bool
 		metadataEnvAllowList []string
 		includedMetrics      container.MetricSet
@@ -57,7 +57,7 @@ func TestContainerReference(t *testing.T) {
 			"/mesos/04e20821-67d3-4bf7-96b4-7d4495f50b28",
 			nil,
 			nil,
-			&containerlibcontainer.CgroupSubsystems{},
+			nil,
 			false,
 			[]string{},
 			nil,
@@ -71,7 +71,7 @@ func TestContainerReference(t *testing.T) {
 			"/mesos/04e20821-67d3-4bf7-96b4-7d4495f50b28",
 			nil,
 			nil,
-			&containerlibcontainer.CgroupSubsystems{},
+			nil,
 			false,
 			[]string{},
 			nil,
@@ -85,7 +85,7 @@ func TestContainerReference(t *testing.T) {
 			"/mesos/04e20821-67d3-4bf7-96b4-7d4495f50b28",
 			nil,
 			nil,
-			&containerlibcontainer.CgroupSubsystems{},
+			nil,
 			false,
 			[]string{},
 			nil,
diff --git a/container/containerd/factory.go b/container/containerd/factory.go
@@ -76,7 +76,7 @@ func (f *containerdFactory) NewContainerHandler(name string, metadataEnvAllowLis
 		name,
 		f.machineInfoFactory,
 		f.fsInfo,
-		&f.cgroupSubsystems,
+		f.cgroupSubsystems,
 		inHostNamespace,
 		containerdMetadataEnvAllowList,
 		f.includedMetrics,
diff --git a/container/containerd/factory_test.go b/container/containerd/factory_test.go
@@ -21,8 +21,6 @@ import (
 	"github.com/containerd/typeurl"
 	specs "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/stretchr/testify/assert"
-
-	containerlibcontainer "github.com/google/cadvisor/container/libcontainer"
 )
 
 func TestIsContainerName(t *testing.T) {
@@ -59,7 +57,7 @@ func TestCanHandleAndAccept(t *testing.T) {
 
 	f := &containerdFactory{
 		client:             mockcontainerdClient(testContainers, nil),
-		cgroupSubsystems:   containerlibcontainer.CgroupSubsystems{},
+		cgroupSubsystems:   nil,
 		fsInfo:             nil,
 		machineInfoFactory: nil,
 		includedMetrics:    nil,
diff --git a/container/containerd/handler.go b/container/containerd/handler.go
@@ -59,13 +59,13 @@ func newContainerdContainerHandler(
 	name string,
 	machineInfoFactory info.MachineInfoFactory,
 	fsInfo fs.FsInfo,
-	cgroupSubsystems *containerlibcontainer.CgroupSubsystems,
+	cgroupSubsystems containerlibcontainer.CgroupSubsystems,
 	inHostNamespace bool,
 	metadataEnvAllowList []string,
 	includedMetrics container.MetricSet,
 ) (container.ContainerHandler, error) {
 	// Create the cgroup paths.
-	cgroupPaths := common.MakeCgroupPaths(cgroupSubsystems.MountPoints, name)
+	cgroupPaths := common.MakeCgroupPaths(cgroupSubsystems, name)
 
 	// Generate the equivalent cgroup manager for this container.
 	cgroupManager, err := containerlibcontainer.NewCgroupManager(name, cgroupPaths)
diff --git a/container/containerd/handler_test.go b/container/containerd/handler_test.go
@@ -50,7 +50,7 @@ func TestHandler(t *testing.T) {
 		name                 string
 		machineInfoFactory   info.MachineInfoFactory
 		fsInfo               fs.FsInfo
-		cgroupSubsystems     *containerlibcontainer.CgroupSubsystems
+		cgroupSubsystems     containerlibcontainer.CgroupSubsystems
 		inHostNamespace      bool
 		metadataEnvAllowList []string
 		includedMetrics      container.MetricSet
@@ -74,7 +74,7 @@ func TestHandler(t *testing.T) {
 			"/kubepods/pod068e8fa0-9213-11e7-a01f-507b9d4141fa/40af7cdcbe507acad47a5a62025743ad3ddc6ab93b77b21363aa1c1d641047c9",
 			nil,
 			nil,
-			&containerlibcontainer.CgroupSubsystems{},
+			nil,
 			false,
 			nil,
 			nil,
@@ -88,7 +88,7 @@ func TestHandler(t *testing.T) {
 			"/kubepods/pod068e8fa0-9213-11e7-a01f-507b9d4141fa/40af7cdcbe507acad47a5a62025743ad3ddc6ab93b77b21363aa1c1d641047c9",
 			&mockedMachineInfo{},
 			nil,
-			&containerlibcontainer.CgroupSubsystems{},
+			nil,
 			false,
 			nil,
 			nil,
@@ -107,7 +107,7 @@ func TestHandler(t *testing.T) {
 			"/kubepods/pod068e8fa0-9213-11e7-a01f-507b9d4141fa/40af7cdcbe507acad47a5a62025743ad3ddc6ab93b77b21363aa1c1d641047c9",
 			&mockedMachineInfo{},
 			nil,
-			&containerlibcontainer.CgroupSubsystems{},
+			nil,
 			false,
 			[]string{"TEST"},
 			nil,
diff --git a/container/crio/factory.go b/container/crio/factory.go
@@ -76,7 +76,7 @@ func (f *crioFactory) NewContainerHandler(name string, metadataEnvAllowList []st
 		f.fsInfo,
 		f.storageDriver,
 		f.storageDir,
-		&f.cgroupSubsystems,
+		f.cgroupSubsystems,
 		inHostNamespace,
 		metadataEnvAllowList,
 		f.includedMetrics,
diff --git a/container/crio/factory_test.go b/container/crio/factory_test.go
@@ -18,15 +18,13 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
-
-	containerlibcontainer "github.com/google/cadvisor/container/libcontainer"
 )
 
 func TestCanHandleAndAccept(t *testing.T) {
 	as := assert.New(t)
 	f := &crioFactory{
 		client:             nil,
-		cgroupSubsystems:   containerlibcontainer.CgroupSubsystems{},
+		cgroupSubsystems:   nil,
 		fsInfo:             nil,
 		machineInfoFactory: nil,
 		storageDriver:      "",
diff --git a/container/crio/handler.go b/container/crio/handler.go
@@ -84,13 +84,13 @@ func newCrioContainerHandler(
 	fsInfo fs.FsInfo,
 	storageDriver storageDriver,
 	storageDir string,
-	cgroupSubsystems *containerlibcontainer.CgroupSubsystems,
+	cgroupSubsystems containerlibcontainer.CgroupSubsystems,
 	inHostNamespace bool,
 	metadataEnvAllowList []string,
 	includedMetrics container.MetricSet,
 ) (container.ContainerHandler, error) {
 	// Create the cgroup paths.
-	cgroupPaths := common.MakeCgroupPaths(cgroupSubsystems.MountPoints, name)
+	cgroupPaths := common.MakeCgroupPaths(cgroupSubsystems, name)
 
 	// Generate the equivalent cgroup manager for this container.
 	cgroupManager, err := containerlibcontainer.NewCgroupManager(name, cgroupPaths)
diff --git a/container/crio/handler_test.go b/container/crio/handler_test.go
@@ -35,7 +35,7 @@ func TestHandler(t *testing.T) {
 		fsInfo               fs.FsInfo
 		storageDriver        storageDriver
 		storageDir           string
-		cgroupSubsystems     *containerlibcontainer.CgroupSubsystems
+		cgroupSubsystems     containerlibcontainer.CgroupSubsystems
 		inHostNamespace      bool
 		metadataEnvAllowList []string
 		includedMetrics      container.MetricSet
@@ -52,7 +52,7 @@ func TestHandler(t *testing.T) {
 			nil,
 			"",
 			"",
-			&containerlibcontainer.CgroupSubsystems{},
+			nil,
 			false,
 			nil,
 			nil,
@@ -68,7 +68,7 @@ func TestHandler(t *testing.T) {
 			nil,
 			"",
 			"",
-			&containerlibcontainer.CgroupSubsystems{},
+			nil,
 			false,
 			nil,
 			nil,
@@ -88,7 +88,7 @@ func TestHandler(t *testing.T) {
 			nil,
 			"",
 			"",
-			&containerlibcontainer.CgroupSubsystems{},
+			nil,
 			false,
 			nil,
 			nil,
diff --git a/container/docker/factory.go b/container/docker/factory.go
@@ -157,7 +157,7 @@ func (f *dockerFactory) NewContainerHandler(name string, metadataEnvAllowList []
 		f.fsInfo,
 		f.storageDriver,
 		f.storageDir,
-		&f.cgroupSubsystems,
+		f.cgroupSubsystems,
 		inHostNamespace,
 		dockerMetadataEnvAllowList,
 		f.dockerVersion,
diff --git a/container/docker/handler.go b/container/docker/handler.go
@@ -121,7 +121,7 @@ func newDockerContainerHandler(
 	fsInfo fs.FsInfo,
 	storageDriver storageDriver,
 	storageDir string,
-	cgroupSubsystems *containerlibcontainer.CgroupSubsystems,
+	cgroupSubsystems containerlibcontainer.CgroupSubsystems,
 	inHostNamespace bool,
 	metadataEnvAllowList []string,
 	dockerVersion []int,
@@ -131,7 +131,7 @@ func newDockerContainerHandler(
 	zfsWatcher *zfs.ZfsWatcher,
 ) (container.ContainerHandler, error) {
 	// Create the cgroup paths.
-	cgroupPaths := common.MakeCgroupPaths(cgroupSubsystems.MountPoints, name)
+	cgroupPaths := common.MakeCgroupPaths(cgroupSubsystems, name)
 
 	// Generate the equivalent cgroup manager for this container.
 	cgroupManager, err := containerlibcontainer.NewCgroupManager(name, cgroupPaths)
diff --git a/container/libcontainer/helpers.go b/container/libcontainer/helpers.go
@@ -29,18 +29,16 @@ import (
 	"k8s.io/klog/v2"
 )
 
-type CgroupSubsystems struct {
-	// Cgroup subsystem to their mount location.
-	// e.g.: "cpu" -> "/sys/fs/cgroup/cpu"
-	MountPoints map[string]string
-}
+// CgroupSubsystems is a map of cgroup controllers to their mount points.
+// For example, "cpu" -> "/sys/fs/cgroup/cpu".
+type CgroupSubsystems map[string]string
 
 // Get information about the cgroup subsystems those we want
 func GetCgroupSubsystems(includedMetrics container.MetricSet) (CgroupSubsystems, error) {
 	// Get all cgroup mounts.
 	allCgroups, err := cgroups.GetCgroupMounts(true)
 	if err != nil {
-		return CgroupSubsystems{}, err
+		return nil, err
 	}
 
 	disableCgroups := map[string]struct{}{}
@@ -82,7 +80,7 @@ func GetAllCgroupSubsystems() (CgroupSubsystems, error) {
 	// Get all cgroup mounts.
 	allCgroups, err := cgroups.GetCgroupMounts(true)
 	if err != nil {
-		return CgroupSubsystems{}, err
+		return nil, err
 	}
 
 	emptyDisableCgroups := map[string]struct{}{}
@@ -91,7 +89,7 @@ func GetAllCgroupSubsystems() (CgroupSubsystems, error) {
 
 func getCgroupSubsystemsHelper(allCgroups []cgroups.Mount, disableCgroups map[string]struct{}) (CgroupSubsystems, error) {
 	if len(allCgroups) == 0 {
-		return CgroupSubsystems{}, fmt.Errorf("failed to find cgroup mounts")
+		return nil, fmt.Errorf("failed to find cgroup mounts")
 	}
 
 	// Trim the mounts to only the subsystems we care about.
@@ -114,9 +112,7 @@ func getCgroupSubsystemsHelper(allCgroups []cgroups.Mount, disableCgroups map[st
 		}
 	}
 
-	return CgroupSubsystems{
-		MountPoints: mountPoints,
-	}, nil
+	return mountPoints, nil
 }
 
 // Cgroup subsystems we support listing (should be the minimal set we need stats from).
diff --git a/container/libcontainer/helpers_test.go b/container/libcontainer/helpers_test.go
@@ -54,43 +54,37 @@ func TestGetCgroupSubsystems(t *testing.T) {
 		{
 			// 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",
-					"hugetlb": "/sys/fs/cgroup/hugetlb",
-					"pids":    "/sys/fs/cgroup/pids",
-				},
+			expected: 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",
+				"hugetlb": "/sys/fs/cgroup/hugetlb",
+				"pids":    "/sys/fs/cgroup/pids",
 			},
 		},
 		{
 			// 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",
-					"hugetlb": "/sys/fs/cgroup/hugetlb",
-					"pids":    "/sys/fs/cgroup/pids",
-				},
+			expected: 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",
+				"hugetlb": "/sys/fs/cgroup/hugetlb",
+				"pids":    "/sys/fs/cgroup/pids",
 			},
 		},
 		{
 			// most subsystems not mounted
 			mounts: cgroupMountsAt("/sys/fs/cgroup", []string{"cpu"}),
-			expected: CgroupSubsystems{
-				MountPoints: map[string]string{
-					"cpu": "/sys/fs/cgroup/cpu",
-				},
+			expected: map[string]string{
+				"cpu": "/sys/fs/cgroup/cpu",
 			},
 		},
 	}
@@ -106,8 +100,8 @@ func TestGetCgroupSubsystems(t *testing.T) {
 		if err != nil {
 			t.Fatalf("[case %d] Expected no error, but got %v", i, err)
 		}
-		if !reflect.DeepEqual(testCase.expected.MountPoints, subSystems.MountPoints) {
-			t.Fatalf("[case %d] Expected %v == %v", i, testCase.expected.MountPoints, subSystems.MountPoints)
+		if !reflect.DeepEqual(testCase.expected, subSystems) {
+			t.Fatalf("[case %d] Expected %v == %v", i, testCase.expected, subSystems)
 		}
 	}
 }
diff --git a/container/raw/factory.go b/container/raw/factory.go
diff --git a/container/raw/handler.go b/container/raw/handler.go
diff --git a/container/raw/watcher.go b/container/raw/watcher.go
