From 097c6d7425808159ca1d69d565424c4fd82bd440 Mon Sep 17 00:00:00 2001
From: Kir Kolyshkin <kolyshkin@gmail.com>
Date: Sat, 7 Aug 2021 12:22:16 -0700
Subject: [PATCH] libct/cg: simplify getting cgroup manager

1. Make Rootless and Systemd flags part of config.Cgroups.

2. Make all cgroup managers (not just fs2) return error (so it can do
   more initialization -- added by the following commits).

3. Replace complicated cgroup manager instantiation in factory_linux
   by a single (and simple) libcontainer/cgroups/manager.New() function.

4. getUnifiedPath is simplified to check that only a single path is
   supplied (rather than checking that other paths, if supplied,
   are the same).

[v2: can't -> cannot]

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
---
 contrib/cmd/sd-helper/helper.go              |   9 +-
 libcontainer/cgroups/fs/fs.go                |  24 ++--
 libcontainer/cgroups/fs/fs_test.go           |   7 +-
 libcontainer/cgroups/fs2/fs2.go              |  23 ++--
 libcontainer/cgroups/manager/new.go          |  78 ++++++++++++
 libcontainer/cgroups/systemd/systemd_test.go |  52 ++++----
 libcontainer/cgroups/systemd/v1.go           |   7 +-
 libcontainer/cgroups/systemd/v1_test.go      |   5 +-
 libcontainer/cgroups/systemd/v2.go           |  24 ++--
 libcontainer/configs/cgroup_linux.go         |   6 +
 libcontainer/factory_linux.go                | 123 ++-----------------
 libcontainer/factory_linux_test.go           |  15 +--
 libcontainer/integration/checkpoint_test.go  |   2 +-
 libcontainer/integration/template_test.go    |   2 +-
 libcontainer/integration/utils_test.go       |   8 +-
 libcontainer/specconv/spec_linux.go          |   2 +
 utils_linux.go                               |  23 +---
 17 files changed, 188 insertions(+), 222 deletions(-)
 create mode 100644 libcontainer/cgroups/manager/new.go

diff --git a/contrib/cmd/sd-helper/helper.go b/contrib/cmd/sd-helper/helper.go
index 156125248a8..fc2bf38af67 100644
--- a/contrib/cmd/sd-helper/helper.go
+++ b/contrib/cmd/sd-helper/helper.go
@@ -57,9 +57,9 @@ func main() {
 	}
 }
 
-func newManager(config *configs.Cgroup) cgroups.Manager {
+func newManager(config *configs.Cgroup) (cgroups.Manager, error) {
 	if cgroups.IsCgroup2UnifiedMode() {
-		return systemd.NewUnifiedManager(config, "", false)
+		return systemd.NewUnifiedManager(config, "")
 	}
 	return systemd.NewLegacyManager(config, nil)
 }
@@ -70,7 +70,10 @@ func unitCommand(cmd, name, parent string) error {
 		Parent:    parent,
 		Resources: &configs.Resources{},
 	}
-	pm := newManager(podConfig)
+	pm, err := newManager(podConfig)
+	if err != nil {
+		return err
+	}
 
 	switch cmd {
 	case "start":
diff --git a/libcontainer/cgroups/fs/fs.go b/libcontainer/cgroups/fs/fs.go
index 36e0c76c447..e40abb45b31 100644
--- a/libcontainer/cgroups/fs/fs.go
+++ b/libcontainer/cgroups/fs/fs.go
@@ -49,18 +49,16 @@ type subsystem interface {
 }
 
 type manager struct {
-	mu       sync.Mutex
-	cgroups  *configs.Cgroup
-	rootless bool // ignore permission-related errors
-	paths    map[string]string
+	mu      sync.Mutex
+	cgroups *configs.Cgroup
+	paths   map[string]string
 }
 
-func NewManager(cg *configs.Cgroup, paths map[string]string, rootless bool) cgroups.Manager {
+func NewManager(cg *configs.Cgroup, paths map[string]string) (cgroups.Manager, error) {
 	return &manager{
-		cgroups:  cg,
-		paths:    paths,
-		rootless: rootless,
-	}
+		cgroups: cg,
+		paths:   paths,
+	}, nil
 }
 
 // isIgnorableError returns whether err is a permission error (in the loose
@@ -121,7 +119,7 @@ func (m *manager) Apply(pid int) (err error) {
 			// explicit cgroup path hasn't been set, we don't bail on error in
 			// case of permission problems. Cases where limits have been set
 			// (and we couldn't create our own cgroup) are handled by Set.
-			if isIgnorableError(m.rootless, err) && m.cgroups.Path == "" {
+			if isIgnorableError(c.Rootless, err) && m.cgroups.Path == "" {
 				delete(m.paths, sys.Name())
 				continue
 			}
@@ -174,10 +172,10 @@ func (m *manager) Set(r *configs.Resources) error {
 	for _, sys := range subsystems {
 		path := m.paths[sys.Name()]
 		if err := sys.Set(path, r); err != nil {
-			if m.rootless && sys.Name() == "devices" {
+			if m.cgroups.Rootless && sys.Name() == "devices" {
 				continue
 			}
-			// When m.rootless is true, errors from the device subsystem are ignored because it is really not expected to work.
+			// When rootless is true, errors from the device subsystem are ignored because it is really not expected to work.
 			// However, errors from other subsystems are not ignored.
 			// see @test "runc create (rootless + limits + no cgrouppath + no permission) fails with informative error"
 			if path == "" {
@@ -249,7 +247,7 @@ func OOMKillCount(path string) (uint64, error) {
 func (m *manager) OOMKillCount() (uint64, error) {
 	c, err := OOMKillCount(m.Path("memory"))
 	// Ignore ENOENT when rootless as it couldn't create cgroup.
-	if err != nil && m.rootless && os.IsNotExist(err) {
+	if err != nil && m.cgroups.Rootless && os.IsNotExist(err) {
 		err = nil
 	}
 
diff --git a/libcontainer/cgroups/fs/fs_test.go b/libcontainer/cgroups/fs/fs_test.go
index 7a91768602b..01293ad1267 100644
--- a/libcontainer/cgroups/fs/fs_test.go
+++ b/libcontainer/cgroups/fs/fs_test.go
@@ -23,8 +23,11 @@ func BenchmarkGetStats(b *testing.B) {
 		Path:      "/some/kind/of/a/path/here",
 		Resources: &configs.Resources{},
 	}
-	m := NewManager(cg, nil, false)
-	err := m.Apply(-1)
+	m, err := NewManager(cg, nil)
+	if err != nil {
+		b.Fatal(err)
+	}
+	err = m.Apply(-1)
 	if err != nil {
 		b.Fatal(err)
 	}
diff --git a/libcontainer/cgroups/fs2/fs2.go b/libcontainer/cgroups/fs2/fs2.go
index e6d38408cc7..067d9791aa0 100644
--- a/libcontainer/cgroups/fs2/fs2.go
+++ b/libcontainer/cgroups/fs2/fs2.go
@@ -20,16 +20,12 @@ type manager struct {
 	// controllers is content of "cgroup.controllers" file.
 	// excludes pseudo-controllers ("devices" and "freezer").
 	controllers map[string]struct{}
-	rootless    bool
 }
 
 // NewManager creates a manager for cgroup v2 unified hierarchy.
 // dirPath is like "/sys/fs/cgroup/user.slice/user-1001.slice/session-1.scope".
 // If dirPath is empty, it is automatically set using config.
-func NewManager(config *configs.Cgroup, dirPath string, rootless bool) (cgroups.Manager, error) {
-	if config == nil {
-		config = &configs.Cgroup{}
-	}
+func NewManager(config *configs.Cgroup, dirPath string) (cgroups.Manager, error) {
 	if dirPath == "" {
 		var err error
 		dirPath, err = defaultDirPath(config)
@@ -39,9 +35,8 @@ func NewManager(config *configs.Cgroup, dirPath string, rootless bool) (cgroups.
 	}
 
 	m := &manager{
-		config:   config,
-		dirPath:  dirPath,
-		rootless: rootless,
+		config:  config,
+		dirPath: dirPath,
 	}
 	return m, nil
 }
@@ -53,7 +48,7 @@ func (m *manager) getControllers() error {
 
 	data, err := cgroups.ReadFile(m.dirPath, "cgroup.controllers")
 	if err != nil {
-		if m.rootless && m.config.Path == "" {
+		if m.config.Rootless && m.config.Path == "" {
 			return nil
 		}
 		return err
@@ -73,7 +68,7 @@ func (m *manager) Apply(pid int) error {
 		// - "runc create (no limits + no cgrouppath + no permission) succeeds"
 		// - "runc create (rootless + no limits + cgrouppath + no permission) fails with permission error"
 		// - "runc create (rootless + limits + no cgrouppath + no permission) fails with informative error"
-		if m.rootless {
+		if m.config.Rootless {
 			if m.config.Path == "" {
 				if blNeed, nErr := needAnyControllers(m.config.Resources); nErr == nil && !blNeed {
 					return nil
@@ -127,7 +122,7 @@ func (m *manager) GetStats() (*cgroups.Stats, error) {
 	if err := fscommon.RdmaGetStats(m.dirPath, st); err != nil && !os.IsNotExist(err) {
 		errs = append(errs, err)
 	}
-	if len(errs) > 0 && !m.rootless {
+	if len(errs) > 0 && !m.config.Rootless {
 		return st, fmt.Errorf("error while statting cgroup v2: %+v", errs)
 	}
 	return st, nil
@@ -171,10 +166,10 @@ func (m *manager) Set(r *configs.Resources) error {
 	}
 	// devices (since kernel 4.15, pseudo-controller)
 	//
-	// When m.rootless is true, errors from the device subsystem are ignored because it is really not expected to work.
+	// When rootless is true, errors from the device subsystem are ignored because it is really not expected to work.
 	// However, errors from other subsystems are not ignored.
 	// see @test "runc create (rootless + limits + no cgrouppath + no permission) fails with informative error"
-	if err := setDevices(m.dirPath, r); err != nil && !m.rootless {
+	if err := setDevices(m.dirPath, r); err != nil && !m.config.Rootless {
 		return err
 	}
 	// cpuset (since kernel 5.0)
@@ -250,7 +245,7 @@ func OOMKillCount(path string) (uint64, error) {
 
 func (m *manager) OOMKillCount() (uint64, error) {
 	c, err := OOMKillCount(m.dirPath)
-	if err != nil && m.rootless && os.IsNotExist(err) {
+	if err != nil && m.config.Rootless && os.IsNotExist(err) {
 		err = nil
 	}
 
diff --git a/libcontainer/cgroups/manager/new.go b/libcontainer/cgroups/manager/new.go
new file mode 100644
index 00000000000..5df120d0f09
--- /dev/null
+++ b/libcontainer/cgroups/manager/new.go
@@ -0,0 +1,78 @@
+package manager
+
+import (
+	"errors"
+	"fmt"
+	"path/filepath"
+
+	"github.com/opencontainers/runc/libcontainer/cgroups"
+	"github.com/opencontainers/runc/libcontainer/cgroups/fs"
+	"github.com/opencontainers/runc/libcontainer/cgroups/fs2"
+	"github.com/opencontainers/runc/libcontainer/cgroups/systemd"
+	"github.com/opencontainers/runc/libcontainer/configs"
+)
+
+// New returns the instance of a cgroup manager, which is chosen
+// based on the local environment (whether cgroup v1 or v2 is used)
+// and the config (whether config.Systemd is set or not).
+func New(config *configs.Cgroup) (cgroups.Manager, error) {
+	return NewWithPaths(config, nil)
+}
+
+// NewWithPaths is similar to New, and can be used in case cgroup paths
+// are already well known, which can save some resources.
+//
+// For cgroup v1, the keys are controller/subsystem name, and the values
+// are absolute filesystem paths to the appropriate cgroups.
+//
+// For cgroup v2, the only key allowed is "" (empty string), and the value
+// is the unified cgroup path.
+func NewWithPaths(config *configs.Cgroup, paths map[string]string) (cgroups.Manager, error) {
+	if config == nil {
+		return nil, errors.New("cgroups/manager.New: config must not be nil")
+	}
+	if config.Systemd && !systemd.IsRunningSystemd() {
+		return nil, errors.New("systemd not running on this host, cannot use systemd cgroups manager")
+	}
+
+	// Cgroup v2 aka unified hierarchy.
+	if cgroups.IsCgroup2UnifiedMode() {
+		path, err := getUnifiedPath(paths)
+		if err != nil {
+			return nil, fmt.Errorf("manager.NewWithPaths: inconsistent paths: %w", err)
+		}
+		if config.Systemd {
+			return systemd.NewUnifiedManager(config, path)
+		}
+		return fs2.NewManager(config, path)
+	}
+
+	// Cgroup v1.
+	if config.Systemd {
+		return systemd.NewLegacyManager(config, paths)
+	}
+
+	return fs.NewManager(config, paths)
+}
+
+// getUnifiedPath is an implementation detail of libcontainer factory.
+// Historically, it saves cgroup paths as per-subsystem path map (as returned
+// by cm.GetPaths(""), but with v2 we only have one single unified path
+// (with "" as a key).
+//
+// This function converts from that map to string (using "" as a key),
+// and also checks that the map itself is sane.
+func getUnifiedPath(paths map[string]string) (string, error) {
+	if len(paths) > 1 {
+		return "", fmt.Errorf("expected a single path, got %+v", paths)
+	}
+	path := paths[""]
+	// can be empty
+	if path != "" {
+		if filepath.Clean(path) != path || !filepath.IsAbs(path) {
+			return "", fmt.Errorf("invalid path: %q", path)
+		}
+	}
+
+	return path, nil
+}
diff --git a/libcontainer/cgroups/systemd/systemd_test.go b/libcontainer/cgroups/systemd/systemd_test.go
index 6976c4ca604..7417bf28789 100644
--- a/libcontainer/cgroups/systemd/systemd_test.go
+++ b/libcontainer/cgroups/systemd/systemd_test.go
@@ -13,6 +13,23 @@ import (
 	"github.com/opencontainers/runc/libcontainer/devices"
 )
 
+func newManager(t *testing.T, config *configs.Cgroup) (m cgroups.Manager) {
+	t.Helper()
+	var err error
+
+	if cgroups.IsCgroup2UnifiedMode() {
+		m, err = NewUnifiedManager(config, "")
+	} else {
+		m, err = NewLegacyManager(config, nil)
+	}
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(func() { _ = m.Destroy() })
+
+	return m
+}
+
 func TestSystemdVersion(t *testing.T) {
 	systemdVersionTests := []struct {
 		verStr      string
@@ -57,13 +74,6 @@ func TestValidUnitTypes(t *testing.T) {
 	}
 }
 
-func newManager(config *configs.Cgroup) cgroups.Manager {
-	if cgroups.IsCgroup2UnifiedMode() {
-		return NewUnifiedManager(config, "", false)
-	}
-	return NewLegacyManager(config, nil)
-}
-
 // TestPodSkipDevicesUpdate checks that updating a pod having SkipDevices: true
 // does not result in spurious "permission denied" errors in a container
 // running under the pod. The test is somewhat similar in nature to the
@@ -79,8 +89,9 @@ func TestPodSkipDevicesUpdate(t *testing.T) {
 
 	podName := "system-runc_test_pod" + t.Name() + ".slice"
 	podConfig := &configs.Cgroup{
-		Parent: "system.slice",
-		Name:   podName,
+		Systemd: true,
+		Parent:  "system.slice",
+		Name:    podName,
 		Resources: &configs.Resources{
 			PidsLimit:   42,
 			Memory:      32 * 1024 * 1024,
@@ -88,8 +99,7 @@ func TestPodSkipDevicesUpdate(t *testing.T) {
 		},
 	}
 	// Create "pod" cgroup (a systemd slice to hold containers).
-	pm := newManager(podConfig)
-	defer pm.Destroy() //nolint:errcheck
+	pm := newManager(t, podConfig)
 	if err := pm.Apply(-1); err != nil {
 		t.Fatal(err)
 	}
@@ -132,9 +142,7 @@ func TestPodSkipDevicesUpdate(t *testing.T) {
 	}()
 
 	// Put the process into a cgroup.
-	cm := newManager(containerConfig)
-	defer cm.Destroy() //nolint:errcheck
-
+	cm := newManager(t, containerConfig)
 	if err := cm.Apply(cmd.Process.Pid); err != nil {
 		t.Fatal(err)
 	}
@@ -184,8 +192,7 @@ func testSkipDevices(t *testing.T, skipDevices bool, expected []string) {
 		},
 	}
 	// Create "pods" cgroup (a systemd slice to hold containers).
-	pm := newManager(podConfig)
-	defer pm.Destroy() //nolint:errcheck
+	pm := newManager(t, podConfig)
 	if err := pm.Apply(-1); err != nil {
 		t.Fatal(err)
 	}
@@ -236,9 +243,7 @@ func testSkipDevices(t *testing.T, skipDevices bool, expected []string) {
 	}()
 
 	// Put the process into a cgroup.
-	m := newManager(config)
-	defer m.Destroy() //nolint:errcheck
-
+	m := newManager(t, config)
 	if err := m.Apply(cmd.Process.Pid); err != nil {
 		t.Fatal(err)
 	}
@@ -305,8 +310,7 @@ func TestUnitExistsIgnored(t *testing.T) {
 		Resources: &configs.Resources{},
 	}
 	// Create "pods" cgroup (a systemd slice to hold containers).
-	pm := newManager(podConfig)
-	defer pm.Destroy() //nolint:errcheck
+	pm := newManager(t, podConfig)
 
 	// create twice to make sure "UnitExists" error is ignored.
 	for i := 0; i < 2; i++ {
@@ -334,8 +338,7 @@ func TestFreezePodCgroup(t *testing.T) {
 	}
 	// Create a "pod" cgroup (a systemd slice to hold containers),
 	// which is frozen initially.
-	pm := newManager(podConfig)
-	defer pm.Destroy() //nolint:errcheck
+	pm := newManager(t, podConfig)
 	if err := pm.Apply(-1); err != nil {
 		t.Fatal(err)
 	}
@@ -402,8 +405,7 @@ func TestFreezePodCgroup(t *testing.T) {
 	}()
 
 	// Put the process into a cgroup.
-	cm := newManager(containerConfig)
-	defer cm.Destroy() //nolint:errcheck
+	cm := newManager(t, containerConfig)
 
 	if err := cm.Apply(cmd.Process.Pid); err != nil {
 		t.Fatal(err)
diff --git a/libcontainer/cgroups/systemd/v1.go b/libcontainer/cgroups/systemd/v1.go
index 470453f3316..9a2f18de8e1 100644
--- a/libcontainer/cgroups/systemd/v1.go
+++ b/libcontainer/cgroups/systemd/v1.go
@@ -24,12 +24,15 @@ type legacyManager struct {
 	dbus    *dbusConnManager
 }
 
-func NewLegacyManager(cg *configs.Cgroup, paths map[string]string) cgroups.Manager {
+func NewLegacyManager(cg *configs.Cgroup, paths map[string]string) (cgroups.Manager, error) {
+	if cg.Rootless {
+		return nil, errors.New("cannot use rootless systemd cgroups manager on cgroup v1")
+	}
 	return &legacyManager{
 		cgroups: cg,
 		paths:   paths,
 		dbus:    newDbusConnManager(false),
-	}
+	}, nil
 }
 
 type subsystem interface {
diff --git a/libcontainer/cgroups/systemd/v1_test.go b/libcontainer/cgroups/systemd/v1_test.go
index 25ac5714bfb..7c7ef55fca1 100644
--- a/libcontainer/cgroups/systemd/v1_test.go
+++ b/libcontainer/cgroups/systemd/v1_test.go
@@ -133,7 +133,10 @@ func TestFreezeBeforeSet(t *testing.T) {
 	for _, tc := range testCases {
 		tc := tc
 		t.Run(tc.desc, func(t *testing.T) {
-			m := NewLegacyManager(tc.cg, nil)
+			m, err := NewLegacyManager(tc.cg, nil)
+			if err != nil {
+				t.Fatal(err)
+			}
 			defer m.Destroy() //nolint:errcheck
 			lm := m.(*legacyManager)
 
diff --git a/libcontainer/cgroups/systemd/v2.go b/libcontainer/cgroups/systemd/v2.go
index 5af4500ead2..7f2e8153eb3 100644
--- a/libcontainer/cgroups/systemd/v2.go
+++ b/libcontainer/cgroups/systemd/v2.go
@@ -22,18 +22,16 @@ type unifiedManager struct {
 	mu      sync.Mutex
 	cgroups *configs.Cgroup
 	// path is like "/sys/fs/cgroup/user.slice/user-1001.slice/session-1.scope"
-	path     string
-	rootless bool
-	dbus     *dbusConnManager
+	path string
+	dbus *dbusConnManager
 }
 
-func NewUnifiedManager(config *configs.Cgroup, path string, rootless bool) cgroups.Manager {
+func NewUnifiedManager(config *configs.Cgroup, path string) (cgroups.Manager, error) {
 	return &unifiedManager{
-		cgroups:  config,
-		path:     path,
-		rootless: rootless,
-		dbus:     newDbusConnManager(rootless),
-	}
+		cgroups: config,
+		path:    path,
+		dbus:    newDbusConnManager(config.Rootless),
+	}, nil
 }
 
 // unifiedResToSystemdProps tries to convert from Cgroup.Resources.Unified
@@ -233,7 +231,7 @@ func (m *unifiedManager) Apply(pid int) error {
 	)
 
 	slice := "system.slice"
-	if m.rootless {
+	if m.cgroups.Rootless {
 		slice = "user.slice"
 	}
 	if c.Parent != "" {
@@ -313,7 +311,7 @@ func (m *unifiedManager) Path(_ string) string {
 func (m *unifiedManager) getSliceFull() (string, error) {
 	c := m.cgroups
 	slice := "system.slice"
-	if m.rootless {
+	if c.Rootless {
 		slice = "user.slice"
 	}
 	if c.Parent != "" {
@@ -324,7 +322,7 @@ func (m *unifiedManager) getSliceFull() (string, error) {
 		}
 	}
 
-	if m.rootless {
+	if c.Rootless {
 		// managerCG is typically "/user.slice/user-${uid}.slice/user@${uid}.service".
 		managerCG, err := getManagerProperty(m.dbus, "ControlGroup")
 		if err != nil {
@@ -366,7 +364,7 @@ func (m *unifiedManager) fsManager() (cgroups.Manager, error) {
 	if err := m.initPath(); err != nil {
 		return nil, err
 	}
-	return fs2.NewManager(m.cgroups, m.path, m.rootless)
+	return fs2.NewManager(m.cgroups, m.path)
 }
 
 func (m *unifiedManager) Freeze(state configs.FreezerState) error {
diff --git a/libcontainer/configs/cgroup_linux.go b/libcontainer/configs/cgroup_linux.go
index 1432b50ba26..25424bdcbfa 100644
--- a/libcontainer/configs/cgroup_linux.go
+++ b/libcontainer/configs/cgroup_linux.go
@@ -31,10 +31,16 @@ type Cgroup struct {
 	// Resources contains various cgroups settings to apply
 	*Resources
 
+	// Systemd tells if systemd should be used to manage cgroups.
+	Systemd bool
+
 	// SystemdProps are any additional properties for systemd,
 	// derived from org.systemd.property.xxx annotations.
 	// Ignored unless systemd is used for managing cgroups.
 	SystemdProps []systemdDbus.Property `json:"-"`
+
+	// Rootless tells if rootless cgroups should be used.
+	Rootless bool
 }
 
 type Resources struct {
diff --git a/libcontainer/factory_linux.go b/libcontainer/factory_linux.go
index 81bc2c29719..4c5bf67bee1 100644
--- a/libcontainer/factory_linux.go
+++ b/libcontainer/factory_linux.go
@@ -14,10 +14,7 @@ import (
 	"github.com/moby/sys/mountinfo"
 	"golang.org/x/sys/unix"
 
-	"github.com/opencontainers/runc/libcontainer/cgroups"
-	"github.com/opencontainers/runc/libcontainer/cgroups/fs"
-	"github.com/opencontainers/runc/libcontainer/cgroups/fs2"
-	"github.com/opencontainers/runc/libcontainer/cgroups/systemd"
+	"github.com/opencontainers/runc/libcontainer/cgroups/manager"
 	"github.com/opencontainers/runc/libcontainer/configs"
 	"github.com/opencontainers/runc/libcontainer/configs/validate"
 	"github.com/opencontainers/runc/libcontainer/intelrdt"
@@ -30,10 +27,7 @@ const (
 	execFifoFilename = "exec.fifo"
 )
 
-var (
-	idRegex      = regexp.MustCompile(`^[\w+-\.]+$`)
-	errNoSystemd = errors.New("systemd not running on this host, can't use systemd as cgroups manager")
-)
+var idRegex = regexp.MustCompile(`^[\w+-\.]+$`)
 
 // InitArgs returns an options func to configure a LinuxFactory with the
 // provided init binary path and arguments.
@@ -54,100 +48,6 @@ func InitArgs(args ...string) func(*LinuxFactory) error {
 	}
 }
 
-func getUnifiedPath(paths map[string]string) string {
-	path := ""
-	for k, v := range paths {
-		if path == "" {
-			path = v
-		} else if v != path {
-			panic(fmt.Errorf("expected %q path to be unified path %q, got %q", k, path, v))
-		}
-	}
-	// can be empty
-	if path != "" {
-		if filepath.Clean(path) != path || !filepath.IsAbs(path) {
-			panic(fmt.Errorf("invalid dir path %q", path))
-		}
-	}
-
-	return path
-}
-
-func systemdCgroupV2(l *LinuxFactory, rootless bool) error {
-	l.NewCgroupsManager = func(config *configs.Cgroup, paths map[string]string) cgroups.Manager {
-		return systemd.NewUnifiedManager(config, getUnifiedPath(paths), rootless)
-	}
-	return nil
-}
-
-// SystemdCgroups is an options func to configure a LinuxFactory to return
-// containers that use systemd to create and manage cgroups.
-func SystemdCgroups(l *LinuxFactory) error {
-	if !systemd.IsRunningSystemd() {
-		return errNoSystemd
-	}
-
-	if cgroups.IsCgroup2UnifiedMode() {
-		return systemdCgroupV2(l, false)
-	}
-
-	l.NewCgroupsManager = func(config *configs.Cgroup, paths map[string]string) cgroups.Manager {
-		return systemd.NewLegacyManager(config, paths)
-	}
-
-	return nil
-}
-
-// RootlessSystemdCgroups is rootless version of SystemdCgroups.
-func RootlessSystemdCgroups(l *LinuxFactory) error {
-	if !systemd.IsRunningSystemd() {
-		return errNoSystemd
-	}
-
-	if !cgroups.IsCgroup2UnifiedMode() {
-		return errors.New("cgroup v2 not enabled on this host, can't use systemd (rootless) as cgroups manager")
-	}
-	return systemdCgroupV2(l, true)
-}
-
-func cgroupfs2(l *LinuxFactory, rootless bool) error {
-	l.NewCgroupsManager = func(config *configs.Cgroup, paths map[string]string) cgroups.Manager {
-		m, err := fs2.NewManager(config, getUnifiedPath(paths), rootless)
-		if err != nil {
-			panic(err)
-		}
-		return m
-	}
-	return nil
-}
-
-func cgroupfs(l *LinuxFactory, rootless bool) error {
-	if cgroups.IsCgroup2UnifiedMode() {
-		return cgroupfs2(l, rootless)
-	}
-	l.NewCgroupsManager = func(config *configs.Cgroup, paths map[string]string) cgroups.Manager {
-		return fs.NewManager(config, paths, rootless)
-	}
-	return nil
-}
-
-// Cgroupfs is an options func to configure a LinuxFactory to return containers
-// that use the native cgroups filesystem implementation to create and manage
-// cgroups.
-func Cgroupfs(l *LinuxFactory) error {
-	return cgroupfs(l, false)
-}
-
-// RootlessCgroupfs is an options func to configure a LinuxFactory to return
-// containers that use the native cgroups filesystem implementation to create
-// and manage cgroups. The difference between RootlessCgroupfs and Cgroupfs is
-// that RootlessCgroupfs can transparently handle permission errors that occur
-// during rootless container (including euid=0 in userns) setup (while still allowing cgroup usage if
-// they've been set up properly).
-func RootlessCgroupfs(l *LinuxFactory) error {
-	return cgroupfs(l, true)
-}
-
 // IntelRdtfs is an options func to configure a LinuxFactory to return
 // containers that use the Intel RDT "resource control" filesystem to
 // create and manage Intel RDT resources (e.g., L3 cache, memory bandwidth).
@@ -201,10 +101,6 @@ func New(root string, options ...func(*LinuxFactory) error) (Factory, error) {
 		CriuPath:  "criu",
 	}
 
-	if err := Cgroupfs(l); err != nil {
-		return nil, err
-	}
-
 	for _, opt := range options {
 		if opt == nil {
 			continue
@@ -241,9 +137,6 @@ type LinuxFactory struct {
 	// Validator provides validation to container configurations.
 	Validator validate.Validator
 
-	// NewCgroupsManager returns an initialized cgroups manager for a single container.
-	NewCgroupsManager func(config *configs.Cgroup, paths map[string]string) cgroups.Manager
-
 	// NewIntelRdtManager returns an initialized Intel RDT manager for a single container.
 	NewIntelRdtManager func(config *configs.Config, id string, path string) intelrdt.Manager
 }
@@ -273,6 +166,10 @@ func (l *LinuxFactory) Create(id string, config *configs.Config) (Container, err
 	if err := os.Chown(containerRoot, unix.Geteuid(), unix.Getegid()); err != nil {
 		return nil, err
 	}
+	cm, err := manager.New(config.Cgroups)
+	if err != nil {
+		return nil, err
+	}
 	c := &linuxContainer{
 		id:            id,
 		root:          containerRoot,
@@ -282,7 +179,7 @@ func (l *LinuxFactory) Create(id string, config *configs.Config) (Container, err
 		criuPath:      l.CriuPath,
 		newuidmapPath: l.NewuidmapPath,
 		newgidmapPath: l.NewgidmapPath,
-		cgroupManager: l.NewCgroupsManager(config.Cgroups, nil),
+		cgroupManager: cm,
 	}
 	if l.NewIntelRdtManager != nil {
 		c.intelRdtManager = l.NewIntelRdtManager(config, id, "")
@@ -312,6 +209,10 @@ func (l *LinuxFactory) Load(id string) (Container, error) {
 		processStartTime: state.InitProcessStartTime,
 		fds:              state.ExternalDescriptors,
 	}
+	cm, err := manager.NewWithPaths(state.Config.Cgroups, state.CgroupPaths)
+	if err != nil {
+		return nil, err
+	}
 	c := &linuxContainer{
 		initProcess:          r,
 		initProcessStartTime: state.InitProcessStartTime,
@@ -322,7 +223,7 @@ func (l *LinuxFactory) Load(id string) (Container, error) {
 		criuPath:             l.CriuPath,
 		newuidmapPath:        l.NewuidmapPath,
 		newgidmapPath:        l.NewgidmapPath,
-		cgroupManager:        l.NewCgroupsManager(state.Config.Cgroups, state.CgroupPaths),
+		cgroupManager:        cm,
 		root:                 containerRoot,
 		created:              state.Created,
 	}
diff --git a/libcontainer/factory_linux_test.go b/libcontainer/factory_linux_test.go
index fe34473cdf6..377bcea90bf 100644
--- a/libcontainer/factory_linux_test.go
+++ b/libcontainer/factory_linux_test.go
@@ -17,7 +17,7 @@ import (
 
 func TestFactoryNew(t *testing.T) {
 	root := t.TempDir()
-	factory, err := New(root, Cgroupfs)
+	factory, err := New(root)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -39,7 +39,7 @@ func TestFactoryNew(t *testing.T) {
 
 func TestFactoryNewIntelRdt(t *testing.T) {
 	root := t.TempDir()
-	factory, err := New(root, Cgroupfs, IntelRdtFs)
+	factory, err := New(root, IntelRdtFs)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -61,7 +61,7 @@ func TestFactoryNewIntelRdt(t *testing.T) {
 
 func TestFactoryNewTmpfs(t *testing.T) {
 	root := t.TempDir()
-	factory, err := New(root, Cgroupfs, TmpfsRoot)
+	factory, err := New(root, TmpfsRoot)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -107,7 +107,7 @@ func TestFactoryNewTmpfs(t *testing.T) {
 }
 
 func TestFactoryLoadNotExists(t *testing.T) {
-	factory, err := New(t.TempDir(), Cgroupfs)
+	factory, err := New(t.TempDir())
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -138,8 +138,9 @@ func TestFactoryLoadContainer(t *testing.T) {
 			},
 		}
 		expectedConfig = &configs.Config{
-			Rootfs: "/mycontainer/root",
-			Hooks:  expectedHooks,
+			Rootfs:  "/mycontainer/root",
+			Hooks:   expectedHooks,
+			Cgroups: &configs.Cgroup{},
 		}
 		expectedState = &State{
 			BaseState: BaseState{
@@ -154,7 +155,7 @@ func TestFactoryLoadContainer(t *testing.T) {
 	if err := marshal(filepath.Join(root, id, stateFilename), expectedState); err != nil {
 		t.Fatal(err)
 	}
-	factory, err := New(root, Cgroupfs, IntelRdtFs)
+	factory, err := New(root, IntelRdtFs)
 	if err != nil {
 		t.Fatal(err)
 	}
diff --git a/libcontainer/integration/checkpoint_test.go b/libcontainer/integration/checkpoint_test.go
index 9dbed42a32c..c86ec8a0c47 100644
--- a/libcontainer/integration/checkpoint_test.go
+++ b/libcontainer/integration/checkpoint_test.go
@@ -62,7 +62,7 @@ func testCheckpoint(t *testing.T, userns bool) {
 	}
 
 	config := newTemplateConfig(t, &tParam{userns: userns})
-	factory, err := libcontainer.New(t.TempDir(), libcontainer.Cgroupfs)
+	factory, err := libcontainer.New(t.TempDir())
 	ok(t, err)
 
 	container, err := factory.Create("test", config)
diff --git a/libcontainer/integration/template_test.go b/libcontainer/integration/template_test.go
index d35904766b9..f56db8956a7 100644
--- a/libcontainer/integration/template_test.go
+++ b/libcontainer/integration/template_test.go
@@ -132,6 +132,7 @@ func newTemplateConfig(t *testing.T, p *tParam) *configs.Config {
 			{Type: configs.NEWNET},
 		}),
 		Cgroups: &configs.Cgroup{
+			Systemd: p.systemd,
 			Resources: &configs.Resources{
 				MemorySwappiness: nil,
 				Devices:          allowedDevices,
@@ -221,7 +222,6 @@ func newTemplateConfig(t *testing.T, p *tParam) *configs.Config {
 	if p.systemd {
 		id := strconv.FormatInt(-int64(time.Now().Nanosecond()), 36)
 		config.Cgroups.Name = strings.ReplaceAll(t.Name(), "/", "_") + id
-		// do not change Parent (see newContainer)
 		config.Cgroups.Parent = "system.slice"
 		config.Cgroups.ScopePrefix = "runc-test"
 	} else {
diff --git a/libcontainer/integration/utils_test.go b/libcontainer/integration/utils_test.go
index 4aae604aec5..63eb9bc3983 100644
--- a/libcontainer/integration/utils_test.go
+++ b/libcontainer/integration/utils_test.go
@@ -126,16 +126,10 @@ func newContainer(t *testing.T, config *configs.Config) (libcontainer.Container,
 	name := strings.ReplaceAll(t.Name(), "/", "_") + strconv.FormatInt(-int64(time.Now().Nanosecond()), 35)
 	root := t.TempDir()
 
-	f, err := libcontainer.New(root, libcontainer.Cgroupfs)
+	f, err := libcontainer.New(root)
 	if err != nil {
 		return nil, err
 	}
-	if config.Cgroups != nil && config.Cgroups.Parent == "system.slice" {
-		f, err = libcontainer.New(root, libcontainer.SystemdCgroups)
-		if err != nil {
-			return nil, err
-		}
-	}
 	return f.Create(name, config)
 }
 
diff --git a/libcontainer/specconv/spec_linux.go b/libcontainer/specconv/spec_linux.go
index 991c08a1996..eac6688aa17 100644
--- a/libcontainer/specconv/spec_linux.go
+++ b/libcontainer/specconv/spec_linux.go
@@ -429,6 +429,8 @@ func CreateCgroupConfig(opts *CreateOpts, defaultDevs []*devices.Device) (*confi
 	)
 
 	c := &configs.Cgroup{
+		Systemd:   useSystemdCgroup,
+		Rootless:  opts.RootlessCgroups,
 		Resources: &configs.Resources{},
 	}
 
diff --git a/utils_linux.go b/utils_linux.go
index 11957f78108..78ad6e2429d 100644
--- a/utils_linux.go
+++ b/utils_linux.go
@@ -17,7 +17,6 @@ import (
 	"golang.org/x/sys/unix"
 
 	"github.com/opencontainers/runc/libcontainer"
-	"github.com/opencontainers/runc/libcontainer/cgroups/systemd"
 	"github.com/opencontainers/runc/libcontainer/configs"
 	"github.com/opencontainers/runc/libcontainer/specconv"
 	"github.com/opencontainers/runc/libcontainer/utils"
@@ -33,26 +32,6 @@ func loadFactory(context *cli.Context) (libcontainer.Factory, error) {
 		return nil, err
 	}
 
-	// We default to cgroupfs, and can only use systemd if the system is a
-	// systemd box.
-	cgroupManager := libcontainer.Cgroupfs
-	rootlessCg, err := shouldUseRootlessCgroupManager(context)
-	if err != nil {
-		return nil, err
-	}
-	if rootlessCg {
-		cgroupManager = libcontainer.RootlessCgroupfs
-	}
-	if context.GlobalBool("systemd-cgroup") {
-		if !systemd.IsRunningSystemd() {
-			return nil, errors.New("systemd cgroup flag passed, but systemd support for managing cgroups is not available")
-		}
-		cgroupManager = libcontainer.SystemdCgroups
-		if rootlessCg {
-			cgroupManager = libcontainer.RootlessSystemdCgroups
-		}
-	}
-
 	intelRdtManager := libcontainer.IntelRdtFs
 
 	// We resolve the paths for {newuidmap,newgidmap} from the context of runc,
@@ -67,7 +46,7 @@ func loadFactory(context *cli.Context) (libcontainer.Factory, error) {
 		newgidmap = ""
 	}
 
-	return libcontainer.New(abs, cgroupManager, intelRdtManager,
+	return libcontainer.New(abs, intelRdtManager,
 		libcontainer.CriuPath(context.GlobalString("criu")),
 		libcontainer.NewuidmapPath(newuidmap),
 		libcontainer.NewgidmapPath(newgidmap))