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

Call grub2-mkconfig on all targets #57

Merged
merged 2 commits into from
May 21, 2020
Merged

Conversation

traylenator
Copy link
Contributor

Currently grub2-mkconfig is called on the first file
that exists from.

"/etc/grub2-efi.cfg",
"/boot/efi/EFI/#{os_name.downcase}/grub.cfg",
"/boot/grub2/grub.cfg",
"/boot/grub/grub.cfg"

So on a CentOS 7 system grub2-efi.cfg is generated only
from /etc/default/grub.

With an EFI system configured for bios boot the
/boot/grub2/grub.cfg is the important one so
kernel_parameter updates are not applied.

Taking inspiration from new-kernel-pkg
to see what happens on kernel upgrade grubby is called for each of
these files that exists so emulating this logic make sense.

Fixes #4

@coveralls
Copy link

coveralls commented May 14, 2020

Coverage Status

Coverage remained the same at 95.402% when pulling 2beb5ed on traylenator:bios into eb5c38e on hercules-team:master.

Currently grub2-mkconfig is called on the first file
that exists from.

```
"/etc/grub2-efi.cfg",
"/boot/efi/EFI/#{os_name.downcase}/grub.cfg",
"/boot/grub2/grub.cfg",
"/boot/grub/grub.cfg"
```

So on a CentOS 7 system `grub2-efi.cfg` is generated only
from `/etc/default/grub`.

With an EFI system configured for bios boot the
`/boot/grub2/grub.cfg` is the important one so
 `kernel_parameter` updates are not applied.

Taking inspiration from [new-kernel-pkg](https://github.com/rhboot/grubby/blob/master/new-kernel-pkg)
to see what happens on kernel upgrade `grubby` is called for each of
these files that exists so emulating this logic make sense.

Fixes voxpupuli#4
@traylenator
Copy link
Contributor Author

If you can give me a hint at what is missing for the coveralls happy to have ago.

@trevor-vaughan
Copy link
Contributor

@traylenator Don't worry about minor coveralls issues. I'll try to take a look at this today. It's been a pain trying to get this right across the board.

Copy link
Contributor

@trevor-vaughan trevor-vaughan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Acceptance tests all pass without issue.

@@ -175,14 +175,15 @@ def flush
if FileTest.file?(c) || ( FileTest.symlink?(c) &&
FileTest.directory?(File.dirname(File.absolute_path(File.readlink(c)))) )

cfg = c
break
cfg.push(c)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More common syntax

Suggested change
cfg.push(c)
cfg << c

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, this can probably be refactored as #select instead of #each to avoid the push.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't get the perl out of me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

select is not immediately obvious due the super call.

@traylenator
Copy link
Contributor Author

For interest some back ground that led to this:

https://techblog.web.cern.ch/techblog/post/bios_uefi_cloud_image/

@jhoblitt
Copy link
Member

I'm labeling this as backwards-incompatible as this has the potential to break the boot config when this module starts doing what it was supposed to be doing.

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.

grub.cfg isn't being properly updated on EFI systems running CentOS 7
5 participants