Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make handling of boot= and root= kargs idempotent #2141

Closed
wants to merge 4 commits into from
Closed

Make handling of boot= and root= kargs idempotent #2141

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6f7197a
dracut/boot-edit: Do nothing if we have a boot= karg
cgwalters Jan 2, 2023
df01601
dracut/ignition-setup: Use boot= karg if it exists
cgwalters Jan 2, 2023
7b4d459
ignition-uuid-root: no-op if we already have a root= karg
cgwalters Jan 10, 2023
d01094a
overlay: More no-ops if we have a boot= and/or root= karg
cgwalters Jan 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions overlay.d/05core/usr/lib/dracut/modules.d/35coreos-ignition/coreos-boot-edit.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@ karg() {
echo "${value}"
}

# If we've already bound boot, then we should have nothing to do.
bootdev=$(karg boot)
if [ -n "$bootdev" ]; then
exit 0
fi

# Mount /boot. Note that we mount /boot but we don't unmount it because we
# are run in a systemd unit with MountFlags=slave so it is unmounted for us.
bootmnt=/mnt/boot_partition
Expand Down
2 changes: 2 additions & 0 deletions overlay.d/05core/usr/lib/dracut/modules.d/35coreos-ignition/coreos-gpt-setup.service
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ After=dev-disk-by\x2dlabel-boot.device
# And since the boot device may be on multipath; optionally wait for it to
# appear via the dynamic target.
After=coreos-multipath-wait.target
# But we do nothing if the disk is already set up
ConditionKernelCommandLine=!root

# Run before services that use device nodes, preventing them from racing
# with udev activity generated by sgdisk
Expand Down
56 changes: 55 additions & 1 deletion overlay.d/05core/usr/lib/dracut/modules.d/35coreos-ignition/coreos-ignition-setup-user.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,60 @@ copy_file_if_exists() {
destination=/usr/lib/ignition
mkdir -p $destination

karg() {
local name="$1" value="${2:-}"
local cmdline=( $(</proc/cmdline) )
for arg in "${cmdline[@]}"; do
if [[ "${arg%%=*}" == "${name}" ]]; then
value="${arg#*=}"
fi
done
echo "${value}"
}

# Copied from
# https://github.com/dracutdevs/dracut/blob/9491e599282d0d6bb12063eddbd192c0d2ce8acf/modules.d/99base/dracut-lib.sh#L586
# rather than sourcing it.
label_uuid_to_dev() {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this should move to rdcore or so?

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
}

# This is copied from coreos-boot-mount-generator which we should likely run
# in the initramfs too
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think that'd be a safer path. This seems to be trying to undo the assumption that /dev/disk/by-label/boot points to our boot disk at first boot, but only for one service, whereas it's an assumption embedded all over the place. The generator could put the found device in e.g. /run/coreos/bootdev?

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
# This is used for the first boot only
elif [ -f /run/coreos/bootfs_uuid ]; then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is written by coreos-boot-edit and isn't relevant here.

bootdev=/dev/disk/by-uuid/$(cat /run/coreos/bootfs_uuid)
fi

if is-live-image; then
# Live image. If the user has supplied a config.ign via an appended
# initrd, put it in the right place.
Expand All @@ -27,6 +81,6 @@ else
mkdir -p $bootmnt
# mount as read-only since we don't strictly need write access and we may be
# running alongside other code that also has it mounted ro
mount -o ro /dev/disk/by-label/boot $bootmnt
mount -o ro $bootdev $bootmnt
copy_file_if_exists "${bootmnt}/ignition/config.ign" "${destination}/user.ign"
fi
2 changes: 2 additions & 0 deletions ...y.d/05core/usr/lib/dracut/modules.d/35coreos-ignition/coreos-ignition-unique-boot.service
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ ConditionPathExists=/etc/initrd-release
OnFailure=emergency.target
OnFailureJobMode=isolate

ConditionKernelCommandLine=!boot

# That's a weak dependency, so service won't fail if boot dissaperears
Wants=dev-disk-by\x2dlabel-boot.device
After=dev-disk-by\x2dlabel-boot.device
Expand Down
1 change: 1 addition & 0 deletions overlay.d/05core/usr/lib/dracut/modules.d/35coreos-ignition/coreos-unique-boot.service
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ DefaultDependencies=no
Before=ignition-diskful.target
Wants=systemd-udevd.service
After=systemd-udevd.service
ConditionKernelCommandLine=!boot
# And since the boot device may be on multipath; optionally wait for it to
# appear via the dynamic target.
After=coreos-multipath-wait.target
Expand Down
1 change: 1 addition & 0 deletions ...lay.d/05core/usr/lib/dracut/modules.d/40ignition-ostree/ignition-ostree-uuid-root.service
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ After=ignition-disks.service
# If we've reprovisioned the rootfs, then there's no need to restamp
ConditionPathExists=!/run/ignition-ostree-transposefs
ConditionKernelCommandLine=!rootfs.roothash
ConditionKernelCommandLine=!root
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would break multipath, where currently you have to put the root karg at install time but we do still want the restamping. Is this actually required for bootc? The service will run but the script should no-op if the filesystem UUID was already randomized.

Similarly for coreos-gpt-setup.service.


After=dev-disk-by\x2dlabel-root.device
# Avoid racing with fsck
Expand Down