Skip to content
This repository has been archived by the owner on Dec 16, 2024. It is now read-only.

Fix ubuntu efi iso creation #25

Merged
merged 2 commits into from
Jan 12, 2024
Merged

Fix ubuntu efi iso creation #25

merged 2 commits into from
Jan 12, 2024

Conversation

Itxaka
Copy link
Member

@Itxaka Itxaka commented Jan 12, 2024

Fix grub.cfg location for ubuntu
Fix grub.efi name cleanup

Fix grub.cfg location for ubuntu
Fix grub.efi name cleanup

Signed-off-by: Itxaka <itxaka@kairos.io>
@Itxaka Itxaka requested a review from a team January 12, 2024 10:02
@@ -183,6 +183,19 @@ func (b BuildISOAction) createEFI(rootdir string, isoDir string) error {
b.cfg.Logger.Errorf("Failed writing grub.cfg: %v", err)
return err
}
// Ubuntu efi searches for the grub.cfg file under /EFI/ubuntu/grub.cfg while we store it under /boot/grub2/grub.cfg
// workaround this by copying it there as well
// TODO: Should we symlink all the distros folders to our single grub.cfg?
Copy link
Contributor

Choose a reason for hiding this comment

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

If symlinks work for this, this sounds like a good idea

// Ubuntu efi searches for the grub.cfg file under /EFI/ubuntu/grub.cfg while we store it under /boot/grub2/grub.cfg
// workaround this by copying it there as well
// TODO: Should we symlink all the distros folders to our single grub.cfg?
err = utils.MkdirAll(b.cfg.Fs, filepath.Join(isoDir, "EFI/ubuntu/"), constants.DirPerm)
Copy link
Contributor

Choose a reason for hiding this comment

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

so every distro will have an ubuntu directory now? Maybe we should try the symlink idea instead. That fix would happen in the Dockerfiles right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Only for isos. The final system should not have that.

Copy link
Member Author

Choose a reason for hiding this comment

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

on the install part, we will need to do the same for ubuntu, but in that point we know what distro is due to being able to read /etc/os-release so we can only trigger the same thing if we are on ubuntu.

Maybe we should do the same here.....

// Ubuntu efi searches for the grub.cfg file under /EFI/ubuntu/grub.cfg while we store it under /boot/grub2/grub.cfg
// workaround this by copying it there as well
// read the os-release from the rootfs to know if we are creating a ubuntu based iso
FLAVOR, err := sdk.OSRelease("FLAVOR", filepath.Join(rootdir, "etc/os-release"))
Copy link
Contributor

Choose a reason for hiding this comment

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

[Minor] No reason to use upper cased variable names. Just flavor should be ok.

Copy link
Member Author

Choose a reason for hiding this comment

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

ERRO[2024-01-12T14:22:07Z] Failed preparing ISO's root tree: KAIROS_flavor key not found 

case sensitive!

Copy link
Member Author

Choose a reason for hiding this comment

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

🤦

Copy link
Contributor

Choose a reason for hiding this comment

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

sorry yes, I meant the golang variable, not the os-release one.

Also do the efi part first before the squashfs so we fail faster

Signed-off-by: Itxaka <itxaka@kairos.io>
@Itxaka Itxaka force-pushed the fix_ubuntu_iso_grub branch from 66788af to b2fd666 Compare January 12, 2024 14:39
@Itxaka Itxaka merged commit 0d1aefd into main Jan 12, 2024
2 checks passed
@Itxaka Itxaka deleted the fix_ubuntu_iso_grub branch January 12, 2024 14:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants