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

live-generator: Avoid tmpfs/overlayfs, add stronger deps #499

Merged
merged 1 commit into from
Jul 2, 2020
Merged

live-generator: Avoid tmpfs/overlayfs, add stronger deps #499

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 0 additions & 12 deletions overlay.d/05core/usr/lib/dracut/modules.d/20live/coreos-populate-writable.service
View file
Open in desktop

This file was deleted.

128 changes: 87 additions & 41 deletions overlay.d/05core/usr/lib/dracut/modules.d/20live/live-generator
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,14 +78,22 @@ else
mkdir -p /run/media/iso
isosrc=dev/disk/by-label/${isoroot}
isosrc_escaped=$(systemd-escape -p --suffix=device "${isosrc}")
initrd_rootdev_target_d="${UNIT_DIR}"/initrd-root-device.target.d
mkdir -p "${initrd_rootdev_target_d}"
cat > "${initrd_rootdev_target_d}/50-root-device.conf" <<EOF
[Unit]
After=${isosrc_escaped}
Requires=${isosrc_escaped}
EOF
cat >"${UNIT_DIR}/run-media-iso.mount" <<EOF
# Automatically generated by live-generator

[Unit]
DefaultDependencies=false
Copy link
Member

Choose a reason for hiding this comment

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

Not new, though since we're on the hunt for unnecessary DefaultDependencies=, I think this is one of them since initrd-root-device.target is after basic.target.

After=initrd-root-device.target
# HACK for https://github.com/coreos/fedora-coreos-config/issues/437
After=systemd-udev-settle.service
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure this is still needed? systemd-udev-settle.service is before sysinit.target, which is before basic.target (which is before initrd-root-device.target).

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't see much ordering around initrd-root-device.target:

sh-5.0# systemctl show initrd-root-device.target |grep -Ee 'Before|After'
Before=initrd.target
After=ignition-disks.service

Copy link
Member

Choose a reason for hiding this comment

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

Hmm weird. I was going according to bootup(7): https://github.com/systemd/systemd/blob/c03ef420fa7157b8d4881636fe72596a06e08bb6/man/bootup.xml#L243-L251. But you're right, there's no obvious ordering that seems to implement that.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, I think actually this may be not sufficient though, the idea is systemd-udev-settle.service doesn't run by default, and so one should actually use both Wants= and Requires= or so. Ah right, multipathd.service is:

sh-5.0# systemctl show systemd-udev-settle.service |grep WantedBy
WantedBy=multipathd.service multipathd-configure.service

Before=initrd-root-fs.target
After=${isosrc_escaped}
Requires=${isosrc_escaped}

[Mount]
What=/${isosrc}
Expand All @@ -100,7 +108,7 @@ EOF
[Unit]
DefaultDependencies=false
Before=initrd-root-fs.target
Requires=run-media-iso.mount
RequiresMountsFor=/run/media/iso

[Mount]
What=/run/media/iso/root.squashfs
Expand All @@ -109,67 +117,105 @@ Type=squashfs
EOF
fi

common_etcvar_unit() {
cat << EOF
# Automatically generated by live-generator
# It turns out that `tmpfs` currently munches all SELinux labels
# we set before policy is loaded, so we make an XFS filesystem
# loopback mounted that's sized the same as /run.
# https://github.com/coreos/fedora-coreos-config/pull/499
cat >"${UNIT_DIR}/sysroot-xfs-ephemeral-mkfs.service" <<'EOF'
[Unit]
DefaultDependencies=false
# Let's be sure we have basic devices, but other than that we
# can run really early.
After=systemd-tmpfiles-setup-dev.service
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, is this for /run? It's mounted by systemd itself on startup.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, I thought so too but I was seeing weird failures until I added this, see #499 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

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

In general I'm skeptical of units with absolutely no dependencies at all.

ConditionPathExists=/usr/lib/initrd-release
# Something seems to be causing us to rerun?
ConditionPathExists=!/run/ephemeral

# Make sure /sysroot is mounted first, since we're mounting under there
Requires=initrd-root-fs.target
After=initrd-root-fs.target

# Make sure our tmpfs is available
RequiresMountsFor=/writable

# Need to do this before Ignition mounts any other filesystems (potentially
# shadowing our own mount).
Before=ignition-mount.service
[Service]
Type=oneshot
RemainAfterExit=yes
ExecStart=/bin/sh -c 'set -euo pipefail; mem=$$(($$(stat -f -c "%%b * %%s / 1024" /run))) && /bin/truncate -s $${mem}k /run/ephemeral.xfsloop'
ExecStart=/sbin/mkfs.xfs /run/ephemeral.xfsloop
ExecStart=/bin/mkdir /run/ephemeral
EOF
}

supports_squashfs_overlayfs=1
case "$(uname -r)" in
4.18.*) supports_squashfs_overlayfs=0
esac
add_requires sysroot-xfs-ephemeral-mkfs.service initrd-root-fs.target

if [ "${supports_squashfs_overlayfs}" = 1 ]; then
common_etcvar_unit > "${UNIT_DIR}/sysroot-etc.mount"
cat >>"${UNIT_DIR}/sysroot-etc.mount" <<EOF
cat >>"${UNIT_DIR}/run-ephemeral.mount" <<EOF
[Unit]
DefaultDependencies=false
Requires=sysroot-xfs-ephemeral-mkfs.service
After=sysroot-xfs-ephemeral-mkfs.service
[Mount]
What=overlay
Where=/sysroot/etc
Type=overlay
Options=lowerdir=/sysroot/etc,upperdir=/writable/etc/upper,workdir=/writable/etc/work,redirect_dir=on,index=on,xino=on
What=/run/ephemeral.xfsloop
Where=/run/ephemeral
Type=xfs
Options=loop,discard
EOF
else
# RHEL8 can't do overlayfs on squashfs, so we just copy
# /etc fully into RAM. It's not a large amount of data.
common_etcvar_unit > "${UNIT_DIR}/sysroot-etc-copy.service"
cat >>"${UNIT_DIR}/sysroot-etc-copy.service" <<EOF

cat >"${UNIT_DIR}/sysroot-xfs-ephemeral-setup.service" <<EOF
[Unit]
DefaultDependencies=false
RequiresMountsFor=/run/ephemeral
ConditionPathExists=/usr/lib/initrd-release
ConditionPathExists=!/run/ephemeral/var
# Make sure /sysroot is mounted first, since we're mounting under there
Requires=sysroot.mount
After=sysroot.mount
# And after OSTree has set up the chroot() equivalent
After=ostree-prepare-root.service

# We're part of assembling the root fs
Before=initrd-root-fs.target

[Service]
Type=oneshot
RemainAfterExit=yes
ExecStart=/bin/cp -a /sysroot/etc /writable/etc-copy
ExecStart=/bin/cp -a /sysroot/etc /run/ephemeral/etc
ExecStart=/bin/mkdir /run/ephemeral/var
EOF

common_etcvar_unit() {
cat << EOF
# Automatically generated by live-generator
[Unit]
DefaultDependencies=false
# Make sure our tmpfs is available
Requires=sysroot-xfs-ephemeral-setup.service
After=sysroot-xfs-ephemeral-setup.service
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 should also have Before=initrd-root-fs.target since the etc and var mounts should be considered part of setting up the root fs too, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, no harm in adding that, but we already have that implicitly because sysroot-relabel.service (the final service in the chain) has it, and is ordered after those units, so it forces the whole chain.

EOF
}

common_etcvar_unit > "${UNIT_DIR}/sysroot-etc.mount"
common_etcvar_unit > "${UNIT_DIR}/sysroot-etc.mount"
cat >>"${UNIT_DIR}/sysroot-etc.mount" <<EOF
After=sysroot-etc-copy.service
Requires=sysroot-etc-copy.service
[Mount]
What=/writable/etc-copy
What=/run/ephemeral/etc
Where=/sysroot/etc
Type=none
Options=bind
EOF
fi
add_requires sysroot-etc.mount initrd-root-fs.target

common_etcvar_unit >"${UNIT_DIR}/sysroot-var.mount"
cat >>"${UNIT_DIR}/sysroot-var.mount" <<EOF
[Mount]
What=/writable/var
What=/run/ephemeral/var
Where=/sysroot/var
Type=none
Options=bind
cgwalters marked this conversation as resolved.
Show resolved Hide resolved
EOF
add_requires sysroot-var.mount initrd-root-fs.target

cat >>"${UNIT_DIR}/sysroot-relabel.service" <<EOF
[Unit]
DefaultDependencies=false
RequiresMountsFor=/sysroot/etc /sysroot/var
Before=initrd-root-fs.target
[Service]
Type=oneshot
RemainAfterExit=yes
# We don't need the full relabeling spam by default for these
StandardOutput=null
ExecStart=/bin/coreos-relabel /etc
ExecStart=/bin/coreos-relabel /var
EOF
add_requires sysroot-relabel.service initrd-root-fs.target
8 changes: 2 additions & 6 deletions overlay.d/05core/usr/lib/dracut/modules.d/20live/module-setup.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ install_and_enable_unit() {
}

install() {
inst_multiple truncate

inst_script "$moddir/is-live-image.sh" \
"/usr/bin/is-live-image"

Expand All @@ -16,9 +18,6 @@ install() {
inst_simple "$moddir/live-generator" \
"$systemdutildir/system-generators/live-generator"

inst_simple "$moddir/coreos-populate-writable.service" \
"$systemdsystemunitdir/coreos-populate-writable.service"

inst_simple "$moddir/coreos-live-unmount-tmpfs-var.sh" \
"/usr/sbin/coreos-live-unmount-tmpfs-var"

Expand All @@ -31,9 +30,6 @@ install() {
install_and_enable_unit "coreos-live-persist-osmet.service" \
"default.target"

inst_simple "$moddir/writable.mount" \
"$systemdsystemunitdir/writable.mount"

inst_simple "$moddir/coreos-liveiso-network-kargs.sh" \
"/usr/sbin/coreos-liveiso-network-kargs"

Expand Down
10 changes: 0 additions & 10 deletions overlay.d/05core/usr/lib/dracut/modules.d/20live/writable.mount
View file
Open in desktop

This file was deleted.