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

libc/cg: convert r.CPU.Cpus/Mems to systemd props #2727

Merged
merged 3 commits into from
Jan 21, 2021

Conversation

kolyshkin
Copy link
Contributor

@kolyshkin kolyshkin commented Jan 8, 2021

Support for systemd properties AllowedCPUs and AllowedMemoryNodes
was added by commit 13afa58, but only for unified resources
of systemd v2 driver.

This adds support for Cpu.Cpus and Cpu.Mems resources to
both systemd v1 and v2 cgroup drivers. If systemd is not new
enough to support the properties, a warning is logged.

An integration test is added to check that the settings work.

While at it:

  • log a warning in a different place checking systemd version;
  • add version check to systemd unified support handling.

@kolyshkin kolyshkin added this to the 1.0.0-rc93 milestone Jan 11, 2021
@kolyshkin kolyshkin changed the title [WIP] libc/cg: convert r.CPU.Cpus/Mems to systemd props libc/cg: convert r.CPU.Cpus/Mems to systemd props Jan 11, 2021
@kolyshkin
Copy link
Contributor Author

This one is ready

@kolyshkin
Copy link
Contributor Author

time="2021-01-14T01:13:19Z" level=error msg="container_linux.go:367: starting container process caused: process_linux.go:340: applying cgroup configuration for process caused: Cannot set property AllowedCPUs, or unknown property."
150

So we need systemd version check...

@kolyshkin kolyshkin marked this pull request as draft January 14, 2021 01:22
As the caller of this function just logs the error, it does not make
sense to pass it. Instead, log it (once) and return -1.

This is a preparation for the second user.

Signed-off-by: Kir Kolyshkin <[email protected]>
Support for systemd properties AllowedCPUs and AllowedMemoryNodes
was added by commit 13afa58, but only for unified resources
of systemd v2 driver.

This adds support for Cpu.Cpus and Cpu.Mems resources to
both systemd v1 and v2 cgroup drivers.

An integration test is added to check that the settings work.

[v2: check for systemd version]
[v3: same in the test]

Signed-off-by: Kir Kolyshkin <[email protected]>
1. Add a check to unifiedResToSystemdProps that systemd is recent enough
   to support AllowedCPUs/AllowedMemoryNodes unit properties, and skip
   setting the property if it is not supported.

   Note that this is not an error as the setting is still applied to
   the underlying cgroupfs -- it's just systemd unit property that is
   being skipped.

2. In all the places we skip an unsupported property, warn about it.

Signed-off-by: Kir Kolyshkin <[email protected]>
@kolyshkin
Copy link
Contributor Author

not ok 104 update cpuset parameters via resources.CPU
# (from function `check_systemd_value' in file tests/integration/helpers.bash, line 207,
#  in test file tests/integration/update.bats, line 407)
#   `check_systemd_value "AllowedCPUs" 0' failed
# runc spec (status=0):
# runc run -d --console-socket /tmp/console.sock test_update (status=0):
# time="2021-01-14T02:35:35Z" level=warning msg="systemd v219 is too old to support CPUQuotaPeriodSec  (setting will still be applied to cgroupfs)"
# time="2021-01-14T02:35:35Z" level=warning msg="systemd v219 is too old to support AllowedCPUs/AllowedMemoryNodes (settings will still be applied to cgroupfs)"
# time="2021-01-14T02:35:35Z" level=warning msg="systemd v219 is too old to support CPUQuotaPeriodSec  (setting will still be applied to cgroupfs)"
# time="2021-01-14T02:35:35Z" level=warning msg="systemd v219 is too old to support AllowedCPUs/AllowedMemoryNodes (settings will still be applied to cgroupfs)"
# systemd AllowedCPUs: current  !? 0 
ok 105 update cpuset parameters via v2 unified map # skip test requires cgroups_v2
ok 106 update rt period and runtime # skip test requires no_systemd
ok 107 update devices [minimal transition rules]
ok 108 update paused container
ok 109 runc version
make: *** [localintegration] Error 1
make: Leaving directory `/vagrant'
Script started, file is typescript

Two issues:

  1. test is run on centos7 where it obviously fails, but it should be skipped. fixed
  2. test has failed and test no failure is reported. Investigating

@kolyshkin kolyshkin marked this pull request as ready for review January 14, 2021 18:49
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.

3 participants