This test is racy for a long time now. All the logs I could find in CI
seem to be dangling symlinks, like the test shows "23 -> ". This means
the fd was closed before we did the call to readlink().
Let's try to disable the GC. This should get rid of the "fds are getting
closed before we read them" part.
Updates: #4297
Signed-off-by: Rodrigo Campos <rodrigo@amutable.com>
By default, readlink is silent about any errors. Make it verbose so we
can better interpret any test failures.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
When running a process inside a container, make sure its stderr is not
nil (except for some trivial cases like cat). Modify waitProcess to show
failed command's stderr, if possible.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Since Wait returns an ExitError if process' exit status is not 0,
checking process status is redundant and this code is never reached.
Remove it.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
add bat integration test for rootfs propagation test, expect to
see the mount propagation is slave, the test will create a isolate mntns
to run the test as the test will mutate the rootfs propagation
Signed-off-by: sean <xujihui1985@gmail.com>
When rootfsPropagation is set to rslave, prepareRoot() was forcing the
rootfs parent mount to MS_PRIVATE before bind-mounting and pivoting into
the rootfs. That breaks the slave relationship needed for HostToContainer
propagation, so later unmount/remount events on host mountpoints under
the rootfs are not reflected inside the running container.
Fix this by keeping the rootfs parent mount as MS_SLAVE for slave-like
rootfs propagation settings, while leaving the final root propagation
remount in place.
Signed-off-by: sean <xujihui1985@gmail.com>
The RUNC_ALLOW_UNSAFE_TESTS variable set in the Cirrus CI env block
does not reach the integration tests because they are executed via
"ssh -tt localhost make ...", which starts a new login shell that
does not inherit the caller's environment. As a result, unsafe tests
are always skipped in Cirrus CI even though the intent is to run them.
Fix this by exporting the variable in /root/.bashrc (same way we
already handle PATH), so the ssh session picks it up.
See #5212 (comment).
Fixes: 9932ad19 ("tests/int: introduce the concept of unsafe tests")
Signed-off-by: RedMakeUp <girafeeblue@gmail.com>
Those tests were added by commit 8d180e96 ("Add support for Linux
Network Devices"), apparently by copy-pasting the test cases which
call simple_cr (all four of them).
While different simple_cr tests make sense as they cover different
code paths in runc and/or check for various regression, the same
variations with netdevice do not make sense, as having a net device
is orthogonal to e.g. bind mount, --debug, or cgroupns.
Remove those.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
When RUNC_USE_SYSTEMD is set, tests/rootless.sh is using
ssh -tt rootless@localhost
to run tests as rootless user. In this case, local environment is not
passed to the user's ssh session (unless explicitly specified), and so
the tests do not get ROOTLESS_FEATURES.
As a result, idmap-related tests are skipped when running as rootless
using systemd cgroup driver:
integration test (systemd driver)
...
[02] run rootless tests ... (idmap)
...
ok 286 runc run detached ({u,g}id != 0) # skip test requires rootless_idmap
...
Fix this by creating a list of environment variables needed by the
tests, and adding those to ssh command line (in case of ssh) or
exporting (in case of sudo) so both cases work similarly.
Also, modify disable_idmap to unset variables set in enable_idmap so
they are not exported at all if idmap is not in features.
Fixes: bf15cc99 ("cgroup v2: support rootless systemd")
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
These helpers all make more sense as a self-contained package and moving
them has the added benefit of removing an unneeded libpathrs dependency
(from libcontainer/utils's import of pathrs-lite) from several test
binaries.
Signed-off-by: Aleksa Sarai <aleksa@amutable.com>
This allows users to automaticaly migrate to the new location
using `go fix`. It has some limitations, but can help smoothen
the transition; for example, taking this file;
```
package main
import (
"github.com/opencontainers/runc/libcontainer/devices"
)
func main() {
_, _ = devices.DeviceFromPath("a", "b")
_, _ = devices.HostDevices()
_, _ = devices.GetDevices("a")
}
```
Running `go fix -mod=readonly ./...` will migrate the code;
```
package main
import (
devices0 "github.com/moby/sys/devices"
)
func main() {
_, _ = devices0.DeviceFromPath("a", "b")
_, _ = devices0.HostDevices()
_, _ = devices0.GetDevices("a")
}
```
updates b345c78dca
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Some of runc integration tests may do something that I would not like
when running those on my development laptop. Examples include
- changing the root mount propagation [1];
- replacing /root/runc [2];
- changing the file in /etc (see checkpoint.bats).
Yet it is totally fine to do all that in a throwaway CI environment,
or inside a Docker container.
Introduce a mechanism to skip specific "unsafe" tests unless an
environment variable, RUNC_ALLOW_UNSAFE_TESTS, is set. Use it
from a specific checkpoint/restore test which modifies
/etc/criu/default.conf.
[1]: https://github.com/opencontainers/runc/pull/5200
[2]: https://github.com/opencontainers/runc/pull/5207
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The runtime-spec [1] currently says:
> 6. Runtime's start command is invoked with the unique identifier of
> the container.
> 7. The startContainer hooks MUST be invoked by the runtime. If any
> startContainer hook fails, the runtime MUST generate an error, stop
> the container, and continue the lifecycle at step 12.
> 8. The runtime MUST run the user-specified program, as specified by
> process.
> 9. The poststart hooks MUST be invoked by the runtime. If any
> poststart hook fails, the runtime MUST generate an error, stop the
> container, and continue the lifecycle at step 12.
> ...
> 11. Runtime's delete command is invoked with the unique identifier of
> the container.
> 12. The container MUST be destroyed by undoing the steps performed
> during create phase (step 2).
> 13. The poststop hooks MUST be invoked by the runtime. If any poststop
> hook fails, the runtime MUST log a warning, but the remaining hooks
> and lifecycle continue as if the hook had succeeded.
Currently, we do 9 before 8 (heck, even before 6), which is clearly
against the spec and results in issues like the one described in [2].
Let's move running poststart hook to after the user-specified process
has started.
NOTE this patch only fixes the order and does not implement removing
the container when the poststart hook failed (as this part of the spec
is controversial -- destroy et al and should probably be, and currently
are, part of "runc delete").
[1]: https://github.com/opencontainers/runtime-spec/blob/main/runtime.md#lifecycle
[2]: https://github.com/opencontainers/runc/issues/5182
Reported-by: ningmingxiao <ning.mingxiao@zte.com.cn>
Reported-by: Erik Sjölund <erik.sjolund@gmail.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Rename c.signal to c.signalInit, and add c.signal which is a lock-less
form of c.Signal.
To be used by the next patch.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The libcontainer/devices package has been moved to moby/sys/devices, so
we can just point users to that and keep some compatibility shims around
until runc 1.6. We don't use it at all so there are no other changes
needed.
Signed-off-by: Aleksa Sarai <aleksa@amutable.com>
When signal installation was moved to a goroutine for performance,
containers that exited quickly could complete before SIGCHLD was
registered, causing runc to hang waiting for the signal.
This fix ensures SIGCHLD is registered immediately in the main thread
before other signals are handled in the goroutine, maintaining performance
while guaranteeing no missed SIGCHLD notifications for fast-exiting
containers.
Reported-by: Ayato Tokubi <atokubi@redhat.com>
Signed-off-by: lifubang <lifubang@acmcoder.com>
For detached container, we don't need to setup signal notifier, because
there is no customer to consume the signals in `forward()`.
Signed-off-by: lifubang <lifubang@acmcoder.com>
In fact, notifySocket has no relationship to signalHandler, we
can move it out of signalHandler to make the code more clear.
Signed-off-by: lifubang <lifubang@acmcoder.com>
We should use a named return value of error, or else we can't
catch all errors when calling defer function, for example we
used a block scope var name `err` for `setupPidfdSocket`.
Signed-off-by: lifubang <lifubang@acmcoder.com>
Kir Kolyshkin