libct/cg/fs: move paths init to NewManager

1. Separate path initialization logic from Apply to initPaths,
   and call initPaths from NewManager, so:
   - we can error out early (in NewManager rather than Apply);
   - always have m.paths available (e.g. in Destroy or Exists).
   - do not unnecessarily call subsysPath from Apply in case
     the paths were already provided.

2. Add a check for non-nil cgroups.Resources to NewManager,
   since initPaths, as well as some controller's Apply methods,
   need it.

3. Move cgroups.Resources.Unified check from Apply to NewManager,
   so we can error out early (same check exists in Set).

Signed-off-by: Kir Kolyshkin <[email protected]>

Author: kolyshkin

libcontainer/cgroups/fs/fs.go

@@ -55,6 +55,23 @@ type manager struct {
 }
 
 func NewManager(cg *configs.Cgroup, paths map[string]string) (cgroups.Manager, error) {
+	// Some v1 controllers (cpu, cpuset, and devices) expect
+	// cgroups.Resources to not be nil in Apply.
+	if cg.Resources == nil {
+		return nil, errors.New("cgroup v1 manager needs configs.Resources to be set during manager creation")
+	}
+	if cg.Resources.Unified != nil {
+		return nil, cgroups.ErrV1NoUnified
+	}
+
+	if paths == nil {
+		var err error
+		paths, err = initPaths(cg)
+		if err != nil {
+			return nil, err
+		}
+	}
+
 	return &manager{
 		cgroups: cg,
 		paths:   paths,
@@ -90,37 +107,21 @@ func (m *manager) Apply(pid int) (err error) {
 	defer m.mu.Unlock()
 
 	c := m.cgroups
-	if c.Resources.Unified != nil {
-		return cgroups.ErrV1NoUnified
-	}
 
-	root, err := rootPath()
-	if err != nil {
-		return err
-	}
-
-	inner, err := innerPath(c)
-
-	m.paths = make(map[string]string)
 	for _, sys := range subsystems {
-		p, err := subsysPath(root, inner, sys.Name())
-		if err != nil {
-			// The non-presence of the devices subsystem is
-			// considered fatal for security reasons.
-			if cgroups.IsNotFound(err) && (c.SkipDevices || sys.Name() != "devices") {
-				continue
-			}
-			return err
+		name := sys.Name()
+		p, ok := m.paths[name]
+		if !ok {
+			continue
 		}
-		m.paths[sys.Name()] = p
 
 		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
 			// (and we couldn't create our own cgroup) are handled by Set.
-			if isIgnorableError(c.Rootless, err) && m.cgroups.Path == "" {
-				delete(m.paths, sys.Name())
+			if isIgnorableError(c.Rootless, err) && c.Path == "" {
+				delete(m.paths, name)
 				continue
 			}
 			return err

libcontainer/cgroups/fs/paths.go

@@ -21,6 +21,36 @@ var (
 
 const defaultCgroupRoot = "/sys/fs/cgroup"
 
+func initPaths(cg *configs.Cgroup) (map[string]string, error) {
+	root, err := rootPath()
+	if err != nil {
+		return nil, err
+	}
+
+	inner, err := innerPath(cg)
+	if err != nil {
+		return nil, err
+	}
+
+	paths := make(map[string]string)
+	for _, sys := range subsystems {
+		name := sys.Name()
+		path, err := subsysPath(root, inner, name)
+		if err != nil {
+			// The non-presence of the devices subsystem
+			// is considered fatal for security reasons.
+			if cgroups.IsNotFound(err) && (cg.SkipDevices || name != "devices") {
+				continue
+			}
+
+			return nil, err
+		}
+		paths[name] = path
+	}
+
+	return paths, nil
+}
+
 func tryDefaultCgroupRoot() string {
 	var st, pst unix.Stat_t
 

libcontainer/factory_linux_test.go

@@ -138,9 +138,11 @@ func TestFactoryLoadContainer(t *testing.T) {
 			},
 		}
 		expectedConfig = &configs.Config{
-			Rootfs:  "/mycontainer/root",
-			Hooks:   expectedHooks,
-			Cgroups: &configs.Cgroup{},
+			Rootfs: "/mycontainer/root",
+			Hooks:  expectedHooks,
+			Cgroups: &configs.Cgroup{
+				Resources: &configs.Resources{},
+			},
 		}
 		expectedState = &State{
 			BaseState: BaseState{