-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Simplify cgroup path handing in v2 via unified API
This unties the Gordian Knot of using GetPaths in cgroupv2 code.
The problem is, the current code uses GetPaths for three kinds of things:
1. Get all the paths to cgroup v1 controllers to save its state (see
(*linuxContainer).currentState(), (*LinuxFactory).loadState()
methods).
2. Get all the paths to cgroup v1 controllers to have the setns process
enter the proper cgroups in `(*setnsProcess).start()`.
3. Get the path to a specific controller (for example,
`m.GetPaths()["devices"]`).
Now, for cgroup v2 instead of a set of per-controller paths, we have only
one single unified path, and a dedicated function `GetUnifiedPath()` to get it.
This discrepancy between v1 and v2 cgroupManager API leads to the
following problems with the code:
- multiple if/else code blocks that have to treat v1 and v2 separately;
- backward-compatible GetPaths() methods in v2 controllers;
- - repeated writing of the PID into the same cgroup for v2;
Overall, it's hard to write the right code with all this, and the code
that is written is kinda hard to follow.
The solution is to slightly change the API to do the 3 things outlined
above in the same manner for v1 and v2:
1. Use `GetPaths()` for state saving and setns process cgroups entering.
2. Introduce and use Path(subsys string) to obtain a path to a
subsystem. For v2, the argument is ignored and the unified path is
returned.
This commit converts all the controllers to the new API, and modifies
all the users to use it.
Signed-off-by: Kir Kolyshkin <[email protected]>- Loading branch information
Showing
8 changed files
with
88 additions
and
127 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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/[email protected]/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 | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.