From a54b4e9886496aab471197bf2a0baca0ca581f2a Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 9 Nov 2022 16:55:05 -0500 Subject: [PATCH] When rebasing, prune previous container by default When using `rpm-ostree rebase` on ostree branch names, by default we prune the previous branch/ref to avoid leaking space (i.e. requiring the user/admin to manually prune it). We do have the `--skip-purge` option to suppress this behavior. When we added the container bits, we ignored this at the time. But we really need to do this by default, for the same reasons around avoiding space leakage. --- rpmostree-cxxrs.cxx | 14 ++++++++++ rpmostree-cxxrs.h | 2 ++ rust/src/lib.rs | 1 + rust/src/sysroot_upgrade.rs | 31 +++++++++++++++++++++ src/daemon/rpmostreed-transaction-types.cxx | 14 ++-------- tests/kolainst/destructive/container-image | 4 +++ 6 files changed, 54 insertions(+), 12 deletions(-) diff --git a/rpmostree-cxxrs.cxx b/rpmostree-cxxrs.cxx index 9277eb0d16..23ac4b97e2 100644 --- a/rpmostree-cxxrs.cxx +++ b/rpmostree-cxxrs.cxx @@ -2059,6 +2059,10 @@ extern "C" ::rust::repr::PtrLen rpmostreecxx$cxxbridge1$query_container_image_commit ( const ::rpmostreecxx::OstreeRepo &repo, ::rust::Str c, ::rust::Box< ::rpmostreecxx::ContainerImageState> *return$) noexcept; + + ::rust::repr::PtrLen + rpmostreecxx$cxxbridge1$purge_refspec (const ::rpmostreecxx::OstreeRepo &repo, + ::rust::Str refspec) noexcept; ::std::size_t rpmostreecxx$cxxbridge1$TempEtcGuard$operator$sizeof () noexcept; ::std::size_t rpmostreecxx$cxxbridge1$TempEtcGuard$operator$alignof () noexcept; ::std::size_t rpmostreecxx$cxxbridge1$FilesystemScriptPrep$operator$sizeof () noexcept; @@ -3618,6 +3622,16 @@ query_container_image_commit (const ::rpmostreecxx::OstreeRepo &repo, ::rust::St return ::std::move (return$.value); } +void +purge_refspec (const ::rpmostreecxx::OstreeRepo &repo, ::rust::Str refspec) +{ + ::rust::repr::PtrLen error$ = rpmostreecxx$cxxbridge1$purge_refspec (repo, refspec); + if (error$.ptr) + { + throw ::rust::impl< ::rust::Error>::error (error$); + } +} + ::std::size_t TempEtcGuard::layout::size () noexcept { diff --git a/rpmostree-cxxrs.h b/rpmostree-cxxrs.h index 8279b4e39b..5476ca7f23 100644 --- a/rpmostree-cxxrs.h +++ b/rpmostree-cxxrs.h @@ -1764,6 +1764,8 @@ pull_container (const ::rpmostreecxx::OstreeRepo &repo, ::rust::Box< ::rpmostreecxx::ContainerImageState> query_container_image_commit (const ::rpmostreecxx::OstreeRepo &repo, ::rust::Str c); +void purge_refspec (const ::rpmostreecxx::OstreeRepo &repo, ::rust::Str refspec); + ::rust::Box< ::rpmostreecxx::TempEtcGuard> prepare_tempetc_guard (::std::int32_t rootfs); ::rust::Box< ::rpmostreecxx::FilesystemScriptPrep> diff --git a/rust/src/lib.rs b/rust/src/lib.rs index 7dad68260d..f36a95a945 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -199,6 +199,7 @@ pub mod ffi { repo: &OstreeRepo, c: &str, ) -> Result>; + fn purge_refspec(repo: &OstreeRepo, refspec: &str) -> Result<()>; } // core.rs diff --git a/rust/src/sysroot_upgrade.rs b/rust/src/sysroot_upgrade.rs index 790bdd1e10..dcb0d4c0ab 100644 --- a/rust/src/sysroot_upgrade.rs +++ b/rust/src/sysroot_upgrade.rs @@ -154,3 +154,34 @@ pub(crate) fn query_container_image_commit( let state = ostree_container::store::query_image_commit(repo, imgcommit)?; Ok(Box::new(state.into())) } + +/// Remove a refspec, which can be either an ostree branch or a container image. +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. + let iref = &cref.imgref; + ostree_container::store::remove_images(repo, [iref])?; + let n = ostree_container::store::gc_image_layers(repo)?; + tracing::debug!("Pruned {n} layers"); + } else if ostree::validate_checksum_string(imgref).is_ok() { + // Nothing to do here + } else { + match ostree::parse_refspec(imgref) { + Ok((remote, ostreeref)) => { + repo.set_ref_immediate( + remote.as_ref().map(|s| s.as_str()), + &ostreeref, + None, + ostree::gio::NONE_CANCELLABLE, + )?; + } + Err(e) => { + // For historical reasons, we ignore errors here + tracing::warn!("{e}"); + } + } + } + Ok(()) +} diff --git a/src/daemon/rpmostreed-transaction-types.cxx b/src/daemon/rpmostreed-transaction-types.cxx index 66f603bc06..d3f7b732e0 100644 --- a/src/daemon/rpmostreed-transaction-types.cxx +++ b/src/daemon/rpmostreed-transaction-types.cxx @@ -1597,19 +1597,9 @@ deploy_transaction_execute (RpmostreedTransaction *transaction, GCancellable *ca return FALSE; /* Are we rebasing? May want to delete the previous ref */ - if (self->refspec && !(deploy_has_bool_option (self, "skip-purge"))) + if (self->refspec && !(deploy_has_bool_option (self, "skip-purge")) && old_refspec) { - g_autofree char *remote = NULL; - g_autofree char *ref = NULL; - - /* The actual rebase has already succeeded, so ignore errors. */ - if (old_refspec && ostree_parse_refspec (old_refspec, &remote, &ref, NULL)) - { - /* Note: In some cases the source origin ref may not actually - * exist; say the admin did a cleanup, or the OS expects post- - * install configuration like subscription-manager. */ - (void)ostree_repo_set_ref_immediate (repo, remote, ref, NULL, cancellable, NULL); - } + CXX_TRY (rpmostreecxx::purge_refspec (*repo, old_refspec), error); } /* Always write out an update variant on vanilla upgrades since it's clearly the most diff --git a/tests/kolainst/destructive/container-image b/tests/kolainst/destructive/container-image index aec7d09e9a..5df0c96c92 100755 --- a/tests/kolainst/destructive/container-image +++ b/tests/kolainst/destructive/container-image @@ -57,6 +57,8 @@ case "${AUTOPKGTEST_REBOOT_MARK:-}" in rpm-ostree status | tee out.txt assert_file_has_content_literal out.txt 'Digest: sha256:' + ostree container image list --repo=/ostree/repo | tee imglist.txt + assert_streq "$(wc -l < imglist.txt)" 1 v1=$(rpm-ostree status --json | jq -r '.deployments[0].version') assert_streq "${v0}" "${v1}" @@ -163,6 +165,8 @@ EOF derived=oci:$image_dir:derived skopeo copy containers-storage:localhost/fcos-derived $derived rpm-ostree rebase ostree-unverified-image:$derived + ostree container image list --repo=/ostree/repo | tee imglist.txt + assert_streq "$(wc -l < imglist.txt)" 1 rm $image_dir -rf /tmp/autopkgtest-reboot 3 ;;