diff --git a/ci/test-container.sh b/ci/test-container.sh index 89b2eb6fbe..b1bb9cfd50 100755 --- a/ci/test-container.sh +++ b/ci/test-container.sh @@ -13,16 +13,25 @@ versionid=$(. /usr/lib/os-release && echo $VERSION_ID) # These hardcoded versions can be kept until Fedora GC's them ignition_url_suffix=2.17.0/4.fc40/x86_64/ignition-2.17.0-4.fc40."$(arch)".rpm case $versionid in + 43) + # We use the same as f42 for 43 as they are compatible + koji_ignition_url="https://koji.fedoraproject.org/koji/buildinfo?buildID=2681489" + koji_kernel_url="https://koji.fedoraproject.org/koji/buildinfo?buildID=2805991" + kver=6.17.0 + krev=0.rc3.31 + ;; 42) - # 2.21.0-1 (this koji url must be different than above version, and different from - # what's in the current image) + # This packages need to be different from what's in the current image. + # 2.21.0-1 koji_ignition_url="https://koji.fedoraproject.org/koji/buildinfo?buildID=2681489" koji_kernel_url="https://koji.fedoraproject.org/koji/buildinfo?buildID=2685011" kver=6.14.0 krev=63 ;; 41) - # 2.19.0-2 (this koji url must be different than above version) + # This koji url must be different than above version, and different from + # what's in the current image. + # 2.19.0-2 koji_ignition_url="https://koji.fedoraproject.org/koji/buildinfo?buildID=2495227" koji_kernel_url="https://koji.fedoraproject.org/koji/buildinfo?buildID=2571615" kver=6.11.4 diff --git a/src/daemon/rpmostree-sysroot-upgrader.cxx b/src/daemon/rpmostree-sysroot-upgrader.cxx index a98db16249..a007023f18 100644 --- a/src/daemon/rpmostree-sysroot-upgrader.cxx +++ b/src/daemon/rpmostree-sysroot-upgrader.cxx @@ -111,10 +111,11 @@ struct RpmOstreeSysrootUpgrader gboolean layering_initialized; /* Whether layering_type is known */ RpmOstreeSysrootUpgraderLayeringType layering_type; - gboolean layering_changed; /* Whether changes to layering should result in a new commit */ - gboolean pkgs_imported; /* Whether pkgs to be layered have been downloaded & imported */ - char *base_revision; /* Non-layered replicated commit */ - char *final_revision; /* Computed by layering; if NULL, only using base_revision */ + gboolean layering_changed; /* Whether changes to layering should result in a new commit */ + gboolean allow_empty_transaction; /* Allow empty DNF transaction for idempotent layering */ + gboolean pkgs_imported; /* Whether pkgs to be layered have been downloaded & imported */ + char *base_revision; /* Non-layered replicated commit */ + char *final_revision; /* Computed by layering; if NULL, only using base_revision */ char **kargs_strv; /* Kernel argument list to be written into deployment */ }; @@ -401,6 +402,13 @@ rpmostree_sysroot_upgrader_get_sack (RpmOstreeSysrootUpgrader *self, GError **er return self->rpmmd_sack; } +void +rpmostree_sysroot_upgrader_set_allow_empty_transaction (RpmOstreeSysrootUpgrader *self, + gboolean allow) +{ + self->allow_empty_transaction = allow; +} + /* * Like ostree_sysroot_upgrader_pull(), but also handles the `baserefspec` we * use when doing layered packages. @@ -998,10 +1006,24 @@ prep_local_assembly (RpmOstreeSysrootUpgrader *self, GCancellable *cancellable, self->layering_changed = strcmp (previous_state_sha512, new_state_sha512) != 0; } else - /* Otherwise, we're transitioning from not-layered to layered, so it - definitely changed */ - self->layering_changed = TRUE; - + { + /* Otherwise, we're transitioning from not-layered to layered, so it + definitely changed */ + self->layering_changed = TRUE; + /* Special case for containers: if the DNF transaction is empty (all packages already in + * base), don't consider this a change even if transitioning from unlayered to layered. This + * happens with idempotent layering when packages are already in the container image. */ + if (self->layering_type == RPMOSTREE_SYSROOT_UPGRADER_LAYERING_RPMMD_REPOS) + { + auto refspec = rpmostree_origin_get_refspec (self->computed_origin); + if (refspec.kind == rpmostreecxx::RefspecType::Container + && rpmostree_dnf_context_has_empty_transaction ( + rpmostree_context_get_dnf (self->ctx))) + { + self->layering_changed = FALSE; + } + } + } return TRUE; } @@ -1021,6 +1043,7 @@ perform_local_assembly (RpmOstreeSysrootUpgrader *self, GCancellable *cancellabl rpmostree_context_set_devino_cache (self->ctx, self->devino_cache); rpmostree_context_set_tmprootfs_dfd (self->ctx, self->tmprootfs_dfd); + rpmostree_context_set_allow_empty_transaction (self->ctx, self->allow_empty_transaction); if (self->layering_type == RPMOSTREE_SYSROOT_UPGRADER_LAYERING_RPMMD_REPOS) { diff --git a/src/daemon/rpmostree-sysroot-upgrader.h b/src/daemon/rpmostree-sysroot-upgrader.h index 7a90dfe32a..b8f60268de 100644 --- a/src/daemon/rpmostree-sysroot-upgrader.h +++ b/src/daemon/rpmostree-sysroot-upgrader.h @@ -98,6 +98,9 @@ const char *rpmostree_sysroot_upgrader_get_base (RpmOstreeSysrootUpgrader *self) DnfSack *rpmostree_sysroot_upgrader_get_sack (RpmOstreeSysrootUpgrader *self, GError **error); +void rpmostree_sysroot_upgrader_set_allow_empty_transaction (RpmOstreeSysrootUpgrader *self, + gboolean allow); + gboolean rpmostree_sysroot_upgrader_pull_base (RpmOstreeSysrootUpgrader *self, const char *dir_to_pull, OstreeRepoPullFlags flags, OstreeAsyncProgress *progress, gboolean *out_changed, diff --git a/src/daemon/rpmostreed-transaction-types.cxx b/src/daemon/rpmostreed-transaction-types.cxx index 7109c2813b..9937d4dd05 100644 --- a/src/daemon/rpmostreed-transaction-types.cxx +++ b/src/daemon/rpmostreed-transaction-types.cxx @@ -1128,6 +1128,8 @@ deploy_transaction_execute (RpmostreedTransaction *transaction, GCancellable *ca if (upgrader == NULL) return FALSE; + rpmostree_sysroot_upgrader_set_allow_empty_transaction (upgrader, idempotent_layering); + g_autoptr (RpmOstreeOrigin) origin = rpmostree_sysroot_upgrader_dup_origin (upgrader); /* Handle local repo remotes immediately; the idea is that the remote is "transient" @@ -1191,6 +1193,7 @@ deploy_transaction_execute (RpmostreedTransaction *transaction, GCancellable *ca } gboolean changed = FALSE; + gboolean remove_changed = FALSE; if (no_initramfs && (rpmostree_origin_get_regenerate_initramfs (origin) || rpmostree_origin_has_initramfs_etc_files (origin))) @@ -1214,7 +1217,10 @@ deploy_transaction_execute (RpmostreedTransaction *transaction, GCancellable *ca if (no_layering) { if (rpmostree_origin_remove_all_packages (origin)) - changed = TRUE; + { + changed = TRUE; + remove_changed = TRUE; + } } else { @@ -1223,43 +1229,60 @@ deploy_transaction_execute (RpmostreedTransaction *transaction, GCancellable *ca * create a new deployment; see https://github.com/projectatomic/rpm-ostree/issues/753 */ if (!rpmostree_origin_remove_packages (origin, util::rust_stringvec_from_strv (uninstall_pkgs), - idempotent_layering, &changed, error)) + idempotent_layering, &remove_changed, error)) return FALSE; + if (remove_changed) + changed = TRUE; } /* lazily loaded cache that's used in a few conditional blocks */ g_autoptr (RpmOstreeRefSack) base_rsack = NULL; + /* Check if we should skip the base-db check: + * When booted from a container image on an ostree host, + * we skip the base commit rpmdb check because container images don't have + * meaningful base-db separation like traditional ostree commits do. */ + gboolean skip_base_check = FALSE; + CXX_TRY_VAR (host_type, rpmostreecxx::get_system_host_type (), error); + if (host_type == rpmostreecxx::SystemHostType::OstreeHost) + { + const auto refspec = rpmostree_origin_get_refspec (origin); + skip_base_check = (refspec.kind == rpmostreecxx::RefspecType::Container); + } + if (install_pkgs) { /* we run a special check here; let's just not allow trying to install a pkg that will * right away become inactive because it's already installed */ - if (!base_rsack) + if (!skip_base_check) { - const char *base = rpmostree_sysroot_upgrader_get_base (upgrader); - base_rsack = rpmostree_get_refsack_for_commit (repo, base, cancellable, error); - if (base_rsack == NULL) - return FALSE; - } + if (!base_rsack) + { + const char *base = rpmostree_sysroot_upgrader_get_base (upgrader); + base_rsack = rpmostree_get_refsack_for_commit (repo, base, cancellable, error); + if (base_rsack == NULL) + return FALSE; + } - for (char **it = install_pkgs; it && *it; it++) - { - const char *pkg = *it; - g_autoptr (GPtrArray) pkgs = rpmostree_get_matching_packages (base_rsack->sack, pkg); - if (pkgs->len > 0 && !allow_inactive) + for (char **it = install_pkgs; it && *it; it++) { - g_autoptr (GString) pkgnames = g_string_new (""); - for (guint i = 0; i < pkgs->len; i++) + const char *pkg = *it; + g_autoptr (GPtrArray) pkgs = rpmostree_get_matching_packages (base_rsack->sack, pkg); + if (pkgs->len > 0 && !allow_inactive) { - auto p = static_cast (pkgs->pdata[i]); - g_string_append_printf (pkgnames, " %s", dnf_package_get_nevra (p)); + g_autoptr (GString) pkgnames = g_string_new (""); + for (guint i = 0; i < pkgs->len; i++) + { + auto p = static_cast (pkgs->pdata[i]); + g_string_append_printf (pkgnames, " %s", dnf_package_get_nevra (p)); + } + return glnx_throw (error, + "\"%s\" is already provided by:%s. Use " + "--allow-inactive to explicitly " + "require it.", + pkg, pkgnames->str); } - return glnx_throw (error, - "\"%s\" is already provided by:%s. Use " - "--allow-inactive to explicitly " - "require it.", - pkg, pkgnames->str); } } @@ -1565,6 +1588,15 @@ deploy_transaction_execute (RpmostreedTransaction *transaction, GCancellable *ca return FALSE; changed = changed || layering_changed; + /* When using containers and nothing changed after prep_layering, return early. + * This happens when all requested packages are already present in the container image. + * For idempotent mode, this avoids the "No packages in transaction" error. + * However, if packages were removed from the origin, we need to deploy to update the origin + * even if no layering is needed. Similarly, if we're rebasing (refspec) or deploying a + * specific revision, we need to deploy. */ + if (skip_base_check && !layering_changed && !remove_changed && !self->refspec && !self->revision) + return TRUE; + if (dry_run) /* Note early return here; we printed the transaction already */ return TRUE; diff --git a/src/libpriv/rpmostree-core-private.h b/src/libpriv/rpmostree-core-private.h index e4bca114b4..36915fc9b5 100644 --- a/src/libpriv/rpmostree-core-private.h +++ b/src/libpriv/rpmostree-core-private.h @@ -40,6 +40,7 @@ struct _RpmOstreeContext std::optional> treefile_owned; rpmostreecxx::Treefile *treefile_rs; /* For composes for now */ gboolean empty; + gboolean allow_empty_transaction; gboolean disable_selinux; char *ref; diff --git a/src/libpriv/rpmostree-core.cxx b/src/libpriv/rpmostree-core.cxx index f195775810..5ef1d677db 100644 --- a/src/libpriv/rpmostree-core.cxx +++ b/src/libpriv/rpmostree-core.cxx @@ -381,6 +381,12 @@ rpmostree_context_set_is_empty (RpmOstreeContext *self) self->empty = TRUE; } +void +rpmostree_context_set_allow_empty_transaction (RpmOstreeContext *self, gboolean allow) +{ + self->allow_empty_transaction = allow; +} + void rpmostree_context_disable_selinux (RpmOstreeContext *self) { @@ -784,7 +790,8 @@ rpmostree_context_setup (RpmOstreeContext *self, const char *install_root, const * we're only installing local RPMs. Missing deps will cause the regular * 'not found' error from libdnf. */ auto pkgs = self->treefile_rs->get_packages (); - if (!pkgs.empty ()) + auto local_pkgs = self->treefile_rs->get_local_packages (); + if (!pkgs.empty () && local_pkgs.empty ()) return glnx_throw (error, "No enabled repositories"); } @@ -4229,8 +4236,11 @@ rpmostree_context_assemble (RpmOstreeContext *self, GCancellable *cancellable, G g_autoptr (GPtrArray) overrides_remove = dnf_goal_get_packages ( dnf_context_get_goal (dnfctx), DNF_PACKAGE_INFO_REMOVE, DNF_PACKAGE_INFO_OBSOLETE, -1); - if (overlays->len == 0 && overrides_remove->len == 0 && overrides_replace->len == 0) - return glnx_throw (error, "No packages in transaction"); + if (rpmostree_dnf_context_has_empty_transaction (dnfctx)) + { + if (!self->allow_empty_transaction) + return glnx_throw (error, "No packages in transaction"); + } /* Sort the packages as rpmtsOrder() only reorder to satisfy dependencies * but doesn't impose any ordering to packages with the same dependencies. diff --git a/src/libpriv/rpmostree-core.h b/src/libpriv/rpmostree-core.h index 4a07918376..72ececb5ad 100644 --- a/src/libpriv/rpmostree-core.h +++ b/src/libpriv/rpmostree-core.h @@ -109,6 +109,7 @@ void rpmostree_context_configure_from_deployment (RpmOstreeContext *self, Ostree OstreeDeployment *cfg_deployment); void rpmostree_context_set_is_empty (RpmOstreeContext *self); +void rpmostree_context_set_allow_empty_transaction (RpmOstreeContext *self, gboolean allow); void rpmostree_context_disable_selinux (RpmOstreeContext *self); const char *rpmostree_context_get_ref (RpmOstreeContext *self); diff --git a/src/libpriv/rpmostree-rpm-util.cxx b/src/libpriv/rpmostree-rpm-util.cxx index 7281c9084f..8634e4c24e 100644 --- a/src/libpriv/rpmostree-rpm-util.cxx +++ b/src/libpriv/rpmostree-rpm-util.cxx @@ -1048,6 +1048,21 @@ rpmostree_print_transaction (DnfContext *dnfctx) rpmostree_output_message ("Empty transaction"); } +/* Helper function to check if a DNF transaction has any packages to install, remove, or update */ +gboolean +rpmostree_dnf_context_has_empty_transaction (DnfContext *dnfctx) +{ + HyGoal goal = dnf_context_get_goal (dnfctx); + + g_autoptr (GPtrArray) install_pkgs = dnf_goal_get_packages (goal, DNF_PACKAGE_INFO_INSTALL, -1); + g_autoptr (GPtrArray) remove_pkgs + = dnf_goal_get_packages (goal, DNF_PACKAGE_INFO_REMOVE, DNF_PACKAGE_INFO_OBSOLETE, -1); + g_autoptr (GPtrArray) update_pkgs + = dnf_goal_get_packages (goal, DNF_PACKAGE_INFO_UPDATE, DNF_PACKAGE_INFO_DOWNGRADE, -1); + + return (install_pkgs->len == 0 && remove_pkgs->len == 0 && update_pkgs->len == 0); +} + G_DEFINE_AUTO_CLEANUP_FREE_FUNC (cap_t, cap_free, NULL) GVariant * diff --git a/src/libpriv/rpmostree-rpm-util.h b/src/libpriv/rpmostree-rpm-util.h index cc28a01be2..f9cc15db55 100644 --- a/src/libpriv/rpmostree-rpm-util.h +++ b/src/libpriv/rpmostree-rpm-util.h @@ -117,6 +117,8 @@ gint rpmostree_pkg_array_compare (DnfPackage **p_pkg1, DnfPackage **p_pkg2); void rpmostree_print_transaction (DnfContext *context); +gboolean rpmostree_dnf_context_has_empty_transaction (DnfContext *dnfctx); + GVariant *rpmostree_fcap_to_ostree_xattr (const char *fcap, GError **error); GVariant *rpmostree_fcap_to_xattr_variant (const char *fcap, GError **error); diff --git a/tests/kolainst/destructive/idempotent-layering b/tests/kolainst/destructive/idempotent-layering new file mode 100755 index 0000000000..344a59aef2 --- /dev/null +++ b/tests/kolainst/destructive/idempotent-layering @@ -0,0 +1,114 @@ +#!/bin/bash +## kola: +## # Increase timeout since this test involves rebasing and container operations +## timeoutMin: 20 +## # This test only runs on FCOS +## distros: fcos +## # Needs internet access for package installation +## tags: "needs-internet platform-independent" +## minMemory: 2048 +# +# Copyright (C) 2025 Red Hat, Inc. +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this library; if not, write to the +# Free Software Foundation, Inc., 59 Temple Place - Suite 330, +# Boston, MA 02111-1307, USA. + +set -euo pipefail + +. ${KOLA_EXT_DATA}/libtest.sh + +set -x + +cd "$(mktemp -d)" + +# TODO: It'd be much better to test this via a registry +image_dir=/var/tmp/fcos +image=oci:$image_dir + +case "${AUTOPKGTEST_REBOOT_MARK:-}" in + "") + checksum=$(rpm-ostree status --json | jq -r '.deployments[0].checksum') + rm -rf "${image_dir}" + # Since we're switching OS update stream, turn off zincati + systemctl mask --now zincati + ostree container encapsulate --repo=/ostree/repo ${checksum} "${image}" --label ostree.bootable=TRUE + + skopeo copy $image containers-storage:localhost/fcos + rm -rf "${image_dir}" + td=$(mktemp -d) + cd ${td} +cat > Containerfile << EOF +FROM localhost/fcos +# Pre-install vim in the container image +RUN dnf install -y vim-enhanced +EOF + + touched_resolv_conf=0 + if test '!' -f /etc/resolv.conf; then + podmanv=$(podman --version) + case "${podmanv#podman version }" in + 3.*) touched_resolv_conf=1; touch /etc/resolv.conf;; + esac + fi + podman build --net=host -t localhost/fcos-derived --squash . + if test "${touched_resolv_conf}" -eq 1; then + rm -vf /etc/resolv.conf + fi + + rpm-ostree rebase ostree-unverified-image:containers-storage:localhost/fcos-derived + rm -rf "$image_dir" + + /tmp/autopkgtest-reboot 1 + ;; + 1) + # Verify vim is installed from the base image + rpm -q vim-enhanced + rpm-ostree status + + # Test 1: Install with --idempotent should succeed without error + # even though vim-enhanced is already in the base image + rpm-ostree install --allow-inactive --idempotent --apply-live -y vim-enhanced + echo "ok idempotent install of existing package" + + + # Test 2: Install with --unchanged-exit-77 should exit with 77 + set +e + rpm-ostree install --allow-inactive --apply-live --unchanged-exit-77 -y vim-enhanced 2>&1 | tee out.txt + rc=$? + set -e + assert_streq "${rc}" "77" + echo "ok unchanged-exit-77 for existing package" + + # Test 3: Install with both --idempotent and --unchanged-exit-77 + set +e + rpm-ostree install --allow-inactive --idempotent --apply-live --unchanged-exit-77 -y vim-enhanced + rc=$? + set -e + assert_streq "${rc}" "77" + echo "ok idempotent with unchanged-exit-77 for existing package" + + # Test 4: Install a new package with --idempotent (should work normally) + rpm-ostree install --idempotent tmux + /tmp/autopkgtest-reboot 2 + ;; + 2) + # Verify tmux is now installed + rpm -q tmux + rpm -q vim-enhanced + echo "ok idempotent install of new package" + + ;; + *) echo "unexpected mark: ${AUTOPKGTEST_REBOOT_MARK}"; exit 1;; +esac