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

Skip stub grub.cfg files (e.g. used on Debian OS family). #65

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

olifre
Copy link
Contributor

@olifre olifre commented Aug 2, 2021

Replacing these may break OS update mechanisms which only update
the main file.

Fixes #51

@olifre olifre force-pushed the grubcfg-skip-stub-confs branch from 5bd04fa to cf44d08 Compare August 3, 2021 23:56
Replacing these may break OS update mechanisms which only update
the main file.
@olifre olifre force-pushed the grubcfg-skip-stub-confs branch from cf44d08 to 6ec048b Compare August 4, 2021 14:59
@trevor-vaughan
Copy link
Contributor

@olifre If you could do another test run to make sure things are OK then I think we're good to go.

@trevor-vaughan trevor-vaughan requested a review from raphink August 4, 2021 15:18
@trevor-vaughan
Copy link
Contributor

@traylenator Do you want to take a whack at this in your environment?

@olifre
Copy link
Contributor Author

olifre commented Aug 4, 2021

@olifre If you could do another test run to make sure things are OK then I think we're good to go.

@trevor-vaughan Already on it, I tested directly with the force-pushed branch ;-). Thanks a lot for the advice on the implementation, I'm still getting used to ruby and had the impression that more functional constructs are commonly used instead of "classic" logic flows, so this did look less "ruby-esque" to me than it probably is.

By now, my test nodes are reinstalled and they still boot fine as expected. I also gave the code snippet a "dry run" on some existing nodes, and it identified the stub / non-stub files correctly. So I have the same test coverage as before, but @traylenator can surely add other combinations (e.g. RHEL nodes with EFI booting).

@traylenator
Copy link
Contributor

@traylenator Do you want to take a whack at this in your environment?

We kind migrated away and switched to https://github.com/atsonkov/puppet-module-grubby let grubby do the heavy lifting.

@olifre
Copy link
Contributor Author

olifre commented Aug 5, 2021

@traylenator Grubby is not an option on our end, since we have both Debian and CentOS-based systems and need to support both — we want to avoid lock-in to any explicit system as far as possible (especially after the surprise RedHat gave all of us a while ago concerning CentOS).

I found an interesting note concerning Fedora 34 here:
https://fedoraproject.org/wiki/GRUB_2#Updating_the_GRUB_configuration_file
which states that they did indeed also switch to using a stub config file at /boot/efi/EFI/fedora/grub.cfg. So while I do not have a Fedora 34 at hand, I believe we also need this patch for Fedora 34 and hence likely also for any distro derived from RHEL 9.

I checked grub2.spec in the SRPM from Fedora 34, and in fact they do:

cat << EOF > ${EFI_HOME}/grub.cfg.stb
search --no-floppy --fs-uuid --set=dev ${BOOT_UUID}
set prefix=(\$dev)${GRUB_DIR}
export \$prefix
configfile \$prefix/grub.cfg
EOF
...
mv ${EFI_HOME}/grub.cfg.stb ${EFI_HOME}/grub.cfg

So the implementation done here should also be correct for Fedora 34 and hence also upcoming RedHat-based distros :-).

@raphink
Copy link
Member

raphink commented Aug 24, 2021

It might actually be more readable to use an Array#select() instead of a map + compact + uniq

@trevor-vaughan
Copy link
Contributor

@raphink I actually prefer it the way it is. If you use a select, you have to reverse the logic and it makes everything else more difficult to read/maintain (IMO)

@olifre
Copy link
Contributor Author

olifre commented Jan 17, 2022

Similar to @trevor-vaughan , I also feel it would not improve readability in this case.
At least I don't see a clean way to drop uniq, since after applying realpath, some paths may be the same on a given system, and handling a single file multiple times in one Puppet run is not wanted.
So I can't think of a way to rewrite this cleaner using the Puppet equivalent of select, which is filter — ideas welcome of course.

@loicbr
Copy link

loicbr commented Apr 15, 2024

I confirm that this module is broken in RHEL9 (9.3 at least-not tried in previous versions).
With the /boot/efi/EFI/fedora/grub.cfg file overwritten by the full config, changes in /boot/grub2/grubenv , such as setting the default boot entry, does not work anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ubuntu ESP grub.cfg config broken by grub-mkconfig
6 participants