From 37c7228d31bb4cbf3877c2146eca2a267bf4891f Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Fri, 26 Apr 2024 11:54:02 -0400 Subject: [PATCH] daemon: use new finalization APIs Now that we've stabilized and made public deployment finalization APIs, let's use them. This also fixes an issue where when creating a locked deployment using the legacy API (i.e. touching the `/run/ostree/staged-deployment-locked` file before calling the staging API), if a staged deployment already exists, libostree would just nuke the lockfile (this behaviour was introduced in https://github.com/ostreedev/ostree/pull/3077). In theory the legacy API (via the lockfile) should keep working, but the core issue is that there's no way for libostree to know if the lockfile is carried-over state, or was freshly created for the current invocation. So let's not try to salvage the legacy API and just move over to the new one. We already have finalization tests; they will now test that the new API functions correctly. But stop looking for the legacy lockfile. We could instead inspect the staged deployment GVariant, but these checks were redundant anyway since the tests verify the finalization by actually rebooting and/or not use `finalize-deployment --allow-unlocked`. Fixes: https://github.com/coreos/fedora-coreos-tracker/issues/1691 --- rust/src/daemon.rs | 2 ++ src/daemon/rpmostree-sysroot-core.h | 4 ---- src/daemon/rpmostree-sysroot-upgrader.cxx | 16 +--------------- src/daemon/rpmostreed-transaction-types.cxx | 10 ++++++---- tests/vmcheck/test-misc-2.sh | 4 ---- 5 files changed, 9 insertions(+), 27 deletions(-) diff --git a/rust/src/daemon.rs b/rust/src/daemon.rs index 51f6d05fbe..b89605b24b 100644 --- a/rust/src/daemon.rs +++ b/rust/src/daemon.rs @@ -166,6 +166,8 @@ pub(crate) fn deployment_populate_variant( /* Staging status */ dict.insert("staged", &deployment.is_staged()); if deployment.is_staged() + // XXX: this should check deployment.is_finalization_locked() too now + // but the libostree Rust bindings need to be updated && std::path::Path::new("/run/ostree/staged-deployment-locked").exists() { dict.insert("finalization-locked", &true); diff --git a/src/daemon/rpmostree-sysroot-core.h b/src/daemon/rpmostree-sysroot-core.h index 14366602b7..03b85787d7 100644 --- a/src/daemon/rpmostree-sysroot-core.h +++ b/src/daemon/rpmostree-sysroot-core.h @@ -33,10 +33,6 @@ G_BEGIN_DECLS /* The legacy dir, which we will just delete if we find it */ #define RPMOSTREE_OLD_TMP_ROOTFS_DIR "extensions/rpmostree/commit" -/* Really, this is an OSTree API, but let's consider it hidden for now like the - * /run/ostree/staged-deployment path and company. */ -#define _OSTREE_SYSROOT_RUNSTATE_STAGED_LOCKED "/run/ostree/staged-deployment-locked" - gboolean rpmostree_syscore_cleanup (OstreeSysroot *sysroot, OstreeRepo *repo, GCancellable *cancellable, GError **error); diff --git a/src/daemon/rpmostree-sysroot-upgrader.cxx b/src/daemon/rpmostree-sysroot-upgrader.cxx index 8a799c7f04..baa4f78e39 100644 --- a/src/daemon/rpmostree-sysroot-upgrader.cxx +++ b/src/daemon/rpmostree-sysroot-upgrader.cxx @@ -1396,27 +1396,13 @@ rpmostree_sysroot_upgrader_deploy (RpmOstreeSysrootUpgrader *self, } OstreeSysrootDeployTreeOpts opts = { + .locked = (self->flags & RPMOSTREE_SYSROOT_UPGRADER_FLAGS_LOCK_FINALIZATION), .override_kernel_argv = self->kargs_strv, .overlay_initrds = (char **)overlay_v, }; if (use_staging) { - /* touch file *before* we stage to avoid races */ - if (self->flags & RPMOSTREE_SYSROOT_UPGRADER_FLAGS_LOCK_FINALIZATION) - { - if (!glnx_shutil_mkdir_p_at (AT_FDCWD, - dirname (strdupa (_OSTREE_SYSROOT_RUNSTATE_STAGED_LOCKED)), - 0755, cancellable, error)) - return FALSE; - - glnx_autofd int fd = open (_OSTREE_SYSROOT_RUNSTATE_STAGED_LOCKED, - O_CREAT | O_WRONLY | O_NOCTTY | O_CLOEXEC, 0640); - if (fd == -1) - return glnx_throw_errno_prefix (error, "touch(%s)", - _OSTREE_SYSROOT_RUNSTATE_STAGED_LOCKED); - } - auto task = rpmostreecxx::progress_begin_task ("Staging deployment"); if (!ostree_sysroot_stage_tree_with_options (self->sysroot, self->osname, target_revision, origin, self->cfg_merge_deployment, &opts, diff --git a/src/daemon/rpmostreed-transaction-types.cxx b/src/daemon/rpmostreed-transaction-types.cxx index 18bfa834a5..af35fa8457 100644 --- a/src/daemon/rpmostreed-transaction-types.cxx +++ b/src/daemon/rpmostreed-transaction-types.cxx @@ -2546,14 +2546,16 @@ finalize_deployment_transaction_execute (RpmostreedTransaction *transaction, if (!check_sd_inhibitor_locks (cancellable, error)) return FALSE; - if (unlink (_OSTREE_SYSROOT_RUNSTATE_STAGED_LOCKED) < 0) + if (!ostree_deployment_is_finalization_locked (default_deployment)) { - if (errno != ENOENT) - return glnx_throw_errno_prefix (error, "unlink(%s)", - _OSTREE_SYSROOT_RUNSTATE_STAGED_LOCKED); if (!vardict_lookup_bool (self->options, "allow-unlocked", FALSE)) return glnx_throw (error, "Staged deployment already unlocked"); } + else + { + if (!ostree_sysroot_change_finalization (sysroot, default_deployment, error)) + return glnx_prefix_error (error, "Failed to unlock finalization of staged deployment"); + } /* And bump sysroot mtime so we reload... a bit awkward, though this is similar to * libostree itself doing this for `ostree admin unlock` (and possibly an `ostree admin` diff --git a/tests/vmcheck/test-misc-2.sh b/tests/vmcheck/test-misc-2.sh index 2bb9d41bdc..6c30f5a5d5 100755 --- a/tests/vmcheck/test-misc-2.sh +++ b/tests/vmcheck/test-misc-2.sh @@ -34,14 +34,12 @@ assert_file_has_content status.txt 'Commit:' booted_csum=$(vm_get_booted_csum) commit=$(vm_cmd ostree commit -b vmcheck --tree=ref=vmcheck --bootable) vm_rpmostree deploy revision="${commit}" --lock-finalization -vm_cmd test -f /run/ostree/staged-deployment-locked cursor=$(vm_get_journal_cursor) vm_reboot assert_streq "$(vm_get_booted_csum)" "${booted_csum}" vm_assert_journal_has_content $cursor 'Not finalizing; deployment is locked for finalization' echo "ok locked deploy staging" vm_rpmostree rebase :"${commit}" --lock-finalization --skip-purge -vm_cmd test -f /run/ostree/staged-deployment-locked cursor=$(vm_get_journal_cursor) vm_reboot assert_streq "$(vm_get_booted_csum)" "${booted_csum}" @@ -56,7 +54,6 @@ cursor=$(vm_get_journal_cursor) vm_cmd env RPMOSTREE_CLIENT_ID=testing-agent-id \ rpm-ostree deploy revision="${commit}" \ --lock-finalization -vm_cmd test -f /run/ostree/staged-deployment-locked if vm_rpmostree finalize-deployment; then assert_not_reached "finalized without expected checksum" elif vm_rpmostree finalize-deployment WRONG_CHECKSUM; then @@ -71,7 +68,6 @@ if vm_rpmostree finalize-deployment "${commit}" 2>err.txt; then assert_not_reached "finalized with inhibitor lock in block mode present" fi assert_file_has_content err.txt 'Reboot blocked' -vm_cmd test -f /run/ostree/staged-deployment-locked # Make sure that staged deployment is still locked. vm_cmd touch /run/wakeup sleep 1 # Wait one second for the process holding the lock to exit. vm_reboot_cmd rpm-ostree finalize-deployment "${commit}"