Skip to content

Commit 2f7d715

Browse files
henrywangclaude
andcommitted
podstorage: Fix pull_from_host_storage for containers-storage source
The previous fix (b64bed4) for unified storage with containers-storage transport had a bug: pull_from_host_storage() called bind_storage_roots() which made bootc storage the default for podman. When podman image push ran, it looked for the source image in bootc storage instead of the host's default storage (/var/lib/containers/storage), causing "image not known" errors. Fix this by: - Not calling bind_storage_roots, so podman uses its default storage - Passing an explicit storage_path parameter to pull_from_host_storage - Specifying the bootc storage path in the destination transport This allows podman to read from host storage and push to bootc storage without changing any defaults. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Xiaofeng Wang <henrywangxf@me.com>
1 parent 4ac4253 commit 2f7d715

5 files changed

Lines changed: 65 additions & 18 deletions

File tree

crates/lib/src/cli.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1688,9 +1688,14 @@ async fn run_from_opt(opt: Opt) -> Result<()> {
16881688
ImageOpts::SetUnified => crate::image::set_unified_entrypoint().await,
16891689
ImageOpts::PullFromDefaultStorage { image } => {
16901690
let storage = get_storage().await?;
1691+
let storage_path = format!(
1692+
"{}/{}",
1693+
storage.physical_root_path,
1694+
crate::podstorage::CStorage::subpath()
1695+
);
16911696
storage
16921697
.get_ensure_imgstore()?
1693-
.pull_from_host_storage(&image)
1698+
.pull_from_host_storage(&image, &storage_path)
16941699
.await
16951700
}
16961701
ImageOpts::Cmd(opt) => {

crates/lib/src/deploy.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -506,11 +506,19 @@ pub(crate) async fn prepare_for_pull_unified(
506506
let pull_msg = format!("Pulling {} to bootc storage", &image_ref_str);
507507
let is_containers_storage = imgref.transport == "containers-storage";
508508
let image_name = imgref.image.clone();
509+
// Compute the storage path for pull_from_host_storage
510+
let storage_path = format!(
511+
"{}/{}",
512+
store.physical_root_path,
513+
crate::podstorage::CStorage::subpath()
514+
);
509515
async_task_with_spinner(&pull_msg, async move {
510516
if is_containers_storage {
511517
// For containers-storage transport, use pull_from_host_storage which
512518
// properly copies from host container storage to bootc storage
513-
imgstore.pull_from_host_storage(&image_name).await?;
519+
imgstore
520+
.pull_from_host_storage(&image_name, &storage_path)
521+
.await?;
514522
Ok(true)
515523
} else {
516524
imgstore

crates/lib/src/image.rs

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -329,9 +329,16 @@ pub(crate) async fn set_unified(sysroot: &crate::store::Storage) -> Result<()> {
329329
&imgref.image
330330
);
331331
let image_name = imgref.image.clone();
332+
let storage_path = format!(
333+
"{}/{}",
334+
sysroot.physical_root_path,
335+
crate::podstorage::CStorage::subpath()
336+
);
332337
let copy_msg = format!("Copying {} to bootc storage", &image_name);
333338
async_task_with_spinner(&copy_msg, async move {
334-
imgstore.pull_from_host_storage(&image_name).await
339+
imgstore
340+
.pull_from_host_storage(&image_name, &storage_path)
341+
.await
335342
})
336343
.await?;
337344
} else {
@@ -348,9 +355,16 @@ pub(crate) async fn set_unified(sysroot: &crate::store::Storage) -> Result<()> {
348355
&imgref.image
349356
);
350357
let image_name = imgref.image.clone();
358+
let storage_path = format!(
359+
"{}/{}",
360+
sysroot.physical_root_path,
361+
crate::podstorage::CStorage::subpath()
362+
);
351363
let copy_msg = format!("Copying {} to bootc storage", &image_name);
352364
async_task_with_spinner(&copy_msg, async move {
353-
imgstore.pull_from_host_storage(&image_name).await
365+
imgstore
366+
.pull_from_host_storage(&image_name, &storage_path)
367+
.await
354368
})
355369
.await?;
356370
} else {

crates/lib/src/install.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1827,9 +1827,16 @@ async fn install_with_sysroot(
18271827
BoundImages::Skip => {}
18281828
BoundImages::Resolved(resolved_bound_images) => {
18291829
// Now copy each bound image from the host's container storage into the target.
1830+
let storage_path = format!(
1831+
"{}/{}",
1832+
storage.physical_root_path,
1833+
crate::podstorage::CStorage::subpath()
1834+
);
18301835
for image in resolved_bound_images {
18311836
let image = image.image.as_str();
1832-
c_storage.pull_from_host_storage(image).await?;
1837+
c_storage
1838+
.pull_from_host_storage(image, &storage_path)
1839+
.await?;
18331840
}
18341841
}
18351842
BoundImages::Unresolved(bound_images) => {

crates/lib/src/podstorage.rs

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ use anyhow::{Context, Result};
1818
use bootc_utils::{AsyncCommandRunExt, CommandRunExt, ExitStatusExt};
1919
use camino::{Utf8Path, Utf8PathBuf};
2020
use cap_std_ext::cap_std::fs::Dir;
21-
use cap_std_ext::cap_tempfile::TempDir;
2221
use cap_std_ext::cmdext::CapStdExtCommandExt;
2322
use cap_std_ext::dirext::CapStdExtDirExt;
2423
use cap_std_ext::{cap_std, cap_tempfile};
@@ -412,24 +411,38 @@ impl CStorage {
412411

413412
/// Copy an image from the default container storage (/var/lib/containers/)
414413
/// to this storage.
414+
///
415+
/// The `storage_path` parameter should be the absolute filesystem path to
416+
/// the bootc storage directory (e.g., "/sysroot/ostree/bootc/storage").
415417
#[context("Pulling from host storage: {image}")]
416-
pub(crate) async fn pull_from_host_storage(&self, image: &str) -> Result<()> {
418+
pub(crate) async fn pull_from_host_storage(
419+
&self,
420+
image: &str,
421+
storage_path: &str,
422+
) -> Result<()> {
423+
// Use podman push with explicit destination storage path.
424+
// We don't use bind_storage_roots here because that would make bootc storage
425+
// the default, preventing podman from finding the source image in the
426+
// host's default storage (/var/lib/containers/storage).
417427
let mut cmd = Command::new("podman");
418428
cmd.stdin(Stdio::null());
419429
cmd.stdout(Stdio::null());
420-
// An ephemeral place for the transient state;
421-
let temp_runroot = TempDir::new(cap_std::ambient_authority())?;
422-
bind_storage_roots(&mut cmd, &self.storage_root, &temp_runroot)?;
423-
424-
// The destination (target stateroot) + container storage dest
425-
let storage_dest = &format!(
426-
"containers-storage:[overlay@{STORAGE_ALIAS_DIR}+/proc/self/fd/{STORAGE_RUN_FD}]"
427-
);
428-
cmd.args(["image", "push", "--remove-signatures", image])
429-
.arg(format!("{storage_dest}{image}"));
430+
431+
// Create a temporary directory for the runtime state
432+
let temp_runroot = tempfile::tempdir().context("Creating temp runroot")?;
433+
let runroot_path = temp_runroot
434+
.path()
435+
.to_str()
436+
.context("Invalid tempdir path")?;
437+
438+
// Destination: bootc storage with explicit paths
439+
// Podman reads from its default storage and pushes to the specified destination
440+
let dest = format!("containers-storage:[overlay@{storage_path}+{runroot_path}]{image}");
441+
442+
cmd.args(["image", "push", "--remove-signatures", image, &dest]);
430443
let mut cmd = AsyncCommand::from(cmd);
431444
cmd.run().await?;
432-
temp_runroot.close()?;
445+
drop(temp_runroot);
433446
Ok(())
434447
}
435448

0 commit comments

Comments
 (0)