-
Notifications
You must be signed in to change notification settings - Fork 26
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
Add support for installing static grub configs #543
Add support for installing static grub configs #543
Conversation
629918d
to
8b88e3f
Compare
OK this is now tested in concert with coreos/coreos-assembler#3631 |
src/grub2/grub-static-greenboot.cfg
Outdated
if [ -n "${boot_counter}" -a "${boot_success}" = "0" ]; then | ||
# if countdown has ended, choose to boot rollback deployment, | ||
# i.e. default=1 on OSTree-based systems. | ||
if [ "${boot_counter}" = "0" -o "${boot_counter}" = "-1" ]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why forcing 0 to be -1 when it can happen with decrement boot_counter
automatically , I have seen similar things here , couldn't understand the logic behind this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no idea either, I'm just copying it from the osbuild code.
But actually the more I think about this, the more it feels like the right move actually is to keep these grub drop-ins in ignition.rpm
and greenboot.rpm
respectively in say /usr/lib/bootupd/grub2-static
or so. WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The higher level direction here is we will switch Image Builder (Edge) to basically run bootupctl backend install
instead of owning the bootloader configs and grub installation on its own.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems fair.
May not be an appropriate question, still I am going to ask to clear my doubts: Though I dont see many user doing this , but package installed using using rpm installer will require additional bootupctl backend install
to be run again to update the bootloader configs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't directly support updating the configs yet.
(In practice of course, it will trivially work to just cp
them manually)
8b88e3f
to
f9ce09c
Compare
This pairs with coreos/bootupd#543 Basically we want bootupd to take care of installing this. Add the config here (where it more closely arguably belongs) so that it can be optionally installed. My proposal for at least the Fedora packaging of Ignition is that we unconditionally depend on bootupd and install this, because there's no one there using Ignition who doesn't want bootupd.
xref coreos/ignition#1728 |
This hard requires - coreos/bootupd#543 - coreos/ignition#1728
This hard requires - coreos/bootupd#543 - coreos/ignition#1728
This ensures that we'll use `/boot/efi` first, which is what is more commonly expected in our current ecosystem.
f9ce09c
to
c37f0d3
Compare
Not tested but LGTM. I would recommend forcing a number-prefixed format for the |
fcf0dab
to
45fe1ae
Compare
Good catch; I've added sorting. A bit less certain about actually forcing a numeric prefix; just saying they're sorted seems OK for now. |
Currently these are duplicated in osbuild and coreos-assembler. We will aim to deduplicate them here. Ideally we'd add support for "day 2" updates of these; I started on a patch for that but it's sadly messy. This is an incremental improvement. Signed-off-by: Colin Walters <[email protected]>
45fe1ae
to
1475f25
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apart from the one general question, LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
This pairs with coreos/bootupd#543 Basically we want bootupd to take care of installing this. Add the config here (where it more closely arguably belongs) so that it can be optionally installed. My proposal for at least the Fedora packaging of Ignition is that we unconditionally depend on bootupd and install this, because there's no one there using Ignition who doesn't want bootupd.
This pairs with coreos/bootupd#543 Basically we want bootupd to take care of installing this. Add the config here (where it more closely arguably belongs) so that it can be optionally installed. My proposal for at least the Fedora packaging of Ignition is that we unconditionally depend on bootupd and install this, because there's no one there using Ignition who doesn't want bootupd.
This pairs with coreos/bootupd#543 to have bootupd support injecting the greenboot grub logic only if greenboot is installed.
bootupd upstream was modified to store default static grub configs and allow for OS integrators to add their own snippets by placing files in /usr/lib/bootupd/grub2-static/configs.d/ [1] When we move to building with osbuild we will start taking advantage of this functionality and calling bootupd with --with-static-configs. Let's go ahead and add our snippets here so they can be leveraged when we start calling bootupd with --with-static-configs. The snippets added here come from the parts of the config currently defined in coreos-assembler that weren't lifted into bootupd [2][3][4]. [1] coreos/bootupd#543 [2] https://github.com/coreos/coreos-assembler/blob/4636b1a5c6dc00b1d6a58b1bfbb199431444336b/src/grub.cfg#L58-L60 [3] https://github.com/coreos/coreos-assembler/blob/4636b1a5c6dc00b1d6a58b1bfbb199431444336b/src/grub.cfg#L71-L85 [4] https://github.com/coreos/coreos-assembler/blob/4636b1a5c6dc00b1d6a58b1bfbb199431444336b/src/grub.cfg#L87-L91
Upstream we are starting to use grub configs that are partially baked into bootupd [1] and partially baked into the image via the OS vendor (in our case a grub2/50_coreos.cfg file [2]). Let's look first to see if grub2/50_coreos.cfg exists when trying to write new console settings before falling back to grub2/grub.cfg. [1] coreos/bootupd#543 [2] coreos/fedora-coreos-config#2769
Upstream we are starting to use grub configs that are partially baked into bootupd [1] and partially baked into the image via the OS vendor (in our case a grub2/50_coreos.cfg file [2]). Let's look first to see if grub2/50_coreos.cfg exists when trying to write new console settings before falling back to grub2/grub.cfg. [1] coreos/bootupd#543 [2] coreos/fedora-coreos-config#2769
Upstream we are starting to use grub configs that are partially baked into bootupd [1] and partially baked into the image via the OS vendor (in our case a grub2/50_coreos.cfg file [2]). Let's look first to see if grub2/50_coreos.cfg exists when trying to write new console settings before falling back to grub2/grub.cfg. [1] coreos/bootupd#543 [2] coreos/fedora-coreos-config#2769
Upstream we are starting to use grub configs that are partially baked into bootupd [1] and partially baked into the image via the OS vendor (in our case a grub2/30_console.cfg file [2]). Let's look first to see if grub2/30_console.cfg exists when trying to write new console settings before falling back to grub2/grub.cfg. [1] coreos/bootupd#543 [2] coreos/fedora-coreos-config#2769
bootupd upstream was modified to store default static grub configs and allow for OS integrators to add their own snippets by placing files in /usr/lib/bootupd/grub2-static/configs.d/ [1] When we move to building with osbuild we will start taking advantage of this functionality and calling bootupd with --with-static-configs. Let's go ahead and add our snippets here so they can be leveraged when we start calling bootupd with --with-static-configs. The snippets added here come from the parts of the config currently defined in coreos-assembler that weren't lifted into bootupd [2][3][4]. [1] coreos/bootupd#543 [2] https://github.com/coreos/coreos-assembler/blob/4636b1a5c6dc00b1d6a58b1bfbb199431444336b/src/grub.cfg#L58-L60 [3] https://github.com/coreos/coreos-assembler/blob/4636b1a5c6dc00b1d6a58b1bfbb199431444336b/src/grub.cfg#L71-L85 [4] https://github.com/coreos/coreos-assembler/blob/4636b1a5c6dc00b1d6a58b1bfbb199431444336b/src/grub.cfg#L87-L91
bootupd upstream was modified to store default static grub configs and allow for OS integrators to add their own snippets by placing files in /usr/lib/bootupd/grub2-static/configs.d/ [1] When we move to building with osbuild we will start taking advantage of this functionality and calling bootupd with --with-static-configs. Let's go ahead and add our snippets here so they can be leveraged when we start calling bootupd with --with-static-configs. The snippets added here come from the parts of the config currently defined in coreos-assembler that weren't lifted into bootupd [2][3][4]. [1] coreos/bootupd#543 [2] https://github.com/coreos/coreos-assembler/blob/4636b1a5c6dc00b1d6a58b1bfbb199431444336b/src/grub.cfg#L58-L60 [3] https://github.com/coreos/coreos-assembler/blob/4636b1a5c6dc00b1d6a58b1bfbb199431444336b/src/grub.cfg#L71-L85 [4] https://github.com/coreos/coreos-assembler/blob/4636b1a5c6dc00b1d6a58b1bfbb199431444336b/src/grub.cfg#L87-L91
bootupd upstream was modified to store default static grub configs and allow for OS integrators to add their own snippets by placing files in /usr/lib/bootupd/grub2-static/configs.d/ [1] When we move to building with osbuild we will start taking advantage of this functionality and calling bootupd with --with-static-configs. Let's go ahead and add our snippets here so they can be leveraged when we start calling bootupd with --with-static-configs. The snippets added here come from the parts of the config currently defined in coreos-assembler that weren't lifted into bootupd [2][3][4]. [1] coreos/bootupd#543 [2] https://github.com/coreos/coreos-assembler/blob/4636b1a5c6dc00b1d6a58b1bfbb199431444336b/src/grub.cfg#L58-L60 [3] https://github.com/coreos/coreos-assembler/blob/4636b1a5c6dc00b1d6a58b1bfbb199431444336b/src/grub.cfg#L71-L85 [4] https://github.com/coreos/coreos-assembler/blob/4636b1a5c6dc00b1d6a58b1bfbb199431444336b/src/grub.cfg#L87-L91 (cherry picked from commit b40f727)
bootupd upstream was modified to store default static grub configs and allow for OS integrators to add their own snippets by placing files in /usr/lib/bootupd/grub2-static/configs.d/ [1] When we move to building with osbuild we will start taking advantage of this functionality and calling bootupd with --with-static-configs. Let's go ahead and add our snippets here so they can be leveraged when we start calling bootupd with --with-static-configs. The snippets added here come from the parts of the config currently defined in coreos-assembler that weren't lifted into bootupd [2][3][4]. [1] coreos/bootupd#543 [2] https://github.com/coreos/coreos-assembler/blob/4636b1a5c6dc00b1d6a58b1bfbb199431444336b/src/grub.cfg#L58-L60 [3] https://github.com/coreos/coreos-assembler/blob/4636b1a5c6dc00b1d6a58b1bfbb199431444336b/src/grub.cfg#L71-L85 [4] https://github.com/coreos/coreos-assembler/blob/4636b1a5c6dc00b1d6a58b1bfbb199431444336b/src/grub.cfg#L87-L91 (cherry picked from commit b40f727)
bootupd upstream was modified to store default static grub configs and allow for OS integrators to add their own snippets by placing files in /usr/lib/bootupd/grub2-static/configs.d/ [1] When we move to building with osbuild we will start taking advantage of this functionality and calling bootupd with --with-static-configs. Let's go ahead and add our snippets here so they can be leveraged when we start calling bootupd with --with-static-configs. The snippets added here come from the parts of the config currently defined in coreos-assembler that weren't lifted into bootupd [2][3][4]. [1] coreos/bootupd#543 [2] https://github.com/coreos/coreos-assembler/blob/4636b1a5c6dc00b1d6a58b1bfbb199431444336b/src/grub.cfg#L58-L60 [3] https://github.com/coreos/coreos-assembler/blob/4636b1a5c6dc00b1d6a58b1bfbb199431444336b/src/grub.cfg#L71-L85 [4] https://github.com/coreos/coreos-assembler/blob/4636b1a5c6dc00b1d6a58b1bfbb199431444336b/src/grub.cfg#L87-L91
This pairs with coreos/bootupd#543 to have bootupd support injecting the greenboot grub logic only if greenboot is installed.
grub: Support multiple platform files
In practice, just one should be enough but let's expand
in case.
Add support for installing static grub configs
Currently these are duplicated in osbuild and coreos-assembler.
We will aim to deduplicate them here.
Ideally we'd add support for "day 2" updates of these; I started
on a patch for that but it's sadly messy.
This is an incremental improvement.