Make boot-file architecture config distro-aware#788
Conversation
|
|
||
| default: | ||
| return TargetOs{}, fmt.Errorf("unknown ID (%s) in os-release", distroId) | ||
| return TargetOs{}, fmt.Errorf("unknown ID (%s) in %s", distroId, sourceLabel) |
There was a problem hiding this comment.
You could simplify this to just "unknown ID (%s)" and then let the caller add the context. That way, you can delete sourceLabel.
| @@ -0,0 +1,120 @@ | |||
| // Copyright (c) Microsoft Corporation. | |||
There was a problem hiding this comment.
Would it make sense to move this logic into the initrdutils package?
| ukiAddonStubBinaryPath string | ||
| } | ||
|
|
||
| var bootloaderFilesConfig = map[string]BootFilesArchConfig{ |
There was a problem hiding this comment.
Can we rename bootloaderFilesConfig so that it is more distro specific, to match bootloaderFilesConfigFedora?
| "amd64": { | ||
| bootBinary: bootx64BinaryFedora, | ||
| grubBinary: grubx64Binary, | ||
| grubNoPrefixBinary: grubx64NoPrefixBinary, |
There was a problem hiding this comment.
I don't think Fedora has a grubx64-noprefix.efi file. It is kind of an AZL3 thing.
| // Move bootloader files from under '<pxe-folder>/efi/boot' to '<pxe-folder>/' | ||
| _, bootFilesConfig, err := getBootArchConfig() | ||
| // Move bootloader files from under '<outputPXEArtifactsDir>/<isoBootloaderDir>' to '<outputPXEArtifactsDir>/'. | ||
| bootFilesConfig, err := distroHandler.GetBootArchConfig() |
There was a problem hiding this comment.
There is a build error for distroHandler
|
|
||
| // initramfsTypeFromFilesStore detects the initramfs type based on the files in the provided file store. | ||
| func initramfsTypeFromFilesStore(filesStore *IsoFilesStore) imagecustomizerapi.InitramfsImageType { | ||
| if filesStore == nil || filesStore.squashfsImagePath == "" { |
There was a problem hiding this comment.
Why do you check filesStore == nil here? This feels wrong.
| return nil, fmt.Errorf("failed to check for squashfs at (%s):\n%w", squashfsPath, err) | ||
| } | ||
| if squashfsExists { | ||
| filesStore.squashfsImagePath = squashfsPath |
There was a problem hiding this comment.
It might be worth adding a comment saying that initramfsTypeFromFilesStore needs the squashfsImagePath value.
| } | ||
|
|
||
| var initrdPath string | ||
| switch initramfsTypeFromFilesStore(filesStore) { |
There was a problem hiding this comment.
This initramfsTypeFromFilesStore call is just reading squashfsExists but in a really roundabout way.
| var err error | ||
| var fields map[string]string | ||
|
|
||
| found := false |
There was a problem hiding this comment.
IMHO removing found makes the code more confusing since it relies on envfile.ParseEnvFile returning nil on an error.
| if err != nil { | ||
| return nil, fmt.Errorf("failed to determine the target OS from initrd (%s):\n%w", initrdPath, err) | ||
| } | ||
| return NewDistroHandler(targetOs) |
There was a problem hiding this comment.
Add the if err != nil {, since it makes it easier to modify the function in the future.
createPXEArtifacts calls distroHandler.GetBootArchConfig() but never received a distroHandler parameter, so the package failed to build with 'undefined: distroHandler'. Add distroHandler as the final parameter and pass it from both call sites in liveosisobuilder.go, which already have a distroHandler in scope.
2 participants
Boot-file paths and ESP layout are now chosen per distro rather than from a single architecture-only global, and the ISO-to-ISO pipeline detects the distro directly from the initrd.
Previously
getBootArchConfig()was a global that only varied by runtime architecture, so every distro received the same Azure-Linux-style ESP layout. This moves the lookup behind theDistroHandlerinterface so each distro picks its own boot-file layout, and adds a way to identify the distro from an initramfs when no rootfs is mounted.What changed
GetBootArchConfig()to theDistroHandlerinterface. Each distro returns its own per-architecture boot-file map./EFI/BOOTwithBOOTX64.EFI/BOOTAA64.EFI). Azure Linux 2/3, ACL, and Ubuntu keep the existing Azure Linux layout.targetos.GetInitrdTargetOs()andNewDistroHandlerFromInitrd(). These read anos-releaseor dracutinitrd-releasefile straight out of the initramfs cpio, auto-detecting gzip and zstd compression from the stream's magic bytes.getBootArchConfig()withbootArchConfigFromMap()and routed every boot-file lookup (UKI creation, PXE artifacts, ISO artifact store, output artifacts) through the distro handler.Distro to ESP boot-file layout
Checklist