search

lxc / lxcri

CRI-O support for lxc
Apache License 2.0
67 stars 19 forks source link

runtime: Implement cgroup2 resource limits. #33

Open r10r opened 3 years ago

r10r commented 3 years ago

Implement resource limits for cgroup2

https://github.com/opencontainers/runtime-spec/blob/master/config-linux.md#control-groups

mikemccracken commented 3 years ago

Verify whether either kubelet or crio already set resource limits, since there are no failing sonobuoy tests regarding cgroup resource limits in the default testset.

Not sure if this is exactly what you were talking about, but crio does include a global default pid_limit in crio.conf. I noticed it today, apparently it's kind of low: https://github.com/cri-o/cri-o/issues/1921

r10r commented 3 years ago

Thanks for the pointer. The pid limit is one of the few limits that is already set

https://github.com/lxc/lxcri/blob/cbc79e17a3ee7bfe321b439ed22e627803488772/cgroup.go#L100

The spec implies that the runtime is reponsible for setting the cgroup limits. If this is the case - then why are there no failing tests, because currently lxcri only set's very of them?

The CgroupPath is part of the OCI spec and is passed to the runtime, and if I'm not mistaken the parent cgroup path already exists. This leads me to the conclusion that either the limits are already set by cri-o or kubelet on the parent cgroup, or there are no tests (within the default testset ) that check that the limits actually work.

Cgroup2 support in the OCI spec is still in progress and I want to make sure that the limits are correctly applied and don't break anything.

But maybe we can just enable the limits and add a switch to manually disable them --cgroup-limits ? Similar to

 --cgroup-devices             allow only devices permitted by container spec (default: true) [$LXCRI_CGROUP_DEVICES]

What do you think?