-
Notifications
You must be signed in to change notification settings - Fork 315
Allow Feature
specification in compute resources
#6962
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
base: develop
Are you sure you want to change the base?
Conversation
Thanks for contributing to ParallelCluster! |
}, | ||
"ComputeResource": { | ||
"Global": ["cpus", "features", "gres", "nodeaddr", "nodehostname", "nodename", "state", "weight"], | ||
"Global": ["cpus", "gres", "nodeaddr", "nodehostname", "nodename", "state", "weight"], |
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.
[Testing] This change is missing coprresponding changes in unit tests and integration tests.
The reason for missing unit test is that CustomSlurmSettingsValidator
takes as input the deny list and we unit test here that a generic deny list given as input is honored.
To improve the testing we need to extend the unit test here to verify that the validator is called with the expected deny list.
The feature captured by this PR and aws/aws-parallelcluster-cookbook#3020, which is its counterpart in the cookbook, requires the extension of the existing end to end test here
We will evaluate internally if we can consider the integration tests out of scope for this PR and be us taking care of it. Will get back to this by tomorrow eod of the day.
Also, have you manually tested this PR together with with aws/aws-parallelcluster-cookbook#3020, which is its required counterpart in the cookbook?
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.
[Documentation] this change requires an entry in our changelog. Can you please add
3.15.0
------
**ENHANCEMENTS**
- Allow specification of `Features` as custom Slurm setting for compute resources.
Description of changes
This change adds support for
Feature
specification forComputeResource
Tests
Manual test:
Feature
parameter is specified inScheduling.SlurmQueues[].ComputeResources[].CustomSlurmSettings
pcluster
command(s) to validate it - should go through.References
This PR is complimentary to a PR in cookbooks repo: aws/aws-parallelcluster-cookbook#3020 - please merge that PR first!
Closes #7039
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.