Apparently, bash with set -e deliberately ignores non-zero return codes
from ! cmd, unless this is the last command. The workaround is to either
use "! cmd || false', "or run ! cmd". Choose the latter, and require
bash-core 1.5.0 (since this is when "run !" was added), replacing the
older check.
Alas I only learned this recently from the bash-core documentation.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This version is already used by Cirrus CI Fedora 37 job, but other CI
jobs are still using 1.3.0.
Bump it everywhere so we can enjoy new version features and fixes.
For one thing, I noticed that new bats is reporting error location
correctly.
We will also be able to use "run !" and "run -N" commands.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
These functions were added in ancient times, facilitating the
docker-in-docker case when cgroup namespace was not available.
As pointed out in commit 2b28b3c276, using init 1 cgroup is not
correct because it won't work in case of host PID namespace.
The last user of GetInitCgroup was removed by commit
54e20217a8. GetInitCgroupPath was never used
as far as I can see, nor was I able to find any external users.
Remove both functions. Modify the comment in libct/cg/fs.subsysPath
to not refer to GetInitCgroupPath.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Systemd v252 (available in CentOS Stream 9 in our CI) added support
for setting cpu.idle (see [1]). The way it works is:
- if CPUWeight == 0, cpu.idle is set to 1;
- if CPUWeight != 0, cpu.idle is set to 0.
This commit implements setting cpu.idle in systemd cgroup driver via a
unit property. In case CPUIdle is set to non-zero value, the driver sets
adds CPUWeight=0 property, which will result in systemd setting cpu.idle
to 1.
Unfortunately, there's no way to set cpu.idle to 0 without also changing
the CPUWeight value, so the driver doesn't do anything if CPUIdle is
explicitly set to 0. This case is handled by the fs driver which is
always used as a followup to setting systemd unit properties.
Also, handle cpu.idle set via unified map. In case it is set to non-zero
value, add CPUWeight=0 property, and ignore cpu.weight (otherwise we'll
get two different CPUWeight properties set).
Add a unit test for new values in unified map, and an integration test case.
[1] https://github.com/systemd/systemd/pull/23299
[2] https://github.com/opencontainers/runc/issues/3786
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Values other than 1 or 0 are ignored by the kernel,
see sched_group_set_idle() in kernel/sched/fair.c
If the added test case ever fails, it means that the kernel now accepts
values other than 0 or 1, and runc needs to adopt to that.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Systemd v252 (available in CentOS Stream 9 in our CI) added support
for setting cpu.idle (see [1]). The way it works is:
- if CPUWeight == 0, cpu.idle is set to 1;
- if CPUWeight != 0, cpu.idle is set to 0.
This behavior breaks the existing test case, as described in [2].
To fix, skip the last check in the test case in case a newer systemd
is used.
Fixes: #3786
[1] https://github.com/systemd/systemd/pull/23299
[2] https://github.com/opencontainers/runc/issues/3786
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In code that checks that the resource name is in the for
Using strings.SplitN is an overkill in this case, resulting in
allocations and thus garbage to collect.
Using strings.IndexByte and checking that result is not less than 1
(meaning there is a period, and it is not the first character) is
sufficient here.
Fixes: 0cb8bf67a3
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Commit d08bc0c1b3 ("runc run: warn on non-empty cgroup") introduced
a warning when a container is started in a non-empty cgroup. Such
configuration has lots of issues.
In addition to that, such configuration is not possible at all when
using the systemd cgroup driver.
As planned, let's promote this warning to an error, and fix the test
case accordingly.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In case a systemd unit fails (for example, timed out or OOM-killed),
systemd keeps the unit. This prevents starting a new container with
the same systemd unit name.
The fix is to call reset-failed in case UnitExists error is returned,
and retry once.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Commit d223e2adae ("Ignore error when starting transient unit
that already exists" modified the code handling errors from startUnit
to ignore UnitExists error.
Apparently it was done so that kubelet can create the same pod slice
over and over without hitting an error (see [1]).
While it works for a pod slice to ensure it exists, it is a gross bug
to ignore UnitExists when creating a container. In this case, the
container init PID won't be added to the systemd unit (and to the
required cgroup), and as a result the container will successfully
run in a current user cgroup, without any cgroup limits applied.
So, fix the code to only ignore UnitExists if we're not adding a process
to the systemd unit. This way, kubelet will keep working as is, but
runc will refuse to create containers which are not placed into a
requested cgroup.
[1] https://github.com/opencontainers/runc/pull/1124
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Since commit 39914db679 this function is not used by runc (see
that commit to learn why this function is not that good).
I was not able to find any external users either.
Since it's not a good function, with no users, and it is rather trivial,
let's remove it right away (rather than mark as deprecated).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
... as a way to maybe catch some CHANGELOG.md bugs at the last moment.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 54cfb25d69)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Previously (see commit 91fa032da4) we found a few issues
using this check, but apparently the CHANGELOG.md is in UTF-8, and
the recently added quote is breaking this, so remove.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 7b3ac330f7)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This fixes a bogus failure in "ro cgroup" test cases when running as
rootless.
The test finds the following mount that is not read-only:
> cgroup2 /sys/fs/cgroup/unified cgroup2 rw,nosuid,nodev,noexec,relatime,nsdelegate 0 0
This happens because:
1. runc spec --rootless adds an rbind /sys mounts, so we have all the
/sys/fs/cgroup/XXX mounts inside the container;
2. Those /sys/fs/cgroup/XXX mounts are shadowed by the /sys/fs/cgroup
tmpfs mount created by mountCgroupV1().
This means that this mount is shadowed, inaccessible, and it can not be
unshadowed, thus it should not be checked.
The fix is to check whether the directory exists, to exclude such
shadowed mounts.
NOTE that item 2 comes from commit ff692f289b60e19b3079cb; before it, we
had the whole hierarchy of host /sys/fs/cgroup visible (though not
writable -- because rootless) from inside of any rootless container.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Fix the following issues with the "ro /sys/fs/cgroup" test:
1. Disable bogus SC2016 warning from shellcheck.
2. Split the test into two -- with and without cgroupns. This is done
because not all systems have cgroupns available (so the "+cgroupns"
test will be skipped).
3. This splitting resulted in a few more bogus shellcheck warnings that
we have to suppress -- due to a known bug in shellcheck (see [1]).
4. s/mount/mounts/ in the test name, because in case of cgroup v1 there
are multiple mounts.
[1] https://github.com/koalaman/shellcheck/issues/2431
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This is a test case for issue reported as #3715. In short, even if a
(non-root) user that the container is run as does not have execute
permission bit set for the executable, it should still work in case runc
has the CAP_DAC_OVERRIDE capability set.
Note that since the upstream golang is also broken (see [1]), this test
will fail for Go 1.20 and 1.20.1 (fix is in Go 1.20.2 as per [2]).
[1] https://go.dev/issue/58552
[2] https://go-review.googlesource.com/c/go/+/469956
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Go 1.20.2 has an important fix to an issue described in [1].
Switch from using Go 1.19 from Dockerfile, which is used for release
binaries and some CI.
[1] https://github.com/golang/go/issues/58624
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The setRecAttr is only called for "bind" case, as cases end with a
return statement. Indeed, recursive mount attributes only make sense for
bind mounts.
Move the code to under case "bind" to improve readability. No change in
logic.
Fixes: 382eba4354
Reported-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Sometimes, the init process is not in the root cgroup.
This can be noted by GetInitPath, which already scrubs the path of `init.scope`.
This was encountered when trying to patch the Kubelet to handle systemd being in a separate cpuset
from root (to allow load balance disabling for containers). At present, there's no way to have libcontainer or runc
manage cgroups in a hierarchy outside of the one init is in (unless the path contains `init.scope`, which is limiting)
Signed-off-by: Peter Hunt <pehunt@redhat.com>
There is some kind of a race in CentOS 7 which sometimes result in one
of these tests failing like this:
systemd_test.go:136: mkdir /sys/fs/cgroup/hugetlb/system.slice/system-runc_test_pods.slice: no such file or directory
or
systemd_test.go:187: open /sys/fs/cgroup/cpuset/system.slice/system-runc_test_pods.slice/cpuset.mems: no such file or directory
As this is only happening on CentOS 7, let's skip this test on this
platform.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Commit 3291d66b98 introduced a check for /proc and /sys, making sure
the destination (dest) is a directory (and not e.g. a symlink).
Later, a hunk from commit 0ca91f44f switched from using filepath.Join
to SecureJoin for dest. As SecureJoin follows and resolves symlinks,
the check whether dest is a symlink no longer works.
To fix, do the check without/before using SecureJoin.
Add integration tests to make sure we won't regress.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Older kernels may return EINVAL on unshare when a process is reading
runc's /proc/$PID/status or /proc/$PID/maps. This was fixed by kernel
commit 12c641ab8270f ("unshare: Unsharing a thread does not require
unsharing a vm") in Linuxt v4.3.
For CentOS 7, the fix was backported to CentOS 7.7 (kernel 3.10.0-1062).
To work around this kernel bug, let's retry on EINVAL a few times.
Reported-by: zzyyzte <zhang.yu58@zte.com.cn>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Kir Kolyshkin