overlay: add 15copy-installer-network dracut module #346

dustymabe · 2020-04-15T03:27:15Z

This dracut module delivers a coreos-copy-installer-network
systemd service and script that will detect when files have
been placed into /boot/ by coreos-installer install --copy-network
and appropriately copy them in place to be used by the initramfs
networking. If files are detected within /boot/coreos-installer-network/
then they will be considered to be the only source of networking
for that ignition boot (i.e. no networking kargs will be applied).

dustymabe · 2020-04-15T03:29:47Z

In order to propagate networking from the live ISO installer environment the installer will place files into a directory in /boot/ (coreos/coreos-installer#212) and the systemd service/script from this PR will pick that up and apply it during the initramfs boot process on the ignition boot (firstboot) of the installed system.

jlebon

It still feels wrong to me that this mechanism takes precedence over karg network options the user might've input. Though I guess we can fix that once we do coreos/coreos-assembler#1298 as discussed in coreos/fedora-coreos-tracker#460.

jlebon · 2020-04-15T19:19:29Z

....d/05core/usr/lib/dracut/modules.d/15copy-installer-network/coreos-copy-installer-network.sh

+    # location for NetworkManager to use the configuration.
+    echo "info: copying files from ${installer_network_dir} to ${initramfs_network_dir}"
+    mkdir -p $initramfs_network_dir
+    cp -v ${installer_network_dir}/* ${initramfs_network_dir}/


I think we also want to delete installer_network_dir here, no? Otherwise, it'll just hang around forever.

I'd rather keep /boot/ mounted read-only here. I don't think it hurts to keep the directory around especially since nothing after firstboot will consume it anyway.

Oh right, forgot about the ro mount.

I don't think it hurts to keep the directory around especially since nothing after firstboot will consume it anyway.

Right yeah, not so much that it conflicts with anything. It's just bad optics IMO to have it stay in the root of /boot forever. For example, back when we used Anaconda, we would actively remove everything in /sysroot except what was needed (coreos/coreos-assembler#492). Nowadays we just have /boot, /ostree, and .coreos-aleph-version.json, which is nice. Similarly an ls /boot right now only has the files we actually need.

WDYT about adding a /run/tmpfiles.d/10-coreos-installer-network.conf file which has:

R /boot/coreos-installer-network - - - - -

?

ok added a second commit to clean up via tmpfiles.d. I can squash it if desired.

....d/05core/usr/lib/dracut/modules.d/15copy-installer-network/coreos-copy-installer-network.sh

...core/usr/lib/dracut/modules.d/15copy-installer-network/coreos-copy-installer-network.service

dustymabe · 2020-04-15T19:53:44Z

It still feels wrong to me that this mechanism takes precedence over karg network options the user might've input.

I don't really feel like it's that wrong considering the user just did an install and explicitly told the installer to bake specific networking configuration in specifically for this first boot. We do have to do it this way for now (ignoring kargs completely) because we do embed default networking args, but as you mention below 👇🏼 we maybe be able to rework it when we no longer do that.

Though I guess we can fix that once we do coreos/coreos-assembler#1298 as discussed in coreos/fedora-coreos-tracker#460.

...core/usr/lib/dracut/modules.d/15copy-installer-network/coreos-copy-installer-network.service

bgilbert · 2020-04-15T20:30:07Z

...core/usr/lib/dracut/modules.d/15copy-installer-network/coreos-copy-installer-network.service

+# [4] https://github.com/dracutdevs/dracut/blob/master/modules.d/35network-manager/module-setup.sh#L36
+#
+[Unit]
+Description=Copy Live ISO Installer Networking Config


Live ISO is a misnomer here. --copy-network can be done from live PXE, or in principle from a Fedora live image running the coreos-installer container.

Technically you are right. How about Copy CoreOS Installer Networking Config ?

We could also keep it generic as just a mechanism for baking in network configs in the image and coreos-installer just happens to use it. So e.g. it'd be Copy Network Configs and the directory would be ${bootmnt}/coreos-network-configs or something.

I'm okay with either of those options.

(This is similar to how we support adding a $boot/ignition/config.ign without the help of coreos-installer, and in fact kola knows how to do this today.)

ok if we want to change the name of the directory now is the time to do it. I named it something specific because I didn't see other uses for this (or at least not other encouraged uses). What do you all think we should do?

bikeshed: /boot/firstboot-network-configs

we decided in IRC to go with: /boot/coreos-firstboot-network

coreos-installer PR to change the dir: coreos/coreos-installer#216

updated this PR to use /boot/coreos-firstboot-network

In another code review [1] we agreed to make the name of the directory more generic. [1] coreos/fedora-coreos-config#346 (comment)

dustymabe · 2020-04-16T16:50:29Z

bringing this out of draft mode. testing looks good (combined with coreos/coreos-installer#216)

jlebon

One comment, LGTM otherwise.

jlebon · 2020-04-16T17:05:21Z

...re/usr/lib/dracut/modules.d/15coreos-copy-firstboot-network/coreos-copy-firstboot-network.sh

+    echo "R ${realroot_firstboot_network_dir} - - - - -" > \
+        /run/tmpfiles.d/15-coreos-firstboot-network.conf


Can we move this out so we always do this if the directory exists, not just if it's not empty?

I'd prefer not. Deleting the files (presumably on success) will already make things hard to debug. This would make it worse I think.

dustymabe · 2020-04-16T20:08:22Z

--- FAIL: ostree.remote (337.27s)

    --- PASS: ostree.remote/add (0.26s)

    --- PASS: ostree.remote/list (0.11s)

    --- PASS: ostree.remote/show-url (0.10s)

    --- PASS: ostree.remote/refs (1.01s)

    --- PASS: ostree.remote/summary (1.83s)

    --- PASS: ostree.remote/delete (0.41s)

        harness.go:731: Found emergency shell on machine 894d3b2a-eaff-4256-93bf-3d442af4d1fe console

This test passes for me when I run it locally so maybe let's retry.

dustymabe · 2020-04-16T20:08:30Z

/retest

cgwalters · 2020-04-16T20:56:54Z

...r/lib/dracut/modules.d/15coreos-copy-firstboot-network/coreos-copy-firstboot-network.service

+#             - i.e. Before=dracut-cmdline.service
+#         - Another is to run *after* nm-config.sh [3] in dracut-cmdline [4]
+#           and just delete all the files created by nm-initrd-generator.
+#             - i.e. After=dracut-cmdline.service, but Before=dracut-initqueue.service


This is a clear and obvious downside of the "running NM via dracut initqueue"...

Agree. Though when we do finally get NM running via systemd we'll need to make sure we work with the team such that there are two separate services: one for running nm-initrd-generator and one for starting NM. This will enable us to hook in between them OR maybe be able to just say Before=nm-initrd-generator.service

dustymabe · 2020-04-16T20:58:58Z

The CI failure This appears to be a race condition. The After=...*boot.device in the systemd unit should be enough, but apparently not (kernel race, fun!):

  [    4.045181] systemd[1]: Found device /dev/disk/by-label/boot.                                                                     
  [  OK  ] Found device /dev/disk/by-label/boot                                                                                        
  [    4.051500] systemd[1]: Starting Copy CoreOS Firstboot Networking Config...                                                       
        Starting  Copy CoreOS Firstboot Networking Config                                                                              
  [    4.060573]  vda: vda1 vda2 vda3 vda4                                                                                             
  [    4.063296] coreos-copy-firstboot-network[479]: mount: /mnt/boot_partition: special device /dev/disk/by-label/boot does not exist.

will add a short loop to wait for the device before continuing

dustymabe · 2020-04-16T21:41:19Z

yay! CI passed now

bgilbert

LGTM

...r/lib/dracut/modules.d/15coreos-copy-firstboot-network/coreos-copy-firstboot-network.service

This dracut module delivers a coreos-copy-firstboot-network systemd service and script that will detect when files have been placed into /boot/ by `coreos-installer install --copy-network` and appropriately copy them in place to be used by the initramfs networking. If files are detected within /boot/coreos-firstboot-network/ then they will be considered to be the only source of networking for that ignition boot (i.e. no networking kargs will be applied).

…t-network This will clean up those files if they existed and were used.

dustymabe · 2020-04-17T14:03:45Z

CI re-ran again because I pushed up a comment change for #346 (comment) and it failed. It appears the same fallacy that plagues After=dev-disk-by\x2dlabel-boot.device also plagues [ -e /dev/disk/by-label/boot ] so I switched the loop to just retry the actual mount command instead.

dustymabe · 2020-04-17T14:05:34Z

if CI does pass I'll look at the logs and hopefully we'll see at least one info: retrying ${bootdev} mount in 1 second... message

Add short while loop to handle a race condition where After=dev-disk-by\x2dlabel-boot.device doesn't seem to be sufficient always.

In another code review [1] we agreed to make the name of the directory more generic. [1] coreos/fedora-coreos-config#346 (comment)

dustymabe · 2020-04-17T14:30:58Z

if CI does pass I'll look at the logs and hopefully we'll see at least one info: retrying ${bootdev} mount in 1 second... message

It did pass! and we do see it!

ostree.unlock/c0895ef6-5c96-417c-a6d7-171050a4c29a/journal.txt:Apr 17 14:21:24.260616 coreos-copy-firstboot-network[481]: info: retrying /dev/disk/by-label/boot mount in 1 second...
ostree.unlock/c0895ef6-5c96-417c-a6d7-171050a4c29a/journal.txt:Apr 17 14:21:25.269617 coreos-copy-firstboot-network[481]: info: retrying /dev/disk/by-label/boot mount in 1 second...
ostree.unlock/c0895ef6-5c96-417c-a6d7-171050a4c29a/console.txt:[    3.755254] coreos-copy-firstboot-network[481]: info: retrying /dev/disk/by-label/boot mount in 1 second...
ostree.unlock/c0895ef6-5c96-417c-a6d7-171050a4c29a/console.txt:[    4.764259] coreos-copy-firstboot-network[481]: info: retrying /dev/disk/by-label/boot mount in 1 second...

👏 👏 👏 👏

cgwalters · 2020-04-17T21:31:43Z

I think the races may be due to coreos-gpt-setup@dev-disk-by\x2dlabel-root.service. We need more ordering against that.

Make use of the new `fetch-offline` stage: coreos/ignition#979 We run this between the `setup` and `fetch` stages (the latter possibly being skipped if networking is not required). We hit the same issue here that `coreos-copy-firstboot-network.service` hit, which is that we can't run before the `cmdline` hook because that runs *before* udev, but we want the `by-*` symlinks for `ignition-setup-user.service`. The hack we do here is to rerun the NM cmdline hook in case ignition dropped a snippet in `/etc/cmdline.d`. As mentioned in coreos/fedora-coreos-config#346, we'll be able to do this more cleanly once we run NM as a systemd service directly.

cgwalters · 2020-06-05T20:19:44Z

Still racy, this PR failed: https://jenkins-coreos-ci.apps.ci.centos.org/blue/organizations/jenkins/github-ci%2Fostreedev%2Fostree/detail/PR-2121/3/pipeline/154

cgwalters · 2020-06-05T20:22:54Z

Filed as coreos/fedora-coreos-tracker#523

Make use of the new `fetch-offline` stage: coreos/ignition#979 We run this between the `setup` and `fetch` stages (the latter possibly being skipped if networking is not required). We hit the same issue here that `coreos-copy-firstboot-network.service` hit, which is that we can't run before the `cmdline` hook because that runs *before* udev, but we want the `by-*` symlinks for `ignition-setup-user.service`. The hack we do here is to rerun the NM cmdline hook in case ignition dropped a snippet in `/etc/cmdline.d`. As mentioned in coreos/fedora-coreos-config#346, we'll be able to do this more cleanly once we run NM as a systemd service directly.

dustymabe mentioned this pull request Apr 15, 2020

install: add --copy-network and --network-dir options coreos/coreos-installer#212

Merged

dustymabe force-pushed the dusty-copy-installer-network branch from 99904e0 to 6016e33 Compare April 15, 2020 19:20

jlebon reviewed Apr 15, 2020

View reviewed changes

dustymabe force-pushed the dusty-copy-installer-network branch from 6016e33 to a873d2b Compare April 15, 2020 19:55

bgilbert reviewed Apr 15, 2020

View reviewed changes

dustymabe added a commit to dustymabe/coreos-installer that referenced this pull request Apr 16, 2020

install: change /boot/ directory for --copy-network

053e4f4

In another code review [1] we agreed to make the name of the directory more generic. [1] coreos/fedora-coreos-config#346 (comment)

dustymabe mentioned this pull request Apr 16, 2020

install: change /boot/ directory for --copy-network coreos/coreos-installer#216

Merged

dustymabe force-pushed the dusty-copy-installer-network branch from a873d2b to 2fe49ad Compare April 16, 2020 16:48

dustymabe marked this pull request as ready for review April 16, 2020 16:49

jlebon approved these changes Apr 16, 2020

View reviewed changes

dustymabe force-pushed the dusty-copy-installer-network branch from 2fe49ad to dffc863 Compare April 16, 2020 20:09

cgwalters reviewed Apr 16, 2020

View reviewed changes

dustymabe force-pushed the dusty-copy-installer-network branch 2 times, most recently from add674d to 03390ec Compare April 16, 2020 21:17

jlebon mentioned this pull request Apr 16, 2020

01bootetc: introduce module for updating dracut etc dracutdevs/dracut#792

Closed

bgilbert approved these changes Apr 17, 2020

View reviewed changes

...r/lib/dracut/modules.d/15coreos-copy-firstboot-network/coreos-copy-firstboot-network.service Outdated Show resolved Hide resolved

dustymabe added 2 commits April 17, 2020 08:16

overlay: adding tmpfiles.d entry for removal of /boot/coreos-firstboo…

f9afb84

…t-network This will clean up those files if they existed and were used.

dustymabe force-pushed the dusty-copy-installer-network branch 2 times, most recently from 8d81ffa to 1d881ac Compare April 17, 2020 14:00

overlay: 15coreos-copy-firstboot-network: handle race condition

ba00a21

Add short while loop to handle a race condition where After=dev-disk-by\x2dlabel-boot.device doesn't seem to be sufficient always.

dustymabe force-pushed the dusty-copy-installer-network branch from 1d881ac to ba00a21 Compare April 17, 2020 14:09

dustymabe added a commit to dustymabe/coreos-installer that referenced this pull request Apr 17, 2020

install: change /boot/ directory for --copy-network

008866f

In another code review [1] we agreed to make the name of the directory more generic. [1] coreos/fedora-coreos-config#346 (comment)

jlebon approved these changes Apr 17, 2020

View reviewed changes

dustymabe merged commit 49de0c7 into coreos:testing-devel Apr 17, 2020

dustymabe deleted the dusty-copy-installer-network branch April 17, 2020 14:51

jlebon mentioned this pull request May 25, 2020

Add new ignition-fetch-offline.service coreos/ignition-dracut#164

Merged

cgwalters mentioned this pull request Jun 5, 2020

races in boot.mount coreos/fedora-coreos-tracker#523

Closed

cgwalters mentioned this pull request Jun 16, 2020

Disabling IPv6 coreos/fedora-coreos-tracker#522

Open

jlebon mentioned this pull request Jun 17, 2020

Support disabling initramfs networking via Ignition coreos/ignition#979

Closed

cgwalters mentioned this pull request Jul 1, 2020

40ignition-ostree: add coreos-inject-rootmap.service #503

Merged

hardys mentioned this pull request Jul 3, 2020

Discuss host network configuration interfaces openshift/enhancements#394

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

overlay: add 15copy-installer-network dracut module #346

overlay: add 15copy-installer-network dracut module #346

dustymabe commented Apr 15, 2020 •

edited

Loading

dustymabe commented Apr 15, 2020

jlebon left a comment

jlebon Apr 15, 2020

dustymabe Apr 15, 2020

jlebon Apr 15, 2020 •

edited

Loading

dustymabe Apr 16, 2020

dustymabe commented Apr 15, 2020

bgilbert Apr 15, 2020

dustymabe Apr 15, 2020

jlebon Apr 15, 2020 •

edited

Loading

bgilbert Apr 15, 2020

jlebon Apr 15, 2020

dustymabe Apr 16, 2020

dustymabe Apr 16, 2020

dustymabe Apr 16, 2020

dustymabe Apr 16, 2020

dustymabe Apr 16, 2020

dustymabe commented Apr 16, 2020

jlebon left a comment

jlebon Apr 16, 2020

dustymabe Apr 16, 2020

dustymabe commented Apr 16, 2020

dustymabe commented Apr 16, 2020

cgwalters Apr 16, 2020

dustymabe Apr 16, 2020

dustymabe commented Apr 16, 2020

dustymabe commented Apr 16, 2020

bgilbert left a comment

dustymabe commented Apr 17, 2020

dustymabe commented Apr 17, 2020

dustymabe commented Apr 17, 2020

cgwalters commented Apr 17, 2020

cgwalters commented Jun 5, 2020

cgwalters commented Jun 5, 2020

		echo "R ${realroot_firstboot_network_dir} - - - - -" > \
		/run/tmpfiles.d/15-coreos-firstboot-network.conf

overlay: add 15copy-installer-network dracut module #346

overlay: add 15copy-installer-network dracut module #346

Conversation

dustymabe commented Apr 15, 2020 • edited Loading

dustymabe commented Apr 15, 2020

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

jlebon Apr 15, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dustymabe commented Apr 15, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlebon Apr 15, 2020 • edited Loading

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

dustymabe commented Apr 16, 2020

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

dustymabe commented Apr 16, 2020

dustymabe commented Apr 16, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dustymabe commented Apr 16, 2020

dustymabe commented Apr 16, 2020

bgilbert left a comment

Choose a reason for hiding this comment

dustymabe commented Apr 17, 2020

dustymabe commented Apr 17, 2020

dustymabe commented Apr 17, 2020

cgwalters commented Apr 17, 2020

cgwalters commented Jun 5, 2020

cgwalters commented Jun 5, 2020

dustymabe commented Apr 15, 2020 •

edited

Loading

jlebon Apr 15, 2020 •

edited

Loading

jlebon Apr 15, 2020 •

edited

Loading