Skip to content

Change slurmctld and slurmd spool defaults#206

Merged
sjpb merged 4 commits into
masterfrom
feat/change-defaults
Feb 27, 2026
Merged

Change slurmctld and slurmd spool defaults#206
sjpb merged 4 commits into
masterfrom
feat/change-defaults

Conversation

@elelaysh
Copy link
Copy Markdown
Contributor

  • StateSaveLocation (slurmctld): change default to /var/spool/slurmctld
    this has no impact on existing deployments because it was already defined in ansible-slurm-appliance
  • SlurmdSpoolDir (slurmd): remove default, hardcoded one in slurm is /var/spool/slurmd
    (note the extra d)
    this has an impact but as it reside on the root disk, it will disappear on upgrades

Previous default didn't clearly identify data
as controller vs slurmd.
openhpc_slurmd_spool_dir was not used in slurm.conf
@elelaysh elelaysh requested a review from a team as a code owner February 23, 2026 16:18
@elelaysh elelaysh force-pushed the feat/change-defaults branch from f5f602a to ef05e60 Compare February 23, 2026 16:18
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the default spool directories for slurmctld and slurmd. The change for slurmctld's StateSaveLocation to /var/spool/slurmctld is a good improvement for clarity. For slurmd, the SlurmdSpoolDir is now removed from the default configuration to rely on Slurm's own default. However, the implementation hardcodes the directory path in one of the tasks, which removes the ability for users to override this setting. I've provided a suggestion to fix this while keeping the new default behavior.

Comment thread tasks/runtime.yml Outdated
@sjpb sjpb enabled auto-merge (squash) February 27, 2026 11:23
@sjpb sjpb merged commit c15221c into master Feb 27, 2026
27 checks passed
@sjpb sjpb deleted the feat/change-defaults branch February 27, 2026 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants