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>
Fixes the following warning:
> In update.bats line 422:
> local root_period=$(cat "${CGROUP_CPU_BASE_PATH}/cpu.rt_period_us")
> ^---------^ SC2155: Declare and assign separately to avoid masking return values.
>
>
> In update.bats line 423:
> local root_runtime=$(cat "${CGROUP_CPU_BASE_PATH}/cpu.rt_runtime_us")
> ^----------^ SC2155: Declare and assign separately to avoid masking return values.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Fixes the following warning:
> In cgroups.bats line 58:
> if [ "$KERNEL_MAJOR" -lt 4 ] || [ "$KERNEL_MAJOR" -eq 4 -a "$KERNEL_MINOR" -le 5 ]; then
> ^-- SC2166: Prefer [ p ] && [ q ] as [ p -a q ] is not well defined.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
1. cd is useless as all the paths are absolute
2. run is redundant, does not make sense to use it
3. use mkdir -p to save a line of code
This also eliminates shellcheck warnings like this one:
> In spec.bats line 8:
> cd "$INTEGRATION_ROOT"
> ^--------------------^ SC2164: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
It's not a regex but a substring, so use a substring match.
Fixes the following warning by shellcheck:
> In mounts.bats line 20:
> [[ "${lines[0]}" =~ '/tmp/bind/config.json' ]]
> ^---------------------^ SC2076: Don't quote right-hand side of =~, it'll match literally rather than as a regex.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The ending EOF should be
- all by itself (i.e. no extra characters on the same line);
- with no whitespace before it.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
TL;DR: this allows to show logs from failed runc restore.
Bats scripts are run with `set -e`. This is well known and obvious,
and yet there are a few errors with respect to that, including a few
"gems" by yours truly.
1. bats scripts are run with `set -e`, meaning that `[ $? -eq 0 ]` is
useless since the execution won't ever reach this line in case of
non-zero exit code from a preceding command. So, remove all such
checks, they are useless and misleading.
2. bats scripts are run with `set -e`, meaning that `ret=$?` is useless
since the execution won't ever reach this line in case of non-zero
exit code from a preceding command.
In particular, the code that calls runc restore needs to save the exit
code, show the errors in the log, and only when check the exit code and
fail if it's non-zero. It can not use `run` (or `runc` which uses `run`)
because of shell redirection that we need to set up.
The solution, implemented in this patch, is to use code like this:
```bash
ret=0
__runc ... || ret=$?
show_logs
[ $ret -eq 0 ]
```
In case __runc exits with non-zero exit code, `ret=$?` is executed, and
it always succeeds, so we won't fail just yet and have a chance to show
logs before checking the value of $ret.
In case __runc succeeds, `ret=$?` is never executed, so $ret will still
be zero (this is the reason why it needs to be set explicitly).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The only two tests that are still skipped on v2 are kmem
and invalid CpuShares test -- since v2 does not support either.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
I have noticed that `go vet` from golang 1.13 ignores the vendor/
subdir, downloading all the modules when invoked in Travis CI env.
As the other go commands, in 1.13 it needs explicit -mod=vendor
flag, so let's provide one.
PS once golang 1.13 is unsupported, we will drop it.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
All the test cases are doing the same checks, only input differs,
so we can unify those using a test data table.
While at it:
- use t.Fatalf where it makes sense (no further checks are possible);
- remove the "XXX" comments as we won't get rid of cgroup Name/Parent.
PS I tried using t.Parallel() as well but it did not result in any
noticeable speedup, so I dropped it for simplicity.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
If the CRIU binary is in a non $PATH location and passed to runc via
'--criu /path/to/criu', this information has not been passed to go-criu
and since the switch to use go-criu for CRIU version detection, non
$PATH CRIU usage was broken. This uses the newly added go-criu interface
to pass the location of the binary to go-criu.
Signed-off-by: Adrian Reber <areber@redhat.com>
...by checking the default path first.
Quick benchmark shows it's about 5x faster on an idle system, and the
gain should be much more on a system doing mounts etc.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(mode&S_IFCHR == S_IFCHR) is the wrong way of checking the type of an
inode because the S_IF* bits are actually not a bitmask and instead must
be checked using S_IF*. This bug was neatly hidden behind a (major == 0)
sanity-check but that was removed by [1].
In addition, add a test that makes sure that HostDevices() doesn't give
rubbish results -- because we broke this and fixed this before[2].
[1]: 24388be71e ("configs: use different types for .Devices and .Resources.Devices")
[2]: 3ed492ad33 ("Handle non-devices correctly in DeviceFromPath")
Fixes: b0d014d0e1 ("libcontainer: one more switch from syscall to x/sys/unix")
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Trying to checkpoint a container out of pod in cri-o fails with:
Error (criu/namespaces.c:1081): Can't dump a pid namespace without the process init
Starting with the upcoming CRIU release 3.15, CRIU can be told to ignore
the PID namespace during checkpointing and to restore processes into an
existing network namespace.
With the changes from this commit and CRIU 3.15 it is possible to
checkpoint a container out of a pod in cri-o.
Signed-off-by: Adrian Reber <areber@redhat.com>
To checkpoint and restore a container with an external network namespace
(like with Podman and CNI), runc tells CRIU to ignore the network
namespace during checkpoint and restore.
This commit moves that code to their own functions to be able to reuse
the same code path for external PID namespaces which are necessary for
checkpointing and restoring containers out of a pod in cri-o.
Signed-off-by: Adrian Reber <areber@redhat.com>
Mrunal Patel