grub: de-hardcode firstboot network configuration #1373

lucab · 2020-04-20T13:36:33Z

This removes the hardcoded network configuration kargs from grub,
moving them into handling within initramfs by dracut-cmdline.service.

Ref: coreos/fedora-coreos-tracker#460
Ref: coreos/fedora-coreos-config#427

lucab · 2020-04-20T13:36:52Z

/hold

jlebon · 2020-04-20T21:10:25Z

See #1298 for the other spots we need to tweak too.

Worth noting in the commit message also that the eventual goal is to remove rd.neednet=1 as well as we move towards on-request network enablement.

dustymabe · 2020-04-21T13:48:04Z

we need to be real careful with this because it will change it for all streams. We need to make sure when we merge this and do our first testing release that we also tag coreos-assembler so we can use the same one for the subsequent stable release (this is related to https://github.com/coreos/fedora-coreos-streams/issues/64)

jlebon · 2020-04-21T14:38:25Z

we need to be real careful with this because it will change it for all streams

Yeah, good point. Tagging cosa would work, though another approach is to just add a new image.yaml knob which cosa keys off of. That way it naturally gets promoted together with content. I think we should do that anyway so that it's easier to ratchet all of this into FCOS/RHCOS atomically.

darkmuggle · 2020-05-20T15:03:59Z

There is a potential way to deal with N streams: embed the kargs into the ignition.firstboot and have grub read them in and then set the defaults on a per distro/stream. The advantage here is decoupling the builder from the configuration. I also think we could take it further, and just have the grub.cfg provided by the stream itself and COSA place it.

That said, I don't have a strong opinion either way. The change seems sensible to me.

ashcrow

I'm OK with this move but let's be careful to ensure all required updates make it into RHCOS as well here (IE: updated afterburn and the referenced service).

src/cmd-buildextend-metal

This is part of moving to conditional networking (coreos/fedora-coreos-tracker#443). Let's move the firstboot kargs here as prep for dropping them entirely. See also: coreos/coreos-assembler#1373

jlebon

Some comments, but LGTM overall! I've tested this as part of coreos/fedora-coreos-config#426.

jlebon · 2020-05-25T19:16:09Z

src/grub.cfg


    # source in the `ignition.firstboot` file which could override the
    # above $ignition_network_kcmdline with static networking config.
-    # This override feature is primarily used by coreos-installer to
-    # persist static networking config provided during install to the
-    # first boot of the machine.


Hmm, I think this comment is still true, no?

jlebon · 2020-05-25T19:17:53Z

src/cmd-buildextend-metal

@@ -143,6 +143,17 @@ if [ "${image_type}" = dasd ] || [ "${image_type}" = metal4k ]; then
    ignition_platform_id=metal
 fi

+# Firstboot kernel arguments.
+firstboot_kargs="$(python3 -c '
+import sys, yaml;


Minor/optional: I think we can drop the semicolons here and below since this is a proper multiline string now?

jlebon · 2020-05-25T19:26:31Z

src/cmd-buildextend-metal

+import sys, yaml;
+LEGACY_DEFAULT = [ "rd.neednet=1", "ip=dhcp,dhcp6" ];
+args = yaml.safe_load(sys.stdin).get("firstboot-kargs", LEGACY_DEFAULT);
+if not args:


One subtlety here that I missed at first (which contributed to why I originally wanted something more explicit than this) is that it appears like this can never be false since we provide a default. Maybe a comment here like: # firstboot-kargs provided, but is empty ?

(I blame PEP8 for convincing a generation of programmers that a list and a boolean are isomorphic)

This removes the hardcoded network configuration kargs from grub, moving them into handling within initramfs by `dracut-cmdline.service`.

lucab · 2020-05-26T09:13:33Z

Rebased. There was actually a bug in the logic writing /boot/ignition.firstboot. I also adjusted the parameter name to match what GRUB and coreos-installer use.
coreos/fedora-coreos-config#427 shows how to configure this logic.

This PR can land as soon review-stamped, as it internally preserve the previously-GRUB-hardcoded defaults.

I manually tested that this works with the logic in Afterburn 4.4.0 in F32.

lucab · 2020-05-26T12:23:00Z

/hold cancel

ashcrow · 2020-05-26T13:27:14Z

This PR can land as soon review-stamped, as it internally preserve the previously-GRUB-hardcoded defaults.

Perfect 👍

/lgtm

openshift-ci-robot · 2020-05-26T13:27:17Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ashcrow, jlebon, lucab

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [ashcrow,jlebon,lucab]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

jlebon · 2020-05-26T13:32:29Z

src/create_disk.sh

@@ -392,7 +395,7 @@ s390x)
 	# stage on s390x, either through zipl->grub2-emu or zipl standalone.
 	# See https://github.com/coreos/ignition-dracut/issues/84
 	# A similar hack is present in https://github.com/coreos/coreos-assembler/blob/master/src/gf-platformid#L55
-	echo "$(grep options $blsfile) ignition.firstboot rd.neednet=1 ip=dhcp,dhcp6" > $tmpfile


Hmm actually, I think we need to add ${firstboot_kargs} here, right? Because s390x doesn't use the GRUB config yet. And in gf_platformid too.

Or... we just sprint to coreos/fedora-coreos-config#426.

This will also affect RHCOS and slower FCOS streams, so let's fix it.

I'm unsure however how to wire this new variable content to gf_platformid. Do we actually need to have it there, if that is only injecting the platform ID?

Meh, my current attempt is at #1479.

We've now reached the end of the conditional networking 🌈 rainbow 🌈. We can rip out the legacy default network firstboot kargs from cosa. These now live in the config repo. E.g.: https://github.com/coreos/fedora-coreos-config/blob/be456c437d4181435022ce47079b587f5bcb0319/overlay.d/05core/usr/lib/dracut/modules.d/15coreos-network/50-afterburn-network-kargs-default.conf For more details, see: coreos#1373 coreos/fedora-coreos-config#426

We've now reached the end of the conditional networking 🌈 rainbow 🌈. We can rip out the legacy default network firstboot kargs from cosa. These now live in the config repo. E.g.: https://github.com/coreos/fedora-coreos-config/blob/be456c437d4181435022ce47079b587f5bcb0319/overlay.d/05core/usr/lib/dracut/modules.d/15coreos-network/50-afterburn-network-kargs-default.conf For more details, see: #1373 coreos/fedora-coreos-config#426

lucab added the do-not-merge/hold label Apr 20, 2020

openshift-ci-robot requested review from ashcrow and cgwalters April 20, 2020 13:36

openshift-ci-robot added the approved label Apr 20, 2020

lucab mentioned this pull request Apr 20, 2020

providers: setup network kernel arguments in initrd coreos/afterburn#390

Merged

jlebon mentioned this pull request Apr 20, 2020

grub: drop default rd.neednet=1 ip=dhcp,dhcp6 kargs #1298

Closed

lucab mentioned this pull request Apr 21, 2020

dracut/firstboot-network: add default network kargs coreos/fedora-coreos-config#362

Merged

cgwalters mentioned this pull request May 15, 2020

Ignition support osbuild/osbuild#371

Merged

ashcrow approved these changes May 20, 2020

View reviewed changes

lucab force-pushed the ups/grub-ipless branch from a4a0c0e to a8d334f Compare May 25, 2020 15:11

jlebon reviewed May 25, 2020

View reviewed changes

src/cmd-buildextend-metal Outdated Show resolved Hide resolved

lucab force-pushed the ups/grub-ipless branch from a8d334f to 0401cb5 Compare May 25, 2020 15:54

jlebon mentioned this pull request May 25, 2020

Move to conditional networking coreos/fedora-coreos-config#426

Merged

jlebon approved these changes May 25, 2020

View reviewed changes

grub: de-hardcode firstboot network configuration

14408ce

This removes the hardcoded network configuration kargs from grub, moving them into handling within initramfs by `dracut-cmdline.service`.

lucab force-pushed the ups/grub-ipless branch from 0401cb5 to 14408ce Compare May 26, 2020 09:07

lucab mentioned this pull request May 26, 2020

image: inject firstboot kargs (ignition-network-kcmdline) coreos/fedora-coreos-config#427

Merged

lucab removed the do-not-merge/hold label May 26, 2020

ashcrow approved these changes May 26, 2020

View reviewed changes

openshift-ci-robot assigned ashcrow May 26, 2020

openshift-ci-robot added the lgtm label May 26, 2020

openshift-merge-robot merged commit 5f2e11e into coreos:master May 26, 2020

jlebon reviewed May 26, 2020

View reviewed changes

lucab deleted the ups/grub-ipless branch May 26, 2020 13:46

lucab mentioned this pull request May 26, 2020

disk/s390x: inject firstboot kargs (ignition-network-kcmdline) #1479

Merged

jlebon mentioned this pull request Aug 21, 2020

Drop support for ignition-network-kcmdline #1678

Merged

jlebon mentioned this pull request Dec 3, 2020

Initramfs network configuration coreos/fedora-coreos-tracker#460

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

grub: de-hardcode firstboot network configuration #1373

grub: de-hardcode firstboot network configuration #1373

lucab commented Apr 20, 2020 •

edited

Loading

lucab commented Apr 20, 2020

jlebon commented Apr 20, 2020

dustymabe commented Apr 21, 2020

jlebon commented Apr 21, 2020

darkmuggle commented May 20, 2020

ashcrow left a comment

jlebon left a comment

jlebon May 25, 2020

jlebon May 25, 2020

jlebon May 25, 2020

lucab May 26, 2020 •

edited

Loading

lucab commented May 26, 2020 •

edited

Loading

lucab commented May 26, 2020

ashcrow commented May 26, 2020

openshift-ci-robot commented May 26, 2020

jlebon May 26, 2020

lucab May 26, 2020 •

edited

Loading

lucab May 26, 2020

grub: de-hardcode firstboot network configuration #1373

grub: de-hardcode firstboot network configuration #1373

Conversation

lucab commented Apr 20, 2020 • edited Loading

lucab commented Apr 20, 2020

jlebon commented Apr 20, 2020

dustymabe commented Apr 21, 2020

jlebon commented Apr 21, 2020

darkmuggle commented May 20, 2020

ashcrow left a comment

Choose a reason for hiding this comment

jlebon left a comment

Choose a reason for hiding this comment

jlebon May 25, 2020

Choose a reason for hiding this comment

jlebon May 25, 2020

Choose a reason for hiding this comment

jlebon May 25, 2020

Choose a reason for hiding this comment

lucab May 26, 2020 • edited Loading

Choose a reason for hiding this comment

lucab commented May 26, 2020 • edited Loading

lucab commented May 26, 2020

ashcrow commented May 26, 2020

openshift-ci-robot commented May 26, 2020

jlebon May 26, 2020

Choose a reason for hiding this comment

lucab May 26, 2020 • edited Loading

Choose a reason for hiding this comment

lucab May 26, 2020

Choose a reason for hiding this comment

lucab commented Apr 20, 2020 •

edited

Loading

lucab May 26, 2020 •

edited

Loading

lucab commented May 26, 2020 •

edited

Loading

lucab May 26, 2020 •

edited

Loading