From 4deb426835d234eaa55046175f7dffaea7864fdb Mon Sep 17 00:00:00 2001 From: William Manley Date: Wed, 15 Jul 2020 15:57:26 +0100 Subject: [PATCH 01/20] Refactor tests/bootloader-entries-crosscheck.py Reduce duplication. --- tests/bootloader-entries-crosscheck.py | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/tests/bootloader-entries-crosscheck.py b/tests/bootloader-entries-crosscheck.py index 41f6956e..b4cdcbf5 100755 --- a/tests/bootloader-entries-crosscheck.py +++ b/tests/bootloader-entries-crosscheck.py @@ -73,21 +73,24 @@ with open(syslinuxpath) as f: syslinux_entry = None syslinux_default = None for line in f: - line = line.strip() - if line.startswith('DEFAULT '): + try: + k, v = line.strip().split(" ", 1) + except ValueError: + continue + if k == 'DEFAULT': if syslinux_entry is not None: - syslinux_default = line.split(' ', 1)[1] - elif line.startswith('LABEL '): + syslinux_default = v + elif k == 'LABEL': if syslinux_entry is not None: syslinux_entries.append(syslinux_entry) syslinux_entry = {} - syslinux_entry['title'] = line.split(' ', 1)[1] - elif line.startswith('KERNEL '): - syslinux_entry['linux'] = line.split(' ', 1)[1] - elif line.startswith('INITRD '): - syslinux_entry['initrd'] = line.split(' ', 1)[1] - elif line.startswith('APPEND '): - syslinux_entry['options'] = line.split(' ', 1)[1] + syslinux_entry['title'] = v + elif k == 'KERNEL': + syslinux_entry['linux'] = v + elif k == 'INITRD': + syslinux_entry['initrd'] = v + elif k == 'APPEND': + syslinux_entry['options'] = v if syslinux_entry is not None: syslinux_entries.append(syslinux_entry) From 0ced9fde7649271d9458ca424aa8c41908634b02 Mon Sep 17 00:00:00 2001 From: William Manley Date: Wed, 15 Jul 2020 15:40:14 +0100 Subject: [PATCH 02/20] sysroot: Support /boot on root or as seperate filesystem for syslinux and u-boot We use a similar trick to having a `sysroot -> .` symlink on the real root here to support both /boot on root as well as on a separate filesystem. No matter how it's mounted `/boot/xyz` will always refer to the file you'd expect. This is nicer than my previous attempts at this because there's no configuration nor auto-detection required. --- src/libostree/ostree-bootloader-syslinux.c | 15 +++++++----- src/libostree/ostree-bootloader-uboot.c | 8 +++---- src/libostree/ostree-sysroot-deploy.c | 6 +++++ tests/bootloader-entries-crosscheck.py | 27 ++++++++++++++++++---- 4 files changed, 41 insertions(+), 15 deletions(-) diff --git a/src/libostree/ostree-bootloader-syslinux.c b/src/libostree/ostree-bootloader-syslinux.c index 5fb8a1db..0055896b 100644 --- a/src/libostree/ostree-bootloader-syslinux.c +++ b/src/libostree/ostree-bootloader-syslinux.c @@ -89,15 +89,15 @@ append_config_from_loader_entries (OstreeBootloaderSyslinux *self, val = ostree_bootconfig_parser_get (config, "linux"); if (!val) return glnx_throw (error, "No \"linux\" key in bootloader config"); - g_ptr_array_add (new_lines, g_strdup_printf ("\tKERNEL %s", val)); + g_ptr_array_add (new_lines, g_strdup_printf ("\tKERNEL /boot%s", val)); val = ostree_bootconfig_parser_get (config, "initrd"); if (val) - g_ptr_array_add (new_lines, g_strdup_printf ("\tINITRD %s", val)); + g_ptr_array_add (new_lines, g_strdup_printf ("\tINITRD /boot%s", val)); val = ostree_bootconfig_parser_get (config, "devicetree"); if (val) - g_ptr_array_add (new_lines, g_strdup_printf ("\tDEVICETREE %s", val)); + g_ptr_array_add (new_lines, g_strdup_printf ("\tDEVICETREE /boot%s", val)); val = ostree_bootconfig_parser_get (config, "options"); if (val) @@ -150,10 +150,13 @@ _ostree_bootloader_syslinux_write_config (OstreeBootloader *bootloader, if (kernel_arg == NULL) return glnx_throw (error, "No KERNEL argument found after LABEL"); - /* If this is a non-ostree kernel, just emit the lines - * we saw. + /* If this is a non-ostree kernel, just emit the lines we saw. + * + * We check for /ostree (without /boot prefix) as well to support + * upgrading ostree from len; i++) { diff --git a/src/libostree/ostree-bootloader-uboot.c b/src/libostree/ostree-bootloader-uboot.c index 1e1f0371..7e23001e 100644 --- a/src/libostree/ostree-bootloader-uboot.c +++ b/src/libostree/ostree-bootloader-uboot.c @@ -134,19 +134,19 @@ create_config_from_boot_loader_entries (OstreeBootloaderUboot *self, "No \"linux\" key in bootloader config"); return FALSE; } - g_ptr_array_add (new_lines, g_strdup_printf ("kernel_image%s=%s", index_suffix, val)); + g_ptr_array_add (new_lines, g_strdup_printf ("kernel_image%s=/boot%s", index_suffix, val)); val = ostree_bootconfig_parser_get (config, "initrd"); if (val) - g_ptr_array_add (new_lines, g_strdup_printf ("ramdisk_image%s=%s", index_suffix, val)); + g_ptr_array_add (new_lines, g_strdup_printf ("ramdisk_image%s=/boot%s", index_suffix, val)); val = ostree_bootconfig_parser_get (config, "devicetree"); if (val) - g_ptr_array_add (new_lines, g_strdup_printf ("fdt_file%s=%s", index_suffix, val)); + g_ptr_array_add (new_lines, g_strdup_printf ("fdt_file%s=/boot%s", index_suffix, val)); val = ostree_bootconfig_parser_get (config, "fdtdir"); if (val) - g_ptr_array_add (new_lines, g_strdup_printf ("fdtdir%s=%s", index_suffix, val)); + g_ptr_array_add (new_lines, g_strdup_printf ("fdtdir%s=/boot%s", index_suffix, val)); val = ostree_bootconfig_parser_get (config, "options"); if (val) diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index cb593020..ec51ad9a 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -1998,6 +1998,12 @@ prepare_new_bootloader_link (OstreeSysroot *sysroot, g_assert ((current_bootversion == 0 && new_bootversion == 1) || (current_bootversion == 1 && new_bootversion == 0)); + /* This allows us to support both /boot on a seperate filesystem to / as well + * as on the same filesystem. */ + if (TEMP_FAILURE_RETRY (symlinkat (".", sysroot->sysroot_fd, "boot/boot")) < 0) + if (errno != EEXIST) + return glnx_throw_errno_prefix (error, "symlinkat"); + g_autofree char *new_target = g_strdup_printf ("loader.%d", new_bootversion); /* We shouldn't actually need to replace but it's easier to reuse diff --git a/tests/bootloader-entries-crosscheck.py b/tests/bootloader-entries-crosscheck.py index b4cdcbf5..605bd080 100755 --- a/tests/bootloader-entries-crosscheck.py +++ b/tests/bootloader-entries-crosscheck.py @@ -97,15 +97,32 @@ with open(syslinuxpath) as f: if len(entries) != len(syslinux_entries): fatal("Found {0} loader entries, but {1} SYSLINUX entries\n".format(len(entries), len(syslinux_entries))) -def assert_matches_key(a, b, key): + +def assert_eq(a, b): + assert a == b, "%r == %r" % (a, b) + + +def assert_key_same_file(a, b, key): aval = a[key] bval = b[key] - if aval != bval: - fatal("Mismatch on {0}: {1} != {2}".format(key, aval, bval)) + sys.stderr.write("aval: %r\nbval: %r\n" % (aval, bval)) + + # Paths in entries are always relative to /boot + entry = os.stat(sysroot + "/boot" + aval) + + # Syslinux entries can be relative to /boot (if it's on another filesystem) + # or relative to / if /boot is on /. + s1 = os.stat(sysroot + bval) + s2 = os.stat(sysroot + "/boot" + bval) + + # A symlink ensures that no matter what they point at the same file + assert_eq(entry, s1) + assert_eq(entry, s2) + for i,(entry,syslinuxentry) in enumerate(zip(entries, syslinux_entries)): - assert_matches_key(entry, syslinuxentry, 'linux') - assert_matches_key(entry, syslinuxentry, 'initrd') + assert_key_same_file(entry, syslinuxentry, 'linux') + assert_key_same_file(entry, syslinuxentry, 'initrd') entry_ostree = get_ostree_option(entry['options']) syslinux_ostree = get_ostree_option(syslinuxentry['options']) if entry_ostree != syslinux_ostree: From b67f029d76624d36feb6a0365b07d491fb70cbe9 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 18 Aug 2020 15:55:47 +0000 Subject: [PATCH 03/20] Post-release version bump --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 2eec7464..0c3c6fed 100644 --- a/configure.ac +++ b/configure.ac @@ -7,7 +7,7 @@ dnl Seed the release notes with `git-shortlog-with-prs ..`. Th dnl `git-evtag` to create the tag and push it. Finally, create a GitHub release and attach dnl the tarball from `make dist`. m4_define([year_version], [2020]) -m4_define([release_version], [5]) +m4_define([release_version], [6]) m4_define([package_version], [year_version.release_version]) AC_INIT([libostree], [package_version], [walters@verbum.org]) is_release_build=yes From 9f8c3f440094d53d968b81d2f825ba6cb77c2c7f Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 18 Aug 2020 18:00:19 +0000 Subject: [PATCH 04/20] tests/inst: Bump to latest ostree and gtk-rs Updating our tests to the latest ostree crate is so deliciously circular. --- tests/inst/Cargo.toml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/inst/Cargo.toml b/tests/inst/Cargo.toml index d47db53c..ee9dfb66 100644 --- a/tests/inst/Cargo.toml +++ b/tests/inst/Cargo.toml @@ -17,9 +17,9 @@ serde_json = "1.0" commandspec = "0.12.2" anyhow = "1.0" tempfile = "3.1.0" -glib = "0.9.1" -gio = "0.8" -ostree = { version = "0.7.1", features = ["v2020_1"] } +glib = "0.10" +gio = "0.9" +ostree = { version = "0.8.0", features = ["v2020_1"] } libtest-mimic = "0.3.0" twoway = "0.2.1" hyper = "0.13" From 1eab48363ba385cba5d57aa09e5a7218d12f25b5 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 18 Aug 2020 23:34:57 +0000 Subject: [PATCH 05/20] pull: Assign idle_src variable before calling unref() This should pacify Coverity, and also just "reads" better too. --- src/libostree/ostree-repo-pull.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 894e4b1f..d817575b 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -436,8 +436,9 @@ ensure_idle_queued (OtPullData *pull_data) idle_src = g_idle_source_new (); g_source_set_callback (idle_src, idle_worker, pull_data, NULL); g_source_attach (idle_src, pull_data->main_context); - g_source_unref (idle_src); pull_data->idle_src = idle_src; + /* Ownership is transferred to pull_data */ + g_source_unref (idle_src); } typedef struct { From 95a751262251b7f8137efb828e9c4825d91cad34 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 18 Aug 2020 23:35:38 +0000 Subject: [PATCH 06/20] prepare-root: Remove unused variable Should quiet Coverity. --- src/switchroot/ostree-prepare-root.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/switchroot/ostree-prepare-root.c b/src/switchroot/ostree-prepare-root.c index 8a68e1f4..f7e4fe47 100644 --- a/src/switchroot/ostree-prepare-root.c +++ b/src/switchroot/ostree-prepare-root.c @@ -101,10 +101,9 @@ sysroot_is_configured_ro (const char *sysroot) bool ret = false; char *line = NULL; size_t len = 0; - ssize_t nread; /* Note getline() will reuse the previous buffer */ bool in_sysroot = false; - while ((nread = getline (&line, &len, f)) != -1) + while (getline (&line, &len, f) != -1) { /* This is an awful hack to avoid depending on GLib in the * initramfs right now. From 22a445c18995e642d63152d36e924e4d85764b99 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 19 Aug 2020 13:09:46 +0000 Subject: [PATCH 07/20] admin/pin: Enforce that index is a number Validate that we're parsing a number; we want to guard against typos. Closes: https://github.com/ostreedev/ostree/issues/2171 --- src/ostree/ot-admin-builtin-pin.c | 9 ++++++++- tests/test-admin-deploy-2.sh | 9 ++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/ostree/ot-admin-builtin-pin.c b/src/ostree/ot-admin-builtin-pin.c index d4337e33..5269dd8c 100644 --- a/src/ostree/ot-admin-builtin-pin.c +++ b/src/ostree/ot-admin-builtin-pin.c @@ -55,7 +55,14 @@ ot_admin_builtin_pin (int argc, char **argv, OstreeCommandInvocation *invocation for (unsigned int i = 1; i < argc; i++) { const char *deploy_index_str = argv[i]; - const int deploy_index = atoi (deploy_index_str); + char *endptr = NULL; + + errno = 0; + const guint64 deploy_index = g_ascii_strtoull (deploy_index_str, &endptr, 10); + if (*endptr != '\0') + return glnx_throw (error, "Invalid index: %s", deploy_index_str); + if (errno == ERANGE) + return glnx_throw (error, "Index too large: %s", deploy_index_str); g_autoptr(OstreeDeployment) target_deployment = ot_admin_get_indexed_deployment (sysroot, deploy_index, error); if (!target_deployment) diff --git a/tests/test-admin-deploy-2.sh b/tests/test-admin-deploy-2.sh index 0fa2df9b..6df4877c 100755 --- a/tests/test-admin-deploy-2.sh +++ b/tests/test-admin-deploy-2.sh @@ -26,7 +26,7 @@ set -euo pipefail # Exports OSTREE_SYSROOT so --sysroot not needed. setup_os_repository "archive" "syslinux" -echo "1..7" +echo "1..8" ${CMD_PREFIX} ostree --repo=sysroot/ostree/repo pull-local --remote=testos testos-repo testos/buildmaster/x86_64-runtime rev=$(${CMD_PREFIX} ostree --repo=sysroot/ostree/repo rev-parse testos/buildmaster/x86_64-runtime) @@ -102,6 +102,13 @@ ${CMD_PREFIX} ostree admin pin -u 0 assert_n_pinned 0 echo "ok pin unpin" +for p in medal 0medal '' 5000 9999999999999999999999999999999999999; do + if ${CMD_PREFIX} ostree admin pin ${p}; then + fatal "created invalid pin ${p}" + fi +done +echo "ok invalid pin" + ${CMD_PREFIX} ostree admin pin 0 1 assert_n_pinned 2 assert_n_deployments 2 From d3fadf14b7f7815845ca79768f10d20913df7d39 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Fri, 21 Aug 2020 09:14:36 +0100 Subject: [PATCH 08/20] boot: Replace deprecated StandardOutput=syslog with journal, etc. systemd deprecated this in v246. Resolves: #2169 Signed-off-by: Simon McVittie --- src/boot/ostree-prepare-root.service | 4 ++-- src/boot/ostree-remount.service | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/boot/ostree-prepare-root.service b/src/boot/ostree-prepare-root.service index 91692205..250ffe71 100644 --- a/src/boot/ostree-prepare-root.service +++ b/src/boot/ostree-prepare-root.service @@ -30,6 +30,6 @@ Before=initrd-root-fs.target Type=oneshot ExecStart=/usr/lib/ostree/ostree-prepare-root /sysroot StandardInput=null -StandardOutput=syslog -StandardError=syslog+console +StandardOutput=journal +StandardError=journal+console RemainAfterExit=yes diff --git a/src/boot/ostree-remount.service b/src/boot/ostree-remount.service index 4c3ed94a..af40453c 100644 --- a/src/boot/ostree-remount.service +++ b/src/boot/ostree-remount.service @@ -35,8 +35,8 @@ Type=oneshot RemainAfterExit=yes ExecStart=/usr/lib/ostree/ostree-remount StandardInput=null -StandardOutput=syslog -StandardError=syslog+console +StandardOutput=journal +StandardError=journal+console [Install] WantedBy=local-fs.target From cc1b70d921e1ae8ea09e211a71095baea373fbb7 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 21 Aug 2020 17:35:03 +0000 Subject: [PATCH 09/20] tests: Check the immutable bit See https://bugzilla.redhat.com/show_bug.cgi?id=1867601 We really want an upstream test for this, even if (to my knowledge) nothing is running ostree's upstream CI on !x86_64. --- tests/inst/src/sysroot.rs | 7 +++++++ tests/inst/src/test.rs | 23 +++++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/tests/inst/src/sysroot.rs b/tests/inst/src/sysroot.rs index 08a3d38f..bb742c23 100644 --- a/tests/inst/src/sysroot.rs +++ b/tests/inst/src/sysroot.rs @@ -31,3 +31,10 @@ fn test_sysroot_ro() -> Result<()> { Ok(()) } + +#[itest] +fn test_immutable_bit() -> Result<()> { + // https://bugzilla.redhat.com/show_bug.cgi?id=1867601 + cmd_has_output(commandspec::sh_command!("lsattr -d /").unwrap(), "-i-")?; + Ok(()) +} diff --git a/tests/inst/src/test.rs b/tests/inst/src/test.rs index 24dc8194..8b8510b8 100644 --- a/tests/inst/src/test.rs +++ b/tests/inst/src/test.rs @@ -49,6 +49,20 @@ pub(crate) fn cmd_fails_with>(mut c: C, pat: &str) -> Resu Ok(()) } +/// Run command and assert that its stdout contains pat +pub(crate) fn cmd_has_output>(mut c: C, pat: &str) -> Result<()> { + let c = c.borrow_mut(); + let o = c.output()?; + if !o.status.success() { + bail!("Command {:?} failed", c); + } + if !twoway::find_bytes(&o.stdout, pat.as_bytes()).is_some() { + dbg!(String::from_utf8_lossy(&o.stdout)); + bail!("Command {:?} stdout did not match: {}", c, pat); + } + Ok(()) +} + pub(crate) fn write_file>(p: P, buf: &str) -> Result<()> { let p = p.as_ref(); let mut f = File::create(p)?; @@ -219,6 +233,15 @@ mod tests { cmd_fails_with(oops(), "nomatch").expect_err("nomatch"); } + #[test] + fn test_output() -> Result<()> { + cmd_has_output(Command::new("true"), "")?; + assert!(cmd_has_output(Command::new("true"), "foo").is_err()); + cmd_has_output(commandspec::sh_command!("echo foobarbaz; echo fooblahbaz").unwrap(), "blah")?; + assert!(cmd_has_output(commandspec::sh_command!("echo foobarbaz").unwrap(), "blah").is_err()); + Ok(()) + } + #[test] fn test_validate_authz() -> Result<()> { assert!(validate_authz("Basic Zm9vdXNlcjpiYXJwdw==".as_bytes())?); From 06ed04a816141914adb9bd3e32392801fce5bc8e Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 21 Aug 2020 17:40:41 +0000 Subject: [PATCH 10/20] linuxfsutil: Pass int to ioctl, not long Otherwise it will fail on big-endian architectures like s390x. Ref https://bugzilla.redhat.com/show_bug.cgi?id=1867601 --- src/libostree/ostree-linuxfsutil.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libostree/ostree-linuxfsutil.c b/src/libostree/ostree-linuxfsutil.c index 231ecf76..cb778def 100644 --- a/src/libostree/ostree-linuxfsutil.c +++ b/src/libostree/ostree-linuxfsutil.c @@ -55,7 +55,7 @@ _ostree_linuxfs_fd_alter_immutable_flag (int fd, if (g_atomic_int_get (&no_alter_immutable)) return TRUE; - unsigned long flags; + int flags = 0; int r = ioctl (fd, EXT2_IOC_GETFLAGS, &flags); if (r == -1) { From 0a6a41a63d719c87313b72cf5ce3cbbcc75d7d0d Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Mon, 24 Aug 2020 14:16:16 -0400 Subject: [PATCH 11/20] configure.ac: Set is_release_build=no We missed this during the post-release version bump. --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 0c3c6fed..6d2c97d2 100644 --- a/configure.ac +++ b/configure.ac @@ -10,7 +10,7 @@ m4_define([year_version], [2020]) m4_define([release_version], [6]) m4_define([package_version], [year_version.release_version]) AC_INIT([libostree], [package_version], [walters@verbum.org]) -is_release_build=yes +is_release_build=no AC_CONFIG_HEADER([config.h]) AC_CONFIG_MACRO_DIR([buildutil]) AC_CONFIG_AUX_DIR([build-aux]) From 33e2d34ea53f2cf4b58afc0f95dc910be49ed00b Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 25 Aug 2020 22:06:13 +0000 Subject: [PATCH 12/20] tests/inst: Port to new sh-inline repo I cleaned up my fork of commandspec (see git log) and am planning to publish to crates. Port to the new API in prep for that. --- tests/inst/Cargo.toml | 8 ++------ tests/inst/src/destructive.rs | 34 +++++++++++++++++----------------- tests/inst/src/repobin.rs | 20 ++++++++++---------- tests/inst/src/sysroot.rs | 2 +- tests/inst/src/test.rs | 4 ++-- tests/inst/src/treegen.rs | 4 ++-- 6 files changed, 34 insertions(+), 38 deletions(-) diff --git a/tests/inst/Cargo.toml b/tests/inst/Cargo.toml index ee9dfb66..b361accc 100644 --- a/tests/inst/Cargo.toml +++ b/tests/inst/Cargo.toml @@ -14,7 +14,8 @@ structopt = "0.3" serde = "1.0.111" serde_derive = "1.0.111" serde_json = "1.0" -commandspec = "0.12.2" +# To be published on crates.io soon +sh-inline = { git = "https://github.com/cgwalters/rust-sh-inline" } anyhow = "1.0" tempfile = "3.1.0" glib = "0.10" @@ -43,8 +44,3 @@ with-procspawn-tempdir = { git = "https://github.com/cgwalters/with-procspawn-te # Internal crate for the test macro itest-macro = { path = "itest-macro" } - -[patch.crates-io] -# See https://github.com/tcr/commandspec/pulls?q=is%3Apr+author%3Acgwalters+ -# If patches don't get reviewed I'll probably fork it. -commandspec = { git = "https://github.com/cgwalters/commandspec", branch = 'walters-master' } diff --git a/tests/inst/src/destructive.rs b/tests/inst/src/destructive.rs index 4d22ea83..d1ebe35d 100644 --- a/tests/inst/src/destructive.rs +++ b/tests/inst/src/destructive.rs @@ -20,7 +20,7 @@ //! AUTOPKGTEST_REBOOT_MARK. use anyhow::{Context, Result}; -use commandspec::sh_execute; +use sh_inline::bash; use rand::seq::SliceRandom; use rand::Rng; use serde::{Deserialize, Serialize}; @@ -133,7 +133,7 @@ impl InterruptStrategy { /// TODO add readonly sysroot handling into base ostree fn testinit() -> Result<()> { assert!(std::path::Path::new("/run/ostree-booted").exists()); - sh_execute!( + bash!( r"if ! test -w /sysroot; then mount -o remount,rw /sysroot fi" @@ -152,7 +152,7 @@ fn generate_update(commit: &str) -> Result<()> { // Amortize the prune across multiple runs; we don't want to leak space, // but traversing all the objects is expensive. So here we only prune 1/5 of the time. if rand::thread_rng().gen_ratio(1, 5) { - sh_execute!( + bash!( "ostree --repo={srvrepo} prune --refs-only --depth=1", srvrepo = SRVREPO )?; @@ -165,7 +165,7 @@ fn generate_update(commit: &str) -> Result<()> { /// and then teach our webserver to redirect to the system for objects it doesn't /// have. fn generate_srv_repo(commit: &str) -> Result<()> { - sh_execute!( + bash!( r#" ostree --repo={srvrepo} init --mode=archive ostree --repo={srvrepo} config set archive.zlib-level 1 @@ -200,7 +200,7 @@ struct RebootStats { } fn upgrade_and_finalize() -> Result<()> { - sh_execute!( + bash!( "rpm-ostree upgrade systemctl start ostree-finalize-staged systemctl stop ostree-finalize-staged" @@ -304,8 +304,8 @@ fn parse_and_validate_reboot_mark>( fn validate_pending_commit(pending_commit: &str, commitstates: &CommitStates) -> Result<()> { if pending_commit != commitstates.target { - sh_execute!("rpm-ostree status -v")?; - sh_execute!( + bash!("rpm-ostree status -v")?; + bash!( "ostree show {pending_commit}", pending_commit = pending_commit )?; @@ -418,7 +418,7 @@ fn impl_transaction_test>( // If we've reached our target iterations, exit the test successfully if mark.iter == ITERATIONS { // TODO also add ostree admin fsck to check the deployment directories - sh_execute!( + bash!( "echo Performing final validation... ostree fsck" )?; @@ -455,7 +455,7 @@ fn impl_transaction_test>( ); // Reset the target ref to booted, and perform a cleanup // to ensure we're re-downloading objects each time - sh_execute!( + bash!( " systemctl stop rpm-ostreed systemctl stop ostree-finalize-staged @@ -498,7 +498,7 @@ fn impl_transaction_test>( // the interrupt strategy. match strategy { InterruptStrategy::Force(ForceInterruptStrategy::Kill9) => { - sh_execute!( + bash!( "systemctl kill -s KILL rpm-ostreed || true systemctl kill -s KILL ostree-finalize-staged || true" )?; @@ -508,7 +508,7 @@ fn impl_transaction_test>( mark.reboot_strategy = Some(strategy.clone()); prepare_reboot(serde_json::to_string(&mark)?)?; // This is a forced reboot - no syncing of the filesystem. - sh_execute!("reboot -ff")?; + bash!("reboot -ff")?; std::thread::sleep(time::Duration::from_secs(60)); // Shouldn't happen anyhow::bail!("failed to reboot"); @@ -522,7 +522,7 @@ fn impl_transaction_test>( // We either rebooted, or failed to reboot } InterruptStrategy::Polite(PoliteInterruptStrategy::Stop) => { - sh_execute!( + bash!( "systemctl stop rpm-ostreed || true systemctl stop ostree-finalize-staged || true" )?; @@ -566,7 +566,7 @@ fn transactionality() -> Result<()> { }; with_webserver_in(&srvrepo, &webserver_opts, move |addr| { let url = format!("http://{}", addr); - sh_execute!( + bash!( "ostree remote delete --if-exists testrepo ostree remote add --set=gpg-verify=false testrepo {url}", url = url @@ -576,13 +576,13 @@ fn transactionality() -> Result<()> { // Also disable some services (like zincati) because we don't want automatic updates // in our reboots, and it currently fails to start. The less // we have in each reboot, the faster reboots are. - sh_execute!("systemctl disable --now zincati fedora-coreos-pinger")?; + bash!("systemctl disable --now zincati fedora-coreos-pinger")?; // And prepare for updates - sh_execute!("rpm-ostree cleanup -pr")?; + bash!("rpm-ostree cleanup -pr")?; generate_update(&commit)?; // Directly set the origin, so that we're not dependent on the pending deployment. // FIXME: make this saner - sh_execute!( + bash!( " ostree admin set-origin testrepo {url} {testref} ostree refs --create testrepo:{testref} {commit} @@ -608,7 +608,7 @@ fn transactionality() -> Result<()> { let mut f = std::io::BufWriter::new(std::fs::File::create(&TDATAPATH)?); serde_json::to_writer(&mut f, &tdata)?; f.flush()?; - sh_execute!("rpm-ostree status")?; + bash!("rpm-ostree status")?; } let tdata = { diff --git a/tests/inst/src/repobin.rs b/tests/inst/src/repobin.rs index 208eae40..b034326a 100644 --- a/tests/inst/src/repobin.rs +++ b/tests/inst/src/repobin.rs @@ -5,12 +5,12 @@ use std::path::Path; use crate::test::*; use anyhow::{Context, Result}; -use commandspec::{sh_command, sh_execute}; +use sh_inline::{bash_command, bash}; use with_procspawn_tempdir::with_procspawn_tempdir; #[itest] fn test_basic() -> Result<()> { - sh_execute!(r"ostree --help >/dev/null")?; + bash!(r"ostree --help >/dev/null")?; Ok(()) } @@ -18,14 +18,14 @@ fn test_basic() -> Result<()> { #[with_procspawn_tempdir] fn test_nofifo() -> Result<()> { assert!(std::path::Path::new(".procspawn-tmpdir").exists()); - sh_execute!( + bash!( r"ostree --repo=repo init --mode=archive mkdir tmproot mkfifo tmproot/afile " )?; cmd_fails_with( - sh_command!( + bash_command!( r#"ostree --repo=repo commit -b fifotest -s "commit fifo" --tree=dir=./tmproot"# ) .unwrap(), @@ -37,7 +37,7 @@ fn test_nofifo() -> Result<()> { #[itest] #[with_procspawn_tempdir] fn test_mtime() -> Result<()> { - sh_execute!( + bash!( r"ostree --repo=repo init --mode=archive mkdir tmproot echo afile > tmproot/afile @@ -45,7 +45,7 @@ fn test_mtime() -> Result<()> { " )?; let ts = Path::new("repo").metadata()?.modified().unwrap(); - sh_execute!( + bash!( r#"ostree --repo=repo commit -b test -s "bump mtime" --tree=dir=tmproot >/dev/null"# )?; assert_ne!(ts, Path::new("repo").metadata()?.modified().unwrap()); @@ -55,7 +55,7 @@ fn test_mtime() -> Result<()> { #[itest] #[with_procspawn_tempdir] fn test_extensions() -> Result<()> { - sh_execute!(r"ostree --repo=repo init --mode=bare")?; + bash!(r"ostree --repo=repo init --mode=bare")?; assert!(Path::new("repo/extensions").exists()); Ok(()) } @@ -78,7 +78,7 @@ fn test_pull_basicauth() -> Result<()> { )?; let osroot = Path::new("osroot"); crate::treegen::mkroot(&osroot)?; - sh_execute!( + bash!( r#"ostree --repo={serverrepo} init --mode=archive ostree --repo={serverrepo} commit -b os --tree=dir={osroot} >/dev/null mkdir client @@ -96,7 +96,7 @@ fn test_pull_basicauth() -> Result<()> { )?; for rem in &["unauth", "badauth"] { cmd_fails_with( - sh_command!( + bash_command!( r#"ostree --repo=client/repo pull origin-{rem} os >/dev/null"#, rem = *rem ) @@ -105,7 +105,7 @@ fn test_pull_basicauth() -> Result<()> { ) .context(rem)?; } - sh_execute!(r#"ostree --repo=client/repo pull origin-goodauth os >/dev/null"#,)?; + bash!(r#"ostree --repo=client/repo pull origin-goodauth os >/dev/null"#,)?; Ok(()) })?; Ok(()) diff --git a/tests/inst/src/sysroot.rs b/tests/inst/src/sysroot.rs index bb742c23..a8ff3eb3 100644 --- a/tests/inst/src/sysroot.rs +++ b/tests/inst/src/sysroot.rs @@ -35,6 +35,6 @@ fn test_sysroot_ro() -> Result<()> { #[itest] fn test_immutable_bit() -> Result<()> { // https://bugzilla.redhat.com/show_bug.cgi?id=1867601 - cmd_has_output(commandspec::sh_command!("lsattr -d /").unwrap(), "-i-")?; + cmd_has_output(sh_inline::bash_command!("lsattr -d /").unwrap(), "-i-")?; Ok(()) } diff --git a/tests/inst/src/test.rs b/tests/inst/src/test.rs index 8b8510b8..6e3a63c0 100644 --- a/tests/inst/src/test.rs +++ b/tests/inst/src/test.rs @@ -237,8 +237,8 @@ mod tests { fn test_output() -> Result<()> { cmd_has_output(Command::new("true"), "")?; assert!(cmd_has_output(Command::new("true"), "foo").is_err()); - cmd_has_output(commandspec::sh_command!("echo foobarbaz; echo fooblahbaz").unwrap(), "blah")?; - assert!(cmd_has_output(commandspec::sh_command!("echo foobarbaz").unwrap(), "blah").is_err()); + cmd_has_output(sh_inline::bash_command!("echo foobarbaz; echo fooblahbaz").unwrap(), "blah")?; + assert!(cmd_has_output(sh_inline::bash_command!("echo foobarbaz").unwrap(), "blah").is_err()); Ok(()) } diff --git a/tests/inst/src/treegen.rs b/tests/inst/src/treegen.rs index 7c28fb70..d4c8bd71 100644 --- a/tests/inst/src/treegen.rs +++ b/tests/inst/src/treegen.rs @@ -1,5 +1,5 @@ use anyhow::{Context, Result}; -use commandspec::sh_execute; +use sh_inline::bash; use openat_ext::{FileExt, OpenatDirExt}; use rand::Rng; use std::fs::File; @@ -140,7 +140,7 @@ pub(crate) fn update_os_tree>( } assert!(mutated > 0); println!("Mutated ELF files: {}", mutated); - sh_execute!("ostree --repo={repo} commit --consume -b {ostref} --base={ostref} --tree=dir={tempdir} --owner-uid 0 --owner-gid 0 --selinux-policy-from-base --link-checkout-speedup --no-bindings --no-xattrs", + bash!("ostree --repo={repo} commit --consume -b {ostref} --base={ostref} --tree=dir={tempdir} --owner-uid 0 --owner-gid 0 --selinux-policy-from-base --link-checkout-speedup --no-bindings --no-xattrs", repo = repo_path.to_str().unwrap(), ostref = ostref, tempdir = tempdir.path().to_str().unwrap()).context("Failed to commit updated content")?; From dac2ad288f40ba93a125e69f78edd6a7c3866074 Mon Sep 17 00:00:00 2001 From: Matt Bilker Date: Tue, 25 Aug 2020 18:12:52 -0400 Subject: [PATCH 13/20] Fix mkinitcpio with newer systemd versions - Fixes systemd failing to determine if `/sysroot` is valid because of `/etc/os-release` not being available yet. - Related: #1759 --- src/boot/mkinitcpio/ostree | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/boot/mkinitcpio/ostree b/src/boot/mkinitcpio/ostree index 7f21cacd..3aa0659f 100644 --- a/src/boot/mkinitcpio/ostree +++ b/src/boot/mkinitcpio/ostree @@ -5,6 +5,6 @@ build() { add_binary /usr/lib/ostree/ostree-remount add_file /usr/lib/systemd/system/ostree-prepare-root.service - add_symlink /usr/lib/systemd/system/initrd-switch-root.target.wants/ostree-prepare-root.service \ + add_symlink /usr/lib/systemd/system/initrd-root-fs.target.wants/ostree-prepare-root.service \ /usr/lib/systemd/system/ostree-prepare-root.service } From ef55c2c981ee543d852d05b36231d0d10bcefde8 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 26 Aug 2020 17:00:19 +0000 Subject: [PATCH 14/20] tests/inst: Update to published sh-inline crate And I made a few more API tweaks, such as supporting `Path` objects directly and also not needing e.g. `commit = commit`, see - https://github.com/cgwalters/rust-sh-inline/commit/cfa7c71126f23545a7d4782cad650eab60e74204 - https://github.com/cgwalters/rust-sh-inline/commit/679bce4cc7ce65641e0c9bd33654510575583de8 --- tests/inst/Cargo.toml | 3 +-- tests/inst/src/destructive.rs | 24 ++++++++++++------------ tests/inst/src/repobin.rs | 10 ++++------ tests/inst/src/test.rs | 9 +++++++-- tests/inst/src/treegen.rs | 6 +++--- 5 files changed, 27 insertions(+), 25 deletions(-) diff --git a/tests/inst/Cargo.toml b/tests/inst/Cargo.toml index b361accc..31b43b4e 100644 --- a/tests/inst/Cargo.toml +++ b/tests/inst/Cargo.toml @@ -14,8 +14,7 @@ structopt = "0.3" serde = "1.0.111" serde_derive = "1.0.111" serde_json = "1.0" -# To be published on crates.io soon -sh-inline = { git = "https://github.com/cgwalters/rust-sh-inline" } +sh-inline = "0.1.0" anyhow = "1.0" tempfile = "3.1.0" glib = "0.10" diff --git a/tests/inst/src/destructive.rs b/tests/inst/src/destructive.rs index d1ebe35d..d6977bff 100644 --- a/tests/inst/src/destructive.rs +++ b/tests/inst/src/destructive.rs @@ -20,10 +20,10 @@ //! AUTOPKGTEST_REBOOT_MARK. use anyhow::{Context, Result}; -use sh_inline::bash; use rand::seq::SliceRandom; use rand::Rng; use serde::{Deserialize, Serialize}; +use sh_inline::bash; use std::collections::BTreeMap; use std::io::Write; use std::path::Path; @@ -305,10 +305,7 @@ fn parse_and_validate_reboot_mark>( fn validate_pending_commit(pending_commit: &str, commitstates: &CommitStates) -> Result<()> { if pending_commit != commitstates.target { bash!("rpm-ostree status -v")?; - bash!( - "ostree show {pending_commit}", - pending_commit = pending_commit - )?; + bash!("ostree show {pending_commit}", pending_commit)?; anyhow::bail!( "Expected target commit={} but pending={} ({:?})", commitstates.target, @@ -455,6 +452,7 @@ fn impl_transaction_test>( ); // Reset the target ref to booted, and perform a cleanup // to ensure we're re-downloading objects each time + let testref = TESTREF; bash!( " systemctl stop rpm-ostreed @@ -462,8 +460,8 @@ fn impl_transaction_test>( ostree reset testrepo:{testref} {booted_commit} rpm-ostree cleanup -pbrm ", - testref = TESTREF, - booted_commit = booted_commit + testref, + booted_commit, ) .with_context(|| { format!( @@ -569,7 +567,7 @@ fn transactionality() -> Result<()> { bash!( "ostree remote delete --if-exists testrepo ostree remote add --set=gpg-verify=false testrepo {url}", - url = url + url )?; if firstrun { @@ -582,16 +580,18 @@ fn transactionality() -> Result<()> { generate_update(&commit)?; // Directly set the origin, so that we're not dependent on the pending deployment. // FIXME: make this saner + let origref = ORIGREF; + let testref = TESTREF; bash!( " ostree admin set-origin testrepo {url} {testref} ostree refs --create testrepo:{testref} {commit} ostree refs --create={origref} {commit} ", - url = url, - origref = ORIGREF, - testref = TESTREF, - commit = commit + url, + origref, + testref, + commit )?; // We gather a single "cycle time" at start as a way of gauging how // long an upgrade should take, so we know when to interrupt. This diff --git a/tests/inst/src/repobin.rs b/tests/inst/src/repobin.rs index b034326a..2180e81f 100644 --- a/tests/inst/src/repobin.rs +++ b/tests/inst/src/repobin.rs @@ -5,7 +5,7 @@ use std::path::Path; use crate::test::*; use anyhow::{Context, Result}; -use sh_inline::{bash_command, bash}; +use sh_inline::{bash, bash_command}; use with_procspawn_tempdir::with_procspawn_tempdir; #[itest] @@ -45,9 +45,7 @@ fn test_mtime() -> Result<()> { " )?; let ts = Path::new("repo").metadata()?.modified().unwrap(); - bash!( - r#"ostree --repo=repo commit -b test -s "bump mtime" --tree=dir=tmproot >/dev/null"# - )?; + bash!(r#"ostree --repo=repo commit -b test -s "bump mtime" --tree=dir=tmproot >/dev/null"#)?; assert_ne!(ts, Path::new("repo").metadata()?.modified().unwrap()); Ok(()) } @@ -88,8 +86,8 @@ fn test_pull_basicauth() -> Result<()> { ostree --repo=repo remote add --set=gpg-verify=false origin-badauth {unauthuri} ostree --repo=repo remote add --set=gpg-verify=false origin-goodauth {authuri} "#, - osroot = osroot.to_str(), - serverrepo = serverrepo.to_str(), + osroot = osroot, + serverrepo = serverrepo, baseuri = baseuri.to_string(), unauthuri = unauthuri.to_string(), authuri = authuri.to_string() diff --git a/tests/inst/src/test.rs b/tests/inst/src/test.rs index 6e3a63c0..9d8e156c 100644 --- a/tests/inst/src/test.rs +++ b/tests/inst/src/test.rs @@ -237,8 +237,13 @@ mod tests { fn test_output() -> Result<()> { cmd_has_output(Command::new("true"), "")?; assert!(cmd_has_output(Command::new("true"), "foo").is_err()); - cmd_has_output(sh_inline::bash_command!("echo foobarbaz; echo fooblahbaz").unwrap(), "blah")?; - assert!(cmd_has_output(sh_inline::bash_command!("echo foobarbaz").unwrap(), "blah").is_err()); + cmd_has_output( + sh_inline::bash_command!("echo foobarbaz; echo fooblahbaz").unwrap(), + "blah", + )?; + assert!( + cmd_has_output(sh_inline::bash_command!("echo foobarbaz").unwrap(), "blah").is_err() + ); Ok(()) } diff --git a/tests/inst/src/treegen.rs b/tests/inst/src/treegen.rs index d4c8bd71..975db472 100644 --- a/tests/inst/src/treegen.rs +++ b/tests/inst/src/treegen.rs @@ -1,7 +1,7 @@ use anyhow::{Context, Result}; -use sh_inline::bash; use openat_ext::{FileExt, OpenatDirExt}; use rand::Rng; +use sh_inline::bash; use std::fs::File; use std::io::prelude::*; use std::os::unix::fs::FileExt as UnixFileExt; @@ -141,8 +141,8 @@ pub(crate) fn update_os_tree>( assert!(mutated > 0); println!("Mutated ELF files: {}", mutated); bash!("ostree --repo={repo} commit --consume -b {ostref} --base={ostref} --tree=dir={tempdir} --owner-uid 0 --owner-gid 0 --selinux-policy-from-base --link-checkout-speedup --no-bindings --no-xattrs", - repo = repo_path.to_str().unwrap(), + repo = repo_path, ostref = ostref, - tempdir = tempdir.path().to_str().unwrap()).context("Failed to commit updated content")?; + tempdir = tempdir.path()).context("Failed to commit updated content")?; Ok(()) } From d5b8929017265597b792928413f5d64ebb223070 Mon Sep 17 00:00:00 2001 From: Felix Krull Date: Tue, 25 Aug 2020 19:57:27 +0200 Subject: [PATCH 15/20] lib: add some missing version tags --- src/libostree/ostree-repo-commit.c | 2 ++ src/libostree/ostree-sign.c | 2 ++ src/libostree/ostree-sign.h | 2 ++ 3 files changed, 6 insertions(+) diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index 0c9de239..20340257 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -4332,6 +4332,8 @@ ostree_repo_commit_modifier_set_sepolicy (OstreeRepoCommitModifier * In many cases, one wants to create a "derived" commit from base commit. * SELinux policy labels are part of that base commit. This API allows * one to easily set up SELinux labeling from a base commit. + * + * Since: 2020.4 */ gboolean ostree_repo_commit_modifier_set_sepolicy_from_commit (OstreeRepoCommitModifier *modifier, diff --git a/src/libostree/ostree-sign.c b/src/libostree/ostree-sign.c index bcb5d0a6..e78ffe26 100644 --- a/src/libostree/ostree-sign.c +++ b/src/libostree/ostree-sign.c @@ -593,6 +593,8 @@ ostree_sign_get_by_name (const gchar *name, GError **error) * Based on ostree_repo_add_gpg_signature_summary implementation. * * Returns: @TRUE if summary file has been signed with all provided keys + * + * Since: 2020.2 */ gboolean ostree_sign_summary (OstreeSign *self, diff --git a/src/libostree/ostree-sign.h b/src/libostree/ostree-sign.h index 0d069059..75dd4837 100644 --- a/src/libostree/ostree-sign.h +++ b/src/libostree/ostree-sign.h @@ -52,6 +52,8 @@ G_GNUC_END_IGNORE_DEPRECATIONS /** * OSTREE_SIGN_NAME_ED25519: * The name of the default ed25519 signing type. + * + * Since: 2020.4 */ #define OSTREE_SIGN_NAME_ED25519 "ed25519" From f4d0b1708028e7be6c1d9632145133cbdded0d26 Mon Sep 17 00:00:00 2001 From: Felix Krull Date: Tue, 25 Aug 2020 20:43:01 +0200 Subject: [PATCH 16/20] lib: mark out parameters as out parameters --- src/libostree/ostree-sign.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libostree/ostree-sign.c b/src/libostree/ostree-sign.c index e78ffe26..ee7e928d 100644 --- a/src/libostree/ostree-sign.c +++ b/src/libostree/ostree-sign.c @@ -271,7 +271,7 @@ ostree_sign_load_pk (OstreeSign *self, * ostree_sign_data: * @self: an #OstreeSign object * @data: the raw data to be signed with pre-loaded secret key - * @signature: in case of success will contain signature + * @signature: (out): in case of success will contain signature * @cancellable: A #GCancellable * @error: a #GError * @@ -305,6 +305,7 @@ ostree_sign_data (OstreeSign *self, * @self: an #OstreeSign object * @data: the raw data to check * @signatures: the signatures to be checked + * @out_success_message: (out) (nullable) (optional): success message returned by the signing engine * @error: a #GError * * Verify given data against signatures with pre-loaded public keys. @@ -372,6 +373,7 @@ _sign_detached_metadata_append (OstreeSign *self, * @self: an #OstreeSign object * @repo: an #OsreeRepo object * @commit_checksum: SHA256 of given commit to verify + * @out_success_message: (out) (nullable) (optional): success message returned by the signing engine * @cancellable: A #GCancellable * @error: a #GError * From b3c7b059eaee3123d5b2523065726e866c533fe9 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Fri, 28 Aug 2020 12:35:28 -0400 Subject: [PATCH 17/20] ostree-prepare-root: Fix /etc bind mount We were bind-mounting the initramfs' `/etc` (to itself) instead of the target deployment `/etc` (to itself). Since we're already `chdir`'ed into it, we can just drop the leading slash. --- src/switchroot/ostree-prepare-root.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/switchroot/ostree-prepare-root.c b/src/switchroot/ostree-prepare-root.c index f7e4fe47..6351babb 100644 --- a/src/switchroot/ostree-prepare-root.c +++ b/src/switchroot/ostree-prepare-root.c @@ -251,7 +251,7 @@ main(int argc, char *argv[]) * sysroot, we still need a writable /etc. And to avoid race conditions * we ensure it's writable in the initramfs, before we switchroot at all. */ - if (mount ("/etc", "/etc", NULL, MS_BIND, NULL) < 0) + if (mount ("etc", "etc", NULL, MS_BIND, NULL) < 0) err (EXIT_FAILURE, "failed to make /etc a bind mount"); /* Pass on the fact that we discovered a readonly sysroot to ostree-remount.service */ int fd = open (_OSTREE_SYSROOT_READONLY_STAMP, O_WRONLY | O_CREAT | O_CLOEXEC, 0644); From a7a751b69f2315635d6ae38a0b1344287b67079a Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Fri, 28 Aug 2020 12:35:29 -0400 Subject: [PATCH 18/20] ostree-remount: Remount /etc rw if needed When we remount `/sysroot` as read-only, we also make `/etc` read-only. This is usually OK because we then remount `/var` read-write, which also flips `/etc` back to read-write... unless `/var` is a separate filesystem and not a bind-mount to the stateroot `/var`. Fix this by just remounting `/etc` read-write in the read-only sysroot case. Eventually, I think we should rework this to set everything up the way we want from the initramfs (#2115). This would also eliminate the window during which `/etc` is read-only while `ostree-remount` runs. --- src/switchroot/ostree-remount.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/switchroot/ostree-remount.c b/src/switchroot/ostree-remount.c index cfd270bb..3981682a 100644 --- a/src/switchroot/ostree-remount.c +++ b/src/switchroot/ostree-remount.c @@ -112,6 +112,11 @@ main(int argc, char *argv[]) bool sysroot_configured_readonly = unlink (_OSTREE_SYSROOT_READONLY_STAMP) == 0; do_remount ("/sysroot", !sysroot_configured_readonly); + /* And also make sure to make /etc rw again. We make this conditional on + * sysroot_configured_readonly because only in that case is it a bind-mount. */ + if (sysroot_configured_readonly) + do_remount ("/etc", true); + /* If /var was created as as an OSTree default bind mount (instead of being a separate filesystem) * then remounting the root mount read-only also remounted it. * So just like /etc, we need to make it read-write by default. From 8408f8913be2430987f5747bbaccc961e6f4bcce Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Fri, 28 Aug 2020 12:49:32 -0400 Subject: [PATCH 19/20] ci: Temporarily import kola test from jlebon's FCOS fork That test will not make it into the fedora-coreos-config repo until the libostree fix gets percolated down. PR is: https://github.com/coreos/fedora-coreos-config/pull/586 But we want to make sure that the fix does work and that we don't regress on it. So manually fetch it for now. --- .cci.jenkinsfile | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.cci.jenkinsfile b/.cci.jenkinsfile index ac65b9c8..5c6bb21d 100644 --- a/.cci.jenkinsfile +++ b/.cci.jenkinsfile @@ -72,6 +72,12 @@ parallel fcos: { tar -C insttree -xzvf insttree.tar.gz rsync -rlv insttree/ / coreos-assembler init --force https://github.com/coreos/fedora-coreos-config + # XXX: We temporarily add these tests until they get merged into FCOS proper + (mkdir -p tests/kola/var-mount + cd tests/kola/var-mount + curl -L --remote-name-all \ + https://raw.githubusercontent.com/jlebon/fedora-coreos-config/pr/var-mount/tests/kola/var-mount/{config.ign,test.sh} + chmod a+x test.sh) mkdir -p overrides/rootfs mv insttree/* overrides/rootfs/ rmdir insttree From 5d2183f63ef5ecf0e6e555c3214dd3fc17a8e5a1 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 3 Sep 2020 18:00:03 +0000 Subject: [PATCH 20/20] Release 2020.6 Let's get the /var mount fix out at least. --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 6d2c97d2..0c3c6fed 100644 --- a/configure.ac +++ b/configure.ac @@ -10,7 +10,7 @@ m4_define([year_version], [2020]) m4_define([release_version], [6]) m4_define([package_version], [year_version.release_version]) AC_INIT([libostree], [package_version], [walters@verbum.org]) -is_release_build=no +is_release_build=yes AC_CONFIG_HEADER([config.h]) AC_CONFIG_MACRO_DIR([buildutil]) AC_CONFIG_AUX_DIR([build-aux])