diff --git a/libcontainer/cgroups/cgroups.go b/libcontainer/cgroups/cgroups.go index c0a965923..cbc430bb2 100644 --- a/libcontainer/cgroups/cgroups.go +++ b/libcontainer/cgroups/cgroups.go @@ -27,27 +27,22 @@ type Manager interface { // Destroys the cgroup set Destroy() error - // The option func SystemdCgroups() and Cgroupfs() require following attributes: - // Paths map[string]string - // Cgroups *configs.Cgroup - // Paths maps cgroup subsystem to path at which it is mounted. - // Cgroups specifies specific cgroup settings for the various subsystems - - // Returns cgroup paths to save in a state file and to be able to - // restore the object later. - GetPaths() map[string]string - - // GetUnifiedPath returns the unified path when running in unified mode. - // The value corresponds to the all values of GetPaths() map. - // - // GetUnifiedPath returns error when running in hybrid mode as well as - // in legacy mode. - GetUnifiedPath() (string, error) + // Path returns a cgroup path to the specified controller/subsystem. + // For cgroupv2, the argument is unused and can be empty. + Path(string) string // Sets the cgroup as configured. Set(container *configs.Config) error - // Gets the cgroup as configured. + // GetPaths returns cgroup path(s) to save in a state file in order to restore later. + // + // For cgroup v1, a key is cgroup subsystem name, and the value is the path + // to the cgroup for this subsystem. + // + // For cgroup v2 unified hierarchy, a key is "", and the value is the unified path. + GetPaths() map[string]string + + // GetCgroups returns the cgroup data as configured. GetCgroups() (*configs.Cgroup, error) } diff --git a/libcontainer/cgroups/fs/apply_raw.go b/libcontainer/cgroups/fs/apply_raw.go index 760d4e6eb..c30601451 100644 --- a/libcontainer/cgroups/fs/apply_raw.go +++ b/libcontainer/cgroups/fs/apply_raw.go @@ -209,15 +209,10 @@ func (m *manager) Destroy() error { return nil } -func (m *manager) GetPaths() map[string]string { +func (m *manager) Path(subsys string) string { m.mu.Lock() - paths := m.paths - m.mu.Unlock() - return paths -} - -func (m *manager) GetUnifiedPath() (string, error) { - return "", errors.New("unified path is only supported when running in unified mode") + defer m.mu.Unlock() + return m.paths[subsys] } func (m *manager) GetStats() (*cgroups.Stats, error) { @@ -299,13 +294,11 @@ func (m *manager) Freeze(state configs.FreezerState) error { } func (m *manager) GetPids() ([]int, error) { - paths := m.GetPaths() - return cgroups.GetPids(paths["devices"]) + return cgroups.GetPids(m.Path("devices")) } func (m *manager) GetAllPids() ([]int, error) { - paths := m.GetPaths() - return cgroups.GetAllPids(paths["devices"]) + return cgroups.GetAllPids(m.Path("devices")) } func getCgroupData(c *configs.Cgroup, pid int) (*cgroupData, error) { @@ -411,6 +404,12 @@ func CheckCpushares(path string, c uint64) error { return nil } +func (m *manager) GetPaths() map[string]string { + m.mu.Lock() + defer m.mu.Unlock() + return m.paths +} + func (m *manager) GetCgroups() (*configs.Cgroup, error) { return m.cgroups, nil } diff --git a/libcontainer/cgroups/fs2/fs2.go b/libcontainer/cgroups/fs2/fs2.go index fc469e103..a0881dac8 100644 --- a/libcontainer/cgroups/fs2/fs2.go +++ b/libcontainer/cgroups/fs2/fs2.go @@ -164,22 +164,8 @@ func (m *manager) Destroy() error { return nil } -// 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, - "freezer": m.dirPath, - } - for c := range m.controllers { - paths[c] = m.dirPath - } - return paths -} - -func (m *manager) GetUnifiedPath() (string, error) { - return m.dirPath, nil +func (m *manager) Path(_ string) string { + return m.dirPath } func (m *manager) Set(container *configs.Config) error { @@ -245,6 +231,12 @@ func (m *manager) Set(container *configs.Config) error { return nil } +func (m *manager) GetPaths() map[string]string { + paths := make(map[string]string, 1) + paths[""] = m.dirPath + return paths +} + func (m *manager) GetCgroups() (*configs.Cgroup, error) { return m.config, nil } diff --git a/libcontainer/cgroups/systemd/unsupported.go b/libcontainer/cgroups/systemd/unsupported.go index 5318a1e91..37fa5035b 100644 --- a/libcontainer/cgroups/systemd/unsupported.go +++ b/libcontainer/cgroups/systemd/unsupported.go @@ -42,8 +42,8 @@ func (m *Manager) GetPaths() map[string]string { return nil } -func (m *Manager) GetUnifiedPath() (string, error) { - return "", fmt.Errorf("Systemd not supported") +func (m *Manager) Path(_ string) string { + return "" } func (m *Manager) GetStats() (*cgroups.Stats, error) { diff --git a/libcontainer/cgroups/systemd/v1.go b/libcontainer/cgroups/systemd/v1.go index 26858517a..70440ea6e 100644 --- a/libcontainer/cgroups/systemd/v1.go +++ b/libcontainer/cgroups/systemd/v1.go @@ -236,15 +236,10 @@ func (m *legacyManager) Destroy() error { return nil } -func (m *legacyManager) GetPaths() map[string]string { +func (m *legacyManager) Path(subsys string) string { m.mu.Lock() - paths := m.paths - m.mu.Unlock() - return paths -} - -func (m *legacyManager) GetUnifiedPath() (string, error) { - return "", errors.New("unified path is only supported when running in unified mode") + defer m.mu.Unlock() + return m.paths[subsys] } func join(c *configs.Cgroup, subsystem string, pid int) (string, error) { @@ -434,6 +429,13 @@ func setKernelMemory(c *configs.Cgroup) error { } return fs.EnableKernelMemoryAccounting(path) } + +func (m *legacyManager) GetPaths() map[string]string { + m.mu.Lock() + defer m.mu.Unlock() + return m.paths +} + func (m *legacyManager) GetCgroups() (*configs.Cgroup, error) { return m.cgroups, nil } diff --git a/libcontainer/cgroups/systemd/v2.go b/libcontainer/cgroups/systemd/v2.go index 4bce04653..d43c855b7 100644 --- a/libcontainer/cgroups/systemd/v2.go +++ b/libcontainer/cgroups/systemd/v2.go @@ -153,8 +153,7 @@ func (m *unifiedManager) Apply(pid int) error { return errors.Wrapf(err, "error while starting unit %q with properties %+v", unitName, properties) } - _, err = m.GetUnifiedPath() - if err != nil { + if err = m.initPath(); err != nil { return err } if err := fs2.CreateCgroupPath(m.path, m.cgroups); err != nil { @@ -188,22 +187,11 @@ func (m *unifiedManager) Destroy() error { return nil } -// this method is for v1 backward compatibility and will be removed -func (m *unifiedManager) GetPaths() map[string]string { - _, _ = m.GetUnifiedPath() - paths := map[string]string{ - "pids": m.path, - "memory": m.path, - "io": m.path, - "cpu": m.path, - "devices": m.path, - "cpuset": m.path, - "freezer": m.path, - } - return paths +func (m *unifiedManager) Path(_ string) string { + return m.path } -// getSliceFull value is used in GetUnifiedPath. +// getSliceFull value is used in initPath. // The value is incompatible with systemdDbus.PropSlice. func (m *unifiedManager) getSliceFull() (string, error) { c := m.cgroups @@ -241,37 +229,35 @@ func (m *unifiedManager) getSliceFull() (string, error) { return slice, nil } -func (m *unifiedManager) GetUnifiedPath() (string, error) { - m.mu.Lock() - defer m.mu.Unlock() +func (m *unifiedManager) initPath() error { if m.path != "" { - return m.path, nil + return nil } sliceFull, err := m.getSliceFull() if err != nil { - return "", err + return err } c := m.cgroups path := filepath.Join(sliceFull, getUnitName(c)) path, err = securejoin.SecureJoin(fs2.UnifiedMountpoint, path) if err != nil { - return "", err + return err } - m.path = path // an example of the final path in rootless: // "/sys/fs/cgroup/user.slice/user-1001.slice/user@1001.service/user.slice/libpod-132ff0d72245e6f13a3bbc6cdc5376886897b60ac59eaa8dea1df7ab959cbf1c.scope" - return m.path, nil + m.path = path + + return nil } func (m *unifiedManager) fsManager() (cgroups.Manager, error) { - path, err := m.GetUnifiedPath() - if err != nil { + if err := m.initPath(); err != nil { return nil, err } - return fs2.NewManager(m.cgroups, path, m.rootless) + return fs2.NewManager(m.cgroups, m.path, m.rootless) } func (m *unifiedManager) Freeze(state configs.FreezerState) error { @@ -283,19 +269,17 @@ func (m *unifiedManager) Freeze(state configs.FreezerState) error { } func (m *unifiedManager) GetPids() ([]int, error) { - path, err := m.GetUnifiedPath() - if err != nil { + if err := m.initPath(); err != nil { return nil, err } - return cgroups.GetPids(path) + return cgroups.GetPids(m.path) } func (m *unifiedManager) GetAllPids() ([]int, error) { - path, err := m.GetUnifiedPath() - if err != nil { + if err := m.initPath(); err != nil { return nil, err } - return cgroups.GetAllPids(path) + return cgroups.GetAllPids(m.path) } func (m *unifiedManager) GetStats() (*cgroups.Stats, error) { @@ -326,6 +310,12 @@ func (m *unifiedManager) Set(container *configs.Config) error { return fsMgr.Set(container) } +func (m *unifiedManager) GetPaths() map[string]string { + paths := make(map[string]string, 1) + paths[""] = m.path + return paths +} + func (m *unifiedManager) GetCgroups() (*configs.Cgroup, error) { return m.cgroups, nil } diff --git a/libcontainer/container_linux.go b/libcontainer/container_linux.go index d4cc35d4d..5cea3d6c2 100644 --- a/libcontainer/container_linux.go +++ b/libcontainer/container_linux.go @@ -64,8 +64,12 @@ type State struct { // Set to true if BaseState.Config.RootlessEUID && BaseState.Config.RootlessCgroups Rootless bool `json:"rootless"` - // Path to all the cgroups setup for a container. Key is cgroup subsystem name - // with the value as the path. + // Paths to all the container's cgroups, as returned by (*cgroups.Manager).GetPaths + // + // For cgroup v1, a key is cgroup subsystem name, and the value is the path + // to the cgroup for this subsystem. + // + // For cgroup v2 unified hierarchy, a key is "", and the value is the unified path. CgroupPaths map[string]string `json:"cgroup_paths"` // NamespacePaths are filepaths to the container's namespaces. Key is the namespace type @@ -648,14 +652,11 @@ func (c *linuxContainer) NotifyOOM() (<-chan struct{}, error) { if c.config.RootlessCgroups { logrus.Warn("getting OOM notifications may fail if you don't have the full access to cgroups") } + path := c.cgroupManager.Path("memory") if cgroups.IsCgroup2UnifiedMode() { - path, err := c.cgroupManager.GetUnifiedPath() - if err != nil { - return nil, err - } return notifyOnOOMV2(path) } - return notifyOnOOM(c.cgroupManager.GetPaths()["memory"]) + return notifyOnOOM(path) } func (c *linuxContainer) NotifyMemoryPressure(level PressureLevel) (<-chan struct{}, error) { @@ -663,7 +664,7 @@ func (c *linuxContainer) NotifyMemoryPressure(level PressureLevel) (<-chan struc if c.config.RootlessCgroups { logrus.Warn("getting memory pressure notifications may fail if you don't have the full access to cgroups") } - return notifyMemoryPressure(c.cgroupManager.GetPaths()["memory"], level) + return notifyMemoryPressure(c.cgroupManager.Path("memory"), level) } var criuFeatures *criurpc.CriuFeatures @@ -1023,23 +1024,12 @@ func (c *linuxContainer) Checkpoint(criuOpts *CriuOpts) error { // CRIU can use cgroup freezer; when rpcOpts.FreezeCgroup // is not set, CRIU uses ptrace() to pause the processes. - var fcg string - switch cgroups.IsCgroup2UnifiedMode() { - case false: - fcg = c.cgroupManager.GetPaths()["freezer"] - case true: - // cgroup v2 freezer is only supported since CRIU release 3.14 - if c.checkCriuVersion(31400) == nil { - fcg, err = c.cgroupManager.GetUnifiedPath() - if err != nil { - // should not happen - return err - } + // Note cgroup v2 freezer is only supported since CRIU release 3.14. + if !cgroups.IsCgroup2UnifiedMode() || c.checkCriuVersion(31400) == nil { + if fcg := c.cgroupManager.Path("freezer"); fcg != "" { + rpcOpts.FreezeCgroup = proto.String(fcg) } } - if fcg != "" { - rpcOpts.FreezeCgroup = proto.String(fcg) - } // append optional criu opts, e.g., page-server and port if criuOpts.PageServer.Address != "" && criuOpts.PageServer.Port != 0 { @@ -1857,23 +1847,17 @@ func (c *linuxContainer) runType() Status { } func (c *linuxContainer) isPaused() (bool, error) { - var fcg, filename, pausedState string + var filename, pausedState string + fcg := c.cgroupManager.Path("freezer") if !cgroups.IsCgroup2UnifiedMode() { - fcg = c.cgroupManager.GetPaths()["freezer"] if fcg == "" { - // A container doesn't have a freezer cgroup + // container doesn't have a freezer cgroup return false, nil } filename = "freezer.state" pausedState = "FROZEN" } else { - var err error - fcg, err = c.cgroupManager.GetUnifiedPath() - if err != nil { - // should not happen - return false, err - } filename = "cgroup.freeze" pausedState = "1" } diff --git a/libcontainer/container_linux_test.go b/libcontainer/container_linux_test.go index 583c1c0c2..356e8f1ad 100644 --- a/libcontainer/container_linux_test.go +++ b/libcontainer/container_linux_test.go @@ -15,11 +15,10 @@ import ( ) type mockCgroupManager struct { - pids []int - allPids []int - stats *cgroups.Stats - paths map[string]string - unifiedPath string + pids []int + allPids []int + stats *cgroups.Stats + paths map[string]string } type mockIntelRdtManager struct { @@ -55,8 +54,8 @@ func (m *mockCgroupManager) GetPaths() map[string]string { return m.paths } -func (m *mockCgroupManager) GetUnifiedPath() (string, error) { - return m.unifiedPath, nil +func (m *mockCgroupManager) Path(subsys string) string { + return m.paths[subsys] } func (m *mockCgroupManager) Freeze(state configs.FreezerState) error {