Commit Graph

423 Commits

Author SHA1 Message Date
Kir Kolyshkin 4d3e52f207 tests/int: fix a bad typo
As a result, cgroup v1 only tests are being skipped.

Fixes: a2123baf63
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-05-06 13:46:21 -07:00
Kir Kolyshkin d620a401d7 tests/int: remove $ROOTLESS, use $EUID
The variable $ROOTLESS, as set by helpers.bash and used in many places,
provides the same value as $EUID which is always set by bash. Since we
are using bash, we can rely on $EUID being omnipresent.

Modify all uses accordingly, and since the value is known to be a
number, omit the quoting.

Similarly, replace all uses of $(id -u) to $EUID.

Do some trivial cleanups along the way, such as
 - simplify some if A; then B; to A && B;
 - do not use [[ instead of [ where not necessary.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-03-23 11:12:44 -07:00
Kir Kolyshkin d330f94b57 tests/int/update.bats: fix extra reqs
This test requires both rootless and root, which does not make sense.

Remove the rootless part.

Fixes: d41a273da
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-03-23 11:12:44 -07:00
Kir Kolyshkin a2123baf63 tests/int: replace CGROUP_UNIFIED with CGROUP_V{1,2}
This makes it work similar to all the other variables we use as binary
flags.

The new 'shellcheck disable' is due to a bug in shellcheck (basically,
it does not track the scope of variables or execution order, assuming
everything is executed as soon as it is seen).

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-03-23 11:12:44 -07:00
Kir Kolyshkin 25ef852ad6 tests/int: use = in test for strings comparison
Strictly speaking, == is for [[ only, not for [ / test,
and, unlike =, the right side is a pattern.

To avoid confusion, use =. In cases where we compare with empty string,
use -z instead.

Keep using [[ in some cases since it does not require quoting the left
and right side of comparison (I trust shellcheck on that one).

This should have no effect (other than the code being a tad more
strict).

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-03-23 11:12:44 -07:00
Mrunal Patel b9d55d5852 Merge pull request #3367 from kolyshkin/tests-add-set-u
tests: add `set -u`
2022-03-22 14:24:31 -07:00
Qiang Huang c258ed0fc3 Merge pull request #3392 from kolyshkin/test-runc-delete-flake
tests/int: runc delete: fix flake, enable for rootless
2022-03-22 09:02:26 +08:00
Rodrigo Campos a99f82add1 tests: Add comment to clarify intent of seccomp-notify tests
While doing the previous fix, I went over all the tests in this file and
made sure they were named correctly. This patch just adds a small
sentence to clarify the intent, and does some minor improvements to some
other test names.

Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
2022-03-15 11:09:20 +01:00
Rodrigo Campos 9f9acd1a0c tests: Improve name of seccomp notify test
There was a typo and instead of "empty" we should have used "non-empty".

Let's add a small sentence explaining the intent (like other tests in
this file) and let's highlight what we expect to happen in this test (to
ignore the listenerPath).

Fixes: #3415

Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
2022-03-15 11:09:00 +01:00
Kir Kolyshkin 728571c16f tests/int: runc delete: fix flake, enable for rootless
The following failure was observed in CI (on centos-stream-8 in
integration-cgroup suite):

	not ok 42 runc delete
	 (from function `fail' in file tests/integration/helpers.bash, line 338,
	  in test file tests/integration/delete.bats, line 30)
	   `[ "$output" = "" ] || fail "cgroup not cleaned up correctly: $output"' failed
	....
	cgroup not cleaned up correctly: /sys/fs/cgroup/pids/system.slice/tmp-bats\x2drun\x2d68012-runc.IPOypI-state-testbusyboxdelete-runc.zriC8C.mount
	/sys/fs/cgroup/cpu,cpuacct/system.slice/tmp-bats\x2drun\x2d68012-runc.IPOypI-state-testbusyboxdelete-runc.zriC8C.mount
	...

Apparently, this is a cgroup systemd creates for a mount unit which
appears then runc does internal /proc/self/exe bind-mount. The test
case should not take it into account.

The second problem with this test is it does not check that cgroup
actually exists when the container is running (so checking that it
was removed after makes less sense). For example, in rootless mode
the cgroup might not have been created.

Fix the find arguments to look for a specific cgroup name, and add
a check that these arguments are correct (i.e. the cgroup is found
when the container is running).

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-03-07 15:33:29 -08:00
Kir Kolyshkin 38c21694ba tests/integration/helpers: set -u
This is a way to prevent the code doing something really bad when a
variable it uses is not set. Good to have since it helps to catch some
logical errors etc.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-02-28 18:41:39 -08:00
Kir Kolyshkin c8c3e8526d tests: fix checks for non-existent variables
Audit all checks for non-empty variables (i.e. ' -z ', ' -n ',
' != ""' and '= ""'), and fix those cases where a variable might be
unset. Those variables (that might not be set) are

 - RUNC_USE_SYSTEMD
 - BATS_RUN_TMPDIR
 - AUX_UID
 - AUX_DIR
 - SD_PARENT_NAME
 - REL_PARENT_PATH
 - ROOT
 - HAVE_CRIU
 - ROOTLESS_FEATURES
 - and a few test-specific or file-specific variables

This should allow us to enable set -u.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-02-28 18:41:35 -08:00
Kir Kolyshkin 99d5c0231f tests/int/{root,list}.bats: ALT_ROOT fixups in teardown
1. Add "unset ALT_ROOT" since it should not be used after teardown is
   called.

2. Remove "rm -rf $ALT_ROOT". It is not needed, because ALT_ROOT is a
   subdirectory of ROOT, which is removed in teardown_bundle.

3. Checking for ALT_ROOT being non-empty is a leftover from the era when
   teardown() was called as the first step from setup(). Since commit
   41670e21f0 this is no longer the case, so the condition
   is no longer needed (plus, the `set -u` which is about to be added
   should catch any possible use of unset ALT_ROOT).

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-02-28 18:39:08 -08:00
Kir Kolyshkin 7da77d802e tests/int: don't add --root if $ROOT is not set
Some tests (those in help.bats and version.bats) do not use setup_bundle
(as they do not need to start any containers), and thus they do not set
$ROOT. As a consequence, these tests now call "runc --root /state" which
is not nice.

Make adding --root conditional (only if $ROOT is set).

Amazingly, this change breaks help.bats tests under rootless, because
"sudo rootless" does not change the value of XDG_RUNTIME_DIR which still
points to root-owned directory, and as a result we have this:

> runc foo -h (status=1):
> the path in $XDG_RUNTIME_DIR must be writable by the user
> time="2022-02-08T07:04:57Z" level=error msg="mkdir /run/user/0/runc: permission denied"

This could be fixed by adding proper $ROOT, but it's easier just to skip
those tests under non-root.

NOTE that version.bats is not broken because -v is handled by urfave/cli
very early, so app.Before function is not run.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-02-28 18:32:10 -08:00
Kir Kolyshkin 9e2a0463e5 tests/int: fix runc_spec for set -u
Older bash versions treats variable as unset if nothing has been
assigned to it. Here is an example from CentOS 7 system:

	[kir@localhost ~]$ bash -u -c 'x() { local args=(); echo "${args[@]}"; }; x'
	bash: args[@]: unbound variable
	[kir@localhost ~]$ echo $BASH_VERSION
	4.2.46(2)-release

Rewrite to work around this.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-02-28 18:32:10 -08:00
lifubang 01f00e1fd5 ensure the path is a sub-cgroup path
Signed-off-by: lifubang <lifubang@acmcoder.com>
2022-02-19 09:45:09 +08:00
Kir Kolyshkin be00ae07c3 ci: shellcheck: update to 0.8.0, fix/suppress new warnings
1. This valid warning is reported by shellcheck v0.8.0:

	In tests/integration/helpers.bash line 38:
	KERNEL_MINOR="${KERNEL_VERSION#$KERNEL_MAJOR.}"
				       ^-----------^ SC2295 (info): Expansions inside ${..} need to be quoted separately, otherwise they match as patterns.

	Did you mean:
	KERNEL_MINOR="${KERNEL_VERSION#"$KERNEL_MAJOR".}"

Fix this.

2. These (invalid) warnings are also reported by the new version:

	In tests/integration/events.bats line 13:
	@test "events --stats" {
	^-- SC2030 (info): Modification of status is local (to subshell caused by @bats test).

	In tests/integration/events.bats line 41:
		[ "$status" -eq 0 ]
		   ^-----^ SC2031 (info): status was modified in a subshell. That change might be lost.

Basically, this is happening because shellcheck do not really track
the call tree and/or local variables. This is a known (and reported)
deficiency, and the alternative to disabling these warnings is moving
the code around, which is worse due to more changes in git history.

So we have to silence/disable these.

3. Update shellcheck to 0.8.0.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-02-10 17:24:04 -08:00
Kir Kolyshkin 6e1d476aad runc: remove --criu option
This was introduced in an initial commit, back in the day when criu was
a highly experimental thing. Today it's not; most users who need it have
it packaged by their distro vendor.

The usual way to run a binary is to look it up in directories listed in
$PATH. This is flexible enough and allows for multiple scenarios (custom
binaries, extra binaries, etc.). This is the way criu should be run.

Make --criu a hidden option (thus removing it from help). Remove the
option from man pages, integration tests, etc. Remove all traces of
CriuPath from data structures.

Add a warning that --criu is ignored and will be removed.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-01-26 20:25:56 -08:00
Kir Kolyshkin 5d77962099 tests/int: use update_config in hooks test
Using "$@" instead of $1 in update_config() allows us to use it from
hooks.bats, where jq is used with more options than usual.

We need to disable SC2016 as otherwise shellcheck sees $something inside
single quotes and think we are losing the shell expansion (we are not).

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-12-08 21:36:27 -08:00
Kir Kolyshkin 9e798e26cb tests/int: ability to specify binary
This can be used to specify a different runc binary, for example:

	sudo -E RUNC=$PWD/runc.mine tests/integration/cwd.bats

A different (but compatible enough) runtime also works:

	sudo -E RUNC=/usr/local/bin/crun tests/integration/cwd.bats

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-12-08 21:25:05 -08:00
Akihiro Suda 382eba4354 Support recursive mount attrs ("rro", "rnosuid", "rnodev", ...)
The new mount option "rro" makes the mount point recursively read-only,
by calling `mount_setattr(2)` with `MOUNT_ATTR_RDONLY` and `AT_RECURSIVE`.
https://man7.org/linux/man-pages/man2/mount_setattr.2.html

Requires kernel >= 5.12.

The "rro" option string conforms to the proposal in util-linux/util-linux Issue 1501.

Fix issue 2823

Similary, this commit also adds the following mount options:
- rrw
- r[no]{suid,dev,exec,relatime,atime,strictatime,diratime,symfollow}

Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
2021-12-07 17:39:57 +09:00
Fraser Tweedale 35d20c4e0b chown cgroup to process uid in container namespace
Delegating cgroups to the container enables more complex workloads,
including systemd-based workloads.  The OCI runtime-spec was
recently updated to explicitly admit such delegation, through
specification of cgroup ownership semantics:

  https://github.com/opencontainers/runtime-spec/pull/1123

Pursuant to the updated OCI runtime-spec, change the ownership of
the container's cgroup directory and particular files therein, when
using cgroups v2 and when the cgroupfs is to be mounted read/write.

As a result of this change, systemd workloads can run in isolated
user namespaces on OpenShift when the sandbox's cgroupfs is mounted
read/write.

It might be possible to implement this feature in other cgroup
managers, but that work is deferred.

Signed-off-by: Fraser Tweedale <ftweedal@redhat.com>
2021-11-30 08:52:59 +10:00
Aleksa Sarai 19d696ec29 merge branch 'pr-3276'
Kir Kolyshkin (2):
  runc run: fix ro /dev
  test/int/mount.bats: refer to github issue

LGTMs: thaJeztah cyphar
Closes #3276
2021-11-26 09:37:43 +11:00
Kir Kolyshkin 50105de1d8 Fix failure with rw bind mount of a ro fuse
As reported in [1], in a case where read-only fuse (sshfs) mount
is used as a volume without specifying ro flag, the kernel fails
to remount it (when adding various flags such as nosuid and nodev),
returning EPERM.

Here's the relevant strace line:

> [pid 333966] mount("/tmp/bats-run-PRVfWc/runc.RbNv8g/bundle/mnt", "/proc/self/fd/7", 0xc0001e9164, MS_NOSUID|MS_NODEV|MS_REMOUNT|MS_BIND|MS_REC, NULL) = -1 EPERM (Operation not permitted)

I was not able to reproduce it with other read-only mounts as the source
(tried tmpfs, read-only bind mount, and an ext2 mount), so somehow this
might be specific to fuse.

The fix is to check whether the source has RDONLY flag, and retry the
remount with this flag added.

A test case (which was kind of hard to write) is added, and it fails
without the fix. Note that rootless user need to be able to ssh to
rootless@localhost in order to sshfs to work -- amend setup scripts
to make it work, and skip the test if the setup is not working.

[1] https://github.com/containers/podman/issues/12205

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-11-18 13:09:41 -08:00
Kir Kolyshkin b247cd392a runc run: fix ro /dev
Commit fb4c27c4b7 (went into v1.0.0-rc93) fixed a bug with
read-only tmpfs, but introduced a bug with read-only /dev.

This happens because /dev is a tmpfs mount and is therefore remounted
read-only a bit earlier than before.

To fix,

1. Revert the part of the above commit which remounts all tmpfs mounts
   as read-only in mountToRootfs.

2. Reuse finalizeRootfs (which is already used to remount /dev
   read-only) to also remount all ro tmpfs mounts that were previously
   mounted rw in mountPropagate.

3. Remove the break in finalizeRootfs, as now we have more than one
   mount to care about.

4. Reorder the if statements in finalizeRootfs to perform the fast check
   (for ro flag) first, and compare the strings second. Since /dev is
   most probably also a tmpfs mount, do the m.Device check first.

Add a test case to validate the fix and prevent future regressions;
make sure it fails before the fix:

 ✗ runc run [ro /dev mount]
   (in test file tests/integration/mounts.bats, line 45)
     `[ "$status" -eq 0 ]' failed
   runc spec (status=0):

   runc run test_busybox (status=1):
   time="2021-11-12T12:19:48-08:00" level=error msg="runc run failed: unable to start container process: error during container init: error mounting \"devpts\" to rootfs at \"/dev/pts\": mkdir /tmp/bats-run-VJXQk7/runc.0Fj70w/bundle/rootfs/dev/pts: read-only file system"

Fixes: fb4c27c4b7
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-11-15 10:37:16 -08:00
Kir Kolyshkin f252eb5436 test/int/mount.bats: refer to github issue
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-11-12 12:10:46 -08:00
Kir Kolyshkin 68c2b6a7d9 runc run: refuse a frozen cgroup
Sometimes a container cgroup already exists but is frozen.
When this happens, runc init hangs, and it's not clear what is going on.

Refuse to run in a frozen cgroup; add a test case.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-11-03 20:36:27 -07:00
Kir Kolyshkin d08bc0c1b3 runc run: warn on non-empty cgroup
Currently runc allows multiple containers to share the same cgroup (for
example, by having the same cgroupPath in config.json). While such
shared configuration might be OK, there are some issues:

 - When each container has its own resource limits, the order of
   containers start determines whose limits will be effectively applied.

 - When one of containers is paused, all others are paused, too.

 - When a container is paused, any attempt to do runc create/run/exec
   end up with runc init stuck inside a frozen cgroup.

 - When a systemd cgroup manager is used, this becomes even worse -- such
   as, stop (or even failed start) of any container results in
   "stopTransientUnit" command being sent to systemd, and so (depending on
   unit properties) other containers can receive SIGTERM, be killed after a
   timeout etc.

Any of the above may lead to various hard-to-debug situations in production
(runc init stuck, cgroup removal error, wrong resource limits, init not
reaping zombies etc.).

One obvious solution is to refuse a non-empty cgroup when starting a new
container. This would be a breaking change though, so let's make it in
steps, with the first step is issue a warning and a deprecated notice
about a non-empty cgroup.

Later (in runc 1.2) we will replace this warning with an error.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-11-03 20:36:27 -07:00
Kir Kolyshkin dd696235a4 runc exec: reject paused container unless --ignore-paused
Currently, if a container is paused (i.e. its cgroup is frozen), runc exec
just hangs, and it is not obvious why.

Refuse to exec in a paused container. Add a test case.

In case runc exec in a paused container is a legit use case,
add --ignore-paused option to override the check. Document it,
add a test case.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-11-03 20:36:27 -07:00
Kir Kolyshkin 7324496f1a tests/int: fix userns for Fedora 35
Some test directories are created using mktemp -d, and so they have
permissions set to 0700 and are thus inaccessible to a user inside
userns. This was workarounded for $ROOT in userns.bats before.

Now, when we have updated Cirrus CI config to use Fedora 35 (rather than
34), userns tests fail:

> runc run failed: unable to start container process: error during
> container init: error preparing rootfs: mount
> /tmp/bats-run-4pCERd/runc.f66gCC/bundle/rootfs:/tmp/bats-run-4pCERd/runc.f66gCC/bundle/rootfs,
> flags: 0x5000: permission denied

Fedora 34 image used kernel v5.11, while Fedora 35 has v5.15.
Apparently, the newer kernel also checks that the parent directories
are accessible by the user before doing mount.

Move the old workaround from userns.bats to helpers.bats, drop the r bit
(not needed), and add $BATS_RUN_TMPDIR (also created by mktemp -d) to
fix userns.bats test failures under Fedora 35.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-11-03 19:07:37 -07:00
Kir Kolyshkin 05272718f4 tests/int/cgroups: fix for misc controller
The misc cgroup controller, introduced in Linux 5.13, is still unknown
to systemd, and thus it cannot delegate it. Add an appropriate fixup
to the test case, similar to an earlier commit 601cf5825f.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-11-03 18:54:16 -07:00
Rodrigo Campos 81fdc8ce1c New integration tests for user namespaces bind sources
The previous commit fixed an issue opening bind mount sources. This
commit just adds integration tests to make sure we don't regress on this
in the future.

Signed-off-by: Alban Crequy <alban@kinvolk.io>
Signed-off-by: Rodrigo Campos <rodrigo@kinvolk.io>
Co-authored-by: Rodrigo Campos <rodrigo@kinvolk.io>
2021-10-12 15:13:45 +02:00
Kir Kolyshkin 0202c398ff runc exec: implement --cgroup
In some setups, multiple cgroups are used inside a container,
and sometime there is a need to execute a process in a particular
sub-cgroup (in case of cgroup v1, for a particular controller).
This is what this commit implements.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-09-27 10:25:42 -07:00
Mauricio Vásquez cc15b887a0 tests: add integration test for cgroups hybrid
Check that runc run and runc exec put the process on the same cgroups v2
when using hybrid mode.

Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-09-27 10:25:12 -07:00
Kir Kolyshkin 6806b2c1c4 runc delete -f: fix for cg v1 + paused container
runc delete -f is not working for a paused container, since in cgroup v1
SIGKILL does nothing if a process is frozen (unlike cgroup v2, in which
you can kill a frozen process with a fatal signal).

Theoretically, we only need this for v1, but doing it for v2 as well is
OK.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-09-15 14:55:14 -07:00
Rodrigo Campos af641cd587 seccomp: Add test using the seccomp agent example
This commit adds the config.json as generated by the script. Note that
the diff is minimal if you see this commit with "git show -w". The
differences are mostly whitespaces and some ordering.

We add a simple test that runs this and expects sucess.

Signed-off-by: Rodrigo Campos <rodrigo@kinvolk.io>
2021-09-10 12:44:43 +02:00
Aleksa Sarai 8bf032602a merge branch 'pr-3047'
Liu Hua (1):
  checkpoint: resolve symlink for external bind mount(fix ci broken)

LGTMs: kolyshkin cyphar
2021-09-09 14:24:26 +10:00
Mauricio Vásquez 00772caec7 tests: add functional tests for seccomp notify
Add functional test to check seccomp notify end-to-end. This test uses the
sample seccomp agent from the contrib/cmd folder.

Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
Signed-off-by: Rodrigo Campos <rodrigo@kinvolk.io>
Co-authored-by: Rodrigo Campos <rodrigo@kinvolk.io>
2021-09-07 13:04:24 +02:00
Mauricio Vásquez 5ae831d9b3 tests: add functional tests for seccomp
Test KILL and ERRNO actions.

Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
2021-09-07 13:04:24 +02:00
Sebastiaan van Stijn b144f3d382 Merge pull request #2825 from lifubang/nodelete
proposal: add --keep to runc run
2021-08-31 10:04:58 +02:00
Kir Kolyshkin 639445789d tests/int: add a "update cpu period with pod limit set" test
Add a test case for an issue fixed by the previous commit.

Unfortunately, this is somewhat complicated as there's no easy way to
create a transient unit, so a binary, sd-helper, had to be added. On top
of that, an ability to create a parent/pod cgroup is added to
helpers.bash, which might be useful for future integration tests.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-08-26 13:11:22 -07:00
Kir Kolyshkin 1b2adcfe56 libct/cg/v1: workaround CPU quota period set failure
As reported in issue 3084, sometimes setting CPU quota period fails
when a new period is lower and a parent cgroup has CPU quota limit set.

This happens as in cgroup v1 the quota and the period can not be set
together (this is fixed in v2), and since the period is being set first,
new_limit = old_quota/new_period may be higher than the parent cgroup
limit.

The fix is to retry setting the period after the quota, to cover all
possible scenarios.

Add a test case to cover a regression caused by an earlier version of
this patch (ignoring a failure of setting invalid period when quota is
not set).

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-08-26 13:11:22 -07:00
Kir Kolyshkin fb629db693 tests/int/helpers: fix shellcheck warnings
... and add the file to be checked by shellcheck.

The warnings fixed are:

In tests/integration/helpers.bash line 10:
INTEGRATION_ROOT=$(dirname "$(readlink -f "$BASH_SOURCE")")
                                           ^----------^ SC2128: Expanding an array without an index only gives the first element.

In tests/integration/helpers.bash line 22:
TESTDATA="${INTEGRATION_ROOT}/testdata"
^------^ SC2034: TESTDATA appears unused. Verify use (or export if used externally).

In tests/integration/helpers.bash line 42:
	echo "runc $@ (status=$status):" >&2
                   ^-- SC2145: Argument mixes string and array. Use * or separate argument.
                              ^-----^ SC2154: status is referenced but not assigned.

In tests/integration/helpers.bash line 43:
	echo "$output" >&2
              ^-----^ SC2154: output is referenced but not assigned.

In tests/integration/helpers.bash line 77:
			| .linux.gidMappings += [{"hostID": '"$(($ROOTLESS_GIDMAP_START + 10))"', "containerID": 1, "size": 20}]
                                                                 ^--------------------^ SC2004: $/${} is unnecessary on arithmetic variables.

In tests/integration/helpers.bash line 78:
			| .linux.gidMappings += [{"hostID": '"$(($ROOTLESS_GIDMAP_START + 100))"', "containerID": 1000, "size": '"$(($ROOTLESS_GIDMAP_LENGTH - 1000))"'}]'
                                                                 ^--------------------^ SC2004: $/${} is unnecessary on arithmetic variables.
                                                                                                                                     ^---------------------^ SC2004: $/${} is unnecessary on arithmetic variables.

In tests/integration/helpers.bash line 125:
			base_path=$(gawk '$(NF-2) == "cgroup" && $NF ~ /\<'${g}'\>/ { print $5; exit }' /proc/self/mountinfo)
                                                                           ^--^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean:
			base_path=$(gawk '$(NF-2) == "cgroup" && $NF ~ /\<'"${g}"'\>/ { print $5; exit }' /proc/self/mountinfo)

In tests/integration/helpers.bash line 127:
			eval CGROUP_${g^^}_BASE_PATH="${base_path}"
                                    ^----^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean:
			eval CGROUP_"${g^^}"_BASE_PATH="${base_path}"

In tests/integration/helpers.bash line 229:
	if [ "x$CGROUP_UNIFIED" = "xyes" ]; then
             ^----------------^ SC2268: Avoid x-prefix in comparisons as it no longer serves a purpose.

Did you mean:
	if [ "$CGROUP_UNIFIED" = "yes" ]; then

In tests/integration/helpers.bash line 234:
		eval cgroup=\$${var}${REL_CGROUPS_PATH}
                              ^----^ SC2086: Double quote to prevent globbing and word splitting.
                                    ^-----------------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean:
		eval cgroup=\$"${var}""${REL_CGROUPS_PATH}"

In tests/integration/helpers.bash line 236:
	cat $cgroup/$source
            ^-----^ SC2086: Double quote to prevent globbing and word splitting.
                    ^-----^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean:
	cat "$cgroup"/"$source"

In tests/integration/helpers.bash line 242:
	current="$(get_cgroup_value $1)"
                                    ^-- SC2086: Double quote to prevent globbing and word splitting.

Did you mean:
	current="$(get_cgroup_value "$1")"

In tests/integration/helpers.bash line 245:
	echo "current" $current "!?" "$expected"
                       ^------^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean:
	echo "current" "$current" "!?" "$expected"

In tests/integration/helpers.bash line 257:
	[ $(id -u) != "0" ] && user="--user"
          ^------^ SC2046: Quote this to prevent word splitting.

In tests/integration/helpers.bash line 259:
	current=$(systemctl show $user --property $source $SD_UNIT_NAME | awk -F= '{print $2}')
                                                  ^-----^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean:
	current=$(systemctl show $user --property "$source" $SD_UNIT_NAME | awk -F= '{print $2}')

In tests/integration/helpers.bash line 261:
	[ "$current" = "$expected" ] || [ -n "$expected2" -a "$current" = "$expected2" ]
                                                          ^-- SC2166: Prefer [ p ] && [ q ] as [ p -a q ] is not well defined.

In tests/integration/helpers.bash line 309:
	check_cgroup_value "cpu.weight" $weight
                                        ^-----^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean:
	check_cgroup_value "cpu.weight" "$weight"

In tests/integration/helpers.bash line 310:
	check_systemd_value "CPUWeight" $weight
                                        ^-----^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean:
	check_systemd_value "CPUWeight" "$weight"

In tests/integration/helpers.bash line 383:
			if [ $CGROUP_UNIFIED = "no" -a ! -e "${CGROUP_MEMORY_BASE_PATH}/memory.memsw.limit_in_bytes" ]; then
                                                    ^-- SC2166: Prefer [ p ] && [ q ] as [ p -a q ] is not well defined.

In tests/integration/helpers.bash line 412:
			local cpu_count=$(grep -c '^processor' /proc/cpuinfo)
                              ^-------^ SC2155: Declare and assign separately to avoid masking return values.

In tests/integration/helpers.bash line 450:
		sleep $delay
                      ^----^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean:
		sleep "$delay"

In tests/integration/helpers.bash line 453:
	echo "Command \"$@\" failed $attempts times. Output: $output"
                        ^-- SC2145: Argument mixes string and array. Use * or separate argument.

In tests/integration/helpers.bash line 471:
	runc state $1
                   ^-- SC2086: Double quote to prevent globbing and word splitting.

Did you mean:
	runc state "$1"

In tests/integration/helpers.bash line 472:
	if [ $2 == "checkpointed" ]; then
             ^-- SC2086: Double quote to prevent globbing and word splitting.

Did you mean:
	if [ "$2" == "checkpointed" ]; then

In tests/integration/helpers.bash line 484:
	mkdir $dir
              ^--^ SC2086: Double quote to prevent globbing and word splitting.

Did you mean:
	mkdir "$dir"

In tests/integration/helpers.bash line 497:
		kill -9 $(cat "$dir/pid")
                        ^---------------^ SC2046: Quote this to prevent word splitting.

In tests/integration/helpers.bash line 508:
	export ROOT=$(mktemp -d "$BATS_RUN_TMPDIR/runc.XXXXXX")
               ^--^ SC2155: Declare and assign separately to avoid masking return values.

In tests/integration/helpers.bash line 512:
	cd "$ROOT/bundle"
        ^---------------^ SC2164: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.

Did you mean:
	cd "$ROOT/bundle" || exit

In tests/integration/helpers.bash line 535:
	cd "$INTEGRATION_ROOT"
        ^--------------------^ SC2164: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.

Did you mean:
	cd "$INTEGRATION_ROOT" || exit

For more information:
  https://www.shellcheck.net/wiki/SC2145 -- Argument mixes string and array. ...
  https://www.shellcheck.net/wiki/SC2034 -- TESTDATA appears unused. Verify u...
  https://www.shellcheck.net/wiki/SC2046 -- Quote this to prevent word splitt...

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-08-23 15:53:26 -07:00
Kir Kolyshkin f65276db35 tests/int/helpers: rm $bundle handling
It is not used since PR 2757, as all tests are run with cd to bundle
directory.

runc_spec argument count checking is removed since otherwise shellcheck
complains:

> SC2120: runc_spec references arguments, but none are ever passed.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-08-23 15:49:45 -07:00
Liu Hua 74ae9e0fc9 checkpoint: resolve symlink for external bind mount(fix ci broken)
runc resolves symlink before doing bind mount. So
we should save original path while formatting CriuReq for
dump and restore.

"checkpoint: resolve symlink for external bind mount" is merged as
da22625f6986f0ef196eaa1f8bb6adce098f0fb7(PR 2902) previously. And reverted
in commit 70fdc0573dced3464e9c31d674559f77c1de3973(PR 3043) duo to behavior changes
caused by commit 0ca91f44f1664da834bc61115a849b56d22f595f(Fixes: CVE-2021-30465)

Signed-off-by: Liu Hua <weldonliu@tencent.com>
2021-08-19 18:42:42 +08:00
Kir Kolyshkin 75761bccf7 Fix codespell warnings, add codespell to ci
The two exceptions I had to add to codespellrc are:
 - CLOS (used by intelrtd);
 - creat (syscall name used in tests/integration/testdata/seccomp_*.json).

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-08-17 16:12:35 -07:00
Mrunal Patel 8772c4dd2f Merge pull request #3109 from kolyshkin/seccomp
seccomp: skip redundant rules
2021-08-03 23:09:00 -04:00
lifubang cb824629ba proposal: add --keep to runc run
Signed-off-by: lifubang <lifubang@acmcoder.com>
2021-08-02 12:51:36 -07:00
Kir Kolyshkin 60e02b4b25 runc exec: fail with exit code of 255
Currently there's no way to distinguish between the two cases:
 - runc exec failed;
 - the command executed returned 1.

This was possible before commit 8477638aab, as runc exec exited with
the code of 255 if exec itself has failed. The code of 255 is the same
convention as used by e.g. ssh.

Re-introduce the feature, document it, and add some tests so it won't be
broken again.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-07-28 13:46:13 -07:00
Kir Kolyshkin 5dd92fd9b4 libct/seccomp: skip redundant rules
This fixes using runc with podman on my system (Fedora 34).

> $ podman --runtime `pwd`/runc run --rm --memory 4M fedora echo it works
> Error: unable to start container process: error adding seccomp filter rule for syscall bdflush: permission denied: OCI permission denied

The problem is, libseccomp returns EPERM when a redundant rule (i.e. the
rule with the same action as the default one) is added, and podman (on
my machine) sets the following rules in config.json:

    <....>
    "seccomp": {
      "defaultAction": "SCMP_ACT_ERRNO",
      "architectures": [
        "SCMP_ARCH_X86_64",
        "SCMP_ARCH_X86",
        "SCMP_ARCH_X32"
      ],
      "syscalls": [
        {
          "names": [
            "bdflush",
            "io_pgetevents",
            <....>
          ],
          "action": "SCMP_ACT_ERRNO",
          "errnoRet": 1
        },
        <....>

(Note that defaultErrnoRet is not set, but it defaults to 1).

With this commit, it works:

> $ podman --runtime `pwd`/runc run --memory 4M fedora echo it works
> it works

Add an integration test (that fails without the fix).

Similar crun commit:
 * https://github.com/containers/crun/commit/08229f3fb904c5ea19a7d9

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2021-07-27 00:04:59 -07:00