Skip to content

Commit

Permalink
Open/create imagestore as needed
Browse files Browse the repository at this point in the history
The previous change in
containers#747
broke for two important scenarios:

- Installing with Anaconda
- Upgrades from previous states

Closes: containers#747

Signed-off-by: Colin Walters <walters@verbum.org>
  • Loading branch information
cgwalters committed Aug 1, 2024
1 parent ee11428 commit 519df35
Show file tree
Hide file tree
Showing 10 changed files with 65 additions and 18 deletions.
7 changes: 7 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions lib/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ tini = "1.3.0"
[dev-dependencies]
indoc = "2.0.5"
similar-asserts = { version = "1.5.0" }
static_assertions = "1.1.0"

[features]
default = ["install"]
Expand Down
9 changes: 7 additions & 2 deletions lib/src/boundimage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,13 +145,18 @@ fn parse_container_file(file_contents: &tini::Ini) -> Result<BoundImage> {
#[context("Pulling bound images")]
pub(crate) async fn pull_images(sysroot: &Storage, bound_images: Vec<BoundImage>) -> Result<()> {
tracing::debug!("Pulling bound images: {}", bound_images.len());
// Only initialize the image storage if we have images to pull
let imgstore = if !bound_images.is_empty() {
sysroot.get_ensure_imgstore()?
} else {
return Ok(());
};
//TODO: do this in parallel
for bound_image in bound_images {
let image = &bound_image.image;
let desc = format!("Updating bound image: {image}");
crate::utils::async_task_with_spinner(&desc, async move {
sysroot
.imgstore
imgstore
.pull(&bound_image.image, PullMode::IfNotExists)
.await
})
Expand Down
16 changes: 10 additions & 6 deletions lib/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -839,22 +839,26 @@ async fn run_from_opt(opt: Opt) -> Result<()> {
}
ImageOpts::PullFromDefaultStorage { image } => {
let sysroot = get_storage().await?;
sysroot.imgstore.pull_from_host_storage(&image).await
sysroot
.get_ensure_imgstore()?
.pull_from_host_storage(&image)
.await
}
ImageOpts::Cmd(opt) => {
let sysroot = get_storage().await?;
let storage = get_storage().await?;
let imgstore = storage.get_ensure_imgstore()?;
match opt {
ImageCmdOpts::List { args } => {
crate::image::imgcmd_entrypoint(&sysroot.imgstore, "list", &args).await
crate::image::imgcmd_entrypoint(imgstore, "list", &args).await
}
ImageCmdOpts::Build { args } => {
crate::image::imgcmd_entrypoint(&sysroot.imgstore, "build", &args).await
crate::image::imgcmd_entrypoint(imgstore, "build", &args).await
}
ImageCmdOpts::Pull { args } => {
crate::image::imgcmd_entrypoint(&sysroot.imgstore, "pull", &args).await
crate::image::imgcmd_entrypoint(imgstore, "pull", &args).await
}
ImageCmdOpts::Push { args } => {
crate::image::imgcmd_entrypoint(&sysroot.imgstore, "push", &args).await
crate::image::imgcmd_entrypoint(imgstore, "push", &args).await
}
}
}
Expand Down
5 changes: 4 additions & 1 deletion lib/src/deploy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,10 @@ pub(crate) async fn prune_container_store(sysroot: &Storage) -> Result<()> {
}
// Convert to a hashset of just the image names
let image_names = HashSet::from_iter(all_bound_images.iter().map(|img| img.image.as_str()));
let pruned = sysroot.imgstore.prune_except_roots(&image_names).await?;
let pruned = sysroot
.get_ensure_imgstore()?
.prune_except_roots(&image_names)
.await?;
tracing::debug!("Pruned images: {}", pruned.len());
Ok(())
}
Expand Down
2 changes: 1 addition & 1 deletion lib/src/image.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ pub(crate) async fn list_entrypoint() -> Result<()> {
println!();

println!("# Logically bound images");
let mut listcmd = sysroot.imgstore.new_image_cmd()?;
let mut listcmd = sysroot.get_ensure_imgstore()?.new_image_cmd()?;
listcmd.arg("list");
listcmd.run()?;

Expand Down
14 changes: 14 additions & 0 deletions lib/src/imgstorage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ pub(crate) struct Storage {
#[allow(dead_code)]
/// Our runtime state
run: Dir,
/// Disallow using this across multiple threads concurrently; while we
/// have internal locking in podman, in the future we may change how
/// things work here. And we don't have a use case right now for
/// concurrent operations.
_unsync: std::cell::Cell<()>,
}

#[derive(Debug, PartialEq, Eq)]
Expand Down Expand Up @@ -160,12 +165,14 @@ impl Storage {
sysroot
.rename(&tmp, sysroot, subpath)
.context("Renaming tmpdir")?;
tracing::debug!("Created image store");
}
Self::open(sysroot, run)
}

#[context("Opening imgstorage")]
pub(crate) fn open(sysroot: &Dir, run: &Dir) -> Result<Self> {
tracing::trace!("Opening container image store");
Self::init_globals()?;
let storage_root = sysroot
.open_dir(SUBPATH)
Expand All @@ -178,6 +185,7 @@ impl Storage {
sysroot: sysroot.try_clone()?,
storage_root,
run,
_unsync: Default::default(),
})
}

Expand Down Expand Up @@ -292,3 +300,9 @@ impl Storage {
Ok(())
}
}

#[cfg(test)]
mod tests {
use super::*;
static_assertions::assert_not_impl_any!(Storage: Sync);
}
5 changes: 4 additions & 1 deletion lib/src/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1289,10 +1289,13 @@ async fn install_with_sysroot(
tracing::debug!("Installed bootloader");

tracing::debug!("Perfoming post-deployment operations");
// Note that we *always* initialize this container storage, even
// if there are no bound images today.
let imgstore = sysroot.get_ensure_imgstore()?;
// Now copy each bound image from the host's container storage into the target.
for image in bound_images {
let image = image.image.as_str();
sysroot.imgstore.pull_from_host_storage(image).await?;
imgstore.pull_from_host_storage(image).await?;
}

Ok(())
Expand Down
2 changes: 1 addition & 1 deletion lib/src/status.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ fn boot_entry_from_deployment(
let store = deployment.store()?;
let store = store.as_ref().unwrap_or(&sysroot.store);
let spec = Some(store.spec());
let status = store.imagestatus(sysroot, deployment, image)?;
let status = store.imagestatus(&*sysroot, deployment, image)?;

(spec, status)
} else {
Expand Down
22 changes: 16 additions & 6 deletions lib/src/store/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::cell::OnceCell;
use std::env;
use std::ops::Deref;

Expand All @@ -16,8 +17,8 @@ mod ostree_container;

pub(crate) struct Storage {
pub sysroot: SysrootLock,
#[allow(dead_code)]
pub imgstore: crate::imgstorage::Storage,
run: Dir,
imgstore: OnceCell<crate::imgstorage::Storage>,
pub store: Box<dyn ContainerImageStoreImpl>,
}

Expand Down Expand Up @@ -52,6 +53,7 @@ impl Deref for Storage {

impl Storage {
pub fn new(sysroot: SysrootLock, run: &Dir) -> Result<Self> {
let run = run.try_clone()?;
let store = match env::var("BOOTC_STORAGE") {
Ok(val) => crate::spec::Store::from_str(&val, true).unwrap_or_else(|_| {
let default = crate::spec::Store::default();
Expand All @@ -61,17 +63,25 @@ impl Storage {
Err(_) => crate::spec::Store::default(),
};

let sysroot_dir = Dir::reopen_dir(&crate::utils::sysroot_fd(&sysroot))?;
let imgstore = crate::imgstorage::Storage::open(&sysroot_dir, run)?;

let store = load(store);

Ok(Self {
sysroot,
run,
store,
imgstore,
imgstore: Default::default(),
})
}

/// Access the image storage; will automatically initialize it if necessary.
pub(crate) fn get_ensure_imgstore(&self) -> Result<&crate::imgstorage::Storage> {
if let Some(imgstore) = self.imgstore.get() {
return Ok(imgstore);
}
let sysroot_dir = Dir::reopen_dir(&crate::utils::sysroot_fd(&self.sysroot))?;
let imgstore = crate::imgstorage::Storage::create(&sysroot_dir, &self.run)?;
Ok(self.imgstore.get_or_init(|| imgstore))
}
}

impl ContainerImageStore for ostree::Deployment {
Expand Down

0 comments on commit 519df35

Please sign in to comment.