-
Notifications
You must be signed in to change notification settings - Fork 207
fix(aws-pcs): stop needrestart from restarting slurmd and killing running jobs #1165
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.
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
Changes from all commits
1f3489d
fc69fd0
04401f6
5af4edc
3a71d07
e0c495e
052e716
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -451,6 +451,25 @@ Resources: | |
| MIME-Version: 1.0 | ||
|
|
||
| runcmd: | ||
| # --- Protect running jobs from unattended-upgrades / needrestart --- | ||
| # apt-daily-upgrade updates base libraries (e.g. glibc). needrestart then | ||
| # auto-restarts every service linked against them. slurmd links libc, so it | ||
| # gets restarted mid-upgrade - which KILLS the jobs running under it (the | ||
| # step is torn down and the job requeues from scratch). Exclude slurmd | ||
| # from needrestart's automatic restart so a security upgrade can | ||
| # never take down a running job. Security packages still install; only the | ||
| # slurmd auto-restart is suppressed. (PCS runs the controller managed-side; | ||
| # slurmd is the only Slurm systemd service on login/compute nodes. qr(^slurmd) | ||
| # also covers the versioned units, e.g. slurmd-25.11.) | ||
| # runcmd runs under /bin/sh (dash) — keep this POSIX-clean, no bashisms. | ||
| - | | ||
| mkdir -p /etc/needrestart/conf.d | ||
| cat > /etc/needrestart/conf.d/90-pcs-slurm.conf <<'NRCONF' | ||
| # AWS PCS: never auto-restart slurmd - restarting it kills | ||
| # the jobs running under it. Managed by add-cng.yaml UserData. | ||
| $nrconf{override_rc} = { qr(^slurmd) => 0 }; | ||
| NRCONF | ||
| echo "needrestart: slurmd excluded from auto-restart" | tee /var/log/pcs-needrestart-guard.log | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| # Post-install script (optional) - generic OnNodeConfigured-style hook. | ||
| # Downloads and runs a user-supplied script (e.g. Enroot/Pyxis install). | ||
| # Accepts BOTH an s3:// URL (fetched with `aws s3 cp` using the instance | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
qr(^slurmd)also prefix-matchesslurmdbd— no change needed here (FYI)qr(^slurmd)is an unanchored-prefix regex, so it also matchesslurmdbd.servicein addition toslurmdand the versionedslurmd-25.11. On PCS login/compute nodes this has no practical effect — as the comment right above correctly notes,slurmdis the only Slurm unit here (the controller andslurmdbdare managed PCS-side). I'd leave it exactly as written: tightening it toqr(^slurmd($|-|\.))would only guard a unit that isn't present on these nodes. Flagging purely so the prefix-match is a known, deliberate property. It's the sameqr(^name) => 0idiom needrestart ships fordbus/display-managers.