From e159c8177a1e2b72b1501259b56834a3e3d256ff Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Fri, 1 Oct 2021 17:02:23 -0400 Subject: [PATCH 1/7] generator-lib.sh: add `karg` helper Lifted from `coreos-boot-edit.sh`. Prep for future patch. --- overlay.d/05core/usr/lib/coreos/generator-lib.sh | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/overlay.d/05core/usr/lib/coreos/generator-lib.sh b/overlay.d/05core/usr/lib/coreos/generator-lib.sh index b133e5ac67..dd19ad813d 100644 --- a/overlay.d/05core/usr/lib/coreos/generator-lib.sh +++ b/overlay.d/05core/usr/lib/coreos/generator-lib.sh @@ -17,3 +17,14 @@ have_karg() { done return 1 } + +karg() { + local name="$1" value="${2:-}" + local cmdline=( $( Date: Fri, 1 Oct 2021 17:02:39 -0400 Subject: [PATCH 2/7] coreos-boot-mount-generator: handle rd.multipath=0 With the new `karg` helper, this is trivial to do now. --- .../lib/systemd/system-generators/coreos-boot-mount-generator | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/overlay.d/05core/usr/lib/systemd/system-generators/coreos-boot-mount-generator b/overlay.d/05core/usr/lib/systemd/system-generators/coreos-boot-mount-generator index a742ff7751..58802cdca9 100755 --- a/overlay.d/05core/usr/lib/systemd/system-generators/coreos-boot-mount-generator +++ b/overlay.d/05core/usr/lib/systemd/system-generators/coreos-boot-mount-generator @@ -67,8 +67,8 @@ EOF # Otherwise, use the usual by-label symlink. # See discussion in https://github.com/coreos/fedora-coreos-config/pull/1022 bootdev=/dev/disk/by-label/boot -# TODO add equivalent of getargbool() so we handle rd.multipath=0 -if have_karg rd.multipath; then +mpath=$(karg rd.multipath) +if [ -n "${mpath}" ] && [ "${mpath}" != 0 ]; then bootdev=/dev/disk/by-label/dm-mpath-boot fi From 1107f905029d0fa8cdda3b7e5d6c503ee9b37a7e Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Fri, 1 Oct 2021 17:14:45 -0400 Subject: [PATCH 3/7] coreos-boot-edit: assume `rdcore` is in PATH There are already other places which assume that `rdcore` is in `PATH` so drop the full path to be more generic. --- .../lib/dracut/modules.d/35coreos-ignition/coreos-boot-edit.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/overlay.d/05core/usr/lib/dracut/modules.d/35coreos-ignition/coreos-boot-edit.sh b/overlay.d/05core/usr/lib/dracut/modules.d/35coreos-ignition/coreos-boot-edit.sh index 3b16813b46..8fca954497 100755 --- a/overlay.d/05core/usr/lib/dracut/modules.d/35coreos-ignition/coreos-boot-edit.sh +++ b/overlay.d/05core/usr/lib/dracut/modules.d/35coreos-ignition/coreos-boot-edit.sh @@ -31,5 +31,5 @@ rm -vrf ${initramfs_firstboot_network_dir} # append rootmap kargs to the BLS configs. root=$(karg root) if [ -z "${root}" ]; then - /usr/bin/rdcore rootmap /sysroot --boot-mount ${bootmnt} + rdcore rootmap /sysroot --boot-mount ${bootmnt} fi From 644372bc77dd5fedf68e8d5a05381475bddc46d4 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Fri, 1 Oct 2021 17:31:33 -0400 Subject: [PATCH 4/7] coreos-boot-mount-generator: support boot= karg This is analogous to `systemd-fstab-generator` parsing `root=` kargs. There is a precedent for this in the FIPS module. In fact, we lift code from there to make sure we're API compatible. The goal of supporting a `boot` karg is to eliminate possible races for the `by-label/boot` symlink in the real root if multiple exist. Part of: https://github.com/coreos/fedora-coreos-tracker/issues/976 --- .../coreos-boot-mount-generator | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/overlay.d/05core/usr/lib/systemd/system-generators/coreos-boot-mount-generator b/overlay.d/05core/usr/lib/systemd/system-generators/coreos-boot-mount-generator index 58802cdca9..43dfcd087d 100755 --- a/overlay.d/05core/usr/lib/systemd/system-generators/coreos-boot-mount-generator +++ b/overlay.d/05core/usr/lib/systemd/system-generators/coreos-boot-mount-generator @@ -61,15 +61,47 @@ EOF add_wants "${unit_name}" } +# Copied from +# https://github.com/dracutdevs/dracut/blob/9491e599282d0d6bb12063eddbd192c0d2ce8acf/modules.d/99base/dracut-lib.sh#L586 +# rather than sourcing it. +label_uuid_to_dev() { + local _dev + _dev="${1#block:}" + case "$_dev" in + LABEL=*) + echo "/dev/disk/by-label/$(echo "${_dev#LABEL=}" | sed 's,/,\\x2f,g;s, ,\\x20,g')" + ;; + PARTLABEL=*) + echo "/dev/disk/by-partlabel/$(echo "${_dev#PARTLABEL=}" | sed 's,/,\\x2f,g;s, ,\\x20,g')" + ;; + UUID=*) + echo "/dev/disk/by-uuid/$(echo "${_dev#UUID=}" | tr "[:upper:]" "[:lower:]")" + ;; + PARTUUID=*) + echo "/dev/disk/by-partuuid/$(echo "${_dev#PARTUUID=}" | tr "[:upper:]" "[:lower:]")" + ;; + esac +} + # If the root device is multipath, hook up /boot to use that too, # based on our custom udev rules in 90-coreos-device-mapper.rules # that creates "label found on mpath" links. # Otherwise, use the usual by-label symlink. # See discussion in https://github.com/coreos/fedora-coreos-config/pull/1022 bootdev=/dev/disk/by-label/boot +bootkarg=$(karg boot) mpath=$(karg rd.multipath) if [ -n "${mpath}" ] && [ "${mpath}" != 0 ]; then bootdev=/dev/disk/by-label/dm-mpath-boot +# Newer nodes inject boot=UUID=..., but we support a larger subset of the dracut/fips API +elif [ -n "${bootkarg}" ]; then + # Adapted from https://github.com/dracutdevs/dracut/blob/9491e599282d0d6bb12063eddbd192c0d2ce8acf/modules.d/01fips/fips.sh#L17 + case "$bootkarg" in + LABEL=* | UUID=* | PARTUUID=* | PARTLABEL=*) + bootdev="$(label_uuid_to_dev "$bootkarg")";; + /dev/*) bootdev=$bootkarg;; + *) echo "Unknown boot karg '${bootkarg}'; falling back to ${bootdev}";; + esac fi # We mount read-only by default mostly to protect From 09a2b1fab6bf40e131e6030c793bd13817858304 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Fri, 1 Oct 2021 17:34:40 -0400 Subject: [PATCH 5/7] coreos-boot-edit: inject `boot=` karg on first boot This ensures that the rootfs will always mount the correct boot filesystem in the future (see previous patch). Part of: https://github.com/coreos/fedora-coreos-tracker/issues/976 --- .../modules.d/35coreos-ignition/coreos-boot-edit.sh | 13 +++++++++++++ tests/kola/root-reprovision/luks/test.sh | 4 ++++ 2 files changed, 17 insertions(+) diff --git a/overlay.d/05core/usr/lib/dracut/modules.d/35coreos-ignition/coreos-boot-edit.sh b/overlay.d/05core/usr/lib/dracut/modules.d/35coreos-ignition/coreos-boot-edit.sh index 8fca954497..1f759b4599 100755 --- a/overlay.d/05core/usr/lib/dracut/modules.d/35coreos-ignition/coreos-boot-edit.sh +++ b/overlay.d/05core/usr/lib/dracut/modules.d/35coreos-ignition/coreos-boot-edit.sh @@ -33,3 +33,16 @@ root=$(karg root) if [ -z "${root}" ]; then rdcore rootmap /sysroot --boot-mount ${bootmnt} fi + +# And similarly, only inject boot= if it's not already present. +boot=$(karg boot) +if [ -z "${boot}" ]; then + # XXX: `rdcore rootmap --inject-boot-karg` or maybe `rdcore bootmap` + eval $(blkid -o export "${bootdev}") + if [ -z "${UUID}" ]; then + # This should never happen + echo "Boot filesystem ${bootdev} has no UUID" >&2 + exit 1 + fi + rdcore kargs --boot-mount ${bootmnt} --append boot=UUID=${UUID} +fi diff --git a/tests/kola/root-reprovision/luks/test.sh b/tests/kola/root-reprovision/luks/test.sh index 656b6feee6..0fbde10329 100755 --- a/tests/kola/root-reprovision/luks/test.sh +++ b/tests/kola/root-reprovision/luks/test.sh @@ -42,6 +42,10 @@ case "${AUTOPKGTEST_REBOOT_MARK:-}" in grep root=UUID= /proc/cmdline grep rd.luks.name= /proc/cmdline ok "found root kargs" + + # while we're here, sanity-check that we have a boot=UUID karg too + grep boot=UUID= /proc/cmdline + ok "found boot karg" ;; *) fatal "unexpected mark: ${AUTOPKGTEST_REBOOT_MARK}";; esac From 89066d5ed78425d663ba9ff2023f136a40190a52 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Fri, 1 Oct 2021 17:37:54 -0400 Subject: [PATCH 6/7] coreos-boot-edit: persist boot UUID in /run The new `boot` karg will only show up on the second boot and onwards. So for the first boot, let's persist it in `/run` to tell `coreos-boot-mount-generator` the UUID to use. --- .../dracut/modules.d/35coreos-ignition/coreos-boot-edit.sh | 3 +++ .../systemd/system-generators/coreos-boot-mount-generator | 3 +++ tests/kola/misc-ro | 7 +++++++ tests/kola/root-reprovision/luks/test.sh | 7 +++++++ 4 files changed, 20 insertions(+) diff --git a/overlay.d/05core/usr/lib/dracut/modules.d/35coreos-ignition/coreos-boot-edit.sh b/overlay.d/05core/usr/lib/dracut/modules.d/35coreos-ignition/coreos-boot-edit.sh index 1f759b4599..4116f03217 100755 --- a/overlay.d/05core/usr/lib/dracut/modules.d/35coreos-ignition/coreos-boot-edit.sh +++ b/overlay.d/05core/usr/lib/dracut/modules.d/35coreos-ignition/coreos-boot-edit.sh @@ -45,4 +45,7 @@ if [ -z "${boot}" ]; then exit 1 fi rdcore kargs --boot-mount ${bootmnt} --append boot=UUID=${UUID} + # but also put it in /run for the first boot real root mount + mkdir -p /run/coreos + echo "${UUID}" > /run/coreos/bootfs_uuid fi diff --git a/overlay.d/05core/usr/lib/systemd/system-generators/coreos-boot-mount-generator b/overlay.d/05core/usr/lib/systemd/system-generators/coreos-boot-mount-generator index 43dfcd087d..5724fdcb26 100755 --- a/overlay.d/05core/usr/lib/systemd/system-generators/coreos-boot-mount-generator +++ b/overlay.d/05core/usr/lib/systemd/system-generators/coreos-boot-mount-generator @@ -102,6 +102,9 @@ elif [ -n "${bootkarg}" ]; then /dev/*) bootdev=$bootkarg;; *) echo "Unknown boot karg '${bootkarg}'; falling back to ${bootdev}";; esac +# This is used for the first boot only +elif [ -f /run/coreos/bootfs_uuid ]; then + bootdev=/dev/disk/by-uuid/$(cat /run/coreos/bootfs_uuid) fi # We mount read-only by default mostly to protect diff --git a/tests/kola/misc-ro b/tests/kola/misc-ro index 05d91c2927..82a2e55c0c 100755 --- a/tests/kola/misc-ro +++ b/tests/kola/misc-ro @@ -198,3 +198,10 @@ if find ${tmpd}/usr/{bin,sbin,libexec} ! -perm -0111 | grep -v clevis-luks-commo fi rm -r ${tmpd} ok "All initrd scripts are executable" + +# Sanity-check that boot is mounted by UUID +if ! systemctl cat boot.mount | grep -q What=/dev/disk/by-uuid; then + systemctl cat boot.mount + fatal "boot mounted not by UUID" +fi +ok "boot mounted by UUID" diff --git a/tests/kola/root-reprovision/luks/test.sh b/tests/kola/root-reprovision/luks/test.sh index 0fbde10329..86f9622d16 100755 --- a/tests/kola/root-reprovision/luks/test.sh +++ b/tests/kola/root-reprovision/luks/test.sh @@ -27,6 +27,13 @@ if ! grep prjquota <<< "${rootflags}"; then fi ok "root mounted with prjquota" +# while we're here, sanity-check that boot is mounted by UUID +if ! systemctl cat boot.mount | grep -q What=/dev/disk/by-uuid; then + systemctl cat boot.mount + fatal "boot mounted not by UUID" +fi +ok "boot mounted by UUID" + case "${AUTOPKGTEST_REBOOT_MARK:-}" in "") # check that ignition-ostree-growfs ran From ce08c51a455c4ac973b178e87dd560739a79b76c Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Tue, 26 Oct 2021 13:31:03 -0400 Subject: [PATCH 7/7] tests: add a simple reboot test All our reboot tests currently target some special feature which significantly modifies the system. But we should also have a reboot test which is just using the defaults. This patch adds such a test. --- tests/kola/misc-ro | 7 ------ tests/kola/reboot/test.sh | 47 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 7 deletions(-) create mode 100755 tests/kola/reboot/test.sh diff --git a/tests/kola/misc-ro b/tests/kola/misc-ro index 82a2e55c0c..05d91c2927 100755 --- a/tests/kola/misc-ro +++ b/tests/kola/misc-ro @@ -198,10 +198,3 @@ if find ${tmpd}/usr/{bin,sbin,libexec} ! -perm -0111 | grep -v clevis-luks-commo fi rm -r ${tmpd} ok "All initrd scripts are executable" - -# Sanity-check that boot is mounted by UUID -if ! systemctl cat boot.mount | grep -q What=/dev/disk/by-uuid; then - systemctl cat boot.mount - fatal "boot mounted not by UUID" -fi -ok "boot mounted by UUID" diff --git a/tests/kola/reboot/test.sh b/tests/kola/reboot/test.sh new file mode 100755 index 0000000000..06d3e12c03 --- /dev/null +++ b/tests/kola/reboot/test.sh @@ -0,0 +1,47 @@ +#!/bin/bash +set -xeuo pipefail +# kola: {"platforms": "qemu"} + +# These are read-only not-necessarily-related checks that verify default system +# configuration both on first and subsequent boots. + +ok() { + echo "ok" "$@" +} + +fatal() { + echo "$@" >&2 + exit 1 +} + +# /var +varsrc=$(findmnt -nvr /var -o SOURCE) +rootsrc=$(findmnt -nvr / -o SOURCE) +[[ $(realpath "$varsrc") == $(realpath "$rootsrc") ]] +ok "/var is backed by rootfs" + +# sanity-check that boot is mounted by UUID +if ! systemctl cat boot.mount | grep -q What=/dev/disk/by-uuid; then + systemctl cat boot.mount + fatal "boot mounted not by UUID" +fi +ok "boot mounted by UUID" + +case "${AUTOPKGTEST_REBOOT_MARK:-}" in + "") + ok "first boot" + /tmp/autopkgtest-reboot rebooted + ;; + + rebooted) + # check for expected default kargs + grep root=UUID= /proc/cmdline + ok "found root karg" + + grep boot=UUID= /proc/cmdline + ok "found boot karg" + + ok "second boot" + ;; + *) fatal "unexpected mark: ${AUTOPKGTEST_REBOOT_MARK}";; +esac