lib: Add experimental unified storage support for install #1601

jmarrero · 2025-09-09T17:28:09Z

Compiles and at least does not fail a bootc switch... and can see the image in the container storage we own.

[core@cosa-devsh ~]$ sudo podman --storage-opt=additionalimagestore=/usr/lib/bootc/storage images
REPOSITORY                       TAG         IMAGE ID      CREATED       SIZE
quay.io/jmarrero_rh/soft-reboot  1           97f84fcb062e  25 hours ago  1.83 GB

This was drafted with the help of Claude Code.

cgwalters

Thanks for working on this!

cgwalters · 2025-09-09T18:20:03Z

crates/lib/src/cli.rs

+    /// Use podman/skopeo to pull image to additionalimagestore, then read from container storage.
+    /// This provides a unified approach that leverages existing container tooling.
+    #[clap(long)]
+    pub(crate) unified: bool,


I don't think this should be a CLI flag that operates just once; it should be something like bootc image --set-unified or so and act persistently.

Also we should support setting this at install time so that it happens from the very start.

(hmm my bad I thought we discussed this but I failed to update the issue #20 or something maybe?)

Ahh man yeah, we talked about saving the config on the origin file IIRC, I forgot.

cgwalters · 2025-09-09T18:20:41Z

crates/lib/src/podman.rs

+    use bootc_utils::CommandRunExt;
+
+    // Use podman pull with additionalimagestore pointing to bootc storage
+    let bootc_storage_path = "/usr/lib/bootc/storage";


Use podstorage.rs instead please

To elaborate on this, an important aspect here is currently the GC of the bootc/storage instance is rooted in the set of LBIs. That will need to be extended to include the host image.

Another big task related to this - we'll eventually really want to use the podman HTTP APIs so we can properly monitor progress. Right now we are very crudely relying on doing this synchronously to the existing tty and reusing the podman CLI tty output. But that can't really work if we ever need to render our own progress APIs.

https://lib.rs/crates/bollard might be a good choice for this?

cgwalters · 2025-09-09T18:21:33Z

crates/lib/src/deploy.rs

+    match prepare_for_pull_unified(repo, imgref, target_imgref, store).await? {
+        PreparedPullResult::AlreadyPresent(existing) => {
+            // Log that the image was already present (Debug level since it's not actionable)
+            const IMAGE_ALREADY_PRESENT_ID: &str = "5c4d3e2f1a0b9c8d7e6f5a4b3c2d1e0f9";


Note that if we go this route, we should also not log to the journal in the ostree pull path because otherwise we're double logging.

cgwalters · 2025-09-09T18:24:07Z

crates/lib/src/cli.rs

+        let fetched = if opts.unified {
+            crate::deploy::pull_unified(repo, imgref, None, opts.quiet, prog.clone(), sysroot).await?
+        } else {
+            crate::deploy::pull(repo, imgref, None, opts.quiet, prog.clone()).await?


This relates to #1599 (comment)

Basically how about changing this code to take a Storage which would hold the persistent flag, and then crate::deploy::pull would itself query that flag and change its behavior.

That would also implicitly then fix the install path to behave the same.

jlebon · 2025-10-07T13:31:09Z

Do we get reflinks when importing from the alternate storage into the ostree repo?

cgwalters · 2025-10-07T14:36:03Z

Do we get reflinks when importing from the alternate storage into the ostree repo?

#20 links to containers/container-libs#144

jlebon · 2025-10-07T15:42:36Z

Do we get reflinks when importing from the alternate storage into the ostree repo?

#20 links to containers/container-libs#144

Hmm, that's about copying between two container storages, but I was asking about the ostree import. But #20 does say:

From there, we do a reflink/hardlink copy into the ostree store (i.e. This would duplicate metadata, but not data on disk. Reflinks should always work.

Which kinda answers the question.

Basically without reflinks into the ostree storage, IMO this makes it a lot less obvious that this new option is objectively better and it instead becomes a discussion of trade-offs.

Add an experimental --experimental-unified-storage flag to bootc install that uses bootc's container storage (/usr/lib/bootc/storage) to pull images first, then imports from there. This is the same approach used for logically bound images (LBIs). Background: The unified storage approach allows bootc to share container images with podman's storage, reducing disk space and enabling offline installs when images are pre-pulled to the host's container storage. Changes: - Add --experimental-unified-storage CLI flag to install subcommands - Add sysroot_path parameter to prepare_for_pull_unified() and pull_unified() to handle the different mount points during install vs upgrade/switch - Handle localhost images specially by exporting from ostree to container storage first (these can't be pulled from a registry) - Skip pull in prepare_for_pull_unified() if image already exists in bootc storage - Add TMT test for install with unified storage flag - Add TMT test for switching to unified storage on running system - Add integration test for system-reinstall-bootc with unified storage The sysroot_path fix is needed because during install the target disk is mounted at a specific path (e.g., /var/mnt), not /sysroot. Skopeo needs the actual filesystem path to find the bootc storage. Relates: bootc-dev#20 Assisted-by: Claude Code (Sonnet 4.5)

jmarrero · 2025-12-02T20:06:50Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces experimental support for unified storage, allowing bootc to pull images into its own container storage before importing them. This is a significant feature that touches installation, upgrade, and switch paths. The changes include new CLI options, a new image set-unified command, and the core logic for the unified pull mechanism.

The implementation is comprehensive and includes new integration tests. My review has identified a few areas for improvement:

A bug in the image reference formatting for unified storage checks during installation.
Inconsistent error handling when checking for images in the unified store.
A potential panic due to an .unwrap() call on a Result.

Addressing these points will improve the robustness and maintainability of this new feature.

gemini-code-assist · 2025-12-02T20:08:41Z

crates/lib/src/image.rs

+        let booted = host.status.booted.as_ref().unwrap();
+        let booted_image = booted.image.as_ref().unwrap();
+        let source = ImageReference {
+            transport: Transport::try_from(booted_image.image.transport.as_str()).unwrap(),


Using .unwrap() here can cause a panic if try_from fails. It's safer to use the ? operator to propagate the error, which will be converted into an anyhow::Error.

Suggested change

transport: Transport::try_from(booted_image.image.transport.as_str()).unwrap(),

transport: Transport::try_from(booted_image.image.transport.as_str())?,

gemini-code-assist · 2025-12-02T20:08:42Z

crates/lib/src/install.rs

+        // Auto-detect: check if image exists in bootc storage (same as upgrade/switch)
+        let imgstore = storage.get_ensure_imgstore()?;
+        imgstore
+            .exists(&format!("{spec_imgref:#}"))


There's a bug here. format!("{spec_imgref:#}") is used to create the image reference string. This uses the Display implementation of ImageReference, which includes signature information (e.g., ostree-unverified-registry:quay.io/...). However, the container storage APIs expect a reference like transport:name or just name for the registry transport. The correct way to format this is using crate::utils::imageref_to_container_ref(), as done in cli.rs.

Additionally, the logic to detect whether to use unified storage is duplicated across cli.rs (in upgrade and switch_ostree) and here in install.rs. This should be refactored into a single helper function to avoid bugs like this and improve maintainability.

Suggested change

.exists(&format!("{spec_imgref:#}"))

.exists(&crate::utils::imageref_to_container_ref(&spec_imgref))

gemini-code-assist · 2025-12-02T20:08:42Z

crates/lib/src/deploy.rs

+
+    // Check if image already exists in bootc storage - if so, skip the pull
+    // This is important for localhost images which can't be pulled from a registry
+    let image_exists = imgstore.exists(&image_ref_str).await.unwrap_or(false);


The error from imgstore.exists() is silently ignored here by using unwrap_or(false). In other places where this check is performed (e.g., cli.rs and install.rs), a warning is logged. This inconsistency can hide potential issues. The error should be propagated or at least logged with a warning for consistency.

let image_exists = match imgstore.exists(&image_ref_str).await { Ok(v) => v, Err(e) => { tracing::warn!("Failed to check for existing image in bootc storage: {e}"); false } };

cgwalters · 2025-12-02T20:51:58Z

On the progress status side maybe we can use https://lib.rs/crates/bollard to talk to podman and I think it should help us get data out of the events?

cgwalters · 2025-12-02T20:43:43Z

crates/lib/src/image.rs

+    );
+
+    // Check if this is a localhost image with registry transport - these images
+    // were built locally and don't exist in any remote registry. We need to


I don't understand why we're special casing this

bootc-bot bot requested a review from cgwalters September 9, 2025 17:28

cgwalters reviewed Sep 9, 2025

View reviewed changes

jmarrero force-pushed the unified-storage-wip branch from 1baf1e8 to 9c3ef81 Compare October 9, 2025 18:15

jmarrero changed the title ~~WIP: Add --unified to use our container-storage for host images~~ WIP: Use our container-storage for host images if the refspec exists on it Oct 9, 2025

jmarrero changed the title ~~WIP: Use our container-storage for host images if the refspec exists on it~~ WIP: Use our container-storage for host images if the refspec exists Oct 9, 2025

jmarrero force-pushed the unified-storage-wip branch 2 times, most recently from e891e6c to be75055 Compare October 15, 2025 17:57

github-actions bot added the area/install Issues related to `bootc install` label Oct 15, 2025

jmarrero force-pushed the unified-storage-wip branch from 291fbd7 to fe10204 Compare November 11, 2025 19:53

jmarrero force-pushed the unified-storage-wip branch 3 times, most recently from 06be8fb to b801a17 Compare November 21, 2025 20:50

github-actions bot added the area/system-reinstall-bootc Issues related to system-reinstall-botoc label Nov 21, 2025

jmarrero force-pushed the unified-storage-wip branch from b801a17 to 68b1ba6 Compare November 21, 2025 20:56

jmarrero force-pushed the unified-storage-wip branch 2 times, most recently from 4f06129 to ee300e3 Compare December 2, 2025 19:18

jmarrero force-pushed the unified-storage-wip branch from ee300e3 to 50c4f70 Compare December 2, 2025 19:28

jmarrero changed the title ~~WIP: Use our container-storage for host images if the refspec exists~~ lib: Add experimental unified storage support for install Dec 2, 2025

gemini-code-assist bot reviewed Dec 2, 2025

View reviewed changes

cgwalters reviewed Dec 2, 2025

View reviewed changes

	transport: Transport::try_from(booted_image.image.transport.as_str()).unwrap(),
	transport: Transport::try_from(booted_image.image.transport.as_str())?,

	.exists(&format!("{spec_imgref:#}"))
	.exists(&crate::utils::imageref_to_container_ref(&spec_imgref))

lib: Add experimental unified storage support for install #1601

Are you sure you want to change the base?

lib: Add experimental unified storage support for install #1601

Uh oh!

Conversation

jmarrero commented Sep 9, 2025

Uh oh!

cgwalters left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jlebon commented Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cgwalters commented Oct 7, 2025

Uh oh!

jlebon commented Oct 7, 2025

Uh oh!

jmarrero commented Dec 2, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

cgwalters commented Dec 2, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jlebon commented Oct 7, 2025 •

edited

Loading