dracut: add afterburn dracut module #224

yuqi-zhang · 2019-06-06T20:38:24Z

Add dracut module to run afterburn in the initramfs, and add an
initial service to fetch hostname on necessary providers (Azure).

yuqi-zhang · 2019-06-06T20:40:10Z

Also see discussion in: yuqi-zhang/afterburn-dracut#1

There is the question of whether we want this to run on the firstboot or all boots. I think the hybrid approach to conditionally track Azure's hostname iif no Ignition hostname is specified is possibly the most clean, but I'm not sure what the best way to detect that would be.

cc @jlebon @ajeddeloh @lucab

ashcrow

Looks sane to me

jlebon · 2019-06-10T15:30:31Z

LGTM modulo the pending discussions around ordering & firstboot semantics.

lucab · 2019-06-11T08:47:06Z

@jlebon has a good point, so I'm backtracking on my initial comment. Let's have the initramfs part first-boot only.

yuqi-zhang · 2019-06-11T20:50:22Z

I read around a bit and I think it mostly makes sense to keep this module as a firstboot initramfs hostname fetch. As for future boots/runtime hostname changes, I don't really foresee dhcp leased hostnames changes (and if we do, I think Azure handles it from what I can tell?), and static configurations are ignition based.

As for ordering, the ignition-files will be dropping static confs if we want to overwrite, so I think the present setup is correct.

Please let me know if either of those two understandings sound incorrect.

jlebon

Just one small comment, otherwise LGTM!

dracut/30afterburn/afterburn-hostname.service

lucab · 2019-06-12T15:13:11Z

dracut/30afterburn/afterburn-hostname.service

+# Azure needs to fetch the hostname in the initramfs
+ConditionKernelCommandLine=|ignition.platform.id=azure
+ConditionFirstBoot=yes
+After=ignition-disks.service


@yuqi-zhang did you test this service unit on an actual Azure machine? Because I think I made a mistake and this should be After=ignition-mount.service.

I guess since it only writes to /sysroot/etc, technically it just needs After=initrd-root-fs.target. That'll work on both FCOS and RHCOS (which doesn't have ignition-mount.service yet).

I added After=initrd-root-fs.target
Wants=initrd-root-fs.target based on https://github.com/coreos/ignition-dracut/blob/spec2x/dracut/30ignition/coreos-static-network.service

Ack. And do we care about ignition-remount-sysroot.service here? (I'm not so familiar with the new Ignition flow in initramfs)

And do we care about ignition-remount-sysroot.service here?

It doesn't hurt, but shouldn't make a difference either, see: coreos/ignition-dracut#38 (comment).

I suppose for safety we should? I'm also not sure on which systems that one actually runs.
And to your earlier point, I didn't test on Azure since I didn't have creds at the time, I made a mock unit with the same dependency tree to run on metal, and it worked as expected.

dracut/30afterburn/afterburn-hostname.service

jlebon · 2019-06-12T16:05:59Z

dracut/30afterburn/afterburn-hostname.service

+
+# This will be firstboot only to set up the correct
+# hostname for kube.
+ConditionFirstBoot=yes


One question, does this actually work correctly in the initrd? For the real root, systemd keys off of /etc/machine-id, but here it has to be able to make that determination before /sysroot is even mounted. So does it key off of the initrd's /etc/machine-id? Except that check will never change result unless we regen the initrd.

An alternative here is to hook into ignition-complete.target, which was made for this: https://github.com/coreos/ignition-dracut/blob/df88988f2f0791ca9ea1e14298c8523501d980c5/dracut/30ignition/ignition-complete.target#L3.

For RHCOS, since it doesn't have that target yet, you could do e.g. ignition-files.service.requires?

Afterburn does not depend on Ignition though, so we probably shouldn't make it use that target. I agree though that using ConditionFirstBoot from the initramfs seems sketchy. Not sure the best path forward here. I think this is a case where the distro-dependence is showing.

Afterburn does not depend on Ignition though, so we probably shouldn't make it use that target.

We explicitly reference ignition targets, I don't really see a harm in cross-linking per se? Unless you want them to be as separate as possible.

I did some testing with a metal-bios image. If I rd.break into the initramfs on the first boot, I see the unit as inactive, and a message that: Condition: start condition failed (and same with subsequent boots). Interesting if I try to systemctl start the process manually, it will add a line CondtionFirstBoot=yes was not met. I'm not sure if that's the expected behaviour or not (i.e. which condition is not being met).

In either case, this does not seem to be very robust... should we

Afterburn does not depend on Ignition though, so we probably shouldn't make it use that target.

I'd say this unit is inherently dependent on Ignition already by simply having ConditionKernelCommandLine=|ignition.platform.id=azure. :)

We could do something fancy here and "connect" the two in the overlay. Though unless there's demand for running Afterburn without Ignition, I'm not sure if it's worth the investment.

I think this is fine now that I think about it. Part of me still just wants one giant fcos-dracut repo, but I think that ship has sailed.

yuqi-zhang · 2019-06-13T20:37:00Z

Ok I've updated it to depend on ignition and tested it to work (firstboot only)

For RHCOS I think we want to carry a patch to make it:

mkdir -p "$initdir/$systemdsystemunitdir/ignition-files.service.requires"
    ln -s "../afterburn-hostname.service" "$initdir/$systemdsystemunitdir/ignition-files.service.requires/afterburn-hostname.service"

for now, since I don't really see a reason to branch.
PTAL.

jlebon

Just some trivial nits, otherwise LGTM!

jlebon · 2019-06-13T21:00:53Z

dracut/30afterburn/module-setup.sh

+        "$systemdutildir/system/afterburn-hostname.service"
+
+    # We want the afterburn-hostname to be firstboot only, so ignition-provided
+    # hostname changes do not get overwritten on subsequentboots


Minor: subsequent boots
Super minor: Ignition

Fixed up, thanks for the review!

Add dracut module to run afterburn in the initramfs, and add an initial service to fetch hostname on necessary providers (Azure). Signed-off-by: Yu Qi Zhang <jerzhang@redhat.com>

yuqi-zhang · 2019-06-14T17:44:34Z

@jlebon Should be good to go. I also opened https://src.fedoraproject.org/rpms/rust-afterburn/pull-request/1 to show how to pull it in once we release a new crate

jlebon

Yup this LGTM!
I don't have merge access, will leave it to someone else to have a final look and merge.

ashcrow reviewed Jun 10, 2019

View reviewed changes

jlebon reviewed Jun 12, 2019

View reviewed changes

dracut/30afterburn/afterburn-hostname.service Show resolved Hide resolved

lucab reviewed Jun 12, 2019

View reviewed changes

yuqi-zhang force-pushed the add-dracut-modules branch 2 times, most recently from c4c4099 to 303ac6d Compare June 12, 2019 15:23

jlebon reviewed Jun 12, 2019

View reviewed changes

dracut/30afterburn/afterburn-hostname.service Show resolved Hide resolved

yuqi-zhang force-pushed the add-dracut-modules branch from 303ac6d to 8ea0971 Compare June 12, 2019 15:49

jlebon reviewed Jun 12, 2019

View reviewed changes

yuqi-zhang force-pushed the add-dracut-modules branch from 8ea0971 to ec8f0f7 Compare June 13, 2019 20:35

jlebon reviewed Jun 13, 2019

View reviewed changes

dracut: add afterburn dracut module

e745830

Add dracut module to run afterburn in the initramfs, and add an initial service to fetch hostname on necessary providers (Azure). Signed-off-by: Yu Qi Zhang <jerzhang@redhat.com>

yuqi-zhang force-pushed the add-dracut-modules branch from ec8f0f7 to e745830 Compare June 13, 2019 21:10

jlebon approved these changes Jun 14, 2019

View reviewed changes

lucab approved these changes Jun 17, 2019

View reviewed changes

lucab merged commit c9dc73a into coreos:master Jun 17, 2019

This was referenced Jun 17, 2019

WIP: Initial import and azure hostname module yuqi-zhang/afterburn-dracut#1

Closed

Cut a new release #229

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dracut: add afterburn dracut module #224

dracut: add afterburn dracut module #224

yuqi-zhang commented Jun 6, 2019

yuqi-zhang commented Jun 6, 2019

ashcrow left a comment

jlebon commented Jun 10, 2019

lucab commented Jun 11, 2019

yuqi-zhang commented Jun 11, 2019

jlebon left a comment

lucab Jun 12, 2019

jlebon Jun 12, 2019

yuqi-zhang Jun 12, 2019

lucab Jun 12, 2019

jlebon Jun 12, 2019

yuqi-zhang Jun 12, 2019

yuqi-zhang Jun 12, 2019

jlebon Jun 12, 2019

ajeddeloh Jun 12, 2019

yuqi-zhang Jun 12, 2019

jlebon Jun 12, 2019

ajeddeloh Jun 12, 2019

yuqi-zhang commented Jun 13, 2019 •

edited

Loading

jlebon left a comment

jlebon Jun 13, 2019

yuqi-zhang Jun 13, 2019

yuqi-zhang commented Jun 14, 2019

jlebon left a comment

dracut: add afterburn dracut module #224

dracut: add afterburn dracut module #224

Conversation

yuqi-zhang commented Jun 6, 2019

yuqi-zhang commented Jun 6, 2019

ashcrow left a comment

Choose a reason for hiding this comment

jlebon commented Jun 10, 2019

lucab commented Jun 11, 2019

yuqi-zhang commented Jun 11, 2019

jlebon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuqi-zhang commented Jun 13, 2019 • edited Loading

jlebon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuqi-zhang commented Jun 14, 2019

jlebon left a comment

Choose a reason for hiding this comment

yuqi-zhang commented Jun 13, 2019 •

edited

Loading