It is assumed that m.config is not nil, so these checks are redundant
(in case it is nil, NewManager panics and this code is unreachable).
Note that cgroups/manager.New checks that config is not nil.
Remove them.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Cgroup controllers should never panic, and yet sometimes they do.
Add a unit test to check that controllers never panic when called with
nil arguments and/or resources, and fix a few found cases.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
cgName and cgParent are only used when cgPath is empty, so move
their cleaning to the body of the appropriate "if" statement.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This way we
- won't re-initialize the paths if they were provided;
- will always have paths ready for every method.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This is already documented but I guess more explanations (in particular,
why the path is being removed from paths) won't hurt.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
1. Separate path initialization logic from Apply to initPaths,
and call initPaths from NewManager, so:
- we can error out early (in NewManager rather than Apply);
- always have m.paths available (e.g. in Destroy or Exists).
- do not unnecessarily call subsysPath from Apply in case
the paths were already provided.
2. Add a check for non-nil cgroups.Resources to NewManager,
since initPaths, as well as some controller's Apply methods,
need it.
3. Move cgroups.Resources.Unified check from Apply to NewManager,
so we can error out early (same check exists in Set).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
1. Make Rootless and Systemd flags part of config.Cgroups.
2. Make all cgroup managers (not just fs2) return error (so it can do
more initialization -- added by the following commits).
3. Replace complicated cgroup manager instantiation in factory_linux
by a single (and simple) libcontainer/cgroups/manager.New() function.
4. getUnifiedPath is simplified to check that only a single path is
supplied (rather than checking that other paths, if supplied,
are the same).
[v2: can't -> cannot]
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Wire through CRIU's support to change the mount context on restore.
This is especially useful if restoring a container in a different pod.
Single container restore uses the same SELinux process label and
same mount context as during checkpointing. If a container is being
restored into an existing pod the process label and the mount context
needs to be changed to the context of the pod.
Changing process label on restore is already supported by runc. This
patch adds the possibility to change the mount context.
Signed-off-by: Adrian Reber <areber@redhat.com>
runc delete -f is not working for a paused container, since in cgroup v1
SIGKILL does nothing if a process is frozen (unlike cgroup v2, in which
you can kill a frozen process with a fatal signal).
Theoretically, we only need this for v1, but doing it for v2 as well is
OK.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
1. Dismantle and remove struct cgroupData. It contained three unrelated
entities (cgroup paths, pid, and resources), and made the code
harder to read. Most importantly, though, it is not needed.
Now, subsystems' Apply methods take path, resources, and pid.
To a reviewer -- the core of the changes is in fs.go and paths.go,
the rest of it is adapting to the new signatures and related test
changes.
2. Dismantle and remove struct cgroupTestUtil. This is a followup
to the previous item -- since cgroupData is gone, there is nothing
to hold in cgroupTestUtil. The change itself is very small (see
util_test.go), but this patch is big because of it -- mostly
because we had to replace helper.cgroup.Resources with
&config.Resources{}.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In case c.Path is set, c.Name and c.Parent are not used, and so
calls to utils.CleanPath are entirely unnecessary. Move them to
inside of the "if" statement body.
Get rid of the intermediate cgPath variable, it is not needed.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Now fs.go is not very readable as its public API functions are
intermixed with internal stuff about getting cgroup paths.
Move that out to paths.go, without changing any code.
Same for the tests -- move paths-related tests to paths_test.go.
This commit is separate to make the review easier.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
As ExpandSlice("system.slice") returns "/system.slice", there is no need
to call it for such paths (and the slash will be added by path.Join
anyway).
The same optimization was already done for v2 as part of commit
bf15cc99b1.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
We were checking if a unit is a slice two times. Consolidate those
checks, and improve comments while we're at it.
The code is the same in v1 and v2 but it's too complicated to factor it
out, thus we just do the same changes in v1.go and v2.go.
No functional change.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Currently, if the log level is not set to e.g. "debug", runc init sends
some debug logs to the parent, which parses and discards it.
It is better to not send those in the first place.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The code already parses an environment variable into an integer twice,
and we're about to add a third one.
Factor it out to getenv_int().
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This makes it possible to use bail() even if logging is not set up
(yet), so we don't have to think whether it's OK to use it or not.
In addition, this might help some unit tests that do not set log
forwarding.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Two new seccomp actions have been added to the libseccomp-golang
dependency, which can be now supported by runc, too.
ActKillThread kills the thread that violated the rule. It is the same as
ActKill. All other threads from the same thread group will continue to
execute.
ActKillProcess kills the process that violated the rule. All threads in
the thread group are also terminated. This action is only usable when
libseccomp API level 3 or higher is supported.
Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
These are just boilerplate and are only really useful for the two
actions which require us to set a default errno/aux value (ActErrno and
ActTrace).
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
Kir Kolyshkin (6):
runc --debug: shorter caller info
libct/logs: do not show caller in nsexec logs
libct/logs: parse log level implicitly
libct/logs: test: make more robust
libct/logs: remove ConfigureLogging
init.go, main.go: don't use logs.ConfigureLogging
LGTMs: thaJeztah cyphar
Kir Kolyshkin (3):
libct/system: add I and P process states
libct/system.Stat: fix/improve/speedup
libct/system/proc_test: fix, improve, add benchmark
LGTMs: thaJeztah cyphar
Extend the SetupSeccomp tests by adding the following cases:
- Test nil config
- Test empty config
- Test bad action and architecture
- Test all possible actions
Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
This commit implements support for the SCMP_ACT_NOTIFY action. It
requires libseccomp-2.5.0 to work but runc still works with older
libseccomp if the seccomp policy does not use the SCMP_ACT_NOTIFY
action.
A new synchronization step between runc[INIT] and runc run is introduced
to pass the seccomp fd. runc run fetches the seccomp fd with pidfd_get
from the runc[INIT] process and sends it to the seccomp agent using
SCM_RIGHTS.
As suggested by @kolyshkin, we also make writeSync() a wrapper of
writeSyncWithFd() and wrap the error there. To avoid pointless errors,
we made some existing code paths just return the error instead of
re-wrapping it. If we don't do it, error will look like:
writing syncT <act>: writing syncT: <err>
By adjusting the code path, now they just look like this
writing syncT <act>: <err>
Signed-off-by: Alban Crequy <alban@kinvolk.io>
Signed-off-by: Rodrigo Campos <rodrigo@kinvolk.io>
Co-authored-by: Rodrigo Campos <rodrigo@kinvolk.io>
SendFds is a helper function for sending a set of file descriptors and a message
over a unix domain socket.
Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
if function returns error before WriteJSON defer, error will not be
printed out, so move this defer as early as possible and use logrus to
print out error if returns before it.
Signed-off-by: xiadanni <xiadanni1@huawei.com>
Commit 9f3d7534ea enabled logrus to show information about log
caller, if --debug is set. It is helpful in many scenarios, but does
not work very well when we are debugging runc init, for example:
# runc --debug run -d xx4557
DEBU[0000]libcontainer/logs/logs.go:45 github.com/opencontainers/runc/libcontainer/logs.processEntry() nsexec[279687]: logging set up
DEBU[0000]libcontainer/logs/logs.go:45 github.com/opencontainers/runc/libcontainer/logs.processEntry() nsexec[279687]: logging set up
DEBU[0000]libcontainer/logs/logs.go:45 github.com/opencontainers/runc/libcontainer/logs.processEntry() nsexec[279687]: => nsexec container setup
DEBU[0000]libcontainer/logs/logs.go:45 github.com/opencontainers/runc/libcontainer/logs.processEntry() nsexec[279687]: update /proc/self/oom_score_adj to '30'
As we're merely forwarding the logs here, printing out filename:line and
function is useless and clutters the logs a log.
To fix, create and use a copy of the standard logger with caller info
turned off.
With this in place, nsexec logs are sane again:
# runc --debug --log-format=text run -d xe34
DEBU[0000] nsexec[293595]: logging set up
DEBU[0000] nsexec[293595]: logging set up
DEBU[0000] nsexec[293595]: => nsexec container setup
DEBU[0000] nsexec[293595]: update /proc/self/oom_score_adj to '30'
This patch also changes Logf to Log in processEntry, as this is what it
should be.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
There's no need to call logrus.ParseLevel as logrus.Level already
implements encoding.TextUnmarshaler interface.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
When playing with the log forwarder, I broke it, but all the units tests
were still passing. This happened because test cases were merely looking
for a word (like "kitten") in the log output, which also happened to be
there in case of an error (as a part of an error message produced by log
forwarder).
Make the test a bit more robust by
- looking for a complete log message, not just part of it;
- also checking that log file does NOT contain errors.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
- add missing colons before error message;
- unify error messages after cmd.Start and cmd.Wait, so that they show
context and the error itself.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Instead of reading a single message, do read all the logs from the init,
and use DisallowUnknownFields for stricter checking.
While at it, use reapChildren to reap zombies (and add an extra check).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The test was not working since at least commit 64bb59f592
renamed pid to stage2_pid (or maybe even earlier), so the pid
was never received (i.e. pid.Pid was 0).
The problem was not caught because os.FindProcess never return an error
on Unix.
Factor out and fix pid decode function:
- use DisallowUnknownInput to get error if JSON will be changed;
- check pids to make sure they are valid
- and use unix.Wait4 to reap zombies.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
1. Make sure we close all file descriptors at the end of the test.
2. Make sure we close child fds after the start.
3. Use newPipe for logs as well, for simplicity and uniformity.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Kir Kolyshkin