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.

Sign up for GitHub

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

Enable rootless cgroup v2 tests + some fixes #2944

Merged
merged 8 commits into from
May 10, 2021
+36 −51
Merged

Enable rootless cgroup v2 tests + some fixes #2944

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
0ed1f80
tests/int/helpers: rm old code
kolyshkin May 6, 2021
4f8ccc5
libct/cg/sd/v2: call initPath from Path
kolyshkin May 6, 2021
353f2ad
tests/int/update.bats: don't set cpuset in setup
kolyshkin May 7, 2021
44fcbfd
tests/int/helpers: generalize require cgroups_freezer
kolyshkin May 7, 2021
40b9791
tests/int: enable/use requires cgroups_<ctrl>
kolyshkin May 7, 2021
601cf58
tests/int/cgroups: don't check for hugetlb
kolyshkin May 7, 2021
ac70a9a
tests/int: run rootless_cgroup tests for v2+systemd
kolyshkin May 6, 2021
12e9cac
Vagrantfile.fedora: set Delegate=yes
kolyshkin May 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Vagrantfile.fedora34
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ EOF
cat > /etc/systemd/system/user@.service.d/delegate.conf << EOF
[Service]
# default: Delegate=pids memory
# NOTE: delegation of cpuset requires systemd >= 244 (Fedora >= 32, Ubuntu >= 20.04). cpuset is ignored on Fedora 31.
Delegate=cpu cpuset io memory pids
# NOTE: delegation of cpuset requires systemd >= 244 (Fedora >= 32, Ubuntu >= 20.04).
Delegate=yes
EOF
systemctl daemon-reload
SHELL
Expand Down
1 change: 1 addition & 0 deletions libcontainer/cgroups/systemd/v2.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,7 @@ func (m *unifiedManager) Destroy() error {
}

func (m *unifiedManager) Path(_ string) string {
_ = m.initPath()
return m.path
}

Expand Down
14 changes: 4 additions & 10 deletions tests/integration/cgroups.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,7 @@ function setup() {
}

@test "runc create (rootless + no limits + cgrouppath + no permission) fails with permission error" {
requires rootless
requires rootless_no_cgroup
# systemd controls the permission, so error does not happen
requires no_systemd
requires rootless rootless_no_cgroup

set_cgroups_path

Expand All @@ -29,10 +26,7 @@ function setup() {
}

@test "runc create (rootless + limits + no cgrouppath + no permission) fails with informative error" {
requires rootless
requires rootless_no_cgroup
# systemd controls the permission, so error does not happen
requires no_systemd
requires rootless rootless_no_cgroup

set_resources_limit

Expand All @@ -55,8 +49,8 @@ function setup() {
if [ "$(id -u)" = "0" ]; then
check_cgroup_value "cgroup.controllers" "$(cat /sys/fs/cgroup/machine.slice/cgroup.controllers)"
else
# shellcheck disable=SC2046
check_cgroup_value "cgroup.controllers" "$(cat /sys/fs/cgroup/user.slice/user-$(id -u).slice/cgroup.controllers)"
# Filter out hugetlb as systemd is unable to delegate it.
check_cgroup_value "cgroup.controllers" "$(sed 's/ hugetlb//' </sys/fs/cgroup/user.slice/user-"$(id -u)".slice/cgroup.controllers)"
fi
else
check_cgroup_value "cgroup.controllers" "$(cat /sys/fs/cgroup/cgroup.controllers)"
Expand Down
43 changes: 23 additions & 20 deletions tests/integration/helpers.bash
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,6 @@ function runc_spec() {
if [[ "$ROOTLESS" -ne 0 ]] && [[ "$ROOTLESS_FEATURES" == *"idmap"* ]]; then
runc_rootless_idmap "$bundle"
fi

# Ensure config.json contains linux.resources
if [[ "$ROOTLESS" -ne 0 ]] && [[ "$ROOTLESS_FEATURES" == *"cgroup"* ]]; then
runc_rootless_cgroup "$bundle"
fi
}

# Helper function to reformat config.json file. Input uses jq syntax.
Expand All @@ -92,12 +87,6 @@ function runc_rootless_idmap() {
| .linux.gidMappings += [{"hostID": '"$(($ROOTLESS_GIDMAP_START + 100))"', "containerID": 1000, "size": '"$(($ROOTLESS_GIDMAP_LENGTH - 1000))"'}]' $bundle
}

# Shortcut to add empty resources as part of a rootless configuration.
function runc_rootless_cgroup() {
bundle="${1:-.}"
update_config '.linux.resources += {"memory":{},"cpu":{},"blockio":{},"pids":{}}' $bundle
}

# Returns systemd version as a number (-1 if systemd is not enabled/supported).
function systemd_version() {
if [ -n "${RUNC_USE_SYSTEMD}" ]; then
Expand All @@ -114,13 +103,21 @@ function init_cgroup_paths() {

if stat -f -c %t /sys/fs/cgroup | grep -qFw 63677270; then
CGROUP_UNIFIED=yes
local controllers="/sys/fs/cgroup/cgroup.controllers"
# For rootless + systemd case, controllers delegation is required,
# so check the controllers that the current user has, not the top one.
# NOTE: delegation of cpuset requires systemd >= 244 (Fedora >= 32, Ubuntu >= 20.04).
if [[ "$ROOTLESS" -ne 0 && -n "$RUNC_USE_SYSTEMD" ]]; then
controllers="/sys/fs/cgroup/user.slice/user-$(id -u).slice/user@$(id -u).service/cgroup.controllers"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better to check cgroup.subtree_control?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Theoretically, if systemd did the job of delegation right, it does not matter.

Fixed nevertheless.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, it seems that was wrong to change cgroup.controllers to cgroup.subtree_control. The first one contains all the controllers that systemd can delegate, while the second one only has a few (pids and memory, if I remember correctly).

Now, as far as I understand, fs2.CreateCgroupPath (called from unifiedManager's Apply) fixes that, enabling all supported controllers and subtree_control, but the tests check it earlier.

Anyway, when I changed it back to cgroups.controllers, it is working, and the tests that require cpuset are now not skipped on cgroup v2 + systemd + rootless.

fi

# "pseudo" controllers do not appear in /sys/fs/cgroup/cgroup.controllers.
# - devices (since kernel 4.15) we must assume to be supported because
# it's quite hard to test.
# - freezer (since kernel 5.2) we can auto-detect by looking for the
# "cgroup.freeze" file a *non-root* cgroup.
CGROUP_SUBSYSTEMS=$(
cat /sys/fs/cgroup/cgroup.controllers
cat "$controllers"
echo devices
)
CGROUP_BASE_PATH=/sys/fs/cgroup
Expand Down Expand Up @@ -274,6 +271,11 @@ function fail() {
exit 1
}

# Check whether rootless runc can use cgroups.
function rootless_cgroup() {
[[ "$ROOTLESS_FEATURES" == *"cgroup"* || -n "$RUNC_USE_SYSTEMD" ]]
}

# Allows a test to specify what things it requires. If the environment can't
# support it, the test is skipped with a message.
function requires() {
Expand Down Expand Up @@ -301,12 +303,12 @@ function requires() {
fi
;;
rootless_cgroup)
if [[ "$ROOTLESS_FEATURES" != *"cgroup"* ]]; then
if ! rootless_cgroup; then
skip_me=1
fi
;;
rootless_no_cgroup)
if [[ "$ROOTLESS_FEATURES" == *"cgroup"* ]]; then
if rootless_cgroup; then
skip_me=1
fi
;;
Expand All @@ -315,12 +317,6 @@ function requires() {
skip_me=1
fi
;;
cgroups_freezer)
init_cgroup_paths
if [[ "$CGROUP_SUBSYSTEMS" != *"freezer"* ]]; then
skip_me=1
fi
;;
cgroups_rt)
init_cgroup_paths
if [ ! -e "${CGROUP_CPU_BASE_PATH}/cpu.rt_period_us" ]; then
Expand Down Expand Up @@ -350,6 +346,13 @@ function requires() {
skip_me=1
fi
;;
cgroups_*)
init_cgroup_paths
var=${var#cgroups_}
if [[ "$CGROUP_SUBSYSTEMS" != *"$var"* ]]; then
skip_me=1
fi
;;
smp)
local cpu_count=$(grep -c '^processor' /proc/cpuinfo)
if [ "$cpu_count" -lt 2 ]; then
Expand Down
25 changes: 6 additions & 19 deletions tests/integration/update.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,23 +14,15 @@ function setup() {

# Set some initial known values
update_config ' .linux.resources.memory |= {"limit": 33554432, "reservation": 25165824}
| .linux.resources.cpu |= {"shares": 100, "quota": 500000, "period": 1000000, "cpus": "0"}
| .linux.resources.cpu |= {"shares": 100, "quota": 500000, "period": 1000000}
| .linux.resources.pids |= {"limit": 20}'
}

# Tests whatever limits are (more or less) common between cgroup
# v1 and v2: memory/swap, pids, and cpuset.
@test "update cgroup v1/v2 common limits" {
[[ "$ROOTLESS" -ne 0 && -z "$RUNC_USE_SYSTEMD" ]] && requires rootless_cgroup
if [[ "$ROOTLESS" -ne 0 && -n "$RUNC_USE_SYSTEMD" ]]; then
file="/sys/fs/cgroup/user.slice/user-$(id -u).slice/user@$(id -u).service/cgroup.controllers"
# NOTE: delegation of cpuset requires systemd >= 244 (Fedora >= 32, Ubuntu >= 20.04).
for f in memory pids cpuset; do
if grep -qwv $f "$file"; then
skip "$f is not enabled in $file"
fi
done
fi
[[ "$ROOTLESS" -ne 0 ]] && requires rootless_cgroup
requires cgroups_memory cgroups_pids cgroups_cpuset
init_cgroup_paths

# run a few busyboxes detached
Expand Down Expand Up @@ -70,11 +62,6 @@ function setup() {
fi

# check that initial values were properly set
check_cgroup_value "cpuset.cpus" 0
if [[ "$CGROUP_UNIFIED" = "yes" ]] && ! grep -qw memory "$CGROUP_PATH/cgroup.controllers"; then
# This happen on containerized environment because "echo +memory > /sys/fs/cgroup/cgroup.subtree_control" fails with EINVAL
skip "memory controller not available"
fi
check_cgroup_value $MEM_LIMIT 33554432
check_systemd_value $SD_MEM_LIMIT 33554432

Expand All @@ -84,7 +71,7 @@ function setup() {
check_cgroup_value "pids.max" 20
check_systemd_value "TasksMax" 20

# update cpuset if supported (i.e. we're running on a multicore cpu)
# update cpuset if possible (i.e. we're running on a multicore cpu)
cpu_count=$(grep -c '^processor' /proc/cpuinfo)
if [ "$cpu_count" -gt 1 ]; then
runc update test_update --cpuset-cpus "1"
Expand Down Expand Up @@ -427,7 +414,7 @@ EOF

@test "update cpuset parameters via resources.CPU" {
[[ "$ROOTLESS" -ne 0 ]] && requires rootless_cgroup
requires smp
requires smp cgroups_cpuset

local AllowedCPUs='AllowedCPUs' AllowedMemoryNodes='AllowedMemoryNodes'
# these properties require systemd >= v244
Expand Down Expand Up @@ -483,7 +470,7 @@ EOF
@test "update cpuset parameters via v2 unified map" {
# This test assumes systemd >= v244
[[ "$ROOTLESS" -ne 0 ]] && requires rootless_cgroup
requires cgroups_v2 smp
requires cgroups_v2 smp cgroups_cpuset

update_config ' .linux.resources.unified |= {
"cpuset.cpus": "0",
Expand Down