This is a very simple test that checks that container.Run do not leak
opened file descriptors.
In fact it does, so we have to add two exclusions:
1. /sys/fs/cgroup is opened once per lifetime in prepareOpenat2(),
provided that cgroupv2 is used and openat2 is available. This
works as intended ("it's not a bug, it's a feature").
2. ebpf program fd is leaked every time we call setDevices() for
cgroupv2 (iow, every container.Run or container.Set leaks 1 fd).
This needs to be fixed, thus FIXME.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Apparently, the parent never closes execFifo fd. Not a problem for runc
per se, but can be an issue for a user of libcontainer.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The full log is very long and it did not gave us any additional clues.
This reverts commit 053e15c001.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
First, add runc --debug exec test cases, very similar to those in
debug.bats but for runc exec (rather than runc run). Do not include json
tests as it is already tested in debug.bats.
Second, add logrus debug to late stages of runc init, and amend the
integration tests to check for those messages. This serves two purposes:
- demonstrate that runc init can be amended with debug logrus which is
properly forwarded to and logged by the parent runc create/run/exec;
- improve the chances to catch the race fixed by the previous commit.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Sometimes debug.bats test cases are failing like this:
> not ok 27 global --debug to --log --log-format 'json'
> # (in test file tests/integration/debug.bats, line 77)
> # `[[ "${output}" == *"child process in init()"* ]]' failed
It happens more when writing to disk.
This issue is caused by the fact that runc spawns log forwarding goroutine
(ForwardLogs) but does not wait for it to finish, resulting in missing
debug lines from nsexec.
ForwardLogs itself, though, never finishes, because it reads from a
reading side of a pipe which writing side is not closed. This is
especially true in case of runc create, which spawns runc init and
exits; meanwhile runc init waits on exec fifo for arbitrarily long
time before doing execve.
So, to fix the failure described above, we need to:
1. Make runc create/run/exec wait for ForwardLogs to finish;
2. Make runc init close its log pipe file descriptor (i.e.
the one which value is passed in _LIBCONTAINER_LOGPIPE
environment variable).
This is exactly what this commit does:
1. Amend ForwardLogs to return a channel, and wait for it in start().
2. In runc init, save the log fd and close it as late as possible.
PS I have to admit I still do not understand why an explicit close of
log pipe fd is required in e.g. (*linuxSetnsInit).Init, right before
the execve which (thanks to CLOEXEC) closes the fd anyway.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
- add check, checkWait, and finish helpers;
- move test cleanup to runLogForwarding;
- introduce and use log struct.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
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>
As pointed out to in [1], seccomp.IsEnabled does some runtime checks,
while here (when printing libseccomp version) we should merely print
the version number of libseccomp, if compiled in.
Change the code to check if the version is non-zero (as seccomp.Version
returns 0, 0, 0 in case seccomp is not compiled in.
[1] https://github.com/opencontainers/runc/pull/2866
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Apparently, scripts/validate-c is not working in CI (or maybe
maintainers ignored the failures from it) -- current C code
gets some changes if we run indent on it.
This commit fixes this, simplifying things along the way.
In particular:
1. Remove "validate" make target, add "cfmt" target that just runs
indent on all *.c files in the repository (NOTE that *.h files
are not included, as before).
This may help a contributor to fix their code -- they just need
to run "make cfmt" now instead of running "make validate" and
copy-pasting the indent command and options from the hint.
2. Split GHA validate/misc into validate/release and validate/cfmt.
The latter checks that the sources are not changed after "make cfmt".
3. Adds a few more options to indent. This was mostly motivated by
trying to save the existing formatting, minimizing the amount of
changes indent produces.
The new options are:
* -il0: sets the offset for goto labels to 0 (currently all labels
but one are not indented -- let's keep it that way);
* -ppi2: sets the indentation for nested preprocessor directives
to 2 spaces (same as it is done in "SYS_memfd_create" defines);
* -cp1: sets the indentation between #else / #endif and the
following comment to 1 space.
4. Reformat the code using the new indent options.
5. Remove the now-unused script/{.validate,validate-c}.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Remove comments with architectures when defining SYS_memfd_create,
as they are redundant, and indent has a funny way of indenting them.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This block apparently does nothing except for creating
a need for additional indentation. Remove it.
While at it, break a long line in this code.
No functional change.
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>
if bfq is not loaded, then io.bfq.weight is not available. io.weight
should always be available and is the next best equivalent thing.
Signed-off-by: Daniel Dao <dqminh89@gmail.com>
Shiming Zhang