oci: Attribute pull errors to the failing blob - #382
Conversation
cgwalters
left a comment
cgwalters
left a comment
There was a problem hiding this comment.
In the past a problem we've had is duplicating this information; at one point I believe we had an error path that output the image digest three times which was crazy verbose.
It's complicated to analyze.
|
Concretely, a failed layer pull now renders as: so each digest appears once, at the level it names. It also comes out shorter than what's there today: the current outer context attaches the config digest to every failure, layer failures included (the misattribution this fixes), and the |
There was a problem hiding this comment.
Pull request overview
Improves OCI import error attribution by identifying the manifest, config, or layer involved.
Changes:
- Adds digest-specific context to config fetch and layer import failures.
- Rewords outer errors as manifest-level operations.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
crates/composefs-oci/src/skopeo.rs |
Improves remote pull error context. |
crates/composefs-oci/src/oci_layout.rs |
Improves local OCI layout import context. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
1deed6c to
fb98717
Compare
A failure anywhere in ensure_config_with_layers() (the config fetch or
any concurrent layer import) was reported as "Failed to pull config
{descriptor}", blaming a config blob that is often intact. The OCI
layout path had the same shape via import_config_and_layers(), reported
as "Failed to import config".
Wrap each layer import with the layer's own digest, keep config context
on the config fetch alone, and reword both outer wraps to describe the
manifest-level operation they actually cover. The config-fetch wrap
also covers get_descriptor(), whose errors previously carried no
context at all. Debugging bootc-dev/bootc#2408, the config-blaming
message cost an hour of registry forensics on a blob that was
byte-perfect; composefs#99 reports the same confusion from a different trigger.
Generated-by: AI
Signed-off-by: Andrew Dunn <andrew@dunn.dev>
fb98717 to
0e5df99
Compare
|
The only red here is Integration tests (centos-stream9), from a run that predates your approval, and it's the debian_bootc flake again; stream10 passed in the same run. Auto-merge won't fire while it's red and I can't re-trigger from a fork. Could you re-run that job? Happy to rebase onto main instead if you'd rather a fresh run, #384 is in there now. |
A failure anywhere in
ensure_config_with_layers()(the config fetch or any concurrent layer import) was reported as "Failed to pull config {descriptor}", blaming a config blob that is often intact. Theoci:layout path has the same shape viaimport_config_and_layers(), reported as "Failed to import config". Debugging bootc-dev/bootc#2408 the config-blaming message cost an hour of registry forensics on a config blob that was byte-perfect; #99 is the same confusion from a different trigger.Both paths now wrap each layer import with the failing layer's own digest, keep config context on the config fetch alone (which also gives
get_descriptor()errors context they previously lacked), and reword the outer wraps to describe the manifest-level operation they actually cover. Demonstrated against a real zstd:chunked image, whose layers currently fail to decode. Before: "Failed to import config sha256:637d...: unexpected EOF reading tar entry". After: "Failed to import image content for manifest sha256:a516...", then "Failed to import layer sha256:791d...", then "unexpected EOF reading tar entry".This is a context-only change with no behavior difference on success paths. There is no existing test seam for either pull path, so verification is clippy plus the full workspace suite plus the live before/after above.
Generated-by: AI
I hit and root-caused the misattribution on my own host and reviewed the change carefully.