Skip to content

Shim mount handler protocol - #14002

Merged
fuweid merged 12 commits into
containerd:mainfrom
dmcgowan:shim-mount-handler-protocol
Aug 26, 2026
Merged

Shim mount handler protocol#14002
fuweid merged 12 commits into
containerd:mainfrom
dmcgowan:shim-mount-handler-protocol

Conversation

@dmcgowan

@dmcgowan dmcgowan commented Aug 21, 2026

Copy link
Copy Markdown
Member

Replaces the shim mount-handling negotiation mechanism — a comma-separated
string in RuntimeInfo.Annotations["containerd.io/runtime-allow-mounts"],
discovered via a separate <shim> -info exec — with a typed extension
attached to the shim's BootstrapResult.

Protocol

message BootstrapResult {
  ...
  repeated Extension extensions = 6;   // new
}

message MountCapabilities {
  repeated string types = 1;      // e.g. "erofs", "loop"
  repeated string transforms = 2; // e.g. "format", "mkfs", "mkdir"
}

A shim attaches a containerd.types.MountCapabilities extension naming the
mount types and transforms it performs itself. containerd ignores an
extension whose type it doesn't recognize, so this and future extensions are
backward compatible with no version handshake. The proto change is additive
(buf breaking clean).

Presence of the extension is authoritative and sufficient on its own — there
is no separate enum/flag gating it. With no other consumer, a flag would only
ever duplicate what the extension's presence already says, while still
permitting states with no single correct interpretation (flag set with no
extension, or the reverse).

Deprecated: containerd.io/runtime-allow-mounts

The mechanism itself — loadShimInfo, shimInfo, the shimInfos cache, and
the runc.v2/runhcs.v1 shortcut — is removed. getRuntimeInfo itself stays,
since it's also used by PluginInfo and validateRuntimeFeatures.

It's not gone for shims that still rely on it, though: Kata's EROFS
snapshotter shim adopted this annotation before the extension existed
(kata-containers#12763),
so removing it outright would break existing deployments with no migration
window. task_mounts_deprecated.go confines a fallback to one file: when a
shim's bootstrap result carries no MountCapabilities extension, its runtime
name is checked against the annotation, same as before, cached per runtime
name for the daemon's lifetime. io.containerd.runc.v2 and
io.containerd.runhcs.v1 are skipped outright, as they were before. The
extension stays authoritative whenever a shim provides it — this only runs in
its absence, and can be deleted outright once known early adopters migrate.

The "<transform>/*" mount-type spelling used by the annotation is not a
general WithAllowMountType feature any more (e.g.
WithAllowMountType("format/*") no longer means anything special); parsing
it is confined to the deprecated fallback, which is the only remaining
producer of that spelling. WithAllowTransform is how a shim expresses that
directly.

Timing change

TaskManager.Create now starts the shim before activating the task's
rootfs mounts, so its advertised extension is known before the daemon decides
what to activate. Starting a shim doesn't need the rootfs; only task.Create
does. A failure between shim start and task creation now tears the shim down
via a deferred cleanup reusing the existing cleanupShimTask helper.

Mount activation itself is extracted into a new taskMountController
(task_mounts.go) rather than growing further inline in Create, since its
state transitions (owned vs. reused activation, NotImplemented,
AlreadyExists) are independent of shim startup and this keeps them testable
without spawning a shim.

Also fixed

  • parseStartResponse decoded a shim's JSON bootstrap response into a
    deprecated 3-field struct, silently dropping everything else. This mattered
    once BootstrapResult could carry extensions: a container joining an
    existing sandbox shim recovers that shim's result from bootstrap.json
    rather than from a fresh start. Fixed by decoding into BootstrapResult
    directly — no change to what's written or how it's stored, encoding/json
    already round-trips every field including unresolvable extensions.
  • TaskManager.Create and Delete both called the mount manager directly
    with no nil check, panicking whenever the mount manager plugin isn't
    registered. taskMountController treats a nil manager as nothing to do.
  • BootstrapResult.FindExtension/BootstrapParams.FindExtension now use the
    generated nil-safe GetExtensions() getter instead of an explicit nil
    receiver check (fuweid).
  • A stale comment referencing the removed activateMounts function
    (fuweid).

Not included

  • BootstrapParams.capabilities (daemon → shim) — no consumer yet.
  • Retyping/removing anything on runc.v2's side — it has no real mount
    handling of its own on this branch, so it advertises nothing.
  • An in-tree producer of MountCapabilities for end-to-end testing. No
    in-tree shim ever set the annotation this replaces either (manager_linux.go
    hardcodes Annotations: nil), so this matches existing precedent. Follow-up:
    teach the failpoint shim to read MountCapabilities from a spec annotation,
    the same way its existing failpoints are configured
    (io.containerd.runtime.v2.shim.failpoint.*), and add an integration test
    asserting the daemon skips the claimed mounts.

Testing

  • Unit tests for extension parsing/translation, mount-option construction,
    bootstrap JSON round-trip (including an extension type the daemon can't
    resolve), taskMountController.Activate's state transitions with a fake
    mount.Manager, and the deprecated annotation fallback's caching,
    denylist, and parsing, with a fake RuntimeInfo querier (no real shim
    binary needed).
  • make check-protos, buf breaking, go vet ./..., full go test run
    including the api module.
**Deprecate "allow mount handlers" in shim info and replace with mount capability extension**

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR replaces the legacy shim mount-handling negotiation mechanism (runtime info annotation discovered via extra -info exec) with an additive capability + typed extension advertised in the shim BootstrapResult, and updates containerd’s mount-activation flow to consult the shim’s advertised capabilities before deciding what the mount manager should perform.

Changes:

  • Extends the bootstrap protocol with BootstrapResult.extensions and adds CAPABILITY_MOUNT_MANAGEMENT, plus a new containerd.types.MountCapabilities extension type.
  • Reorders TaskManager.Create to start the shim before rootfs activation and uses the shim’s BootstrapResult (or the deprecated annotation as fallback) to derive mount activation options.
  • Fixes bootstrap JSON parsing/round-tripping to preserve full BootstrapResult data (capabilities/extensions), with added unit/integration test coverage and updated docs.

Reviewed changes

Copilot reviewed 18 out of 28 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
vendor/modules.txt Vendors github.com/containerd/containerd/api via local ./api replacement.
vendor/github.com/containerd/containerd/api/types/mount.proto Vendors new MountCapabilities proto message.
vendor/github.com/containerd/containerd/api/types/mount.pb.go Regenerated vendor proto bindings for MountCapabilities.
vendor/github.com/containerd/containerd/api/runtime/bootstrap/v1/helpers.go Vendors helper methods for capabilities/extensions on bootstrap messages.
vendor/github.com/containerd/containerd/api/runtime/bootstrap/v1/bootstrap.proto Vendors bootstrap protocol additions (extensions, mount capability).
vendor/github.com/containerd/containerd/api/runtime/bootstrap/v1/bootstrap.pb.go Regenerated vendor bootstrap proto bindings.
vendor/github.com/containerd/containerd/api/LICENSE Removes vendored LICENSE file for the vendored api module.
integration/failpoint/cmd/containerd-shim-runc-fp-v1/main_linux.go Failpoint shim can advertise mount-management capability via env var for tests.
integration/failpoint/cmd/containerd-shim-runc-fp-v1/main_linux_test.go Unit tests for parsing the failpoint shim mount capability spec.
go.sum Removes sums for github.com/containerd/containerd/api due to local replace.
go.mod Adds replace github.com/containerd/containerd/api => ./api.
docs/shim-capabilities.md New doc describing capability registry and mount management capability.
docs/runtime-v2.md Documents how capabilities + extensions are used in bootstrap protocol.
docs/mounts.md Updates mount activation docs to prefer transforms and new shim capability path.
core/runtime/v2/task_manager.go Starts shim before mount activation; adds capability-based mount claiming + deprecation warning.
core/runtime/v2/task_manager_mounts_test.go Tests shim capability/extension vs annotation fallback behavior.
core/runtime/v2/shim.go Preserves full BootstrapResult (capabilities/extensions) and fixes JSON parsing to avoid dropping fields.
core/runtime/v2/shim_test.go Adds protobuf-safe equality + tests ensuring bootstrap round-trip preserves extensions.
core/runtime/v2/binary.go Persists shim bootstrap result in shim instance for later capability checks.
core/mount/manager/manager.go Adjusts transform-application logic to consult AllowTransforms and AllowMountTypes.
core/mount/manager.go Adds AllowTransforms and WithAllowTransform; normalizes legacy format/* into transforms.
core/mount/manager_test.go Unit tests for transform normalization and option behavior.
api/types/mount.proto Adds MountCapabilities message in the in-repo api module.
api/types/mount.pb.go Regenerated proto bindings for in-repo MountCapabilities.
api/runtime/bootstrap/v1/helpers.go Adds BootstrapResult helpers (capabilities/extensions).
api/runtime/bootstrap/v1/bootstrap.proto Adds extensions field and CAPABILITY_MOUNT_MANAGEMENT to the bootstrap protocol.
api/runtime/bootstrap/v1/bootstrap.pb.go Regenerated bindings for bootstrap protocol additions.
api/next.txtpb Updates API descriptor snapshot for the new messages/fields.
Files not reviewed (2)
  • api/runtime/bootstrap/v1/bootstrap.pb.go: Generated file
  • api/types/mount.pb.go: Generated file

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread api/runtime/bootstrap/v1/helpers.go Outdated
Comment thread core/mount/manager.go Outdated
Comment on lines 110 to 113
@@ -98,14 +111,34 @@ func WithLabels(labels map[string]string) ActivateOpt {
// of the mounts will support. Even if there is a custom handler
// registered for the mount type to the mount handler, these mounts
// should not performed unless required to support subsequent mounts.
Copilot AI review requested due to automatic review settings August 21, 2026 07:28
@dmcgowan
dmcgowan force-pushed the shim-mount-handler-protocol branch from f0a3b2b to c403ebd Compare August 21, 2026 07:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 19 out of 29 changed files in this pull request and generated no new comments.

Files not reviewed (2)
  • api/runtime/bootstrap/v1/bootstrap.pb.go: Generated file
  • api/types/mount.pb.go: Generated file
Suppressed comments (1)

go.mod:162

  • The new replace github.com/containerd/containerd/api => ./api causes go mod vendor to vendor the local ./api tree. In this PR that also results in vendor/github.com/containerd/containerd/api/ no longer containing a LICENSE file (it used to), while many other vendored modules do. Please ensure license/attribution expectations for the vendored github.com/containerd/containerd/api module are still met (e.g., by keeping a LICENSE/NOTICE file in the api/ module so it is preserved when vendored).
replace github.com/containerd/containerd/api => ./api

@fuweid

fuweid commented Aug 21, 2026

Copy link
Copy Markdown
Member

do we target this to v2.4?

@dmcgowan dmcgowan added this to the 2.4 milestone Aug 21, 2026
@dmcgowan dmcgowan moved this from Needs Triage to Needs Reviewers in Pull Request Review Aug 21, 2026
@dmcgowan
dmcgowan marked this pull request as ready for review August 21, 2026 20:45
@dmcgowan

Copy link
Copy Markdown
Member Author

@fuweid yes, I'd like to get this in for 2.4. We could make this a breaking change (low impact) by removing the runtime info check. I was never a fan of that and think its only used by Nerdbox now, which can switch to this after its merged.

@dmcgowan
dmcgowan requested review from fuweid and a balanced review from Copilot August 21, 2026 20:48
@dmcgowan
dmcgowan requested a review from mxpv August 21, 2026 20:50

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 19 out of 29 changed files in this pull request and generated 5 comments.

Files not reviewed (2)
  • api/runtime/bootstrap/v1/bootstrap.pb.go: Generated file
  • api/types/mount.pb.go: Generated file

Comment thread integration/failpoint/cmd/containerd-shim-runc-fp-v1/main_linux.go Outdated
Comment thread docs/mounts.md Outdated
`containerd.io/runtime-allow-mounts` annotation in their runtime info, a comma
separated list using the `"<transformer>/*"` convention for transformers,
which containerd translates the same way `WithAllowMountType` does. It is
honoured only when a shim advertises no capabilities.
Comment thread core/runtime/v2/task_manager.go
Comment thread core/mount/manager/manager.go Outdated
Comment thread core/mount/manager.go Outdated
Comment thread core/runtime/v2/task_manager.go Outdated
// shim instance, and does not require a separate invocation of the shim
// binary to discover. This annotation is still honoured for shims that do
// not advertise the capability.
allowedMounts = "containerd.io/runtime-allow-mounts"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious - should we drop this label in 2.4 ? I doubt there are external uses besides nerdbox.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but was thinking of doing that in a quick follow up. I can try and add it here though

@hsiangkao hsiangkao Aug 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious - should we drop this label in 2.4 ? I doubt there are external uses besides nerdbox.

kata is already using it for erofs snapshotter (https://github.com/kata-containers/kata-containers/pull/12763/files#diff-23c0f841997f951b150ed63026815477c1540196e579ce68afa68268e669edb0R32), including nvidia and msft use cases..

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mxpv @dmcgowan I hope to leave it for a while so that people can have time to migrate to the new way.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @dmcgowan we have enabled erofs snapshotter in kata for a while with the annotation of "containerd.io/runtime-allow-mounts",please see https://github.com/kata-containers/kata-containers/blob/7c6ba40c54ea29b459b4997cff948caaba4e658b/src/runtime-rs/crates/shim/src/bin/main.rs#L32 Is there a migration window for us to plan the transition. I believe it's definitly a breaking change for kata containers.
CC @fidencio

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is breaking but only for 2.4. I would rather consider adding special cases for kata (check allowed-mounts for specific runtimes) rather than keep the complicated logic of having it defined in two different parts of the flow.

Generally the bootstrap protocol is a bit of a breaking change already that we are trying to complete. My preference is to work with a couple early adopters to make it smooth rather than keep the logic we want to get rid of though. We already had a special case in there for runc (that this will get rid of), so having a temporary special case for kata isn't so bad (if kata and no extensions, check info, removed before 2.6 lts).

@dmcgowan dmcgowan Aug 23, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To clariify the logic cleanup, rather than always having an info call the is done before the first shim is started and cached for the life of the process, the information is part of the bootstrap negotiation. This allows the shim to return the result based on the running shim instance rather than a static set always defined for the shim, which could allow the shim to better decide mount types based on environmental conditionals such as rootless.

Not directly related to this change but this also allows us to more easily evolve the protocol so that the shim can own more of the runtime filesystem state. This is helpful for lazy loading which often makes the filesystem reliant on an external process for the life of a shim.

Comment thread core/runtime/v2/task_manager.go Outdated
Comment thread api/types/mount.proto Outdated
Copilot AI review requested due to automatic review settings August 22, 2026 07:49
@dmcgowan
dmcgowan force-pushed the shim-mount-handler-protocol branch from c403ebd to 5426b0f Compare August 22, 2026 07:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 19 out of 29 changed files in this pull request and generated 5 comments.

Files not reviewed (2)
  • api/runtime/bootstrap/v1/bootstrap.pb.go: Generated file
  • api/types/mount.pb.go: Generated file
Suppressed comments (2)

core/mount/manager/manager.go:159

  • This removes the previously documented WithAllowMountType("format/*") behavior, so existing API callers (and legacy annotation values) silently stop claiming transforms and containerd performs them on the host. WithAllowTransform can be added without breaking the wildcard contract by retaining the p+"/*" check.
		return !slices.Contains(config.AllowTransforms, p) && !slices.Contains(config.AllowMountTypes, t)

core/runtime/v2/task_manager.go:200

  • For the normal shared-sandbox path with opts.Address set, ShimManager.Start constructs a fresh BootstrapResult containing only version/protocol/address (shim_manager.go:232-236). Therefore this value has no extensions, and containers joining that sandbox never inherit the shim's mount handling—the exact reload scenario this change intends to support. Recover the sandbox's persisted bootstrap result (or otherwise propagate its advertised fields) even when an explicit endpoint is supplied.
	var bootstrap *bootapi.BootstrapResult
	if sc, ok := shim.(shimCapabilities); ok {
		bootstrap = sc.BootstrapResult()
	}

Comment thread api/runtime/bootstrap/v1/bootstrap.proto
Comment thread core/runtime/v2/task_mounts.go Outdated
Comment thread core/runtime/v2/task_manager.go Outdated
Comment thread docs/shim-capabilities.md Outdated
Comment thread docs/runtime-v2.md Outdated
Comment thread api/runtime/bootstrap/v1/helpers.go Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 24 out of 35 changed files in this pull request and generated 1 comment.

Files not reviewed (2)
  • api/runtime/bootstrap/v1/bootstrap.pb.go: Generated file
  • api/types/mount.pb.go: Generated file
Suppressed comments (1)

go.mod:161

  • This local replacement makes the new API compile only when this repository is the main module. Go ignores a dependency module's replace directives, so downstream users of github.com/containerd/containerd/v2 will still resolve the required containerd/api v1.12.0-beta.0, which does not contain MountCapabilities or BootstrapResult.Extensions, and compilation will fail. The root module needs to require a published API version (or pseudo-version) containing these changes and remove the local replacement before merge.
replace github.com/containerd/containerd/api => ./api

Comment thread core/mount/manager.go
Comment on lines +125 to +128
func WithAllowTransform(transform string) ActivateOpt {
return func(o *ActivateOptions) {
o.AllowTransforms = append(o.AllowTransforms, transform)
}
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>
A shim that performs some mount types or transformations itself, so that
containerd's mount manager does not perform them on its behalf, had no
way to say so other than a comma separated string in the generic
annotations map of its RuntimeInfo.

Add a containerd.types.MountCapabilities extension a shim can attach to
its BootstrapResult, naming the mount types and transforms it handles.
Detail lives in an extension rather than fields of BootstrapResult so
that containerd can ignore an extension whose type it does not
recognize, letting a shim advertise unconditionally.

Presence of the extension is enough: it is not gated behind a
Capability enum value, since with no other consumer of that field an
enum value here would only ever duplicate what the extension's
presence already says, while still permitting states with no single
correct interpretation, such as the enum value being set with no
extension attached, or the reverse.

This is additive: only a new field is added to BootstrapResult, and
buf breaking is clean.

Signed-off-by: Derek McGowan <derek@mcg.dev>
Pull in the mount capabilities extension.

The api module is consumed as a published version, so a local replace
is required to build against the unreleased proto changes. The replace
is added by 'make protos' whenever api/next.txtpb changes. It must be
dropped and go.mod pinned to a released api version before this is
merged.

This also moves the root module's own github.com/sirupsen/logrus
requirement from v1.10.1 to v1.10.2, which is not an intended dependency
bump. The published api module (v1.12.0-beta.0) requires logrus v1.9.3,
so it does not affect the root's resolution; the local api/go.mod on
upstream main already requires v1.10.2, and once the replace substitutes
it in, MVS takes the higher of the two. Verified by running 'go mod tidy'
on a copy of this tree with the replace directive removed: the root
stays at v1.10.1. This reverts on its own once the replace above is
dropped.

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 caller that applies a mount transform itself had to say so by naming the
mount type "format/*", a sentinel the mount manager constructs internally
by concatenating a transform name with "/*". Both sides had to
independently know and agree on that convention, and the resulting string
is neither a mount type nor a transform name.

Add AllowTransforms, and WithAllowTransform to set it, so a transform can
be claimed by its own name instead. The "/*" convention is removed rather
than kept as a second accepted spelling: it existed only to be produced by
the runtime-allow-mounts annotation, which is removed in an earlier commit
in this series, and there is no other producer of it in this tree.

Signed-off-by: Derek McGowan <derek@mcg.dev>
Activate's planning phase — deciding, for each mount, which transforms
the manager must apply and whether the manager or the caller performs
the mount itself — was inline in a bolt transaction spanning hundreds
of lines, and every existing test of it goes through the full Activate
path, which needs root to mount anything for real.

Extract it into planActivation, a pure function of the mount list,
activation options, and registered transforms/handlers. No behavior
change; this is what makes the next commit's fix testable without
root.

Signed-off-by: Derek McGowan <derek@mcg.dev>
A claimed transform was recorded but never actually left unapplied:
every parsed transformer in a mount's chain was applied in full
regardless of what the caller had claimed, so a shim advertising it
performs "mkdir" in "format/mkdir/overlay" never got the chance to,
since the manager silently applied it anyway. This predates the shim
capability protocol — it was reachable before through the "format/*"
annotation convention — but this series turns it into a documented,
supported way for a shim to claim a transform, so a shim relying on it
(at least one is: Kata's EROFS snapshotter shim, per its annotation
usage) would find its claim silently ignored.

A claimed transform can only be honored as a suffix of the chain: each
transform's input is the previous one's output, so an outer, unclaimed
transform must still be applied even when an inner one is claimed,
since the inner one cannot run without it. planActivation now tracks,
per mount, how much of its chain — starting from the beginning — the
manager must apply itself: through the last unclaimed transform, which
by construction covers every transform before it too. Applying only
that prefix and returning the true, contiguous claimed suffix to the
caller is what actually makes the claim meaningful. This is only
observable for mounts[firstSystemMount], the one mount partially
transformed by the manager and returned to the caller to finish; every
earlier mount is fully internal to the manager and was, and remains,
always fully transformed regardless of any claim.

While changing this expression, also guard it against the case where
firstSystemMount reaches len(mounts) — every mount consumed inside the
manager, with no system mount left to transform — which indexes out of
range today. This is a distinct, pre-existing latent panic (for
example, reachable via WithTemporary combined with a transformed mount
type), tracked separately from the claim-handling fix above; guarding
it here only prevents a crash newly reachable by touching this line,
it does not address the mount.ActivationInfo.System non-empty
invariant that scenario also violates.

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>
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#14002.

Signed-off-by: Derek McGowan <derek@mcg.dev>
Kata's EROFS snapshotter shim set containerd.io/runtime-allow-mounts
before the MountCapabilities bootstrap extension existed
(kata-containers/kata-containers#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>
Add a registry entry for the containerd.types.MountCapabilities extension,
and update the mount and bootstrap protocol docs for extensions replacing
the reserved-for-future-use capabilities field, and for the removal of the
runtime-allow-mounts annotation and the "/*" transform convention.

Signed-off-by: Derek McGowan <derek@mcg.dev>
Neither doc previously said how much of a claimed transform chain is
actually honored; a reader could assume claiming any transform in a
chain was sufficient on its own. State the suffix rule plainly, with
the format/mkdir/overlay example the code itself now enforces, and
call out that format in particular can only ever be claimed as part
of a suffix that covers what follows it, never alone, since it
resolves mount points internal to the mount manager.

Signed-off-by: Derek McGowan <derek@mcg.dev>
Copilot AI review requested due to automatic review settings August 26, 2026 18:35
@dmcgowan
dmcgowan force-pushed the shim-mount-handler-protocol branch from 17613af to fbd9f37 Compare August 26, 2026 18:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 24 out of 35 changed files in this pull request and generated no new comments.

Files not reviewed (2)
  • api/runtime/bootstrap/v1/bootstrap.pb.go: Generated file
  • api/types/mount.pb.go: Generated file
Suppressed comments (1)

go.mod:161

  • A local replace is only honored by the main module. Consumers importing github.com/containerd/containerd/v2 will ignore this directive and resolve the required containerd/api v1.12.0-beta.0, which does not contain MountCapabilities, BootstrapResult.Extensions, or the new helper methods used by this PR, so downstream builds will fail. Please depend on a published API version/pseudo-version containing these changes and remove the local replacement.
replace github.com/containerd/containerd/api => ./api

@fuweid
fuweid enabled auto-merge August 26, 2026 19:20
@fuweid
fuweid added this pull request to the merge queue Aug 26, 2026
Merged via the queue into containerd:main with commit 341a4e0 Aug 26, 2026
57 checks passed
@github-project-automation github-project-automation Bot moved this from Review In Progress to Done in Pull Request Review Aug 26, 2026
@dmcgowan
dmcgowan deleted the shim-mount-handler-protocol branch August 26, 2026 22:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Development

Successfully merging this pull request may close these issues.

6 participants