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

installer: T7049: Fix GRUB boot with RAID1 #4387

Open
wants to merge 1 commit into
base: current
Choose a base branch
from

Conversation

woodsb02
Copy link

Change summary

Rename directory in EFI system partition from:
From: \EFI\VyOS (RAID disk 1)
To: \EFI\VyOS

This prevents GRUB dropping to a grub prompt rather than showing the VyOS boot menu, after installing with the RAID1 option.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes)
  • Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
  • Other (please describe):

Related Task(s)

Refer bug: https://vyos.dev/T7049

Related PR(s)

How to test / Smoketest result

Create virtual machine with 2 hard drives of identical size.
Boot VyOS nightly ISO, and "install image".
Accept automatically proposed RAID1 arrangement for disks.
Reboot

Before this fix, the GRUB prompt will be displayed upon reboot.
After this fix, the GRUB menu will be correctly displayed and VyOS will boot normally.

Checklist:

  • I have read the CONTRIBUTING document
  • I have linked this PR to one or more Phabricator Task(s)
  • I have run the components SMOKETESTS if applicable
  • My commit headlines contain a valid Task id
  • My change requires a change to the documentation
  • I have updated the documentation accordingly

Rename directory in EFI system partition from:
From: \EFI\VyOS (RAID disk 1)
To: \EFI\VyOS

This prevents GRUB dropping to a grub prompt rather than showing the VyOS boot menu, after installing with the RAID1 option.

Refer bug: https://vyos.dev/T7049
@woodsb02 woodsb02 requested a review from a team as a code owner March 10, 2025 16:27
Copy link

👍
No issues in PR Title / Commit Title

@woodsb02
Copy link
Author

@jestabro I believe this was introduced during this commit:
e036f78#diff-13c7c184e32505bd9831fe2d991183af9f49fe2e1e0737ce70a2d3c6923f3ac1L33-R594

Would you be able to review this pull request please?

@jestabro
Copy link
Contributor

Yes, thanks @woodsb02 , I will take a look ...

Copy link

CI integration 👍 passed!

Details

CI logs

  • CLI Smoketests (no interfaces) 👍 passed
  • CLI Smoketests (interfaces only) 👍 passed
  • Config tests 👍 passed
  • RAID1 tests 👍 passed
  • TPM tests 👍 passed

@andamasov andamasov requested a review from Copilot March 10, 2025 22:56
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

PR Overview

This PR fixes an issue where GRUB drops to a prompt instead of displaying the VyOS boot menu during RAID1 installations.

  • Updated the GRUB installation call in image_installer.py by removing the RAID disk identifier from the EFI directory name.

Reviewed Changes

File Description
src/op_mode/image_installer.py Removed extra id parameter from grub.install to align with new naming

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

@@ -877,8 +877,7 @@ def install_image() -> None:
for disk_target in l:
disk.partition_mount(disk_target.partition['efi'], f'{DIR_DST_ROOT}/boot/efi')
grub.install(disk_target.name, f'{DIR_DST_ROOT}/boot/',
f'{DIR_DST_ROOT}/boot/efi',
id=f'VyOS (RAID disk {l.index(disk_target) + 1})')
f'{DIR_DST_ROOT}/boot/efi')
Copy link
Preview

Copilot AI Mar 10, 2025

Choose a reason for hiding this comment

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

Removing the 'id' parameter changes the way GRUB identifies the EFI directory; please ensure that the grub.install function is updated accordingly and that no unintended side effects occur from dropping the RAID disk identifier.

Suggested change
f'{DIR_DST_ROOT}/boot/efi')
f'{DIR_DST_ROOT}/boot/efi', disk_target.id)

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

2 participants