rpm: speed up cross-arch worker builds by installing deps into mounted target rootfs#931
Conversation
603ebbc to
73adaf7
Compare
8fa6941 to
2330e0d
Compare
There was a problem hiding this comment.
Pull request overview
This PR optimizes cross-architecture RPM worker builds by installing build dependencies into a mounted target rootfs while running the package manager on the native BuildKit executor platform. This approach avoids slow QEMU emulation and "exec format error" issues during cross-architecture package installation.
Changes:
- Adds native executor platform detection from BuildKit worker options with platform normalization
- Implements cross-arch build flow: resolve target base image, run install on build platform with --installroot, and return mutated rootfs as target-platform worker
- Adds optional --forcearch support for dnf-based distros (applied when supported by dnf/tdnf)
- Enables CacheAddPlatform for Rocky 8/9, Azure Linux 3, and Mariner 2 to prevent cross-arch cache/repo metadata collisions
- Improves GitHub workflow reliability by simplifying registry mirror setup and adding error handling
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| targets/linux/rpm/distro/worker.go | Adds platform detection, normalization, and cross-arch worker build logic with InstallIntoRoot support |
| targets/linux/rpm/distro/dnf_install.go | Implements DnfForceArch option and InstallIntoRoot function for cross-arch package installation |
| targets/linux/rpm/rockylinux/v8.go | Enables platform-specific cache keys for Rocky Linux 8 |
| targets/linux/rpm/rockylinux/v9.go | Enables platform-specific cache keys for Rocky Linux 9 |
| targets/linux/rpm/azlinux/azlinux3.go | Enables platform-specific cache keys for Azure Linux 3 |
| targets/linux/rpm/azlinux/mariner2.go | Enables platform-specific cache keys for Mariner 2 |
| .github/workflows/worker-images.yml | Simplifies registry mirror setup and adds error handling for gh release view command |
| force_arch="` + forceArch + `" | ||
| install_root="` + installRoot + `" |
There was a problem hiding this comment.
The shell variable interpolation in lines 169-170 could be vulnerable to injection attacks if forceArch or installRoot contain shell metacharacters. Consider sanitizing these values or using proper quoting. However, since these values come from trusted internal sources (rpmArchFromPlatform and rootfsPath constant), this is less critical but still a best practice concern.
| force_arch="` + forceArch + `" | |
| install_root="` + installRoot + `" | |
| force_arch=` + fmt.Sprintf("%q", forceArch) + ` | |
| install_root=` + fmt.Sprintf("%q", installRoot) + ` |
4d76b66 to
889bd33
Compare
889bd33 to
edf8eaf
Compare
cpuguy83
left a comment
There was a problem hiding this comment.
I think we've extended the distro Config too far here to accommodate tdnf's limitations. The shell script fallback chain in the tdnf path (detect --forcearch → try installing dnf via tdnf → fall back to chroot+qemu) is complex and hard to test. The ExtraCacheDirs field also only exists because we might switch package managers at runtime.
Instead, I'd like to see this refactored so that azlinux/mariner configs just include dnf in their BuilderPackages. That way:
- We get a single code path: dnf --forcearch --installroot works the same across all RPM distros
- The ExtraCacheDirs field goes away (no runtime package manager switching)
- The ~70 lines of tdnf-specific shell with runtime --forcearch detection, tdnf install dnf fallback, and chroot fallback all go away
- The supports_forcearch grep-on-help-text detection goes away
More broadly, I think the cross-build logic would benefit from being a per-distro concern rather than living entirely in the centralized Config. Each distro knows its own setup requirements (e.g. azlinux needs dnf installed, rocky already has it). A Worker interface or per-distro setup hook would make this kind of customization cleaner than adding more fields to Config
targets/linux/rpm/distro/distro.go
Outdated
| CacheDir string | ||
|
|
||
| // Additional cache directories to mount (e.g. tdnf-based distros may also want /var/cache/dnf). | ||
| ExtraCacheDirs []string |
There was a problem hiding this comment.
Can we make CacheDir a []string?
There was a problem hiding this comment.
Makes sense, i have tried to make it looks simpler using above suggestions please check, Thanks
7887841 to
2a05ca0
Compare
2a05ca0 to
7d8dc77
Compare
This PR speeds up cross-arch RPM worker image builds by running the package manager on the native BuildKit executor while installing packages into a mounted target rootfs via --installroot. For dnf-based distros we use dnf --forcearch=<target>; for tdnf-based distros we prefer dnf (bootstrapped when needed), with a safe fallback to running tdnf inside the target rootfs under QEMU/chroot when cross-install is not possible. Signed-off-by: Kartik Joshi <karikjoshi21@gmail.com>
7d8dc77 to
cbc34d1
Compare
cpuguy83
left a comment
There was a problem hiding this comment.
Much cleaner here, thanks for the update!
Some basic test results like I did for the deb changes:
amd64->arm64
pr - 5m9s
main - 6m20s
amd64->amd64
pr - 2m14s
main - 2m20s
Enable faster and more reliable cross-arch RPM worker builds by installing build dependencies into a mounted target rootfs while running the package manager on the native BuildKit executor platform (avoids slow/full emulation and “exec format error” during install).
Changes
1. resolve target base image as target platform
2. run install on build platform and install into mounted target rootfs via --installroot
3. return the mutated rootfs as the target-platform worker