Skip to content

Commit

Permalink
merge branch 'pr-2944'
Browse files Browse the repository at this point in the history 
Kir Kolyshkin (8):
  tests/int/helpers: rm old code
  libct/cg/sd/v2: call initPath from Path
  tests/int/update.bats: don't set cpuset in setup
  tests/int/helpers: generalize require cgroups_freezer
  tests/int: enable/use requires cgroups_<ctrl>
  tests/int/cgroups: don't check for hugetlb
  tests/int: run rootless_cgroup tests for v2+systemd
  Vagrantfile.fedora: set Delegate=yes

LGTMs: AkihiroSuda cyphar
Closes #2944
  • Loading branch information
@cyphar
cyphar committed May 10, 2021
2 parents fc8a0d9 + 12e9cac commit 62250c1
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 51 deletions.
4 changes: 2 additions & 2 deletions Vagrantfile.fedora34
View file
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
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
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
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"
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
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

0 comments on commit 62250c1

Please sign in to comment.