Commit Graph

6316 Commits

Author SHA1 Message Date
Kir Kolyshkin e42c219fea Merge pull request #3820 from kolyshkin/ubu-22.04
ci/gha: add ubuntu 22.04
2023-04-11 13:40:27 -07:00
Kir Kolyshkin 953e1cc485 ci/gha: switch to or add ubuntu 22.04
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>
2023-04-10 10:32:02 -07:00
Mrunal Patel d8a3daacbd Merge pull request #3815 from kolyshkin/bump-bats
bump bats-core, fix some tests, use new features
2023-04-08 08:55:21 -07:00
Kir Kolyshkin 1789002048 Merge pull request #3819 from opencontainers/dependabot/go_modules/golang.org/x/net-0.9.0
build(deps): bump golang.org/x/net from 0.8.0 to 0.9.0
2023-04-07 15:44:40 -07:00
dependabot[bot] 439673d510 build(deps): bump golang.org/x/net from 0.8.0 to 0.9.0
Bumps [golang.org/x/net](https://github.com/golang/net) from 0.8.0 to 0.9.0.
- [Release notes](https://github.com/golang/net/releases)
- [Commits](https://github.com/golang/net/compare/v0.8.0...v0.9.0)

---
updated-dependencies:
- dependency-name: golang.org/x/net
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
2023-04-07 04:59:18 +00:00
Mrunal Patel 941e5924ef Merge pull request #3814 from kolyshkin/go-1.19-minor
ci/cirrus: use Go 1.19.x not 1.19
2023-04-06 14:27:14 -07:00
Kir Kolyshkin fd1a79ffc8 ci/cirrus: improve host_info
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>
2023-04-06 10:52:05 -07:00
Kir Kolyshkin 873d7bb3a3 ci/cirrus: use Go 1.19.x not 1.19
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>
2023-04-06 10:51:43 -07:00
Kir Kolyshkin 611bbacb3b libct/cg: add misc controller to v1 drivers
This is just so that the container can join the misc controller.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2023-04-05 15:49:58 -07:00
Kir Kolyshkin 9b71787be0 tests/int: fix some checks
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>
2023-04-05 15:22:23 -07:00
Kir Kolyshkin 9dbb9f90b9 ci: bump bats 1.3.0 -> 1.8.2
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>
2023-04-05 15:22:23 -07:00
Sebastiaan van Stijn 5726682966 Merge pull request #3813 from opencontainers/dependabot/go_modules/golang.org/x/sys-0.7.0
build(deps): bump golang.org/x/sys from 0.6.0 to 0.7.0
2023-04-05 21:01:27 +02:00
dependabot[bot] a6e95c53f4 build(deps): bump golang.org/x/sys from 0.6.0 to 0.7.0
Bumps [golang.org/x/sys](https://github.com/golang/sys) from 0.6.0 to 0.7.0.
- [Release notes](https://github.com/golang/sys/releases)
- [Commits](https://github.com/golang/sys/compare/v0.6.0...v0.7.0)

---
updated-dependencies:
- dependency-name: golang.org/x/sys
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
2023-04-05 04:59:43 +00:00
Akihiro Suda 67b542bb4e Merge pull request #3810 from kolyshkin/rm-get-init-cgroup-path
libct/cg: rm GetInitCgroup[Path]
2023-04-05 10:59:37 +09:00
Akihiro Suda 1b4cf1d2ae Merge pull request #3809 from opencontainers/dependabot/github_actions/lumaxis/shellcheck-problem-matchers-2
build(deps): bump lumaxis/shellcheck-problem-matchers from 1 to 2
2023-04-05 03:26:36 +09:00
Akihiro Suda 0cab3b3dda Merge pull request #3779 from fish98/main
tests: Fix fuzzer location in oss-fuzz config
2023-04-05 03:26:04 +09:00
Kir Kolyshkin fd5debf3aa libct/cg: rm GetInitCgroup[Path]
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>
2023-04-04 11:19:25 -07:00
dependabot[bot] 1034cfa826 build(deps): bump lumaxis/shellcheck-problem-matchers from 1 to 2
Bumps [lumaxis/shellcheck-problem-matchers](https://github.com/lumaxis/shellcheck-problem-matchers) from 1 to 2.
- [Release notes](https://github.com/lumaxis/shellcheck-problem-matchers/releases)
- [Commits](https://github.com/lumaxis/shellcheck-problem-matchers/compare/v1...v2)

---
updated-dependencies:
- dependency-name: lumaxis/shellcheck-problem-matchers
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
2023-04-04 17:44:14 +00:00
Kir Kolyshkin cc60a390ad Merge pull request #3784 from haircommander/root-cgroup-no-init
libctr/cgroups: don't take init's cgroup into account
2023-04-04 09:34:26 -07:00
Mrunal Patel 7ba53a1526 Merge pull request #3788 from kolyshkin/systemd-cpu-idle
cg/sd: support CPUWeight=idle (aka cpu.idle=1)
2023-04-04 08:07:34 -07:00
Kir Kolyshkin ed9651bc71 libct/cg/sd: support setting cpu.idle via systemd
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>
2023-04-03 18:25:07 -07:00
Kir Kolyshkin b5ecad7ba3 tests/int/update: test bad cpu.idle values
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>
2023-04-03 18:25:07 -07:00
Kir Kolyshkin 3ffbd4c85a tests/int: fix update cpu.idle failure on CS9
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>
2023-04-03 18:25:07 -07:00
Kir Kolyshkin 509b312cfb libct/cg/sd/v2: unifiedResToSystemdProps nit
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>
2023-04-03 18:24:47 -07:00
Kir Kolyshkin 9f2451346e Merge pull request #3782 from kolyshkin/fix-sd-start
Fix systemd cgroup driver's Apply
2023-04-03 11:27:17 -07:00
Akihiro Suda 17922e3612 Merge pull request #3800 from kolyshkin/fp-ch
CHANGELOG: forward-port 1.1.4 and 1.1.5 changes
2023-04-03 18:12:10 +09:00
Kir Kolyshkin ba618705f8 Merge pull request #3797 from kolyshkin/enter-pid
libct/cg: rm EnterPid
2023-03-31 13:01:39 -07:00
Kir Kolyshkin 82bc89cd10 runc run: refuse a non-empty cgroup
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>
2023-03-30 19:55:39 -07:00
Kir Kolyshkin 1d18743f9e libct/cg/sd: reset-failed and retry startUnit on UnitExists
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>
2023-03-30 19:55:39 -07:00
Kir Kolyshkin c253342061 libct/cg/sd: ignore UnitExists only for Apply(-1)
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>
2023-03-30 19:55:39 -07:00
Kir Kolyshkin c6e8cb7926 libct/cg/sd: refactor startUnit
Move error handling earlier, removing "if err == nil" block.

No change of logic.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2023-03-30 19:55:39 -07:00
Kir Kolyshkin 9f32ce6a2d CHANGELOG: forward-port 1.1.4 and 1.1.5 changes
...from the tip of release-1.1 branch (commit 060a61c69d).

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2023-03-30 19:46:53 -07:00
Kir Kolyshkin 73acc77be5 libct/cg: rm EnterPid
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>
2023-03-31 13:33:00 +11:00
Kir Kolyshkin 4ff4904603 Makefile: add verify-changelog as release dependency
... 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>
2023-03-30 19:11:31 -07:00
Kir Kolyshkin b2fc0a589c verify-changelog: allow non-ASCII
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>
2023-03-30 19:11:11 -07:00
Kir Kolyshkin 8edf478e34 Merge pull request #3798 from kolyshkin/fix-mount-test
Fix mount test
2023-03-30 18:32:27 -07:00
Kir Kolyshkin 370e3be202 tests/int/mounts: only check non-shadowed mounts
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>
2023-03-30 14:45:14 -07:00
Kir Kolyshkin a37109ce02 tests/int/mount: fix issues with ro cgroup test
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>
2023-03-30 14:44:03 -07:00
Qiang Huang 0d62b950e6 Merge pull request from GHSA-m8cg-xc2p-r3fc
rootless: fix /sys/fs/cgroup mounts
2023-03-29 14:18:15 +08:00
Akihiro Suda 7f3f4bee8a Merge pull request #3753 from kolyshkin/user-exec
Fix runc run "permission denied" when rootless
2023-03-29 00:39:12 +09:00
Akihiro Suda 2b221a6ab7 Merge pull request #3787 from kolyshkin/rec-fixup
mountToRootfs: minor refactor
2023-03-28 12:53:38 +09:00
Kir Kolyshkin 8293ef2e74 tests/int: test for CAP_DAC_OVERRIDE
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>
2023-03-27 15:16:15 -07:00
Kir Kolyshkin 8491d33482 Fix runc run "permission denied" when rootless
Since commit 957d97bcf4 was made to fix issue [7],
a few things happened:

- a similar functionality appeared in go 1.20 [1], so the issue
  mentioned in the comment (being removed) is no longer true;
- a bug in runc was found [2], which also affects go [3];
- the bug was fixed in go 1.21 [4] and 1.20.2 [5];
- a similar fix was made to x/sys/unix.Faccessat [6].

The essense of [2] is, 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.

To fix this [2] without reintroducing the older bug [7]:
- drop own Eaccess implementation;
- use the one from x/sys/unix for Go 1.19 (depends on [6]);
- do not use anything when Go 1.20+ is used.

NOTE it is virtually impossible to fix the bug [2] when Go 1.20 or Go
1.20.1 is used because of [3].

A test case is added by a separate commit.

Fixes: #3715.

[1] https://go-review.googlesource.com/c/go/+/414824
[2] https://github.com/opencontainers/runc/issues/3715
[3] https://go.dev/issue/58552
[4] https://go-review.googlesource.com/c/go/+/468735
[5] https://go-review.googlesource.com/c/go/+/469956
[6] https://go-review.googlesource.com/c/sys/+/468877
[7] https://github.com/opencontainers/runc/issues/3520

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2023-03-27 15:15:48 -07:00
Kir Kolyshkin 99a337f66d Dockefile: bump go go 1.20
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>
2023-03-27 15:13:43 -07:00
Kir Kolyshkin 3e3db2883b Merge pull request #3778 from kolyshkin/skip-flaky-ce7
libct/cg/dev: skip flaky test of CentOS 7
2023-03-27 13:39:36 -07:00
Kir Kolyshkin da98076c97 mountToRootfs: minor refactor
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>
2023-03-27 12:47:05 -07:00
Peter Hunt~ 54e20217a8 libctr/cgroups: don't take init's cgroup into account
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>
2023-03-27 13:16:46 -04:00
Akihiro Suda da5047c5d8 Merge pull request #3781 from yanggangtony/fix-typo
fix wrong notes for `const MaxNameLen`
2023-03-26 07:02:40 +09:00
Akihiro Suda 948ef27c7a Merge pull request #3773 from kolyshkin/no-symlinks
Prohibit /proc and /sys to be symlinks
2023-03-25 22:58:27 +09:00
Kir Kolyshkin a7a836effa libct/cg/dev: skip flaky test of CentOS 7
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>
2023-03-22 20:01:39 -07:00