The POST /containers/{id}/update API accepts BlkioWeightDevice,
BlkioDeviceReadBps, BlkioDeviceWriteBps, BlkioDeviceReadIOps, and
BlkioDeviceWriteIOps in its Resources body, but these five fields were
silently ignored when updating a running container.
The root cause was in toContainerdResources (daemon/update_linux.go):
only BlkioWeight was mapped into specs.LinuxBlockIO; the per-device
fields were never converted, so tsk.UpdateResources never wrote to
cgroupv2 io.max or the cgroupv1 blkio throttle files.
Fix by calling the existing getBlkioWeightDevices and
getBlkioThrottleDevices helpers (already used in oci_linux.go for
container creation) to populate all five fields. The function signature
is extended to return an error so that stat(2) failures on invalid
device paths are surfaced to the caller instead of being silently
dropped.
The API makes distinction between nil and zero-length slices while
doing. A nil per-device blkio field means the caller did not request an
update for that setting, while a non-nil empty slice means the caller
explicitly requested the setting to be cleared.
The Windows stub is updated to match the new signature.
Signed-off-by: Alexis Couvreur <alexiscouvreur.pro@gmail.com>
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
| ... | ... |
@@ -269,6 +269,21 @@ func (container *Container) UpdateContainer(hostConfig *containertypes.HostConfi |
| 269 | 269 |
if resources.BlkioWeight != 0 {
|
| 270 | 270 |
cResources.BlkioWeight = resources.BlkioWeight |
| 271 | 271 |
} |
| 272 |
+ if resources.BlkioWeightDevice != nil {
|
|
| 273 |
+ cResources.BlkioWeightDevice = resources.BlkioWeightDevice |
|
| 274 |
+ } |
|
| 275 |
+ if resources.BlkioDeviceReadBps != nil {
|
|
| 276 |
+ cResources.BlkioDeviceReadBps = resources.BlkioDeviceReadBps |
|
| 277 |
+ } |
|
| 278 |
+ if resources.BlkioDeviceWriteBps != nil {
|
|
| 279 |
+ cResources.BlkioDeviceWriteBps = resources.BlkioDeviceWriteBps |
|
| 280 |
+ } |
|
| 281 |
+ if resources.BlkioDeviceReadIOps != nil {
|
|
| 282 |
+ cResources.BlkioDeviceReadIOps = resources.BlkioDeviceReadIOps |
|
| 283 |
+ } |
|
| 284 |
+ if resources.BlkioDeviceWriteIOps != nil {
|
|
| 285 |
+ cResources.BlkioDeviceWriteIOps = resources.BlkioDeviceWriteIOps |
|
| 286 |
+ } |
|
| 272 | 287 |
if resources.CPUShares != 0 {
|
| 273 | 288 |
cResources.CPUShares = resources.CPUShares |
| 274 | 289 |
} |
| ... | ... |
@@ -93,7 +93,12 @@ func (daemon *Daemon) update(name string, hostConfig *container.HostConfig) erro |
| 93 | 93 |
return err |
| 94 | 94 |
} |
| 95 | 95 |
|
| 96 |
- if err := tsk.UpdateResources(context.TODO(), toContainerdResources(hostConfig.Resources)); err != nil {
|
|
| 96 |
+ resources, err := toContainerdResources(hostConfig.Resources) |
|
| 97 |
+ if err != nil {
|
|
| 98 |
+ restoreConfig = true |
|
| 99 |
+ return errCannotUpdate(ctr.ID, err) |
|
| 100 |
+ } |
|
| 101 |
+ if err := tsk.UpdateResources(context.TODO(), resources); err != nil {
|
|
| 97 | 102 |
restoreConfig = true |
| 98 | 103 |
// TODO: it would be nice if containerd responded with better errors here so we can classify this better. |
| 99 | 104 |
return errCannotUpdate(ctr.ID, errdefs.System(err)) |
| ... | ... |
@@ -8,13 +8,59 @@ import ( |
| 8 | 8 |
"github.com/opencontainers/runtime-spec/specs-go" |
| 9 | 9 |
) |
| 10 | 10 |
|
| 11 |
-func toContainerdResources(resources container.Resources) *libcontainerdtypes.Resources {
|
|
| 11 |
+func toContainerdResources(resources container.Resources) (*libcontainerdtypes.Resources, error) {
|
|
| 12 | 12 |
var r libcontainerdtypes.Resources |
| 13 | 13 |
|
| 14 |
- if resources.BlkioWeight != 0 {
|
|
| 15 |
- r.BlockIO = &specs.LinuxBlockIO{
|
|
| 16 |
- Weight: &resources.BlkioWeight, |
|
| 14 |
+ // little helper to lazily initialize the BlockIO struct only if needed |
|
| 15 |
+ blockIO := func() *specs.LinuxBlockIO {
|
|
| 16 |
+ if r.BlockIO == nil {
|
|
| 17 |
+ r.BlockIO = &specs.LinuxBlockIO{}
|
|
| 17 | 18 |
} |
| 19 |
+ return r.BlockIO |
|
| 20 |
+ } |
|
| 21 |
+ |
|
| 22 |
+ weightDevices, err := getBlkioWeightDevices(resources) |
|
| 23 |
+ if err != nil {
|
|
| 24 |
+ return nil, err |
|
| 25 |
+ } |
|
| 26 |
+ if resources.BlkioWeightDevice != nil {
|
|
| 27 |
+ blockIO().WeightDevice = weightDevices |
|
| 28 |
+ } |
|
| 29 |
+ |
|
| 30 |
+ readBpsDevices, err := getBlkioThrottleDevices(resources.BlkioDeviceReadBps) |
|
| 31 |
+ if err != nil {
|
|
| 32 |
+ return nil, err |
|
| 33 |
+ } |
|
| 34 |
+ if resources.BlkioDeviceReadBps != nil {
|
|
| 35 |
+ blockIO().ThrottleReadBpsDevice = readBpsDevices |
|
| 36 |
+ } |
|
| 37 |
+ |
|
| 38 |
+ writeBpsDevices, err := getBlkioThrottleDevices(resources.BlkioDeviceWriteBps) |
|
| 39 |
+ if err != nil {
|
|
| 40 |
+ return nil, err |
|
| 41 |
+ } |
|
| 42 |
+ if resources.BlkioDeviceWriteBps != nil {
|
|
| 43 |
+ blockIO().ThrottleWriteBpsDevice = writeBpsDevices |
|
| 44 |
+ } |
|
| 45 |
+ |
|
| 46 |
+ readIOpsDevices, err := getBlkioThrottleDevices(resources.BlkioDeviceReadIOps) |
|
| 47 |
+ if err != nil {
|
|
| 48 |
+ return nil, err |
|
| 49 |
+ } |
|
| 50 |
+ if resources.BlkioDeviceReadIOps != nil {
|
|
| 51 |
+ blockIO().ThrottleReadIOPSDevice = readIOpsDevices |
|
| 52 |
+ } |
|
| 53 |
+ |
|
| 54 |
+ writeIOpsDevices, err := getBlkioThrottleDevices(resources.BlkioDeviceWriteIOps) |
|
| 55 |
+ if err != nil {
|
|
| 56 |
+ return nil, err |
|
| 57 |
+ } |
|
| 58 |
+ if resources.BlkioDeviceWriteIOps != nil {
|
|
| 59 |
+ blockIO().ThrottleWriteIOPSDevice = writeIOpsDevices |
|
| 60 |
+ } |
|
| 61 |
+ |
|
| 62 |
+ if resources.BlkioWeight != 0 {
|
|
| 63 |
+ blockIO().Weight = &resources.BlkioWeight |
|
| 18 | 64 |
} |
| 19 | 65 |
|
| 20 | 66 |
cpu := specs.LinuxCPU{
|
| ... | ... |
@@ -68,5 +114,5 @@ func toContainerdResources(resources container.Resources) *libcontainerdtypes.Re |
| 68 | 68 |
} |
| 69 | 69 |
|
| 70 | 70 |
r.Pids = getPidsLimit(resources) |
| 71 |
- return &r |
|
| 71 |
+ return &r, nil |
|
| 72 | 72 |
} |
| ... | ... |
@@ -7,5 +7,9 @@ import ( |
| 7 | 7 |
) |
| 8 | 8 |
|
| 9 | 9 |
func TestToContainerdResources_Defaults(t *testing.T) {
|
| 10 |
- checkResourcesAreUnset(t, toContainerdResources(container.Resources{}))
|
|
| 10 |
+ r, err := toContainerdResources(container.Resources{})
|
|
| 11 |
+ if err != nil {
|
|
| 12 |
+ t.Fatal(err) |
|
| 13 |
+ } |
|
| 14 |
+ checkResourcesAreUnset(t, r) |
|
| 11 | 15 |
} |
| ... | ... |
@@ -5,7 +5,7 @@ import ( |
| 5 | 5 |
libcontainerdtypes "github.com/moby/moby/v2/daemon/internal/libcontainerd/types" |
| 6 | 6 |
) |
| 7 | 7 |
|
| 8 |
-func toContainerdResources(resources container.Resources) *libcontainerdtypes.Resources {
|
|
| 8 |
+func toContainerdResources(resources container.Resources) (*libcontainerdtypes.Resources, error) {
|
|
| 9 | 9 |
// We don't support update, so do nothing |
| 10 |
- return nil |
|
| 10 |
+ return nil, nil |
|
| 11 | 11 |
} |
| ... | ... |
@@ -2,16 +2,20 @@ package container |
| 2 | 2 |
|
| 3 | 3 |
import ( |
| 4 | 4 |
"context" |
| 5 |
+ "fmt" |
|
| 6 |
+ "os" |
|
| 5 | 7 |
"strconv" |
| 6 | 8 |
"strings" |
| 7 | 9 |
"testing" |
| 8 | 10 |
"time" |
| 9 | 11 |
|
| 12 |
+ "github.com/moby/moby/api/types/blkiodev" |
|
| 10 | 13 |
containertypes "github.com/moby/moby/api/types/container" |
| 11 | 14 |
"github.com/moby/moby/client" |
| 12 | 15 |
"github.com/moby/moby/v2/integration/internal/container" |
| 13 | 16 |
"github.com/moby/moby/v2/internal/testutil" |
| 14 | 17 |
"github.com/moby/moby/v2/internal/testutil/request" |
| 18 |
+ "golang.org/x/sys/unix" |
|
| 15 | 19 |
"gotest.tools/v3/assert" |
| 16 | 20 |
is "gotest.tools/v3/assert/cmp" |
| 17 | 21 |
"gotest.tools/v3/skip" |
| ... | ... |
@@ -213,3 +217,120 @@ func TestUpdatePidsLimit(t *testing.T) {
|
| 213 | 213 |
}) |
| 214 | 214 |
} |
| 215 | 215 |
} |
| 216 |
+ |
|
| 217 |
+// blkioTestDevice finds a usable block device on the host by examining the |
|
| 218 |
+// device backing the root filesystem. The returned path (e.g. "/dev/sda") |
|
| 219 |
+// and the decimal major:minor string are suitable for docker update flags and |
|
| 220 |
+// cgroup file lookups respectively. |
|
| 221 |
+func blkioTestDevice(t *testing.T) (devPath, majMin string) {
|
|
| 222 |
+ t.Helper() |
|
| 223 |
+ |
|
| 224 |
+ var st unix.Stat_t |
|
| 225 |
+ assert.NilError(t, unix.Stat("/", &st), "stat /")
|
|
| 226 |
+ |
|
| 227 |
+ major := unix.Major(st.Dev) |
|
| 228 |
+ minor := unix.Minor(st.Dev) |
|
| 229 |
+ |
|
| 230 |
+ // Resolve the device name via sysfs so we get a real /dev/… path. |
|
| 231 |
+ uevent, err := os.ReadFile(fmt.Sprintf("/sys/dev/block/%d:%d/uevent", major, minor))
|
|
| 232 |
+ if err != nil {
|
|
| 233 |
+ t.Skipf("cannot resolve block device for / from sysfs: %v", err)
|
|
| 234 |
+ } |
|
| 235 |
+ var name string |
|
| 236 |
+ for _, line := range strings.Split(string(uevent), "\n") {
|
|
| 237 |
+ if after, ok := strings.CutPrefix(line, "DEVNAME="); ok {
|
|
| 238 |
+ name = strings.TrimSpace(after) |
|
| 239 |
+ break |
|
| 240 |
+ } |
|
| 241 |
+ } |
|
| 242 |
+ if name == "" {
|
|
| 243 |
+ t.Skip("DEVNAME not found in sysfs uevent for / block device")
|
|
| 244 |
+ } |
|
| 245 |
+ return "/dev/" + name, fmt.Sprintf("%d:%d", major, minor)
|
|
| 246 |
+} |
|
| 247 |
+ |
|
| 248 |
+// parseIOMax returns the value of field (rbps/wbps/riops/wiops) for the given |
|
| 249 |
+// major:minor device from a cgroup v2 io.max file content. |
|
| 250 |
+func parseIOMax(content, majMin, field string) string {
|
|
| 251 |
+ for _, line := range strings.Split(content, "\n") {
|
|
| 252 |
+ if !strings.HasPrefix(line, majMin+" ") {
|
|
| 253 |
+ continue |
|
| 254 |
+ } |
|
| 255 |
+ for _, part := range strings.Fields(line[len(majMin)+1:]) {
|
|
| 256 |
+ if k, v, ok := strings.Cut(part, "="); ok && k == field {
|
|
| 257 |
+ return v |
|
| 258 |
+ } |
|
| 259 |
+ } |
|
| 260 |
+ } |
|
| 261 |
+ return "" |
|
| 262 |
+} |
|
| 263 |
+ |
|
| 264 |
+// TestUpdateBlkioThrottleDevices verifies that docker update correctly applies |
|
| 265 |
+// per-device blkio throttle limits to the container's cgroup. |
|
| 266 |
+// |
|
| 267 |
+// On cgroup v2 the limits are reflected in /sys/fs/cgroup/io.max inside the |
|
| 268 |
+// container. On cgroup v1 only the API-level values are verified (the blkio |
|
| 269 |
+// throttle files are not bind-mounted into the container namespace). |
|
| 270 |
+func TestUpdateBlkioThrottleDevices(t *testing.T) {
|
|
| 271 |
+ skip.If(t, testEnv.DaemonInfo.OSType == "windows") |
|
| 272 |
+ skip.If(t, testEnv.DaemonInfo.CgroupDriver == "none") |
|
| 273 |
+ |
|
| 274 |
+ devPath, majMin := blkioTestDevice(t) |
|
| 275 |
+ |
|
| 276 |
+ ctx := setupTest(t) |
|
| 277 |
+ apiClient := testEnv.APIClient() |
|
| 278 |
+ |
|
| 279 |
+ cID := container.Run(ctx, t, apiClient) |
|
| 280 |
+ |
|
| 281 |
+ const ( |
|
| 282 |
+ readBps uint64 = 5 * 1024 * 1024 // 5 MiB/s |
|
| 283 |
+ writeBps uint64 = 2 * 1024 * 1024 // 2 MiB/s |
|
| 284 |
+ readIops uint64 = 100 |
|
| 285 |
+ writeIops uint64 = 50 |
|
| 286 |
+ ) |
|
| 287 |
+ |
|
| 288 |
+ _, err := apiClient.ContainerUpdate(ctx, cID, client.ContainerUpdateOptions{
|
|
| 289 |
+ Resources: &containertypes.Resources{
|
|
| 290 |
+ BlkioDeviceReadBps: []*blkiodev.ThrottleDevice{{Path: devPath, Rate: readBps}},
|
|
| 291 |
+ BlkioDeviceWriteBps: []*blkiodev.ThrottleDevice{{Path: devPath, Rate: writeBps}},
|
|
| 292 |
+ BlkioDeviceReadIOps: []*blkiodev.ThrottleDevice{{Path: devPath, Rate: readIops}},
|
|
| 293 |
+ BlkioDeviceWriteIOps: []*blkiodev.ThrottleDevice{{Path: devPath, Rate: writeIops}},
|
|
| 294 |
+ }, |
|
| 295 |
+ }) |
|
| 296 |
+ assert.NilError(t, err) |
|
| 297 |
+ |
|
| 298 |
+ // Verify API-level values are persisted. |
|
| 299 |
+ inspect, err := apiClient.ContainerInspect(ctx, cID, client.ContainerInspectOptions{})
|
|
| 300 |
+ assert.NilError(t, err) |
|
| 301 |
+ assert.Assert(t, is.Len(inspect.Container.HostConfig.BlkioDeviceReadBps, 1)) |
|
| 302 |
+ assert.Check(t, is.Equal(inspect.Container.HostConfig.BlkioDeviceReadBps[0].Rate, readBps)) |
|
| 303 |
+ assert.Assert(t, is.Len(inspect.Container.HostConfig.BlkioDeviceWriteBps, 1)) |
|
| 304 |
+ assert.Check(t, is.Equal(inspect.Container.HostConfig.BlkioDeviceWriteBps[0].Rate, writeBps)) |
|
| 305 |
+ assert.Assert(t, is.Len(inspect.Container.HostConfig.BlkioDeviceReadIOps, 1)) |
|
| 306 |
+ assert.Check(t, is.Equal(inspect.Container.HostConfig.BlkioDeviceReadIOps[0].Rate, readIops)) |
|
| 307 |
+ assert.Assert(t, is.Len(inspect.Container.HostConfig.BlkioDeviceWriteIOps, 1)) |
|
| 308 |
+ assert.Check(t, is.Equal(inspect.Container.HostConfig.BlkioDeviceWriteIOps[0].Rate, writeIops)) |
|
| 309 |
+ |
|
| 310 |
+ // On cgroup v2 verify the limits landed in io.max inside the container. |
|
| 311 |
+ if testEnv.DaemonInfo.CgroupVersion != "2" {
|
|
| 312 |
+ return |
|
| 313 |
+ } |
|
| 314 |
+ |
|
| 315 |
+ ctx, cancel := context.WithTimeout(ctx, 30*time.Second) |
|
| 316 |
+ defer cancel() |
|
| 317 |
+ |
|
| 318 |
+ res, err := container.Exec(ctx, apiClient, cID, []string{"cat", "/sys/fs/cgroup/io.max"})
|
|
| 319 |
+ assert.NilError(t, err) |
|
| 320 |
+ assert.Assert(t, is.Len(res.Stderr(), 0)) |
|
| 321 |
+ assert.Equal(t, 0, res.ExitCode) |
|
| 322 |
+ |
|
| 323 |
+ ioMax := res.Stdout() |
|
| 324 |
+ assert.Check(t, is.Equal(parseIOMax(ioMax, majMin, "rbps"), strconv.FormatUint(readBps, 10)), |
|
| 325 |
+ "io.max rbps for %s (io.max content: %q)", majMin, ioMax) |
|
| 326 |
+ assert.Check(t, is.Equal(parseIOMax(ioMax, majMin, "wbps"), strconv.FormatUint(writeBps, 10)), |
|
| 327 |
+ "io.max wbps for %s (io.max content: %q)", majMin, ioMax) |
|
| 328 |
+ assert.Check(t, is.Equal(parseIOMax(ioMax, majMin, "riops"), strconv.FormatUint(readIops, 10)), |
|
| 329 |
+ "io.max riops for %s (io.max content: %q)", majMin, ioMax) |
|
| 330 |
+ assert.Check(t, is.Equal(parseIOMax(ioMax, majMin, "wiops"), strconv.FormatUint(writeIops, 10)), |
|
| 331 |
+ "io.max wiops for %s (io.max content: %q)", majMin, ioMax) |
|
| 332 |
+} |