fix a failed test TestRunOOMExitCode by AntaresS · Pull Request #37194 · moby/moby

AntaresS · 2018-06-01T23:22:45Z

Signed-off-by: Anda Xu [email protected]

- What I did

fix the failed test - TestRunOOMExitCode
- the original test fails on rhel_7.4, rhel_7.3-dm and rhel_7.4-dm in engine 18.03. When memory limit set lower than 7MB it will raise an error of "device or resource busy" from docker-runc. Here is a failure snippet from test

01:18:52 FAIL: /go/src/github.com/docker/docker/integration-cli/docker_cli_run_unix_test.go:617: DockerSuite.TestRunOOMExitCode
01:18:52 
01:18:52 /go/src/github.com/docker/docker/integration-cli/docker_cli_run_unix_test.go:630:
01:18:52     ...open /go/src/github.com/docker/docker/integration-cli/docker_cli_run_unix_test.go: no such file or directory
01:18:52 ... value *errors.errorString = &errors.errorString{s:"wrong exit code for OOM container: expected 137, got 125 (output: \"/usr/bin/docker: Error response from daemon: OCI runtime create failed: container_linux.go:348: starting container process caused \\\"process_linux.go:402: container init caused \\\\\\\"process_linux.go:367: setting cgroup config for procHooks process caused \\\\\\\\\\\\\\\"failed to write 4194304 to memory.limit_in_bytes: write /sys/fs/cgroup/memory/docker/c31458a08eb2d786394076fd50f84106afc6d7d708053de7fe44a4e214a09391/memory.limit_in_bytes: device or resource busy\\\\\\\\\\\\\\\"\\\\\\\"\\\": unknown.\\ntime=\\\"2018-06-01T01:18:52Z\\\" level=error msg=\\\"error waiting for container: context canceled\\\" \\n\")"} ("wrong exit code for OOM container: expected 137, got 125 (output: \"/usr/bin/docker: Error response from daemon: OCI runtime create failed: container_linux.go:348: starting container process caused \\\"process_linux.go:402: container init caused \\\\\\\"process_linux.go:367: setting cgroup config for procHooks process caused \\\\\\\\\\\\\\\"failed to write 4194304 to memory.limit_in_bytes: write /sys/fs/cgroup/memory/docker/c31458a08eb2d786394076fd50f84106afc6d7d708053de7fe44a4e214a09391/memory.limit_in_bytes: device or resource busy\\\\\\\\\\\\\\\"\\\\\\\"\\\": unknown.\\ntime=\\\"2018-06-01T01:18:52Z\\\" level=error msg=\\\"error waiting for container: context canceled\\\" \\n\")")

Main problem is very likely to be a centos based kernel bug. When docker-runc tries to write memory limit in /sys/fs/cgroup/memory/docker//memory.limit_in_bytes it returns a "device or resource busy" error. There seems to be conflict with kernel process. Related link reporting a similar error - https://unix.stackexchange.com/questions/412040/cgroups-memory-limit-write-error-device-or-resource-busy

- How I did it

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

cc @tonistiigi

AntaresS · 2018-06-01T23:22:59Z

cc @thaJeztah @tiborvass

tiborvass

LGTM

thaJeztah · 2018-06-02T03:59:41Z

do we know why?

/cc @kolyshkin

The reason might be that the current usage is above the value we're trying to use as a limit, and as kernel can't guarantee this limit to be enforced, it replies with EBUSY. I have yet to check that though, and don't have an idea why it works with other kernels.

I was able to repro this on my laptop (Ubuntu 18.10, vanilla kernel 4.16.12) so it's not specific to RHEL:

kir@kd:~/git/rpm$ time docker run -m 4MB busybox sh -c 'x=aaaa; while true; do x=$x$x$x$x; done'; echo $?
docker: Error response from daemon: OCI runtime create failed: container_linux.go:296: starting container process caused "process_linux.go:398: container init caused "process_linux.go:365: setting cgroup config for procHooks process caused \"failed to write 4194304 to memory.limit_in_bytes: write /sys/fs/cgroup/memory/docker/c766d5121094498b1d88a37ebddf67aa7379b177cd9eb38efa9bcc4aa75aa842/memory.limit_in_bytes: device or resource busy\""": unknown.
ERRO[0000] error waiting for container: context canceled

real 0m0.732s
user 0m0.011s
sys 0m0.003s
125

Hmm, I was correct earlier; kernel code tries to set the limit, if the limit can't be set because current usage is more than the new limit, or because kernel can't free any pages it returns EBUSY (meaning the limit is too low). Code is mm/memcontrol.c, function mem_cgroup_resize_limit.

So
(1) this is not kernel version specific (although recent kernels appear to try harder to release memory to set the limit)
(2) this is correct kernel behavior, not a bug
(3) it seems that docker needs more than 4MB to start a container.

I'd suggest to

remove the comment about rhel 7.4 et al kernels replacing it with something like

it appears that 8MB of memory is the minimum needed to reliably start a container

raise the limit to 8MB

kolyshkin · 2018-06-04T17:58:06Z

Hmm, I was correct earlier; kernel code tries to set the limit, if the limit can't be set because current usage is more than the new limit, or because kernel can't free any pages it returns EBUSY (meaning the limit is too low). Code is mm/memcontrol.c, function mem_cgroup_resize_limit.

So
(1) this is not kernel version specific (although recent kernels appear to try harder to release memory to set the limit)
(2) this is correct kernel behavior, not a bug
(3) it seems that docker needs more than 4MB to start a container.

I'd suggest to

remove the comment about rhel 7.4 et al kernels replacing it with something like

it appears that 8MB of memory is the minimum needed to reliably start a container

raise the limit to 8MB

tophj-ibm · 2018-06-04T19:07:19Z

Also: this test, as well as TestEventsOOMDisableFalse / True, are currently being skipped by the CI due to the CI machines not having cgroup / swap enabled.

IIRC when we enabled it first ppc64le, there was an issue that might be a ppc64le specific issue, and we never got around to the other architectures. It's probably worth testing enabling this on janky so the tests can check these failures in the future.

thaJeztah · 2018-06-04T20:18:05Z

IIRC when we enabled it first ppc64le, there was an issue that might be a ppc64le specific issue

Ah, yes, didn't that relate to "swap" being enabled on some machines (therefore not resulting in OOM, but instead memory being swapped to disk)?

kolyshkin · 2018-06-04T21:22:20Z

So, there's memory limit and memory+swap limit, by default if you set memory limit to say 8M the mem+swap limit is set to 16M (I believe by dockerd, not the kernel). Even if there's no swap available, OOM killer can be initiated. That's just from the top of my head, maybe I'm not aware of some nuances.

Signed-off-by: Anda Xu <[email protected]>

codecov · 2018-06-04T21:29:33Z

Codecov Report

Merging #37194 into master will decrease coverage by 0.03%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #37194      +/-   ##
==========================================
- Coverage   34.62%   34.59%   -0.04%     
==========================================
  Files         605      605              
  Lines       44765    44765              
==========================================
- Hits        15499    15485      -14     
- Misses      27166    27178      +12     
- Partials     2100     2102       +2

AntaresS · 2018-06-04T21:30:53Z

I'd suggest to
remove the comment about rhel 7.4 et al kernels replacing it with something like
it appears that 8MB of memory is the minimum needed to reliably start a container raise the limit to 8MB

👍

Fixed.

thaJeztah

LGTM

These tests were skipped because of an issue with the test, which was fixed by moby#37194 Signed-off-by: Christopher Jones <[email protected]>

AntaresS requested a review from vdemeester as a code owner June 1, 2018 23:22

GordonTheTurtle added the status/0-triage label Jun 1, 2018

AntaresS force-pushed the fix-test-failure branch from d37fc44 to 1824e3d Compare June 1, 2018 23:25

tiborvass approved these changes Jun 1, 2018

View reviewed changes

thaJeztah reviewed Jun 2, 2018

View reviewed changes

vdemeester added status/2-code-review and removed status/0-triage labels Jun 4, 2018

kolyshkin requested changes Jun 4, 2018

View reviewed changes

thaJeztah added the area/testing label Jun 4, 2018

fix a failed test

1d9973c

Signed-off-by: Anda Xu <[email protected]>

AntaresS force-pushed the fix-test-failure branch from 1824e3d to 1d9973c Compare June 4, 2018 21:29

thaJeztah approved these changes Jun 5, 2018

View reviewed changes

thaJeztah added status/4-merge and removed status/2-code-review labels Jun 5, 2018

thaJeztah merged commit ca25df3 into moby:master Jun 5, 2018

tophj-ibm added a commit to tophj-ibm/moby that referenced this pull request Jun 6, 2018

Enable OOM killer tests on ppc64le

1f96feb

These tests were skipped because of an issue with the test, which was fixed by moby#37194 Signed-off-by: Christopher Jones <[email protected]>

tophj-ibm mentioned this pull request Jun 6, 2018

[TESTING DO NOT MERGE] Enable OOM killer tests on ppc64le #37220

Closed

thaJeztah changed the title ~~fix a failed test~~ fix a failed test TestRunOOMExitCode Jul 16, 2018

Conversation

AntaresS commented Jun 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AntaresS commented Jun 1, 2018

Uh oh!

tiborvass left a comment

Choose a reason for hiding this comment

Uh oh!

thaJeztah Jun 2, 2018

Choose a reason for hiding this comment

Uh oh!

kolyshkin Jun 4, 2018

Choose a reason for hiding this comment

Uh oh!

kolyshkin Jun 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kolyshkin Jun 4, 2018

Choose a reason for hiding this comment

Uh oh!

kolyshkin Jun 4, 2018

Choose a reason for hiding this comment

Uh oh!

tophj-ibm commented Jun 4, 2018

Uh oh!

thaJeztah commented Jun 4, 2018

Uh oh!

kolyshkin commented Jun 4, 2018

Uh oh!

codecov Bot commented Jun 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

AntaresS commented Jun 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

AntaresS commented Jun 1, 2018 •

edited

Loading

kolyshkin Jun 4, 2018 •

edited

Loading

codecov Bot commented Jun 4, 2018 •

edited

Loading

AntaresS commented Jun 4, 2018 •

edited

Loading