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

Initial v2 resources.unified systemd support #2669

Merged
merged 11 commits into from
Nov 6, 2020

Conversation

kolyshkin
Copy link
Contributor

@kolyshkin kolyshkin commented Oct 30, 2020

In case systemd is used as cgroups manager, and a user sets some
resources using unified resource map (as per opencontainers/runtime-spec#1040),
systemd is not aware of any parameters, so there will be a discrepancy between
the cgroupfs state and systemd unit state.

Let's try to fix that by converting known unified resources to systemd
properties.

Resources/properties currently supported:

  • pids.max / TasksMax
  • cpu.max / CPUQuota*
  • cpu.weight / CPUWeight
  • cpuset.cpus / AllowedCPUs
  • cpuset.mems / AllowedMemoryNodes
  • memory.{high,low,min,max,swap.max} / Memory{High,Low,Min,Max,SwapMap}

TODO:

  • io.weight, io.max, io.latency / IO* (not in this PR)

ℹ️ While at it, fix runc update wrt unified resources which was missing from #2584.

Fixes: #2603

@kolyshkin
Copy link
Contributor Author

This is just a POC for now, @AkihiroSuda @mrunalp @cyphar PTAL.

Going to add more resources later.

@kolyshkin kolyshkin force-pushed the systemd-unified branch 2 times, most recently from fa06cf5 to 0975020 Compare November 4, 2020 00:18
@kolyshkin
Copy link
Contributor Author

Update: added cpu.max (aka CPUQuota*), fix runc update wrt resources.unified.

@kolyshkin kolyshkin force-pushed the systemd-unified branch 4 times, most recently from b7160aa to 5ec1cae Compare November 4, 2020 03:39
@kolyshkin kolyshkin changed the title [WIP/RFC] Initial v2 resources.unified systemd support Initial v2 resources.unified systemd support Nov 4, 2020
@kolyshkin kolyshkin marked this pull request as ready for review November 4, 2020 03:44
AkihiroSuda
AkihiroSuda previously approved these changes Nov 4, 2020
@kolyshkin
Copy link
Contributor Author

Fixed a stupid bug with cpu.max handling

mrunalp
mrunalp previously approved these changes Nov 5, 2020
@kolyshkin kolyshkin marked this pull request as draft November 5, 2020 05:01
@kolyshkin
Copy link
Contributor Author

OK, cpuset. conversion to DBus property needs more attention, I'll continue tomorrow.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Those were missing for some reason, so we did not fail the test in case
"runc update" returned an error.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In case systemd is used as cgroups manager, and a user sets some
resources using unified resource map (as per [1]), systemd is not
aware of any parameters, so there will be a discrepancy between
the cgroupfs state and systemd unit state.

Let's try to fix that by converting known unified resources to systemd
properties.

Currently, this is only implemented for pids.max as a POC.

Some other parameters (that might or might not have systemd unit
property equivalents) are:

$ ls -l | grep w-
-rw-r--r--. 1 root root 0 Oct 10 13:57 cgroup.freeze
-rw-r--r--. 1 root root 0 Oct 10 13:57 cgroup.max.depth
-rw-r--r--. 1 root root 0 Oct 10 13:57 cgroup.max.descendants
-rw-r--r--. 1 root root 0 Oct 10 13:57 cgroup.procs
-rw-r--r--. 1 root root 0 Oct 21 09:43 cgroup.subtree_control
-rw-r--r--. 1 root root 0 Oct 10 13:57 cgroup.threads
-rw-r--r--. 1 root root 0 Oct 10 13:57 cgroup.type
-rw-r--r--. 1 root root 0 Oct 22 10:30 cpu.max
-rw-r--r--. 1 root root 0 Oct 10 13:57 cpu.pressure
-rw-r--r--. 1 root root 0 Oct 22 10:30 cpuset.cpus
-rw-r--r--. 1 root root 0 Oct 22 10:30 cpuset.cpus.partition
-rw-r--r--. 1 root root 0 Oct 22 10:30 cpuset.mems
-rw-r--r--. 1 root root 0 Oct 22 10:30 cpu.weight
-rw-r--r--. 1 root root 0 Oct 22 10:30 cpu.weight.nice
-rw-r--r--. 1 root root 0 Oct 22 10:30 hugetlb.1GB.max
-rw-r--r--. 1 root root 0 Oct 22 10:30 hugetlb.1GB.rsvd.max
-rw-r--r--. 1 root root 0 Oct 22 10:30 hugetlb.2MB.max
-rw-r--r--. 1 root root 0 Oct 22 10:30 hugetlb.2MB.rsvd.max
-rw-r--r--. 1 root root 0 Oct 22 10:30 io.bfq.weight
-rw-r--r--. 1 root root 0 Oct 22 10:30 io.latency
-rw-r--r--. 1 root root 0 Oct 22 10:30 io.max
-rw-r--r--. 1 root root 0 Oct 10 13:57 io.pressure
-rw-r--r--. 1 root root 0 Oct 22 10:30 io.weight
-rw-r--r--. 1 root root 0 Oct 10 13:57 memory.high
-rw-r--r--. 1 root root 0 Oct 10 13:57 memory.low
-rw-r--r--. 1 root root 0 Oct 10 13:57 memory.max
-rw-r--r--. 1 root root 0 Oct 10 13:57 memory.min
-rw-r--r--. 1 root root 0 Oct 10 13:57 memory.oom.group
-rw-r--r--. 1 root root 0 Oct 10 13:57 memory.pressure
-rw-r--r--. 1 root root 0 Oct 10 13:57 memory.swap.high
-rw-r--r--. 1 root root 0 Oct 10 13:57 memory.swap.max

Surely, it is a manual conversion for every such case...

[1] opencontainers/runtime-spec#1040

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Un-inline defCPUQuotaPeriod constant. To be used by the next commit.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
To be used by the following commit.

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>
 * cpuset.cpus -> AllowedCPUs
 * cpuset.mems -> AllowedMemoryNodes

No test for cgroup v2 resources.unified override, as this requires a
separate test case, and all the unified resources are handled uniformly
so there's little sense to test all parameters.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@kolyshkin kolyshkin marked this pull request as ready for review November 6, 2020 03:53
@kolyshkin
Copy link
Contributor Author

OK, cpuset. conversion to DBus property needs more attention, I'll continue tomorrow.

Made it working; PTAL @mrunalp @AkihiroSuda

@mrunalp mrunalp merged commit 27227a9 into opencontainers:master Nov 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cgroups.Resources.Unified support wrt systemd driver
4 participants