feat(slurm-controller): add x11_parameters to cloud_parameters - #6145
feat(slurm-controller): add x11_parameters to cloud_parameters#6145qsogia wants to merge 3 commits into
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
conflines() moved into SlurmConfigGenerator upstream; X11Parameters now set in the shared get_conf_options() so all Slurm version generators inherit it.
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 introduces the ability to configure Slurm's 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 adds support for configuring X11 parameters in the SchedMD Slurm GCP v6 controller module. Specifically, it introduces the x11_parameters optional string variable to the cloud_parameters object in both the main module and the slurm_files submodule, updates the Python configuration script (conf.py) to map this parameter to X11Parameters, and updates the corresponding README documentation files. I have no feedback to provide as the changes are complete and consistent.
Neelabh94
added
the
release-improvements
|
/gcbrun |
There was a problem hiding this comment.
Please add / update unit tests in community/modules/scheduler/schedmd-slurm-gcp-v6-controller/modules/slurm_files/scripts/tests/test_conf.py for this change
| tree_width = optional(number) | ||
| prolog_flags = optional(string) | ||
| switch_type = optional(string) | ||
| x11_parameters = optional(string) |
There was a problem hiding this comment.
A comment explaining that that srun --x11 requires xorg-x11-xauth (or equivalent xauth package) installed on both login and compute VM images would be helpful here.
| tree_width = optional(number) | ||
| prolog_flags = optional(string) | ||
| switch_type = optional(string) | ||
| x11_parameters = optional(string) |
There was a problem hiding this comment.
We should add a validation block for variable "cloud_parameters" in
community/modules/scheduler/schedmd-slurm-gcp-v6-controller/variables.tf that catches blueprint misconfigurations at terraform plan time. Something like this:
variable "cloud_parameters" {
description = "cloud.conf options. Default behavior defined in scripts/conf.py"
type = object({
.... <Existing>
switch_type = optional(string)
x11_parameters = optional(string)
})
default = {}
nullable = false
validation {
condition = (
var.cloud_parameters.x11_parameters == null ||
(var.cloud_parameters.prolog_flags != null && can(regex("(^|,)X11(,|$)", var.cloud_parameters.prolog_flags)))
)
error_message = "When 'x11_parameters' is specified in cloud_parameters, 'prolog_flags' must also be specified and include 'X11' (e.g. prolog_flags = \"X11\")."
}
}
|
Please NOTE : We recently upgraded our repository's Go version to 1.26 (#6173). We strongly recommend rebasing your open branches onto the latest develop branch to avoid or resolve any PR test failures. Thank you! |
|
this PR has been inactive for 7 days and has changes requested. @qsogia, please address the requested changes or close the PR if it's no longer needed. |
3 participants
Supersedes #5363, which the inactivity bot auto-closed on 2026-08-14 and which GitHub refuses to reopen. Same change, now merged up to current
develop.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→X11ParametersinSlurmConfigGenerator.get_conf_options()modules/slurm_files/variables.tf— addx11_parameters = optional(string)tocloud_parametersvariables.tf— same addition at the top-level controller moduleSince #5363 was opened,
conflines()was refactored intoSlurmConfigGenerator, so the mapping now lives in the sharedget_conf_options().SlurmConfigGeneratorV2511extends it viasuper()andSlurmConfigGeneratorV2411does not override it, so 24.11, 25.05 and 25.11 all pick it up. Verified all three emitX11Parameterswhen the setting is present, andtests/test_conf.pypasses unchanged (unset values are filtered out bydict_to_conf, so default output is unaffected).Usage
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