libct/configs: move cgroup configs to libct/cgroups#4472
libct/configs: move cgroup configs to libct/cgroups#4472AkihiroSuda merged 5 commits intoopencontainers:mainfrom
Conversation
e90eb15 to
6b55c70
Compare
|
No longer a draft; PTAL @opencontainers/runc-maintainers 🙏🏻 This is the first step to implement the proposal to split libcontainer/cgroups out of runc |
dims
left a comment
There was a problem hiding this comment.
Love the clean separation. thank you!
(please remove [draft] from the PR title as well?)
|
|
||
| import cg "github.com/opencontainers/runc/libcontainer/cgroups/configs" | ||
|
|
||
| // Deprecated: use [github.com/opencontainers/runc/libcontainer/cgroups/configs]. |
There was a problem hiding this comment.
Not really important (just an alternative) as these don't point to specific types, I think it's possible to just add the // Deprecated: to the package;
package configs // Deprecated: use [github.com/opencontainers/runc/libcontainer/cgroups/configs].
libcontainer/container_linux.go
Outdated
| "golang.org/x/sys/unix" | ||
|
|
||
| "github.com/opencontainers/runc/libcontainer/cgroups" | ||
| cgConfig "github.com/opencontainers/runc/libcontainer/cgroups/configs" |
There was a problem hiding this comment.
do you know if these types are also imported elsewhere separate from the cgroups package itself? considering if that was not the case, these types could potentially be merged with the cgroups package 🤔
There was a problem hiding this comment.
Good question.
Looks like there's no need to have a separate package for cgroups-related structures.
The PR is now updated to use libcontainer/cgroups. The only negative effect is the patch is now bigger (had to replace configs with cgroups everywhere.
PTAL @thaJeztah 🙏🏻
There was a problem hiding this comment.
Thanks! I wanted to give the changes a quick try to see if nothing bad fell out of it, so I opened some test-PRs in moby to double-check;
efab503 to
157e5c6
Compare
These:
> Error: libcontainer/cgroups/fs2/cpu.go:15:6: var-naming: func isCpuSet should be isCPUSet (revive)
> func isCpuSet(r *cgroups.Resources) bool {
> ^
> Error: libcontainer/cgroups/fs2/cpu.go:19:6: var-naming: func setCpu should be setCPU (revive)
> func setCpu(dirPath string, r *cgroups.Resources) error {
> ^
They are going to be shown after next commits because of linter-extra CI
job (which, due to major changes, now thinks it's a new code so extra
linters apply).
Fixing it beforehand.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
We have quite a few external users of libcontainer/cgroups packages, and they all have to depend on libcontainer/configs as well. Let's move cgroup-related configuration to libcontainer/croups. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This PR moves cgroup-related configuration from libcontainer/configs to libcontainer/cgroups,
while providing backward-compatible aliases.
This is the first step to implement the proposal to split libcontainer/cgroups out of runc (which stems from this k8s discussion).
Updates: #4618