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

Grub2 grub_user fix #32

Merged
merged 5 commits into from
May 9, 2018

Conversation

trevor-vaughan
Copy link
Contributor

The user generation script was incorrectly outputting additional
erronous information

The user generation script was incorrectly outputting additional
erronous information
@coveralls
Copy link

coveralls commented Apr 30, 2018

Coverage Status

Coverage remained the same at 95.402% when pulling dbc070f on trevor-vaughan:grub2_user_fix into ce7389d on hercules-team:master.

Copy link

@jeannegreulich jeannegreulich left a comment

Choose a reason for hiding this comment

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

When you call the puppetx::augeasprovidersGrub::grub2_cfg to find the grub.cfg file it takes the first one it finds. Suggest you add a check for /sys/firmware/efi and if efi, get only the efi files and visa versa. Someone might go in and erroneaulsy edit or create a /boot/grub2 config file and they will be hosed. Also why include /boot/grub/grub.cfg since that is only grub not grub2?

Also, name the file in grub.d something so it will get included after 01_users because the superuser setting from 01_users will win if it is later in the grub file.

Copy link

@jeannegreulich jeannegreulich left a comment

Choose a reason for hiding this comment

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

I reviewed these and tested them out on a system and they work.

@trevor-vaughan trevor-vaughan merged commit e773a9a into voxpupuli:master May 9, 2018
@olifre
Copy link
Contributor

olifre commented Jul 19, 2018

Also why include /boot/grub/grub.cfg since that is only grub not grub2?

Since that's what Ubuntu uses for Grub2 configuration.
This removal, which was not documented in the commit message, has broken at least Ubuntu (maybe also Debian).

Grub1 would use menu.lst which is completely unrelated to the files looked for here.

See #36 for my fixup PR.

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

Successfully merging this pull request may close these issues.

None yet

4 participants