[develop] upgrade slurm #5695
[develop] upgrade slurm #5695
Conversation
| @@ -1749,7 +1749,7 @@ def _gpu_resource_check(slurm_commands, partition, instance_type, instance_type_ | |||
| def _test_slurm_version(remote_command_executor): | |||
There was a problem hiding this comment.
Could we remove this slurm version check from the CLI and keep it just in the kitchen tests?
This will avoid us to create a patch for the CLI everytime we're updating slum version.
There was a problem hiding this comment.
I completely agree!
We discussed this many times and for a reason or another always postponed the removal of this check.
I'll take this opportunity to do it.
There was a problem hiding this comment.
And I'll go even further and remove also this check
# Since we're installing PMIx v4.2.6, we expect to see pmix and pmix_v4 in the output.
...
assert_that(mpi_list_output).matches(r"\s+pmix_v4($|\s+)")
since we are checking and validating this in the cookbook.
There was a problem hiding this comment.
We may still need the patch for the Changelog.
NSsirena
force-pushed
the
dev/upgrade-slurm
branch
from
c4d5feb to
7170c21
Compare
Codecov ReportAll modified lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #5695 +/- ##
========================================
Coverage 89.95% 89.95%
========================================
Files 180 180
Lines 15526 15526
========================================
Hits 13966 13966
Misses 1560 1560
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Signed-off-by: Nicola Sirena <[email protected]>
NSsirena
force-pushed
the
dev/upgrade-slurm
branch
from
7170c21 to
9ab0c18
Compare
NSsirena
added
the
skip-changelog-update
| # Ensure the expected PMIx version is listed when running `srun --mpi=list`. | ||
| # Since we're installing PMIx v3.1.5, we expect to see pmix and pmix_v3 in the output. | ||
| # Sample output: | ||
| # [ec2-user@ip-172-31-33-187 ~]$ srun 2>&1 --mpi=list | ||
| # srun: MPI types are... | ||
| # srun: none | ||
| # srun: openmpi | ||
| # srun: pmi2 | ||
| # srun: pmix | ||
| # srun: pmix_v3 | ||
| # srun: pmix_vX | ||
| # | ||
| # _vX is the Major number of the PMIx version installed and used to compile slurm. | ||
| # We check this in the cookbook, so we do not repeat the check here | ||
| mpi_list_output = remote_command_executor.run_remote_command("srun 2>&1 --mpi=list").stdout | ||
| assert_that(mpi_list_output).matches(r"\s+pmix($|\s+)") |
There was a problem hiding this comment.
I would say that these are also canary-like tests and we don't have to test them here. But that's probably something for the future.
Description of changes
Porting in develop of the fix about PMIx and the slurm upgrade made in Release-3.7 branch.
After discussing the PR internally we decided to remove the version check from the integration tests since we already test and validate the versions in the Cookbook package.
This will also avoid and extra PR for each version upgrade.
Tests
References
Checklist
developadd the branch name as prefix in the PR title (e.g.[release-3.6]).Please review the guidelines for contributing and Pull Request Instructions.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.