-
Notifications
You must be signed in to change notification settings - Fork 12
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
Deploy topology.conf unconditionally #53
Conversation
As mentioned in #52, this is the change I made locally to get srun working. I'm happy to revise the approach and add a new path through the previous conditional statement, if preferred. |
Left comment on #52 before I saw PR. This approach is fine, as it's essentially the same end result. |
2f3c27f
to
d4268d7
Compare
Some unit tests are failing due to lint issues with trailing commas in the unit tests. |
I'll take a closer look, I was fighting vscode trying to make excessive changes with formatting. Bear with me. |
srun will fail if slurm.conf defines use of a topology plugin but topology.conf does not exist on login nodes. Previously, topology.conf was only deployed to slurmd and slurmctld nodes. ``` $ srun -c 1 -n 1 --mem 1g --pty bash srun: error: s_p_parse_file: cannot stat file /etc/slurm/topology.conf: No such file or directory, retrying in 1sec up to 60sec srun: fatal: something wrong with opening/reading /etc/slurm/topology.conf: No such file or directory ``` This change removes the conditional and deploys topology.conf to all nodes. Fixes treydock#52
This is released with v3.1.0. |
srun will fail if slurm.conf defines use of a topology plugin but topology.conf does not exist on login nodes. Previously, topology.conf was only deployed to slurmd and slurmctld nodes.
This change removes the conditional and deploys topology.conf to all nodes.
Fixes #52