Skip to content

Conversation

@nick-stroud
Copy link
Collaborator

@nick-stroud nick-stroud commented Jun 2, 2022

Allows for passing of startup_script to Slurm V4 using the use keyword. Modules only fall back to this behavior if the other startup script variables are not explicitly set (ex: compute_startup_script)

The controller module takes startup scripts for both compute and controller. I elected to not fallback to the startup_script variable unless neither were set. This is to avoid confusion where startup_script is being only partially overwritten.

Tested:

  • Ran ghpc create community/examples/spack-gromacs.yaml --vars project_id=test and verified in generated terraform that in main.tf, slurm_login contained startup_script = module.spack-startup.startup_script.
  • Ran integration tests that contains spack-growmacs example.

Submission Checklist

  • Have you installed and run this change against pre-commit? (pre-commit install)
  • Are all tests passing? (make tests)
  • Have you written unit tests to cover this change?
  • Is unit test coverage still above 80%?
  • Have you updated all applicable documentation?
  • Have you followed the guidelines in our Contributing document?

@nick-stroud nick-stroud requested a review from tpdownes June 2, 2022 00:25
@nick-stroud nick-stroud assigned tpdownes and nick-stroud and unassigned tpdownes Jun 2, 2022
@nick-stroud nick-stroud force-pushed the slurm_startup_script_variable branch from ee86f17 to 0dbe695 Compare June 2, 2022 17:51
@nick-stroud nick-stroud assigned tpdownes and unassigned nick-stroud Jun 2, 2022
Copy link
Contributor

@tpdownes tpdownes left a comment

Choose a reason for hiding this comment

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

Please wait for the full integration checks to go through.

@tpdownes tpdownes assigned nick-stroud and unassigned tpdownes Jun 2, 2022
@nick-stroud nick-stroud merged commit 0b27251 into GoogleCloudPlatform:develop Jun 2, 2022
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