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>
When a layer fails to prepare during parallel unpack we break out of the
launch loop but never return the error, so unpack() can report success and
label the image with a chainID that was never created. Keep the error and
return it once the already queued layers have been drained, so those still
commit as they would in sequential mode. Also end the layer's tracing span,
which leaked on this path.
Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
Replace the snapshots.ErrAlreadyStaged sentinel error with a
read-only-mounts signal: a snapshotter (e.g. erofs serving a layer
content cache hit) now returns Prepare mounts normally, with no
error, when the layer content is already staged into the active
snapshot. The unpacker's isStaged helper checks the last mount's
ReadOnly() to decide whether to skip fetch+apply and just commit.
Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
The cache used to serve a hit by committing the layer during Prepare and
returning ErrAlreadyExists, which is incompatible with parallel unpacking:
the "rebase" capability defers the parent to Commit time, so a
commit-at-Prepare layer ends up parentless. The erofs snapshotter therefore
had to disable rebase whenever the cache was enabled, making every cache
*miss* fall back to a fully sequential (slower-than-cold) pull.
Introduce snapshots.ErrAlreadyStaged, returned from Prepare to mean "the
active snapshot's content is staged; skip the layer download and apply, but
still Commit it" (where the parent is applied). Unlike ErrAlreadyExists it
does not end the layer's lifecycle. The unpacker handles it by emitting a
status whose bottom half runs the normal serialized commit (rebasing the
parent in), and the metadata snapshotter threads it through Prepare like a
normal active snapshot. The erofs cache hit now stages the blob and returns
ErrAlreadyStaged instead of committing, so it can advertise "rebase"
unconditionally: hits skip download+conversion and misses stay parallel.
Also promote the "rebase" capability string to snapshots.RebaseCap (shared by
the unpacker, erofs, and overlay).
Follow up: fetch is range-based, so only a fully cached contiguous
prefix skips downloads; a miss in a lower layer still pulls everything above
it. Per-layer fetch-skip is left as a follow-up.
Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
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>
Add a --dmverity flag to `ctr images build-erofs-cache`. When set, each cached
erofs blob is dm-verity formatted (the hash tree is appended in place) and a
.dmverity sidecar is written alongside it. This is required when the erofs
snapshotter runs with dmverity_mode=on, which rejects cache hits that lack a
sidecar; without it such layers would have to be formatted out-of-band.
Extract the differ's dm-verity formatting into a shared dmverity.FormatLayer so
the differ and the cache builder share one implementation; the differ's
formatDmverityLayer now delegates to it.
Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
Add `ctr images build-erofs-cache <image> <cache_dir>`, which reads an
already-pulled image's layers from the content store, converts each into a
directly-mountable erofs blob, and writes them under
<cache_dir>/<algorithm>/<hex>.erofs keyed by the layer's diffID. This
populates the directory the erofs snapshotter's layer_content_cache reads
on pull; because the key is the source diffID, layers shared across images
converge on one blob. No converted image is produced.
Extract the per-layer uncompress + mkfs.erofs step out of LayerConvertFunc
into an exported ConvertLayerToErofs so the image converter and the cache
builder share a single conversion path.
Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
Add a layer_content_cache option to the erofs snapshotter: a directory of
pre-converted, diffID-keyed erofs layer blobs (in production a read-only
mountpoint-s3/FUSE mount) that the snapshotter sources layers from on pull
instead of downloading and converting them per node.
On an image-layer extraction Prepare, if the layer's blob is present in the
cache the snapshotter stages it as a symlink, commits it as the target
chainID in the same transaction, and returns ErrAlreadyExists. This uses the
existing remote-snapshot protocol, so containerd skips both the layer
download and the tar->erofs conversion; no core changes are needed. Any miss
(cache disabled, no snapshot.ref/diff-id labels, blob absent, unreadable
cache) falls through to the normal path, so pulls keep working.
Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.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>
Because selinux contexts can contain commas, context strings may be
quoted in the single-syscall mount API. This quoting is not permitted
when using the fsconfig API, so strip the quotes when preparing the
system call arguments.
Signed-off-by: Daniel De Graaf <dgdegra@uwe.nsa.gov>
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>
According to the OCI distribution spec registries may include
informational warnings in HTTP Warning headers:
- e612a6e1e1/spec.md (warnings)
- https://www.rfc-editor.org/rfc/rfc7234#section-5.5
This change implements support for handling these warnings and
propagating them to the resolver.
This patch adds a new, optional WarningHandler interface field to
ResolverOptions that allows callers to receive and process warnings sent
by registries via HTTP Warning headers with warn-code 299.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
When a registry returns 403 Forbidden on a HEAD request (e.g., manifest
resolve or push existence check), the diagnostic error body is lost
because HEAD responses carry no body per HTTP spec. This leaves users
with an opaque "403 Forbidden" message and no actionable guidance.
Add a follow-up GET on HEAD 403 to retrieve the registry's OCI error
body. The existing unexpectedResponseErr machinery already parses the
body into structured errors — it just needs the body to be present. The
fallback lives in a shared withGETErrorBody helper used by both the
pusher and resolver: it only enriches when the GET also returns 403, and
preserves the original HEAD request's method and status while borrowing
just the body, so the resulting error's status and body stay consistent.
Scoped to 403 only because it is rare (CMK key disabled, IP firewall,
RBAC misconfiguration) and its body is highly diagnostic, while other
status codes either already use GET or have bodies that add no value.
Fixes#8969
Signed-off-by: Andrew Au <cshung@gmail.com>
Disable stat usage since we never consume the data.
This can reduce unnecessary contention during transactions.
Signed-off-by: Wei Fu <fuweid89@gmail.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>
Extend the garbage-collection framework so a collectible resource can emit
forward references during graph traversal, in addition to the existing
back-reference mechanism.
A CollectionContext may now implement the optional collectionWithReferences
interface:
References(ctx context.Context, node gc.Node, fn func(gc.Node))
When the GC visits a node whose resource type was registered by an external
collector, gcContext.references consults the per-type References
implementation after the built-in core resource types are handled.
This is the forward-reference analogue of collectionWithBackRefs. Whereas
ActiveWithBackRefs must enumerate every edge up front and the gcContext
holds all of them in its backRefs map for the entire collection, References
is invoked on demand for a single node. A collector whose resources fan
out to many other nodes can therefore emit those edges without retaining
them in memory for the gc context.
This commit is intentionally a no-op: no plugin registers a collector that
uses collectionWithReferences yet. It is isolated here so that concurrent
development efforts that depend on this interface can be proposed and
reviewed upstream independently.
Signed-off-by: Derek McGowan <derek@mcg.dev>
Allow the last host to retry on transient network errors to incrase the
likelihood of the operation succeeding and help reduce flaky tests.
Signed-off-by: Derek McGowan <derek@mcg.dev>
TestImagesCreateUpdateDelete asserts that an image's updatedat is
strictly after its createdat. Both timestamps are stamped via
time.Now().UTC(), which strips the monotonic reading, so the comparison
falls back to the wall clock. On platforms with coarse timer resolution
(e.g. Windows, which advances system time at the ~15.6ms tick), the
Create and Update calls can land in the same tick and produce identical
timestamps, making the strict After() check fail intermittently.
Wait for the wall clock to advance past the creation timestamp before
updating so the assertion stays meaningful without depending on clock
resolution. On fine-resolution clocks the loop runs zero iterations.
Signed-off-by: Austin Vazquez <austin.vazquez@docker.com>
Some proxy stream setup and receive paths still returned raw RPC
status errors while neighboring proxy methods normalized them with
errgrpc.ToNative. This made errdefs checks depend on which proxy API
surfaced the same remote failure.
Normalize event subscription setup and receive errors, and streaming
stream creation errors, while preserving io.EOF for completed receive
streams.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Most content proxy operations normalize remote RPC errors before
returning them, including stream receive errors from Walk and write
errors from the remote writer. remoteReaderAt.ReadAt was an outlier and
returned raw status errors from Read and Recv.
Callers that use content.ReadBlob through the proxy can then fail
errdefs checks, such as treating concurrent content deletion as
NotFound.
Convert non-EOF read stream errors with errgrpc.ToNative so ReaderAt
matches the rest of the content proxy while preserving io.EOF.
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
The CRI progress reporter cancels an image pull if it sees no progress
for 5 seconds. It tracks this through active HTTP requests. During
remote fetches, the HTTP response reader is closed via a deferred
call after `content.Copy` completes.
Diagnosis:
`content.Copy` handles both downloading the stream and committing
the writer to the content store. Any delays during the database
commit phase (e.g. from database locks, slow disk syncs, or concurrent
pull deduplication blocks) keep the HTTP connection open. The progress
reporter sees the request is still active (`activeReqs = 1`) but no new
bytes are coming in, leading to a premature timeout cancellation.
Reproduction:
We reproduced this flakiness deterministically on a GCE VM under a
simulated 2 Mbps ingress bandwidth limit using Linux traffic control
ingress policing (`tc filter ... action police rate 2mbit`). Under this
slowness, the download took longer than the progress timeout during the
slow commit phase, triggering context cancellation and failing the
`TestCRIImagePullTimeout/HoldingContentOpenWriterWithLocalPull` test.
Solution:
To fix this, we wrap the HTTP reader in a `closeOnEOFReader` or
`closeOnEOFReadSeeker` before handing it to `content.Copy`. If the
underlying connection reader implements `io.Seeker`, it is dynamically
wrapped in `closeOnEOFReadSeeker` to forward `Seek` operations. This
ensures that O(1) Range seeks are fully preserved during network
resumes or retries. The wrappers automatically close the underlying
network stream as soon as `Read()` returns `io.EOF` (when the download
completes, before the database commit begins). This drops `activeReqs`
to `0` early, freeing the socket and preventing progress timeouts
during commits. A `sync.Once` ensures that subsequent deferred
`Close()` calls do not double-decrement the reporter.
How it was tested:
Verified the fix on a GCE VM under a simulated 2 Mbps ingress
bandwidth limit. Verified seeker safety via standalone logic audits
and trace proofs.
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>
Setting the GC labels ensures that extra references may get garbage
collected when the original image using them is removed.
Signed-off-by: Derek McGowan <derek@mcg.dev>
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>
Extrareferences may have the prefix flag with the digest added.
Currently they are not being processed. The option today which sets the
digest ref will set both prefix and add digest flags.
Signed-off-by: Derek McGowan <derek@mcg.dev>
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>
Kernel 6.12.80+ returns 'fsync=volatile' instead of just 'volatile'
in mount options, which breaks containerd's exact string matching
checks.
Fixes this issue by adding support for 'fsync=volatile' in addition
to the existing 'volatile' check in RemoveVolatileOption and
addVolatileOptionOnImageVolumeMount.
Assisted-by: Antigravity
Signed-off-by: Chris Henzie <chrishenzie@gmail.com>
