Commit Graph

4642 Commits

Author SHA1 Message Date
Sebastiaan van Stijn 3eb469b029 libcontainer: remove redundant strings.Join()
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2020-10-01 00:00:47 +02:00
Mrunal Patel ffc30bb00d Merge pull request #2606 from kolyshkin/intel-rdt
libcontainer/intelrdt: use moby/sys/mountinfo
2020-09-30 13:14:10 -07:00
Mrunal Patel d5098eaae4 Merge pull request #2604 from kolyshkin/fscommon-I
libct/cgroups/fscommon: nits
2020-09-30 13:12:16 -07:00
Akihiro Suda 890cc2aa60 Merge pull request #2612 from thaJeztah/concat
use string-concatenation instead of sprintf for simple cases
2020-10-01 01:56:28 +09:00
Akihiro Suda d4885caaaa Merge pull request #2578 from kolyshkin/fix-destdir
Makefile: fix DESTDIR handling
2020-10-01 01:43:21 +09:00
Kir Kolyshkin 54a8b24c04 Merge pull request #2610 from thaJeztah/hostport_splitting
checkpoint: setPageServer: use net.SplitHostPort instead of strings.Split
2020-09-30 09:33:04 -07:00
Kir Kolyshkin 22f11bafc3 Merge pull request #2611 from thaJeztah/const
libcontainer/cgroups: use const for templates
2020-09-30 09:01:42 -07:00
Kir Kolyshkin 8ee3e560e3 Merge pull request #2616 from thaJeztah/update_logrus
go.mod: update containerd/console v1.0.1, sirupsen/logrus v1.7.0
2020-09-30 09:00:44 -07:00
Mrunal Patel fdd00a693e Merge pull request #2614 from kolyshkin/systemd-race
libct/cgroups/systemd: eliminate runc/systemd race
2020-09-30 08:20:11 -07:00
Mrunal Patel bbf33043af Merge pull request #2615 from kolyshkin/cgroups-ebusy
runc run: fix removing cgroups on error; improve logging on error
2020-09-30 08:18:20 -07:00
Sebastiaan van Stijn 819fd68392 go.mod: sirupsen/logrus v1.7.0
full diff: https://github.com/sirupsen/logrus/compare/v1.6.0...v1.7.0

removes dependency on github.com/konsorten/go-windows-terminal-sequences

Features:
   * a new buffer pool management API has been added
   * a set of `<LogLevel>Fn()` functions have been added

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2020-09-30 14:00:16 +02:00
Sebastiaan van Stijn 0eb66c955c go.mod: github.com/containerd/console v1.0.1
full diff: https://github.com/containerd/console/compare/v1.0.0...v1.0.1

Fixes compatibility with current versions of golang.org/x/sys

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2020-09-30 13:58:27 +02:00
Sebastiaan van Stijn 8bf216728c use string-concatenation instead of sprintf for simple cases
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2020-09-30 10:51:59 +02:00
Kir Kolyshkin ecfad5a118 Merge pull request #2574 from EduardoVega/2249-add-integration-tests-resource-devices
Add integration tests for cgroup devices
2020-09-30 00:19:27 -07:00
Kir Kolyshkin a4d5e8a27b libcontainer/ignoreTerminateError: ignore SIGKILL
When we call terminate(), we kill the process, and wait
returns the error indicating the process was killed.
This is exactly what we expect here, so there is no reason
to treat it as an error.

Before this patch, when a container with invalid cgroup parameters is
started:

> WARN[0000] unable to terminate initProcess               error="signal: killed"
> ERRO[0000] container_linux.go:366: starting container process caused: process_linux.go:495: container init caused: process_linux.go:458: setting cgroup config for procHooks process caused: failed to write "555": open /sys/fs/cgroup/blkio/user.slice/xx33/blkio.weight: permission denied

After:

> ERRO[0000] container_linux.go:366: starting container process caused: process_linux.go:495: container init caused: process_linux.go:458: setting cgroup config for procHooks process caused: failed to write "555": open /sys/fs/cgroup/blkio/user.slice/xx33/blkio.weight: permission denied

I.e. the useless warning is gone.

NOTE this breaks a couple of integration test cases, since they were
expecting a particular message in the second line, and now due to
"signal: killed" removed it's in the first line. Fix those, too.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2020-09-29 23:55:02 -07:00
Kir Kolyshkin dc42459130 libct/(*initProcess).start: fix removing cgroups on error
In case cgroup configuration is invalid (some parameters can't be set
etc.), p.manager.Set fails, the error is returned, and then we try to
remove cgroups (by calling p.manager.Destroy) in a defer.

The problem is, the container init is not yet killed (as it is killed in
the caller, i.e. (*linuxContainer).start), so cgroup removal fails like
this:

> time="2020-09-26T07:46:25Z" level=warning msg="Failed to remove cgroup (will retry)" error="rmdir /sys/fs/cgroup/net_cls,net_prio/kubepods.slice/kubepods-burstable.slice/kubepods-burstable-pod28ce6e74_694c_4b77_a953_dc01e182ac76.slice/crio-f6984c5eeb6c6b49ff3f036bdcb9ded317b3d0b2469ebbb35705442a2afd98c2.scope: device or resource busy"
> ...
> time="2020-09-26T07:46:27Z" level=error msg="Failed to remove cgroup" error="rmdir /sys/fs/cgroup/net_cls,net_prio/kubepods.slice/kubepods-burstable.slice/kubepods-burstable-pod28ce6e74_694c_4b77_a953_dc01e182ac76.slice/crio-f6984c5eeb6c6b49ff3f036bdcb9ded317b3d0b2469ebbb35705442a2afd98c2.scope: device or resource busy"

The above is repeated for every controller, and looks quite scary.

To fix, move the init termination to the abovementioned defer.

Do the same for (*setnsProcess).start() for uniformity.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2020-09-29 18:49:29 -07:00
Kir Kolyshkin 8699596dce libct/(*setnsProcess).Start: use retErr
It is not a good practice to have the name `err` for the error returned
by a function.  Switch to `retErr`.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2020-09-29 18:38:31 -07:00
Kir Kolyshkin 38447895a5 libct/cgroups/systemd: eliminate runc/systemd race
In case it takes more than 1 second for systemd to create a unit,
startUnit() times out with a warning and then runc proceeds
(to create cgroups using fs manager and so on).

Now runc and systemd are racing, and multiple scenarios are possible.

In one such scenario, by the time runc calls systemd manager's Apply()
the unit is not yet created, the dbusConnection.SetUnitProperties()
call fails with "unit xxx.scope not found", and the whole container
start also fails.

To eliminate the race, we need to return an error in case the timeout is
hit.

To reduce the chance to fail, increase the timeout from 1 to 30 seconds,
to not error out too early on a busy/slow system (and times like 3-5
seconds are not unrealistic).

While at it, as the timeout is quite long now, make sure to not leave
a stray timer.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2020-09-29 17:43:15 -07:00
Kir Kolyshkin 6c83d23ffc libcontainer/cgroups/fscommon: improve doc
Document ReadFile and WriteFile. Fix doc for ParseUint to be in
canonical form.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2020-09-29 11:58:14 -07:00
Kir Kolyshkin 31f0f5b7e0 libct/cg/fscommon.GetCgroupParamUint: improve
1. Use GetCgroupParamString as the initial part of both functions are
   the same and we can reuse it. This also gives us whatever security
   measures GetCgroupParamString has (see previous commit).

2. Fix the error wrapping to not add the value, as it is already a part
   of the error returned by ParseUint.

3. Improve docstring.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2020-09-29 11:58:14 -07:00
Kir Kolyshkin e76ac1c054 libct/cg/fscommon.GetCgroupParamString: use ReadFile
1. Use own ReadFile wrapper instead of ioutils.ReadFile.
   This makes it use the security measures of ReadFile.

2. Improve doc.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2020-09-29 11:58:14 -07:00
Kir Kolyshkin aac4d1f5c3 libct/cg/fscommon/GetCgroupParamKeyValue: nits
2. Fix wrapping the error to not have the value as it's already
   part of the error returned from ParseUint.

3. Fix/improve doc.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2020-09-29 11:57:49 -07:00
Kir Kolyshkin d167be2925 libct/cgroups/fs2/statHugeTlb: error message nits
1. Don't wrap the error from fscommon.GetCgroupParamUint as it already
   contains the file name.

2. Don't put file name when wrapping the error from ioutil.ReadFile
   since it already has it.

3. Don't reconstruct file name, use existing one since it's available.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2020-09-29 11:57:18 -07:00
Kir Kolyshkin 2c70d23840 libct/intelrdt: add TestFindIntelRdtMountpointDir
Heavily based on work by Paweł Szulik <pawel.szulik@intel.com>

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2020-09-29 09:21:32 -07:00
Sebastiaan van Stijn ab2b5dfa8a libcontainer/cgroups: use const for templates
The names of these templates overlapped with some local
variables, which made reading the code somewhat confusing.

Changing them to a const, given that these were not updated anywere.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2020-09-29 13:34:44 +02:00
Sebastiaan van Stijn b7c446b31a checkpoint: setPageServer: use net.SplitHostPort instead of strings.Split
strings.Split() isn't the best solution for this, as it will not handle (e.g.)
IPv6 addresses.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2020-09-29 12:28:03 +02:00
Akihiro Suda 49d4507c77 Merge pull request #2595 from thaJeztah/libcontainer_caps_refactor
Libcontainer: refactor capabilities code and remove RHEL6 workaround
2020-09-29 13:53:40 +09:00
Kir Kolyshkin f1c1fdf911 libcontainer/intelrdt: use moby/sys/mountinfo
It might be a tad slower but it surely more correct and well maintained,
so it's better to use it than rely on a custom implementation which is
kind of hard to get entirely right.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2020-09-28 20:18:01 -07:00
Mrunal Patel 43d2b10300 Merge pull request #2592 from kolyshkin/tests-int-nits
tests/integration: nits
2020-09-28 09:27:03 -07:00
Akihiro Suda 6e5320ffa8 Merge pull request #2584 from kolyshkin/unified-support
libct/cgroups: support Cgroups.Resources.Unified
2020-09-26 18:48:16 +09:00
Kir Kolyshkin 4929c05ad1 tests/int: add cgroupv2 unified resources tests
Note that various invalid keys are covered by test cases in
libcontainer/integration/exec_test.go

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2020-09-24 15:29:40 -07:00
Kir Kolyshkin 6e2159be74 tests/int/cgroups: make sure to rm containers
Container test_cgroups_group was missing from the teardown.

Fixes: c91fe9aeba.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2020-09-24 15:29:40 -07:00
Kir Kolyshkin b006f4a180 libct/cgroups: support Cgroups.Resources.Unified
Add support for unified resource map (as per [1]), and add some test
cases for the new functionality.

[1] https://github.com/opencontainers/runtime-spec/pull/1040

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2020-09-24 15:29:35 -07:00
Akihiro Suda 33faa5d0e2 Merge pull request #2575 from fuweid/update-state-store
libcontainer: Store state.json before sync procRun
2020-09-23 22:12:15 +09:00
Mrunal Patel 190fcf244c Merge pull request #2593 from kolyshkin/fix-verify-deps
Makefile: fix vendor and verify-dependencies
2020-09-21 18:57:42 -07:00
Kir Kolyshkin 30601efa81 tests/int/spec.bats: simplify
Most of whatever is happening in the test cases is already available in
setup_hello and teardown_hello. Use these.

Rewrite the validation test to be more compact and not dependent
on `pwd` value. Also, pinning xeipuuv/gojsonschema to a particular
version is no longer required.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2020-09-21 03:11:02 -07:00
Kir Kolyshkin 6c21de38a1 tests/int/spec: only run once for rootless
The purpose of the spec validation test is to make sure
that config.json generated by runc spec is valid. Therefore,
it does not depend on ROOTLESS_FEATURES.

Taking that into account, and given the fact that this test
involves cloning repos and building some code, it makes sense
to not run it 4 times for various rootless features.

Time it takes to execute spec.bats in rootless mode has improved.

Before:
	real	0m21.286s
	user	0m37.837s
	sys	0m5.745s

After:
	real	0m13.162s
	user	0m30.814s
	sys	0m4.050s

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2020-09-21 03:10:19 -07:00
Kir Kolyshkin 186a38ba73 tests/int: whitespace cleanup
Fix
 - whitespace at EOL
 - empty lines at EOF

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2020-09-21 02:47:23 -07:00
Kir Kolyshkin 792d2c3b36 tests/int/cgroups.bats: rm unused code
Commit d78ae51a2 adds this line, but the file is never created.

This is probably a leftover from copy-pasting update.bats code.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2020-09-21 02:47:23 -07:00
Kir Kolyshkin 908b7076e9 tests/int/*.bats: make sure to delete containers
With some grep|awk help, found a few places where the containers
supposed to be removed in teardown() weren't.

To find container names:

	grep -E '^[[:space:]]*\<_*_*runc\>.*\<(create|run|restore)\>'

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2020-09-21 02:47:23 -07:00
Sebastiaan van Stijn 1c3af27519 libcontainer: newContainerCapList() refactor to reduce duplicated code
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2020-09-19 11:26:27 +02:00
Sebastiaan van Stijn 8820a1459a libcontainer: initialize slices with length
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2020-09-19 11:04:29 +02:00
Sebastiaan van Stijn f5c96b7460 libcontainer: remove unneeded sprintf and intermediate variable
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2020-09-19 10:58:56 +02:00
Sebastiaan van Stijn b9e26ad8c6 libcontainer: remove workaround for RHEL6 kernels
This was a workaround for RHEL6 (2.6.xx) kernels, which have not
been supported by container runtimes for a long time, so should
be safe to remove this workaround.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2020-09-19 10:45:38 +02:00
Sebastiaan van Stijn 373811bad4 libcontainer: rename cap variable as it collides with built-in
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2020-09-19 10:43:31 +02:00
Kir Kolyshkin 074e67ad06 Makefile: fix vendor and verify-dependencies
Commit a08ab87fe added these targets. Alas, the `go mod tidy` never
worked, as it was written as part of `export` statement:

	export GO111MODULE=on \
        $(GO) mod tidy && \
	...

which is the same as

	export GO111MODULE=on $(GO) mod tidy && ...

which exports a bunch of variables, such as `go`, `mod`, and `tidy`,
but does not run it.

The fix would be to add a semicolon after the `export` statement,
but since GO111MODULE is not really needed here (maybe some older
golang versions needed it?), let's just drop it.

With this dropped, && does not make any sense, so drop it, too.

NOTE that if someone tries

	GO111MODULE=off make vendor

it will fail, but I guess it is expected.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2020-09-17 12:50:54 -07:00
Aleksa Sarai d636ad6256 merge branch 'pr-2591' into master
Sebastiaan van Stijn (1):
  vendor: github.com/moby/sys/mountinfo v0.2.0

LGTMs: @cyphar @AkihiroSuda
Closes #2591
2020-09-17 19:29:45 +10:00
Sebastiaan van Stijn 71c10e3c78 vendor: github.com/moby/sys/mountinfo v0.2.0
full diff: https://github.com/moby/sys/compare/mountinfo/v0.1.3...mountinfo/v0.2.0

Bug fixes:

- Fix path unescaping for paths with double quotes

Improvements:

- Mounted: speed up by adding fast paths using openat2 (Linux-only) and stat
- Mounted: relax path requirements (allow relative, non-cleaned paths, symlinks)
- Unescape fstype and source fields
- Documentation improvements

Testing/CI:

- Unit tests: exclude darwin
- CI: run tests under Fedora 32 to test openat2
- TestGetMounts: fix for Ubuntu build system
- Makefile: fix ignoring test failures
- CI: add cross build

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2020-09-16 22:52:26 +02:00
Kir Kolyshkin ba8687fc28 tests/int/helpers: fix indentation
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2020-09-16 13:17:50 -07:00
Kir Kolyshkin fdb0590c40 tests/int/helpers: simplify set_cgroup_mount_writable
Use update_config like in all the other places.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2020-09-16 13:17:44 -07:00