Skip to content

Commit

Permalink
When rebasing, prune previous container by default
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
cgwalters committed Nov 11, 2022
1 parent 545f50d commit a54b4e9
Show file tree
Hide file tree
Showing 6 changed files with 54 additions and 12 deletions.
14 changes: 14 additions & 0 deletions rpmostree-cxxrs.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
{
Expand Down
2 changes: 2 additions & 0 deletions rpmostree-cxxrs.h
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
1 change: 1 addition & 0 deletions rust/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ pub mod ffi {
repo: &OstreeRepo,
c: &str,
) -> Result<Box<ContainerImageState>>;
fn purge_refspec(repo: &OstreeRepo, refspec: &str) -> Result<()>;
}

// core.rs
Expand Down
31 changes: 31 additions & 0 deletions rust/src/sysroot_upgrade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}
14 changes: 2 additions & 12 deletions src/daemon/rpmostreed-transaction-types.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions tests/kolainst/destructive/container-image
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
Expand Down Expand Up @@ -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
;;
Expand Down

0 comments on commit a54b4e9

Please sign in to comment.