This commit rewrites and simplifies a lot of this logic to reduce it's
complexity, and also handle the case where the container doesn't have
it's own pid-namespace, which means that we're not guaranteed to receive
the init exit last.
This is achieved by replacing `s.pendingExecs` with `s.runningExecs`,
for which both (previously) pending and de facto running execs are
considered.
The new exit handling logic can be summed up by:
- when we receive an init exit, stash it it in `s.containerInitExit`,
and if a container's init process has exited, refuse new execs.
- (if the container does not have it's own pidns) kill all running
processes (if the container has a private pid-namespace, then all
processes will be dead already).
- wait for the container's running exec count (which includes execs
which have been started but might still early exit) to get to 0.
- publish the stashed away init exit.
Signed-off-by: Laura Brehm <laurabrehm@hey.com>
The runc task state machine prevents execs from being created after the
init process has exited, but there are no guards against starting a
created exec after the init process has exited. That leaves a small
window for starting an exec to race our handling of the init process
exiting. Normally this is not an issue in practice: the kernel will
atomically kill all processes in a PID namespace when its "init" process
terminates, and will not allow new processes to fork(2) into the PID
namespace afterwards. Therefore the racing exec is guaranteed by the
kernel to not be running after the init process terminates. On the other
hand, when the container does not have a private PID namespace (i.e. the
container's init process is not the "init" process of the container's
PID namespace), the kernel does not automatically kill other container
processes on init exit and will happily allow runc to start an exec
process at any time. It is the runc shim's responsibility to clean up
the container when the init process exits in this situation by killing
all the container's remaining processes. Block execs from being started
after the container's init process has exited to prevent the processes
from leaking, and to avoid violating the task service's assumption that
an exec can be running iff the init process is also running.
Signed-off-by: Cory Snider <csnider@mirantis.com>
It's not true that `s.mu` needs to be held when calling
`handleProcessExit`, and indeed hasn't been the case for a
while – see 892dc54bd2.
Signed-off-by: Laura Brehm <laurabrehm@hey.com>
The main reason is to improve the comment about pidfd in Go 1.23+.
While at it:
- avoid slice manipulation as we only need count;
- avoid repeating "/proc/self/fd".
Updates: #10345.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Update the local content plugin to register itself in a consistent way
as other plugins. This also allows the separate package to define its
own configuration more cleanly.
Signed-off-by: Derek McGowan <derek@mcg.dev>
Commit 8437c567d8 migrated the use of the
userns package to the github.com/moby/sys/user module.
After further discussion with maintainers, it was decided to move the
userns package to a separate module, as it has no direct relation with
"user" operations (other than having "user" in its name).
This patch migrates our code to use the new module.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
When running the test on Ubuntu focal (kernel version 5.4), the
symlink for pidfd is anon_inode:[pidfd].
Updates: #10345
Signed-off-by: Shengjing Zhu <zhsj@debian.org>
Old shims do not implement containerd.task.v3.Task, but it can be
useful to use a new ctr with an older shim especially during upgrade
scenarios.
Signed-off-by: Samuel Karp <samuelkarp@google.com>
The v2 shim interface supports grouping, so a single shim can manage
multiple tasks. Prior to this change, the `shim state` command could
only query the state of the primary task (task that shares the same ID
as the shim).
Signed-off-by: Samuel Karp <samuelkarp@google.com>
There are a couple directories that get created under the default
state directory ("/run/containerd") even when containerd is configured
to use a different location for its state directory. Create the default
state directory even if containerd is configured to use a different
state directory location. This ensure pkg/shim and pkg/fifo won't create
the default state directory with incorrect permissions when calling
os.MkdirAll for their respective subdirectories.
Signed-off-by: Erikson Tung <etung@netflix.com>
The userns package in libcontainer was integrated into the moby/sys/user
module at commit [3778ae603c706494fd1e2c2faf83b406e38d687d][1].
This patch deprecates the containerd fork of that package, and adds it as
an alias for the moby/sys/user/userns package.
[1]: 3778ae603c
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This PR ignores a new pidfd file descriptor that is introduced in
gotip (future 1.23) and should not be considered when detecting fd leaks.
Fixes#10345
Signed-off-by: Mauri de Souza Meneguzzo <mauri870@gmail.com>
There's a couple spots where we know exactly how large
the destination buffer should be, so pre-size these to
avoid any reallocs to a higher capacity.
Signed-off-by: Danny Canter <danny@dcantah.dev>
Unfortunately, this is a rather large diff, but perhaps worth a one-time
"rip off the bandaid" for v2. This patch removes the use of "gocontext"
as alias for stdLib's "context", and uses "cliContext" for uses of
cli.context.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Since Go 1.20, math/rand does not need explicit seeding:
https://go.dev/doc/go1.20#minor_library_changes
Go <= 1.19 is no longer supported due to EOL.
Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
ctr currently silently ignores several flags by default (without --local) and
the user can't know which flags are supported until they see the code.
This commit fixes ctr to return an explicit error when it finds an unsupported
flag.
Signed-off-by: Kohei Tokunaga <ktokunaga.mail@gmail.com>
This commit gets rid of the TODO by moving the check to use the
pluginInfo() infrastructure.
The check is only enforced for shims that return info that can be read
as type runtime.Features. For shims that don't provide that, we just
ignore it, as those shims might not be affected by this.
Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
Allow the api to stay at the same v1 go package name and keep using a
1.x version number. This indicates the API is still at 1.x and allows
sharing proto types with containerd 1.6 and 1.7 releases.
Signed-off-by: Derek McGowan <derek@mcg.dev>
This includes migrating from cdi.GetRegistry() to cdi.Configure() and
using top-level cdi Refresh and InjectDevices functions as applicable.
Signed-off-by: Evan Lezar <elezar@nvidia.com>
Currenlty transfer service doesn't handle plain HTTP connection.
This commit fixes this issue by propagating
`(core/remotes/docker/config).HostOptions.DefaultScheme` from client to the
transfer service.
This commit also fixes ctr to use this feature for "--plain-http" flag.
Signed-off-by: Kohei Tokunaga <ktokunaga.mail@gmail.com>
Set 'DisableSliceFlagSeparator = true'
urfave/cli/v2 uses ',' as default string slice separator.
That means '--mount type=bind,src=/src,des=/des,options=rbind:rw'
will be token as four bind mount options.
Fixes: #10003
Signed-off-by: baijia <baijia.wr@antgroup.com>
In order to make sure that we don't publish task exit events for init
processes before we do for execs in that container, we added logic to
`processExits` in 892dc54bd2 to skip these
and let the pending exec's `handleStarted` closure process them.
However, the conditional logic in `processExits` added was faulty - we
should only defer processing of exit events related to init processes,
not other execs. Due to this missing condition,
892dc54bd2 introduced a bug where, if
there are many concurrent execs for the same container/init pid, exec
exits are skipped and then never published, resulting in hanging
clients.
This commit adds the missing logic to `processExits`.
Signed-off-by: Laura Brehm <laurabrehm@hey.com>
