For a previous attempt to fix that (and added test cases), see commit
9087f2e827.
Alas, it's not always working because of cgroup directory TOCTOU.
To solve this and avoid the race, add an error _after_ the operation.
Implement it as a method that ignores the error that should be ignored.
Instead of currentStatus(), use faster runType(), since we are not
interested in Paused status here.
For Processes(), remove the pre-op check, and only use it after getting
an error, making the non-error path more straightforward.
For Signal(), add a second check after getting an error. The first check
is left as is because signalAllProcesses might print a warning if the
cgroup does not exist, and we'd like to avoid that.
This should fix an occasional failure like this one:
not ok 84 kill detached busybox
# (in test file tests/integration/kill.bats, line 27)
# `[ "$status" -eq 0 ]' failed
....
# runc kill test_busybox KILL (status=0):
# runc kill -a test_busybox 0 (status=1):
# time="2023-04-04T18:24:27Z" level=error msg="lstat /sys/fs/cgroup/devices/system.slice/runc-test_busybox.scope: no such file or directory"
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
These checks ensure that all of the keys in the runc.keyring list are
actually the keys of the specified user and that the users themselves
are actually maintainers.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
We need to make sure the release is being signed by a key that is
actually listed as a trusted signing key, and we also need to ask the
person cutting the release whether the list of trusted keys is
acceptable.
Also add some verification checks after a release is signed to make sure
everything was signed with the correct keys.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
In order to allow any of the maintainers to cut releases for runc,
create a keyring file that distributions can use to verify that releases
are signed by one of the maintainers.
The format matches the gpg-offline format used by openSUSE packaging,
but it can be easily imported with "gpg --import" so any distribution
should be able to handle this keyring format wtihout issues.
Each key includes the GitHub handle of the associated user. There isn't
any way for this information to be automatically verified (outside of
using something like keybase.io) but since all changes of this file need
to be approved by maintainers this is okay for now.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
For test jobs, add ubuntu 22.04 into the matrix, so we can test of both
cgroup v1 and v2.
For validate jobs, just switch to ubuntu 22.04
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
1. Do not use echo, as this results in lines like this:
...
echo "-----"
-----
...
2. Move "cat /proc/cpuinfo" to be the last one, as the output is usually
very long.
3. Add "go version" to CentOS jobs.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This variable is used in curl to download a go release, so we are using
the initial Go 1.19 release in Cirrus CI, not the latest Go 1.19.x
release.
From the CI perspective, it makes more sense to use the latest release.
Add some jq magic to extract the latest minor release information
from the download page, and use it.
This brings Cirrus CI jobs logic in line with all the others (GHA,
Dockerfile), where by 1.20 we actually mean "latest 1.20.x".
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
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>