Skip to content
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

Untangle exclusive and enable_placement, update examples. #3362

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

mr0re1
Copy link
Collaborator

@mr0re1 mr0re1 commented Dec 7, 2024

Motivations:

  • Enable usage of placements with dense reservations.

  • Encourage usage of placements by making them to always work regardless of nodeset configuration.

Done:
Remove constraints:

  • "If a reservation is specified, var.enable_placement must be false" - dense reservations support external placements, non-dense reservation will ignore Slurm-created placement.

  • "If any non-static nodesets has enable_placement, var.exclusive must be set true" - nodeset-wise (non-exclusive) placement can be used with dynamic nodes.

Remove enable_placement: false from SlurmV6 examples.

Reasons for users to set enable_placement=false:

  • If non-dense reservation is used, user can avoid extra-cost of creating placement policies;
  • If customer wants to avoid "all or nothing" VM provisioning behaviour;
  • If customer wants to intentionally have "spread" VMs (e.g. for reliability reasons)

@mr0re1 mr0re1 added the release-chore To not include into release notes label Dec 7, 2024
@mr0re1 mr0re1 requested a review from alyssa-sm December 7, 2024 17:29
@mr0re1 mr0re1 changed the title Relax constraint around reservation and static nodesets with placement Untangle exclusive and enable_placement, update examples. Dec 7, 2024
@mr0re1 mr0re1 requested review from tpdownes and cboneti December 7, 2024 20:41
@alyssa-sm
Copy link
Contributor

There is a error in PR-validation with one of the validate_golden_copy failing that should be addressed but otherwise LGTM

@alyssa-sm alyssa-sm assigned mr0re1 and unassigned alyssa-sm Dec 10, 2024
@mr0re1 mr0re1 force-pushed the res_pp branch 2 times, most recently from ebde2b9 to 955dc44 Compare January 10, 2025 21:44
@mr0re1 mr0re1 assigned cboneti and unassigned mr0re1 Jan 10, 2025
@mr0re1 mr0re1 added the do-not-merge Block merging of this PR label Jan 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge Block merging of this PR release-chore To not include into release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants