Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] cgroup2: the first cut #102

Closed
wants to merge 1 commit into from

Conversation

AkihiroSuda
Copy link
Member

@AkihiroSuda AkihiroSuda commented Oct 26, 2019

Currently, only PID controller is implemented as an example.
Other controllers will be implemented in other PRs.

Interface changes:

  • Cgroup interface now excludes V1-specific and V2-specific methods.
    type CgroupV1 interface{Cgroup, ...} extends Cgroup for V1-specific methods.
    Same applies to type CgroupV2 interface{Cgroup, ...}.
diff --git a/control.go b/control.go
index a024fd6..eb27708 100644
--- a/control.go
+++ b/control.go
@@ -20,6 +20,7 @@ import (
        "os"
 
        v1 "github.com/containerd/cgroups/stats/v1"
+       v2 "github.com/containerd/cgroups/stats/v2"
        specs "github.com/opencontainers/runtime-spec/specs-go"
 )
 
@@ -61,29 +62,42 @@ type Cgroup interface {
        New(string, *specs.LinuxResources) (Cgroup, error)
        // Add adds a process to the cgroup (cgroup.procs)
        Add(Process) error
-       // AddTask adds a process to the cgroup (tasks)
-       AddTask(Process) error
        // Delete removes the cgroup as a whole
        Delete() error
        // MoveTo moves all the processes under the calling cgroup to the provided one
        // subsystems are moved one at a time
        MoveTo(Cgroup) error
-       // Stat returns the stats for all subsystems in the cgroup
-       Stat(...ErrorHandler) (*v1.Metrics, error)
        // Update updates all the subsystems with the provided resource changes
        Update(resources *specs.LinuxResources) error
-       // Processes returns all the processes in a select subsystem for the cgroup
-       Processes(Name, bool) ([]Process, error)
-       // Tasks returns all the tasks in a select subsystem for the cgroup
-       Tasks(Name, bool) ([]Task, error)
        // Freeze freezes or pauses all processes inside the cgroup
        Freeze() error
        // Thaw thaw or resumes all processes inside the cgroup
        Thaw() error
-       // OOMEventFD returns the memory subsystem's event fd for OOM events
-       OOMEventFD() (uintptr, error)
        // State returns the cgroups current state
        State() State
        // Subsystems returns all the subsystems in the cgroup
        Subsystems() []Subsystem
 }
+
+type CgroupV1 interface {
+       Cgroup
+       // AddTask adds a process to the cgroup (tasks)
+       AddTask(Process) error
+       // Stat returns the stats for all subsystems in the cgroup
+       Stat(...ErrorHandler) (*v1.Metrics, error)
+       // Processes returns all the processes in a select subsystem for the cgroup
+       Processes(Name, bool) ([]Process, error)
+       // Tasks returns all the tasks in a select subsystem for the cgroup
+       Tasks(Name, bool) ([]Task, error)
+       // OOMEventFD returns the memory subsystem's event fd for OOM events
+       OOMEventFD() (uintptr, error)
+}
+
+type CgroupV2 interface {
+       Cgroup
+       // Stat returns the stats for all subsystems in the cgroup
+       Stat(...ErrorHandler) (*v2.Metrics, error)
+       // Processes returns all the processes in the cgroup
+       Processes(bool) ([]Process, error)
+       // TODO: support memory.events
+}
  • Now Hierarchy() returns whether the hierarchy is in unified-mode or not.
diff --git a/hierarchy.go b/hierarchy.go
index 9221bf3..ac088c5 100644
--- a/hierarchy.go
+++ b/hierarchy.go
@@ -17,4 +17,4 @@
 package cgroups
 
 // Hierarchy enableds both unified and split hierarchy for cgroups
-type Hierarchy func() ([]Subsystem, error)
+type Hierarchy func() (subsystems []Subsystem, unifiedMode bool, err error)

cmd/cgroupsplayground/main.go is included as an example of the new API.

@AkihiroSuda AkihiroSuda force-pushed the cgroup2-poc branch 2 times, most recently from 397762d to 5d0860d Compare October 26, 2019 14:42
Currently, only PID controller is implemented as an example.
Other controllers will be implemented in other PRs.

Interface changes:
* `Cgroup` interface now excludes V1-specific and V2-specific methods.
  `type CgroupV1 interface{Cgroup, ...}` extends `Cgroup` for V1-specific methods.
  Same applies to `type CgroupV2 interface{Cgroup, ...}`.

* Now `Hierarchy()` returns whether the hierarchy is in unified-mode or not.

`cmd/cgroupsplayground/main.go` is included as an example of the new API.

Signed-off-by: Akihiro Suda <[email protected]>
@codecov-io
Copy link

codecov-io commented Oct 26, 2019

Codecov Report

Merging #102 into master will decrease coverage by 3.67%.
The diff coverage is 20.58%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #102      +/-   ##
==========================================
- Coverage   45.42%   41.74%   -3.68%     
==========================================
  Files          23       26       +3     
  Lines        1638     1806     +168     
==========================================
+ Hits          744      754      +10     
- Misses        768      917     +149     
- Partials      126      135       +9
Impacted Files Coverage Δ
errors.go 50% <ø> (ø) ⬆️
paths.go 57.4% <0%> (-13.05%) ⬇️
systemd.go 0% <0%> (ø) ⬆️
subsystem.go 60.71% <0%> (ø) ⬆️
v1.go 35.89% <0%> (-4.11%) ⬇️
v2.go 0% <0%> (ø)
subsystem_v2.go 0% <0%> (ø)
pids_v2.go 0% <0%> (ø)
utils.go 61.57% <10.81%> (-9.94%) ⬇️
cgroup.go 41.64% <40.86%> (-6.8%) ⬇️
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5fbad35...b5526ec. Read the comment docs.

@AkihiroSuda
Copy link
Member Author

RFC: when cgroup.type is threaded, Processes() should return threads as []Process? Or it should return an error?

@crosbymichael
Copy link
Member

We could do this or we can make a new v2 subdir and have a v2 native API to it. Then we can make a wrapper that adapts the v2 api into the current interfaces that is in the root of the tree.

I say this, because if we think of cgroup usage for the next few years, it maybe cleaner to have a "pure" v2 implementation apart from v1 for maintenance

@AkihiroSuda
Copy link
Member Author

Do you plan to open PR?

AkihiroSuda added a commit to AkihiroSuda/containerd-cgroups that referenced this pull request Oct 30, 2019
alternative to containerd#102

v2 package is completely split from v1.

Signed-off-by: Akihiro Suda <[email protected]>
@AkihiroSuda
Copy link
Member Author

opened #103, RFC

AkihiroSuda added a commit to AkihiroSuda/containerd-cgroups that referenced this pull request Oct 30, 2019
alternative to containerd#102

v2 package is completely split from v1.

Signed-off-by: Akihiro Suda <[email protected]>
AkihiroSuda added a commit to AkihiroSuda/containerd-cgroups that referenced this pull request Oct 30, 2019
alternative to containerd#102

v2 package is completely split from v1.

Signed-off-by: Akihiro Suda <[email protected]>
AkihiroSuda added a commit to AkihiroSuda/containerd-cgroups that referenced this pull request Nov 1, 2019
alternative to containerd#102

v2 package is completely split from v1.

Signed-off-by: Akihiro Suda <[email protected]>
AkihiroSuda added a commit to AkihiroSuda/containerd-cgroups that referenced this pull request Nov 1, 2019
alternative to containerd#102

v2 package is completely split from v1.

Signed-off-by: Akihiro Suda <[email protected]>
AkihiroSuda added a commit to AkihiroSuda/containerd-cgroups that referenced this pull request Nov 4, 2019
alternative to containerd#102

v2 package is completely split from v1.

Signed-off-by: Akihiro Suda <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants