I have noticed that ConfigureLogs do not return an error in case logging
was already configured -- instead it just warns about it. So I went
ahead and changed the warning to the actual error...
... only to discover I broke things badly, because in case of runc init
logging is configured twice. The fix is to not configure logging in case
we are init.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Error handling is slightly cleaner this way.
While at it, do minor refactoring and fix error logging
in processEntry.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
1. Remove redundant "echo $output" from the first test case, as "runc"
helper function already logs the output.
2. Show the contents of log.out to stderr, so it case of error we can
see what is going on.
3. Remove the check that `log.out` file exists. This check is redundant,
since right after it we do `cat log.out` and check its exit code.
4. Factor out common checks into check_debug.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Sometimes the test fails without any clear diagnostics:
> not ok 16 checkpoint --lazy-pages and restore
> (in test file tests/integration/checkpoint.bats, line 191)
> `[ "$out" = "0000000 000000 0000001" ]' failed
> ...
> criu failed: type NOTIFY errno 3\nlog file: work-dir/dump.log
We look for and print errors via grep, but in the above case
there are nothing that is denoted error in the log.
So, let's show the damn log in its entirely (note it is only shown
if test fails).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Sometimes github gives up 5xx errors, for example:
> panic: getImages error exit status 1 (output: curl: (22) The requested URL returned error: 502
> Failed to get https://github.com/docker-library/busybox/raw/dist-i386/stable/glibc/busybox.tar.xz
This is unfortunate but temporary, and adding --retry should handle
at least some cases, improving CI stability.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
bfq weight controller (i.e. io.bfq.weight if present) is still using the
same bfq weight scheme (i.e 1->1000, see [1].) Unfortunately the
documentation for this was wrong, and only fixed recently [2].
Therefore, if we map blkio weight to io.bfq.weight, there's no need to
do any conversion. Otherwise, we will try to write invalid value which
results in error such as:
```
time="2021-02-03T14:55:30Z" level=error msg="container_linux.go:367: starting container process caused: process_linux.go:495: container init caused: process_linux.go:458: setting cgroup config for procHooks process caused: failed to write \"7475\": write /sys/fs/cgroup/runc-cgroups-integration-test/test-cgroup/io.bfq.weight: numerical result out of range"
```
[1] https://github.com/torvalds/linux/blob/master/Documentation/block/bfq-iosched.rst
[2] https://github.com/torvalds/linux/commit/65752aef0a407e1ef17ec78a7fc31ba4e0b360f9
Signed-off-by: Daniel Dao <dqminh89@gmail.com>
An exec may fail due to memory shortage (cgroup memory limits being too
tight), and an error message provided in this case is clueless:
> $ sudo ../runc/runc exec xx56 top
> ERRO[0000] exec failed: container_linux.go:367: starting container process caused: read init-p: connection reset by peer
Same as the previous commit for run/start, check the OOM kill counter
and report an OOM kill.
The differences from run are
1. The container is already running and OOM kill counter might not be
zero. This is why we have to read the counter before exec and after
it failed.
2. An unrelated OOM kill event might occur in parallel with our exec
(and I see no way to find out which process was killed, except to
parse kernel logs which seems excessive and not very reliable).
This is why we report _possible_ OOM kill.
With this commit, the error message looks like:
> ERRO[0000] exec failed: container_linux.go:367: starting container process caused: process_linux.go:105: possibly OOM-killed caused: read init-p: connection reset by peer
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In some cases, container init fails to start because it is killed by
the kernel OOM killer. The errors returned by runc in such cases are
semi-random and rather cryptic. Below are a few examples.
On cgroup v1 + systemd cgroup driver:
> process_linux.go:348: copying bootstrap data to pipe caused: write init-p: broken pipe
> process_linux.go:352: getting the final child's pid from pipe caused: EOF
On cgroup v2:
> process_linux.go:495: container init caused: read init-p: connection reset by peer
> process_linux.go:484: writing syncT 'resume' caused: write init-p: broken pipe
This commits adds the OOM method to cgroup managers, which tells whether
the container was OOM-killed. In case that has happened, the original error
is discarded (unless --debug is set), and the new OOM error is reported
instead:
> ERRO[0000] container_linux.go:367: starting container process caused: container init was OOM-killed (memory limit too low?)
Also, fix the rootless test cases that are failing because they expect
an error in the first line, and we have an additional warning now:
> unable to get oom kill count" error="no directory specified for memory.oom_control
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This makes the code simpler and more future-proof, in case
any more values will appear in hugetlb.*.events.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Generalize the libct/getValueFromCgroup() as fscommon.GetValueByKey(),
and document it.
No changes other than using fscommon.ParseUint to convert the value.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This is to benefit from openat2() implementation, on kernels
that support it. Theoretically this also improves security.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
1. This is the only function in the package with Get prefix
that does not read a file (but parses a string). Rename
accordingly, and convert the callers.
GetCgroupParamKeyValue -> ParseKeyValue
2. Use strings.Split rather than strings.Fields. Split by a space
is 2x faster, plus we can limit the splitting. The downside is
we have to strip a newline in one of the callers.
3. Improve the doc and the code flow.
4. Fix a test case with invalid data (spaces at BOL).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
EBUSY when trying to set memory limit may mean the new limit is too low
(lower than the current usage, and the kernel can't do anything).
Provide a more specific error for such case.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
1. Factor out setMemory and setSwap
2. Pass cgroup.Resources (rather than cgroup) to setMemoryAndSwap().
3. Merge the duplicated "set memory, set swap" case.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Currently, we read and parse 5 different files while we only need 1.
Use GetCgroupParamUint() directly to get current limit.
While at it, remove the workaround previously needed for the unit test,
and make it a bit more verbose.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Add integration test coverage for code initially added by commit
d8b8f76c4f ("Fix problem when update memory and swap memory",
2016-04-05).
This is in addition to existing unit test,
TestMemorySetSwapSmallerThanMemory.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Kir Kolyshkin