spec: linux: add support for the PIDs cgroup #64
Conversation
|
Please note that Linux 4.3 still hasn't hit, so I'd recommend abstaining from merging this until we actually hit 4.3-rc1 (which will be in 6 weeks or so), unless we're okay with merging a no-op configuration feature. |
|
LGTM, but let's wait until upstream kernel merges the pids controller. |
|
Thanks @cyphar. We will wait for the upstream. |
|
I am marking this for the post-draft release in hopes it will be merged with no problems by then :) |
spec_linux.go
Outdated
There was a problem hiding this comment.
If the pid limit is not immutable, it might make sense to set to 0 for some use cases. Should we be restricting the input domain in the spec? What if I want an empty container to hold a network namespace for other containers to use? Are there use cases for containers without processes?
There was a problem hiding this comment.
I guess that's a fair point. It does make it slightly annoying because the default value will have to be -1 (which means we can't do new(spec.Linux) without modifying it each time). I'll fix this later.
There was a problem hiding this comment.
Actually, I'm not sure that you're right. In order for the PID limit to make sense, you have to have a process in the cgroup. Nothing stops you from attaching more processes to a cgroup, so it's illogical to claim that a PID limit of 0 has any real use. Combine that with the annoyance of not being able to do new(spec.Linux) or spec.Linux{}, and it's clear that a limit of 0 is not useful.
|
The PIDs changes have been merged into Linus' tree as of torvalds/linux@8bdc69b. I'll update this patch in a bit. |
|
nicely |
Add support for the PIDs cgroup as a cgroup resource constraint in the Linux container specification. Since PIDs are a real resource, we need to support the ability to limit them. The PIDs cgroup subsystem is available in Linux 4.3+. Signed-off-by: Aleksa Sarai <[email protected]>
|
merged upstream, lgtm. |
|
Will some of these features set platform version boundaries?
|
|
@vbatts I feel it is sort of up to the runtime to decide whether it will/won't run something based on its configuration. |
|
Fair
|
|
So, cool. |
spec: linux: add support for the PIDs cgroup
8 participants
Add support for the PIDs cgroup as a cgroup resource constraint in the
Linux container specification. Since PIDs are a real resource, we need
to support the ability to limit them.
The PIDs cgroup subsystem is available in Linux 4.3+.
Signed-off-by: Aleksa Sarai [email protected]
I realised while implementing opencontainers/runc#58 that if we want to
have PIDs support in the
config.jsonwe need to update the spec too.