runc already tells CRIU to restore into an existing network or PID
namespace if there is a path to a namespace specified in config.json.
PID and network have special handling in CRIU using CRIU's inherit_fd
interface.
For UTS, IPC and MOUNT namespaces CRIU can join those existing
namespaces using CRIU's join_ns interface.
This is especially interesting for environments where containers are
running in a pod which already has running containers (pause for
example) with namespaces configured and the restored container needs to
join these namespaces.
CRIU has no support to join an existing CGROUP namespace (yet?) why
restoring a container with a path specified to a CGROUP namespace will
be aborted by runc.
CRIU would have support to restore a container into an existing time
namespace, but runc does not yet seem to support time namespaces.
Signed-off-by: Adrian Reber <areber@redhat.com>
full diff: https://github.com/moby/sys/compare/mountinfo/v0.1.3...mountinfo/v0.2.0
Bug fixes:
- Fix path unescaping for paths with double quotes
Improvements:
- Mounted: speed up by adding fast paths using openat2 (Linux-only) and stat
- Mounted: relax path requirements (allow relative, non-cleaned paths, symlinks)
- Unescape fstype and source fields
- Documentation improvements
Testing/CI:
- Unit tests: exclude darwin
- CI: run tests under Fedora 32 to test openat2
- TestGetMounts: fix for Ubuntu build system
- Makefile: fix ignoring test failures
- CI: add cross build
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
For test cases where we used pipes for container's stdin/stdout/stderr,
stderr was redirected to the same pipe as stdout, which practically
means it is lost.
These redirects to fd is needed not because we check that container is
working by writing to its stdin and reading from stdout (see
check_pipes), but also because bats redirects test stdout/stderr to a
file, which makes c/r impossible (as the file is outside of container).
This is why we can't just do something like `2>stderr.log`, and have
to do what is done in this commit.
Introduce and use another pipe for stdout, to be used for both runc run
and runc restore, so it will be shown in case of errors.
Since its handling is somewhat complicated and is used from 4 places
(2 for run, 2 for restore), separate it into a helper functions.
NOTE the code assumes that runc exits with non-zero exit code in case
there is anything that needs to be shown to a user from runc's stderr.
While at it, add error checking to runc run calls.
Hopefully, this will help debug those rare checkpoint failures in CI.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Commit 417f5ff40 added a code to close fds and kill processes, which
should have helped in an event of test case failure.
In fact, each test case is executed in a subshell, so
- any variables set there can't reach teardown();
- all the fds are closed (as the process is gone).
Now, I am not sure about the processes, but the code being removed
has never worked anyway, so it does not make sense to keep it.
Normally, those are waited for. In case of a test case failure,
well, the subsequent cases might fail, too.
Fixes: 417f5ff40d
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This fixes the following issue with the test:
not ok 12 checkpoint --lazy-pages and restore
# (in test file tests/integration/checkpoint.bats, line 202)
# `[ $ret -eq 0 ]' failed
...
# grep: ./work-dir/restore.log: No such file or directory
One might think that `--work-path ./image-dir` is a mistake, but it's
not, since `criu lazy-pages -D ./image-dir` creates a socket in
./image-dir, and then `criu restore --lazy-pages` expects a socket in
the workdir.
Fixes: e232a71a3d
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
These tests expect group name to be "nogroup", while recent busybox
changed that to "nobody".
Use numeric uids/gids to fix.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This test started to fail, as it expected the output from
`id` to be
uid=1000 gid=1000 groups=100(users),65534(nogroup)
while the actual output is now
uid=1000 gid=1000 groups=100(users),65534(nobody)
Apparently, busybox image changed group name.
As we're only interested in ids, not names, and to fix the test,
let's use `id -G`.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
It's expect that signalAllProcesses is invoked when container shares
pid namespace. share pid ns contains the following conditions:
{
// no specify pid ns
}
{
"type": "pid",
"path": "/proc/${num}/ns/pid"
}
Signed-off-by: Shukui Yang <jryangshukui@jd.com>
Signed-off-by: Shukui Yang <keloyangsk@gmail.com>
In all these cases, getSubsystemPath() was already called, and its
result stored in m.paths map. It makes no sense to not reuse it.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
We call joinCgroups() from Apply, and in there we iterate through the
list of subsystems, calling getSubsystemPath() for each. This is
expensive, since every getSubsystemPath() involves parsing mountinfo.
At the end of Apply(), we iterate through the list of subsystems to fill
the m.paths, again calling getSubsystemPath() for every subsystem.
As a result, we parse mountinfo about 20 times here.
Let's find the paths first and reuse m.paths in joinCgroups().
While at it, since join() is just two calls now, inline it.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
When paths are set, we only need to place the PID into proper
cgroups, and we do know all the paths already.
Both fs/d.path() and systemd/v1/getSubsystemPath() parse
/proc/self/mountinfo, and the only reason they are used
here is to check whether the subsystem is available.
Use a much simpler/faster check instead.
Frankly, I am not sure why the check is needed at all. Maybe it should
be dropped.
Also, for fs driver, since d is no longer used in this code path,
move its initialization to after it.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This looks like this is just filling logs for years, since the kernel never
added the support for automatically labeling /dev/mqueue.
Removes these dmesg lines
[ 1731.969847] SELinux: mount invalid. Same superblock, different security settings for (dev mqueue, type mqueue)
[ 1736.985146] SELinux: mount invalid. Same superblock, different security settings for (dev mqueue, type mqueue)
[ 1738.356796] SELinux: mount invalid. Same superblock, different security settings for (dev mqueue, type mqueue)
[ 1738.479952] SELinux: mount invalid. Same superblock, different security settings for (dev mqueue, type mqueue)
[ 1738.628935] SELinux: mount invalid. Same superblock, different security settings for (dev mqueue, type mqueue)
[ 1763.433276] SELinux: mount invalid. Same superblock, different security settings for (dev mqueue, type mqueue)
[ 1806.802133] SELinux: mount invalid. Same superblock, different security settings for (dev mqueue, type mqueue)
[ 1806.982003] SELinux: mount invalid. Same superblock, different security settings for (dev mqueue, type mqueue)
[ 1808.955390] SELinux: mount invalid. Same superblock, different security settings for (dev mqueue, type mqueue)
[ 1815.951076] SELinux: mount invalid. Same superblock, different security settings for (dev mqueue, type mqueue)
[ 1827.257757] SELinux: mount invalid. Same superblock, different security settings for (dev mqueue, type mqueue)
[ 1828.947888] SELinux: mount invalid. Same superblock, different security settings for (dev mqueue, type mqueue)
[ 1834.964451] SELinux: mount invalid. Same superblock, different security settings for (dev mqueue, type mqueue)
[ 1835.941465] SELinux: mount invalid. Same superblock, different security settings for (dev mqueue, type mqueue)
Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
The `apparmor_parser` binary is not really required for a system to run
AppArmor from a runc perspective. How to apply the profile is more in
the responsibility of higher level runtimes like Podman and Docker,
which may do the binary check on their own.
Signed-off-by: Sascha Grunert <sgrunert@suse.com>
Note that stdout/stderr are already redirected by bats'
`run` command, so the only way to get a controlling terminal
is to open /dev/tty (which fails if there isn't one).
Here's how I tested the failure to open /dev/tty:
> [root@kir-rhat ~]# ssh -T root@localhost cat ./runme
> cd /home/kir/go/src/github.com/opencontainers/runc
> ./runc run -b tst xxx-$$
> echo $?
>
> [root@kir-rhat ~]# ssh -T root@localhost ./runme
> time="2020-07-31T16:35:47-07:00" level=error msg="chdir tst: no such file or directory"
> 1
If anyone knows how to obtain an tty-less environment without using
ssh -T, please raise your hand.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This fixes the following failure:
> sudo runc run -b bundle ctr </dev/null
> WARN[0000] exit status 2
> ERRO[0000] container_linux.go:367: starting container process caused: process_linux.go:459: container init caused:
The "exit status 2" with no error message is caused by SIGHUP
which is sent to init by the kernel when we are losing the
controlling terminal. If we choose to ignore that, we'll get
panic in console.Current(), which is addressed by [1].
Otherwise, the issue here is simple: the code assumes stdin
is opened to a terminal, and fails to work otherwise. Some
standard Linux tools (e.g. stty, top) do the same (modulo panic),
while some others (reset, tput) use the trick of trying
all the three std streams (starting with stderr as it is least likely
to be redirected), and if all three fails, open /dev/tty.
This commit does a similar thing (see initHostConsole).
It also replaces the call to console.Current(), which may panic
(see [1]), by reusing the t.hostConsole.
Finally, a simple test case is added.
Fixes: https://github.com/opencontainers/runc/issues/2485
[1] https://github.com/containerd/console/pull/37
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Commit 335f0806c added a test case doing
```bash
for i in $(seq 1); do
...
done
```
and it does not make any sense to have it since we're only performing
a single iteration.
Remove the code.
I have not touched the indentation, for the sake of cleaner review,
also because already have different intentation in different tests;
this should be addressed separately.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Cesar Talledo (2):
Remove runc default devices that overlap with spec devices.
Skip redundant setup for /dev/ptmx when specified explicitly in the OCI spec.
LGTMs: @AkihiroSuda @cyphar
Closes#2522
Currently all the shellcheck warnings are fixed, and we'd like it to
stay thay way. So, add shellcheck call to validate target in Makefile,
which is run on Travis CI.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Ignore the shellcheck warnings like this one:
> In tty.bats line 32:
> update_config '(.. | select(.[]? == "sh")) += ["-c", "stat -c %u:%g $(tty) | tr : \\\\n"]'
> ^-- SC2016: Expressions don't expand in single quotes, use double quotes for that.
While at it, fix some minor whitespace issues in tty.bats.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Ignore warnings like this:
> In checkpoint.bats line 169:
> PIDS_TO_KILL=($cpt_pid)
> ^------^ SC2206: Quote to prevent word splitting/globbing, or split robustly with mapfile or read -a.
Since in all the cases we deal with either pids or fds, and they don't
have spaces.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Fix or ignore warnings like this one:
> In cgroups.bats line 107:
> if [ $(id -u) = "0" ]; then
> ^------^ SC2046: Quote this to prevent word splitting.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Those are pretty simple to allow shellcheck to fix these, so
this commit is courtesy of
> shellcheck -i SC2086 -i SC2006 -f diff *.bats > fix.diff
> patch -p1 < fix.diff
repeated 3 times ;)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This fixes the following warning, and implements a suggestion:
> In update.bats line 426:
> IFS='/' read -r -a dirs <<< $(echo ${CGROUP_CPU} | sed -e s@^${CGROUP_CPU_BASE_PATH}/@@)
> ^-- SC2046: Quote this to prevent word splitting.
> ^-- SC2001: See if you can use ${variable//search/replace} instead.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Adrian Reber