Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the AIS system testing workflow by extracting FIO-related test steps from the main run_system_tests job into a separate run_FIO_tests job. The motivation (per the PR description) is to eventually use FIO packages produced by ROCm/fio instead of building FIO separately, though this PR currently still builds FIO from source as an intermediate step.
Changes:
- Removed FIO repository checkout and build directory creation from the
run_system_testsjob - Added cleanup steps to the
run_system_testsjob to ensure proper resource cleanup - Created a new
run_FIO_testsjob that checks out the FIO repository and runs FIO-based tests
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| with: | ||
| repository: ROCm/fio | ||
| ref: hipFile | ||
| path: fio |
There was a problem hiding this comment.
The new run_FIO_tests job is missing the checkout of the hipFile repository. The FIO configure step (line 280-281) references HIPFILE=/ais/hipFile and HIPFILELIB paths, and the FIO test steps (lines 305, 316) reference /ais/hipFile/util/fio/write-read-verify.fio. Without checking out the hipFile repository, these paths won't exist and the job will fail. Add a checkout step for the hipFile repository similar to line 48-51 in the run_system_tests job.
| path: fio | |
| path: fio | |
| - name: Fetching hipFile repository... | |
| uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd #v6.0.2 | |
| with: | |
| repository: ROCm/hipFile | |
| path: hipFile |
| - name: Download hipFile runtime package | ||
| uses: actions/download-artifact@37930b1c2abaa49bbe596cd826c3c89aef350131 #v7.0.0 | ||
| with: | ||
| name: ${{ inputs.ais_hipfile_pkg_filename }} | ||
| - name: Download hipFile development package | ||
| uses: actions/download-artifact@37930b1c2abaa49bbe596cd826c3c89aef350131 #v7.0.0 | ||
| with: | ||
| name: ${{ inputs.ais_hipfile_pkg_dev_filename }} |
There was a problem hiding this comment.
The new run_FIO_tests job is missing the download of the hipFile build directory artifact. The FIO configure step at line 281 references HIPFILELIB=${HIPFILE}/build/src/amd_detail/ which expects the hipFile build artifacts to be present. Without downloading the hipfile-build-dir artifact (as done in line 60-64 of run_system_tests), the FIO configuration and build will fail. Add a step to download the hipfile-build-dir artifact and copy it into the container similar to lines 60-64 and 133-137 in the run_system_tests job.
| "${AIS_CONTAINER_NAME}" \ | ||
| /bin/bash -c ' | ||
| cp -R /mnt/ais /ais | ||
| mkdir /ais/hipFile/build |
There was a problem hiding this comment.
The directory creation command attempts to create /ais/hipFile/build but the hipFile repository has not been checked out or copied into the container in this job. This will fail because there is no /ais/hipFile directory. After adding the hipFile repository checkout step, ensure it's copied into the container before this step, or remove this line since the hipFile build directory artifact should be downloaded and copied separately (as done in the run_system_tests job at lines 60-64 and 133-137).
| mkdir /ais/hipFile/build |
| if: ${{ always() }} | ||
| run: rm -rf ${GITHUB_WORKSPACE}/* ${GITHUB_WORKSPACE}/.* | ||
| run_FIO_tests: | ||
| runs-on: [linux, AIS] |
There was a problem hiding this comment.
The new run_FIO_tests job is missing a dependency on the run_system_tests job. Without specifying 'needs: run_system_tests', both jobs could run concurrently on the same AIS self-hosted runner, potentially causing resource contention (competing for GPU devices, disk I/O, etc.). Consider adding 'needs: run_system_tests' to ensure sequential execution and avoid resource conflicts, unless parallel execution is intentionally desired.
| runs-on: [linux, AIS] | |
| runs-on: [linux, AIS] | |
| needs: run_system_tests |
This is in preparation for calling out to the ROCm/fio workflow to build and package FIO with hipFile support separately. This workflow would then install the FIO packages produced instead of compiling it separately. This will allow us to at least sanity check the FIO packages we produce and publish.
03afdaa to
a2b2af8
Compare
Motivation
TBD - Use the FIO packages produced by ROCm/fio instead of building it separately here.
Technical Details
Test Plan
Test Result
Submission Checklist
AIHIPFILE-121