Skip to content

Commit d5ba655

Browse files
committed
composefs: Add unified storage pull path
For registry transports, pull_composefs_repo() now goes through bootc-owned containers-storage first (via podman pull), then imports from there into the composefs repo via cstor (zero-copy reflink/hardlink). This means the source image remains in containers-storage after upgrade, enabling 'podman run <booted-image>'. Non-registry transports (oci:, containers-storage:, docker-daemon:) continue using the direct skopeo path. Also fix composefs_oci::pull() callsite to pass the new zerocopy parameter added in the composefs-rs import-cstor-rs-rebase branch. Clean up GC to use CStorage::create() directly instead of going through storage.get_ensure_imgstore() which requires ostree and fails on composefs-only systems. Remove the unreferenced fsck module. Assisted-by: OpenCode (Claude Opus 4) Signed-off-by: Colin Walters <walters@verbum.org>
1 parent 95d13fd commit d5ba655

File tree

13 files changed

+787
-158
lines changed

13 files changed

+787
-158
lines changed

Cargo.lock

Lines changed: 531 additions & 56 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ clap_mangen = { version = "0.3.0" }
4444
# [patch."https://github.com/composefs/composefs-rs"]
4545
# cfsctl = { path = "/path/to/composefs-rs/crates/cfsctl" }
4646
# The Justfile will auto-detect these and bind-mount them into container builds.
47-
cfsctl = { git = "https://github.com/composefs/composefs-rs", rev = "b5fe0b8e2ff2d5ae2d79d3303cfef36f96cd00b2", package = "cfsctl" }
47+
cfsctl = { git = "https://github.com/cgwalters/composefs-rs", branch = "import-cstor-rs", package = "cfsctl" }
4848
fn-error-context = "0.2.1"
4949
futures-util = "0.3"
5050
hex = "0.4.3"
@@ -119,7 +119,3 @@ todo = "deny"
119119
needless_borrow = "allow"
120120
needless_borrows_for_generic_args = "allow"
121121

122-
123-
# Patched by composefs-rs CI to test against local composefs-rs
124-
[patch."https://github.com/composefs/composefs-rs"]
125-
cfsctl = { path = "/workspaces/composefs-rs/crates/cfsctl" } # Patched by composefs-rs

crates/lib/src/bootc_composefs/gc.rs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,8 @@ pub(crate) async fn composefs_gc(
303303
// deployments that predate the manifest→image link.
304304
let mut live_manifest_digests: Vec<composefs_oci::OciDigest> = Vec::new();
305305
let mut additional_roots = Vec::new();
306+
// Container image names for containers-storage pruning.
307+
let mut live_container_images: std::collections::HashSet<String> = Default::default();
306308

307309
// Read existing tags before the deployment loop so we can search
308310
// them for deployments that lack manifest_digest in their origin.
@@ -324,6 +326,19 @@ pub(crate) async fn composefs_gc(
324326
additional_roots.push(verity.clone());
325327

326328
if let Some(ini) = read_origin(sysroot, verity)? {
329+
// Collect the container image name for containers-storage GC.
330+
if let Some(container_ref) =
331+
ini.get::<String>("origin", ostree_ext::container::deploy::ORIGIN_CONTAINER)
332+
{
333+
// Parse the ostree image reference to extract the bare image name
334+
// (e.g. "quay.io/foo:tag" from "ostree-unverified-image:docker://quay.io/foo:tag")
335+
let image_name = container_ref
336+
.parse::<ostree_ext::container::OstreeImageReference>()
337+
.map(|r| r.imgref.name)
338+
.unwrap_or_else(|_| container_ref.clone());
339+
live_container_images.insert(image_name);
340+
}
341+
327342
if let Some(manifest_digest_str) =
328343
ini.get::<String>(ORIGIN_KEY_IMAGE, ORIGIN_KEY_MANIFEST_DIGEST)
329344
{
@@ -407,6 +422,21 @@ pub(crate) async fn composefs_gc(
407422
.map(|x| x.as_str())
408423
.collect::<Vec<_>>();
409424

425+
// Prune containers-storage: remove images not backing any live deployment.
426+
if !dry_run && !live_container_images.is_empty() {
427+
let subpath = crate::podstorage::CStorage::subpath();
428+
if sysroot.try_exists(&subpath).unwrap_or(false) {
429+
let run = Dir::open_ambient_dir("/run", cap_std_ext::cap_std::ambient_authority())?;
430+
let imgstore = crate::podstorage::CStorage::create(&sysroot, &run, None)?;
431+
let roots: std::collections::HashSet<&str> =
432+
live_container_images.iter().map(|s| s.as_str()).collect();
433+
let pruned = imgstore.prune_except_roots(&roots).await?;
434+
if !pruned.is_empty() {
435+
tracing::info!("Pruned {} images from containers-storage", pruned.len());
436+
}
437+
}
438+
}
439+
410440
// Run garbage collection. Tags root the OCI metadata chain
411441
// (manifest → config → layers). The additional_roots protect EROFS
412442
// images for deployments that predate the manifest→image link;

crates/lib/src/bootc_composefs/repo.rs

Lines changed: 146 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use fn_error_context::context;
2+
use std::process::Command;
23
use std::sync::Arc;
34

45
use anyhow::{Context, Result};
@@ -13,12 +14,13 @@ use composefs_oci::{
1314
pull_image as composefs_oci_pull_image, skopeo::PullResult, tag_image,
1415
};
1516

16-
use ostree_ext::container::ImageReference as OstreeExtImgRef;
17+
use ostree_ext::containers_image_proxy;
1718

1819
use cap_std_ext::cap_std::{ambient_authority, fs::Dir};
1920

2021
use crate::composefs_consts::BOOTC_TAG_PREFIX;
2122
use crate::install::{RootSetup, State};
23+
use crate::podstorage::CStorage;
2224

2325
/// Create a composefs OCI tag name for the given manifest digest.
2426
///
@@ -69,24 +71,18 @@ pub(crate) async fn initialize_composefs_repository(
6971
repo.set_insecure();
7072
}
7173

72-
let OstreeExtImgRef {
73-
name: image_name,
74-
transport,
75-
} = &state.source.imageref;
74+
let imgref = get_imgref(&transport.to_string(), image_name)?;
7675

77-
let mut config = crate::deploy::new_proxy_config();
78-
ostree_ext::container::merge_default_container_proxy_opts(&mut config)?;
76+
// Use the unified path: first into containers-storage on the target
77+
// rootfs, then cstor zero-copy into composefs. This ensures the image
78+
// is available for `podman run` from first boot.
79+
let run = Dir::open_ambient_dir("/run", ambient_authority())?;
80+
let imgstore = CStorage::create(rootfs_dir, &run, None)?;
81+
let storage_path = root_setup.physical_root_path.join(CStorage::subpath());
7982

80-
// Pull without a reference tag; we tag explicitly afterward so we
81-
// control the tag name format.
8283
let repo = Arc::new(repo);
83-
let (pull_result, _stats) = composefs_oci_pull_image(
84-
&repo,
85-
&format!("{transport}{image_name}"),
86-
None,
87-
Some(config),
88-
)
89-
.await?;
84+
let pull_result =
85+
pull_composefs_unified(&imgstore, storage_path.as_str(), &repo, &imgref).await?;
9086

9187
// Tag the manifest as a bootc-owned GC root.
9288
let tag = bootc_tag_for_manifest(&pull_result.manifest_digest.to_string());
@@ -107,24 +103,24 @@ pub(crate) async fn initialize_composefs_repository(
107103
Ok(pull_result)
108104
}
109105

110-
/// skopeo (in composefs-rs) doesn't understand "registry:"
111-
/// This function will convert it to "docker://" and return the image ref
106+
/// Convert a transport string and image name into a `containers_image_proxy::ImageReference`.
112107
///
113-
/// Ex
114-
/// docker://quay.io/some-image
115-
/// containers-storage:some-image
116-
/// docker-daemon:some-image-id
117-
pub(crate) fn get_imgref(transport: &str, image: &str) -> String {
118-
let img = image.strip_prefix(":").unwrap_or(&image);
119-
let transport = transport.strip_suffix(":").unwrap_or(&transport);
120-
121-
if transport == "registry" || transport == "docker://" {
108+
/// The `spec::ImageReference` stores transport as a string (e.g. "registry:",
109+
/// "containers-storage:"). This parses that into a proper typed reference
110+
/// that renders correctly for skopeo (e.g. "docker://quay.io/some-image").
111+
pub(crate) fn get_imgref(transport: &str, image: &str) -> Result<containers_image_proxy::ImageReference> {
112+
let img = image.strip_prefix(':').unwrap_or(image);
113+
// Normalize: strip trailing separator if present, then reconstruct
114+
// in the canonical form that ImageReference expects.
115+
let transport = transport.strip_suffix(':').unwrap_or(transport);
116+
let s = if transport == "registry" {
117+
// "registry:" is our internal name; the wire format is "docker://"
122118
format!("docker://{img}")
123-
} else if transport == "docker-daemon" {
124-
format!("docker-daemon:{img}")
125119
} else {
126120
format!("{transport}:{img}")
127-
}
121+
};
122+
s.as_str().try_into()
123+
.with_context(|| format!("Parsing image reference '{s}'"))
128124
}
129125

130126
/// Result of pulling a composefs repository, including the OCI manifest digest
@@ -137,25 +133,114 @@ pub(crate) struct PullRepoResult {
137133
pub(crate) manifest_digest: String,
138134
}
139135

140-
/// Pulls the `image` from `transport` into a composefs repository at /sysroot
141-
/// Checks for boot entries in the image and returns them
136+
/// Pull an image via unified storage: first into bootc-owned containers-storage,
137+
/// then from there into the composefs repository via cstor (zero-copy
138+
/// reflink/hardlink).
139+
///
140+
/// The caller provides:
141+
/// - `imgstore`: the bootc-owned `CStorage` instance (may be on an arbitrary
142+
/// mount point during install, or under `/sysroot` during upgrade)
143+
/// - `storage_path`: the absolute filesystem path to that containers-storage
144+
/// directory, so cstor and skopeo can find it (e.g.
145+
/// `/mnt/sysroot/ostree/bootc/storage` during install, or
146+
/// `/sysroot/ostree/bootc/storage` during upgrade)
147+
///
148+
/// This ensures the image is available in containers-storage for `podman run`
149+
/// while also populating the composefs repo for booting.
150+
async fn pull_composefs_unified(
151+
imgstore: &CStorage,
152+
storage_path: &str,
153+
repo: &Arc<crate::store::ComposefsRepository>,
154+
imgref: &containers_image_proxy::ImageReference,
155+
) -> Result<PullResult<Sha512HashValue>> {
156+
let image = &imgref.name;
157+
158+
// Stage 1: get the image into bootc-owned containers-storage.
159+
if imgref.transport == containers_image_proxy::Transport::ContainerStorage {
160+
// The image is in the default containers-storage (/var/lib/containers/storage).
161+
// Copy it into bootc-owned storage.
162+
tracing::info!("Unified pull: copying {image} from host containers-storage");
163+
imgstore
164+
.pull_from_host_storage(image)
165+
.await
166+
.context("Copying image from host containers-storage into bootc storage")?;
167+
} else {
168+
// For registry (docker://), oci:, docker-daemon:, etc. — pull
169+
// via the native podman API with streaming progress display.
170+
let pull_ref = imgref.to_string();
171+
tracing::info!("Unified pull: fetching {pull_ref} into containers-storage");
172+
imgstore
173+
.pull_with_progress(&pull_ref)
174+
.await
175+
.context("Pulling image into bootc containers-storage")?;
176+
}
177+
178+
// Stage 2: import layers+config from containers-storage into composefs
179+
// via cstor (zero-copy reflink/hardlink from overlay diff/).
180+
let cstor_imgref_str = format!("containers-storage:{image}");
181+
tracing::info!("Unified pull: importing layers from {cstor_imgref_str} (zero-copy)");
182+
183+
// TODO: composefs-rs should accept a storage path parameter directly
184+
// instead of relying on environment variables. For now, set $STORAGE_OPTS
185+
// so cstor can discover bootc's private containers-storage.
186+
#[allow(unsafe_code)]
187+
// SAFETY: No other threads are reading STORAGE_OPTS concurrently during pull.
188+
unsafe {
189+
std::env::set_var(
190+
"STORAGE_OPTS",
191+
format!("additionalimagestore={storage_path}"),
192+
);
193+
}
194+
let cstor_result = composefs_oci::pull(repo, &cstor_imgref_str, None, None, false)
195+
.await
196+
.context("Importing layers from containers-storage into composefs")?;
197+
198+
tracing::info!(
199+
"Unified pull: cstor import complete (config {}), importing manifest",
200+
cstor_result.config_digest
201+
);
202+
203+
// Stage 3: fetch and store the OCI manifest via skopeo.
204+
// The layers+config are already in composefs (imported above), so skopeo
205+
// will see them as AlreadyPresent and only create the manifest splitstream.
206+
let mut config = crate::deploy::new_proxy_config();
207+
ostree_ext::container::merge_default_container_proxy_opts(&mut config)?;
208+
let mut cmd = Command::new("skopeo");
209+
crate::podstorage::set_additional_image_store(&mut cmd, storage_path);
210+
config.skopeo_cmd = Some(cmd);
211+
212+
let (pull_result, _stats) =
213+
composefs_oci_pull_image(repo, &cstor_imgref_str, None, Some(config))
214+
.await
215+
.context("Storing manifest from containers-storage")?;
216+
217+
Ok(pull_result)
218+
}
219+
220+
/// Pulls the `image` from `transport` into a composefs repository at /sysroot.
221+
///
222+
/// For registry transports, this uses the unified storage path: the image is
223+
/// first pulled into bootc-owned containers-storage (so it's available for
224+
/// `podman run`), then imported from there into the composefs repo.
225+
///
226+
/// Checks for boot entries in the image and returns them.
142227
#[context("Pulling composefs repository")]
143228
pub(crate) async fn pull_composefs_repo(
144-
transport: &String,
145-
image: &String,
229+
transport: &str,
230+
image: &str,
146231
allow_missing_fsverity: bool,
147232
) -> Result<PullRepoResult> {
148233
const COMPOSEFS_PULL_JOURNAL_ID: &str = "4c3b2a1f0e9d8c7b6a5f4e3d2c1b0a9f8";
149234

235+
let imgref = get_imgref(transport, image)?;
236+
150237
tracing::info!(
151238
message_id = COMPOSEFS_PULL_JOURNAL_ID,
152239
bootc.operation = "pull",
153240
bootc.source_image = image,
154-
bootc.transport = transport,
241+
bootc.transport = %imgref.transport,
155242
bootc.allow_missing_fsverity = allow_missing_fsverity,
156-
"Pulling composefs image {}:{}",
157-
transport,
158-
image
243+
"Pulling composefs image {imgref}",
159244
);
160245

161246
let rootfs_dir = Dir::open_ambient_dir("/sysroot", ambient_authority())?;
@@ -165,17 +250,15 @@ pub(crate) async fn pull_composefs_repo(
165250
repo.set_insecure();
166251
}
167252

168-
let final_imgref = get_imgref(transport, image);
169-
170-
tracing::debug!("Image to pull {final_imgref}");
253+
let repo = Arc::new(repo);
171254

172-
let mut config = crate::deploy::new_proxy_config();
173-
ostree_ext::container::merge_default_container_proxy_opts(&mut config)?;
255+
// Create bootc-owned containers-storage on the rootfs.
256+
let run = Dir::open_ambient_dir("/run", ambient_authority())?;
257+
let imgstore = CStorage::create(&rootfs_dir, &run, None)?;
258+
let storage_path = format!("/sysroot/{}", CStorage::subpath());
174259

175-
let repo = Arc::new(repo);
176-
let (pull_result, _stats) = composefs_oci_pull_image(&repo, &final_imgref, None, Some(config))
177-
.await
178-
.context("Pulling composefs repo")?;
260+
let pull_result =
261+
pull_composefs_unified(&imgstore, &storage_path, &repo, &imgref).await?;
179262

180263
// Tag the manifest as a bootc-owned GC root.
181264
let tag = bootc_tag_for_manifest(&pull_result.manifest_digest.to_string());
@@ -227,39 +310,35 @@ mod tests {
227310

228311
#[test]
229312
fn test_get_imgref_registry_transport() {
230-
assert_eq!(
231-
get_imgref("registry:", IMAGE_NAME),
232-
format!("docker://{IMAGE_NAME}")
233-
);
313+
let r = get_imgref("registry:", IMAGE_NAME).unwrap();
314+
assert_eq!(r.transport, containers_image_proxy::Transport::Registry);
315+
assert_eq!(r.name, IMAGE_NAME);
316+
assert_eq!(r.to_string(), format!("docker://{IMAGE_NAME}"));
234317
}
235318

236319
#[test]
237320
fn test_get_imgref_containers_storage() {
238-
assert_eq!(
239-
get_imgref("containers-storage", IMAGE_NAME),
240-
format!("containers-storage:{IMAGE_NAME}")
241-
);
321+
let r = get_imgref("containers-storage", IMAGE_NAME).unwrap();
322+
assert_eq!(r.transport, containers_image_proxy::Transport::ContainerStorage);
323+
assert_eq!(r.name, IMAGE_NAME);
242324

243-
assert_eq!(
244-
get_imgref("containers-storage:", IMAGE_NAME),
245-
format!("containers-storage:{IMAGE_NAME}")
246-
);
325+
let r = get_imgref("containers-storage:", IMAGE_NAME).unwrap();
326+
assert_eq!(r.transport, containers_image_proxy::Transport::ContainerStorage);
327+
assert_eq!(r.name, IMAGE_NAME);
247328
}
248329

249330
#[test]
250331
fn test_get_imgref_edge_cases() {
251-
assert_eq!(
252-
get_imgref("registry", IMAGE_NAME),
253-
format!("docker://{IMAGE_NAME}")
254-
);
332+
let r = get_imgref("registry", IMAGE_NAME).unwrap();
333+
assert_eq!(r.transport, containers_image_proxy::Transport::Registry);
334+
assert_eq!(r.to_string(), format!("docker://{IMAGE_NAME}"));
255335
}
256336

257337
#[test]
258338
fn test_get_imgref_docker_daemon_transport() {
259-
assert_eq!(
260-
get_imgref("docker-daemon", IMAGE_NAME),
261-
format!("docker-daemon:{IMAGE_NAME}")
262-
);
339+
let r = get_imgref("docker-daemon", IMAGE_NAME).unwrap();
340+
assert_eq!(r.transport, containers_image_proxy::Transport::DockerDaemon);
341+
assert_eq!(r.name, IMAGE_NAME);
263342
}
264343

265344
#[test]

crates/lib/src/bootc_composefs/state.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -194,16 +194,14 @@ pub(crate) fn update_target_imgref_in_origin(
194194
booted_cfs: &BootedComposefs,
195195
imgref: &ImageReference,
196196
) -> Result<()> {
197+
let imgref = get_imgref(&imgref.transport, &imgref.image)?;
197198
add_update_in_origin(
198199
storage,
199200
booted_cfs.cmdline.digest.as_ref(),
200201
"origin",
201202
&[(
202203
ORIGIN_CONTAINER,
203-
&format!(
204-
"ostree-unverified-image:{}",
205-
get_imgref(&imgref.transport, &imgref.image)
206-
),
204+
&format!("ostree-unverified-image:{imgref}"),
207205
)],
208206
)
209207
}
@@ -288,7 +286,7 @@ pub(crate) async fn write_composefs_state(
288286
..
289287
} = &target_imgref;
290288

291-
let imgref = get_imgref(&transport, &image_name);
289+
let imgref = get_imgref(transport, image_name)?;
292290

293291
let mut config = tini::Ini::new().section("origin").item(
294292
ORIGIN_CONTAINER,

0 commit comments

Comments
 (0)