This was added by commit 5aa82c950 back in the day when we thought
runc is going to be cross-platform. It's very clear now it's Linux-only
package.
While at it, further clarify it in README that we're Linux only.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
For files that end with _linux.go or _linux_test.go, there is no need to
specify linux build tag, as it is assumed from the file name.
In addition, rename libcontainer/notify_linux_v2.go -> libcontainer/notify_v2_linux.go
for the file name to make sense.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
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>
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>
This reverts commit 814f3ae1d9. This
changed the on-disk state which breaks runc when it has to operate on
containers started with an older runc version. Working around this is
far more complicated than just reverting it.
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Looking into data generated by setting
GODEBUG="inittrace=1"
I have noticed this line:
init github.com/opencontainers/runc/libcontainer/cgroups/devices @1.2 ms, 0.020 ms clock, 10512 bytes, 133 allocs
This is the leader for both bytes and allocs among the packages from
this repo, and all of it is caused by a single regex:
> var devicesListRegexp = regexp.MustCompile(`^([abc])\s+(\d+|\*):(\d+|\*)\s+([rwm]+)$`)
It seems that the same parsing can be done without relying on
a regular expression, no decrease in readability, and 2x faster
(according to the benchmark added), and also makes runc start
slightly faster and leaner.
Before:
BenchmarkParseLine-4 176240 6768 ns/op 6576 B/op 64 allocs/op
After:
BenchmarkParseLine-4 322441 3535 ns/op 5520 B/op 53 allocs/op
[v2: single split with SplitFunc; fix a typo in error message]
[v3: rebase after 3159 merge; re-ran benchmarks (results are similar)]
Co-authored-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
... 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>
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>
Use the term "clos group" instead of "container_id group" as the group
that a container belongs to is not necessarily tied to its container id.
Signed-off-by: Markus Lehtonen <markus.lehtonen@intel.com>
This is helpful to kubernetes in cases it knows for sure that the freeze
is not required (since it created the systemd unit with no device
restrictions).
As the code is trivial, no tests are required.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Add a test for freezeBeforeSet, checking various scenarios including
those that were failing before the fix in the previous commit.
[v2: add more cases, add a check before creating a unit.]
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This fixes the behavior intended to avoid freezing containers/control
groups without it being necessary. This is important for end users of
libcontainer who rely on the behavior of no freeze.
The previous implementation would always get error trying to get
DevicePolicy from the Unit via dbus, since the Unit interface doesn't
contain DevicePolicy.
Signed-off-by: Odin Ugedal <odin@uged.al>
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>
Commit 2bab4a5 resulted in a warning from gcc:
nsexec.c: In function ‘write_log’:
nsexec.c:171:2: warning: ignoring return value of ‘write’, declared with attribute warn_unused_result [-Wunused-result]
171 | write(logfd, json, ret);
| ^~~~~~~~~~~~~~~~~~~~~~~
As there's nothing we can or want to do in case write fails,
let's just tell the compiler we're not going to use it.
Fixes: 2bab4a5
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Today we support the seccomp build tag only that is used by default.
However, we are not testing that compiling without any build tag works.
I found the CI didn't catch this when working on #2682, that the CI was
green but compilation without build tags was broken.
We test compilation without build tags only, compilation with the only
build tag supported is done extensively in other actions.
Signed-off-by: Rodrigo Campos <rodrigo@kinvolk.io>
The contents of the pointer returned on asprintf() error are undefined
i.e., it can be anything there. We set it to NULL on error so that
free() afterwards won't get a garbage pointer.
This patch applies the above to message and stage as well to be
consistent with what we do for json.
Signed-off-by: Kailun Qin <kailun.qin@intel.com>
According to C standards, `size_t` is always an unsigned integer type.
Thus, checking unsigned expressions to be less than zero is not needed.
Signed-off-by: Kailun Qin <kailun.qin@intel.com>
Possibly there was a specific reason to use a rune for this, but I noticed
that there's various parts in the code that has to convert values from a
string to this type. Using a string as type for this can simplify some of
that code.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
GCP images description at [1] claims that:
- For CentOS 8 and CentOS Stream 8, the PowerTools repository is
enabled.
- For CentOS 7, EPEL is enabled.
Apparently,
- we do not need epel for centos-stream-8;
- powertools is not enabled on centos-stream-8 despite [1].
Anyway, the less yum commands the better, as we have seen those fail
sometimes due to occasional networking problems etc.
[1] https://cloud.google.com/compute/docs/images/os-details#centos
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
As reported in issue 3119, there is a race in nsexec logging
that can lead to garbled json received by log forwarder, which
complains about it with a "failed to decode" error.
This happens because dprintf (used since the very beginning of nsexec
logging introduced in commit ba3cabf932) relies on multiple write(2)
calls, and with additional logging added by 64bb59f592 a race is
possible between runc init parent and its children.
The fix is to prepare a string and write it using a single call to
write(2).
[v2: NULLify json on error from asprintf]
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Kir Kolyshkin (3):
libct/cg: GetAllPids: optimize for go 1.16+
libct/cg: improve GetAllPids and readProcsFile
libct/cg: move GetAllPids out of utils.go
LGTMs: AkihiroSuda cyphar
Closes#3133
Kir Kolyshkin