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

Implementing aarch64 image build capabilities #475

Closed
wants to merge 3 commits into from

Conversation

rdoxenham
Copy link
Contributor

In this PR I introduce the ability to use an aarch64 system with EIB, that allows the user to generate either RAW or ISO images with the same architecture, i.e. it's not possible to cross-build with these patches (e.g. x86_64 image on aarch64, or vice versa) as it's a) not possible to currently use the target architecture for the type of images that we cache, nor the hauler/nmc binary architectures, and b) not possible to currently spin up guestfish with a different qemu architecture to be able to run the image modification tools, but I think this is a good step forward to enabling customers to build aarch64 images at this time.

Note that we will need to rebase on leap:15.6 for ISO images, as the aarch64 ISO's need a newer version of xorriso to be able to repack the aarch64 ISO. I've tested these changes on both aarch64 and x86_64 based systems for both raw and iso (albeit with the previous comment in mind, I tested that pathway on 15.6).

Credit for this largely goes to @dbw7 and @diconico07 for their prior investigation and inputs.

Fixes: #193

Copy link
Contributor

@jdob jdob left a comment

Choose a reason for hiding this comment

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

Absolutely insane to me how small this patch is. I haven't tested it yet but will shortly. I just added the one quick comment about the comments in the Dockerfile.

Dockerfile Outdated
@@ -27,6 +27,7 @@ FROM opensuse/leap:15.5
# 4. RPM resolution logic
# 5. Embedded artefact registry
# 6. Network configuration
# 7. RAW image modification on aarch64
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't needed here. The intention was to explain the contents of this one big ass RUN line, but the aarch64 stuff is done at a different level after this.

Copy link
Contributor

@dbw7 dbw7 left a comment

Choose a reason for hiding this comment

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

Changes look good, added some comments which I’m curious to hear your thoughts on. The big unknown here is testing this on a variety of builds (especially with RPMs and embedded images) to see what breaks and what doesn’t.

ROOT_PART=/dev/sda3

# Make the necessarry adaptations for aarch64
if [[ $(uname -m) == "aarch64" ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should use the variable that’s set to the arch in the image definition file and then template it in as it allows us to remove this if check and just rely on a single variable which is a little bit safer.

Copy link
Contributor

Choose a reason for hiding this comment

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

I vote for doing this too - we should be able to simplify the script and move everything to Go code.

# Make the necessarry adaptations for aarch64
if [[ $(uname -m) == "aarch64" ]]; then
if ! test -f /dev/kvm; then
export LIBGUESTFS_BACKEND_SETTINGS=force_tcg
Copy link
Contributor

Choose a reason for hiding this comment

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

I personally believe that this should be the default behavior. Getting KVM to work in a container (in my experience) has been very error prone and while investigating the x86 EIB builds, I found that it resorts to “tcg” anyway so my opinion is that we just make use force_tcg as the default and avoid KVM.

Dockerfile Outdated
fi

# Clean up the repos to reduce size
RUN zypper clean -a
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to be done in the same layer as the installation, otherwise it will still be there.

@ipetrov117
Copy link
Contributor

ipetrov117 commented Jun 13, 2024

Not sure whether this should be covered by this PR, but have we tested whether the RPM resolution logic works with the suggested implementation?

I did a quick test on an aarch64 Tumbleweed with your changes + an EIB image using leap:15.6 as base and the RPM resolution logic failed with a libguestfs error. Could be doing something wrong though 🤷

# guestfish looks for very specific locations on the filesystem for UEFI firmware
# and also expects the boot kernel to be a portable executable (PE), not ELF.
RUN if [[ $(uname -m) == "aarch64" ]]; then \
zypper install -y qemu-uefi-aarch64; \
Copy link

Choose a reason for hiding this comment

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

How much space does this package take up? If it's trivial, it may make sense to go ahead and install the aarch64 emulator in the x86 image as well to make working on cross complications easier in the future.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is something we already do in the downstream builds.

@ipetrov117
Copy link
Contributor

Not sure whether this should be covered by this PR, but have we tested whether the RPM resolution logic works with the suggested implementation?

Tested RPM resolution logic on an arm system using arm SLE Micro. Image was built successful.

@atanasdinov
Copy link
Contributor

Resolved in #490.

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.

Add support for building image on an aarch64 host machine
7 participants