Skip to content

Add support for --from-delta for bootc switch and update - #2449

Open
alexlarsson wants to merge 6 commits into
bootc-dev:mainfrom
alexlarsson:from-delta
Open

alexlarsson wants to merge 6 commits into
bootc-dev:mainfrom
alexlarsson:from-delta

Conversation

@alexlarsson

Copy link
Copy Markdown
Contributor

This adds native support for oci-delta files to bootc, for both the native composefs backend and the ostree backend.

The composefs backend just uses the existing support in composefs-rs, whereas the ostree backend has a bit more custom code. This code uses the generic delta code in composefs-rs to avoid having to reimplement delta stuff as we're applying deltas directly. This depends on composefs/composefs-rs#394 and means we have a WIP commit that points to this MR. The intent is to get some release with that out before merging this PR.

Note: I have not really tested the native composefs codepath, because I didn't manage to get such a setup going. I will try to do this.

@github-actions github-actions Bot added area/install Issues related to `bootc install` area/ostree Issues related to ostree area/documentation Updates to the documentation labels Sep 10, 2026
@bootc-bot
bootc-bot Bot requested a review from cgwalters September 10, 2026 11:50
The diff_id appended for the generated layer was the bare hex encoding
rather than the `sha256:...` form the spec requires. Our own importer
does not look at it, so nothing noticed, but any other consumer of an
image built by this helper trips over it.

Assisted-by: AI
Signed-off-by: Alexander Larsson <alexl@redhat.com>
@alexlarsson

Copy link
Copy Markdown
Contributor Author

New version that is not depending on the new composefs-rs, instead using the separated out delta parsing from the rust code in containers/oci-delta.

This adds a `--from-delta PATH` flag to `bootc upgrade` and `bootc
switch` that applies an oci-delta in one step. This commit add support
only for the composefs backend, and uses the native delta pull
support in composefs-rs. Later commits add ostree backend support.

Basic parsing of the delta file is done by the oci-delta rust bindings.

See https://github.com/containers/oci-delta for details of the format.

Unified storage cannot apply a delta yet and will fail. Real support
for this would require delta support in containers/storage.

Generated-by: AI
Signed-off-by: Alexander Larsson <alexl@redhat.com>
The importer has always fetched both the image metadata and the layer
bytes over the same containers-image-proxy connection. To apply an OCI
delta we need to split those apart: the manifest and config come out of
the delta file, and each layer is either already in the repository or
reconstructed locally from a binary patch. No network access at all.

Introduce a `LayerSource` trait, which is just the existing `fetch_layer`
signature behind a trait object, and a `ProxyLayerSource` implementing
today's behaviour. `PreparedImport` now carries a LayerSource instead of a
bare `OpenedImage`, and `ImageImporter::prepare_from_manifest` builds a
`PreparedImport` from a manifest and config the caller already has, with
the layers coming from whatever source it passes in.

`ImageImporter::new_without_proxy` skips spawning skopeo entirely, so an
importer that will never touch the network does not need it installed.

Along the way, `prepare_internal` splits into `check_sigverify` and
`diff_previous_state` so both prepare paths share the "is this already
imported?" logic, and `cache_pending` moves into `create_prepared_import`
where both paths reach it.

`ProxyLayerSource` also fetches the proxy's layer info once and caches
it, where before both `unencapsulate_base()` and `import()` asked for it
separately.

Assisted-by: AI
Signed-off-by: Alexander Larsson <alexl@redhat.com>
We will need this later. We're reusing most of the old
list_container_deployment_manifests() for this.

Signed-off-by: Alexander Larsson <alexl@redhat.com>
`bootc upgrade --from-delta` and `bootc switch --from-delta` worked only
with the composefs backend. Wire them up for ostree too.

A tar-diff patch reads the source image by *path*, not from the original
tar byte stream, so the source does not have to be a container layer:
an ostree commit holding the same filesystem works just as well.
`OstreeDataSource` serves file content out of a commit, and
`DeltaLayerSource` plugs that into the container importer in place of
the image proxy, so nothing is fetched.

ostree does not store an image's root filesystem verbatim - the tar
importer moves `/etc` to `/usr/etc`, and `/var` to
`/usr/share/factory/var` on ostree older than v2024.3 - so a source path
is tried at each of the locations that importer could have put it.

Each reconstructed layer is streamed to the importer over a pipe, with
the reconstruction itself running as the driver future that
`join_fetch` already polls concurrently, so a multi-GB base layer never
has to be spooled to disk.

The source image is looked up by config digest among the images in the
repository, and an absent source is fatal: there is no guarantee of a
network connection at the point a delta is applied, so falling back to
the registry would defeat the purpose.

Byte-level progress is not reported for delta layers: the reconstructed
bytes are uncompressed, so counting them against the descriptor's
compressed size would overshoot. Per-layer start/completion still is.

A layer's ref is written as the layer is unpacked, i.e. before the
driver future has had the chance to report a diff_id mismatch, so a
failed apply can leave a ref to unverified content that a retry would
reuse. Prune the unreferenced layers after a failed delta apply.

Also, `switch --from-delta` no longer short-circuits when the image
specification is unchanged - a delta names one specific target digest,
and switching to the reference you are already tracking is exactly how
it is normally used.

Assisted-by: AI
Signed-off-by: Alexander Larsson <alexl@redhat.com>
Three tests covering the whole path from a delta on disk to an imported
image, at increasing degrees of realism:

`test_apply_delta_whole_layers` builds a delta whose patches are the
target layers carried verbatim. The format allows that, and the layer
reconstruction dispatches on media type, so this exercises parsing,
validation, source lookup and the import while needing no external
tooling - it runs everywhere.

`test_apply_delta_chunked` and `test_apply_delta_derived` use the real
`oci-delta` to build the delta, and skip themselves when it is not
installed. The first covers an ostree-native image, whose layers are
made of repo objects under `sysroot/ostree/` that do not exist in the
commit's file tree - it passes only because oci-delta is told never to
ask for one. The second covers a derived layer, an ordinary root
filesystem tar, which is the case that actually drives
`OstreeDataSource`; the file it patches lives at `/etc/bigconf`, i.e.
at a path the importer relocates, and the test asserts both that it
lands at `/usr/etc/bigconf` and that the patch is far smaller than the
layer, which it can only be if the content really was read back out of
the source commit.

`oci-delta` goes in a new optional package list, because it is not
available on every distribution we build on.

Assisted-by: AI
Signed-off-by: Alexander Larsson <alexl@redhat.com>

@cgwalters cgwalters left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Just an initial skim overall looks sane

Comment thread crates/lib/src/delta.rs
}

/// Reject `--from-delta` for an image that also has to be in containers-storage.
pub(crate) fn reject_unified_storage(delta: &Delta, use_unified: bool) -> Result<()> {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

So...we are going to need a plan to make this work though...

I think the most viable thing will be extending podman-container-tools/container-libs#651 to support pushes?

I guess we can merge this as is and just add "fix deltas" to the unified storage tracker TODO (can you do that as part of this PR?)

Comment thread crates/lib/src/delta.rs Outdated
}

/// Check the delta's internal consistency, to fail early
fn validate(p: &ParsedDelta) -> Result<()> {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

(Also this should be part of impl ParsedDelta right)

Comment thread crates/lib/src/delta.rs
let config = ImageConfiguration::from_reader(&p.target_config_raw[..])
.context("Parsing embedded target config")?;
let layers = p.target_manifest.layers();
ensure!(!layers.is_empty(), "Delta target image has no layers");

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Like on this topic there is https://docs.rs/nonempty/0.12.0/nonempty/ for example (and arguably we should use that in oci-spec by default for images, or at least have a standard falliable accessor)

Comment thread crates/lib/src/delta.rs
}

/// Verify that `data` hashes to `expected`.
fn verify_digest(what: &str, data: &[u8], expected: &Digest) -> Result<()> {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think what should be an impl Read in the general case for this, I'd be surprised if we don't have a variant of this somewhere

delta.validate_image_reference(spec_imgref)?;
crate::delta::reject_unified_storage(delta, use_unified)?;
if !quiet {
println!("Applying delta {}", delta.describe());

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This needs to use the progress API.

We may need a blanket ban on (e)println! in crates/lib

cgwalters-bot added a commit to cgwalters-bot/bootc that referenced this pull request Sep 23, 2026
println!() panics if stdout is closed (e.g. a broken pipe when piping
into head), whereas writeln!() lets us propagate the error normally.
These call sites are the actual output of a subcommand (digests, JSON,
tables, GC stats), so an explicit stdout writer is the right thing
anyway.

Prep for denying clippy::print_stdout in crates/lib, see
bootc-dev#2449 (comment)

Generated-by: AI
cgwalters-bot added a commit to cgwalters-bot/bootc that referenced this pull request Sep 23, 2026
fsck() already takes an explicit writer for its errors, but the
"ok: <check>" lines bypassed it and went straight to stdout. The only
caller passes stdout, so the output is unchanged; this just makes the
function honor its own API.

Prep for denying clippy::print_stdout in crates/lib, see
bootc-dev#2449 (comment)

Generated-by: AI
cgwalters-bot added a commit to cgwalters-bot/bootc that referenced this pull request Sep 23, 2026
Subcommand implementations such as install, upgrade, switch and
rollback print status lines for a human at a terminal ("Installing bootloader
via bootupd", "Installation complete!", ...). We want to deny direct
println!/eprintln! in the library, but exempting each of these
functions with #[expect] would also exempt everything else in them,
and several are very large.

Instead, put the one exemption on a pair of small helpers and use them
at these call sites, so a new stray println!() anywhere else, including
inside those functions, still trips the lint. Output is unchanged:
cli_status!() writes a line to stdout and cli_warn!() to stderr, just
as the calls they replace did.

Prep for denying clippy::print_stdout in crates/lib, see
bootc-dev#2449 (comment)

Generated-by: AI
cgwalters-bot added a commit to cgwalters-bot/bootc that referenced this pull request Sep 23, 2026
Library code writing directly to stdout/stderr is easy to add and
hard to notice in review, and it corrupts output for callers that
expect something machine-readable (or that want progress routed
through ProgressWriter). This came up in
bootc-dev#2449 (comment)
where a new println!() slipped into the composefs pull path.

Deny both lints at the crate root so new call sites fail clippy.
They are restriction lints, so the -A clippy::all in the Makefile
validate target doesn't touch them and CI enforces the deny as is.

Human-facing status output already goes through cli_status!() and
cli_warn!(). The few remaining direct prints get a targeted #[expect]
with a reason: global_init() runs before tracing is set up,
journal_send() reports that the journal itself is failing, and the
spinner has a plain-text fallback when there is no tty. Using expect
rather than allow means the attribute gets flagged once it is no
longer needed.

Tests are exempted via allow-print-in-tests in clippy.toml.

Generated-by: AI

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/documentation Updates to the documentation area/install Issues related to `bootc install` area/ostree Issues related to ostree

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants