Commit Graph

5939 Commits

Author SHA1 Message Date
Kir Kolyshkin 6a3fe1618f libcontainer: remove LinuxFactory
Since LinuxFactory has become the means to specify containers state
top directory (aka --root), and is only used by two methods (Create
and Load), it is easier to pass root to them directly.

Modify all the users and the docs accordingly.

While at it, fix Create and Load docs (those that were originally moved
from the Factory interface docs).

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-03-22 23:44:31 -07:00
Kir Kolyshkin 6a29787bc9 libct/factory: make some methods functions
These do not have to be methods.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-03-22 23:29:24 -07:00
Kir Kolyshkin 8358a0ecbb libct: StartInitialization: decouple from factory
StartInitialization does not have to be a method of Factory (while
it is clear why it was done that way initially, now we only have
Linux containers so it does not make sense).

Fix callers and docs accordingly.

No change in functionality.

Also, since this was the only user of libcontainer.New with the empty
string as an argument, the corresponding check can now be removed
from it.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-03-22 23:29:24 -07:00
Kir Kolyshkin a78c9a0184 libct: remove Factory interface
The only implementation is LinuxFactory, let's use this directly.

Move the piece of documentation about Create from removed factory.go to
the factory_linux.go.

The LinuxFactory is to be removed later.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-03-22 23:29:24 -07:00
Kir Kolyshkin 71bc308b7f libct/New: remove options argument
It is not used by anyone.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-03-22 23:29:24 -07:00
Kir Kolyshkin b6514469a8 libct: remove TmpfsRoot
Is is not used by anyone

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-03-22 23:29:24 -07:00
Kir Kolyshkin 6e624d6f20 Merge pull request #3419 from kolyshkin/go1.18
Add / switch to Go 1.18
2022-03-22 18:02:37 -07:00
Sebastiaan van Stijn 404d9162b4 Merge pull request #3424 from kolyshkin/fix-badges
README,libct/README: fix pkg.go.dev badges
2022-03-23 00:33:06 +01:00
Mrunal Patel b9d55d5852 Merge pull request #3367 from kolyshkin/tests-add-set-u
tests: add `set -u`
2022-03-22 14:24:31 -07:00
Kir Kolyshkin a0f8847e2a Drop go 1.16
Require go 1.17 from now on, since go 1.16 is no longer supported.
Drop go1.16 compatibility.

NOTE we also have to install go 1.18 from Vagrantfile, because
Fedora 35 comes with Go 1.16.x which can't be used.

Note the changes to go.mod and vendor are due to
https://go.dev/doc/go1.17#tools

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-03-22 12:22:22 -07:00
Kir Kolyshkin 5211cc3f7e Add / switch to Go 1.18
Switch to Go 1.18 as the default Go version.

Support for Go 1.16 is removed by the next commit.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-03-22 12:22:22 -07:00
Kir Kolyshkin 7cec81e060 libct: suppress strings.Title deprecation warning
Function strings.Title is deprecated as of Go 1.18, because it does not
handle some corner cases good enough. In this case, though, it is
perfectly fine to use it since we have a single ASCII word as an
argument, and strings.Title won't be removed until at least Go 2.0.

Suppress the deprecation warning.

The alternative is to not capitalize the namespace string; this will break
restoring of a container checkpointed by earlier version of runc.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-03-22 12:22:10 -07:00
Kir Kolyshkin fcab941e4d ci: switch to golangci-lint 1.45
For release notes, see
https://github.com/golangci/golangci-lint/releases/tag/v1.45.0

Notably, it adds support for Go 1.18.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-03-22 12:16:23 -07:00
Qiang Huang c258ed0fc3 Merge pull request #3392 from kolyshkin/test-runc-delete-flake
tests/int: runc delete: fix flake, enable for rootless
2022-03-22 09:02:26 +08:00
Kir Kolyshkin 3618079cab README.md: add cirrus-ci badge
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-03-21 17:19:06 -07:00
Kir Kolyshkin f309a69a48 README,libct/README: fix pkg.go.dev badges
What used to be godoc.org is now pkg.go.dev, and while the old URLs
still work, they might be broken in the future.

Updated badges are generated via https://pkg.go.dev/badge/

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-03-21 17:05:41 -07:00
Sebastiaan van Stijn f6e5831cd0 Merge pull request #3420 from kolyshkin/rootlessEUIDMount
libct/configs/validate: rootlessEUIDMount: speedup
2022-03-21 17:09:54 +01:00
Kir Kolyshkin 48006d0007 libct/configs/validate: rootlessEUIDMount: speedup
1. Fix function docs. In particular, remove the part
   which is not true ("verifies that the user isn't trying to set up any
   mounts they don't have the rights to do"), and fix the part that
   says "that doesn't resolve to root" (which is no longer true since
   commit d8b669400a).

2. Replace fmt.Sscanf (which is slow and does lots of allocations)
   with strings.TrimPrefix and strconv.Atoi.

3. Add a benchmark for rootlessEUIDMount. Comparing the old and the new
   implementations:

	name                 old time/op    new time/op    delta
	RootlessEUIDMount-4    1.01µs ± 2%    0.16µs ± 1%  -84.15%  (p=0.008 n=5+5)

	name                 old alloc/op   new alloc/op   delta
	RootlessEUIDMount-4      224B ± 0%       80B ± 0%  -64.29%  (p=0.008 n=5+5)

	name                 old allocs/op  new allocs/op  delta
	RootlessEUIDMount-4      7.00 ± 0%      1.00 ± 0%  -85.71%  (p=0.008 n=5+5)

Note this code is already tested (in rootless_test.go).

Fixes: d8b669400a
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-03-17 13:39:55 -07:00
Akihiro Suda 98b75befc4 Merge pull request #3416 from kinvolk/rata/seccomp-misc
tests: Improve seccomp-notify test names and description
2022-03-17 13:02:03 +09:00
Rodrigo Campos a99f82add1 tests: Add comment to clarify intent of seccomp-notify tests
While doing the previous fix, I went over all the tests in this file and
made sure they were named correctly. This patch just adds a small
sentence to clarify the intent, and does some minor improvements to some
other test names.

Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
2022-03-15 11:09:20 +01:00
Rodrigo Campos 9f9acd1a0c tests: Improve name of seccomp notify test
There was a typo and instead of "empty" we should have used "non-empty".

Let's add a small sentence explaining the intent (like other tests in
this file) and let's highlight what we expect to happen in this test (to
ignore the listenerPath).

Fixes: #3415

Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
2022-03-15 11:09:00 +01:00
Sebastiaan van Stijn 51e607f2cd Merge pull request #3356 from kolyshkin/user-dbus
libct/cg/sd: simplify DetectUserDbusSessionBusAddress
2022-03-09 17:36:02 +01:00
Akihiro Suda 7fd8b57001 Merge pull request #3374 from kolyshkin/cli-nits
Assorted CLI nitpicks
2022-03-09 12:49:23 +09:00
Sebastiaan van Stijn 4e0d6898f6 Merge pull request #3370 from kolyshkin/bump-gofumpt
ci: bump golangci-lint to v1.44, golangci-lint-action to v3
2022-03-08 12:57:11 +01:00
Kir Kolyshkin 728571c16f tests/int: runc delete: fix flake, enable for rootless
The following failure was observed in CI (on centos-stream-8 in
integration-cgroup suite):

	not ok 42 runc delete
	 (from function `fail' in file tests/integration/helpers.bash, line 338,
	  in test file tests/integration/delete.bats, line 30)
	   `[ "$output" = "" ] || fail "cgroup not cleaned up correctly: $output"' failed
	....
	cgroup not cleaned up correctly: /sys/fs/cgroup/pids/system.slice/tmp-bats\x2drun\x2d68012-runc.IPOypI-state-testbusyboxdelete-runc.zriC8C.mount
	/sys/fs/cgroup/cpu,cpuacct/system.slice/tmp-bats\x2drun\x2d68012-runc.IPOypI-state-testbusyboxdelete-runc.zriC8C.mount
	...

Apparently, this is a cgroup systemd creates for a mount unit which
appears then runc does internal /proc/self/exe bind-mount. The test
case should not take it into account.

The second problem with this test is it does not check that cgroup
actually exists when the container is running (so checking that it
was removed after makes less sense). For example, in rootless mode
the cgroup might not have been created.

Fix the find arguments to look for a specific cgroup name, and add
a check that these arguments are correct (i.e. the cgroup is found
when the container is running).

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-03-07 15:33:29 -08:00
Kir Kolyshkin f7637defb8 ci: use golangci-lint-action v3, GO_VERSION
golangci-lint-action v3 no longer installs golang itself, and the
version that comes with Ubuntu is not new/good enough.

Install go 1.17.x explicitly.

Introduce GO_VERSION environment variable to avoid duplication,
and use it instead of 1.x in other places, so that implicit go update
won't bring some unexpected failures.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-03-07 10:57:35 -08:00
Kir Kolyshkin f7d4613492 ci: bump golangci-lint to v1.44
Also, remove "must be specified without patch version" as this is no
longer true.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-03-07 10:42:01 -08:00
Kir Kolyshkin 89733cd055 Format sources using gofumpt 0.2.1
... which adds a wee more whitespace fixes.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-03-07 10:42:01 -08:00
Kir Kolyshkin ec9e81bfb0 Merge pull request #3400 from opencontainers/dependabot/github_actions/actions/checkout-3
build(deps): bump actions/checkout from 2 to 3
2022-03-07 10:33:42 -08:00
Akihiro Suda f7bce6b769 Merge pull request #3382 from marquiz/devel/rdt-validation
configs/validate: looser validation for RDT
2022-03-07 14:36:23 +09:00
Akihiro Suda 4d2e4f1a6b Merge pull request #3381 from lifubang/exec-subcgroup
ensure the path is a sub-cgroup path
2022-03-07 14:35:49 +09:00
Sebastiaan van Stijn 1515d93639 Merge pull request #3389 from kolyshkin/delegate-enoent
libct/cg/sd/v2: fix ENOENT on cgroup delegation
2022-03-04 10:00:12 +01:00
dependabot[bot] a43485c92c build(deps): bump actions/checkout from 2 to 3
Bumps [actions/checkout](https://github.com/actions/checkout) from 2 to 3.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](https://github.com/actions/checkout/compare/v2...v3)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
2022-03-02 04:20:24 +00:00
Kir Kolyshkin 1a93520841 libct/cg/sd: simplify DetectUserDbusSessionBusAddress
Apparently, "systemctl --user --no-pager show-environment" is useless
without DBUS_SESSION_BUS_ADDRESS or XDG_RUNTIME_DIR set:

	$ echo $DBUS_SESSION_BUS_ADDRESS, $XDG_RUNTIME_DIR
	unix:path=/run/user/1000/bus, /run/user/1000
	$ systemctl --user --no-pager show-environment | grep DBUS_SESS
	DBUS_SESSION_BUS_ADDRESS=unix:path=/run/user/1000/bus
	$ unset DBUS_SESSION_BUS_ADDRESS
	$ systemctl --user --no-pager show-environment | grep DBUS_SESS
	DBUS_SESSION_BUS_ADDRESS=unix:path=/run/user/1000/bus
	$ unset XDG_RUNTIME_DIR
	$ systemctl --user --no-pager show-environment | grep DBUS_SESS
	Failed to connect to bus: $DBUS_SESSION_BUS_ADDRESS and $XDG_RUNTIME_DIR not defined (consider using --machine=<user>@.host --user to connect to bus of other user)

So, it does not make sense to try it to get the address.

Also, it does not make sense to suggest  "systemctl --user start dbus"
either, for the same reason, so remove that suggestion from the error
message text.

Since DBUS_SESSION_BUS_ADDRESS environment variable, on which the code
relies, is et by dbus-run-session (or dbus-launch, or something similar
that is supposed to be run during the login process), add a suggestion
to re-login.

Finally, fix the following linter warning:

> error-strings: error strings should not be capitalized or end with punctuation or a newline (revive)

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-03-01 11:39:01 -08:00
Kir Kolyshkin 11895cd087 libct/cg/sd: escape dbus address value
D-Bus specification [1] requires that the values in server address need
to be escaped in a special way, and other clients perform the needed
escaping (e.g. systemd [2] does that, as well as recent godbus [3]).

More to say, it is important to perform such escaping, since if dbus
sees a character that should have been escaped but it's not, it returns
an error [4].

Fix tryDiscoverDbusSessionBusAddress to use dbus.EscapeBusAddressValue
function, recently added to godbus [3].

[1] https://dbus.freedesktop.org/doc/dbus-specification.html#addresses
[2] https://github.com/systemd/systemd/blob/5efbd0bf897a990ebe43d7dc69141d87c404ac9a/src/libsystemd/sd-bus/bus-internal.c#L294-L318
[3] https://github.com/godbus/dbus/pull/302
[4] https://gitlab.freedesktop.org/dbus/dbus/-/blob/37b76d13738e782fe2eb12abdd0179745c0b3f81/dbus/dbus-address.c#L330

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-03-01 11:39:01 -08:00
Kir Kolyshkin 38c21694ba tests/integration/helpers: set -u
This is a way to prevent the code doing something really bad when a
variable it uses is not set. Good to have since it helps to catch some
logical errors etc.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-02-28 18:41:39 -08:00
Kir Kolyshkin c8c3e8526d tests: fix checks for non-existent variables
Audit all checks for non-empty variables (i.e. ' -z ', ' -n ',
' != ""' and '= ""'), and fix those cases where a variable might be
unset. Those variables (that might not be set) are

 - RUNC_USE_SYSTEMD
 - BATS_RUN_TMPDIR
 - AUX_UID
 - AUX_DIR
 - SD_PARENT_NAME
 - REL_PARENT_PATH
 - ROOT
 - HAVE_CRIU
 - ROOTLESS_FEATURES
 - and a few test-specific or file-specific variables

This should allow us to enable set -u.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-02-28 18:41:35 -08:00
Kir Kolyshkin 99d5c0231f tests/int/{root,list}.bats: ALT_ROOT fixups in teardown
1. Add "unset ALT_ROOT" since it should not be used after teardown is
   called.

2. Remove "rm -rf $ALT_ROOT". It is not needed, because ALT_ROOT is a
   subdirectory of ROOT, which is removed in teardown_bundle.

3. Checking for ALT_ROOT being non-empty is a leftover from the era when
   teardown() was called as the first step from setup(). Since commit
   41670e21f0 this is no longer the case, so the condition
   is no longer needed (plus, the `set -u` which is about to be added
   should catch any possible use of unset ALT_ROOT).

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-02-28 18:39:08 -08:00
Kir Kolyshkin 7da77d802e tests/int: don't add --root if $ROOT is not set
Some tests (those in help.bats and version.bats) do not use setup_bundle
(as they do not need to start any containers), and thus they do not set
$ROOT. As a consequence, these tests now call "runc --root /state" which
is not nice.

Make adding --root conditional (only if $ROOT is set).

Amazingly, this change breaks help.bats tests under rootless, because
"sudo rootless" does not change the value of XDG_RUNTIME_DIR which still
points to root-owned directory, and as a result we have this:

> runc foo -h (status=1):
> the path in $XDG_RUNTIME_DIR must be writable by the user
> time="2022-02-08T07:04:57Z" level=error msg="mkdir /run/user/0/runc: permission denied"

This could be fixed by adding proper $ROOT, but it's easier just to skip
those tests under non-root.

NOTE that version.bats is not broken because -v is handled by urfave/cli
very early, so app.Before function is not run.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-02-28 18:32:10 -08:00
Kir Kolyshkin 9e2a0463e5 tests/int: fix runc_spec for set -u
Older bash versions treats variable as unset if nothing has been
assigned to it. Here is an example from CentOS 7 system:

	[kir@localhost ~]$ bash -u -c 'x() { local args=(); echo "${args[@]}"; }; x'
	bash: args[@]: unbound variable
	[kir@localhost ~]$ echo $BASH_VERSION
	4.2.46(2)-release

Rewrite to work around this.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-02-28 18:32:10 -08:00
Akihiro Suda ad93b0c6c9 Merge pull request #3398 from opencontainers/dependabot/go_modules/github.com/godbus/dbus/v5-5.1.0
build(deps): bump github.com/godbus/dbus/v5 from 5.0.6 to 5.1.0
2022-03-01 11:00:14 +09:00
dependabot[bot] ab9609dbc2 build(deps): bump github.com/godbus/dbus/v5 from 5.0.6 to 5.1.0
Bumps [github.com/godbus/dbus/v5](https://github.com/godbus/dbus) from 5.0.6 to 5.1.0.
- [Release notes](https://github.com/godbus/dbus/releases)
- [Commits](https://github.com/godbus/dbus/compare/v5.0.6...v5.1.0)

---
updated-dependencies:
- dependency-name: github.com/godbus/dbus/v5
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
2022-02-28 04:22:13 +00:00
Kir Kolyshkin 8c04b98100 libct/cg/sd/v2: fix ENOENT on cgroup delegation
Apparently, not all files listed in /sys/kernel/cgroup/delegate must
exist in every cgroup, so we should ignore ENOENT.

Dot not ignore ENOENT on the directory itself though.

Change cgroupFilesToChown to not return ".", and refactor it to not do
any dynamic slice appending in case we're using the default built-in
list of files.

Fixes: 35d20c4e0
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-02-21 10:41:56 -08:00
lifubang 01f00e1fd5 ensure the path is a sub-cgroup path
Signed-off-by: lifubang <lifubang@acmcoder.com>
2022-02-19 09:45:09 +08:00
Kir Kolyshkin 40b0088681 loadFactory: remove
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-02-18 16:05:29 -08:00
Kir Kolyshkin d1fca8e599 list: report error when non-existent --root is specified
It is questionable whether runc list should return an empty list of
containers when non-existent --root is specified or not.

The current behavior is the directory is always created and then the
empty list of container is shown.

To my mind, specifying a non-existent root is an error and should be
reported as such. This is what this patch does.

For backward compatibility, if --root is not set (i.e. a default is
used), ENOENT is not reported.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-02-18 16:05:29 -08:00
Kir Kolyshkin 2b07e751b5 reviseRootDir: skip default values, add validation
1. In case --root option is not provided, do nothing.

2. Instead of checking if root value is empty string, check it after
   filepath.Abs, and reject "/". Improve docstring while at it.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-02-18 16:05:29 -08:00
Kir Kolyshkin 899342b5d4 main: improve XDG_RUNTIME_DIR handling
1. Variable xdgRuntimeDir is only checked to be non-empty. Change it to
   a boolean.

2. Refactor so that os.Getenv is not called twice.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-02-18 16:05:29 -08:00
Kir Kolyshkin eb2f08dc4e checkpoint,restore,list: don't call fatal
There is a mix of styles when handling CLI commands. In most cases we
return an error, which is handled from app.Run in main.go (it calls
fatal if there is an error).

In a few cases, though, we call fatal(err) from random places.

Let's be consistent and always return an error. The only exception is
runc exec, which needs to exit with a particular exit code.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-02-18 16:05:29 -08:00
Kir Kolyshkin 36786c361a list, utils: remove redundant code
The value of root is already an absolute path since commit
ede8a86ec1, so it does not make sense to call filepath.Abs()
again.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2022-02-18 16:05:29 -08:00