-
Notifications
You must be signed in to change notification settings - Fork 33
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
base: master
Are you sure you want to change the base?
Conversation
5bd04fa
to
cf44d08
Compare
Replacing these may break OS update mechanisms which only update the main file.
cf44d08
to
6ec048b
Compare
@olifre If you could do another test run to make sure things are OK then I think we're good to go. |
@traylenator Do you want to take a whack at this in your environment? |
@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). |
We kind migrated away and switched to https://github.com/atsonkov/puppet-module-grubby let grubby do the heavy lifting. |
@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: I checked
So the implementation done here should also be correct for Fedora 34 and hence also upcoming RedHat-based distros :-). |
It might actually be more readable to use an |
@raphink I actually prefer it the way it is. If you use a |
Similar to @trevor-vaughan , I also feel it would not improve readability in this case. |
I confirm that this module is broken in RHEL9 (9.3 at least-not tried in previous versions). |
Replacing these may break OS update mechanisms which only update
the main file.
Fixes #51