Upgrade Slurm version to 25.11.2#5237
Upgrade Slurm version to 25.11.2#5237AdarshK15 wants to merge 1 commit intoGoogleCloudPlatform:developfrom
Conversation
Summary of ChangesHello @AdarshK15, 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 Slurm image build process by upgrading the core Slurm version to 25.11.2, along with its Python dependency to 3.12 and Ansible to 8.7.0. These changes ensure compatibility with newer software versions and streamline the build environment setup, adjusting the Ansible installation method and the source repository for Slurm-GCP configurations. Highlights
Changelog
Activity
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 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 counter productive. 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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. 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 upgrades the Slurm version to 25.11.2, Python to 3.12, and Ansible to 8.7.0 in the hpc-build-slurm-image example. While the version upgrades are welcome, several issues need to be addressed. Most critically, the example now points to a personal GitHub fork for slurm-gcp, which must be changed back to the official repository to align with consistency guidelines. Additionally, there's a redundant Ansible installation that should be cleaned up, and a potentially required Python package for SELinux has been removed, which might cause problems during the image build.
| set -e -o pipefail | ||
| ansible-pull \ | ||
| -U https://github.com/GoogleCloudPlatform/slurm-gcp -C $(vars.build_from_git_ref) \ | ||
| -U https://github.com/AdarshK15/slurm-gcp -C $(vars.build_from_git_ref) \ |
There was a problem hiding this comment.
The ansible-pull command is now pulling from a personal GitHub fork (AdarshK15/slurm-gcp) instead of the official GoogleCloudPlatform/slurm-gcp repository. This should be reverted to the official repository to ensure the image is built from the correct and trusted source. Personal forks should not be used in examples.
-U https://github.com/GoogleCloudPlatform/slurm-gcp -C $(vars.build_from_git_ref) \References
- To maintain consistency and trust across blueprints, all components, including the main repository for
ansible-pull, should be fetched from the officialGoogleCloudPlatform/slurm-gcprepository rather than personal forks.
| settings: | ||
| # Do not create Ansible virtual env; Install system wide Ansible below. | ||
| install_ansible: false | ||
| install_ansible: true |
There was a problem hiding this comment.
Setting install_ansible: true causes the install_ansible.sh script to run, which installs Ansible in a virtual environment. However, the prep-for-slurm-build.sh runner script also installs Ansible system-wide. This is redundant and can lead to conflicts. The comment on line 41 also states "Do not create Ansible virtual env". To avoid redundancy and align with the intended system-wide installation, this should be set to false.
install_ansible: false| python3 -m pip install pip --upgrade | ||
| python3 -m pip install ansible==6.7.0 | ||
| python3 -m pip install selinux | ||
| python3 -m pip install ansible==8.7.0 |
There was a problem hiding this comment.
The python3 -m pip install selinux command was removed. The slurm-gcp Ansible playbook may require Python bindings for SELinux to manage file contexts correctly on the local machine. If this is required, it should be restored to prevent potential issues during the image build.
python3 -m pip install ansible==8.7.0
python3 -m pip install selinux1 participant
upgrading Slurm version to 25.11.2, python to 3.12 and ansible to 8.7.0.
Submission Checklist
NOTE: Community submissions can take up to 2 weeks to be reviewed.
Please take the following actions before submitting this pull request.