From 813cb3eb94659971c69c0dd19b2d5a9737891f15 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Sun, 5 Apr 2020 21:02:06 -0700 Subject: [PATCH] cgroupv2: fix fs2 cgroup init fs2 cgroup driver was not working because it did not enable controllers while creating cgroup directory; instead it was merely doing MkdirAll() and gathered the list of available controllers in NewManager(). Also, cgroup should be created in Apply(), not while creating a new manager instance. To fix: 1. Move the createCgroupsv2Path function from systemd driver to fs2 driver, renaming it to CreateCgroupPath. Use in Apply() from both fs2 and systemd drivers. 2. Delay available controllers map initialization to until it is needed. With this patch: - NewManager() only performs minimal initialization (initializin m.dirPath, if not provided); - Apply() properly creates cgroup path, enabling the controllers; - m.controllers is initialized lazily on demand. Signed-off-by: Kir Kolyshkin --- libcontainer/cgroups/fs2/create.go | 49 +++++++++++++++++++ libcontainer/cgroups/fs2/fs2.go | 47 ++++++++++-------- .../cgroups/systemd/unified_hierarchy.go | 41 +--------------- 3 files changed, 77 insertions(+), 60 deletions(-) create mode 100644 libcontainer/cgroups/fs2/create.go diff --git a/libcontainer/cgroups/fs2/create.go b/libcontainer/cgroups/fs2/create.go new file mode 100644 index 00000000..fb5e5050 --- /dev/null +++ b/libcontainer/cgroups/fs2/create.go @@ -0,0 +1,49 @@ +package fs2 + +import ( + "bytes" + "io/ioutil" + "os" + "path/filepath" + "strings" +) + +// CreateCgroupPath creates cgroupv2 path, enabling all the +// available controllers in the process. +func CreateCgroupPath(path string) (Err error) { + const file = UnifiedMountpoint + "/cgroup.controllers" + content, err := ioutil.ReadFile(file) + if err != nil { + return err + } + + ctrs := bytes.Fields(content) + res := append([]byte("+"), bytes.Join(ctrs, []byte(" +"))...) + + current := "/sys/fs" + elements := strings.Split(path, "/") + for i, e := range elements[3:] { + current = filepath.Join(current, e) + if i > 0 { + if err := os.Mkdir(current, 0755); err != nil { + if !os.IsExist(err) { + return err + } + } else { + // If the directory was created, be sure it is not left around on errors. + current := current + defer func() { + if Err != nil { + os.Remove(current) + } + }() + } + } + if i < len(elements[3:])-1 { + if err := ioutil.WriteFile(filepath.Join(current, "cgroup.subtree_control"), res, 0755); err != nil { + return err + } + } + } + return nil +} diff --git a/libcontainer/cgroups/fs2/fs2.go b/libcontainer/cgroups/fs2/fs2.go index 4f047bd5..f32f647d 100644 --- a/libcontainer/cgroups/fs2/fs2.go +++ b/libcontainer/cgroups/fs2/fs2.go @@ -37,38 +37,38 @@ func NewManager(config *configs.Cgroup, dirPath string, rootless bool) (cgroups. return nil, err } } - controllers, err := detectControllers(dirPath) - if err != nil && !rootless { - return nil, err - } m := &manager{ - config: config, - dirPath: dirPath, - controllers: controllers, - rootless: rootless, + config: config, + dirPath: dirPath, + rootless: rootless, } return m, nil } -func detectControllers(dirPath string) (map[string]struct{}, error) { - if err := os.MkdirAll(dirPath, 0755); err != nil { - return nil, err +func (m *manager) getControllers() error { + if m.controllers != nil { + return nil } - controllersPath := filepath.Join(dirPath, "cgroup.controllers") - controllersData, err := ioutil.ReadFile(controllersPath) - if err != nil { - return nil, err + + file := filepath.Join(m.dirPath, "cgroup.controllers") + data, err := ioutil.ReadFile(file) + if err != nil && !m.rootless { + return err } - controllersFields := strings.Fields(string(controllersData)) - controllers := make(map[string]struct{}, len(controllersFields)) - for _, c := range controllersFields { - controllers[c] = struct{}{} + fields := strings.Fields(string(data)) + m.controllers = make(map[string]struct{}, len(fields)) + for _, c := range fields { + m.controllers[c] = struct{}{} } - return controllers, nil + + return nil } func (m *manager) Apply(pid int) error { + if err := CreateCgroupPath(m.dirPath); err != nil { + return err + } if err := cgroups.WriteCgroupProc(m.dirPath, pid); err != nil && !m.rootless { return err } @@ -89,6 +89,9 @@ func (m *manager) GetStats() (*cgroups.Stats, error) { ) st := cgroups.NewStats() + if err := m.getControllers(); err != nil { + return st, err + } // pids (since kernel 4.5) if _, ok := m.controllers["pids"]; ok { @@ -144,6 +147,7 @@ func (m *manager) Destroy() error { // GetPaths is for compatibility purpose and should be removed in future func (m *manager) GetPaths() map[string]string { + _ = m.getControllers() paths := map[string]string{ // pseudo-controller for compatibility "devices": m.dirPath, @@ -163,6 +167,9 @@ func (m *manager) Set(container *configs.Config) error { if container == nil || container.Cgroups == nil { return nil } + if err := m.getControllers(); err != nil { + return err + } var errs []error // pids (since kernel 4.5) if _, ok := m.controllers["pids"]; ok { diff --git a/libcontainer/cgroups/systemd/unified_hierarchy.go b/libcontainer/cgroups/systemd/unified_hierarchy.go index 9f4a4e5a..18280371 100644 --- a/libcontainer/cgroups/systemd/unified_hierarchy.go +++ b/libcontainer/cgroups/systemd/unified_hierarchy.go @@ -3,8 +3,6 @@ package systemd import ( - "bytes" - "io/ioutil" "math" "os" "path/filepath" @@ -153,7 +151,7 @@ func (m *unifiedManager) Apply(pid int) error { if err != nil { return err } - if err := createCgroupsv2Path(m.path); err != nil { + if err := fs2.CreateCgroupPath(m.path); err != nil { return err } return nil @@ -224,43 +222,6 @@ func (m *unifiedManager) GetUnifiedPath() (string, error) { return m.path, nil } -func createCgroupsv2Path(path string) (Err error) { - content, err := ioutil.ReadFile("/sys/fs/cgroup/cgroup.controllers") - if err != nil { - return err - } - - ctrs := bytes.Fields(content) - res := append([]byte("+"), bytes.Join(ctrs, []byte(" +"))...) - - current := "/sys/fs" - elements := strings.Split(path, "/") - for i, e := range elements[3:] { - current = filepath.Join(current, e) - if i > 0 { - if err := os.Mkdir(current, 0755); err != nil { - if !os.IsExist(err) { - return err - } - } else { - // If the directory was created, be sure it is not left around on errors. - current := current - defer func() { - if Err != nil { - os.Remove(current) - } - }() - } - } - if i < len(elements[3:])-1 { - if err := ioutil.WriteFile(filepath.Join(current, "cgroup.subtree_control"), res, 0755); err != nil { - return err - } - } - } - return nil -} - func (m *unifiedManager) fsManager() (cgroups.Manager, error) { path, err := m.GetUnifiedPath() if err != nil {