Apparently it is inevitable that we have to read mountinfo multiple
times when dealing with cgroup v1. It seems we can only do it once
and reuse the data, without major modifications to the code.
This commit does a few things.
1. Drop our custom mountinfo parser implementation in favor of
moby/sys/mountinfo. While the custom parser is faster
(about 2x according to benchmark) for this particular case,
the one from the package is more correct and future-proof.
2. Read mountinfo only once, caching all the entries with fstype of
cgroup. With this, there's no need to worry about performance
degradation introduced above.
3. Drop "isSubsystemAvailable" optimization (introduced by commit
2a1a6cdf44) because now with the cache it is probably slowing
things down.
4. Modify the tests accordingly.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Note that `make ci` in Travis included `make test`, but we're already
doing that elsewhere (see .github/workflows/test.yml).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In here we have to use Docker, as Ubuntu does not support
all the architectures we're compile-testing here.
Since this is the only step that is using Docker,
there is no sense to separate `make runcimage` from
the rest of it. In case we'll have to use Docker image
more, it will make sense to do so.
While at it, ditch script/tmpmount (added by commit 1735ad788f),
because
- it required root (because mount);
- it is probably no longer needed.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
There are three tests that only differ in delays (and a few other
minor details line whitespace formatting and number of retries).
Let's unify those so maintaining this code will be easier.
This also implicitly increases the number of retries from 3 to 10
for the non-default --interval tests.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The test setup is complicated, as we want to run it as non-root,
but we need to create a directory owned by a particular user,
and root is needed for chown. To do that, we hijack enable_idmap.
Now, there is no ideal place to clean up AUX_DIR. A function
cleanup is called after the test to do so. Note that it won't
be called if the test fails.
This was verified to fail before and pass after the
previous commit.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
commit 5e0e67d76c moved the chdir to be one of the
first steps of finalizing the namespace of the container.
However, this causes issues when the cwd is not accessible by the user running runc, but rather
as the container user.
Thus, setupUser has to happen before we call chdir. setupUser still happens before setting the caps,
so the user should be privileged enough to mitigate the issues fixed in 5e0e67d76c
Signed-off-by: Peter Hunt <pehunt@redhat.com>
Commit 5e0e67d7 ("fix permission denied") modified some code but
did not provide a test case.
This is a test case that was tested to fail before and succeed after
the above commit.
For more details, see https://github.com/opencontainers/runc/pull/2086
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In case many net.* sysctls are provided, and we're not running
in the host netns, the function keep repeating isNetNS check
for every such sysctl. This is a waste of resources.
Do the isNetNS check only once, and only if needed.
Note that using sync.Once() is not really needed here; we could
have used a boolean variable to skip the repeated check, but
it looks more idiomatic that way.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In case nsfs mount (such as /run/docker/netns/xxxx) is provided as
the netns path, the current way of determining whether path is of
host netns or not is not working.
The proper way to check is to do stat(2) and compare dev_t and
inode fields, which is what this commit does.
This is a minimal fix which does not try to optimize repeated
check in case more than one net.* sysctl is given and there is
no error.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Use sync.Once to init Intel RDT when needed for a small speedup to
operations which do not require Intel RDT.
Simplify IntelRdtManager initialization in LinuxFactory.
Signed-off-by: Xiaochen Shen <xiaochen.shen@intel.com>
The apparmor tag was introduced in a01ed80 (2014) to make cgo dependency
on libapparmor optional.
However, the cgo dependency was removed in db093f6 (2017), so it is no
longer meaningful to keep apparmor build tag.
Close#2704
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
> libcontainer/intelrdt/monitoring.go:24:2: SA5001: should check returned error before deferring file.Close() (staticcheck)
> defer file.Close()
> ^
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
> libcontainer/container_linux.go:768:2: S1017: should replace this `if` statement with an unconditional `strings.TrimPrefix` (gosimple)
> if strings.HasPrefix(mountDest, c.config.Rootfs) {
> ^
> libcontainer/container_linux.go:1150:2: S1017: should replace this `if` statement with an unconditional `strings.TrimPrefix` (gosimple)
> if strings.HasPrefix(mountDest, c.config.Rootfs) {
> ^
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
> libcontainer/container_linux.go:683:2: S1021: should merge variable declaration with assignment on next line (gosimple)
> var t criurpc.CriuReqType
> ^
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
... use the one from unix instead.
Coincidentally, this fixes this warning from gosimple linter:
> libcontainer/integration/exec_test.go:448:2: S1021: should merge variable declaration with assignment on next line (gosimple)
> var netAdminBit uint
> ^
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This fixes the following warnings:
> libcontainer/integration/exec_test.go:369:18: S1030: should use stdout.String() instead of string(stdout.Bytes()) (gosimple)
> outputStatus := string(stdout.Bytes())
> ^
> libcontainer/integration/exec_test.go:422:18: S1030: should use stdout.String() instead of string(stdout.Bytes()) (gosimple)
> outputStatus := string(stdout.Bytes())
> ^
> libcontainer/integration/exec_test.go:486:18: S1030: should use stdout.String() instead of string(stdout.Bytes()) (gosimple)
> outputGroups := string(stdout.Bytes())
> ^
> libcontainer/integration/execin_test.go:191:18: S1030: should use stdout.String() instead of string(stdout.Bytes()) (gosimple)
> outputGroups := string(stdout.Bytes())
> ^
> libcontainer/integration/execin_test.go:474:9: S1030: should use stdout.String() instead of string(stdout.Bytes()) (gosimple)
> out := string(stdout.Bytes())
> ^
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Commit 4415446c32 introduces this function which is never used.
Remove it.
This fixes
> libcontainer/container_linux.go:1813:26: func `(*linuxContainer).deleteState` is unused (unused)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
> libcontainer/cgroups/utils.go:282:4: SA4006: this value of `paths` is never used (staticcheck)
> paths = make(map[string]string)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In case we get ENOSYS from openat2(2), this is expected, so log that
we're falling back to using securejoin as debug.
Otherwise, log it as a warning (as the error is unexpected, but we're
still good to go).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In case openat2 is available, it will be used to guarantee
that we're not accessing anything other than cgroupfs[2] files.
In cases when openat2 is not available, or when cgroup has a
non-standard prefix (not "/sys/fs/cgroup", which might theoretically
be the case on some very old installs and/or very custom systems),
fall back to using securejoin + os.Open like we did before.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This "reverts" commit 9ff7b82f9. We use the repo provided and maintained
by the CRIU team instead of compiling it from source.
While at it,
1. Don't install packages that were solely needed to compile CRIU.
2. Combine all apt-related stuff into a single step, so we don't
download and then remove apt metadata 3 times.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Kir Kolyshkin