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

[post 1.0] RFC: separate out libcontainer/cgroups, maybe merge with containerd/cgroups #3007

Open
kolyshkin opened this issue Jun 7, 2021 · 11 comments
Milestone

Comments

@kolyshkin
Copy link
Contributor

Despite the warnings we have, libcontainer is used by lots of other projects, like kubernetes, cri-o etc. One particular piece is libcontainer/cgroups.

I think we should separate libcontainer/cgroups out to say opencontainers/cgroups, and possibly merge with containerd/cgroups (which from the first glance has better API but probably not as practical implementation esp for cgroup v2). Maybe the way to go is

  1. move containerd/cgroups under opencontainers
  2. port all the good parts of libcontainer/cgroups to the new opencontainers/cgroups
  3. adopt runc and other users of libcontainer/cgroups to the new opencontainers/cgroups

This is a big and potentially disruptive project but it should clear things up a lot,
resulting in better code, clearer APIs, and less effort duplication.

(Ultimately, all of libcontainer should either be separated out or moved into internal, but this is a larger goal)

WDYT @opencontainers/runc-maintainers @crosbymichael @thaJeztah @estesp @dmcgowan

@kolyshkin kolyshkin added this to the post-1.0 milestone Jun 7, 2021
@mrunalp
Copy link
Contributor

mrunalp commented Jun 7, 2021

👍

@thaJeztah
Copy link
Member

Definitely 👍. I don't see benefit in multiple projects maintaining code for handling this, so if we can share the effort in maintaining it, would (IMO) benefit everyone. Handling cgroups is already "hairy" (with lots of oddities).
Happy to help with this (even if only to "chop wood / carry water")

@AkihiroSuda
Copy link
Member

Separating out the go module might be fine, but separating the repo will cause "vendoring hell", so I still prefer monorepo

@thaJeztah
Copy link
Member

@AkihiroSuda any specific repositories where you think this would become vendoring hell? (Thinking of circular dependencies). Of course, the repo would need to try to stick to SemVer(isn) (no breaking changes).

@AkihiroSuda
Copy link
Member

The repo of runc and libct/cgroups themselves will face issues, as testing runc requires libct/cgroups and vice versa.

For similar reason, we merged back libnetwork into Moby.

@cyphar
Copy link
Member

cyphar commented Jun 8, 2021

Alternatively we can just move all of libcontainer into an internal package (which Go won't let you import) and we just leave libcontainer/cgroups in an importable path. I had planned to do something like this for a while. Same goes with libcontainer/user (Docker uses this, as well as a few other functions).

I think this would be far more preferable to having a separate repo -- and if we plan to have more regular releases than that isn't a strong argument to have a separate repo either.

@AkihiroSuda
Copy link
Member

Alternatively we can just move all of libcontainer into an internal package (which Go won't let you import) and we just leave libcontainer/cgroups in an importable path. I had planned to do something like this for a while.

SGTM

@kolyshkin
Copy link
Contributor Author

Yeah, moving parts of libcontainer (that are not used by other projects) to under internal would be a good thing to do, we should try that after 1.0

@rhatdan
Copy link
Contributor

rhatdan commented Jun 9, 2021

Can we still consolidate the cgroup handling from containerd and libcontainer?

@kolyshkin
Copy link
Contributor Author

Can we still consolidate the cgroup handling from containerd and libcontainer?

Ultimately this is still my goal, although it's not yet clear how to do it in a monorepo. Maybe we can just steal/adopt containerd/cgroups API, while mostly retaining libcontainer/cgroups implementation. All this is after #3007 (comment) is implemented.

@dims
Copy link
Contributor

dims commented Jun 22, 2021

@kolyshkin can we please do this for libcontainer/user as well? will help us in containerd ( containerd/containerd#5635 (comment) )

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

No branches or pull requests

7 participants