Skip to content

Conversation

@deepssin
Copy link
Contributor

  • Add _update_uefi_grub_config() to sync UEFI GRUB config
  • Fixes issue where systems reboot into old kernel on UEFI

@deepssin deepssin requested a review from a team as a code owner December 18, 2025 13:57
@deepssin deepssin requested review from amathuria, batrick, djgalloway and kamoltat and removed request for a team December 18, 2025 13:57
@djgalloway
Copy link
Contributor

How does this work for non-UEFI systems? Should we maybe detect EFI vs BIOS and only run those if EFI=true?

@dmick
Copy link
Member

dmick commented Dec 18, 2025

I have seen suggestions that /etc/grub.cfg is always a symlink to the "right" grub.cfg. if that's true that would be better.

Also, does this code run for BLS systems? If BLS is used we should not be touching grub.cfg at all

Comment on lines 934 to 939
efi_vendor = remote.sh('ls -d /boot/efi/EFI/*/ 2>/dev/null | head -1 | xargs basename || echo ""').strip()
if efi_vendor:
result = remote.run(args=['test', '-f', '/boot/efi/EFI/{}/grub.cfg'.format(efi_vendor)], check_status=False)
if result.exitstatus == 0:
remote.run(args=['sudo', 'cp', grubconfig, '/boot/efi/EFI/{}/grub.cfg'.format(efi_vendor)])
log.info("Updated UEFI GRUB config at /boot/efi/EFI/{}/grub.cfg".format(efi_vendor))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
efi_vendor = remote.sh('ls -d /boot/efi/EFI/*/ 2>/dev/null | head -1 | xargs basename || echo ""').strip()
if efi_vendor:
result = remote.run(args=['test', '-f', '/boot/efi/EFI/{}/grub.cfg'.format(efi_vendor)], check_status=False)
if result.exitstatus == 0:
remote.run(args=['sudo', 'cp', grubconfig, '/boot/efi/EFI/{}/grub.cfg'.format(efi_vendor)])
log.info("Updated UEFI GRUB config at /boot/efi/EFI/{}/grub.cfg".format(efi_vendor))
efi_vendor = remote.sh('find /boot/efi/EFI/ -depth -mindepth 1 -print -quit')
if efi_vendor:
result = remote.run(args=['test', '-f', '{}/grub.cfg'.format(efi_vendor)], check_status=False)
if result.exitstatus == 0:
remote.run(args=['sudo', 'cp', grubconfig, '{}/grub.cfg'.format(efi_vendor)])
log.info("Updated UEFI GRUB config at {}/grub.cfg".format(efi_vendor))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Switched to find instead of ls -d as suggested. Initially it only found the first directory (/boot/efi/EFI/BOOT) because the command used -print -quit, which stops after the first match. The system actually boots from /boot/efi/EFI/centos/grub.cfg, so only updating BOOT didn't work.

  • So have removed -quit to find all EFI vendor directories
  • Correcting the option order: placing global options (-mindepth, -maxdepth) before -type

@deepssin
Copy link
Contributor Author

How does this work for non-UEFI systems? Should we maybe detect EFI vs BIOS and only run those if EFI=true?

Have added check https://github.com/deepssin/teuthology/blob/0408de85a4d6eca6e538f650f0963db7f75d15d3/teuthology/task/kernel.py#L983

@deepssin
Copy link
Contributor Author

I have seen suggestions that /etc/grub.cfg is always a symlink to the "right" grub.cfg. if that's true that would be better.

Also, does this code run for BLS systems? If BLS is used we should not be touching grub.cfg at all

I verified this on my system /etc/grub.cfg doesn't exist:

[root@node1 ~]# ls -l /etc/grub.cfgls: cannot access '/etc/grub.cfg': No such file or directory

What exists is /etc/grub2.cfg, which is a symlink to /boot/grub2/grub.cfg (not a UEFI location):

[root@node1 ~]# ls -l /etc/grub2.cfglrwxrwxrwx. 1 root root 22 Aug 7 18:10 /etc/grub2.cfg -> ../boot/grub2/grub.cfg

So the symlink check isn't reliable across systems is what I understand:

  • Check if the system is UEFI (via /sys/firmware/efi)
  • If UEFI, copy grub.cfg to all EFI vendor directories (e.g., /boot/efi/EFI/BOOT, /boot/efi/EFI/centos)

This avoids relying on symlink configurations that vary. The code now always updates all EFI vendor directories when on UEFI systems, ensuring the correct one is updated regardless of which vendor directory the firmware uses.

@djgalloway
Copy link
Contributor

I'm still confused. I am no coder but if I'm understanding what I'm reading correctly, this does not handle BLS at all. Is that right?

This commit does work but, again, I don't know how sane or valid or copacetic it is: 8ce0b02

@dmick
Copy link
Member

dmick commented Dec 24, 2025

You are correct, @djgalloway, and I don't understand why we're ignoring BLS either

- Add _update_uefi_grub_config() to sync UEFI GRUB config
- Fixes issue where systems reboot into old kernel on UEFI

Signed-off-by: deepssin <deepssin@redhat.com>
@deepssin
Copy link
Contributor Author

deepssin commented Jan 5, 2026

Thanks for pointing that out @dmick @djgalloway — you’re right that on BLS systems we shouldn’t be touching grub.cfg at all.

I’ve updated the logic so that:

BLS systems are handled exclusively via grubby / BLS entry IDs, and we no longer sync or rely on any grub.cfg content in that path.

The UEFI vendor grub.cfg sync is now only done in the non-BLS path, where we actually regenerate grub.cfg and kernel selection depends on it.

This keeps BLS handling fully independent of grub.cfg while still addressing the UEFI case where firmware boots directly from /boot/efi/EFI/*/grub.cfg.

Please let me know if this matches the intended split, or if you’d prefer a different UEFI handling strategy on BLS systems.

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.

4 participants