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

05core: re-order and rename some dracut modules #800

Merged
merged 2 commits into from
Jan 4, 2021

Conversation

jlebon
Copy link
Member

@jlebon jlebon commented Jan 4, 2021

Unlike ln -sf, systemctl add-{requires,wants} wants to verify that
the target and member units all exist. Because we had some dracut
modules which ran earlier than the 30ignition module, we would lose
the service enablements which hook into e.g. ignition-complete.target
and related units.

Let's simply rename our modules so that they're ordered after
30ignition. While we're here, let's consistently prefix them by
coreos-. So overall, the changes are:

  • 30ignition-coreos -> 35coreos-ignition
  • 20live -> 35coreos-live
  • 15coreos-network -> 35coreos-network

I've verified that there are now no "Failed to add dependency on unit"
error messages at build-time. (The next patch enforces this more
strictly.)

Fixes: #778
Closes: #799

Unlike `ln -sf`, `systemctl add-{requires,wants}` wants to verify that
the target and member units all exist. Because we had some dracut
modules which ran earlier than the `30ignition` module, we would lose
the service enablements which hook into e.g. `ignition-complete.target`
and related units.

Let's simply rename our modules so that they're ordered after
`30ignition`. While we're here, let's consistently prefix them by
`coreos-`. So overall, the changes are:
- `30ignition-coreos` -> `35coreos-ignition`
- `20live` -> `35coreos-live`
- `15coreos-network` -> `35coreos-network`

I've verified that there are now no "Failed to add dependency on unit"
error messages at build-time. (The next patch enforces this more
strictly.)

Fixes: coreos#778
Closes: coreos#799
Copy link
Member

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@dustymabe dustymabe left a comment

Choose a reason for hiding this comment

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

LGTM

I almost wonder if we should add some of this context to overlay.d/README.md.

Because dracut directly sources `module-setup.sh` scripts and isn't
compatible with `set -e`, we can't just turn it on in our modules.
Instead, let's just manually add `|| exit 1` in all the calls to
`systemctl add-{requires,wants}` so that we catch regressions like
coreos#799 at build time
in the future.
@jlebon jlebon enabled auto-merge (rebase) January 4, 2021 21:36
@jlebon
Copy link
Member Author

jlebon commented Jan 4, 2021

I almost wonder if we should add some of this context to overlay.d/README.md.

Having it in overlay.d/README.md felt too high-level, so instead I added some context in the locations where we do || exit 1 (which essentially includes all the module-setup.sh files of the dracut modules which are affected by this).

@jlebon jlebon merged commit bdcebad into coreos:testing-devel Jan 4, 2021
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
```
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 jlebon deleted the pr/re-order-dracut branch April 23, 2023 23:29
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.

missing ignition-diskful and ignition-complete targets when building initrd
3 participants