fix(roce_backend): build MOFED ISO filename from target architecture - #1400
Open
100milliongold wants to merge 1 commit into
Open
fix(roce_backend): build MOFED ISO filename from target architecture#1400100milliongold wants to merge 1 commit into
100milliongold wants to merge 1 commit into
Conversation
The ISO filename was hardcoded, including the distro release and architecture
that the two variables above it already carry. The name is therefore wrong on
aarch64, and wrong on Ubuntu 22.04 even on x86_64. All four combinations of
{24.04, 22.04} x {x86_64, aarch64} are published, so three of the four names
were unreachable.
Build the name from mofed_version, ansible_distribution_version and
ansible_architecture -- the expression roles/mofed already uses
(roles/mofed/defaults/main.yml:6), so the two roles stop disagreeing.
No architecture alias map is added here, unlike the two changes above: the point
is to match roles/mofed, which does not normalise aliases either. There is no
practical difference because ansible_architecture is uname -m, which never
reports arm64 on Linux.
The README quotes this block verbatim and is updated to match.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
roles/roce_backend/vars/main.ymlhardcodes the ISO filename, including thedistro version and architecture that the two variables above it already carry:
The ISO is published per distro release and per architecture, so this name is
wrong on aarch64, and also wrong on Ubuntu 22.04 even on x86_64.
Both are in scope.
README.md:43lists "Ubuntu 22.04 LTS and 24.04 LTS", andline 52 names them again "for generic Kubernetes and Slurm deployments";
docs/deepops/testing.md:126says "DeepOps currently uses Ubuntu 22.04 andUbuntu 24.04 for setup and Molecule GitHub Actions"; and per-release vars files
exist for both (
roles/nvidia-dgx/vars/ubuntu-22.04.yml,roles/nhc/vars/ubuntu-22.04.yml).Change
Build the name from
mofed_version,ansible_distribution_version, andansible_architecture— the same expressionroles/mofedalready uses(
roles/mofed/defaults/main.yml), so the two roles stop disagreeing.No architecture alias map is introduced here, unlike PR ① and ②. The point of
this change is to stop disagreeing with
roles/mofed, and that role(
roles/mofed/defaults/main.yml:6) also interpolatesansible_architecturedirectly without normalising aliases — adding a map here would reintroduce the
mismatch. The difference has no practical effect:
ansible_architectureisuname -m, which never reportsarm64on Linux. For the record, anarm64input would render
...-ubuntu24.04-arm64.iso, which returns HTTP 404; only theaarch64name exists.README updated to match, since it quotes this block verbatim.
Verification
All four combinations of {24.04, 22.04} × {x86_64, aarch64} are published, so
every name this expression renders resolves to a real ISO:
The aarch64 24.04 ISO reports
content-type: application/x-iso9660-image. Thethree names other than the first are unreachable with the hardcoded value.