feat(slurm-controller): add x11_parameters to cloud_parameters#5363
feat(slurm-controller): add x11_parameters to cloud_parameters#5363qsogia wants to merge 2 commits intoGoogleCloudPlatform:developfrom
Conversation
Adds x11_parameters to the cloud_parameters object so that
X11Parameters can be set in slurm.conf via the blueprint without
forking slurm.conf.tpl. The setting flows through cloud.conf, which
is already included by slurm.conf via the existing include directive.
Example blueprint usage:
cloud_parameters:
prolog_flags: X11
x11_parameters: enable_send_credentials
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
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 enhances the Slurm GCP controller by integrating a new Highlights
Changelog
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 adds the x11_parameters setting to cloud_parameters, allowing for the configuration of Slurm's X11Parameters directive. The changes are implemented correctly across the Python configuration script and the Terraform variable definitions. My review focuses on ensuring the module documentation is updated to reflect this new parameter, which is a requirement for maintainability as per the project's style guide.
| tree_width = optional(number) | ||
| prolog_flags = optional(string) | ||
| switch_type = optional(string) | ||
| x11_parameters = optional(string) |
There was a problem hiding this comment.
The new x11_parameters field should be documented in the module's README.md to ensure the documentation is complete and clear for users. Please run the pre-commit-terraform-docs-hook to update the inputs table. This aligns with the repository's style guide on documentation.
References
- Module README.md files should be clear and provide sufficient information on usage, inputs, and outputs. (link)
| tree_width = optional(number) | ||
| prolog_flags = optional(string) | ||
| switch_type = optional(string) | ||
| x11_parameters = optional(string) |
There was a problem hiding this comment.
To maintain documentation quality, the new x11_parameters field should be added to this module's README.md. Please regenerate the documentation to update the cloud_parameters input description. This is required by the repository's style guide.
References
- Module README.md files should be clear and provide sufficient information on usage, inputs, and outputs. (link)
1 participant
Description
Adds
x11_parametersto thecloud_parametersobject inschedmd-slurm-gcp-v6-controllerso that Slurm'sX11Parametersdirective can be set via the blueprint without forkingslurm.conf.tpl.The setting flows through
cloud.conf(generated byconf.py), which is already included byslurm.confvia the existinginclude cloud.confdirective — no template changes required.Changes
modules/slurm_files/scripts/conf.py— mapx11_parameters→X11Parametersinconflines()modules/slurm_files/variables.tf— addx11_parameters = optional(string)tocloud_parametersvariables.tf— same addition at the top-level controller moduleUsage
Why
PrologFlags=X11was already supported viacloud_parameters.prolog_flags.X11Parameterswas not. Both are required forsrun --x11to work:PrologFlags=X11— enables X11 forwarding inslurmctldat allocation timeX11Parameters=enable_send_credentials— forwards the MIT-MAGIC-COOKIE from the login node to the compute node so the display tunnel can authenticate