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.

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

overlay.d: add 30lvmdevices overlay #2566

Merged
merged 2 commits into from
Aug 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions manifests/fedora-coreos-base.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ ostree-layers:
- overlay/09misc
- overlay/20platform-chrony
- overlay/25azure-udev-rules
- overlay/30lvmdevices

# Be minimal
recommends: false
Expand Down
8 changes: 8 additions & 0 deletions overlay.d/30lvmdevices/etc/lvm/devices/system.devices
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# LVM uses devices listed in this file.
#
# This is an empty lvmdevices(8) file placed here by the CoreOS overlays.
# The existence of the file prevents LVM from auto-activating any LVM devices
# that aren't explicitly allowlisted by being added to this file. Any newly
# added PV/VG devices that get created via pvcreate/vgreate will have an entry
# added here automatically by the tools. For more information on this see
# https://github.com/coreos/fedora-coreos-tracker/issues/1517
2 changes: 2 additions & 0 deletions overlay.d/30lvmdevices/statoverride
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# Config file for overriding permission bits on overlay files/dirs
# Format: =<file mode in decimal> <absolute path to a file or directory>
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
enable coreos-populate-lvmdevices.service
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
[Unit]
Description=CoreOS Populate LVM Devices File
Documentation=https://github.com/coreos/fedora-coreos-tracker/issues/1517
# Only run this import once.
ConditionPathExists=!/var/lib/coreos-populate-lvmdevices.stamp
# Don't add default dependencies so we can run early enough to populate
# the devices file before any LVM devices are used.
DefaultDependencies=false
# Since our ConditionPathExists file lives under /var/lib/ let's make
# sure any filesystems/mounts that exist and are needed to access that
# directory are set up.
RequiresMountsFor=/var/lib
# On OpenShift/Kubernetes we want to ensure we run before kubernetes
# comes up where storage drivers may be initiated and present more LVM
# devices to the system that we don't want to be considered.
Before=kubelet.service
Copy link
Member

Choose a reason for hiding this comment

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

A bit of a layering violation here, which ultimately at least for OCP we'll fix with something in the shape of openshift/machine-config-operator#3865

But obviously a good idea to have here now.

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. It's probably not needed since we have DefaultDependencies=false, but I was trying to make sure we had the case covered. I'm happy to remove it if you think it's safe to do so.


[Service]
Type=oneshot
RemainAfterExit=yes
ExecStart=/usr/libexec/coreos-populate-lvmdevices
ExecStartPost=touch /var/lib/coreos-populate-lvmdevices.stamp

[Install]
WantedBy=default.target
46 changes: 46 additions & 0 deletions overlay.d/30lvmdevices/usr/libexec/coreos-populate-lvmdevices
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
#!/bin/bash
set -euo pipefail

# This script will detect any LVM devices and add them to the lvmdevices
# file, which will force this host to only consider those devices in
# the future. This script should be run once to do the population and
# should not need to be run again. See
# https://github.com/coreos/fedora-coreos-tracker/issues/1517

LVMDEVICESFILENAME="system.devices"
LVMDEVICESFILE="/etc/lvm/devices/${LVMDEVICESFILENAME}"

# If the devices file doesn't exist that is a bit odd because we
# shipped it in the same update this migration script runs but let's
# just bail out. Someone could have deleted the lvmdevices file and
# then later accidentally run the migration script again.
if [ ! -f $LVMDEVICESFILE ]; then
echo "$LVMDEVICESFILE does not exist. Exiting."
exit 0
fi

# If the file exists and the file is different than what was shipped
# then we exit early. In this case the system likely already had an
# lvmdevices file defined already.
if ! diff -u "/usr/${LVMDEVICESFILE}" "${LVMDEVICESFILE}"; then
echo "Detected modified $LVMDEVICESFILE file. Exiting."
exit 0
fi

# Detect all existing PVs using `pvs` with a blank devicesfile
# setting, which will un-limit the search.
PVS=$(pvs --devicesfile="" --noheadings -o pv_name)

if [ -z "$PVS" ]; then
echo "No LVM devices detected. Exiting."
exit 0
fi

echo "Populating lvmdevices file with detected devices."
for pv in $(pvs --devicesfile="" --noheadings -o pv_name); do
echo "Adding ${pv} to lvmdevices file $LVMDEVICESFILE"
lvmdevices --journal output --adddev "$pv" --devicesfile "$LVMDEVICESFILENAME"
done

echo "Activating lvmdevices after devices file population"
pvscan --cache --activate ay
16 changes: 15 additions & 1 deletion overlay.d/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ such as `azure`, `aws`, `gcp`. The chrony config for these NTP servers
should override other chrony configuration (e.g. DHCP-provided)
configuration.

25-azure-udev-rules
25azure-udev-rules
-------------------

Add udev rules for SRIOV networking on Azure. The udev rules are also
Expand All @@ -79,3 +79,17 @@ bits to include the rules in the initramfs too.
[1] https://github.com/coreos/fedora-coreos-tracker/issues/1383
[2] https://github.com/Azure/WALinuxAgent/pull/1622
[3] https://src.fedoraproject.org/rpms/WALinuxAgent/pull-request/4

30lvmdevices
-------------------

Populate an lvmdevices(8) file to limit LVM from autoactivating all
devices it sees in a system. By default systems will get a "blank"
configuration file with a comment in it explaining what it is used
for. There is also a one-time "populate" service that will run and
add any devices it sees into the devices file. This will serve to
import existing devices on upgrading systems or new systems with
pre-existing LVM devices attached. See the tracker issue [1] for more
information.

[1] https://github.com/coreos/fedora-coreos-tracker/issues/1517
65 changes: 65 additions & 0 deletions tests/kola/disks/lvmdevices
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
#!/bin/bash
## kola:
## description: Verify LVM devices file handling works as expected.
## This test is meant to cover coreos-populate-lvmdevices.service.
## # additionalDisks is only supported on qemu.
## platforms: qemu
## # A few extra disks to set up LVM on.
## additionalDisks: ["1G", "2G"]

set -xeuo pipefail

. $KOLA_EXT_DATA/commonlib.sh

LVMDEVICESFILENAME="system.devices"
LVMDEVICESFILE="/etc/lvm/devices/${LVMDEVICESFILENAME}"

# Check that coreos-populate-lvmdevices did run
if [ ! -e /var/lib/coreos-populate-lvmdevices.stamp ]; then
fatal "coreos-populate-lvmdevices didn't run"
fi

case "${AUTOPKGTEST_REBOOT_MARK:-}" in
"")
# Verify the lvmdevices file by default matches what was shipped.
if ! diff -u "/usr/${LVMDEVICESFILE}" "${LVMDEVICESFILE}"; then
fatal "Detected modified $LVMDEVICESFILE file."
fi

# Set up LVM on the two disks and set up a vg/lv/fs/mount on one
# of them. Specify a blank devicesfile so the *create commands
# won't touch our devices file.
pvcreate --devicesfile="" /dev/vda /dev/vdb
travier marked this conversation as resolved.
Show resolved Hide resolved
vgcreate --devicesfile="" vg1 /dev/vda
travier marked this conversation as resolved.
Show resolved Hide resolved
lvcreate --devicesfile="" vg1 --name=lv1 -l 100%FREE
mkfs.ext4 /dev/vg1/lv1
echo "/dev/vg1/lv1 /srv/ ext4 defaults 0 2" >> /etc/fstab

# Remove the stamp file to force the "migration" to happen on
# next boot.
rm -f /var/lib/coreos-populate-lvmdevices.stamp

# reboot to simulate running migration for first time on a
# system with pre-existing LVM devices
/tmp/autopkgtest-reboot rebooted
;;

rebooted)
# Check that the devices are in the devices file.
grep -q 'DEVNAME=/dev/vda' "$LVMDEVICESFILE" || fatal "Missing vda in devices file"
travier marked this conversation as resolved.
Show resolved Hide resolved
grep -q 'DEVNAME=/dev/vdb' "$LVMDEVICESFILE" || fatal "Missing vdb in devices file"

# Check that we can see the PVs
if [ "$(pvs --noheadings | wc -l)" != '2' ]; then
fatal "Incorrect number of LVM PVs detected"
fi

# Check that /srv/ is a mountpoint
if ! mountpoint /srv; then
fatal "/srv/ is not mounted, but it should be"
fi

ok "LVM Devices file populated correctly"
;;
*) fatal "unexpected mark: ${AUTOPKGTEST_REBOOT_MARK}";;
esac