Creating symlinks on Windows requires administrator privileges or
Developer Mode, so NewBundle fails with "A required privilege is not
held by the client" when containerd creates a task without them.
Replace the direct os.Symlink and os.Readlink calls with platform
specific helpers. On non-Windows platforms they keep the existing
symlink behaviour. On Windows, createWorkLink creates a directory
junction using go-winio's reparse point encoding, which does not need
elevated privileges. os.Readlink resolves both symlinks and junctions,
so resolveWorkLink uses it on both platforms.
createWorkLink closes the directory handle before removing the
half-created junction directory on failure. The handle is opened
without sharing, so removing the directory while it is still open would
fail.
Signed-off-by: Craig Gumbley <craiggumbley@gmail.com>
Signed-off-by: Eric Curtin <eric.curtin@docker.com>
Assisted-by: OpenCode (anthropic/claude-sonnet-5-5)
This reverts commit f078cebbd1.
Propagating State deadline errors causes Get to fail and List to omit
tasks when a shim is temporarily unresponsive, such as during slow
cleanup in Delete. A timeout does not mean the task no longer exists,
and omitting it makes container lifecycle management harder for clients
and CRI.
Restore the previous behavior of returning UNKNOWN on State timeouts,
so tasks remain visible and callers can reconcile their state.
Refs: https://github.com/containerd/containerd/pull/14380#pullrequestreview-5689176711
Signed-off-by: Wei Fu <fuweid89@gmail.com>
To make task.Delete retriable, this patch caches the delete result in
memory after the shim successfully deletes the task. If a later shutdown
attempt fails, a subsequent Delete retry can complete cleanup and return the
cached exit information.
If containerd restarts after deleting the task, the in-memory cache is lost,
and containerd will publish TaskExit and TaskDelete events again to
subscribers. Currently, the exit status returned by shim-delete is always
137. In a follow-up, containerd-shim-runc-v2 should store the actual exit
status in the state directory, so that shim-delete can reuse it instead of
returning a hard-coded value.
Implements: #8981
Signed-off-by: Wei Fu <fuweid89@gmail.com>
Kata's EROFS snapshotter shim set containerd.io/runtime-allow-mounts
before the MountCapabilities bootstrap extension existed
(https://github.com/kata-containers/kata-containers/pull/12763), so
removing the annotation outright would break it and anyone else who
adopted it early with no migration window.
Rather than keep the annotation as a first-class, permanently
supported mechanism, confine falling back to it in
task_mounts_deprecated.go: when a shim's bootstrap result carries no
MountCapabilities extension, its runtime name is checked against the
annotation as a migration path, with the result cached per runtime
name for the life of the daemon, same as the removed mechanism did.
Two runtimes known to never set it, runc.v2 and runhcs.v1, are
skipped outright rather than queried.
The extension stays authoritative whenever a shim provides it: this
fallback only runs in its absence, and is confined to one file so it
can be deleted outright once known early adopters have migrated.
Signed-off-by: Derek McGowan <derek@mcg.dev>
When a container joins an existing sandbox via a Task API address supplied
directly by the sandbox controller (rather than by restoring
bootstrap.json), ShimManager.Start synthesized a BootstrapResult carrying
only the version, protocol and address. A shim that advertised
MountCapabilities at sandbox startup was therefore treated as if it had
advertised nothing for every container that joins the sandbox this way,
silently ignoring mount handling it explicitly claimed.
Recover the extensions from the sandbox's shim instance, which containerd
already holds in memory, via the existing shimCapabilities interface. This
is best effort: an external sandboxer whose shim instance containerd does
not track degrades to no extensions rather than failing sandbox startup,
matching today's behavior.
Reported independently by fuweid and Copilot on containerd/containerd#14002.
Signed-off-by: Derek McGowan <derek@mcg.dev>
A shim declared the mount types it performs itself through the annotation
removed in an earlier commit, discovered by invoking the shim binary a
second time with -info. Read them instead from the MountCapabilities
extension a shim advertises in its bootstrap result, which is already
available from the shim start that Create() performs anyway.
This requires starting the shim before activating the task's mounts.
Starting a shim does not need the rootfs; only the task.Create call
consumes it. Reordering means a failure after the shim has started must
now tear the shim down, so the teardown that was inlined in the
task.Create error path becomes a deferred cleanup covering the whole
window, reusing the existing cleanupShimTask helper. That defer is
registered after the mount deactivation defer so that the shim, which
may still be using the mounts, is stopped first.
Extract the mount activation logic into a new taskMountController in
task_mounts.go, rather than growing it further inline in Create(): it
already has its own state transitions (owned vs. reused activation,
NotImplemented, AlreadyExists) independent of shim startup, and this
keeps that reasoning testable without spawning a shim.
Also fix two latent nil pointer dereferences: Create() and Delete() both
called the mount.Manager interface directly with no nil check, which
panics whenever the mount manager plugin is not registered.
taskMountController.Activate and Deactivate treat a nil manager as
nothing to do.
Signed-off-by: Derek McGowan <derek@mcg.dev>
parseStartResponse decodes a JSON bootstrap response into the deprecated
three-field client.BootstrapParams, so every other field of BootstrapResult
is silently discarded.
This is reached when reloading a bundle: writeBootstrapParams stores
bootstrap.json with encoding/json, and readBootstrapParams parses it back
through here. It matters once BootstrapResult carries extensions, because a
container joining an existing sandbox shim recovers that shim's result from
bootstrap.json rather than from a fresh shim start, and so would have seen
none of them.
Decode into BootstrapResult instead. The stored format does not change and
neither reader nor writer changes what it produces; encoding/json already
round-trips every field, including extensions, whose Any is an opaque type
URL and byte slice to it and so needs no type resolution, and including a
capability value this version of containerd does not define, which must
survive rather than being silently dropped.
This also removes the last use of the deprecated client.BootstrapParams.
Also fix TestRestoreBootstrapParams, which compared protobuf messages with
reflect-based deep equality. That is not sound, as generated types carry
internal state that varies with how the message was built.
Signed-off-by: Derek McGowan <derek@mcg.dev>
A shim declared the mount types it performs itself through a comma
separated string in the generic annotations map of its RuntimeInfo,
discovered by invoking the shim binary a second time with -info. The
values had to reproduce by hand the "<transform>/*" sentinel the mount
manager constructs internally, there was nowhere to put anything but a
name, and it was keyed by runtime name rather than shim instance, so it
could not reflect anything about the environment a particular shim is
running in.
Remove it along with loadShimInfo, shimInfo and the shimInfos cache,
and the runc.v2/runhcs.v1 shortcut that only existed to avoid paying
for it on the default runtimes. This is a breaking change targeted at
2.4; known usage is limited to Nerdbox, which can adopt the shim
capability protocol added in the following commits after this merges.
getRuntimeInfo itself stays: it is also used by PluginInfo and
validateRuntimeFeatures.
Signed-off-by: Derek McGowan <derek@mcg.dev>
TaskService.Shutdown may be invoked multiple times. A grouped shim
itself does not terminate while it still has active tasks. It terminates only
after receiving a Shutdown request when no active tasks remain.
The shim manager should invoke TaskService.Shutdown after deleting every task,
regardless of whether the task represents sandbox or not. This ensures that the
shim receives Shutdown when the last active task is deleted.
In CRI, the pause container uses oom_score_adj=-998, making it unlikely to be
OOM-killed. However, it may still exit or be killed before the other containers
in the sandbox. SandboxService.Shutdown and TaskService.Shutdown are independent
APIs. Sandbox service should make sure there is no active containers
before shutdown. So, for the task service, we should invoke Shutdown for
each task.
Removing the supportSandboxAPIVersion check does not change behavior for shims
using task API versions below v3. Those shims were already forced down the
non-sandboxed path and received Shutdown after every task deletion. The behavior
change applies only to sandboxed tasks using task API v3, which now receive
Shutdown after deletion as well.
Signed-off-by: Wei Fu <fuweid89@gmail.com>
loadShim runs during runtime plugin initialization and issued its task
RPCs on the caller's context, which carries no deadline. A shim that
still owns its socket but has stopped serving, for example one leaked by
a previous containerd whose shutdown timed out, accepts the connection
and then never answers. containerd then never finishes starting and its
API socket is never created.
Give loadShim a single loadTimeout budget covering the whole load. Nested
timeouts can only shorten a deadline, so the load stays bounded however
many calls it makes.
Reaping a shim we give up on runs on its own cleanup budget, detached
from the caller so that a cancelled load cannot leave the shim behind. A
failed delete returns before shutting the shim down and closing its
client, so both are done here, and the failure is now propagated: delete
removes the bundle only when it succeeds, and returning an error lets
loadShims remove it, so a shim that cannot be reaped is no longer
reloaded on every subsequent start.
Co-authored-by: Harshal Patel <106813066+HarshalPatel1972@users.noreply.github.com>
Signed-off-by: Austin Vazquez <austin.vazquez@docker.com>
When a shim delete hits a timeout, currently the error message does not
indicate that the delete was killed rather than failed to complete.
Signed-off-by: Derek McGowan <derek@mcg.dev>
The shim imported github.com/checkpoint-restore/checkpointctl/lib for a
single string constant.
Consumers that embed only the runtime v2 plugin (and never touch
internal/cri) therefore had to vendor the checkpointctl package.
Declare the file name as a local constant instead.
checkpointctl still remains a direct requirement here because
internal/cri/server uses its types, JSON readers, and annotations, but
it is no longer reachable from the runtime v2 import graph.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Right now the statement is treating any error from the shim as leaking
(len(pInfo == 0 is true for any error). It seems the intent was to only
treat shims this way if the error was not found, or if there's
legitimately no pids associated with the shim. Let's fix that up to
avoid orphaning shims that just had a small error in reading pids.
Link: https://github.com/containerd/containerd/issues/13784
Signed-off-by: Andrew Halaney <ahalaney@netflix.com>
Shim start output was trimmed before protobuf decoding.
Because arbitrary protobuf fields may legitimately end with whitespace
bytes, including `\n`, trimming could corrupt metadata or capabilities
and cause decoding to fail.
Pass the raw command output to `parseStartResponse` and attempt protobuf
decoding before modifying the response.
Whitespace trimming now applies only to legacy JSON and plain-address
responses.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Perform an early validation check on both container checkpoint and
restore paths to fail-fast if the CRIU binary is missing or is older
than the minimum required version (3.16.0).
To support runtime-configured environments, the validation respects the
custom PATH from the shim manager environment if configured, skipping
any relative paths to avoid incorrect daemon-relative resolution. If not
configured, it falls back to a standard system PATH lookup. The check
result is cached using sync.Once to prevent redundant process spawning.
Assisted-by: Antigravity
Signed-off-by: Samuel Karp <samuelkarp@google.com>
Read short-circuited on `if dpc.c == nil` before calling
`dpc.wg.Wait()` which races with the dialer goroutine spawned in
openShimLog. The dialer assigns `dpc.c = c` (and may set `dpc.conerr`)
outside any lock; the only synchronization is the WaitGroup, and Read
skipped it on the fast path.
Signed-off-by: Austin Vazquez <austin.vazquez@docker.com>
Switch the CRI integration layer from containerd's forked Kubernetes helpers
and clients to the upstream Kubernetes modules, and finalize the dependency
update to Kubernetes v0.36.0.
Replace the remaining internal helper copies with upstream packages:
- internal/cri/clock -> k8s.io/utils/clock
- internal/cri/executil -> upstream CRI exec helpers
- internal/cri/resourcequantity -> k8s.io/apimachinery/pkg/api/resource
- internal/cri/setutils -> k8s.io/apimachinery/pkg/util/sets
- internal/cri/types/labels.go -> internal/cri/labels
- integration/cri-api/pkg/apis/services.go -> k8s.io/cri-api/pkg/apis/services.go
Adopt the upstream CRI clients directly:
- add k8s.io/cri-client v0.36.0, k8s.io/cri-streaming v0.36.0, and
k8s.io/streaming v0.36.0 as direct dependencies
- promote k8s.io/utils to a direct dependency and pull in
k8s.io/component-base v0.36.0 indirectly
- keep integration/remote as a thin containerd adapter around cri-client,
because the integration tests still need the stream-shaped
GetContainerEvents RPC
Finalize the Kubernetes dependency update from v0.36.0-rc.0 to v0.36.0,
refresh vendor/, and drop the obsolete internal utility copies.
Also fix the protobuf MessageState mutex-copy vet failures exposed by the new
APIs and close the temporary integration CRI clients explicitly.
Signed-off-by: Davanum Srinivas <davanum@gmail.com>
Correctly handle cases where the mount activation still exists:
- If activation is fully activate, then just return already exists and
allow the caller to return error or call Info to continue.
- If activation is stale or incomplete due to crash during activation,
overwrite the identifier and cleanup the incomplete activation during
activate.
Signed-off-by: Derek McGowan <derek@mcg.dev>
Allow the socket directory to be directly configured by the shim manager
with reasonable defaults when not set. The default for root users will
still be the same directory under the default state directory. For
non-root users a temp directory will be used as default if the state
directory is not owned by the user.
Signed-off-by: Derek McGowan <derek@mcg.dev>
Send the socket directory from containerd to the shim. The shim still
decides where the socket goes but can use the environment variable
passed from containerd to ensure the socket is placed in the configured
directory with proper permission.
This is needed for some rootless cases which do not have permission to
the default state directory as currently set. The directory being
hardcoded by the shim means it is currently not possible to change the
location the shim will listen at.
Signed-off-by: Derek McGowan <derek@mcg.dev>
- Use time.NewTimer + Stop() instead of time.After to avoid timer leaks
- Treat context.DeadlineExceeded as retryable (pipe busy, not just missing)
- Wrap last dial error instead of os.ErrNotExist for better diagnostics
- Update makeConnection godoc to reflect current BootstrapResult type
Signed-off-by: Esteban Ginez <esteban.ginez@docker.com>
The shim "start" helper returns the named pipe address before the
daemon process has created the pipe via winio.ListenPipe(). On busy
Windows systems, containerd may try to connect before the pipe exists.
Add awaitPipeReady() — the start helper now polls the pipe address
(up to 5s, 10ms intervals) before writing the bootstrap result to
stdout. This follows hcsshim's readiness pattern where the shim
verifies its endpoint is ready before signaling the parent.
As a safety net, also parameterize makeConnection() with a dialer so
binary.Start() uses AnonDialer (retry) for new shims while loadShim()
keeps AnonReconnectDialer (fail-fast) for reconnects per #3659.
On Unix, awaitPipeReady() is a no-op: domain sockets appear atomically.
Signed-off-by: Esteban Ginez <esteban.ginez@docker.com>
When running hostNetwork containers with the shim sandboxer, the spec
passed to NewBundle is never initialized to a typed nil and is instead a
bare golang nil. Fix this by guarding the dereference to avoid crashing
when such a container is created.
Signed-off-by: Fletcher Woodruff <fwood@amazon.com>
When a shim becomes unresponsive (e.g., stopped via SIGSTOP), ttrpc
communication times out with `context deadline exceeded`.
Currently, this error is not properly propagated, causing redundant API
calls and slow container listing by client sides.
Specifically, when executing the API to check the task state, it appears
that the `context deadline exceeded` error via ttrpc is not being handled
within `shimTask.State()` and `getProcessState()`.
As a result, when this error occurs, clients such as nerdctl cannot
recognize this error, and it is thought that the issue described below is
occurring:
- https://github.com/containerd/nerdctl/issues/4720
Therefore, this commit adds error handling to ensure timeouts are properly
handled by client sides.
Signed-off-by: Hayato Kiwata <dev@haytok.jp>
Craig Gumbley