Runtime spec says:
> sysctl (object, OPTIONAL) allows kernel parameters to be modified at
> runtime for the container. For more information, see the sysctl(8)
> man page.
and sysctl(8) says:
> variable
> The name of a key to read from. An example is
> kernel.ostype. The '/' separator is also accepted in place of a '.'.
Apparently, runc config validator do not support sysctls with / as a
separator. Fortunately this is a one-line fix.
Add some more test data where / is used as a separator.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Add a unit test to check that bind mounts that have a part of its
path non accessible by others still work when using user namespaces.
To do this, we also modify newRoot() to return rootfs directories that
can be traverse by others, so the rootfs created works for all test
(either running in a userns or not).
Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
Signed-off-by: Rodrigo Campos <rodrigo@kinvolk.io>
Co-authored-by: Rodrigo Campos <rodrigo@kinvolk.io>
1. Use t.TempDir instead of ioutil.TempDir. This means no need for an
explicit cleanup, which removes some code, including newTestBundle
and newTestRoot.
2. Move newRootfs invocation down to newTemplateConfig, removing a need
for explicit rootfs creation. Also, remove rootfs from tParam as it
is no longer needed (there was a since test case in which two
containers shared the same rootfs, but it does not look like it's
required for the test).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In addition to freezing and thawing a container via Pause/Resume,
there is a way to also do so via Set.
This way was broken though and is being fixed by a few preceding
commits. The test is added to make sure this is fixed and won't regress.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
gofumpt (mvdan.cc/gofumpt) is a fork of gofmt with stricter rules.
Brought to you by
git ls-files \*.go | grep -v ^vendor/ | xargs gofumpt -s -w
Looking at the diff, all these changes make sense.
Also, replace gofmt with gofumpt in golangci.yml.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
When running inside a Docker container, systemd is not available. The
new TestFdLeaksSystemd forgot to include the relevant t.Skip section.
Fixes: a7feb42395 ("libct/int: add TestFdLeaksSystemd")
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Add a test to check that container.Run do not leak file descriptors.
Before the previous commit, it fails like this:
exec_test.go:2030: extra fd 8 -> socket:[659703]
exec_test.go:2030: extra fd 11 -> socket:[658715]
exec_test.go:2033: found 2 extra fds after container.Run
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Since commit 88e8350de2 the error message is different, so the check
is not working. In addition, for the cgroup v2 case, and it seems that
PID controller is always available these days.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
1. Do not create the same container named "test" over and over.
2. Fix randomization issues when generating container and cgroup names.
The issues were:
* math/rand used without seeding
* complex rand/md5/hexencode sequence
In both cases, replace with nanosecond time encoded with digits and
lowercase letters.
3. Add test name to container and cgroup names. For example, this is
how systemd log has changed:
Before: Started libcontainer container test16ddfwutxgjte.
After: Started libcontainer container TestPidsSystemd-4oaqvr.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This is somewhat radical approach to deal with kernel memory.
Per-cgroup kernel memory limiting was always problematic. A few
examples:
- older kernels had bugs and were even oopsing sometimes (best example
is RHEL7 kernel);
- kernel is unable to reclaim the kernel memory so once the limit is
hit a cgroup is toasted;
- some kernel memory allocations don't allow failing.
In addition to that,
- users don't have a clue about how to set kernel memory limits
(as the concept is much more complicated than e.g. [user] memory);
- different kernels might have different kernel memory usage,
which is sort of unexpected;
- cgroup v2 do not have a [dedicated] kmem limit knob, and thus
runc silently ignores kernel memory limits for v2;
- kernel v5.4 made cgroup v1 kmem.limit obsoleted (see
https://github.com/torvalds/linux/commit/0158115f702b).
In view of all this, and as the runtime-spec lists memory.kernel
and memory.kernelTCP as OPTIONAL, let's ignore kernel memory
limits (for cgroup v1, same as we're already doing for v2).
This should result in less bugs and better user experience.
The only bad side effect from it might be that stat can show kernel
memory usage as 0 (since the accounting is not enabled).
[v2: add a warning in specconv that limits are ignored]
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This is a very simple test that checks that container.Run do not leak
opened file descriptors.
In fact it does, so we have to add two exclusions:
1. /sys/fs/cgroup is opened once per lifetime in prepareOpenat2(),
provided that cgroupv2 is used and openat2 is available. This
works as intended ("it's not a bug, it's a feature").
2. ebpf program fd is leaked every time we call setDevices() for
cgroupv2 (iow, every container.Run or container.Set leaks 1 fd).
This needs to be fixed, thus FIXME.
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>
Fix a merge issue between 0aa0fae393 ("Kill all processes in cgroup even if init process Wait fails")
& 73d93eeb01 ("libct/int: make newTemplateConfig argument a struct") that
resulted in passing the wrong datatype to newTemplateConfig in
TestPIDHostInitProcessWait.
Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
If the cgroup's init process doesn't complete successfully, Wait returns a
non-nil error. We should still kill all the process in the cgroup if process
namespace is shared. Otherwise, it may result in process leak.
Fixes#2632
Signed-off-by: Chaitanya Bandi <kbandi@cs.stonybrook.edu>
It seems that a few tests add a cgroup mount in case userns is not set.
Let's do it inside newTemplateConfig() for all tests.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
...so we can add more fields later.
This commit is mostly courtesy of
sed -i 's/newTemplateConfig(rootfs)/newTemplateConfig(\&tParam{rootfs: rootfs})/g'
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The only two tests that are still skipped on v2 are kmem
and invalid CpuShares test -- since v2 does not support either.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
As the baby step, only unit tests are executed.
Failing tests are currently skipped and will be fixed in follow-up PRs.
Fix#2124
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
For some reason, libcontainer/integration has a whole bunch of incorrect
usages of libcontainer.Factory -- causing test failures with a set of
security patches that will be published soon. Fixing ths is fairly
trivial (switch to creating a new libcontainer.Factory once in each
process, rather than creating one in TestMain globally).
Signed-off-by: Aleksa Sarai <asarai@suse.de>
Finish off the work started in a344b2d6 (sync up `HookState` with OCI
spec `State`, 2016-12-19, #1201).
And drop HookState, since there's no need for a local alias for
specs.State.
Also set c.initProcess in newInitProcess to support OCIState calls
from within initProcess.start(). I think the cyclic references
between linuxContainer and initProcess are unfortunate, but didn't
want to address that here.
I've also left the timing of the Prestart hooks alone, although the
spec calls for them to happen before start (not as part of creation)
[1,2]. Once the timing gets fixed we can drop the
initProcessStartTime hacks which initProcess.start currently needs.
I'm not sure why we trigger the prestart hooks in response to both
procReady and procHooks. But we've had two prestart rounds in
initProcess.start since 2f276498 (Move pre-start hooks after container
mounts, 2016-02-17, #568). I've left that alone too.
I really think we should have len() guards to avoid computing the
state when .Hooks is non-nil but the particular phase we're looking at
is empty. Aleksa, however, is adamantly against them [3] citing a
risk of sloppy copy/pastes causing the hook slice being len-guarded to
diverge from the hook slice being iterated over within the guard. I
think that ort of thing is very lo-risk, because:
* We shouldn't be copy/pasting this, right? DRY for the win :).
* There's only ever a few lines between the guard and the guarded
loop. That makes broken copy/pastes easy to catch in review.
* We should have test coverage for these. Guarding with the wrong
slice is certainly not the only thing you can break with a sloppy
copy/paste.
But I'm not a maintainer ;).
[1]: https://github.com/opencontainers/runtime-spec/blob/v1.0.0/config.md#prestart
[2]: https://github.com/opencontainers/runc/issues/1710
[3]: https://github.com/opencontainers/runc/pull/1741#discussion_r233331570
Signed-off-by: W. Trevor King <wking@tremily.us>
Cgroup namespace can be configured in `config.json` as other
namespaces. Here is an example:
```
"namespaces": [
{
"type": "pid"
},
{
"type": "network"
},
{
"type": "ipc"
},
{
"type": "uts"
},
{
"type": "mount"
},
{
"type": "cgroup"
}
],
```
Note that if you want to run a container which has shared cgroup ns with
another container, then it's strongly recommended that you set
proper `CgroupsPath` of both containers(the second container's cgroup
path must be the subdirectory of the first one). Or there might be
some unexpected results.
Signed-off-by: Yuanhong Peng <pengyuanhong@huawei.com>
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
There is a race in runc exec when the init process stops just before
the check for the container status. It is then wrongly assumed that
we are trying to start an init process instead of an exec process.
This commit add an Init field to libcontainer Process to distinguish
between init and exec processes to prevent this race.
Signed-off-by: Mrunal Patel <mrunalp@gmail.com>
Previously if oomScoreAdj was not set in config.json we would implicitly
set oom_score_adj to 0. This is not allowed according to the spec:
> If oomScoreAdj is not set, the runtime MUST NOT change the value of
> oom_score_adj.
Change this so that we do not modify oom_score_adj if oomScoreAdj is not
present in the configuration. While this modifies our internal
configuration types, the on-disk format is still compatible.
Signed-off-by: Aleksa Sarai <asarai@suse.de>
Handle err return value of fmt.Scanf, os.Pipe and unix.ParseUnixRights.
Found with honnef.co/go/tools/cmd/staticcheck
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
Since syscall is outdated and broken for some architectures,
use x/sys/unix instead.
There are still some dependencies on the syscall package that will
remain in syscall for the forseeable future:
Errno
Signal
SysProcAttr
Additionally:
- os still uses syscall, so it needs to be kept for anything
returning *os.ProcessState, such as process.Wait.
Signed-off-by: Christy Perez <christy@linux.vnet.ibm.com>
When process config doesnt specify capabilities anywhere, we should not panic
because setting capabilities are optional.
Signed-off-by: Daniel Dao <dqminh89@gmail.com>
