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

40ignition-ostree: copy ESP contents as independent filesystems #794

Merged
merged 6 commits into from
Dec 22, 2020
Merged

40ignition-ostree: copy ESP contents as independent filesystems #794

merged 6 commits into from
Dec 22, 2020

Conversation

bgilbert
Copy link
Contributor

@bgilbert bgilbert commented Dec 20, 2020

If the firmware writes to an individual ESP replica, the RAID will desynchronize. Linux md will return reads from either replica, and then any dependent writes could corrupt the filesystem. To prevent this, FCCT will no longer put the ESP on a RAID (coreos/butane#178); instead we create multiple independent filesystems and copy the contents to each. This is okay because bootupd and fwupd should be the only things that care about the contents of the ESP.

Drop the mount unit for /boot/efi, since there's no longer going to be a "canonical ESP" to mount.

Don't worry too much about backward compatibility because we're making this change soon after the functionality landed, and before it was documented. For the record, old configs will fail on new systems (because the partitions will be RAID members) but new configs will skip copying /boot on old systems (because there's no filesystem labeled EFI-SYSTEM).

Fixes coreos/fedora-coreos-tracker#694 and RHBZ 1909453. Closes coreos/fedora-coreos-tracker#581. Obsoletes #407. Discussion in #718 and design doc update in coreos/enhancements#4.

Avoid a bunch of incidental mkfs output in the journal for
ignition-ostree-transposefs-detect.service.
kelvinfan001 added a commit to kelvinfan001/bootupd that referenced this pull request Dec 21, 2020
Following coreos/fedora-coreos-config#794,
the mount unit for `/boot/efi` will be dropped, since there is no
longer going to be a "canonical ESP"to mount.

No longer `ProtectClock=yes` in `bootupd.service` to allow bootupd
to mount the ESP.
kelvinfan001 added a commit to kelvinfan001/bootupd that referenced this pull request Dec 21, 2020
Following coreos/fedora-coreos-config#794,
the mount unit for `/boot/efi` will be dropped, since there is no
longer going to be a "canonical ESP"to mount.

No longer `ProtectClock=yes` in `bootupd.service` to allow bootupd
to mount the ESP.
kelvinfan001 added a commit to kelvinfan001/bootupd that referenced this pull request Dec 21, 2020
Following coreos/fedora-coreos-config#794,
the mount unit for `/boot/efi` will be dropped, since there is no
longer going to be a "canonical ESP" to mount.

No longer `ProtectClock=yes` in `bootupd.service` to allow bootupd
to mount the ESP.
kelvinfan001 added a commit to kelvinfan001/bootupd that referenced this pull request Dec 21, 2020
Following coreos/fedora-coreos-config#794,
the mount unit for `/boot/efi` will be dropped, since there is no
longer going to be a "canonical ESP" to mount.

No longer `ProtectClock=yes` in `bootupd.service` to allow bootupd
to mount the ESP.
kelvinfan001 added a commit to kelvinfan001/bootupd that referenced this pull request Dec 21, 2020
Following coreos/fedora-coreos-config#794,
the mount unit for `/boot/efi` will be dropped, since there is no
longer going to be a "canonical ESP" to mount.

No longer `ProtectClock=yes` in `bootupd.service` to allow bootupd
to mount the ESP.
kelvinfan001 added a commit to kelvinfan001/bootupd that referenced this pull request Dec 21, 2020
Following coreos/fedora-coreos-config#794,
the mount unit for `/boot/efi` will be dropped, since there is no
longer going to be a "canonical ESP" to mount.

No longer `ProtectClock=yes` in `bootupd.service` to allow bootupd
to mount the ESP.
Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

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

Some minor comments, but LGTM overall!

@@ -2,6 +2,7 @@
set -euo pipefail

boot_sector_size=440
esp_typeguid=c12a7328-f81f-11d2-ba4b-00a0c93ec93b
Copy link
Member

Choose a reason for hiding this comment

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

And this commit, Closes: https://github.com/coreos/fedora-coreos-tracker/issues/581?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

581 was reopened only to track that the original implementation was going to change, so this commit doesn't really fix 581 in a fundamental way. Closing that issue is more of a bookkeeping thing.

On RAID systems we're now going to have multiple ESPs, no one of which is
the "canonical ESP", so there's nothing we can mount here.  Drop the
mount unit.

Fixes: coreos/fedora-coreos-tracker#694
Make it clear that mount_and_restore_filesystem locates the filesystem
by FS label.
If the firmware writes to an individual replica, the RAID will
desynchronize.  Linux md will return reads from either replica, and then
any dependent writes could corrupt the filesystem.  To prevent this,
fcct will no longer put the ESP on a RAID; instead we create multiple
independent filesystems and copy the contents to each.  This is okay
because bootupd and fwupd should be the only things that care about the
contents of the ESP.

Don't worry too much about backward compatibility because we're making
this change soon after the functionality landed, and before it was
documented.  For the record, old configs will fail on new systems
(because the partitions will be RAID members) but new configs will
skip copying /boot on old systems (because there's no filesystem labeled
"EFI-SYSTEM").
@bgilbert
Copy link
Contributor Author

Updated!

@bgilbert bgilbert enabled auto-merge (rebase) December 22, 2020 21:06
@bgilbert bgilbert merged commit 16591e9 into coreos:testing-devel Dec 22, 2020
@bgilbert bgilbert deleted the raid/testing-devel branch December 22, 2020 21:51
kelvinfan001 added a commit to kelvinfan001/bootupd that referenced this pull request Dec 23, 2020
Following coreos/fedora-coreos-config#794,
the mount unit for `/boot/efi` will be dropped, since there is no
longer going to be a "canonical ESP" to mount.

No longer `ProtectClock=yes` in `bootupd.service` to allow bootupd
to mount the ESP.
kelvinfan001 added a commit to kelvinfan001/bootupd that referenced this pull request Dec 24, 2020
Following coreos/fedora-coreos-config#794,
the ESP will no longer be mounted by default in FCOS. Mount the
ESP at a directory with a random name in bootupd's sandboxed /tmp
ourselves before reading from/writing to it.

Note that ESPs on redundant disks is not yet supported, this commit
simply mounts the device with part-label `EFI-SYSTEM` if one exists.

`ProtectClock=yes` in `bootupd.service` is preventing the ESP from
being mounted, remove that for now.
kelvinfan001 added a commit to kelvinfan001/bootupd that referenced this pull request Dec 24, 2020
Following coreos/fedora-coreos-config#794,
the ESP will no longer be mounted in /boot. Adjust tests to mount
it before reading from it.
kelvinfan001 added a commit to kelvinfan001/bootupd that referenced this pull request Dec 24, 2020
Following coreos/fedora-coreos-config#794,
the ESP will no longer be mounted in /boot. Adjust tests to mount
it before reading from it.
jlebon added a commit to jlebon/os that referenced this pull request Jan 5, 2021
Specifically,
- the ESP unraiding change (coreos/fedora-coreos-config#794)
- working around buggy udev (coreos/fedora-coreos-config#779)
- --copy-network regression (coreos/fedora-coreos-config#800)

For the latter, also apply the same `|| exit 1` change from
coreos/fedora-coreos-config#800 to our own
dracut modules for good measure.

```
$ git shortlog --invert-grep --author='CoreOS Bot' 6c9f15c8857772f730cdb0b5b2df143e778c5d0d..bdcebad9c07a7f90661865ef8001d07fda896922
Benjamin Gilbert (6):
      40ignition-ostree: silence mkfs.xfs
      coreos-boot-mount-generator: stop mounting /boot/efi
      40ignition-ostree: rename mount_and_restore_filesystem
      40ignition-ostree: create mountpoint in mount_verbose
      40ignition-ostree: add get_partlabels_for_parttype helper
      40ignition-ostree: copy ESP contents as independent filesystems

Dusty Mabe (1):
      overrides: drop graduated overrides

Jonathan Lebon (7):
      40ignition-ostree/transposefs: also trigger udev on by-label link mismatch for ESP
      40ignition-ostree/transposefs: factor out function to restore fs
      40ignition-ostree/transposefs: relabel the root of reprovisioned filesystems
      40ignition-ostree/transposefs: load zram with num_devices=0
      Add rawhide repo file
      05core: re-order and rename some dracut modules
      05core: add `|| exit 1` to `systemctl add-{requires,wants}` calls

Kelvin Fan (1):
      overlay.d/15fcos: Stop utilizing c-l-h-m private dir

Prashanth Sundararaman (1):
      tests: Enable TPM test for all arches except s390x

Timothée Ravier (1):
      manifests/fedora-coreos-base: Mask systemd-repart
```
kelvinfan001 added a commit to kelvinfan001/bootupd that referenced this pull request Jan 5, 2021
Following coreos/fedora-coreos-config#794,
the ESP will no longer be mounted by default in FCOS. Mount the
ESP at a directory with a random name in bootupd's sandboxed /tmp
ourselves before reading from/writing to it.

Note that ESPs on redundant disks is not yet supported, this commit
simply mounts the device with part-label `EFI-SYSTEM` if one exists.

`ProtectClock=yes` in `bootupd.service` is preventing the ESP from
being mounted, remove that for now.
kelvinfan001 added a commit to kelvinfan001/bootupd that referenced this pull request Jan 5, 2021
Following coreos/fedora-coreos-config#794,
the ESP will no longer be mounted in /boot. Adjust tests to mount
it before reading from it.
openshift-merge-robot pushed a commit to coreos/bootupd that referenced this pull request Jan 5, 2021
Following coreos/fedora-coreos-config#794,
the ESP will no longer be mounted by default in FCOS. Mount the
ESP at a directory with a random name in bootupd's sandboxed /tmp
ourselves before reading from/writing to it.

Note that ESPs on redundant disks is not yet supported, this commit
simply mounts the device with part-label `EFI-SYSTEM` if one exists.

`ProtectClock=yes` in `bootupd.service` is preventing the ESP from
being mounted, remove that for now.
openshift-merge-robot pushed a commit to coreos/bootupd that referenced this pull request Jan 5, 2021
Following coreos/fedora-coreos-config#794,
the ESP will no longer be mounted in /boot. Adjust tests to mount
it before reading from it.
jlebon added a commit to jlebon/os that referenced this pull request Jan 6, 2021
Specifically,
- the ESP unraiding change (coreos/fedora-coreos-config#794)
- working around buggy udev (coreos/fedora-coreos-config#779)
- --copy-network regression (coreos/fedora-coreos-config#800)
- Azure udev rules in initramfs (coreos/fedora-coreos-config#786)

For the latter, also apply the same `|| exit 1` change from
coreos/fedora-coreos-config#800 to our own
dracut modules for good measure.

```
$ git shortlog --invert-grep --author='CoreOS Bot' 6c9f15c8857772f730cdb0b5b2df143e778c5d0d..8c07b7391473910ba3884ee0d3763743805ac78f
Benjamin Gilbert (6):
      40ignition-ostree: silence mkfs.xfs
      coreos-boot-mount-generator: stop mounting /boot/efi
      40ignition-ostree: rename mount_and_restore_filesystem
      40ignition-ostree: create mountpoint in mount_verbose
      40ignition-ostree: add get_partlabels_for_parttype helper
      40ignition-ostree: copy ESP contents as independent filesystems

Dusty Mabe (1):
      overrides: drop graduated overrides

Jonathan Lebon (7):
      40ignition-ostree/transposefs: also trigger udev on by-label link mismatch for ESP
      40ignition-ostree/transposefs: factor out function to restore fs
      40ignition-ostree/transposefs: relabel the root of reprovisioned filesystems
      40ignition-ostree/transposefs: load zram with num_devices=0
      Add rawhide repo file
      05core: re-order and rename some dracut modules
      05core: add `|| exit 1` to `systemctl add-{requires,wants}` calls

Kelvin Fan (1):
      overlay.d/15fcos: Stop utilizing c-l-h-m private dir

Luca BRUNO (1):
      overrides: drop stale Afterburn entries

Micah Abbott (2):
      use WALinuxAgent-udev package for Azure udev rules
      overlay: add new module for installing Azure udev rules

Prashanth Sundararaman (1):
      tests: Enable TPM test for all arches except s390x

Timothée Ravier (1):
      manifests/fedora-coreos-base: Mask systemd-repart
```
jlebon added a commit to jlebon/os that referenced this pull request Jan 7, 2021
We don't mount `/boot/efi` anymore, so the `ls` invocation succeeds
because it's just targeting an empty dir.

Matches coreos/fedora-coreos-config#794.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Do not mount /boot/efi by default metal: Support redundant bootable disks
2 participants