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

use WALinuxAgent-udev package for Azure udev rules #786

Merged
merged 2 commits into from
Jan 6, 2021

Conversation

miabbott
Copy link
Member

BZ#1748432 was filed while working on #159 and the udev rules were
split out into a separate package as part of F32. Let's stop carrying
the rules in the overlay and use the provided package now.

@miabbott
Copy link
Member Author

I stumbled on this while doing some triage on a downstream BZ; see the comments here showing that the udev rules are not in the initrd of RHCOS:

https://bugzilla.redhat.com/show_bug.cgi?id=1756173#c4
https://bugzilla.redhat.com/show_bug.cgi?id=1756173#c6

This PR by itself builds fine (haven't actually tested in Azure yet). I think I also need a simple dracut module to have the rules added to the initrd. Any suggestions on the right ordering? (i.e. 05core, somewhere else, something entirely new?)

@lucab
Copy link
Contributor

lucab commented Dec 17, 2020

see the comments here showing that the udev rules are not in the initrd of RHCOS

It looks like we'd need the package to be updated to install them into the initramfs too, not only in the rootfs.

More generally, it looks like we'd benefit from having this dracut shortcoming fixed.

@miabbott
Copy link
Member Author

Never dabbled in dracut modules before, but I came up with this that seems to get the rules installed:

diff --git a/overlay.d/05core/usr/lib/dracut/modules.d/25coreos-udev/module-setup.sh b/overlay.d/05core/usr/lib/dracut/modules.d/25coreos-udev/module-setup.sh
new file mode 100644
index 0000000..4ad324d
--- /dev/null
+++ b/overlay.d/05core/usr/lib/dracut/modules.d/25coreos-udev/module-setup.sh
@@ -0,0 +1,13 @@
+#!/bin/bash
+# -*- mode: shell-script; indent-tabs-mode: nil; sh-basic-offset: 4; -*-
+# ex: ts=8 sw=4 sts=4 et filetype=sh
+
+# We want to provide Azure udev rules as part of the initrd, so that Ignition
+# is able to detect disks and act on them.
+
+install() {
+    inst_simple WALinuxAgent-udev
+    inst_multiple \
+        /usr/lib/udev/rules.d/66-azure-storage.rules \
+        /usr/lib/udev/rules.d/99-azure-product-uuid.rules
+}

@dustymabe
Copy link
Member

would it be most appropriate to add the dracut snippet to the WALinux-udev RPM?

@miabbott
Copy link
Member Author

miabbott commented Dec 17, 2020

would it be most appropriate to add the dracut snippet to the WALinux-udev RPM?

I think that is a good long-term solution; I'm proposing this because it is something that we would like to include in RHCOS in the near term.

@cgwalters
Copy link
Member

would it be most appropriate to add the dracut snippet to the WALinux-udev RPM?

I think this is only relevant on Ignition based systems though.

@dustymabe
Copy link
Member

would it be most appropriate to add the dracut snippet to the WALinux-udev RPM?

I think this is only relevant on Ignition based systems though.

If it helps the argument, it's worth noting that we'd like to support Ignition as an option for the cloud base image at some point in the future.

@miabbott
Copy link
Member Author

If it helps the argument, it's worth noting that we'd like to support Ignition as an option for the cloud base image at some point in the future.

Filed a BZ to discuss this further with the maintainers - https://bugzilla.redhat.com/show_bug.cgi?id=1909287

@dustymabe
Copy link
Member

Just clearing up my stance: I don't think we need to block on having the dracut bits in the RPM. The open request in the BZ is plenty.

@jlebon
Copy link
Member

jlebon commented Jan 5, 2021

WFM too to just ship the dracut glue to add the rules to the initrd for now. What you have in #786 (comment) mostly LGTM. (You should be able to drop the inst_simple WALinuxAgent-udev bit, and maybe name it e.g. 25coreos-azure-udev instead and add a link to https://bugzilla.redhat.com/show_bug.cgi?id=1909287 in there?)

@miabbott miabbott marked this pull request as ready for review January 5, 2021 20:05
@miabbott
Copy link
Member Author

miabbott commented Jan 5, 2021

Incorporated feedback from @jlebon and successfully tested an FCOS image in Azure. (Our docs for booting on Azure are really good and easy to follow!)

Was able to successfully boot the image like this:

az vm create -n "${az_vm_name}" -g "${az_resource_group}" --image "${az_image_name}" --admin-username core --custom-data "$(cat ${ignition_path})" --attach-data-disks data0

And used an Ignition config that had the following:

{
  "ignition": {
    "version": "3.2.0"
  },
  "passwd": {
    "users": [
      {
        "name": "miabbott",
        "sshAuthorizedKeys": [
          "ssh-rsa AAAAB3Nz..."
        ]
      }
    ]
  },
  "storage": {
    "disks": [
      {
        "device": "/dev/disk/azure/scsi1/lun0",
        "partitions": [
          {
            "label": "varlibetcd",
            "sizeMiB": 0,
            "startMiB": 0
          }
        ]
      }
    ],
    "filesystems": [
      {
        "device": "/dev/disk/by-partlabel/varlibetcd",
        "format": "xfs",
        "path": "/var/lib/etcd"
      }
    ]
  },
  "systemd": {
    "units": [
      {
        "contents": "# Generated by FCCT\n[Unit]\nBefore=local-fs.target\nRequires=systemd-fsck@dev-disk-by\\x2dpartlabel-varlibetcd.service\nAfter=systemd-fsck@dev-disk-by\\x2dpartlabel-varlibetcd.service\n\n[Mount]\nWhere=/var/lib/etcd\nWhat=/dev/disk/by-partlabel/varlibetcd\nType=xfs\n\n[Install]\nRequiredBy=local-fs.target",
        "enabled": true,
        "name": "var-lib-etcd.mount"
      }
    ]
  }
}

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.

Typo in the commit message (WALinux-udev instead of WALinuxAgent-udev). LGTM otherwise!

So, I just realized that the el8 version of WALinuxAgent doesn't yet have a -udev subpackage. Were you planning to request that, or just keep shipping the rules there as FCOS did before this PR? (The divergence makes it slightly less appealing, though it's also nice to test out this cleanup in FCOS first.)

@miabbott
Copy link
Member Author

miabbott commented Jan 5, 2021

Typo in the commit message (WALinux-udev instead of WALinuxAgent-udev). LGTM otherwise!

So, I just realized that the el8 version of WALinuxAgent doesn't yet have a -udev subpackage. Were you planning to request that, or just keep shipping the rules there as FCOS did before this PR? (The divergence makes it slightly less appealing, though it's also nice to test out this cleanup in FCOS first.)

Dang, didn't realize the -udev package wasn't present in el8. I have doubts about getting that package built and shipped in time for RHCOS 4.7, so we'll probably have to carry the rules for RHCOS and install them in the initramfs separately.

@miabbott
Copy link
Member Author

miabbott commented Jan 5, 2021

Filed https://bugzilla.redhat.com/show_bug.cgi?id=1913074 for the el8 subpackage

BZ#1748432 was filed while working on coreos#159 and the udev rules were
split out into a separate package as part of F32.  Let's stop carrying
the rules in the overlay and use the provided package now.
We want the Azure udev rules present in the initrd, so that Ignition
is able to detect the disks and act on them.

If the udev rules end up being installed into the initramfs as part of
the WALinuxAgent-udev package, the dracut module should be removed.

See https://bugzilla.redhat.com/show_bug.cgi?id=1909287
@miabbott
Copy link
Member Author

miabbott commented Jan 6, 2021

Fixed up the commit message ⬆️

@jlebon jlebon changed the title use WALinux-udev package for Azure udev rules use WALinuxAgent-udev package for Azure udev rules Jan 6, 2021
@jlebon jlebon enabled auto-merge (rebase) January 6, 2021 14:02
@jlebon jlebon merged commit 8c07b73 into coreos:testing-devel Jan 6, 2021
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 6, 2021
This overlay includes the Azure udev rules which were removed from
05core in coreos/fedora-coreos-config#786 in
favour of using `WALinuxAgent-udev` instead. Though that package isn't
yet in el8 (see https://bugzilla.redhat.com/show_bug.cgi?id=1913074).
HuijingHei added a commit to HuijingHei/os that referenced this pull request Jun 7, 2022
HuijingHei added a commit to HuijingHei/os that referenced this pull request Jun 7, 2022
openshift-cherrypick-robot pushed a commit to openshift-cherrypick-robot/os that referenced this pull request Jun 9, 2022
miabbott pushed a commit to openshift/os that referenced this pull request Jun 10, 2022
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.

5 participants