From 600fc98018b7540eaee5c053c30f7e050fd8f9b3 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 1 Sep 2023 07:54:09 -0400 Subject: [PATCH 1/2] Update to ostree-ext 0.11.6 --- Cargo.lock | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5b1492f372..b56792b01f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2194,16 +2194,15 @@ dependencies = [ [[package]] name = "ostree-ext" -version = "0.11.5" +version = "0.11.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "35e5fef7847d469bfffdc14800d9e76bf5c21acb136429c613ed2728b7f2d6a4" +checksum = "2fddb6a46fd0ce94594921d4b37796edda8c45bd239862f6e7429258cd16d423" dependencies = [ "anyhow", "async-compression 0.3.15", "bitflags 1.3.2", "camino", "cap-std-ext", - "cap-tempfile", "chrono", "clap", "containers-image-proxy", From 2ed82eea08ba0e4c9d4639b0c39f380dcb45bbf7 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 31 Aug 2023 08:33:05 -0400 Subject: [PATCH 2/2] Remove unreferenced container images in cleanup, not rebase This allows users to pin a deployment and have the container image be retained. Closes: https://github.com/coreos/rpm-ostree/issues/4391 --- rpmostree-cxxrs.cxx | 10 +++--- rpmostree-cxxrs.h | 3 +- rust/src/lib.rs | 2 +- rust/src/sysroot_upgrade.rs | 37 +++++++--------------- src/daemon/rpmostree-sysroot-core.cxx | 2 +- tests/kolainst/destructive/container-image | 8 ++++- 6 files changed, 26 insertions(+), 36 deletions(-) diff --git a/rpmostree-cxxrs.cxx b/rpmostree-cxxrs.cxx index 750b5fe17e..42b1de545f 100644 --- a/rpmostree-cxxrs.cxx +++ b/rpmostree-cxxrs.cxx @@ -2063,9 +2063,8 @@ extern "C" ::rpmostreecxx::OstreeRepo const &repo, ::rpmostreecxx::GCancellable const &cancellable, ::rust::Str imgref, ::rust::Box< ::rpmostreecxx::ContainerImageState> *return$) noexcept; - ::rust::repr::PtrLen rpmostreecxx$cxxbridge1$container_prune ( - ::rpmostreecxx::OstreeRepo const &repo, - ::rpmostreecxx::GCancellable const &cancellable) noexcept; + ::rust::repr::PtrLen + rpmostreecxx$cxxbridge1$container_prune (::rpmostreecxx::OstreeSysroot const &sysroot) noexcept; ::rust::repr::PtrLen rpmostreecxx$cxxbridge1$query_container_image_commit ( ::rpmostreecxx::OstreeRepo const &repo, ::rust::Str c, @@ -3659,10 +3658,9 @@ pull_container (::rpmostreecxx::OstreeRepo const &repo, } void -container_prune (::rpmostreecxx::OstreeRepo const &repo, - ::rpmostreecxx::GCancellable const &cancellable) +container_prune (::rpmostreecxx::OstreeSysroot const &sysroot) { - ::rust::repr::PtrLen error$ = rpmostreecxx$cxxbridge1$container_prune (repo, cancellable); + ::rust::repr::PtrLen error$ = rpmostreecxx$cxxbridge1$container_prune (sysroot); if (error$.ptr) { throw ::rust::impl< ::rust::Error>::error (error$); diff --git a/rpmostree-cxxrs.h b/rpmostree-cxxrs.h index db9a687f31..488776a899 100644 --- a/rpmostree-cxxrs.h +++ b/rpmostree-cxxrs.h @@ -1767,8 +1767,7 @@ ::rust::Box< ::rpmostreecxx::ContainerImageState> pull_container (::rpmostreecxx::OstreeRepo const &repo, ::rpmostreecxx::GCancellable const &cancellable, ::rust::Str imgref); -void container_prune (::rpmostreecxx::OstreeRepo const &repo, - ::rpmostreecxx::GCancellable const &cancellable); +void container_prune (::rpmostreecxx::OstreeSysroot const &sysroot); ::rust::Box< ::rpmostreecxx::ContainerImageState> query_container_image_commit (::rpmostreecxx::OstreeRepo const &repo, ::rust::Str c); diff --git a/rust/src/lib.rs b/rust/src/lib.rs index d7d767612d..1bd877a620 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -196,7 +196,7 @@ pub mod ffi { cancellable: &GCancellable, imgref: &str, ) -> Result>; - fn container_prune(repo: &OstreeRepo, cancellable: &GCancellable) -> Result<()>; + fn container_prune(sysroot: &OstreeSysroot) -> Result<()>; fn query_container_image_commit( repo: &OstreeRepo, c: &str, diff --git a/rust/src/sysroot_upgrade.rs b/rust/src/sysroot_upgrade.rs index b2cc640489..e4a483f9a5 100644 --- a/rust/src/sysroot_upgrade.rs +++ b/rust/src/sysroot_upgrade.rs @@ -4,9 +4,8 @@ use crate::cxxrsutil::*; use crate::ffi::{output_message, ContainerImageState}; -use anyhow::Result; +use anyhow::{Context, Result}; use ostree::glib; -use ostree::prelude::*; use ostree_container::store::{ ImageImporter, ImageProxyConfig, ImportProgress, ManifestLayerState, PrepareResult, }; @@ -140,28 +139,16 @@ pub(crate) fn pull_container( Ok(Box::new(r)) } -pub(crate) fn layer_prune( - repo: &ostree::Repo, - cancellable: Option<&ostree::gio::Cancellable>, -) -> Result<()> { - if let Some(c) = cancellable { - c.set_error_if_cancelled()?; - } - tracing::debug!("pruning image layers"); - crate::try_fail_point!("sysroot-upgrade::layer-prune"); - let n_pruned = ostree_ext::container::store::gc_image_layers(repo)?; - systemd::journal::print(6, &format!("Pruned container image layers: {n_pruned}")); +/// Run a prune of container images that are not referenced by a deployment. +pub(crate) fn container_prune(sysroot: &crate::FFIOstreeSysroot) -> CxxResult<()> { + let sysroot = &sysroot.glib_reborrow(); + tracing::debug!("Pruning container images"); + crate::try_fail_point!("sysroot-upgrade::container-prune"); + let sysroot = &ostree_ext::sysroot::SysrootLock::from_assumed_locked(sysroot); + ostree_container::deploy::remove_undeployed_images(sysroot).context("Pruning images")?; Ok(()) } -/// Run a prune of container image layers. -pub(crate) fn container_prune( - repo: &crate::FFIOstreeRepo, - cancellable: &crate::FFIGCancellable, -) -> CxxResult<()> { - layer_prune(&repo.glib_reborrow(), Some(&cancellable.glib_reborrow())).map_err(Into::into) -} - /// C++ wrapper for querying image state; requires a pulled image pub(crate) fn query_container_image_commit( repo: &crate::FFIOstreeRepo, @@ -176,10 +163,10 @@ pub(crate) fn query_container_image_commit( pub(crate) fn purge_refspec(repo: &crate::FFIOstreeRepo, imgref: &str) -> CxxResult<()> { let repo = &repo.glib_reborrow(); tracing::debug!("Purging {imgref}"); - if let Ok(cref) = OstreeImageReference::try_from(imgref) { - // It's a container, use the ostree-ext APIs to prune it. - ostree_container::store::remove_image(repo, &cref.imgref)?; - layer_prune(repo, None)?; + if OstreeImageReference::try_from(imgref).is_ok() { + // It's a container, so we will defer to the new model of pruning + // container images that have no deployments in cleanup. + tracing::debug!("No action needed"); } else if ostree::validate_checksum_string(imgref).is_ok() { // Nothing to do here } else { diff --git a/src/daemon/rpmostree-sysroot-core.cxx b/src/daemon/rpmostree-sysroot-core.cxx index 6f28252133..946266c509 100644 --- a/src/daemon/rpmostree-sysroot-core.cxx +++ b/src/daemon/rpmostree-sysroot-core.cxx @@ -214,7 +214,7 @@ rpmostree_syscore_cleanup (OstreeSysroot *sysroot, OstreeRepo *repo, GCancellabl /* Refs for the live state */ ROSCXX_TRY (applylive_sync_ref (*sysroot), error); - CXX_TRY (rpmostreecxx::container_prune (*repo, *cancellable), error); + CXX_TRY (rpmostreecxx::container_prune (*sysroot), error); /* And do a prune */ guint64 freed_space; diff --git a/tests/kolainst/destructive/container-image b/tests/kolainst/destructive/container-image index fc812d975f..2222f78281 100755 --- a/tests/kolainst/destructive/container-image +++ b/tests/kolainst/destructive/container-image @@ -164,7 +164,8 @@ EOF fi rpm-ostree rebase ostree-unverified-image:containers-storage:localhost/fcos-derived ostree container image list --repo=/ostree/repo | tee imglist.txt - assert_streq "$(wc -l < imglist.txt)" 1 + # We now only prune when the deployment is removed + assert_streq "$(wc -l < imglist.txt)" 2 rm $image_dir -rf /tmp/autopkgtest-reboot 3 ;; @@ -187,6 +188,11 @@ EOF echo -e "[Daemon]\nAutomaticUpdatePolicy=apply" > /etc/rpm-ostreed.conf rpm-ostree reload + # This should now prune the previous image + rpm-ostree cleanup -pr + ostree container image list --repo=/ostree/repo | tee imglist.txt + assert_streq "$(wc -l < imglist.txt)" 1 + # Now revert back to the base image, but keep our layered package foo rpm-ostree rebase ostree-unverified-image:containers-storage:localhost/fcos /tmp/autopkgtest-reboot 4