Build DOCA-OFED kernel modules for Rocky 10#2310
Conversation
DOCA OFED support has now been added for Rocky 10
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for DOCA OFED on Rocky Linux 10 for both x86_64 and aarch64 architectures. The changes involve updating repository configurations, kernel version matrices, and adding retry logic for Pulp-related tasks. Feedback focuses on improving maintainability and consistency by replacing hardcoded version strings with variables in repository definitions and variable names. Additionally, it is recommended to use kolla_base_arch instead of os_arch in push-ofed.yml to ensure correct repository selection during cross-architecture builds and to maintain consistency in failure messages.
| doca: | ||
| baseurl: "{{ stackhpc_repo_rhel10_doca_url }}" | ||
| description: "DOCA Online Repo {{ stackhpc_pulp_doca_version }} - RHEL $releasever" | ||
| description: "DOCA Online Repo 3.2.2 - RHEL $releasever" |
There was a problem hiding this comment.
The DOCA version 3.2.2 is hardcoded here, whereas for Rocky 9 it uses the stackhpc_pulp_doca_version variable. For better maintainability and consistency, consider using a variable here as well.
description: "DOCA Online Repo {{ stackhpc_pulp_doca_version }} - RHEL $releasever"| # password: "{{ stackhpc_repo_mirror_password | default(omit, true) }}" | ||
| doca-modules: | ||
| baseurl: "{{ stackhpc_repo_rhel10_doca_modules_url }}" | ||
| description: "OFED Kernel module repository for DOCA 3.2.2 - RHEL $releasever" |
There was a problem hiding this comment.
The DOCA version 3.2.2 is hardcoded in the description. It should ideally use a variable for consistency with other repository definitions.
description: "OFED Kernel module repository for DOCA {{ stackhpc_pulp_doca_version }} - RHEL $releasever"| "6": 2.9.3 | ||
| "7": 3.2.2 | ||
| stackhpc_pulp_doca_version: "{{ stackhpc_pulp_doca_version_matrix[stackhpc_pulp_repo_rocky_9_minor_version | string] | default('2.9.1') }}" | ||
| stackhpc_pulp_doca_version: "{{ stackhpc_pulp_doca_version_matrix[stackhpc_pulp_repo_rocky_9_minor_version | string] | default('2.9.3') }}" |
There was a problem hiding this comment.
The stackhpc_pulp_doca_version variable is currently tied to stackhpc_pulp_repo_rocky_9_minor_version. This is misleading when working with Rocky 10. Consider refactoring this logic to use the appropriate minor version variable based on the OS release (e.g., stackhpc_pulp_repo_rocky_10_minor_version when os_release == '10').
| doca_rocky_10_version_lookup_var: "stackhpc_pulp_repo_doca_3_2_2_rhel10_x86_64_version" | ||
| doca_rocky_10_aarch64_version_lookup_var: "stackhpc_pulp_repo_doca_3_2_2_rhel10_aarch64_version" | ||
| doca_rocky_10_modules_version_lookup_var: "stackhpc_pulp_repo_doca_3_2_2_rhel10_modules_version" | ||
| doca_rocky_10_modules_aarch64_version_lookup_var: "stackhpc_pulp_repo_doca_3_2_2_rhel10_modules_aarch64_version" |
There was a problem hiding this comment.
The lookup variables for Rocky 10 have the DOCA version 3_2_2 hardcoded in their names. This deviates from the dynamic pattern used for Rocky 9 (lines 26-29). This makes it more difficult to update the DOCA version for Rocky 10 in the future.
| - name: DOCA Online Repo 3.2.2 - RHEL 10 | ||
| url: "{{ stackhpc_release_pulp_content_url }}/doca/3.2.2/rhel10/x86_64/{{ stackhpc_pulp_repo_doca_3_2_2_rhel10_x86_64_version }}" | ||
| distribution_name: "doca-3.2.2-rhel10-" | ||
| distribution_name: "doca-3.2.2-rhel10-x86_64-" |
There was a problem hiding this comment.
The DOCA version 3.2.2 is hardcoded in the distribution name. Consider using a variable for consistency with the Rocky 9 repository definitions.
distribution_name: "doca-{{ stackhpc_pulp_doca_version }}-rhel10-x86_64-"
1 participant
DOCA OFED support has now been added for Rocky 10