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>
All cgroup managers has Apply() and Set() methods:
- Apply is used to create a cgroup (and, in case of systemd,
a systemd unit) and/or put a PID into the cgroup (and unit);
- Set is used to set various cgroup resources and limits.
The fs/fs2 cgroup manager implements the functionality as described above.
The systemd v1/v2 manager deviate -- it sets *most* of cgroup limits
(those that can be projected to systemd unit properties) in Apply(),
and then again *all* cgroup limits in Set (first indirectly via systemd
properties -- same as in Apply, then via cgroupfs).
This commit removes setting the cgroup limits from Apply,
so now the systemd manager behaves the same way as the fs manager.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
1. Get rid of fixed ROOT, *_BUNDLE, and CONSOLE_SOCKET dirs.
Now they are temporary directories created in setup_bundle.
2. Automate containers cleanup: instead of having to specify all
containers to be removed, list and destroy everything (which is
now possible since every test case has its own unique root).
3. Randomize cgroup paths so two tests running in parallel won't
use the same cgroup.
Now it's theoretically possible to execute tests in parallel.
Practically it's not possible yet because bats uses GNU parallel,
which do not provide a terminal for whatever it executes, and
many runc tests (all those that run containers with terminal:
true) needs a tty. This may possibly be addressed later.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
1. Do not depend on $DEBIAN_BUNDLE, instead use the current directory.
2. Simplify setup() by calling teardown().
3. In teardown(), check that LIBPATH is set.
4. Move global variables into setup.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
It's a little known fact, but BATS_TMPDIR is just /tmp, and it is not
being cleaned by bats after the test.
BATS_RUN_TMPDIR (available since bats 1.2.1) is $BATS_TMPDIR/bats-run-$$
(i.e. it is per bats run and includes PID), and it is cleaned up after
the test (unless --no-tmpdir-cleanup is given).
Use BATS_RUN_TMPDIR for better isolation and cleanup.
[v2: check for BATS_RUN_TMPDIR, effectively requiring bats >= 1.2.1]
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Overloading $HELLO_BUNDLE with a second purpose of having an alternative
runc root is confusing.
Instead, let's create a temp directory in setup() and clean it up in
teardown().
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
... by using retry, to avoid code duplication.
While at it, make it check the number of arguments and error out on
invalid usage.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
It is only used in a couple of places and is easy to replace
with ROOT=xxx teardown_running_container.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
All the tests are run with cd to bundle directory, so
it does not make ense to explicitly specify it.
This relies on the feature of functions like set_cgroups_path
or update_config to use current directory by default.
In those cases where we need to change the directory
(runc create/run -b), save the current one and use it later.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The test verifies if the device file can be queried using
'access(dev_name, F_OK)' when the permissions are set to 'rw'. The call
does not explicitly trigger read or write access but should succeed.
Signed-off-by: Vasiliy Ulyanov <vulyanov@suse.de>
Aleksa Sarai