voxpupuli / puppet-augeasproviders_grub

Augeas-based grub type and providers
Apache License 2.0
9 stars 33 forks source link

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

Open olifre opened 3 years ago

olifre commented 3 years ago

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

Fixes #51

trevor-vaughan commented 3 years ago

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

trevor-vaughan commented 3 years ago

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

olifre commented 3 years ago

@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 commented 3 years ago

@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 commented 3 years ago

@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 commented 2 years ago

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

trevor-vaughan commented 2 years ago

@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 commented 2 years ago

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 commented 4 months ago

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.