From dc907e8d11963786c3458e1bbc124d4584aa0025 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 5 Aug 2021 17:25:55 -0700 Subject: [PATCH 1/8] libct/cg/sd/v*.go: nit We were checking if a unit is a slice two times. Consolidate those checks, and improve comments while we're at it. The code is the same in v1 and v2 but it's too complicated to factor it out, thus we just do the same changes in v1.go and v2.go. No functional change. Signed-off-by: Kir Kolyshkin --- libcontainer/cgroups/systemd/v1.go | 12 ++++-------- libcontainer/cgroups/systemd/v2.go | 12 ++++-------- 2 files changed, 8 insertions(+), 16 deletions(-) diff --git a/libcontainer/cgroups/systemd/v1.go b/libcontainer/cgroups/systemd/v1.go index 54266901ac1..c540ced7a60 100644 --- a/libcontainer/cgroups/systemd/v1.go +++ b/libcontainer/cgroups/systemd/v1.go @@ -120,12 +120,14 @@ func (m *legacyManager) Apply(pid int) error { properties = append(properties, systemdDbus.PropDescription("libcontainer container "+c.Name)) - // if we create a slice, the parent is defined via a Wants= if strings.HasSuffix(unitName, ".slice") { + // If we create a slice, the parent is defined via a Wants=. properties = append(properties, systemdDbus.PropWants(slice)) } else { - // otherwise, we use Slice= + // Otherwise it's a scope, which we put into a Slice=. properties = append(properties, systemdDbus.PropSlice(slice)) + // Assume scopes always support delegation (supported since systemd v218). + properties = append(properties, newProp("Delegate", true)) } // only add pid if its valid, -1 is used w/ general slice creation. @@ -133,12 +135,6 @@ func (m *legacyManager) Apply(pid int) error { properties = append(properties, newProp("PIDs", []uint32{uint32(pid)})) } - // Check if we can delegate. This is only supported on systemd versions 218 and above. - if !strings.HasSuffix(unitName, ".slice") { - // Assume scopes always support delegation. - properties = append(properties, newProp("Delegate", true)) - } - // Always enable accounting, this gets us the same behaviour as the fs implementation, // plus the kernel has some problems with joining the memory cgroup at a later time. properties = append(properties, diff --git a/libcontainer/cgroups/systemd/v2.go b/libcontainer/cgroups/systemd/v2.go index 106720d84b2..5af4500ead2 100644 --- a/libcontainer/cgroups/systemd/v2.go +++ b/libcontainer/cgroups/systemd/v2.go @@ -242,12 +242,14 @@ func (m *unifiedManager) Apply(pid int) error { properties = append(properties, systemdDbus.PropDescription("libcontainer container "+c.Name)) - // if we create a slice, the parent is defined via a Wants= if strings.HasSuffix(unitName, ".slice") { + // If we create a slice, the parent is defined via a Wants=. properties = append(properties, systemdDbus.PropWants(slice)) } else { - // otherwise, we use Slice= + // Otherwise it's a scope, which we put into a Slice=. properties = append(properties, systemdDbus.PropSlice(slice)) + // Assume scopes always support delegation (supported since systemd v218). + properties = append(properties, newProp("Delegate", true)) } // only add pid if its valid, -1 is used w/ general slice creation. @@ -255,12 +257,6 @@ func (m *unifiedManager) Apply(pid int) error { properties = append(properties, newProp("PIDs", []uint32{uint32(pid)})) } - // Check if we can delegate. This is only supported on systemd versions 218 and above. - if !strings.HasSuffix(unitName, ".slice") { - // Assume scopes always support delegation. - properties = append(properties, newProp("Delegate", true)) - } - // Always enable accounting, this gets us the same behaviour as the fs implementation, // plus the kernel has some problems with joining the memory cgroup at a later time. properties = append(properties, From c7e0864d5fc3682f68aee8569011c89f66d98773 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 5 Aug 2021 17:09:22 -0700 Subject: [PATCH 2/8] libct/cg/sd/v1: factor out initPaths No functional change. Signed-off-by: Kir Kolyshkin --- libcontainer/cgroups/systemd/v1.go | 49 ++++++++++++++++++------------ 1 file changed, 30 insertions(+), 19 deletions(-) diff --git a/libcontainer/cgroups/systemd/v1.go b/libcontainer/cgroups/systemd/v1.go index c540ced7a60..5be2dd2fea0 100644 --- a/libcontainer/cgroups/systemd/v1.go +++ b/libcontainer/cgroups/systemd/v1.go @@ -99,6 +99,34 @@ func genV1ResourcesProperties(r *configs.Resources, cm *dbusConnManager) ([]syst return properties, nil } +// initPaths initializes m.paths. Supposed to be called under m.mu held. +func (m *legacyManager) initPaths() error { + if m.paths != nil { + return nil + } + + paths := make(map[string]string) + for _, s := range legacySubsystems { + subsystemPath, err := getSubsystemPath(m.cgroups, s.Name()) + if err != nil { + // Even if it's `not found` error, we'll return err + // because devices cgroup is hard requirement for + // container security. + if s.Name() == "devices" { + return err + } + // Don't fail if a cgroup hierarchy was not found, just skip this subsystem + if cgroups.IsNotFound(err) { + continue + } + return err + } + paths[s.Name()] = subsystemPath + } + m.paths = paths + return nil +} + func (m *legacyManager) Apply(pid int) error { var ( c = m.cgroups @@ -154,26 +182,9 @@ func (m *legacyManager) Apply(pid int) error { return err } - paths := make(map[string]string) - for _, s := range legacySubsystems { - subsystemPath, err := getSubsystemPath(m.cgroups, s.Name()) - if err != nil { - // Even if it's `not found` error, we'll return err - // because devices cgroup is hard requirement for - // container security. - if s.Name() == "devices" { - return err - } - // Don't fail if a cgroup hierarchy was not found, just skip this subsystem - if cgroups.IsNotFound(err) { - continue - } - return err - } - paths[s.Name()] = subsystemPath + if err := m.initPaths(); err != nil { + return err } - m.paths = paths - if err := m.joinCgroups(pid); err != nil { return err } From 63c84917f392598352c8e9002dbda4dbccf5cce7 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 5 Aug 2021 18:00:57 -0700 Subject: [PATCH 3/8] libct/cg/sd/v1: optimize initPaths It does not make sense to calculate slice and unit 10+ times. Move those out of the loop. Signed-off-by: Kir Kolyshkin --- libcontainer/cgroups/systemd/v1.go | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/libcontainer/cgroups/systemd/v1.go b/libcontainer/cgroups/systemd/v1.go index 5be2dd2fea0..a871b3b44ec 100644 --- a/libcontainer/cgroups/systemd/v1.go +++ b/libcontainer/cgroups/systemd/v1.go @@ -105,9 +105,23 @@ func (m *legacyManager) initPaths() error { return nil } + c := m.cgroups + + slice := "system.slice" + if c.Parent != "" { + slice = c.Parent + } + + var err error + slice, err = ExpandSlice(slice) + if err != nil { + return err + } + unit := getUnitName(c) + paths := make(map[string]string) for _, s := range legacySubsystems { - subsystemPath, err := getSubsystemPath(m.cgroups, s.Name()) + subsystemPath, err := getSubsystemPath(slice, unit, s.Name()) if err != nil { // Even if it's `not found` error, we'll return err // because devices cgroup is hard requirement for @@ -242,7 +256,7 @@ func (m *legacyManager) joinCgroups(pid int) error { return nil } -func getSubsystemPath(c *configs.Cgroup, subsystem string) (string, error) { +func getSubsystemPath(slice, unit, subsystem string) (string, error) { mountpoint, err := cgroups.FindCgroupMountpoint("", subsystem) if err != nil { return "", err @@ -255,17 +269,7 @@ func getSubsystemPath(c *configs.Cgroup, subsystem string) (string, error) { // if pid 1 is systemd 226 or later, it will be in init.scope, not the root initPath = strings.TrimSuffix(filepath.Clean(initPath), "init.scope") - slice := "system.slice" - if c.Parent != "" { - slice = c.Parent - } - - slice, err = ExpandSlice(slice) - if err != nil { - return "", err - } - - return filepath.Join(mountpoint, initPath, slice, getUnitName(c)), nil + return filepath.Join(mountpoint, initPath, slice, unit), nil } func (m *legacyManager) Freeze(state configs.FreezerState) error { From eb09df749abf625cecbd8c83dcdba6b9ddd95fe7 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 5 Aug 2021 18:08:01 -0700 Subject: [PATCH 4/8] libct/cg/sd/v1: initPaths: minor optimization As ExpandSlice("system.slice") returns "/system.slice", there is no need to call it for such paths (and the slash will be added by path.Join anyway). The same optimization was already done for v2 as part of commit bf15cc99b1e7b486dfb. Signed-off-by: Kir Kolyshkin --- libcontainer/cgroups/systemd/v1.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/libcontainer/cgroups/systemd/v1.go b/libcontainer/cgroups/systemd/v1.go index a871b3b44ec..470453f3316 100644 --- a/libcontainer/cgroups/systemd/v1.go +++ b/libcontainer/cgroups/systemd/v1.go @@ -109,14 +109,13 @@ func (m *legacyManager) initPaths() error { slice := "system.slice" if c.Parent != "" { - slice = c.Parent + var err error + slice, err = ExpandSlice(c.Parent) + if err != nil { + return err + } } - var err error - slice, err = ExpandSlice(slice) - if err != nil { - return err - } unit := getUnitName(c) paths := make(map[string]string) From 19b542a576a2db3d66a9b384e4822cc506bd52fc Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 10 Aug 2021 12:57:43 -0700 Subject: [PATCH 5/8] libct/cg/fs: move internal code out of fs.go Now fs.go is not very readable as its public API functions are intermixed with internal stuff about getting cgroup paths. Move that out to paths.go, without changing any code. Same for the tests -- move paths-related tests to paths_test.go. This commit is separate to make the review easier. Signed-off-by: Kir Kolyshkin --- libcontainer/cgroups/fs/fs.go | 163 ------------------------ libcontainer/cgroups/fs/fs_test.go | 96 -------------- libcontainer/cgroups/fs/paths.go | 175 ++++++++++++++++++++++++++ libcontainer/cgroups/fs/paths_test.go | 104 +++++++++++++++ 4 files changed, 279 insertions(+), 259 deletions(-) create mode 100644 libcontainer/cgroups/fs/paths.go create mode 100644 libcontainer/cgroups/fs/paths_test.go diff --git a/libcontainer/cgroups/fs/fs.go b/libcontainer/cgroups/fs/fs.go index 1c1e3f3e63e..391c9532bc7 100644 --- a/libcontainer/cgroups/fs/fs.go +++ b/libcontainer/cgroups/fs/fs.go @@ -4,7 +4,6 @@ import ( "errors" "fmt" "os" - "path/filepath" "sync" "golang.org/x/sys/unix" @@ -12,7 +11,6 @@ import ( "github.com/opencontainers/runc/libcontainer/cgroups" "github.com/opencontainers/runc/libcontainer/cgroups/fscommon" "github.com/opencontainers/runc/libcontainer/configs" - libcontainerUtils "github.com/opencontainers/runc/libcontainer/utils" ) var ( @@ -63,105 +61,6 @@ func NewManager(cg *configs.Cgroup, paths map[string]string, rootless bool) cgro } } -// The absolute path to the root of the cgroup hierarchies. -var ( - cgroupRootLock sync.Mutex - cgroupRoot string -) - -const defaultCgroupRoot = "/sys/fs/cgroup" - -func tryDefaultCgroupRoot() string { - var st, pst unix.Stat_t - - // (1) it should be a directory... - err := unix.Lstat(defaultCgroupRoot, &st) - if err != nil || st.Mode&unix.S_IFDIR == 0 { - return "" - } - - // (2) ... and a mount point ... - err = unix.Lstat(filepath.Dir(defaultCgroupRoot), &pst) - if err != nil { - return "" - } - - if st.Dev == pst.Dev { - // parent dir has the same dev -- not a mount point - return "" - } - - // (3) ... of 'tmpfs' fs type. - var fst unix.Statfs_t - err = unix.Statfs(defaultCgroupRoot, &fst) - if err != nil || fst.Type != unix.TMPFS_MAGIC { - return "" - } - - // (4) it should have at least 1 entry ... - dir, err := os.Open(defaultCgroupRoot) - if err != nil { - return "" - } - names, err := dir.Readdirnames(1) - if err != nil { - return "" - } - if len(names) < 1 { - return "" - } - // ... which is a cgroup mount point. - err = unix.Statfs(filepath.Join(defaultCgroupRoot, names[0]), &fst) - if err != nil || fst.Type != unix.CGROUP_SUPER_MAGIC { - return "" - } - - return defaultCgroupRoot -} - -// Gets the cgroupRoot. -func getCgroupRoot() (string, error) { - cgroupRootLock.Lock() - defer cgroupRootLock.Unlock() - - if cgroupRoot != "" { - return cgroupRoot, nil - } - - // fast path - cgroupRoot = tryDefaultCgroupRoot() - if cgroupRoot != "" { - return cgroupRoot, nil - } - - // slow path: parse mountinfo - mi, err := cgroups.GetCgroupMounts(false) - if err != nil { - return "", err - } - if len(mi) < 1 { - return "", errors.New("no cgroup mount found in mountinfo") - } - - // Get the first cgroup mount (e.g. "/sys/fs/cgroup/memory"), - // use its parent directory. - root := filepath.Dir(mi[0].Mountpoint) - - if _, err := os.Stat(root); err != nil { - return "", err - } - - cgroupRoot = root - return cgroupRoot, nil -} - -type cgroupData struct { - root string - innerPath string - config *configs.Cgroup - pid int -} - // isIgnorableError returns whether err is a permission error (in the loose // sense of the word). This includes EROFS (which for an unprivileged user is // basically a permission error) and EACCES (for similar reasons) as well as @@ -315,68 +214,6 @@ func (m *manager) GetAllPids() ([]int, error) { return cgroups.GetAllPids(m.Path("devices")) } -func getCgroupData(c *configs.Cgroup, pid int) (*cgroupData, error) { - root, err := getCgroupRoot() - if err != nil { - return nil, err - } - - if (c.Name != "" || c.Parent != "") && c.Path != "" { - return nil, errors.New("cgroup: either Path or Name and Parent should be used") - } - - // XXX: Do not remove this code. Path safety is important! -- cyphar - cgPath := libcontainerUtils.CleanPath(c.Path) - cgParent := libcontainerUtils.CleanPath(c.Parent) - cgName := libcontainerUtils.CleanPath(c.Name) - - innerPath := cgPath - if innerPath == "" { - innerPath = filepath.Join(cgParent, cgName) - } - - return &cgroupData{ - root: root, - innerPath: innerPath, - config: c, - pid: pid, - }, nil -} - -func (raw *cgroupData) path(subsystem string) (string, error) { - // If the cgroup name/path is absolute do not look relative to the cgroup of the init process. - if filepath.IsAbs(raw.innerPath) { - mnt, err := cgroups.FindCgroupMountpoint(raw.root, subsystem) - // If we didn't mount the subsystem, there is no point we make the path. - if err != nil { - return "", err - } - - // Sometimes subsystems can be mounted together as 'cpu,cpuacct'. - return filepath.Join(raw.root, filepath.Base(mnt), raw.innerPath), nil - } - - // Use GetOwnCgroupPath instead of GetInitCgroupPath, because the creating - // process could in container and shared pid namespace with host, and - // /proc/1/cgroup could point to whole other world of cgroups. - parentPath, err := cgroups.GetOwnCgroupPath(subsystem) - if err != nil { - return "", err - } - - return filepath.Join(parentPath, raw.innerPath), nil -} - -func join(path string, pid int) error { - if path == "" { - return nil - } - if err := os.MkdirAll(path, 0o755); err != nil { - return err - } - return cgroups.WriteCgroupProc(path, pid) -} - func (m *manager) GetPaths() map[string]string { m.mu.Lock() defer m.mu.Unlock() diff --git a/libcontainer/cgroups/fs/fs_test.go b/libcontainer/cgroups/fs/fs_test.go index b4430603f1e..7a91768602b 100644 --- a/libcontainer/cgroups/fs/fs_test.go +++ b/libcontainer/cgroups/fs/fs_test.go @@ -1,108 +1,12 @@ package fs import ( - "path/filepath" - "strings" "testing" "github.com/opencontainers/runc/libcontainer/cgroups" "github.com/opencontainers/runc/libcontainer/configs" ) -func TestInvalidCgroupPath(t *testing.T) { - if cgroups.IsCgroup2UnifiedMode() { - t.Skip("cgroup v2 is not supported") - } - - root, err := getCgroupRoot() - if err != nil { - t.Fatalf("couldn't get cgroup root: %v", err) - } - - testCases := []struct { - test string - path, name, parent string - }{ - { - test: "invalid cgroup path", - path: "../../../../../../../../../../some/path", - }, - { - test: "invalid absolute cgroup path", - path: "/../../../../../../../../../../some/path", - }, - { - test: "invalid cgroup parent", - parent: "../../../../../../../../../../some/path", - name: "name", - }, - { - test: "invalid absolute cgroup parent", - parent: "/../../../../../../../../../../some/path", - name: "name", - }, - { - test: "invalid cgroup name", - parent: "parent", - name: "../../../../../../../../../../some/path", - }, - { - test: "invalid absolute cgroup name", - parent: "parent", - name: "/../../../../../../../../../../some/path", - }, - { - test: "invalid cgroup name and parent", - parent: "../../../../../../../../../../some/path", - name: "../../../../../../../../../../some/path", - }, - { - test: "invalid absolute cgroup name and parent", - parent: "/../../../../../../../../../../some/path", - name: "/../../../../../../../../../../some/path", - }, - } - - for _, tc := range testCases { - t.Run(tc.test, func(t *testing.T) { - config := &configs.Cgroup{Path: tc.path, Name: tc.name, Parent: tc.parent} - - data, err := getCgroupData(config, 0) - if err != nil { - t.Fatalf("couldn't get cgroup data: %v", err) - } - - // Make sure the final innerPath doesn't go outside the cgroup mountpoint. - if strings.HasPrefix(data.innerPath, "..") { - t.Errorf("SECURITY: cgroup innerPath is outside cgroup mountpoint!") - } - - // Double-check, using an actual cgroup. - deviceRoot := filepath.Join(root, "devices") - devicePath, err := data.path("devices") - if err != nil { - t.Fatalf("couldn't get cgroup path: %v", err) - } - if !strings.HasPrefix(devicePath, deviceRoot) { - t.Errorf("SECURITY: cgroup path() is outside cgroup mountpoint!") - } - }) - } -} - -func TestTryDefaultCgroupRoot(t *testing.T) { - res := tryDefaultCgroupRoot() - exp := defaultCgroupRoot - if cgroups.IsCgroup2UnifiedMode() { - // checking that tryDefaultCgroupRoot does return "" - // in case /sys/fs/cgroup is not cgroup v1 root dir. - exp = "" - } - if res != exp { - t.Errorf("tryDefaultCgroupRoot: want %q, got %q", exp, res) - } -} - func BenchmarkGetStats(b *testing.B) { if cgroups.IsCgroup2UnifiedMode() { b.Skip("cgroup v2 is not supported") diff --git a/libcontainer/cgroups/fs/paths.go b/libcontainer/cgroups/fs/paths.go new file mode 100644 index 00000000000..befa81bcb07 --- /dev/null +++ b/libcontainer/cgroups/fs/paths.go @@ -0,0 +1,175 @@ +package fs + +import ( + "errors" + "os" + "path/filepath" + "sync" + + "golang.org/x/sys/unix" + + "github.com/opencontainers/runc/libcontainer/cgroups" + "github.com/opencontainers/runc/libcontainer/configs" + "github.com/opencontainers/runc/libcontainer/utils" +) + +// The absolute path to the root of the cgroup hierarchies. +var ( + cgroupRootLock sync.Mutex + cgroupRoot string +) + +const defaultCgroupRoot = "/sys/fs/cgroup" + +func tryDefaultCgroupRoot() string { + var st, pst unix.Stat_t + + // (1) it should be a directory... + err := unix.Lstat(defaultCgroupRoot, &st) + if err != nil || st.Mode&unix.S_IFDIR == 0 { + return "" + } + + // (2) ... and a mount point ... + err = unix.Lstat(filepath.Dir(defaultCgroupRoot), &pst) + if err != nil { + return "" + } + + if st.Dev == pst.Dev { + // parent dir has the same dev -- not a mount point + return "" + } + + // (3) ... of 'tmpfs' fs type. + var fst unix.Statfs_t + err = unix.Statfs(defaultCgroupRoot, &fst) + if err != nil || fst.Type != unix.TMPFS_MAGIC { + return "" + } + + // (4) it should have at least 1 entry ... + dir, err := os.Open(defaultCgroupRoot) + if err != nil { + return "" + } + names, err := dir.Readdirnames(1) + if err != nil { + return "" + } + if len(names) < 1 { + return "" + } + // ... which is a cgroup mount point. + err = unix.Statfs(filepath.Join(defaultCgroupRoot, names[0]), &fst) + if err != nil || fst.Type != unix.CGROUP_SUPER_MAGIC { + return "" + } + + return defaultCgroupRoot +} + +// Gets the cgroupRoot. +func getCgroupRoot() (string, error) { + cgroupRootLock.Lock() + defer cgroupRootLock.Unlock() + + if cgroupRoot != "" { + return cgroupRoot, nil + } + + // fast path + cgroupRoot = tryDefaultCgroupRoot() + if cgroupRoot != "" { + return cgroupRoot, nil + } + + // slow path: parse mountinfo + mi, err := cgroups.GetCgroupMounts(false) + if err != nil { + return "", err + } + if len(mi) < 1 { + return "", errors.New("no cgroup mount found in mountinfo") + } + + // Get the first cgroup mount (e.g. "/sys/fs/cgroup/memory"), + // use its parent directory. + root := filepath.Dir(mi[0].Mountpoint) + + if _, err := os.Stat(root); err != nil { + return "", err + } + + cgroupRoot = root + return cgroupRoot, nil +} + +type cgroupData struct { + root string + innerPath string + config *configs.Cgroup + pid int +} + +func getCgroupData(c *configs.Cgroup, pid int) (*cgroupData, error) { + root, err := getCgroupRoot() + if err != nil { + return nil, err + } + + if (c.Name != "" || c.Parent != "") && c.Path != "" { + return nil, errors.New("cgroup: either Path or Name and Parent should be used") + } + + // XXX: Do not remove this code. Path safety is important! -- cyphar + cgPath := utils.CleanPath(c.Path) + cgParent := utils.CleanPath(c.Parent) + cgName := utils.CleanPath(c.Name) + + innerPath := cgPath + if innerPath == "" { + innerPath = filepath.Join(cgParent, cgName) + } + + return &cgroupData{ + root: root, + innerPath: innerPath, + config: c, + pid: pid, + }, nil +} + +func (raw *cgroupData) path(subsystem string) (string, error) { + // If the cgroup name/path is absolute do not look relative to the cgroup of the init process. + if filepath.IsAbs(raw.innerPath) { + mnt, err := cgroups.FindCgroupMountpoint(raw.root, subsystem) + // If we didn't mount the subsystem, there is no point we make the path. + if err != nil { + return "", err + } + + // Sometimes subsystems can be mounted together as 'cpu,cpuacct'. + return filepath.Join(raw.root, filepath.Base(mnt), raw.innerPath), nil + } + + // Use GetOwnCgroupPath instead of GetInitCgroupPath, because the creating + // process could in container and shared pid namespace with host, and + // /proc/1/cgroup could point to whole other world of cgroups. + parentPath, err := cgroups.GetOwnCgroupPath(subsystem) + if err != nil { + return "", err + } + + return filepath.Join(parentPath, raw.innerPath), nil +} + +func join(path string, pid int) error { + if path == "" { + return nil + } + if err := os.MkdirAll(path, 0o755); err != nil { + return err + } + return cgroups.WriteCgroupProc(path, pid) +} diff --git a/libcontainer/cgroups/fs/paths_test.go b/libcontainer/cgroups/fs/paths_test.go new file mode 100644 index 00000000000..58ebfc7c410 --- /dev/null +++ b/libcontainer/cgroups/fs/paths_test.go @@ -0,0 +1,104 @@ +package fs + +import ( + "path/filepath" + "strings" + "testing" + + "github.com/opencontainers/runc/libcontainer/cgroups" + "github.com/opencontainers/runc/libcontainer/configs" +) + +func TestInvalidCgroupPath(t *testing.T) { + if cgroups.IsCgroup2UnifiedMode() { + t.Skip("cgroup v2 is not supported") + } + + root, err := getCgroupRoot() + if err != nil { + t.Fatalf("couldn't get cgroup root: %v", err) + } + + testCases := []struct { + test string + path, name, parent string + }{ + { + test: "invalid cgroup path", + path: "../../../../../../../../../../some/path", + }, + { + test: "invalid absolute cgroup path", + path: "/../../../../../../../../../../some/path", + }, + { + test: "invalid cgroup parent", + parent: "../../../../../../../../../../some/path", + name: "name", + }, + { + test: "invalid absolute cgroup parent", + parent: "/../../../../../../../../../../some/path", + name: "name", + }, + { + test: "invalid cgroup name", + parent: "parent", + name: "../../../../../../../../../../some/path", + }, + { + test: "invalid absolute cgroup name", + parent: "parent", + name: "/../../../../../../../../../../some/path", + }, + { + test: "invalid cgroup name and parent", + parent: "../../../../../../../../../../some/path", + name: "../../../../../../../../../../some/path", + }, + { + test: "invalid absolute cgroup name and parent", + parent: "/../../../../../../../../../../some/path", + name: "/../../../../../../../../../../some/path", + }, + } + + for _, tc := range testCases { + t.Run(tc.test, func(t *testing.T) { + config := &configs.Cgroup{Path: tc.path, Name: tc.name, Parent: tc.parent} + + data, err := getCgroupData(config, 0) + if err != nil { + t.Fatalf("couldn't get cgroup data: %v", err) + } + + // Make sure the final innerPath doesn't go outside the cgroup mountpoint. + if strings.HasPrefix(data.innerPath, "..") { + t.Errorf("SECURITY: cgroup innerPath is outside cgroup mountpoint!") + } + + // Double-check, using an actual cgroup. + deviceRoot := filepath.Join(root, "devices") + devicePath, err := data.path("devices") + if err != nil { + t.Fatalf("couldn't get cgroup path: %v", err) + } + if !strings.HasPrefix(devicePath, deviceRoot) { + t.Errorf("SECURITY: cgroup path() is outside cgroup mountpoint!") + } + }) + } +} + +func TestTryDefaultCgroupRoot(t *testing.T) { + res := tryDefaultCgroupRoot() + exp := defaultCgroupRoot + if cgroups.IsCgroup2UnifiedMode() { + // checking that tryDefaultCgroupRoot does return "" + // in case /sys/fs/cgroup is not cgroup v1 root dir. + exp = "" + } + if res != exp { + t.Errorf("tryDefaultCgroupRoot: want %q, got %q", exp, res) + } +} From 5c7cb837c77ed98b74751fbcb6f1289e1dfd13e8 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 10 Aug 2021 13:06:31 -0700 Subject: [PATCH 6/8] libct/cg/fs: micro optimization In case c.Path is set, c.Name and c.Parent are not used, and so calls to utils.CleanPath are entirely unnecessary. Move them to inside of the "if" statement body. Get rid of the intermediate cgPath variable, it is not needed. Signed-off-by: Kir Kolyshkin --- libcontainer/cgroups/fs/paths.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/libcontainer/cgroups/fs/paths.go b/libcontainer/cgroups/fs/paths.go index befa81bcb07..671353ac861 100644 --- a/libcontainer/cgroups/fs/paths.go +++ b/libcontainer/cgroups/fs/paths.go @@ -122,13 +122,11 @@ func getCgroupData(c *configs.Cgroup, pid int) (*cgroupData, error) { return nil, errors.New("cgroup: either Path or Name and Parent should be used") } - // XXX: Do not remove this code. Path safety is important! -- cyphar - cgPath := utils.CleanPath(c.Path) - cgParent := utils.CleanPath(c.Parent) - cgName := utils.CleanPath(c.Name) - - innerPath := cgPath + // XXX: Do not remove CleanPath. Path safety is important! -- cyphar + innerPath := utils.CleanPath(c.Path) if innerPath == "" { + cgParent := utils.CleanPath(c.Parent) + cgName := utils.CleanPath(c.Name) innerPath = filepath.Join(cgParent, cgName) } From 7d1cb320adf6f1c8994d1688068ce54f5cf000c3 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 10 Aug 2021 13:10:19 -0700 Subject: [PATCH 7/8] libct/cg/fs: rename join to apply As this is called from the Apply() method, it's a natural name. Signed-off-by: Kir Kolyshkin --- libcontainer/cgroups/fs/blkio.go | 2 +- libcontainer/cgroups/fs/cpuacct.go | 2 +- libcontainer/cgroups/fs/devices.go | 2 +- libcontainer/cgroups/fs/freezer.go | 2 +- libcontainer/cgroups/fs/hugetlb.go | 2 +- libcontainer/cgroups/fs/memory.go | 2 +- libcontainer/cgroups/fs/name.go | 2 +- libcontainer/cgroups/fs/net_cls.go | 2 +- libcontainer/cgroups/fs/net_prio.go | 2 +- libcontainer/cgroups/fs/paths.go | 2 +- libcontainer/cgroups/fs/perf_event.go | 2 +- libcontainer/cgroups/fs/pids.go | 2 +- libcontainer/cgroups/fs/rdma.go | 2 +- 13 files changed, 13 insertions(+), 13 deletions(-) diff --git a/libcontainer/cgroups/fs/blkio.go b/libcontainer/cgroups/fs/blkio.go index 2d764001ead..14b572cfe72 100644 --- a/libcontainer/cgroups/fs/blkio.go +++ b/libcontainer/cgroups/fs/blkio.go @@ -21,7 +21,7 @@ func (s *BlkioGroup) Name() string { } func (s *BlkioGroup) Apply(path string, d *cgroupData) error { - return join(path, d.pid) + return apply(path, d.pid) } func (s *BlkioGroup) Set(path string, r *configs.Resources) error { diff --git a/libcontainer/cgroups/fs/cpuacct.go b/libcontainer/cgroups/fs/cpuacct.go index 076704ab742..537bada66a1 100644 --- a/libcontainer/cgroups/fs/cpuacct.go +++ b/libcontainer/cgroups/fs/cpuacct.go @@ -35,7 +35,7 @@ func (s *CpuacctGroup) Name() string { } func (s *CpuacctGroup) Apply(path string, d *cgroupData) error { - return join(path, d.pid) + return apply(path, d.pid) } func (s *CpuacctGroup) Set(_ string, _ *configs.Resources) error { diff --git a/libcontainer/cgroups/fs/devices.go b/libcontainer/cgroups/fs/devices.go index 2c8917049ea..20a11e8c670 100644 --- a/libcontainer/cgroups/fs/devices.go +++ b/libcontainer/cgroups/fs/devices.go @@ -29,7 +29,7 @@ func (s *DevicesGroup) Apply(path string, d *cgroupData) error { // is a hard requirement for container's security. return errSubsystemDoesNotExist } - return join(path, d.pid) + return apply(path, d.pid) } func loadEmulator(path string) (*cgroupdevices.Emulator, error) { diff --git a/libcontainer/cgroups/fs/freezer.go b/libcontainer/cgroups/fs/freezer.go index cf1c5efb0a5..1dc4dd9fc2f 100644 --- a/libcontainer/cgroups/fs/freezer.go +++ b/libcontainer/cgroups/fs/freezer.go @@ -20,7 +20,7 @@ func (s *FreezerGroup) Name() string { } func (s *FreezerGroup) Apply(path string, d *cgroupData) error { - return join(path, d.pid) + return apply(path, d.pid) } func (s *FreezerGroup) Set(path string, r *configs.Resources) (Err error) { diff --git a/libcontainer/cgroups/fs/hugetlb.go b/libcontainer/cgroups/fs/hugetlb.go index 8477fa6b356..f95e3be25d1 100644 --- a/libcontainer/cgroups/fs/hugetlb.go +++ b/libcontainer/cgroups/fs/hugetlb.go @@ -15,7 +15,7 @@ func (s *HugetlbGroup) Name() string { } func (s *HugetlbGroup) Apply(path string, d *cgroupData) error { - return join(path, d.pid) + return apply(path, d.pid) } func (s *HugetlbGroup) Set(path string, r *configs.Resources) error { diff --git a/libcontainer/cgroups/fs/memory.go b/libcontainer/cgroups/fs/memory.go index 8e10c22b2e7..a349ad4b636 100644 --- a/libcontainer/cgroups/fs/memory.go +++ b/libcontainer/cgroups/fs/memory.go @@ -31,7 +31,7 @@ func (s *MemoryGroup) Name() string { } func (s *MemoryGroup) Apply(path string, d *cgroupData) (err error) { - return join(path, d.pid) + return apply(path, d.pid) } func setMemory(path string, val int64) error { diff --git a/libcontainer/cgroups/fs/name.go b/libcontainer/cgroups/fs/name.go index ce9f4a0a5ff..056bf825d9e 100644 --- a/libcontainer/cgroups/fs/name.go +++ b/libcontainer/cgroups/fs/name.go @@ -17,7 +17,7 @@ func (s *NameGroup) Name() string { func (s *NameGroup) Apply(path string, d *cgroupData) error { if s.Join { // ignore errors if the named cgroup does not exist - _ = join(path, d.pid) + _ = apply(path, d.pid) } return nil } diff --git a/libcontainer/cgroups/fs/net_cls.go b/libcontainer/cgroups/fs/net_cls.go index d1364292697..ef9514fb86d 100644 --- a/libcontainer/cgroups/fs/net_cls.go +++ b/libcontainer/cgroups/fs/net_cls.go @@ -14,7 +14,7 @@ func (s *NetClsGroup) Name() string { } func (s *NetClsGroup) Apply(path string, d *cgroupData) error { - return join(path, d.pid) + return apply(path, d.pid) } func (s *NetClsGroup) Set(path string, r *configs.Resources) error { diff --git a/libcontainer/cgroups/fs/net_prio.go b/libcontainer/cgroups/fs/net_prio.go index cdc810baea6..38c26989a66 100644 --- a/libcontainer/cgroups/fs/net_prio.go +++ b/libcontainer/cgroups/fs/net_prio.go @@ -12,7 +12,7 @@ func (s *NetPrioGroup) Name() string { } func (s *NetPrioGroup) Apply(path string, d *cgroupData) error { - return join(path, d.pid) + return apply(path, d.pid) } func (s *NetPrioGroup) Set(path string, r *configs.Resources) error { diff --git a/libcontainer/cgroups/fs/paths.go b/libcontainer/cgroups/fs/paths.go index 671353ac861..9556f8b4c09 100644 --- a/libcontainer/cgroups/fs/paths.go +++ b/libcontainer/cgroups/fs/paths.go @@ -162,7 +162,7 @@ func (raw *cgroupData) path(subsystem string) (string, error) { return filepath.Join(parentPath, raw.innerPath), nil } -func join(path string, pid int) error { +func apply(path string, pid int) error { if path == "" { return nil } diff --git a/libcontainer/cgroups/fs/perf_event.go b/libcontainer/cgroups/fs/perf_event.go index 8ffa83e094e..486c7fe45be 100644 --- a/libcontainer/cgroups/fs/perf_event.go +++ b/libcontainer/cgroups/fs/perf_event.go @@ -12,7 +12,7 @@ func (s *PerfEventGroup) Name() string { } func (s *PerfEventGroup) Apply(path string, d *cgroupData) error { - return join(path, d.pid) + return apply(path, d.pid) } func (s *PerfEventGroup) Set(_ string, _ *configs.Resources) error { diff --git a/libcontainer/cgroups/fs/pids.go b/libcontainer/cgroups/fs/pids.go index 55bb91fa6e8..33bb64c9b83 100644 --- a/libcontainer/cgroups/fs/pids.go +++ b/libcontainer/cgroups/fs/pids.go @@ -16,7 +16,7 @@ func (s *PidsGroup) Name() string { } func (s *PidsGroup) Apply(path string, d *cgroupData) error { - return join(path, d.pid) + return apply(path, d.pid) } func (s *PidsGroup) Set(path string, r *configs.Resources) error { diff --git a/libcontainer/cgroups/fs/rdma.go b/libcontainer/cgroups/fs/rdma.go index 5fb25abe766..3548db3b6e0 100644 --- a/libcontainer/cgroups/fs/rdma.go +++ b/libcontainer/cgroups/fs/rdma.go @@ -13,7 +13,7 @@ func (s *RdmaGroup) Name() string { } func (s *RdmaGroup) Apply(path string, d *cgroupData) error { - return join(path, d.pid) + return apply(path, d.pid) } func (s *RdmaGroup) Set(path string, r *configs.Resources) error { From e6928865631319907e61bf9bfee02ebae210adc9 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 10 Aug 2021 13:42:09 -0700 Subject: [PATCH 8/8] libct/cg/fs: refactor 1. Dismantle and remove struct cgroupData. It contained three unrelated entities (cgroup paths, pid, and resources), and made the code harder to read. Most importantly, though, it is not needed. Now, subsystems' Apply methods take path, resources, and pid. To a reviewer -- the core of the changes is in fs.go and paths.go, the rest of it is adapting to the new signatures and related test changes. 2. Dismantle and remove struct cgroupTestUtil. This is a followup to the previous item -- since cgroupData is gone, there is nothing to hold in cgroupTestUtil. The change itself is very small (see util_test.go), but this patch is big because of it -- mostly because we had to replace helper.cgroup.Resources with &config.Resources{}. Signed-off-by: Kir Kolyshkin --- libcontainer/cgroups/fs/blkio.go | 4 +- libcontainer/cgroups/fs/blkio_test.go | 148 ++++++++++++---------- libcontainer/cgroups/fs/cpu.go | 15 +-- libcontainer/cgroups/fs/cpu_test.go | 72 ++++++----- libcontainer/cgroups/fs/cpuacct.go | 4 +- libcontainer/cgroups/fs/cpuacct_test.go | 12 +- libcontainer/cgroups/fs/cpuset.go | 9 +- libcontainer/cgroups/fs/cpuset_test.go | 41 +++--- libcontainer/cgroups/fs/devices.go | 7 +- libcontainer/cgroups/fs/devices_test.go | 27 ++-- libcontainer/cgroups/fs/freezer.go | 4 +- libcontainer/cgroups/fs/freezer_test.go | 24 ++-- libcontainer/cgroups/fs/fs.go | 16 ++- libcontainer/cgroups/fs/hugetlb.go | 4 +- libcontainer/cgroups/fs/hugetlb_test.go | 41 +++--- libcontainer/cgroups/fs/memory.go | 4 +- libcontainer/cgroups/fs/memory_test.go | 152 ++++++++++++----------- libcontainer/cgroups/fs/name.go | 6 +- libcontainer/cgroups/fs/net_cls.go | 4 +- libcontainer/cgroups/fs/net_cls_test.go | 13 +- libcontainer/cgroups/fs/net_prio.go | 4 +- libcontainer/cgroups/fs/net_prio_test.go | 10 +- libcontainer/cgroups/fs/paths.go | 37 ++---- libcontainer/cgroups/fs/paths_test.go | 10 +- libcontainer/cgroups/fs/perf_event.go | 4 +- libcontainer/cgroups/fs/pids.go | 4 +- libcontainer/cgroups/fs/pids_test.go | 37 +++--- libcontainer/cgroups/fs/rdma.go | 4 +- libcontainer/cgroups/fs/util_test.go | 36 ++---- 29 files changed, 386 insertions(+), 367 deletions(-) diff --git a/libcontainer/cgroups/fs/blkio.go b/libcontainer/cgroups/fs/blkio.go index 14b572cfe72..c81b6562ac5 100644 --- a/libcontainer/cgroups/fs/blkio.go +++ b/libcontainer/cgroups/fs/blkio.go @@ -20,8 +20,8 @@ func (s *BlkioGroup) Name() string { return "blkio" } -func (s *BlkioGroup) Apply(path string, d *cgroupData) error { - return apply(path, d.pid) +func (s *BlkioGroup) Apply(path string, _ *configs.Resources, pid int) error { + return apply(path, pid) } func (s *BlkioGroup) Set(path string, r *configs.Resources) error { diff --git a/libcontainer/cgroups/fs/blkio_test.go b/libcontainer/cgroups/fs/blkio_test.go index eb6257c0d41..7810a67fc31 100644 --- a/libcontainer/cgroups/fs/blkio_test.go +++ b/libcontainer/cgroups/fs/blkio_test.go @@ -176,25 +176,27 @@ func TestBlkioSetWeight(t *testing.T) { for _, legacyIOScheduler := range []bool{false, true} { // Populate cgroup - helper := NewCgroupTestUtil("blkio", t) + path := tempDir(t, "blkio") weightFilename := "blkio.bfq.weight" if legacyIOScheduler { weightFilename = "blkio.weight" } - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ weightFilename: strconv.Itoa(weightBefore), }) // Apply new configuration - helper.CgroupData.config.Resources.BlkioWeight = weightAfter + r := &configs.Resources{ + BlkioWeight: weightAfter, + } blkio := &BlkioGroup{} - if err := blkio.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + if err := blkio.Set(path, r); err != nil { t.Fatal(err) } // Verify results if weightFilename != blkio.weightFilename { t.Fatalf("weight filename detection failed: expected %q, detected %q", weightFilename, blkio.weightFilename) } - value, err := fscommon.GetCgroupParamUint(helper.CgroupPath, weightFilename) + value, err := fscommon.GetCgroupParamUint(path, weightFilename) if err != nil { t.Fatal(err) } @@ -211,30 +213,32 @@ func TestBlkioSetWeightDevice(t *testing.T) { for _, legacyIOScheduler := range []bool{false, true} { // Populate cgroup - helper := NewCgroupTestUtil("blkio", t) + path := tempDir(t, "blkio") weightFilename := "blkio.bfq.weight" weightDeviceFilename := "blkio.bfq.weight_device" if legacyIOScheduler { weightFilename = "blkio.weight" weightDeviceFilename = "blkio.weight_device" } - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ weightFilename: "", weightDeviceFilename: weightDeviceBefore, }) // Apply new configuration wd := configs.NewWeightDevice(8, 0, 500, 0) weightDeviceAfter := wd.WeightString() - helper.CgroupData.config.Resources.BlkioWeightDevice = []*configs.WeightDevice{wd} + r := &configs.Resources{ + BlkioWeightDevice: []*configs.WeightDevice{wd}, + } blkio := &BlkioGroup{} - if err := blkio.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + if err := blkio.Set(path, r); err != nil { t.Fatal(err) } // Verify results if weightDeviceFilename != blkio.weightDeviceFilename { t.Fatalf("weight_device filename detection failed: expected %q, detected %q", weightDeviceFilename, blkio.weightDeviceFilename) } - value, err := fscommon.GetCgroupParamString(helper.CgroupPath, weightDeviceFilename) + value, err := fscommon.GetCgroupParamString(path, weightDeviceFilename) if err != nil { t.Fatal(err) } @@ -246,7 +250,7 @@ func TestBlkioSetWeightDevice(t *testing.T) { // regression #274 func TestBlkioSetMultipleWeightDevice(t *testing.T) { - helper := NewCgroupTestUtil("blkio", t) + path := tempDir(t, "blkio") const ( weightDeviceBefore = "8:0 400" @@ -261,20 +265,22 @@ func TestBlkioSetMultipleWeightDevice(t *testing.T) { weightDeviceAfter := wd2.WeightString() blkio := &BlkioGroup{} - blkio.detectWeightFilenames(helper.CgroupPath) + blkio.detectWeightFilenames(path) if blkio.weightDeviceFilename != "blkio.bfq.weight_device" { t.Fatalf("when blkio controller is unavailable, expected to use \"blkio.bfq.weight_device\", tried to use %q", blkio.weightDeviceFilename) } - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ blkio.weightDeviceFilename: weightDeviceBefore, }) - helper.CgroupData.config.Resources.BlkioWeightDevice = []*configs.WeightDevice{wd1, wd2} - if err := blkio.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + r := &configs.Resources{ + BlkioWeightDevice: []*configs.WeightDevice{wd1, wd2}, + } + if err := blkio.Set(path, r); err != nil { t.Fatal(err) } - value, err := fscommon.GetCgroupParamString(helper.CgroupPath, blkio.weightDeviceFilename) + value, err := fscommon.GetCgroupParamString(path, blkio.weightDeviceFilename) if err != nil { t.Fatal(err) } @@ -284,11 +290,11 @@ func TestBlkioSetMultipleWeightDevice(t *testing.T) { } func TestBlkioBFQDebugStats(t *testing.T) { - helper := NewCgroupTestUtil("blkio", t) - helper.writeFileContents(blkioBFQDebugStatsTestFiles) + path := tempDir(t, "blkio") + writeFileContents(t, path, blkioBFQDebugStatsTestFiles) blkio := &BlkioGroup{} actualStats := *cgroups.NewStats() - err := blkio.GetStats(helper.CgroupPath, &actualStats) + err := blkio.GetStats(path, &actualStats) if err != nil { t.Fatal(err) } @@ -338,12 +344,12 @@ func TestBlkioBFQDebugStats(t *testing.T) { } func TestBlkioMultipleStatsFiles(t *testing.T) { - helper := NewCgroupTestUtil("blkio", t) - helper.writeFileContents(blkioBFQDebugStatsTestFiles) - helper.writeFileContents(blkioCFQStatsTestFiles) + path := tempDir(t, "blkio") + writeFileContents(t, path, blkioBFQDebugStatsTestFiles) + writeFileContents(t, path, blkioCFQStatsTestFiles) blkio := &BlkioGroup{} actualStats := *cgroups.NewStats() - err := blkio.GetStats(helper.CgroupPath, &actualStats) + err := blkio.GetStats(path, &actualStats) if err != nil { t.Fatal(err) } @@ -393,11 +399,11 @@ func TestBlkioMultipleStatsFiles(t *testing.T) { } func TestBlkioBFQStats(t *testing.T) { - helper := NewCgroupTestUtil("blkio", t) - helper.writeFileContents(blkioBFQStatsTestFiles) + path := tempDir(t, "blkio") + writeFileContents(t, path, blkioBFQStatsTestFiles) blkio := &BlkioGroup{} actualStats := *cgroups.NewStats() - err := blkio.GetStats(helper.CgroupPath, &actualStats) + err := blkio.GetStats(path, &actualStats) if err != nil { t.Fatal(err) } @@ -459,7 +465,7 @@ func TestBlkioStatsNoFilesBFQDebug(t *testing.T) { } for _, testCase := range testCases { - helper := NewCgroupTestUtil("cpuset", t) + path := tempDir(t, "cpuset") tempBlkioTestFiles := map[string]string{} for i, v := range blkioBFQDebugStatsTestFiles { @@ -467,10 +473,10 @@ func TestBlkioStatsNoFilesBFQDebug(t *testing.T) { } delete(tempBlkioTestFiles, testCase.filename) - helper.writeFileContents(tempBlkioTestFiles) + writeFileContents(t, path, tempBlkioTestFiles) cpuset := &CpusetGroup{} actualStats := *cgroups.NewStats() - err := cpuset.GetStats(helper.CgroupPath, &actualStats) + err := cpuset.GetStats(path, &actualStats) if err != nil { t.Errorf(fmt.Sprintf("test case '%s' failed unexpectedly: %s", testCase.desc, err)) } @@ -478,12 +484,12 @@ func TestBlkioStatsNoFilesBFQDebug(t *testing.T) { } func TestBlkioCFQStats(t *testing.T) { - helper := NewCgroupTestUtil("blkio", t) - helper.writeFileContents(blkioCFQStatsTestFiles) + path := tempDir(t, "blkio") + writeFileContents(t, path, blkioCFQStatsTestFiles) blkio := &BlkioGroup{} actualStats := *cgroups.NewStats() - err := blkio.GetStats(helper.CgroupPath, &actualStats) + err := blkio.GetStats(path, &actualStats) if err != nil { t.Fatal(err) } @@ -573,7 +579,7 @@ func TestBlkioStatsNoFilesCFQ(t *testing.T) { } for _, testCase := range testCases { - helper := NewCgroupTestUtil("cpuset", t) + path := tempDir(t, "cpuset") tempBlkioTestFiles := map[string]string{} for i, v := range blkioCFQStatsTestFiles { @@ -581,10 +587,10 @@ func TestBlkioStatsNoFilesCFQ(t *testing.T) { } delete(tempBlkioTestFiles, testCase.filename) - helper.writeFileContents(tempBlkioTestFiles) + writeFileContents(t, path, tempBlkioTestFiles) cpuset := &CpusetGroup{} actualStats := *cgroups.NewStats() - err := cpuset.GetStats(helper.CgroupPath, &actualStats) + err := cpuset.GetStats(path, &actualStats) if err != nil { t.Errorf(fmt.Sprintf("test case '%s' failed unexpectedly: %s", testCase.desc, err)) } @@ -592,8 +598,8 @@ func TestBlkioStatsNoFilesCFQ(t *testing.T) { } func TestBlkioStatsUnexpectedNumberOfFields(t *testing.T) { - helper := NewCgroupTestUtil("blkio", t) - helper.writeFileContents(map[string]string{ + path := tempDir(t, "blkio") + writeFileContents(t, path, map[string]string{ "blkio.io_service_bytes_recursive": "8:0 Read 100 100", "blkio.io_serviced_recursive": servicedRecursiveContents, "blkio.io_queued_recursive": queuedRecursiveContents, @@ -606,15 +612,15 @@ func TestBlkioStatsUnexpectedNumberOfFields(t *testing.T) { blkio := &BlkioGroup{} actualStats := *cgroups.NewStats() - err := blkio.GetStats(helper.CgroupPath, &actualStats) + err := blkio.GetStats(path, &actualStats) if err == nil { t.Fatal("Expected to fail, but did not") } } func TestBlkioStatsUnexpectedFieldType(t *testing.T) { - helper := NewCgroupTestUtil("blkio", t) - helper.writeFileContents(map[string]string{ + path := tempDir(t, "blkio") + writeFileContents(t, path, map[string]string{ "blkio.io_service_bytes_recursive": "8:0 Read Write", "blkio.io_serviced_recursive": servicedRecursiveContents, "blkio.io_queued_recursive": queuedRecursiveContents, @@ -627,15 +633,15 @@ func TestBlkioStatsUnexpectedFieldType(t *testing.T) { blkio := &BlkioGroup{} actualStats := *cgroups.NewStats() - err := blkio.GetStats(helper.CgroupPath, &actualStats) + err := blkio.GetStats(path, &actualStats) if err == nil { t.Fatal("Expected to fail, but did not") } } func TestThrottleRecursiveBlkioStats(t *testing.T) { - helper := NewCgroupTestUtil("blkio", t) - helper.writeFileContents(map[string]string{ + path := tempDir(t, "blkio") + writeFileContents(t, path, map[string]string{ "blkio.io_service_bytes_recursive": "", "blkio.io_serviced_recursive": "", "blkio.io_queued_recursive": "", @@ -650,7 +656,7 @@ func TestThrottleRecursiveBlkioStats(t *testing.T) { blkio := &BlkioGroup{} actualStats := *cgroups.NewStats() - err := blkio.GetStats(helper.CgroupPath, &actualStats) + err := blkio.GetStats(path, &actualStats) if err != nil { t.Fatal(err) } @@ -684,8 +690,8 @@ func TestThrottleRecursiveBlkioStats(t *testing.T) { } func TestThrottleBlkioStats(t *testing.T) { - helper := NewCgroupTestUtil("blkio", t) - helper.writeFileContents(map[string]string{ + path := tempDir(t, "blkio") + writeFileContents(t, path, map[string]string{ "blkio.io_service_bytes_recursive": "", "blkio.io_serviced_recursive": "", "blkio.io_queued_recursive": "", @@ -700,7 +706,7 @@ func TestThrottleBlkioStats(t *testing.T) { blkio := &BlkioGroup{} actualStats := *cgroups.NewStats() - err := blkio.GetStats(helper.CgroupPath, &actualStats) + err := blkio.GetStats(path, &actualStats) if err != nil { t.Fatal(err) } @@ -734,7 +740,7 @@ func TestThrottleBlkioStats(t *testing.T) { } func TestBlkioSetThrottleReadBpsDevice(t *testing.T) { - helper := NewCgroupTestUtil("blkio", t) + path := tempDir(t, "blkio") const ( throttleBefore = `8:0 1024` @@ -743,17 +749,19 @@ func TestBlkioSetThrottleReadBpsDevice(t *testing.T) { td := configs.NewThrottleDevice(8, 0, 2048) throttleAfter := td.String() - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "blkio.throttle.read_bps_device": throttleBefore, }) - helper.CgroupData.config.Resources.BlkioThrottleReadBpsDevice = []*configs.ThrottleDevice{td} + r := &configs.Resources{ + BlkioThrottleReadBpsDevice: []*configs.ThrottleDevice{td}, + } blkio := &BlkioGroup{} - if err := blkio.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + if err := blkio.Set(path, r); err != nil { t.Fatal(err) } - value, err := fscommon.GetCgroupParamString(helper.CgroupPath, "blkio.throttle.read_bps_device") + value, err := fscommon.GetCgroupParamString(path, "blkio.throttle.read_bps_device") if err != nil { t.Fatal(err) } @@ -763,7 +771,7 @@ func TestBlkioSetThrottleReadBpsDevice(t *testing.T) { } func TestBlkioSetThrottleWriteBpsDevice(t *testing.T) { - helper := NewCgroupTestUtil("blkio", t) + path := tempDir(t, "blkio") const ( throttleBefore = `8:0 1024` @@ -772,17 +780,19 @@ func TestBlkioSetThrottleWriteBpsDevice(t *testing.T) { td := configs.NewThrottleDevice(8, 0, 2048) throttleAfter := td.String() - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "blkio.throttle.write_bps_device": throttleBefore, }) - helper.CgroupData.config.Resources.BlkioThrottleWriteBpsDevice = []*configs.ThrottleDevice{td} + r := &configs.Resources{ + BlkioThrottleWriteBpsDevice: []*configs.ThrottleDevice{td}, + } blkio := &BlkioGroup{} - if err := blkio.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + if err := blkio.Set(path, r); err != nil { t.Fatal(err) } - value, err := fscommon.GetCgroupParamString(helper.CgroupPath, "blkio.throttle.write_bps_device") + value, err := fscommon.GetCgroupParamString(path, "blkio.throttle.write_bps_device") if err != nil { t.Fatal(err) } @@ -792,7 +802,7 @@ func TestBlkioSetThrottleWriteBpsDevice(t *testing.T) { } func TestBlkioSetThrottleReadIOpsDevice(t *testing.T) { - helper := NewCgroupTestUtil("blkio", t) + path := tempDir(t, "blkio") const ( throttleBefore = `8:0 1024` @@ -801,17 +811,19 @@ func TestBlkioSetThrottleReadIOpsDevice(t *testing.T) { td := configs.NewThrottleDevice(8, 0, 2048) throttleAfter := td.String() - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "blkio.throttle.read_iops_device": throttleBefore, }) - helper.CgroupData.config.Resources.BlkioThrottleReadIOPSDevice = []*configs.ThrottleDevice{td} + r := &configs.Resources{ + BlkioThrottleReadIOPSDevice: []*configs.ThrottleDevice{td}, + } blkio := &BlkioGroup{} - if err := blkio.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + if err := blkio.Set(path, r); err != nil { t.Fatal(err) } - value, err := fscommon.GetCgroupParamString(helper.CgroupPath, "blkio.throttle.read_iops_device") + value, err := fscommon.GetCgroupParamString(path, "blkio.throttle.read_iops_device") if err != nil { t.Fatal(err) } @@ -821,7 +833,7 @@ func TestBlkioSetThrottleReadIOpsDevice(t *testing.T) { } func TestBlkioSetThrottleWriteIOpsDevice(t *testing.T) { - helper := NewCgroupTestUtil("blkio", t) + path := tempDir(t, "blkio") const ( throttleBefore = `8:0 1024` @@ -830,17 +842,19 @@ func TestBlkioSetThrottleWriteIOpsDevice(t *testing.T) { td := configs.NewThrottleDevice(8, 0, 2048) throttleAfter := td.String() - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "blkio.throttle.write_iops_device": throttleBefore, }) - helper.CgroupData.config.Resources.BlkioThrottleWriteIOPSDevice = []*configs.ThrottleDevice{td} + r := &configs.Resources{ + BlkioThrottleWriteIOPSDevice: []*configs.ThrottleDevice{td}, + } blkio := &BlkioGroup{} - if err := blkio.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + if err := blkio.Set(path, r); err != nil { t.Fatal(err) } - value, err := fscommon.GetCgroupParamString(helper.CgroupPath, "blkio.throttle.write_iops_device") + value, err := fscommon.GetCgroupParamString(path, "blkio.throttle.write_iops_device") if err != nil { t.Fatal(err) } diff --git a/libcontainer/cgroups/fs/cpu.go b/libcontainer/cgroups/fs/cpu.go index cd81cabc185..6c79f899b48 100644 --- a/libcontainer/cgroups/fs/cpu.go +++ b/libcontainer/cgroups/fs/cpu.go @@ -19,24 +19,19 @@ func (s *CpuGroup) Name() string { return "cpu" } -func (s *CpuGroup) Apply(path string, d *cgroupData) error { - // This might happen if we have no cpu cgroup mounted. - // Just do nothing and don't fail. - if path == "" { - return nil - } +func (s *CpuGroup) Apply(path string, r *configs.Resources, pid int) error { if err := os.MkdirAll(path, 0o755); err != nil { return err } // We should set the real-Time group scheduling settings before moving // in the process because if the process is already in SCHED_RR mode // and no RT bandwidth is set, adding it will fail. - if err := s.SetRtSched(path, d.config.Resources); err != nil { + if err := s.SetRtSched(path, r); err != nil { return err } - // Since we are not using join(), we need to place the pid - // into the procs file unlike other subsystems. - return cgroups.WriteCgroupProc(path, d.pid) + // Since we are not using apply(), we need to place the pid + // into the procs file. + return cgroups.WriteCgroupProc(path, pid) } func (s *CpuGroup) SetRtSched(path string, r *configs.Resources) error { diff --git a/libcontainer/cgroups/fs/cpu_test.go b/libcontainer/cgroups/fs/cpu_test.go index 958e6040e72..bbdd45a7118 100644 --- a/libcontainer/cgroups/fs/cpu_test.go +++ b/libcontainer/cgroups/fs/cpu_test.go @@ -7,27 +7,30 @@ import ( "github.com/opencontainers/runc/libcontainer/cgroups" "github.com/opencontainers/runc/libcontainer/cgroups/fscommon" + "github.com/opencontainers/runc/libcontainer/configs" ) func TestCpuSetShares(t *testing.T) { - helper := NewCgroupTestUtil("cpu", t) + path := tempDir(t, "cpu") const ( sharesBefore = 1024 sharesAfter = 512 ) - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "cpu.shares": strconv.Itoa(sharesBefore), }) - helper.CgroupData.config.Resources.CpuShares = sharesAfter + r := &configs.Resources{ + CpuShares: sharesAfter, + } cpu := &CpuGroup{} - if err := cpu.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + if err := cpu.Set(path, r); err != nil { t.Fatal(err) } - value, err := fscommon.GetCgroupParamUint(helper.CgroupPath, "cpu.shares") + value, err := fscommon.GetCgroupParamUint(path, "cpu.shares") if err != nil { t.Fatal(err) } @@ -37,7 +40,7 @@ func TestCpuSetShares(t *testing.T) { } func TestCpuSetBandWidth(t *testing.T) { - helper := NewCgroupTestUtil("cpu", t) + path := tempDir(t, "cpu") const ( quotaBefore = 8000 @@ -50,23 +53,25 @@ func TestCpuSetBandWidth(t *testing.T) { rtPeriodAfter = 7000 ) - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "cpu.cfs_quota_us": strconv.Itoa(quotaBefore), "cpu.cfs_period_us": strconv.Itoa(periodBefore), "cpu.rt_runtime_us": strconv.Itoa(rtRuntimeBefore), "cpu.rt_period_us": strconv.Itoa(rtPeriodBefore), }) - helper.CgroupData.config.Resources.CpuQuota = quotaAfter - helper.CgroupData.config.Resources.CpuPeriod = periodAfter - helper.CgroupData.config.Resources.CpuRtRuntime = rtRuntimeAfter - helper.CgroupData.config.Resources.CpuRtPeriod = rtPeriodAfter + r := &configs.Resources{ + CpuQuota: quotaAfter, + CpuPeriod: periodAfter, + CpuRtRuntime: rtRuntimeAfter, + CpuRtPeriod: rtPeriodAfter, + } cpu := &CpuGroup{} - if err := cpu.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + if err := cpu.Set(path, r); err != nil { t.Fatal(err) } - quota, err := fscommon.GetCgroupParamUint(helper.CgroupPath, "cpu.cfs_quota_us") + quota, err := fscommon.GetCgroupParamUint(path, "cpu.cfs_quota_us") if err != nil { t.Fatal(err) } @@ -74,7 +79,7 @@ func TestCpuSetBandWidth(t *testing.T) { t.Fatal("Got the wrong value, set cpu.cfs_quota_us failed.") } - period, err := fscommon.GetCgroupParamUint(helper.CgroupPath, "cpu.cfs_period_us") + period, err := fscommon.GetCgroupParamUint(path, "cpu.cfs_period_us") if err != nil { t.Fatal(err) } @@ -82,7 +87,7 @@ func TestCpuSetBandWidth(t *testing.T) { t.Fatal("Got the wrong value, set cpu.cfs_period_us failed.") } - rtRuntime, err := fscommon.GetCgroupParamUint(helper.CgroupPath, "cpu.rt_runtime_us") + rtRuntime, err := fscommon.GetCgroupParamUint(path, "cpu.rt_runtime_us") if err != nil { t.Fatal(err) } @@ -90,7 +95,7 @@ func TestCpuSetBandWidth(t *testing.T) { t.Fatal("Got the wrong value, set cpu.rt_runtime_us failed.") } - rtPeriod, err := fscommon.GetCgroupParamUint(helper.CgroupPath, "cpu.rt_period_us") + rtPeriod, err := fscommon.GetCgroupParamUint(path, "cpu.rt_period_us") if err != nil { t.Fatal(err) } @@ -100,7 +105,7 @@ func TestCpuSetBandWidth(t *testing.T) { } func TestCpuStats(t *testing.T) { - helper := NewCgroupTestUtil("cpu", t) + path := tempDir(t, "cpu") const ( nrPeriods = 2000 @@ -110,13 +115,13 @@ func TestCpuStats(t *testing.T) { cpuStatContent := fmt.Sprintf("nr_periods %d\nnr_throttled %d\nthrottled_time %d\n", nrPeriods, nrThrottled, throttledTime) - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "cpu.stat": cpuStatContent, }) cpu := &CpuGroup{} actualStats := *cgroups.NewStats() - err := cpu.GetStats(helper.CgroupPath, &actualStats) + err := cpu.GetStats(path, &actualStats) if err != nil { t.Fatal(err) } @@ -131,36 +136,36 @@ func TestCpuStats(t *testing.T) { } func TestNoCpuStatFile(t *testing.T) { - helper := NewCgroupTestUtil("cpu", t) + path := tempDir(t, "cpu") cpu := &CpuGroup{} actualStats := *cgroups.NewStats() - err := cpu.GetStats(helper.CgroupPath, &actualStats) + err := cpu.GetStats(path, &actualStats) if err != nil { t.Fatal("Expected not to fail, but did") } } func TestInvalidCpuStat(t *testing.T) { - helper := NewCgroupTestUtil("cpu", t) + path := tempDir(t, "cpu") cpuStatContent := `nr_periods 2000 nr_throttled 200 throttled_time fortytwo` - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "cpu.stat": cpuStatContent, }) cpu := &CpuGroup{} actualStats := *cgroups.NewStats() - err := cpu.GetStats(helper.CgroupPath, &actualStats) + err := cpu.GetStats(path, &actualStats) if err == nil { t.Fatal("Expected failed stat parsing.") } } func TestCpuSetRtSchedAtApply(t *testing.T) { - helper := NewCgroupTestUtil("cpu", t) + path := tempDir(t, "cpu") const ( rtRuntimeBefore = 0 @@ -169,21 +174,22 @@ func TestCpuSetRtSchedAtApply(t *testing.T) { rtPeriodAfter = 7000 ) - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "cpu.rt_runtime_us": strconv.Itoa(rtRuntimeBefore), "cpu.rt_period_us": strconv.Itoa(rtPeriodBefore), }) - helper.CgroupData.config.Resources.CpuRtRuntime = rtRuntimeAfter - helper.CgroupData.config.Resources.CpuRtPeriod = rtPeriodAfter + r := &configs.Resources{ + CpuRtRuntime: rtRuntimeAfter, + CpuRtPeriod: rtPeriodAfter, + } cpu := &CpuGroup{} - helper.CgroupData.pid = 1234 - if err := cpu.Apply(helper.CgroupPath, helper.CgroupData); err != nil { + if err := cpu.Apply(path, r, 1234); err != nil { t.Fatal(err) } - rtRuntime, err := fscommon.GetCgroupParamUint(helper.CgroupPath, "cpu.rt_runtime_us") + rtRuntime, err := fscommon.GetCgroupParamUint(path, "cpu.rt_runtime_us") if err != nil { t.Fatal(err) } @@ -191,7 +197,7 @@ func TestCpuSetRtSchedAtApply(t *testing.T) { t.Fatal("Got the wrong value, set cpu.rt_runtime_us failed.") } - rtPeriod, err := fscommon.GetCgroupParamUint(helper.CgroupPath, "cpu.rt_period_us") + rtPeriod, err := fscommon.GetCgroupParamUint(path, "cpu.rt_period_us") if err != nil { t.Fatal(err) } @@ -199,7 +205,7 @@ func TestCpuSetRtSchedAtApply(t *testing.T) { t.Fatal("Got the wrong value, set cpu.rt_period_us failed.") } - pid, err := fscommon.GetCgroupParamUint(helper.CgroupPath, "cgroup.procs") + pid, err := fscommon.GetCgroupParamUint(path, "cgroup.procs") if err != nil { t.Fatal(err) } diff --git a/libcontainer/cgroups/fs/cpuacct.go b/libcontainer/cgroups/fs/cpuacct.go index 537bada66a1..d3bd7e111c7 100644 --- a/libcontainer/cgroups/fs/cpuacct.go +++ b/libcontainer/cgroups/fs/cpuacct.go @@ -34,8 +34,8 @@ func (s *CpuacctGroup) Name() string { return "cpuacct" } -func (s *CpuacctGroup) Apply(path string, d *cgroupData) error { - return apply(path, d.pid) +func (s *CpuacctGroup) Apply(path string, _ *configs.Resources, pid int) error { + return apply(path, pid) } func (s *CpuacctGroup) Set(_ string, _ *configs.Resources) error { diff --git a/libcontainer/cgroups/fs/cpuacct_test.go b/libcontainer/cgroups/fs/cpuacct_test.go index 753a0194b71..70b237a0b61 100644 --- a/libcontainer/cgroups/fs/cpuacct_test.go +++ b/libcontainer/cgroups/fs/cpuacct_test.go @@ -24,8 +24,8 @@ const ( ) func TestCpuacctStats(t *testing.T) { - helper := NewCgroupTestUtil("cpuacct.", t) - helper.writeFileContents(map[string]string{ + path := tempDir(t, "cpuacct") + writeFileContents(t, path, map[string]string{ "cpuacct.usage": cpuAcctUsageContents, "cpuacct.usage_percpu": cpuAcctUsagePerCPUContents, "cpuacct.stat": cpuAcctStatContents, @@ -34,7 +34,7 @@ func TestCpuacctStats(t *testing.T) { cpuacct := &CpuacctGroup{} actualStats := *cgroups.NewStats() - err := cpuacct.GetStats(helper.CgroupPath, &actualStats) + err := cpuacct.GetStats(path, &actualStats) if err != nil { t.Fatal(err) } @@ -64,8 +64,8 @@ func TestCpuacctStats(t *testing.T) { } func TestCpuacctStatsWithoutUsageAll(t *testing.T) { - helper := NewCgroupTestUtil("cpuacct.", t) - helper.writeFileContents(map[string]string{ + path := tempDir(t, "cpuacct") + writeFileContents(t, path, map[string]string{ "cpuacct.usage": cpuAcctUsageContents, "cpuacct.usage_percpu": cpuAcctUsagePerCPUContents, "cpuacct.stat": cpuAcctStatContents, @@ -73,7 +73,7 @@ func TestCpuacctStatsWithoutUsageAll(t *testing.T) { cpuacct := &CpuacctGroup{} actualStats := *cgroups.NewStats() - err := cpuacct.GetStats(helper.CgroupPath, &actualStats) + err := cpuacct.GetStats(path, &actualStats) if err != nil { t.Fatal(err) } diff --git a/libcontainer/cgroups/fs/cpuset.go b/libcontainer/cgroups/fs/cpuset.go index d4edfaafad2..550baa42756 100644 --- a/libcontainer/cgroups/fs/cpuset.go +++ b/libcontainer/cgroups/fs/cpuset.go @@ -20,8 +20,8 @@ func (s *CpusetGroup) Name() string { return "cpuset" } -func (s *CpusetGroup) Apply(path string, d *cgroupData) error { - return s.ApplyDir(path, d.config.Resources, d.pid) +func (s *CpusetGroup) Apply(path string, r *configs.Resources, pid int) error { + return s.ApplyDir(path, r, pid) } func (s *CpusetGroup) Set(path string, r *configs.Resources) error { @@ -166,9 +166,8 @@ func (s *CpusetGroup) ApplyDir(dir string, r *configs.Resources, pid int) error if err := s.ensureCpusAndMems(dir, r); err != nil { return err } - - // because we are not using d.join we need to place the pid into the procs file - // unlike the other subsystems + // Since we are not using apply(), we need to place the pid + // into the procs file. return cgroups.WriteCgroupProc(dir, pid) } diff --git a/libcontainer/cgroups/fs/cpuset_test.go b/libcontainer/cgroups/fs/cpuset_test.go index 4918aaa2867..8933b3ca351 100644 --- a/libcontainer/cgroups/fs/cpuset_test.go +++ b/libcontainer/cgroups/fs/cpuset_test.go @@ -6,6 +6,7 @@ import ( "github.com/opencontainers/runc/libcontainer/cgroups" "github.com/opencontainers/runc/libcontainer/cgroups/fscommon" + "github.com/opencontainers/runc/libcontainer/configs" ) const ( @@ -37,24 +38,26 @@ var cpusetTestFiles = map[string]string{ } func TestCPUSetSetCpus(t *testing.T) { - helper := NewCgroupTestUtil("cpuset", t) + path := tempDir(t, "cpuset") const ( cpusBefore = "0" cpusAfter = "1-3" ) - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "cpuset.cpus": cpusBefore, }) - helper.CgroupData.config.Resources.CpusetCpus = cpusAfter + r := &configs.Resources{ + CpusetCpus: cpusAfter, + } cpuset := &CpusetGroup{} - if err := cpuset.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + if err := cpuset.Set(path, r); err != nil { t.Fatal(err) } - value, err := fscommon.GetCgroupParamString(helper.CgroupPath, "cpuset.cpus") + value, err := fscommon.GetCgroupParamString(path, "cpuset.cpus") if err != nil { t.Fatal(err) } @@ -64,24 +67,26 @@ func TestCPUSetSetCpus(t *testing.T) { } func TestCPUSetSetMems(t *testing.T) { - helper := NewCgroupTestUtil("cpuset", t) + path := tempDir(t, "cpuset") const ( memsBefore = "0" memsAfter = "1" ) - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "cpuset.mems": memsBefore, }) - helper.CgroupData.config.Resources.CpusetMems = memsAfter + r := &configs.Resources{ + CpusetMems: memsAfter, + } cpuset := &CpusetGroup{} - if err := cpuset.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + if err := cpuset.Set(path, r); err != nil { t.Fatal(err) } - value, err := fscommon.GetCgroupParamString(helper.CgroupPath, "cpuset.mems") + value, err := fscommon.GetCgroupParamString(path, "cpuset.mems") if err != nil { t.Fatal(err) } @@ -91,12 +96,12 @@ func TestCPUSetSetMems(t *testing.T) { } func TestCPUSetStatsCorrect(t *testing.T) { - helper := NewCgroupTestUtil("cpuset", t) - helper.writeFileContents(cpusetTestFiles) + path := tempDir(t, "cpuset") + writeFileContents(t, path, cpusetTestFiles) cpuset := &CpusetGroup{} actualStats := *cgroups.NewStats() - err := cpuset.GetStats(helper.CgroupPath, &actualStats) + err := cpuset.GetStats(path, &actualStats) if err != nil { t.Fatal(err) } @@ -205,7 +210,7 @@ func TestCPUSetStatsMissingFiles(t *testing.T) { }, } { t.Run(testCase.desc, func(t *testing.T) { - helper := NewCgroupTestUtil("cpuset", t) + path := tempDir(t, "cpuset") tempCpusetTestFiles := map[string]string{} for i, v := range cpusetTestFiles { @@ -214,19 +219,19 @@ func TestCPUSetStatsMissingFiles(t *testing.T) { if testCase.removeFile { delete(tempCpusetTestFiles, testCase.filename) - helper.writeFileContents(tempCpusetTestFiles) + writeFileContents(t, path, tempCpusetTestFiles) cpuset := &CpusetGroup{} actualStats := *cgroups.NewStats() - err := cpuset.GetStats(helper.CgroupPath, &actualStats) + err := cpuset.GetStats(path, &actualStats) if err != nil { t.Errorf("failed unexpectedly: %q", err) } } else { tempCpusetTestFiles[testCase.filename] = testCase.contents - helper.writeFileContents(tempCpusetTestFiles) + writeFileContents(t, path, tempCpusetTestFiles) cpuset := &CpusetGroup{} actualStats := *cgroups.NewStats() - err := cpuset.GetStats(helper.CgroupPath, &actualStats) + err := cpuset.GetStats(path, &actualStats) if err == nil { t.Error("failed to return expected error") diff --git a/libcontainer/cgroups/fs/devices.go b/libcontainer/cgroups/fs/devices.go index 20a11e8c670..2476186a640 100644 --- a/libcontainer/cgroups/fs/devices.go +++ b/libcontainer/cgroups/fs/devices.go @@ -20,8 +20,8 @@ func (s *DevicesGroup) Name() string { return "devices" } -func (s *DevicesGroup) Apply(path string, d *cgroupData) error { - if d.config.SkipDevices { +func (s *DevicesGroup) Apply(path string, r *configs.Resources, pid int) error { + if r.SkipDevices { return nil } if path == "" { @@ -29,7 +29,8 @@ func (s *DevicesGroup) Apply(path string, d *cgroupData) error { // is a hard requirement for container's security. return errSubsystemDoesNotExist } - return apply(path, d.pid) + + return apply(path, pid) } func loadEmulator(path string) (*cgroupdevices.Emulator, error) { diff --git a/libcontainer/cgroups/fs/devices_test.go b/libcontainer/cgroups/fs/devices_test.go index b2d1624cec8..4819eee5366 100644 --- a/libcontainer/cgroups/fs/devices_test.go +++ b/libcontainer/cgroups/fs/devices_test.go @@ -4,35 +4,38 @@ import ( "testing" "github.com/opencontainers/runc/libcontainer/cgroups/fscommon" + "github.com/opencontainers/runc/libcontainer/configs" "github.com/opencontainers/runc/libcontainer/devices" ) func TestDevicesSetAllow(t *testing.T) { - helper := NewCgroupTestUtil("devices", t) + path := tempDir(t, "devices") - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "devices.allow": "", "devices.deny": "", "devices.list": "a *:* rwm", }) - helper.CgroupData.config.Resources.Devices = []*devices.Rule{ - { - Type: devices.CharDevice, - Major: 1, - Minor: 5, - Permissions: devices.Permissions("rwm"), - Allow: true, + r := &configs.Resources{ + Devices: []*devices.Rule{ + { + Type: devices.CharDevice, + Major: 1, + Minor: 5, + Permissions: devices.Permissions("rwm"), + Allow: true, + }, }, } d := &DevicesGroup{testingSkipFinalCheck: true} - if err := d.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + if err := d.Set(path, r); err != nil { t.Fatal(err) } // The default deny rule must be written. - value, err := fscommon.GetCgroupParamString(helper.CgroupPath, "devices.deny") + value, err := fscommon.GetCgroupParamString(path, "devices.deny") if err != nil { t.Fatal(err) } @@ -41,7 +44,7 @@ func TestDevicesSetAllow(t *testing.T) { } // Permitted rule must be written. - if value, err := fscommon.GetCgroupParamString(helper.CgroupPath, "devices.allow"); err != nil { + if value, err := fscommon.GetCgroupParamString(path, "devices.allow"); err != nil { t.Fatal(err) } else if value != "c 1:5 rwm" { t.Errorf("Got the wrong value (%q), set devices.allow failed.", value) diff --git a/libcontainer/cgroups/fs/freezer.go b/libcontainer/cgroups/fs/freezer.go index 1dc4dd9fc2f..987f1bf5e74 100644 --- a/libcontainer/cgroups/fs/freezer.go +++ b/libcontainer/cgroups/fs/freezer.go @@ -19,8 +19,8 @@ func (s *FreezerGroup) Name() string { return "freezer" } -func (s *FreezerGroup) Apply(path string, d *cgroupData) error { - return apply(path, d.pid) +func (s *FreezerGroup) Apply(path string, _ *configs.Resources, pid int) error { + return apply(path, pid) } func (s *FreezerGroup) Set(path string, r *configs.Resources) (Err error) { diff --git a/libcontainer/cgroups/fs/freezer_test.go b/libcontainer/cgroups/fs/freezer_test.go index b441bf74b69..bbdd3717dbe 100644 --- a/libcontainer/cgroups/fs/freezer_test.go +++ b/libcontainer/cgroups/fs/freezer_test.go @@ -8,19 +8,21 @@ import ( ) func TestFreezerSetState(t *testing.T) { - helper := NewCgroupTestUtil("freezer", t) + path := tempDir(t, "freezer") - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "freezer.state": string(configs.Frozen), }) - helper.CgroupData.config.Resources.Freezer = configs.Thawed + r := &configs.Resources{ + Freezer: configs.Thawed, + } freezer := &FreezerGroup{} - if err := freezer.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + if err := freezer.Set(path, r); err != nil { t.Fatal(err) } - value, err := fscommon.GetCgroupParamString(helper.CgroupPath, "freezer.state") + value, err := fscommon.GetCgroupParamString(path, "freezer.state") if err != nil { t.Fatal(err) } @@ -30,15 +32,15 @@ func TestFreezerSetState(t *testing.T) { } func TestFreezerSetInvalidState(t *testing.T) { - helper := NewCgroupTestUtil("freezer", t) + path := tempDir(t, "freezer") - const ( - invalidArg configs.FreezerState = "Invalid" - ) + const invalidArg configs.FreezerState = "Invalid" - helper.CgroupData.config.Resources.Freezer = invalidArg + r := &configs.Resources{ + Freezer: invalidArg, + } freezer := &FreezerGroup{} - if err := freezer.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err == nil { + if err := freezer.Set(path, r); err == nil { t.Fatal("Failed to return invalid argument error") } } diff --git a/libcontainer/cgroups/fs/fs.go b/libcontainer/cgroups/fs/fs.go index 391c9532bc7..36e0c76c447 100644 --- a/libcontainer/cgroups/fs/fs.go +++ b/libcontainer/cgroups/fs/fs.go @@ -38,10 +38,12 @@ var errSubsystemDoesNotExist = errors.New("cgroup: subsystem does not exist") type subsystem interface { // Name returns the name of the subsystem. Name() string - // Returns the stats, as 'stats', corresponding to the cgroup under 'path'. + // GetStats fills in the stats for the subsystem. GetStats(path string, stats *cgroups.Stats) error - // Creates and joins the cgroup represented by 'cgroupData'. - Apply(path string, c *cgroupData) error + // Apply creates and joins a cgroup, adding pid into it. Some + // subsystems use resources to pre-configure the cgroup parents + // before creating or joining it. + Apply(path string, r *configs.Resources, pid int) error // Set sets the cgroup resources. Set(path string, r *configs.Resources) error } @@ -94,14 +96,16 @@ func (m *manager) Apply(pid int) (err error) { return cgroups.ErrV1NoUnified } - d, err := getCgroupData(m.cgroups, pid) + root, err := rootPath() if err != nil { return err } + inner, err := innerPath(c) + m.paths = make(map[string]string) for _, sys := range subsystems { - p, err := d.path(sys.Name()) + p, err := subsysPath(root, inner, sys.Name()) if err != nil { // The non-presence of the devices subsystem is // considered fatal for security reasons. @@ -112,7 +116,7 @@ func (m *manager) Apply(pid int) (err error) { } m.paths[sys.Name()] = p - if err := sys.Apply(p, d); err != nil { + if err := sys.Apply(p, c.Resources, pid); err != nil { // In the case of rootless (including euid=0 in userns), where an // explicit cgroup path hasn't been set, we don't bail on error in // case of permission problems. Cases where limits have been set diff --git a/libcontainer/cgroups/fs/hugetlb.go b/libcontainer/cgroups/fs/hugetlb.go index f95e3be25d1..86650128c8d 100644 --- a/libcontainer/cgroups/fs/hugetlb.go +++ b/libcontainer/cgroups/fs/hugetlb.go @@ -14,8 +14,8 @@ func (s *HugetlbGroup) Name() string { return "hugetlb" } -func (s *HugetlbGroup) Apply(path string, d *cgroupData) error { - return apply(path, d.pid) +func (s *HugetlbGroup) Apply(path string, _ *configs.Resources, pid int) error { + return apply(path, pid) } func (s *HugetlbGroup) Set(path string, r *configs.Resources) error { diff --git a/libcontainer/cgroups/fs/hugetlb_test.go b/libcontainer/cgroups/fs/hugetlb_test.go index ac4ec1533f5..ba54f35a16c 100644 --- a/libcontainer/cgroups/fs/hugetlb_test.go +++ b/libcontainer/cgroups/fs/hugetlb_test.go @@ -24,7 +24,7 @@ const ( ) func TestHugetlbSetHugetlb(t *testing.T) { - helper := NewCgroupTestUtil("hugetlb", t) + path := tempDir(t, "hugetlb") const ( hugetlbBefore = 256 @@ -32,27 +32,28 @@ func TestHugetlbSetHugetlb(t *testing.T) { ) for _, pageSize := range HugePageSizes { - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ fmt.Sprintf(limit, pageSize): strconv.Itoa(hugetlbBefore), }) } + r := &configs.Resources{} for _, pageSize := range HugePageSizes { - helper.CgroupData.config.Resources.HugetlbLimit = []*configs.HugepageLimit{ + r.HugetlbLimit = []*configs.HugepageLimit{ { Pagesize: pageSize, Limit: hugetlbAfter, }, } hugetlb := &HugetlbGroup{} - if err := hugetlb.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + if err := hugetlb.Set(path, r); err != nil { t.Fatal(err) } } for _, pageSize := range HugePageSizes { limit := fmt.Sprintf(limit, pageSize) - value, err := fscommon.GetCgroupParamUint(helper.CgroupPath, limit) + value, err := fscommon.GetCgroupParamUint(path, limit) if err != nil { t.Fatal(err) } @@ -63,9 +64,9 @@ func TestHugetlbSetHugetlb(t *testing.T) { } func TestHugetlbStats(t *testing.T) { - helper := NewCgroupTestUtil("hugetlb", t) + path := tempDir(t, "hugetlb") for _, pageSize := range HugePageSizes { - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ fmt.Sprintf(usage, pageSize): hugetlbUsageContents, fmt.Sprintf(maxUsage, pageSize): hugetlbMaxUsageContents, fmt.Sprintf(failcnt, pageSize): hugetlbFailcnt, @@ -74,7 +75,7 @@ func TestHugetlbStats(t *testing.T) { hugetlb := &HugetlbGroup{} actualStats := *cgroups.NewStats() - err := hugetlb.GetStats(helper.CgroupPath, &actualStats) + err := hugetlb.GetStats(path, &actualStats) if err != nil { t.Fatal(err) } @@ -85,39 +86,39 @@ func TestHugetlbStats(t *testing.T) { } func TestHugetlbStatsNoUsageFile(t *testing.T) { - helper := NewCgroupTestUtil("hugetlb", t) - helper.writeFileContents(map[string]string{ + path := tempDir(t, "hugetlb") + writeFileContents(t, path, map[string]string{ maxUsage: hugetlbMaxUsageContents, }) hugetlb := &HugetlbGroup{} actualStats := *cgroups.NewStats() - err := hugetlb.GetStats(helper.CgroupPath, &actualStats) + err := hugetlb.GetStats(path, &actualStats) if err == nil { t.Fatal("Expected failure") } } func TestHugetlbStatsNoMaxUsageFile(t *testing.T) { - helper := NewCgroupTestUtil("hugetlb", t) + path := tempDir(t, "hugetlb") for _, pageSize := range HugePageSizes { - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ fmt.Sprintf(usage, pageSize): hugetlbUsageContents, }) } hugetlb := &HugetlbGroup{} actualStats := *cgroups.NewStats() - err := hugetlb.GetStats(helper.CgroupPath, &actualStats) + err := hugetlb.GetStats(path, &actualStats) if err == nil { t.Fatal("Expected failure") } } func TestHugetlbStatsBadUsageFile(t *testing.T) { - helper := NewCgroupTestUtil("hugetlb", t) + path := tempDir(t, "hugetlb") for _, pageSize := range HugePageSizes { - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ fmt.Sprintf(usage, pageSize): "bad", maxUsage: hugetlbMaxUsageContents, }) @@ -125,22 +126,22 @@ func TestHugetlbStatsBadUsageFile(t *testing.T) { hugetlb := &HugetlbGroup{} actualStats := *cgroups.NewStats() - err := hugetlb.GetStats(helper.CgroupPath, &actualStats) + err := hugetlb.GetStats(path, &actualStats) if err == nil { t.Fatal("Expected failure") } } func TestHugetlbStatsBadMaxUsageFile(t *testing.T) { - helper := NewCgroupTestUtil("hugetlb", t) - helper.writeFileContents(map[string]string{ + path := tempDir(t, "hugetlb") + writeFileContents(t, path, map[string]string{ usage: hugetlbUsageContents, maxUsage: "bad", }) hugetlb := &HugetlbGroup{} actualStats := *cgroups.NewStats() - err := hugetlb.GetStats(helper.CgroupPath, &actualStats) + err := hugetlb.GetStats(path, &actualStats) if err == nil { t.Fatal("Expected failure") } diff --git a/libcontainer/cgroups/fs/memory.go b/libcontainer/cgroups/fs/memory.go index a349ad4b636..b7c75f94138 100644 --- a/libcontainer/cgroups/fs/memory.go +++ b/libcontainer/cgroups/fs/memory.go @@ -30,8 +30,8 @@ func (s *MemoryGroup) Name() string { return "memory" } -func (s *MemoryGroup) Apply(path string, d *cgroupData) (err error) { - return apply(path, d.pid) +func (s *MemoryGroup) Apply(path string, _ *configs.Resources, pid int) error { + return apply(path, pid) } func setMemory(path string, val int64) error { diff --git a/libcontainer/cgroups/fs/memory_test.go b/libcontainer/cgroups/fs/memory_test.go index 7d13d250111..d305a62a393 100644 --- a/libcontainer/cgroups/fs/memory_test.go +++ b/libcontainer/cgroups/fs/memory_test.go @@ -6,6 +6,7 @@ import ( "github.com/opencontainers/runc/libcontainer/cgroups" "github.com/opencontainers/runc/libcontainer/cgroups/fscommon" + "github.com/opencontainers/runc/libcontainer/configs" ) const ( @@ -38,7 +39,7 @@ whatever=100 N0=0 ) func TestMemorySetMemory(t *testing.T) { - helper := NewCgroupTestUtil("memory", t) + path := tempDir(t, "memory") const ( memoryBefore = 314572800 // 300M @@ -47,19 +48,21 @@ func TestMemorySetMemory(t *testing.T) { reservationAfter = 314572800 // 300M ) - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "memory.limit_in_bytes": strconv.Itoa(memoryBefore), "memory.soft_limit_in_bytes": strconv.Itoa(reservationBefore), }) - helper.CgroupData.config.Resources.Memory = memoryAfter - helper.CgroupData.config.Resources.MemoryReservation = reservationAfter + r := &configs.Resources{ + Memory: memoryAfter, + MemoryReservation: reservationAfter, + } memory := &MemoryGroup{} - if err := memory.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + if err := memory.Set(path, r); err != nil { t.Fatal(err) } - value, err := fscommon.GetCgroupParamUint(helper.CgroupPath, "memory.limit_in_bytes") + value, err := fscommon.GetCgroupParamUint(path, "memory.limit_in_bytes") if err != nil { t.Fatal(err) } @@ -67,7 +70,7 @@ func TestMemorySetMemory(t *testing.T) { t.Fatal("Got the wrong value, set memory.limit_in_bytes failed.") } - value, err = fscommon.GetCgroupParamUint(helper.CgroupPath, "memory.soft_limit_in_bytes") + value, err = fscommon.GetCgroupParamUint(path, "memory.soft_limit_in_bytes") if err != nil { t.Fatal(err) } @@ -77,24 +80,26 @@ func TestMemorySetMemory(t *testing.T) { } func TestMemorySetMemoryswap(t *testing.T) { - helper := NewCgroupTestUtil("memory", t) + path := tempDir(t, "memory") const ( memoryswapBefore = 314572800 // 300M memoryswapAfter = 524288000 // 500M ) - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "memory.memsw.limit_in_bytes": strconv.Itoa(memoryswapBefore), }) - helper.CgroupData.config.Resources.MemorySwap = memoryswapAfter + r := &configs.Resources{ + MemorySwap: memoryswapAfter, + } memory := &MemoryGroup{} - if err := memory.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + if err := memory.Set(path, r); err != nil { t.Fatal(err) } - value, err := fscommon.GetCgroupParamUint(helper.CgroupPath, "memory.memsw.limit_in_bytes") + value, err := fscommon.GetCgroupParamUint(path, "memory.memsw.limit_in_bytes") if err != nil { t.Fatal(err) } @@ -104,7 +109,7 @@ func TestMemorySetMemoryswap(t *testing.T) { } func TestMemorySetMemoryLargerThanSwap(t *testing.T) { - helper := NewCgroupTestUtil("memory", t) + path := tempDir(t, "memory") const ( memoryBefore = 314572800 // 300M @@ -113,7 +118,7 @@ func TestMemorySetMemoryLargerThanSwap(t *testing.T) { memoryswapAfter = 838860800 // 800M ) - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "memory.limit_in_bytes": strconv.Itoa(memoryBefore), "memory.memsw.limit_in_bytes": strconv.Itoa(memoryswapBefore), // Set will call getMemoryData when memory and swap memory are @@ -123,14 +128,16 @@ func TestMemorySetMemoryLargerThanSwap(t *testing.T) { "memory.failcnt": "0", }) - helper.CgroupData.config.Resources.Memory = memoryAfter - helper.CgroupData.config.Resources.MemorySwap = memoryswapAfter + r := &configs.Resources{ + Memory: memoryAfter, + MemorySwap: memoryswapAfter, + } memory := &MemoryGroup{} - if err := memory.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + if err := memory.Set(path, r); err != nil { t.Fatal(err) } - value, err := fscommon.GetCgroupParamUint(helper.CgroupPath, "memory.limit_in_bytes") + value, err := fscommon.GetCgroupParamUint(path, "memory.limit_in_bytes") if err != nil { t.Fatal(err) } @@ -138,7 +145,7 @@ func TestMemorySetMemoryLargerThanSwap(t *testing.T) { t.Fatal("Got the wrong value, set memory.limit_in_bytes failed.") } - value, err = fscommon.GetCgroupParamUint(helper.CgroupPath, "memory.memsw.limit_in_bytes") + value, err = fscommon.GetCgroupParamUint(path, "memory.memsw.limit_in_bytes") if err != nil { t.Fatal(err) } @@ -148,7 +155,7 @@ func TestMemorySetMemoryLargerThanSwap(t *testing.T) { } func TestMemorySetSwapSmallerThanMemory(t *testing.T) { - helper := NewCgroupTestUtil("memory", t) + path := tempDir(t, "memory") const ( memoryBefore = 629145600 // 600M @@ -157,19 +164,21 @@ func TestMemorySetSwapSmallerThanMemory(t *testing.T) { memoryswapAfter = 524288000 // 500M ) - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "memory.limit_in_bytes": strconv.Itoa(memoryBefore), "memory.memsw.limit_in_bytes": strconv.Itoa(memoryswapBefore), }) - helper.CgroupData.config.Resources.Memory = memoryAfter - helper.CgroupData.config.Resources.MemorySwap = memoryswapAfter + r := &configs.Resources{ + Memory: memoryAfter, + MemorySwap: memoryswapAfter, + } memory := &MemoryGroup{} - if err := memory.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + if err := memory.Set(path, r); err != nil { t.Fatal(err) } - value, err := fscommon.GetCgroupParamUint(helper.CgroupPath, "memory.limit_in_bytes") + value, err := fscommon.GetCgroupParamUint(path, "memory.limit_in_bytes") if err != nil { t.Fatal(err) } @@ -177,7 +186,7 @@ func TestMemorySetSwapSmallerThanMemory(t *testing.T) { t.Fatalf("Got the wrong value (%d != %d), set memory.limit_in_bytes failed", value, memoryAfter) } - value, err = fscommon.GetCgroupParamUint(helper.CgroupPath, "memory.memsw.limit_in_bytes") + value, err = fscommon.GetCgroupParamUint(path, "memory.memsw.limit_in_bytes") if err != nil { t.Fatal(err) } @@ -187,22 +196,24 @@ func TestMemorySetSwapSmallerThanMemory(t *testing.T) { } func TestMemorySetMemorySwappinessDefault(t *testing.T) { - helper := NewCgroupTestUtil("memory", t) + path := tempDir(t, "memory") swappinessBefore := 60 // default is 60 swappinessAfter := uint64(0) - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "memory.swappiness": strconv.Itoa(swappinessBefore), }) - helper.CgroupData.config.Resources.MemorySwappiness = &swappinessAfter + r := &configs.Resources{ + MemorySwappiness: &swappinessAfter, + } memory := &MemoryGroup{} - if err := memory.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + if err := memory.Set(path, r); err != nil { t.Fatal(err) } - value, err := fscommon.GetCgroupParamUint(helper.CgroupPath, "memory.swappiness") + value, err := fscommon.GetCgroupParamUint(path, "memory.swappiness") if err != nil { t.Fatal(err) } @@ -212,8 +223,8 @@ func TestMemorySetMemorySwappinessDefault(t *testing.T) { } func TestMemoryStats(t *testing.T) { - helper := NewCgroupTestUtil("memory", t) - helper.writeFileContents(map[string]string{ + path := tempDir(t, "memory") + writeFileContents(t, path, map[string]string{ "memory.stat": memoryStatContents, "memory.usage_in_bytes": memoryUsageContents, "memory.limit_in_bytes": memoryLimitContents, @@ -233,7 +244,7 @@ func TestMemoryStats(t *testing.T) { memory := &MemoryGroup{} actualStats := *cgroups.NewStats() - err := memory.GetStats(helper.CgroupPath, &actualStats) + err := memory.GetStats(path, &actualStats) if err != nil { t.Fatal(err) } @@ -263,8 +274,8 @@ func TestMemoryStats(t *testing.T) { } func TestMemoryStatsNoStatFile(t *testing.T) { - helper := NewCgroupTestUtil("memory", t) - helper.writeFileContents(map[string]string{ + path := tempDir(t, "memory") + writeFileContents(t, path, map[string]string{ "memory.usage_in_bytes": memoryUsageContents, "memory.max_usage_in_bytes": memoryMaxUsageContents, "memory.limit_in_bytes": memoryLimitContents, @@ -272,15 +283,15 @@ func TestMemoryStatsNoStatFile(t *testing.T) { memory := &MemoryGroup{} actualStats := *cgroups.NewStats() - err := memory.GetStats(helper.CgroupPath, &actualStats) + err := memory.GetStats(path, &actualStats) if err != nil { t.Fatal(err) } } func TestMemoryStatsNoUsageFile(t *testing.T) { - helper := NewCgroupTestUtil("memory", t) - helper.writeFileContents(map[string]string{ + path := tempDir(t, "memory") + writeFileContents(t, path, map[string]string{ "memory.stat": memoryStatContents, "memory.max_usage_in_bytes": memoryMaxUsageContents, "memory.limit_in_bytes": memoryLimitContents, @@ -288,15 +299,15 @@ func TestMemoryStatsNoUsageFile(t *testing.T) { memory := &MemoryGroup{} actualStats := *cgroups.NewStats() - err := memory.GetStats(helper.CgroupPath, &actualStats) + err := memory.GetStats(path, &actualStats) if err == nil { t.Fatal("Expected failure") } } func TestMemoryStatsNoMaxUsageFile(t *testing.T) { - helper := NewCgroupTestUtil("memory", t) - helper.writeFileContents(map[string]string{ + path := tempDir(t, "memory") + writeFileContents(t, path, map[string]string{ "memory.stat": memoryStatContents, "memory.usage_in_bytes": memoryUsageContents, "memory.limit_in_bytes": memoryLimitContents, @@ -304,15 +315,15 @@ func TestMemoryStatsNoMaxUsageFile(t *testing.T) { memory := &MemoryGroup{} actualStats := *cgroups.NewStats() - err := memory.GetStats(helper.CgroupPath, &actualStats) + err := memory.GetStats(path, &actualStats) if err == nil { t.Fatal("Expected failure") } } func TestMemoryStatsNoLimitInBytesFile(t *testing.T) { - helper := NewCgroupTestUtil("memory", t) - helper.writeFileContents(map[string]string{ + path := tempDir(t, "memory") + writeFileContents(t, path, map[string]string{ "memory.stat": memoryStatContents, "memory.usage_in_bytes": memoryUsageContents, "memory.max_usage_in_bytes": memoryMaxUsageContents, @@ -320,15 +331,15 @@ func TestMemoryStatsNoLimitInBytesFile(t *testing.T) { memory := &MemoryGroup{} actualStats := *cgroups.NewStats() - err := memory.GetStats(helper.CgroupPath, &actualStats) + err := memory.GetStats(path, &actualStats) if err == nil { t.Fatal("Expected failure") } } func TestMemoryStatsBadStatFile(t *testing.T) { - helper := NewCgroupTestUtil("memory", t) - helper.writeFileContents(map[string]string{ + path := tempDir(t, "memory") + writeFileContents(t, path, map[string]string{ "memory.stat": "rss rss", "memory.usage_in_bytes": memoryUsageContents, "memory.max_usage_in_bytes": memoryMaxUsageContents, @@ -337,15 +348,15 @@ func TestMemoryStatsBadStatFile(t *testing.T) { memory := &MemoryGroup{} actualStats := *cgroups.NewStats() - err := memory.GetStats(helper.CgroupPath, &actualStats) + err := memory.GetStats(path, &actualStats) if err == nil { t.Fatal("Expected failure") } } func TestMemoryStatsBadUsageFile(t *testing.T) { - helper := NewCgroupTestUtil("memory", t) - helper.writeFileContents(map[string]string{ + path := tempDir(t, "memory") + writeFileContents(t, path, map[string]string{ "memory.stat": memoryStatContents, "memory.usage_in_bytes": "bad", "memory.max_usage_in_bytes": memoryMaxUsageContents, @@ -354,15 +365,15 @@ func TestMemoryStatsBadUsageFile(t *testing.T) { memory := &MemoryGroup{} actualStats := *cgroups.NewStats() - err := memory.GetStats(helper.CgroupPath, &actualStats) + err := memory.GetStats(path, &actualStats) if err == nil { t.Fatal("Expected failure") } } func TestMemoryStatsBadMaxUsageFile(t *testing.T) { - helper := NewCgroupTestUtil("memory", t) - helper.writeFileContents(map[string]string{ + path := tempDir(t, "memory") + writeFileContents(t, path, map[string]string{ "memory.stat": memoryStatContents, "memory.usage_in_bytes": memoryUsageContents, "memory.max_usage_in_bytes": "bad", @@ -371,15 +382,15 @@ func TestMemoryStatsBadMaxUsageFile(t *testing.T) { memory := &MemoryGroup{} actualStats := *cgroups.NewStats() - err := memory.GetStats(helper.CgroupPath, &actualStats) + err := memory.GetStats(path, &actualStats) if err == nil { t.Fatal("Expected failure") } } func TestMemoryStatsBadLimitInBytesFile(t *testing.T) { - helper := NewCgroupTestUtil("memory", t) - helper.writeFileContents(map[string]string{ + path := tempDir(t, "memory") + writeFileContents(t, path, map[string]string{ "memory.stat": memoryStatContents, "memory.usage_in_bytes": memoryUsageContents, "memory.max_usage_in_bytes": memoryMaxUsageContents, @@ -388,29 +399,30 @@ func TestMemoryStatsBadLimitInBytesFile(t *testing.T) { memory := &MemoryGroup{} actualStats := *cgroups.NewStats() - err := memory.GetStats(helper.CgroupPath, &actualStats) + err := memory.GetStats(path, &actualStats) if err == nil { t.Fatal("Expected failure") } } func TestMemorySetOomControl(t *testing.T) { - helper := NewCgroupTestUtil("memory", t) + path := tempDir(t, "memory") const ( oomKillDisable = 1 // disable oom killer, default is 0 ) - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "memory.oom_control": strconv.Itoa(oomKillDisable), }) memory := &MemoryGroup{} - if err := memory.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + r := &configs.Resources{} + if err := memory.Set(path, r); err != nil { t.Fatal(err) } - value, err := fscommon.GetCgroupParamUint(helper.CgroupPath, "memory.oom_control") + value, err := fscommon.GetCgroupParamUint(path, "memory.oom_control") if err != nil { t.Fatal(err) } @@ -420,12 +432,12 @@ func TestMemorySetOomControl(t *testing.T) { } func TestNoHierarchicalNumaStat(t *testing.T) { - helper := NewCgroupTestUtil("memory", t) - helper.writeFileContents(map[string]string{ + path := tempDir(t, "memory") + writeFileContents(t, path, map[string]string{ "memory.numa_stat": memoryNUMAStatNoHierarchyContents + memoryNUMAStatExtraContents, }) - actualStats, err := getPageUsageByNUMA(helper.CgroupPath) + actualStats, err := getPageUsageByNUMA(path) if err != nil { t.Fatal(err) } @@ -470,13 +482,13 @@ anon=183 N0=12 badone `, }, } - helper := NewCgroupTestUtil("memory", t) + path := tempDir(t, "memory") for _, c := range memoryNUMAStatBadContents { - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "memory.numa_stat": c.contents, }) - _, err := getPageUsageByNUMA(helper.CgroupPath) + _, err := getPageUsageByNUMA(path) if err == nil { t.Errorf("case %q: expected error, got nil", c.desc) } @@ -484,9 +496,9 @@ anon=183 N0=12 badone } func TestWithoutNumaStat(t *testing.T) { - helper := NewCgroupTestUtil("memory", t) + path := tempDir(t, "memory") - actualStats, err := getPageUsageByNUMA(helper.CgroupPath) + actualStats, err := getPageUsageByNUMA(path) if err != nil { t.Fatal(err) } diff --git a/libcontainer/cgroups/fs/name.go b/libcontainer/cgroups/fs/name.go index 056bf825d9e..b8d5d849c52 100644 --- a/libcontainer/cgroups/fs/name.go +++ b/libcontainer/cgroups/fs/name.go @@ -14,10 +14,10 @@ func (s *NameGroup) Name() string { return s.GroupName } -func (s *NameGroup) Apply(path string, d *cgroupData) error { +func (s *NameGroup) Apply(path string, _ *configs.Resources, pid int) error { if s.Join { - // ignore errors if the named cgroup does not exist - _ = apply(path, d.pid) + // Ignore errors if the named cgroup does not exist. + _ = apply(path, pid) } return nil } diff --git a/libcontainer/cgroups/fs/net_cls.go b/libcontainer/cgroups/fs/net_cls.go index ef9514fb86d..abfd09ce83a 100644 --- a/libcontainer/cgroups/fs/net_cls.go +++ b/libcontainer/cgroups/fs/net_cls.go @@ -13,8 +13,8 @@ func (s *NetClsGroup) Name() string { return "net_cls" } -func (s *NetClsGroup) Apply(path string, d *cgroupData) error { - return apply(path, d.pid) +func (s *NetClsGroup) Apply(path string, _ *configs.Resources, pid int) error { + return apply(path, pid) } func (s *NetClsGroup) Set(path string, r *configs.Resources) error { diff --git a/libcontainer/cgroups/fs/net_cls_test.go b/libcontainer/cgroups/fs/net_cls_test.go index 6c36f055597..085c0615123 100644 --- a/libcontainer/cgroups/fs/net_cls_test.go +++ b/libcontainer/cgroups/fs/net_cls_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/opencontainers/runc/libcontainer/cgroups/fscommon" + "github.com/opencontainers/runc/libcontainer/configs" ) const ( @@ -13,22 +14,24 @@ const ( ) func TestNetClsSetClassid(t *testing.T) { - helper := NewCgroupTestUtil("net_cls", t) + path := tempDir(t, "net_cls") - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "net_cls.classid": strconv.FormatUint(classidBefore, 10), }) - helper.CgroupData.config.Resources.NetClsClassid = classidAfter + r := &configs.Resources{ + NetClsClassid: classidAfter, + } netcls := &NetClsGroup{} - if err := netcls.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + if err := netcls.Set(path, r); err != nil { t.Fatal(err) } // As we are in mock environment, we can't get correct value of classid from // net_cls.classid. // So. we just judge if we successfully write classid into file - value, err := fscommon.GetCgroupParamUint(helper.CgroupPath, "net_cls.classid") + value, err := fscommon.GetCgroupParamUint(path, "net_cls.classid") if err != nil { t.Fatal(err) } diff --git a/libcontainer/cgroups/fs/net_prio.go b/libcontainer/cgroups/fs/net_prio.go index 38c26989a66..da74d377989 100644 --- a/libcontainer/cgroups/fs/net_prio.go +++ b/libcontainer/cgroups/fs/net_prio.go @@ -11,8 +11,8 @@ func (s *NetPrioGroup) Name() string { return "net_prio" } -func (s *NetPrioGroup) Apply(path string, d *cgroupData) error { - return apply(path, d.pid) +func (s *NetPrioGroup) Apply(path string, _ *configs.Resources, pid int) error { + return apply(path, pid) } func (s *NetPrioGroup) Set(path string, r *configs.Resources) error { diff --git a/libcontainer/cgroups/fs/net_prio_test.go b/libcontainer/cgroups/fs/net_prio_test.go index 7983041a41e..453ff363b81 100644 --- a/libcontainer/cgroups/fs/net_prio_test.go +++ b/libcontainer/cgroups/fs/net_prio_test.go @@ -16,15 +16,17 @@ var prioMap = []*configs.IfPrioMap{ } func TestNetPrioSetIfPrio(t *testing.T) { - helper := NewCgroupTestUtil("net_prio", t) + path := tempDir(t, "net_prio") - helper.CgroupData.config.Resources.NetPrioIfpriomap = prioMap + r := &configs.Resources{ + NetPrioIfpriomap: prioMap, + } netPrio := &NetPrioGroup{} - if err := netPrio.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + if err := netPrio.Set(path, r); err != nil { t.Fatal(err) } - value, err := fscommon.GetCgroupParamString(helper.CgroupPath, "net_prio.ifpriomap") + value, err := fscommon.GetCgroupParamString(path, "net_prio.ifpriomap") if err != nil { t.Fatal(err) } diff --git a/libcontainer/cgroups/fs/paths.go b/libcontainer/cgroups/fs/paths.go index 9556f8b4c09..fb4ff8b80d5 100644 --- a/libcontainer/cgroups/fs/paths.go +++ b/libcontainer/cgroups/fs/paths.go @@ -69,8 +69,8 @@ func tryDefaultCgroupRoot() string { return defaultCgroupRoot } -// Gets the cgroupRoot. -func getCgroupRoot() (string, error) { +// rootPath finds and returns path to the root of the cgroup hierarchies. +func rootPath() (string, error) { cgroupRootLock.Lock() defer cgroupRootLock.Unlock() @@ -105,21 +105,9 @@ func getCgroupRoot() (string, error) { return cgroupRoot, nil } -type cgroupData struct { - root string - innerPath string - config *configs.Cgroup - pid int -} - -func getCgroupData(c *configs.Cgroup, pid int) (*cgroupData, error) { - root, err := getCgroupRoot() - if err != nil { - return nil, err - } - +func innerPath(c *configs.Cgroup) (string, error) { if (c.Name != "" || c.Parent != "") && c.Path != "" { - return nil, errors.New("cgroup: either Path or Name and Parent should be used") + return "", errors.New("cgroup: either Path or Name and Parent should be used") } // XXX: Do not remove CleanPath. Path safety is important! -- cyphar @@ -130,25 +118,20 @@ func getCgroupData(c *configs.Cgroup, pid int) (*cgroupData, error) { innerPath = filepath.Join(cgParent, cgName) } - return &cgroupData{ - root: root, - innerPath: innerPath, - config: c, - pid: pid, - }, nil + return innerPath, nil } -func (raw *cgroupData) path(subsystem string) (string, error) { +func subsysPath(root, inner, subsystem string) (string, error) { // If the cgroup name/path is absolute do not look relative to the cgroup of the init process. - if filepath.IsAbs(raw.innerPath) { - mnt, err := cgroups.FindCgroupMountpoint(raw.root, subsystem) + if filepath.IsAbs(inner) { + mnt, err := cgroups.FindCgroupMountpoint(root, subsystem) // If we didn't mount the subsystem, there is no point we make the path. if err != nil { return "", err } // Sometimes subsystems can be mounted together as 'cpu,cpuacct'. - return filepath.Join(raw.root, filepath.Base(mnt), raw.innerPath), nil + return filepath.Join(root, filepath.Base(mnt), inner), nil } // Use GetOwnCgroupPath instead of GetInitCgroupPath, because the creating @@ -159,7 +142,7 @@ func (raw *cgroupData) path(subsystem string) (string, error) { return "", err } - return filepath.Join(parentPath, raw.innerPath), nil + return filepath.Join(parentPath, inner), nil } func apply(path string, pid int) error { diff --git a/libcontainer/cgroups/fs/paths_test.go b/libcontainer/cgroups/fs/paths_test.go index 58ebfc7c410..3a4d45fc212 100644 --- a/libcontainer/cgroups/fs/paths_test.go +++ b/libcontainer/cgroups/fs/paths_test.go @@ -14,7 +14,7 @@ func TestInvalidCgroupPath(t *testing.T) { t.Skip("cgroup v2 is not supported") } - root, err := getCgroupRoot() + root, err := rootPath() if err != nil { t.Fatalf("couldn't get cgroup root: %v", err) } @@ -67,19 +67,19 @@ func TestInvalidCgroupPath(t *testing.T) { t.Run(tc.test, func(t *testing.T) { config := &configs.Cgroup{Path: tc.path, Name: tc.name, Parent: tc.parent} - data, err := getCgroupData(config, 0) + inner, err := innerPath(config) if err != nil { t.Fatalf("couldn't get cgroup data: %v", err) } - // Make sure the final innerPath doesn't go outside the cgroup mountpoint. - if strings.HasPrefix(data.innerPath, "..") { + // Make sure the final inner path doesn't go outside the cgroup mountpoint. + if strings.HasPrefix(inner, "..") { t.Errorf("SECURITY: cgroup innerPath is outside cgroup mountpoint!") } // Double-check, using an actual cgroup. deviceRoot := filepath.Join(root, "devices") - devicePath, err := data.path("devices") + devicePath, err := subsysPath(root, inner, "devices") if err != nil { t.Fatalf("couldn't get cgroup path: %v", err) } diff --git a/libcontainer/cgroups/fs/perf_event.go b/libcontainer/cgroups/fs/perf_event.go index 486c7fe45be..b86955c8f31 100644 --- a/libcontainer/cgroups/fs/perf_event.go +++ b/libcontainer/cgroups/fs/perf_event.go @@ -11,8 +11,8 @@ func (s *PerfEventGroup) Name() string { return "perf_event" } -func (s *PerfEventGroup) Apply(path string, d *cgroupData) error { - return apply(path, d.pid) +func (s *PerfEventGroup) Apply(path string, _ *configs.Resources, pid int) error { + return apply(path, pid) } func (s *PerfEventGroup) Set(_ string, _ *configs.Resources) error { diff --git a/libcontainer/cgroups/fs/pids.go b/libcontainer/cgroups/fs/pids.go index 33bb64c9b83..1f13532a5ad 100644 --- a/libcontainer/cgroups/fs/pids.go +++ b/libcontainer/cgroups/fs/pids.go @@ -15,8 +15,8 @@ func (s *PidsGroup) Name() string { return "pids" } -func (s *PidsGroup) Apply(path string, d *cgroupData) error { - return apply(path, d.pid) +func (s *PidsGroup) Apply(path string, _ *configs.Resources, pid int) error { + return apply(path, pid) } func (s *PidsGroup) Set(path string, r *configs.Resources) error { diff --git a/libcontainer/cgroups/fs/pids_test.go b/libcontainer/cgroups/fs/pids_test.go index d05ec7215e6..9d9a7ce8f18 100644 --- a/libcontainer/cgroups/fs/pids_test.go +++ b/libcontainer/cgroups/fs/pids_test.go @@ -6,6 +6,7 @@ import ( "github.com/opencontainers/runc/libcontainer/cgroups" "github.com/opencontainers/runc/libcontainer/cgroups/fscommon" + "github.com/opencontainers/runc/libcontainer/configs" ) const ( @@ -14,19 +15,21 @@ const ( ) func TestPidsSetMax(t *testing.T) { - helper := NewCgroupTestUtil("pids", t) + path := tempDir(t, "pids") - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "pids.max": "max", }) - helper.CgroupData.config.Resources.PidsLimit = maxLimited + r := &configs.Resources{ + PidsLimit: maxLimited, + } pids := &PidsGroup{} - if err := pids.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + if err := pids.Set(path, r); err != nil { t.Fatal(err) } - value, err := fscommon.GetCgroupParamUint(helper.CgroupPath, "pids.max") + value, err := fscommon.GetCgroupParamUint(path, "pids.max") if err != nil { t.Fatal(err) } @@ -36,19 +39,21 @@ func TestPidsSetMax(t *testing.T) { } func TestPidsSetUnlimited(t *testing.T) { - helper := NewCgroupTestUtil("pids", t) + path := tempDir(t, "pids") - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "pids.max": strconv.Itoa(maxLimited), }) - helper.CgroupData.config.Resources.PidsLimit = maxUnlimited + r := &configs.Resources{ + PidsLimit: maxUnlimited, + } pids := &PidsGroup{} - if err := pids.Set(helper.CgroupPath, helper.CgroupData.config.Resources); err != nil { + if err := pids.Set(path, r); err != nil { t.Fatal(err) } - value, err := fscommon.GetCgroupParamString(helper.CgroupPath, "pids.max") + value, err := fscommon.GetCgroupParamString(path, "pids.max") if err != nil { t.Fatal(err) } @@ -58,16 +63,16 @@ func TestPidsSetUnlimited(t *testing.T) { } func TestPidsStats(t *testing.T) { - helper := NewCgroupTestUtil("pids", t) + path := tempDir(t, "pids") - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "pids.current": strconv.Itoa(1337), "pids.max": strconv.Itoa(maxLimited), }) pids := &PidsGroup{} stats := *cgroups.NewStats() - if err := pids.GetStats(helper.CgroupPath, &stats); err != nil { + if err := pids.GetStats(path, &stats); err != nil { t.Fatal(err) } @@ -81,16 +86,16 @@ func TestPidsStats(t *testing.T) { } func TestPidsStatsUnlimited(t *testing.T) { - helper := NewCgroupTestUtil("pids", t) + path := tempDir(t, "pids") - helper.writeFileContents(map[string]string{ + writeFileContents(t, path, map[string]string{ "pids.current": strconv.Itoa(4096), "pids.max": "max", }) pids := &PidsGroup{} stats := *cgroups.NewStats() - if err := pids.GetStats(helper.CgroupPath, &stats); err != nil { + if err := pids.GetStats(path, &stats); err != nil { t.Fatal(err) } diff --git a/libcontainer/cgroups/fs/rdma.go b/libcontainer/cgroups/fs/rdma.go index 3548db3b6e0..5bbe0f35f81 100644 --- a/libcontainer/cgroups/fs/rdma.go +++ b/libcontainer/cgroups/fs/rdma.go @@ -12,8 +12,8 @@ func (s *RdmaGroup) Name() string { return "rdma" } -func (s *RdmaGroup) Apply(path string, d *cgroupData) error { - return apply(path, d.pid) +func (s *RdmaGroup) Apply(path string, _ *configs.Resources, pid int) error { + return apply(path, pid) } func (s *RdmaGroup) Set(path string, r *configs.Resources) error { diff --git a/libcontainer/cgroups/fs/util_test.go b/libcontainer/cgroups/fs/util_test.go index 4a4013774df..85842b73532 100644 --- a/libcontainer/cgroups/fs/util_test.go +++ b/libcontainer/cgroups/fs/util_test.go @@ -11,45 +11,29 @@ import ( "testing" "github.com/opencontainers/runc/libcontainer/cgroups" - "github.com/opencontainers/runc/libcontainer/configs" ) func init() { cgroups.TestMode = true } -type cgroupTestUtil struct { - // cgroup data to use in tests. - CgroupData *cgroupData - - // Path to the mock cgroup directory. - CgroupPath string - - t *testing.T -} - -// Creates a new test util for the specified subsystem -func NewCgroupTestUtil(subsystem string, t *testing.T) *cgroupTestUtil { - d := &cgroupData{ - config: &configs.Cgroup{ - Resources: &configs.Resources{}, - }, - root: t.TempDir(), - } - testCgroupPath := filepath.Join(d.root, subsystem) +// tempDir creates a new test directory for the specified subsystem. +func tempDir(t *testing.T, subsystem string) string { + path := filepath.Join(t.TempDir(), subsystem) // Ensure the full mock cgroup path exists. - if err := os.MkdirAll(testCgroupPath, 0o755); err != nil { + if err := os.Mkdir(path, 0o755); err != nil { t.Fatal(err) } - return &cgroupTestUtil{CgroupData: d, CgroupPath: testCgroupPath, t: t} + return path } -// Write the specified contents on the mock of the specified cgroup files. -func (c *cgroupTestUtil) writeFileContents(fileContents map[string]string) { +// writeFileContents writes the specified contents on the mock of the specified +// cgroup files. +func writeFileContents(t *testing.T, path string, fileContents map[string]string) { for file, contents := range fileContents { - err := cgroups.WriteFile(c.CgroupPath, file, contents) + err := cgroups.WriteFile(path, file, contents) if err != nil { - c.t.Fatal(err) + t.Fatal(err) } } }