From 648ab74617b3667c6fbe51116f7cb0af0b5dafe0 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Tue, 30 Jan 2024 09:53:34 -0500 Subject: [PATCH 1/5] composepost: Constantify `local-fs.target.requires` path Follow-up from #4728. --- rust/src/composepost.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/rust/src/composepost.rs b/rust/src/composepost.rs index a967ae4479..3183e3d1b5 100644 --- a/rust/src/composepost.rs +++ b/rust/src/composepost.rs @@ -52,6 +52,8 @@ pub(crate) const RPMOSTREE_RPMDB_LOCATION: &str = "usr/share/rpm"; const RPMOSTREE_SYSIMAGE_RPMDB: &str = "usr/lib/sysimage/rpm"; pub(crate) const TRADITIONAL_RPMDB_LOCATION: &str = "var/lib/rpm"; +const SD_LOCAL_FS_TARGET_REQUIRES: &str = "usr/lib/systemd/system/local-fs.target.requires"; + #[context("Moving {}", name)] fn dir_move_if_exists(src: &cap_std::fs::Dir, dest: &cap_std::fs::Dir, name: &str) -> Result<()> { if src.symlink_metadata(name).is_ok() { @@ -633,7 +635,7 @@ fn compose_postprocess_state_overlays(rootfs_dfd: &Dir) -> Result<()> { let mut db = cap_std::fs::DirBuilder::new(); db.recursive(true); db.mode(0o755); - let localfs_requires = Path::new("usr/lib/systemd/system/local-fs.target.requires"); + let localfs_requires = Path::new(SD_LOCAL_FS_TARGET_REQUIRES); rootfs_dfd.ensure_dir_with(localfs_requires, &db)?; const UNITS: &[&str] = &[ @@ -1011,9 +1013,8 @@ fn convert_path_to_tmpfiles_d_recurse( } fn state_overlay_enabled(rootfs_dfd: &cap_std::fs::Dir, state_overlay: &str) -> Result { - let linkname = format!( - "usr/lib/systemd/system/local-fs.target.requires/ostree-state-overlay@{state_overlay}.service" - ); + let linkname = + format!("{SD_LOCAL_FS_TARGET_REQUIRES}/ostree-state-overlay@{state_overlay}.service"); match rootfs_dfd.symlink_metadata_optional(&linkname)? { Some(meta) if meta.is_symlink() => Ok(true), Some(_) => Err(anyhow!("{linkname} is not a symlink")), From fde9e9e28fc84ed2f8a2d2164434bd1746a81eda Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Tue, 30 Jan 2024 10:02:45 -0500 Subject: [PATCH 2/5] ci: Test `opt-usrlocal-overlays` end-to-end in Prow CI Since both Prow and CoreOS CI do a compose and run kola tests, we can change one of them to cover some different options without losing coverage on the default path. Follow-up to #4728. --- ci/prow/fcos-e2e.sh | 5 ++++ tests/kolainst/destructive/state-overlays | 29 +++++++++++++---------- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/ci/prow/fcos-e2e.sh b/ci/prow/fcos-e2e.sh index 2aaef627f4..46d5db3c7c 100755 --- a/ci/prow/fcos-e2e.sh +++ b/ci/prow/fcos-e2e.sh @@ -9,7 +9,12 @@ ls -al /usr/bin/rpm-ostree rpm-ostree --version cd $(mktemp -d) cosa init https://github.com/coreos/fedora-coreos-config/ +# let's turn on opt-usrlocal-overlays in this test since CoreOS CI already +# covers the off path +echo -e '\nopt-usrlocal-overlays: true\n' >> src/config/manifest.yaml cp /cosa/component-rpms/*.rpm overrides/rpm +# XXX: temporarily import new ostree until it makes it into FCOS +(cd overrides/rpm && curl -L --remote-name-all https://kojipkgs.fedoraproject.org//packages/ostree/2024.2/1.fc39/x86_64/ostree-{,libs-}2024.2-1.fc39.x86_64.rpm) cosa fetch cosa build cosa kola run 'ext.rpm-ostree.*' diff --git a/tests/kolainst/destructive/state-overlays b/tests/kolainst/destructive/state-overlays index 468b96b871..2d00afa661 100755 --- a/tests/kolainst/destructive/state-overlays +++ b/tests/kolainst/destructive/state-overlays @@ -2,7 +2,7 @@ ## kola: ## tags: "needs-internet" -set -euo pipefail +set -xeuo pipefail . ${KOLA_EXT_DATA}/libtest.sh @@ -31,19 +31,23 @@ case "${AUTOPKGTEST_REBOOT_MARK:-}" in rpm-ostree override replace https://bodhi.fedoraproject.org/updates/FEDORA-2024-6c7480dd2f fi - # FCOS doesn't enable opt-usrlocal-overlays so use the hack instead - mkdir -p /etc/systemd/system/rpm-ostreed.service.d/ - cat > /etc/systemd/system/rpm-ostreed.service.d/state-overlay.conf < /etc/systemd/system/rpm-ostreed.service.d/state-overlay.conf < /etc/systemd/system/move-usr-local.service < /etc/systemd/system/move-usr-local.service < Date: Fri, 23 Feb 2024 16:47:57 -0500 Subject: [PATCH 3/5] tests/kolainst: don't use apply-live in with state overlays That combination does not work yet: https://github.com/coreos/rpm-ostree/pull/4810#issuecomment-1939351259 --- tests/kolainst/destructive/apply-live | 7 +++++++ tests/kolainst/destructive/cliwrap | 10 +++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/tests/kolainst/destructive/apply-live b/tests/kolainst/destructive/apply-live index f9d87160e0..25310aeca4 100755 --- a/tests/kolainst/destructive/apply-live +++ b/tests/kolainst/destructive/apply-live @@ -25,6 +25,13 @@ set -x cd $(mktemp -d) +# apply-live is not yet compatible with state overlays +# https://github.com/coreos/rpm-ostree/pull/4810#issuecomment-1939351259 +if jq -e '.["opt-usrlocal-overlays"]' /usr/share/rpm-ostree/treefile.json; then + echo "skip apply-live does not work currently with state overlays" + exit 0 +fi + case "${AUTOPKGTEST_REBOOT_MARK:-}" in "") diff --git a/tests/kolainst/destructive/cliwrap b/tests/kolainst/destructive/cliwrap index fc64a29099..7f2b578d29 100755 --- a/tests/kolainst/destructive/cliwrap +++ b/tests/kolainst/destructive/cliwrap @@ -27,9 +27,13 @@ libtest_prepare_offline libtest_enable_repover 0 cd $(mktemp -d) +case "${AUTOPKGTEST_REBOOT_MARK:-}" in +"") rpm-ostree deploy --ex-cliwrap=true -rpm-ostree apply-live # yep it works! +/tmp/autopkgtest-reboot 1 +;; +1) wrapdir="/usr/libexec/rpm-ostree/wrapped" if ! test -d "${wrapdir}"; then fatal "Missing ${wrapdir}" @@ -67,3 +71,7 @@ rpm -qa >/dev/null rpm --verify bash >out.txt || true assert_not_file_has_content "ostree-based" echo "ok cliwrap undo" +;; + +*) echo "unexpected mark: ${AUTOPKGTEST_REBOOT_MARK}"; exit 1;; +esac From b7534f4016e87a3a322db3fcb4389dab1d309110 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Mon, 26 Feb 2024 22:04:23 -0500 Subject: [PATCH 4/5] compose: Add more error prefixing This helped me narrow down an issue I was hitting. --- src/app/rpmostree-compose-builtin-tree.cxx | 4 ++++ src/libpriv/rpmostree-postprocess.cxx | 2 ++ 2 files changed, 6 insertions(+) diff --git a/src/app/rpmostree-compose-builtin-tree.cxx b/src/app/rpmostree-compose-builtin-tree.cxx index 41c87dd340..63648288ed 100644 --- a/src/app/rpmostree-compose-builtin-tree.cxx +++ b/src/app/rpmostree-compose-builtin-tree.cxx @@ -871,6 +871,8 @@ static gboolean impl_install_tree (RpmOstreeTreeComposeContext *self, gboolean *out_changed, GCancellable *cancellable, GError **error) { + GLNX_AUTO_PREFIX_ERROR ("Installing packages", error); + /* Set this early here, so we only have to set it one more time in the * complete exit path too. */ @@ -1134,6 +1136,8 @@ pull_local_into_target_repo (OstreeRepo *src_repo, OstreeRepo *dest_repo, const static gboolean impl_commit_tree (RpmOstreeTreeComposeContext *self, GCancellable *cancellable, GError **error) { + GLNX_AUTO_PREFIX_ERROR ("Postprocessing and committing", error); + auto gpgkey = (*self->treefile_rs)->get_gpg_key (); /* pick up any initramfs regeneration args to shove into the metadata */ diff --git a/src/libpriv/rpmostree-postprocess.cxx b/src/libpriv/rpmostree-postprocess.cxx index b6c2179cd1..5239f981ca 100644 --- a/src/libpriv/rpmostree-postprocess.cxx +++ b/src/libpriv/rpmostree-postprocess.cxx @@ -591,6 +591,8 @@ cleanup_selinux_lockfiles (int rootfs_fd, GCancellable *cancellable, GError **er gboolean rpmostree_rootfs_postprocess_common (int rootfs_fd, GCancellable *cancellable, GError **error) { + GLNX_AUTO_PREFIX_ERROR ("Doing common postprocessing", error); + if (!rename_if_exists (rootfs_fd, "etc", rootfs_fd, "usr/etc", error)) return FALSE; From eee3bb15e822c769d386d6aa21acad539701540d Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Mon, 26 Feb 2024 22:07:43 -0500 Subject: [PATCH 5/5] compose: Inject our static tmpfiles.d dropins earlier Rather than injecting our "integration" intmpfiles.d dropins in postprocessing, make it part of the tree during the install phase. This allows the new code that generates `rpm-ostree-autovar.conf` to take it into account when trying to get rid of duplicates from the dropins derived per-package. Adjust the idempotency marker in `postprocess_final()` to use `usr/lib/ passwd` since we can't use the tmpfiles.d dropin for that anymore there. --- rust/src/passwd.rs | 3 ++ src/app/rpmostree-compose-builtin-tree.cxx | 40 ++++++++++++++++++++ src/libpriv/rpmostree-postprocess.cxx | 44 ++-------------------- 3 files changed, 46 insertions(+), 41 deletions(-) diff --git a/rust/src/passwd.rs b/rust/src/passwd.rs index 821497d82d..79ee488f80 100644 --- a/rust/src/passwd.rs +++ b/rust/src/passwd.rs @@ -106,6 +106,9 @@ pub fn passwd_cleanup(rootfs_dfd: i32) -> Result<()> { /// in /usr/etc at this point), and splitting it into two streams: a new /// /etc/passwd that just contains the root entry, and /usr/lib/passwd which /// contains everything else. +/// +/// Note: the presence of /usr/lib/passwd is used in postprocess_final() to make +/// it idempotent. See related comment there. #[context("Migrating 'passwd' to /usr/lib")] pub fn migrate_passwd_except_root(rootfs_dfd: i32) -> CxxResult<()> { static ETCSRC_PATH: &str = "usr/etc/passwd"; diff --git a/src/app/rpmostree-compose-builtin-tree.cxx b/src/app/rpmostree-compose-builtin-tree.cxx index 63648288ed..d52974b9e6 100644 --- a/src/app/rpmostree-compose-builtin-tree.cxx +++ b/src/app/rpmostree-compose-builtin-tree.cxx @@ -477,6 +477,46 @@ install_packages (RpmOstreeTreeComposeContext *self, gboolean *out_unmodified, std::string (previous_ref), opt_unified_core), error); + /* Assembly will regen the rpm-ostree-autovar.conf tmpfiles.d dropin; let's + * make sure to add our own static dropins before that so that they're taken + * into account when looking for dupes. */ + g_print ("Adding rpm-ostree-0-integration.conf\n"); + + /* This is useful if we're running in an uninstalled configuration, e.g. + * during tests. */ + const char *pkglibdir_path = g_getenv ("RPMOSTREE_UNINSTALLED_PKGLIBDIR") ?: PKGLIBDIR; + glnx_autofd int pkglibdir_dfd = -1; + if (!glnx_opendirat (AT_FDCWD, pkglibdir_path, TRUE, &pkglibdir_dfd, error)) + return FALSE; + + if (!glnx_shutil_mkdir_p_at (rootfs_dfd, "usr/lib/tmpfiles.d", 0755, cancellable, error)) + return FALSE; + + if (!glnx_file_copy_at (pkglibdir_dfd, "rpm-ostree-0-integration.conf", NULL, rootfs_dfd, + "usr/lib/tmpfiles.d/rpm-ostree-0-integration.conf", + GLNX_FILE_COPY_NOXATTRS, /* Don't take selinux label */ + cancellable, error)) + return FALSE; + + if ((*self->treefile_rs)->get_opt_usrlocal_overlays ()) + { + if (!glnx_file_copy_at ( + pkglibdir_dfd, "rpm-ostree-0-integration-opt-usrlocal-compat.conf", NULL, rootfs_dfd, + "usr/lib/tmpfiles.d/rpm-ostree-0-integration-opt-usrlocal-compat.conf", + GLNX_FILE_COPY_NOXATTRS, /* Don't take selinux label */ + cancellable, error)) + return FALSE; + } + else + { + if (!glnx_file_copy_at (pkglibdir_dfd, "rpm-ostree-0-integration-opt-usrlocal.conf", NULL, + rootfs_dfd, + "usr/lib/tmpfiles.d/rpm-ostree-0-integration-opt-usrlocal.conf", + GLNX_FILE_COPY_NOXATTRS, /* Don't take selinux label */ + cancellable, error)) + return FALSE; + } + if (opt_unified_core) { if (!rpmostree_context_import (self->corectx, cancellable, error)) diff --git a/src/libpriv/rpmostree-postprocess.cxx b/src/libpriv/rpmostree-postprocess.cxx index 5239f981ca..4329513d55 100644 --- a/src/libpriv/rpmostree-postprocess.cxx +++ b/src/libpriv/rpmostree-postprocess.cxx @@ -368,13 +368,12 @@ postprocess_final (int rootfs_dfd, rpmostreecxx::Treefile &treefile, gboolean un { GLNX_AUTO_PREFIX_ERROR ("Finalizing rootfs", error); - /* Use installation of the tmpfiles integration as an "idempotence" marker to + /* Use the presence of /usr/lib/passwd as an "idempotence" marker to * avoid doing postprocessing twice, which can happen when mixing `compose * postprocess-root` with `compose commit`. */ - const char tmpfiles_integration_path[] = "usr/lib/tmpfiles.d/rpm-ostree-0-integration.conf"; - if (!glnx_fstatat_allow_noent (rootfs_dfd, tmpfiles_integration_path, NULL, AT_SYMLINK_NOFOLLOW, - error)) + const char usr_lib_passwd[] = "usr/lib/password"; + if (!glnx_fstatat_allow_noent (rootfs_dfd, usr_lib_passwd, NULL, AT_SYMLINK_NOFOLLOW, error)) return FALSE; if (errno == 0) return TRUE; @@ -443,43 +442,6 @@ postprocess_final (int rootfs_dfd, rpmostreecxx::Treefile &treefile, gboolean un if (!rpmostree_rootfs_postprocess_common (rootfs_dfd, cancellable, error)) return FALSE; - g_print ("Adding rpm-ostree-0-integration.conf\n"); - /* This is useful if we're running in an uninstalled configuration, e.g. - * during tests. */ - const char *pkglibdir_path = g_getenv ("RPMOSTREE_UNINSTALLED_PKGLIBDIR") ?: PKGLIBDIR; - glnx_autofd int pkglibdir_dfd = -1; - - if (!glnx_opendirat (AT_FDCWD, pkglibdir_path, TRUE, &pkglibdir_dfd, error)) - return FALSE; - - if (!glnx_shutil_mkdir_p_at (rootfs_dfd, "usr/lib/tmpfiles.d", 0755, cancellable, error)) - return FALSE; - - if (!glnx_file_copy_at (pkglibdir_dfd, "rpm-ostree-0-integration.conf", NULL, rootfs_dfd, - tmpfiles_integration_path, - GLNX_FILE_COPY_NOXATTRS, /* Don't take selinux label */ - cancellable, error)) - return FALSE; - - if (treefile.get_opt_usrlocal_overlays ()) - { - if (!glnx_file_copy_at ( - pkglibdir_dfd, "rpm-ostree-0-integration-opt-usrlocal-compat.conf", NULL, rootfs_dfd, - "usr/lib/tmpfiles.d/rpm-ostree-0-integration-opt-usrlocal-compat.conf", - GLNX_FILE_COPY_NOXATTRS, /* Don't take selinux label */ - cancellable, error)) - return FALSE; - } - else - { - if (!glnx_file_copy_at (pkglibdir_dfd, "rpm-ostree-0-integration-opt-usrlocal.conf", NULL, - rootfs_dfd, - "usr/lib/tmpfiles.d/rpm-ostree-0-integration-opt-usrlocal.conf", - GLNX_FILE_COPY_NOXATTRS, /* Don't take selinux label */ - cancellable, error)) - return FALSE; - } - /* Handle kernel/initramfs if we're not doing a container */ if (!container) {