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

Support setting kernel args for the installed OS image #533

Closed
abhinavdahiya opened this issue Mar 14, 2019 · 31 comments · Fixed by coreos/coreos-installer#268
Closed

Support setting kernel args for the installed OS image #533

abhinavdahiya opened this issue Mar 14, 2019 · 31 comments · Fixed by coreos/coreos-installer#268

Comments

@abhinavdahiya
Copy link

When PXE booting FCOS/RHCOS the coreos-installer provides options to install OS image and the Ignition config that should be used to configure on the reboot.

There are cases where users might want to provide kernel args for the OS image that is installed by coreos-installer.

An example is:

the FCOS OS image by default has console=ttyS0,115200n8 as the kernel parameter. But when you try to PXE boot FCOS in Packet.net console=ttyS1,115200n8 is required to get output on the SOS console

also an Ignition config that needs to fetch extra configuration from network, rd.neednet=1 is required so that Ignition has DHCP/network available in initramfs.

@bgilbert
Copy link
Contributor

Neither of those examples are completely on point: the kernel options for PXE-booted systems are controlled by the PXE server; the Packet platform support will include the correct console settings; and the rd.neednet requirement is a bug.

As much as I don't like the idea of users overriding kernel parameters, though, I think this is a good point. @ajeddeloh, WDYT?

@ajeddeloh
Copy link
Contributor

I can see wanting to set the default kernel params for an installed to disk system on BM at install time. For cloud/PXE it doesn't make much sense. That being said the implementation for diskfull BM is hairy:

Currently, with anaconda created images there's not a good way to do this without booting the system.
With BLS images (working on this) there's also no easy way to do this (could edit the BLS fragments themselves but then it'll get lost on upgrade)
Eventually we could have a grub var like $kernel_extra_args or something read from a snippet in /boot but that's a ways off.

@cgwalters was talking at some point about ostree owning the kcmdline and having it be part of the commit, but I'd argue the user should be able to augment it without needing to create new ostree commits. (I agree the defaults ought to be part of the commit).

@dustymabe
Copy link
Member

I can see wanting to set the default kernel params for an installed to disk system on BM at install time.

yep. I asked @abhinavdahiya to open this issue after chatting with him. I thought it was a reasonable feature request.

@bgilbert
Copy link
Contributor

One use case for kargs: configuring SMT for mitigating L1TF and MDS. That does apply more to bare metal than VMs, but bare-metal clouds that provide preinstalled images (e.g. Packet) will need it. Thus, doing it from the installer isn't really enough; it should be possible to set kargs from Ignition.

@cgwalters
Copy link
Member

I actually argued for kargs in Ignition but Andrew talked me out of it.

openshift/machine-config-operator#388 is the MCO issue here, and has an open PR now. For RHCOS we are likely to just rely on the fact that we generally always need to do an initial pivot (i.e. upgrade and reboot) and use that as the opportunity to change kernel args.

I guess an interesting question here is whether Ignition should also support "upgrade and reboot" - I would be shocked if this hadn't come up in the CL context. If Ignition supports that then it also argues for kargs?

@arithx
Copy link
Contributor

arithx commented May 16, 2019

I'd currently lean towards having a service in the initramfs with a conditional requirement that if met performs a reboot. With that in place we could implement a way of writing kargs via Ignition without adding specific logic to Ignition itself and use sugar in the distro implementation of CT to simplify the process for users.

@cgwalters
Copy link
Member

One thing to remember if we pursue this is to move ignition-firstboot-complete.service from the real rootfs into Ignition itself so we don't run Ignition twice.

@ajeddeloh
Copy link
Contributor

My current thinking is write a flag file to like /tmp/needs-reboot or something similar to trigger it. Would be a behavior of the initramfs not Ignition.

@bgilbert
Copy link
Contributor

@cgwalters I'm not aware of Ignition upgrade+reboot ever being discussed for CL. #34 is relevant though.

@ajeddeloh I was hoping that the unit would detect the actual file written by Ignition that requires the reboot (/etc/kcmdline or something). Requiring the config to write out a separate flag file seems awkward.

@bgilbert
Copy link
Contributor

Filed https://github.com/coreos/ignition-dracut/issues/81 for rebooting from the initramfs.

@cgwalters
Copy link
Member

I realized in openshift/machine-config-operator#762 that it'd really be a lot better if we handed kargs in a way integrated with Ignition, since otherwise we need to carry "MCD parts" in RHCOS that runs very early.

@cgwalters
Copy link
Member

@cgwalters I'm not aware of Ignition upgrade+reboot ever being discussed for CL. #34 is relevant though.

For RHCOS to share this mechanism fully we also need upgrades. Now...I am just hesitating a lot on pushing the whole stack into the initramfs versus the alternative of booting to a special coreos-firstboot.target or so, and running whatever we need there from the real root, before we start other services. This would mean that any 3rd parties dropping units that are ConditionFirstBoot= would have to explicitly hook them in there.

@bgilbert
Copy link
Contributor

Now...I am just hesitating a lot on pushing the whole stack into the initramfs versus the alternative of booting to a special coreos-firstboot.target or so, and running whatever we need there from the real root, before we start other services.

@cgwalters Could you expand on that? (Maybe in #34.) To me the initramfs seems strictly better, for the same reason that Ignition runs there.

@cgwalters
Copy link
Member

With the new "unified" metal/BIOS no-grub metal disk images in FCOS (but not RHCOS), the canonical source of the kernel arguments is in /boot/loader. That makes having coreos-installer change them rather easy.

@dustymabe
Copy link
Member

agree.. some related discussion: #191 (comment)

@jlebon
Copy link
Member

jlebon commented Apr 22, 2020

So... now that we have the new coreos-installer, one could just mount /boot post-install (and pre-reboot) and change the BLS configs. Though... the presence of --firstboot-args is kinda asking for a similar --persistent-args facilitator.

@jlebon
Copy link
Member

jlebon commented Apr 22, 2020

I guess this gets back to https://github.com/coreos/ignition-dracut/issues/81 though; if we have kargs handling built-in, then we should just recommend that so it's the same workflow across cloud and bare metal.

@jlebon
Copy link
Member

jlebon commented Apr 22, 2020

There's some overlap with the auto-forward magic we do with the network kargs too though. For example, if console= kargs were passed in for the install boot, it's very likely that users want those kargs to stick around too. (The difference though with the network kargs is that we want it on every boot, not just on the first boot). Tempting to auto-forward that too, though need to balance the UX with keeping magic at a minimum.

@brokenjacobs
Copy link

brokenjacobs commented Jun 4, 2020

Just to add to this, we have the exact same issue installing on bare metal. IPMI on our supermicro bare metal platform requires console=ttyS1,115200n8 not console=ttyS0,115200n8. And using the append option at install time does not work on the first reboot. So we are in the dark until we are able to use rpm-ostree kargs --replace... in a systemd unit on the first reboot. Not ideal.

Creating an additional 'platform' for a very common bare metal use case seems like a lot of work just to have control over basic kernel arguments.

If the 'metal' platform isn't going to set the serial console args properly for a common bare metal use case (IPMI serial console), perhaps it shouldn't set a serial console option at all, and let it be set at install time by the installer if necessary?

@jlebon
Copy link
Member

jlebon commented Jun 10, 2020

Some more thoughts on this. I think we're all agreed that a much bigger/nicer win is karg management through the Ignition config (let's call this "kargs-via-Ignition").

That said, I think even with kargs-via-Ignition, there is value in having coreos-installer install learn a --persistent-args switch:

  1. It's very likely that any implementation of kargs-via-Ignition will require an early reboot. On some bare metal systems, reboots are expensive and slow. It's silly and frustrating to sysadmins to forgo the shortcut of setting the kargs at install time.
  2. Debugging issues with the kargs-via-Ignition path itself will require being able to access the console output. If the right console argument is not there from the start, then there's a gap in the install --> reboot --> reboot with kargs process where sysadmins are simply in the dark.

Given these, I think we should support setting kargs at install time more easily than having to manually mount the boot partition and fiddling with the BLS entries.

Additionally, when we do implement kargs-via-Ignition, there's no reason coreos-installer install --persistent-kargs can't leverage this so that the kargs are properly registered as "user-provided". The initrd code would then optimize for this case and not reboot, while still having those kargs managed via /etc or whatever.

@jlebon jlebon transferred this issue from coreos/coreos-installer Jun 10, 2020
@jlebon
Copy link
Member

jlebon commented Jun 10, 2020

Moved this to the tracker since I think this is a cross-cutting issue.

@jlebon
Copy link
Member

jlebon commented Jun 11, 2020

OK, opened coreos/coreos-installer#268. With this one can do e.g.

$ coreos-installer install --persistent-args 'console=ttyS1,115200n8' /dev/sda

(There is still the question of whether we want to auto-forward the console argument so users don't have to specify it. It's really tempting...)

@brokenjacobs
Copy link

that will only work if the console=ttyS0,115200n8 arg is removed from the default. Otherwise you get no console.

Another point I would like to bring up here is that sometimes kernel arguments are needed on hardware platforms to get them to boot at all. Back on CoreOS we had a raid card that would only work if we disabled the intel iommu with a kernel argument, otherwise you couldn't install or boot the system. Not being able to set the arguments from the moment the kernel loads after install (at least optionally) will break some hardware platforms.

For my use case, being able to completely specify the kernel arguments in the ignition would work just fine, so long as the console=ttyS0,115200n8 is removed from the default arguments.

@dustymabe
Copy link
Member

that will only work if the console=ttyS0,115200n8 arg is removed from the default. Otherwise you get no console.

The last entry will be the primary console, right? My understanding is that if you had console=tty0 console=ttyS0,115200n8 console=ttyS1,115200n8 all on the command line the primary console would be the last entry (console=ttyS1,115200n8).

Another point I would like to bring up here is that sometimes kernel arguments are needed on hardware platforms to get them to boot at all. Back on CoreOS we had a raid card that would only work if we disabled the intel iommu with a kernel argument, otherwise you couldn't install or boot the system. Not being able to set the arguments from the moment the kernel loads after install (at least optionally) will break some hardware platforms.

I think @jlebon addresses this in #533 (comment) and it's why he opened coreos/coreos-installer#268 so that users can add kargs persistently via the installer. Does that not address this need?

For my use case, being able to completely specify the kernel arguments in the ignition would work just fine, so long as the console=ttyS0,115200n8 is removed from the default arguments.

Yeah I think we would definitely need some way to "remove" existing arguments with "kargs-via-Ignition". It might even be beneficial to add the ability to remove args via the coreos-installer install --persistent-kargs approach as well. Maybe starting a karg with - could mean we want it removed, though that may make parsing the CLI options for coreos-installer harder.

@brokenjacobs
Copy link

brokenjacobs commented Jun 12, 2020

In our testing the first argument becomes the serial console, not the last. Which means no console on the first reboot, but it comes back after manipulating args with rpm-ostree and rebooting.

@dustymabe
Copy link
Member

In our testing the first argument becomes the serial console, not the last.

May I ask what you are testing? Are you testing the un-merged code in coreos/coreos-installer#268 ?

@brokenjacobs
Copy link

No. We are using coreos stable appending kernel args with the installer. The kernel ends up with two serial console arguments

@jlebon
Copy link
Member

jlebon commented Jun 15, 2020

In our testing the first argument becomes the serial console, not the last.

Ahh yup, good catch. I think we were under the assumption that the last console= wins. But it's a little more nuanced than that. Reading from the docs and testing, between console= arguments of different types, the last one wins for who gets control of /dev/console, but within the set of console arguments for the same type, the first one wins (not just for control of /dev/console, but any output at all).

For my use case, being able to completely specify the kernel arguments in the ignition would work just fine, so long as the console=ttyS0,115200n8 is removed from the default arguments.

Yeah I think we would definitely need some way to "remove" existing arguments with "kargs-via-Ignition". It might even be beneficial to add the ability to remove args via the coreos-installer install --persistent-kargs approach as well.

Yeah... I wanted to avoid supporting this until the need came up (and ideally just leave the more complex operations to the kargs-via-Ignition path). --persistent-args as implemented in coreos/coreos-installer#268 also is nicely symmetrical with --firstboot-args.

But given that setting console args is one of the primary arguments for doing this, I'll change coreos/coreos-installer#268 to support this. (And in the end it also helps with point (1) of #533 (comment)).

@dustymabe dustymabe added the status/pending-upstream-release Fixed upstream. Waiting on an upstream component source code release. label Jun 26, 2020
@dustymabe
Copy link
Member

This made it into coreos-installer v0.3.0.

@dustymabe dustymabe added status/pending-testing-release Fixed upstream. Waiting on a testing release. and removed status/pending-upstream-release Fixed upstream. Waiting on an upstream component source code release. labels Jul 17, 2020
@dustymabe
Copy link
Member

The fix for this went into testing stream release 32.20200726.2.0. Please try out the new release and report issues.

@dustymabe dustymabe added status/pending-stable-release Fixed upstream and in testing. Waiting on stable release. and removed status/pending-testing-release Fixed upstream. Waiting on a testing release. labels Jul 29, 2020
@dustymabe
Copy link
Member

The fix for this went into stable stream release 32.20200726.3.1.

@dustymabe dustymabe removed the status/pending-stable-release Fixed upstream and in testing. Waiting on stable release. label Aug 12, 2020
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 a pull request may close this issue.

8 participants