Skip to content

feat(slurm): add startup_script support to slurm login nodes #5800

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
kvenkatachala333 wants to merge 1 commit into
base: develop
Choose a base branch
from
+18 −1
Draft

feat(slurm): add startup_script support to slurm login nodes #5800

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion community/modules/scheduler/schedmd-slurm-gcp-v6-controller/login.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ module "login" {

login_nodes = each.value

startup_scripts = local.login_startup_scripts
startup_scripts = concat(local.login_startup_scripts, each.value.startup_script)

@gemini-code-assist gemini-code-assist Bot Jun 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

If each.value.startup_script is explicitly set to null or is missing, the concat function will fail at plan-time because it expects all arguments to be lists.

To prevent plan-time crashes and safely handle null or missing values (in alignment with the general rule of keeping variables nullable and using safe fallbacks), use a try() block to safely access the attribute and default to an empty list [] if it is null or missing.

  startup_scripts         = concat(local.login_startup_scripts, try(each.value.startup_script != null ? each.value.startup_script : [], []))
References
  1. To allow variables to be explicitly set to null without automatically overriding them to default configurations, keep the variable nullable and use try() blocks in the main configuration to safely access its attributes and prevent plan-time crashes.

@LAVEEN LAVEEN Jun 17, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you please check if this is required to be addressed or not

startup_scripts_timeout = var.login_startup_scripts_timeout

network_storage = var.login_network_storage
Expand Down
4 changes: 4 additions & 0 deletions community/modules/scheduler/schedmd-slurm-gcp-v6-controller/variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,10 @@ variable "login_nodes" {
termination_action = optional(string)
disk_encryption_key = optional(string)
disk_encryption_key_service_account = optional(string)
startup_script = optional(list(object({
filename = string
content = string
})), [])
}))
default = []
validation {
Expand Down
7 changes: 7 additions & 0 deletions community/modules/scheduler/schedmd-slurm-gcp-v6-login/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,11 @@ locals {
scopes = var.service_account_scopes
}

ghpc_startup_script = [{
filename = "ghpc_login_startup.sh"
content = var.startup_script
}]
Comment on lines +66 to +69

@gemini-code-assist gemini-code-assist Bot Jun 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

If var.startup_script is explicitly set to null, content will be null. This will cause a plan-time crash in the downstream google_storage_bucket_object.startup_scripts resource when computing md5(each.value).

To prevent this, instead of overriding the user's explicit null setting with a default string, conditionally omit the startup script configuration entirely if var.startup_script is null.

  ghpc_startup_script = var.startup_script != null ? [{
    filename = "ghpc_login_startup.sh"
    content  = var.startup_script
  }] : []
References
  1. To allow variables to be explicitly set to null without automatically overriding them to default configurations, keep the variable nullable and use try() blocks in the main configuration to safely access its attributes and prevent plan-time crashes.


# lower, replace `_` with `-`, and remove any non-alphanumeric characters
group_name = replace(
replace(
Expand Down Expand Up @@ -114,6 +119,8 @@ locals {
static_ips = var.static_ips
bandwidth_tier = var.bandwidth_tier

startup_script = local.ghpc_startup_script

subnetwork = var.subnetwork_self_link
tags = var.tags
}
Expand Down
6 changes: 6 additions & 0 deletions community/modules/scheduler/schedmd-slurm-gcp-v6-login/variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -430,3 +430,9 @@ variable "subnetwork_self_link" {
type = string
description = "Subnet to deploy to."
}

variable "startup_script" {
description = "Startup script used by the login VMs."
type = string
default = "# no-op"
}
Loading