Skip to content

runc update: support per-device weight and iops #4775

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

Merged
merged 2 commits into from
Jun 20, 2025
Merged

runc update: support per-device weight and iops #4775

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7696402
runc update: support per-device weight and iops
kolyshkin May 30, 2025
0b01dcc
runc update: handle duplicated devs properly
kolyshkin Jun 12, 2025
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
35 changes: 29 additions & 6 deletions tests/integration/cgroups.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,17 +174,40 @@ function setup() {
runc run -d --console-socket "$CONSOLE_SOCKET" test_dev_weight
[ "$status" -eq 0 ]

# The loop device itself is no longer needed.
losetup -d "$dev"

if [ -v CGROUP_V2 ]; then
file="io.bfq.weight"
else
file="blkio.bfq.weight_device"
fi
weights=$(get_cgroup_value $file)
[[ "$weights" == *"default 333"* ]]
[[ "$weights" == *"$major:$minor 444"* ]]
weights1=$(get_cgroup_value $file)

# Check that runc update works.
runc update -r - test_dev_weight <<EOF
{
"blockIO": {
"weight": 111,
"weightDevice": [
{
"major": $major,
"minor": $minor,
"weight": 222
}
]
}
}
EOF
weights2=$(get_cgroup_value $file)

# The loop device itself is no longer needed.
losetup -d "$dev"

# Check original values.
grep '^default 333$' <<<"$weights1"
grep "^$major:$minor 444$" <<<"$weights1"
# Check updated values.
grep '^default 111$' <<<"$weights2"
grep "^$major:$minor 222$" <<<"$weights2"

}

@test "runc run (per-device multiple iops via unified)" {
Expand Down
16 changes: 16 additions & 0 deletions tests/integration/helpers.bash
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,22 @@ function check_cpu_weight() {
check_systemd_value "CPUWeight" "$weight"
}

function check_cgroup_dev_iops() {
local dev=$1 rbps=$2 wbps=$3 riops=$4 wiops=$5

if [ -v CGROUP_V2 ]; then
iops=$(get_cgroup_value "io.max")
printf "== io.max ==\n%s\n" "$iops"
grep "^$dev rbps=$rbps wbps=$wbps riops=$riops wiops=$wiops$" <<<"$iops"
return
fi

grep "^$dev ${rbps}$" <<<"$(get_cgroup_value blkio.throttle.read_bps_device)"
grep "^$dev ${wbps}$" <<<"$(get_cgroup_value blkio.throttle.write_bps_device)"
grep "^$dev ${riops}$" <<<"$(get_cgroup_value blkio.throttle.read_iops_device)"
grep "^$dev ${wiops}$" <<<"$(get_cgroup_value blkio.throttle.write_iops_device)"
}

# Helper function to set a resources limit
function set_resources_limit() {
update_config '.linux.resources.pids.limit |= 100'
Expand Down
62 changes: 62 additions & 0 deletions tests/integration/update.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -892,3 +892,65 @@ EOF
runc update test_update --memory 1024
wait_for_container 10 1 test_update stopped
}

@test "update per-device iops/bps values" {
[ $EUID -ne 0 ] && requires rootless_cgroup

# We need a major number of any disk device. Usually those are partitioned,
# with the device itself having minor of 0, and partitions are 1, 2...
major=$(awk '$2 == 0 {print $1; exit}' /proc/partitions)
if [ "$major" = "0" ] || [ "$major" = "" ]; then
echo "=== /proc/partitions ==="
cat /proc/partitions
echo "==="
skip "can't get device major number from /proc/partitions (got $major)"
fi
# Add an entry to check that
# - existing devices can be updated;
# - duplicates are handled properly;
# (see func upsert* in update.go).
update_config ' .linux.resources.blockIO.throttleReadBpsDevice |= [
{ major: '"$major"', minor: 0, rate: 485760 },
{ major: '"$major"', minor: 0, rate: 485760 }
]'

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

runc update -r - test_update <<EOF
{
"blockIO": {
"throttleReadBpsDevice": [
{
"major": $major,
"minor": 0,
"rate": 10485760
}
],
"throttleWriteBpsDevice": [
{
"major": $major,
"minor": 0,
"rate": 9437184
}
],
"throttleReadIOPSDevice": [
{
"major": $major,
"minor": 0,
"rate": 1000
}
],
"throttleWriteIOPSDevice": [
{
"major": $major,
"minor": 0,
"rate": 900
}
]
}
}
EOF
[ "$status" -eq 0 ]
check_cgroup_dev_iops "$major:0" 10485760 9437184 1000 900
}
62 changes: 62 additions & 0 deletions update.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"errors"
"fmt"
"os"
"slices"
"strconv"

"github.com/opencontainers/cgroups"
Expand Down Expand Up @@ -273,6 +274,25 @@ other options are ignored.
if r.BlockIO.Weight != nil {
config.Cgroups.Resources.BlkioWeight = *r.BlockIO.Weight
}
if r.BlockIO.LeafWeight != nil {
config.Cgroups.Resources.BlkioLeafWeight = *r.BlockIO.LeafWeight
}
// For devices, we either update an existing one, or insert a new one.
for _, wd := range r.BlockIO.WeightDevice {
config.Cgroups.Resources.BlkioWeightDevice = upsertWeightDevice(config.Cgroups.Resources.BlkioWeightDevice, wd)
}
for _, td := range r.BlockIO.ThrottleReadBpsDevice {
config.Cgroups.Resources.BlkioThrottleReadBpsDevice = upsertThrottleDevice(config.Cgroups.Resources.BlkioThrottleReadBpsDevice, td)
}
for _, td := range r.BlockIO.ThrottleWriteBpsDevice {
config.Cgroups.Resources.BlkioThrottleWriteBpsDevice = upsertThrottleDevice(config.Cgroups.Resources.BlkioThrottleWriteBpsDevice, td)
}
for _, td := range r.BlockIO.ThrottleReadIOPSDevice {
config.Cgroups.Resources.BlkioThrottleReadIOPSDevice = upsertThrottleDevice(config.Cgroups.Resources.BlkioThrottleReadIOPSDevice, td)
}
for _, td := range r.BlockIO.ThrottleWriteIOPSDevice {
config.Cgroups.Resources.BlkioThrottleWriteIOPSDevice = upsertThrottleDevice(config.Cgroups.Resources.BlkioThrottleWriteIOPSDevice, td)
}

// Setting CPU quota and period independently does not make much sense,
// but historically runc allowed it and this needs to be supported
Expand Down Expand Up @@ -381,3 +401,45 @@ other options are ignored.
return container.Set(config)
},
}

func upsertWeightDevice(devices []*cgroups.WeightDevice, wd specs.LinuxWeightDevice) []*cgroups.WeightDevice {
// Iterate backwards because in case of a duplicate
// the last one will be used.
Comment on lines +406 to +407
Copy link
Member

@rata rata Jun 18, 2025

Choose a reason for hiding this comment

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

I've seen the comment, the commit msg, etc. I might be tired, but I don't see why to do it in reverse order or not.

If it's duplicated, the first value must take precedence instead of the last value? Is this on the spec? Intuitively, it seems like an error to me (the config.json contradicts itself).

Copy link
Contributor Author

@kolyshkin kolyshkin Jun 19, 2025

Choose a reason for hiding this comment

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

To add a bit of context, this was added as a response to @lifubang suggestion at: #4775 (comment)

The spec doesn't tell anything about duplicated entries (meaning that they are implicitly allowed, and the precedence rules are not documented either).

The runc behavior is/was to apply all rules one-by-one, so if there is a duplicated rule, the latter one overwrites the former. This is the reason why we iterate backwards.

In practice, though, it doesn't matter much, because if there are no duplicated entries, the order doesn't matter, and my infinite belief in human decency tells me no one has duplicated entries anyway. Yet, if someone does, this trick here saves them from a disaster.

Copy link
Contributor Author

@kolyshkin kolyshkin Jun 19, 2025

Choose a reason for hiding this comment

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

You can also run the test case from the second commit without the fix from the same commit to see what's happening if we iterate as usual.

Copy link
Member

@rata rata Jun 20, 2025

Choose a reason for hiding this comment

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

Yes, sorry, I wanted to say I read that too. But today, a little bit less tired, I think I see it. We return after setting it, I didn't realize the for loop ended when we found the entry. Sorry!

It is not exactly the same behavior as before, if we were processing all the rules, though. Because if one property (let's say weight) wasn't set in the last entry of the config, it would have the value of the previous entry that set it.

I think this is edgy enough that we might be able to change it without anyone realizing. So let's try it :)

Copy link
Contributor Author

@kolyshkin kolyshkin Jun 20, 2025

Choose a reason for hiding this comment

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

It is not exactly the same behavior as before, if we were processing all the rules, though. Because if one property (let's say weight) wasn't set in the last entry of the config, it would have the value of the previous entry that set it.

I saw that one, too, but I think no one is actually using LeafWeight these days, so practically we only have one property.

for i, dev := range slices.Backward(devices) {
if dev.Major != wd.Major || dev.Minor != wd.Minor {
continue
}
// Update weights for existing device.
if wd.Weight != nil {
devices[i].Weight = *wd.Weight
}
if wd.LeafWeight != nil {
devices[i].LeafWeight = *wd.LeafWeight
}
return devices
}

// New device -- append it.
var weight, leafWeight uint16
if wd.Weight != nil {
weight = *wd.Weight
}
if wd.LeafWeight != nil {
leafWeight = *wd.LeafWeight
}

return append(devices, cgroups.NewWeightDevice(wd.Major, wd.Minor, weight, leafWeight))
}

func upsertThrottleDevice(devices []*cgroups.ThrottleDevice, td specs.LinuxThrottleDevice) []*cgroups.ThrottleDevice {
// Iterate backwards because in case of a duplicate
// the last one will be used.
for i, dev := range slices.Backward(devices) {
if dev.Major == td.Major && dev.Minor == td.Minor {
devices[i].Rate = td.Rate
return devices
}
}

return append(devices, cgroups.NewThrottleDevice(td.Major, td.Minor, td.Rate))
}