Support Managed Lustre for A3 HighGPU Slurm clusters. The Lustre client is compiled and installed using DKMS because a pre-compiled client module package for the TCPX kernel does not currently exist in the Artifact Registry.#5974
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request updates the A3 HighGPU Slurm blueprint to utilize Managed Lustre as the primary network storage solution. To ensure compatibility with the specialized tcpx kernel used in these environments, the changes include a robust fallback installation method using DKMS, ensuring the Lustre client can be built from source when standard pre-compiled binaries are missing. These updates also include necessary configuration adjustments to the deployment blueprints and test suites to support the new storage architecture. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![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 updates the A3 High GPU Slurm blueprint to use Managed-Lustre instead of Filestore, including adding a script to install Lustre client modules for the TCPX kernel. Additionally, the Lustre client installation scripts are updated to fallback to DKMS installation if pre-compiled modules are not found. The review feedback highlights critical issues with error handling in these shell scripts, where toggling set +e/set -e or using compound commands in || lists can silently mask installation failures. It is recommended to refactor these into standard if ! ... blocks to ensure robust error propagation.
| set +e | ||
| apt-get install -y lustre-client-modules-${TCPX_KERNEL_VERSION} | ||
| if [ $? -ne 0 ]; then | ||
| echo "Pre-compiled package lustre-client-modules-${TCPX_KERNEL_VERSION} not found. Installing via DKMS for ${TCPX_KERNEL_VERSION}..." | ||
| apt-get install -y dkms lustre-client-modules-dkms lustre-client-utils | ||
| echo "Building DKMS modules specifically for ${TCPX_KERNEL_VERSION}..." | ||
| dkms autoinstall -k "${TCPX_KERNEL_VERSION}" | ||
| fi | ||
| set -e |
There was a problem hiding this comment.
Toggling set +e and set -e globally is risky and can mask failures. Because set +e is active during the entire fallback block, any failure in apt-get install -y dkms ... or dkms autoinstall will be silently ignored, and the script will continue. Using if ! command; then keeps set -e active for all commands inside the then block, ensuring that any failure during the DKMS fallback installation or build will immediately fail the script with an error.
if ! apt-get install -y lustre-client-modules-${TCPX_KERNEL_VERSION}; then
echo "Pre-compiled package lustre-client-modules-${TCPX_KERNEL_VERSION} not found. Installing via DKMS for ${TCPX_KERNEL_VERSION}..."
apt-get install -y dkms lustre-client-modules-dkms lustre-client-utils
echo "Building DKMS modules specifically for ${TCPX_KERNEL_VERSION}..."
dkms autoinstall -k "${TCPX_KERNEL_VERSION}"
fi| apt install -y "lustre-client-modules-$(uname -r)" lustre-client-utils || { | ||
| echo "Pre-compiled package lustre-client-modules-$(uname -r) not found. Attempting install via lustre-client-modules-dkms..." | ||
| apt install -y dkms lustre-client-modules-dkms lustre-client-utils | ||
| } || (echo "Error finding Lustre module packages, Lustre package may not exist for this kernel version" && exit 1) |
There was a problem hiding this comment.
In Bash, when a compound command (like { ... }) is part of an || list, set -e is disabled for all commands inside that block. If apt install -y dkms ... fails, it won't trigger an immediate exit. Additionally, the error handler uses (echo ... && exit 1) which runs in a subshell and may not behave as expected depending on the shell environment. Using standard if ! ... blocks avoids subshells and inheritance issues, making the code much more readable and robust.
| apt install -y "lustre-client-modules-$(uname -r)" lustre-client-utils || { | |
| echo "Pre-compiled package lustre-client-modules-$(uname -r) not found. Attempting install via lustre-client-modules-dkms..." | |
| apt install -y dkms lustre-client-modules-dkms lustre-client-utils | |
| } || (echo "Error finding Lustre module packages, Lustre package may not exist for this kernel version" && exit 1) | |
| if ! apt install -y "lustre-client-modules-$(uname -r)" lustre-client-utils; then | |
| echo "Pre-compiled package lustre-client-modules-$(uname -r) not found. Attempting install via lustre-client-modules-dkms..." | |
| if ! apt install -y dkms lustre-client-modules-dkms lustre-client-utils; then | |
| echo "Error finding Lustre module packages, Lustre package may not exist for this kernel version" >&2 | |
| exit 1 | |
| fi | |
| fi |
| apt install -y "lustre-client-modules-$(uname -r)" lustre-client-utils || { | ||
| echo "Pre-compiled package lustre-client-modules-$(uname -r) not found. Attempting install via lustre-client-modules-dkms..." | ||
| apt install -y dkms lustre-client-modules-dkms lustre-client-utils | ||
| } || (echo "Error finding Lustre module packages, Lustre package may not exist for this kernel version" && exit 1) |
There was a problem hiding this comment.
In Bash, when a compound command (like { ... }) is part of an || list, set -e is disabled for all commands inside that block. If apt install -y dkms ... fails, it won't trigger an immediate exit. Additionally, the error handler uses (echo ... && exit 1) which runs in a subshell and may not behave as expected depending on the shell environment. Using standard if ! ... blocks avoids subshells and inheritance issues, making the code much more readable and robust.
| apt install -y "lustre-client-modules-$(uname -r)" lustre-client-utils || { | |
| echo "Pre-compiled package lustre-client-modules-$(uname -r) not found. Attempting install via lustre-client-modules-dkms..." | |
| apt install -y dkms lustre-client-modules-dkms lustre-client-utils | |
| } || (echo "Error finding Lustre module packages, Lustre package may not exist for this kernel version" && exit 1) | |
| if ! apt install -y "lustre-client-modules-$(uname -r)" lustre-client-utils; then | |
| echo "Pre-compiled package lustre-client-modules-$(uname -r) not found. Attempting install via lustre-client-modules-dkms..." | |
| if ! apt install -y dkms lustre-client-modules-dkms lustre-client-utils; then | |
| echo "Error finding Lustre module packages, Lustre package may not exist for this kernel version" >&2 | |
| exit 1 | |
| fi | |
| fi |
2 participants
Description
This PR transitions the A3 HighGPU (a3-highgpu-8g) Slurm blueprint from using Filestore to using Managed Lustre as the primary network storage.
Because the A3 HighGPU nodes utilize a specialized linux-gcp-tcpx kernel that currently lacks pre-compiled Lustre client packages in the Artifact Registry, this PR introduces DKMS fallback logic to compile the client from source
Testing
Used the on demand capacity to launch the cluster and executed the NCCL tests against it.
Submission Checklist
NOTE: Community submissions can take up to 2 weeks to be reviewed.
Please take the following actions before submitting this pull request.