Skip to content

Commit

Permalink
Address @kolyshkin comments
Browse files Browse the repository at this point in the history
Signed-off-by: Cédric Clerget <cedric.clerget@gmail.com>
  • Loading branch information
cclerget committed Apr 3, 2024
1 parent 27dcbf1 commit ea43f8e
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 49 deletions.
4 changes: 3 additions & 1 deletion libcontainer/cgroups/fs/fs.go
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,9 @@ func GetEffectiveCPUs(cpusetPath string, cgroups *configs.Cgroup) string {
}

// Iterates until it goes to the cgroup root path.
for path := filepath.Clean(cpusetPath); path != defaultCgroupRoot; path = filepath.Dir(path) {
// It's required for containers in which cpuset controller
// is not enabled, in this case a parent cgroup is used.
for path := cpusetPath; path != defaultCgroupRoot; path = filepath.Dir(path) {
cpus, err := fscommon.GetCgroupParamString(path, "cpuset.effective_cpus")
if err == nil {
return cpus
Expand Down
8 changes: 7 additions & 1 deletion libcontainer/cgroups/fs2/fs2.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/opencontainers/runc/libcontainer/cgroups"
"github.com/opencontainers/runc/libcontainer/cgroups/fscommon"
"github.com/opencontainers/runc/libcontainer/configs"
"github.com/opencontainers/runc/libcontainer/utils"
)

type parseError = fscommon.ParseError
Expand All @@ -33,6 +34,9 @@ func NewManager(config *configs.Cgroup, dirPath string) (*Manager, error) {
if err != nil {
return nil, err
}
} else {
// Clean path for safety.
dirPath = utils.CleanPath(dirPath)
}

m := &Manager{
Expand Down Expand Up @@ -327,9 +331,11 @@ func (m *Manager) GetEffectiveCPUs() string {
}

// Iterates until it goes outside of the cgroup root path.
// It's required for containers in which cpuset controller
// is not enabled, in this case a parent cgroup is used.
outsidePath := filepath.Dir(UnifiedMountpoint)

for path := filepath.Clean(m.dirPath); path != outsidePath; path = filepath.Dir(path) {
for path := m.dirPath; path != outsidePath; path = filepath.Dir(path) {
cpus, err := fscommon.GetCgroupParamString(path, "cpuset.cpus.effective")
if err == nil {
return cpus
Expand Down
65 changes: 18 additions & 47 deletions tests/integration/exec.bats
Original file line number Diff line number Diff line change
Expand Up @@ -342,22 +342,17 @@ EOF
}

@test "runc exec with isolated cpus affinity temporary transition [cgroup cpuset]" {
requires root
requires root cgroups_cpuset

tmp=$(mktemp -d "$BATS_RUN_TMPDIR/runc.XXXXXX")

set_cgroup_cpuset_all_cpus
local all_cpus
all_cpus="$(cat /sys/devices/system/cpu/online)"

update_config ".linux.resources.cpu.cpus = \"$all_cpus\""
all_cpus="$(get_all_online_cpus)"

# set temporary isolated CPU affinity transition
update_config '.annotations += {"org.opencontainers.runc.exec.isolated-cpu-affinity-transition": "temporary"}'

local mems
mems="$(cat /sys/devices/system/node/online 2>/dev/null || true)"
[[ -n $mems ]] && update_config ".linux.resources.cpu.mems = \"$mems\""

runc run -d --console-socket "$CONSOLE_SOCKET" test_isolated_temporary_transition
[ "$status" -eq 0 ]

Expand All @@ -375,25 +370,17 @@ EOF
}

@test "runc exec with isolated cpus affinity definitive transition [cgroup cpuset]" {
requires root
requires root cgroups_cpuset

tmp=$(mktemp -d "$BATS_RUN_TMPDIR/runc.XXXXXX")

set_cgroup_cpuset_all_cpus
local all_cpus
all_cpus="$(cat /sys/devices/system/cpu/online)"

local first_cpu="0"
[[ $all_cpus =~ [^0-9]*([0-9]+)([-,][0-9]+)? ]] && first_cpu=${BASH_REMATCH[1]}

update_config ".linux.resources.cpu.cpus = \"$all_cpus\""
all_cpus="$(get_all_online_cpus)"

# set definitive isolated CPU affinity transition
update_config '.annotations += {"org.opencontainers.runc.exec.isolated-cpu-affinity-transition": "definitive"}'

local mems
mems="$(cat /sys/devices/system/node/online 2>/dev/null || true)"
[[ -n $mems ]] && update_config ".linux.resources.cpu.mems = \"$mems\""

runc run -d --console-socket "$CONSOLE_SOCKET" test_isolated_definitive_transition
[ "$status" -eq 0 ]

Expand All @@ -411,34 +398,29 @@ EOF
load /etc/os-release

# fix unbound variable in condition below
PLATFORM_ID=${PLATFORM_ID:-}
VERSION_ID=${VERSION_ID:-}

allowed_cpus=$all_cpus
# use first cpu on systems with RHEL >= 9 or systems with kernel >= 6.2
if [[ "${ID_LIKE:-}" =~ "rhel" && "${PLATFORM_ID#platform:el*}" -ge "9" ]] || is_kernel_gte 6.2; then
allowed_cpus=$first_cpu
if [[ "${ID_LIKE:-}" =~ "rhel" && "${VERSION_ID%%.*}" -ge "9" ]] || is_kernel_gte 6.2; then
allowed_cpus="$(get_first_online_cpu)"
fi

[[ "${lines[0]}" == "Cpus_allowed_list: $allowed_cpus" ]]
}

@test "runc exec with isolated cpus affinity bad transition [cgroup cpuset]" {
requires root
requires root cgroups_cpuset

tmp=$(mktemp -d "$BATS_RUN_TMPDIR/runc.XXXXXX")

set_cgroup_cpuset_all_cpus
local all_cpus
all_cpus="$(cat /sys/devices/system/cpu/online)"

update_config ".linux.resources.cpu.cpus = \"$all_cpus\""
all_cpus="$(get_all_online_cpus)"

# set a bad isolated CPU affinity transition
update_config '.annotations += {"org.opencontainers.runc.exec.isolated-cpu-affinity-transition": "bad"}'

local mems
mems="$(cat /sys/devices/system/node/online 2>/dev/null || true)"
[[ -n $mems ]] && update_config ".linux.resources.cpu.mems = \"$mems\""

runc run -d --console-socket "$CONSOLE_SOCKET" test_isolated_bad_transition
[ "$status" -eq 0 ]

Expand All @@ -455,21 +437,13 @@ EOF
}

@test "runc exec with taskset affinity [cgroup cpuset]" {
requires root
requires root cgroups_cpuset

set_cgroup_cpuset_all_cpus
local all_cpus
all_cpus="$(cat /sys/devices/system/cpu/online)"

local first_cpu="0"
[[ $all_cpus =~ [^0-9]*([0-9]+)([-,][0-9]+)? ]] && first_cpu=${BASH_REMATCH[1]}
all_cpus="$(get_all_online_cpus)"

update_config ".linux.resources.cpu.cpus = \"$all_cpus\""

local mems
mems="$(cat /sys/devices/system/node/online 2>/dev/null || true)"
[[ -n $mems ]] && update_config ".linux.resources.cpu.mems = \"$mems\""

taskset -p -c "$first_cpu" $$
taskset -p -c "$(get_first_online_cpu)" $$

runc run -d --console-socket "$CONSOLE_SOCKET" test_with_taskset
[ "$status" -eq 0 ]
Expand All @@ -487,12 +461,9 @@ EOF
requires rootless cgroups_v2

local all_cpus
all_cpus="$(cat /sys/devices/system/cpu/online)"

local first_cpu="0"
[[ $all_cpus =~ [^0-9]*([0-9]+)([-,][0-9]+)? ]] && first_cpu=${BASH_REMATCH[1]}
all_cpus="$(get_all_online_cpus)"

taskset -p -c "$first_cpu" $$
taskset -p -c "$(get_first_online_cpu)" $$

runc run -d --console-socket "$CONSOLE_SOCKET" test_with_taskset
[ "$status" -eq 0 ]
Expand Down
21 changes: 21 additions & 0 deletions tests/integration/helpers.bash
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,27 @@ function set_cgroup_mount_writable() {
update_config '.mounts |= map((select(.type == "cgroup") | .options -= ["ro"]) // .)'
}

# Helper function to get all online cpus.
function get_all_online_cpus() {
cat /sys/devices/system/cpu/online
}

# Helper function to get the first online cpu.
function get_first_online_cpu() {
[[ $(get_all_online_cpus) =~ [^0-9]*([0-9]+)([-,][0-9]+)? ]] && echo "${BASH_REMATCH[1]}"
}

# Helper function to set all cpus/mems in container cgroup cpuset.
function set_cgroup_cpuset_all_cpus() {
update_config ".linux.resources.cpu.cpus = \"$(get_all_online_cpus)\""

local mems
mems="$(cat /sys/devices/system/node/online 2>/dev/null || true)"
if [[ -n $mems ]]; then
update_config ".linux.resources.cpu.mems = \"$mems\""
fi
}

# Fails the current test, providing the error given.
function fail() {
echo "$@" >&2
Expand Down

0 comments on commit ea43f8e

Please # to comment.