-
Notifications
You must be signed in to change notification settings - Fork 315
[Subnet Prioritization] Support capacity-optimized-prioritized and prioritized Allocation Strategy #6865
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
[Subnet Prioritization] Support capacity-optimized-prioritized and prioritized Allocation Strategy #6865
Changes from all commits
bcb3346
bd77a73
96d1e01
1360560
8cf10aa
7f303dd
26ddfec
13d8cfa
f8de4a5
c7889cd
b6506cf
a3b7672
33d3fd1
406d5e8
dee33ca
f2a0c7e
2e94485
d2c2ba6
db7a9c5
99773c4
ce299af
aa81737
24129ff
444859a
09c0170
5117270
ca4f2e7
c033811
48ccdd0
1f37961
80ab65b
aafcfdf
cc57784
08e1901
f0b36fb
aed81d6
ed7725c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -221,17 +221,22 @@ class InstancesAllocationStrategyValidator(Validator, _FlexibleInstanceTypesVali | |
| """Confirm Allocation Strategy matches with the Capacity Type.""" | ||
|
|
||
| def _validate(self, compute_resource_name: str, capacity_type: Enum, allocation_strategy: Enum, **kwargs): | ||
| """On-demand Capacity type only supports "lowest-price" allocation strategy.""" | ||
| """On-demand Capacity type only supports "lowest-price" and "prioritized" allocation strategy.""" | ||
| valid_on_demand_allocation_strategy = { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [CodeStyle] This is a constant dictionary, let's defined it in a common script, ideally where the constants about strategies are defined. There is not reason to redefine it every time this function gets called. |
||
| cluster_config.AllocationStrategy.LOWEST_PRICE, | ||
| cluster_config.AllocationStrategy.PRIORITIZED, | ||
| } | ||
| if ( | ||
| capacity_type == cluster_config.CapacityType.ONDEMAND | ||
| and allocation_strategy | ||
| and allocation_strategy != cluster_config.AllocationStrategy.LOWEST_PRICE | ||
| and allocation_strategy not in valid_on_demand_allocation_strategy | ||
| ): | ||
| alloc_strategy_msg = allocation_strategy.value if allocation_strategy else "not set" | ||
| self._add_failure( | ||
| f"Compute Resource {compute_resource_name} is using an OnDemand CapacityType but the Allocation " | ||
| f"Strategy specified is {alloc_strategy_msg}. OnDemand CapacityType can only use '" | ||
| f"{cluster_config.AllocationStrategy.LOWEST_PRICE.value}' allocation strategy.", | ||
| f"Compute Resource {compute_resource_name} is using an OnDemand CapacityType but " | ||
| f"the Allocation Strategy specified is {alloc_strategy_msg}. OnDemand CapacityType can only use '" | ||
| f"{cluster_config.AllocationStrategy.LOWEST_PRICE.value}' or " | ||
| f"'{cluster_config.AllocationStrategy.PRIORITIZED.value}' allocation strategy.", | ||
| FailureLevel.ERROR, | ||
| ) | ||
| if capacity_type == cluster_config.CapacityType.CAPACITY_BLOCK and allocation_strategy: | ||
|
|
@@ -241,6 +246,19 @@ def _validate(self, compute_resource_name: str, capacity_type: Enum, allocation_ | |
| "When using CAPACITY_BLOCK CapacityType, allocation strategy should not be set.", | ||
| FailureLevel.ERROR, | ||
| ) | ||
| if ( | ||
| capacity_type == cluster_config.CapacityType.SPOT | ||
| and allocation_strategy == cluster_config.AllocationStrategy.PRIORITIZED | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could take the chance here to generalize the check the same way we did for on demand, i,.e,. define in a constant the valid strategies for spot and check against that constant. |
||
| ): | ||
| self._add_failure( | ||
| f"Compute Resource {compute_resource_name} is using a SPOT CapacityType but the " | ||
| f"Allocation Strategy specified is {allocation_strategy.value}. SPOT CapacityType can only use " | ||
| f"'{cluster_config.AllocationStrategy.LOWEST_PRICE.value}', " | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Better to define the valid strategies for spot into a constant and concatenate those into the message |
||
| f"'{cluster_config.AllocationStrategy.CAPACITY_OPTIMIZED.value}', " | ||
| f"'{cluster_config.AllocationStrategy.PRICE_CAPACITY_OPTIMIZED.value}' " | ||
| f"or '{cluster_config.AllocationStrategy.CAPACITY_OPTIMIZED_PRIORITIZED.value}' allocation strategy.", | ||
| FailureLevel.ERROR, | ||
| ) | ||
|
|
||
|
|
||
| class InstancesMemorySchedulingWarningValidator(Validator): | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,6 +39,7 @@ Scheduling: | |
| - Name: compute_resource2 | ||
| InstanceType: c4.2xlarge | ||
| - Name: queue2 | ||
| AllocationStrategy: "prioritized" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: double quotes are not required in yaml |
||
| Networking: | ||
| SubnetIds: | ||
| - subnet-23456789 | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| Image: | ||
| Os: {{ os }} | ||
| HeadNode: | ||
| SharedStorageType: {{ shared_headnode_storage_type }} | ||
| InstanceType: {{ instance }} | ||
| Networking: | ||
| SubnetId: {{ public_subnet_id }} | ||
| Ssh: | ||
| KeyName: {{ key_name }} | ||
| Scheduling: | ||
| Scheduler: slurm | ||
| SlurmQueues: | ||
| - Name: queue1 | ||
| CapacityType: ONDEMAND | ||
| AllocationStrategy: prioritized | ||
| ComputeResources: | ||
| - Name: queue1-i1 | ||
| Instances: | ||
| - InstanceType: {{ instance }} | ||
| MinCount: 0 | ||
| MaxCount: 10 | ||
| Networking: | ||
| SubnetIds: | ||
| - {{ public_subnet_ids[0] }} | ||
| - {{ public_subnet_ids[1] }} | ||
| - Name: queue2 | ||
| CapacityType: SPOT | ||
| AllocationStrategy: capacity-optimized-prioritized | ||
| ComputeResources: | ||
| - Name: queue2-i1 | ||
| Instances: | ||
| - InstanceType: {{ instance }} | ||
| MinCount: 0 | ||
| MaxCount: 10 | ||
| Networking: | ||
| SubnetIds: | ||
| - {{ public_subnet_ids[0] }} | ||
| - {{ public_subnet_ids[1] }} |
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.
Same as https://github.com/aws/aws-parallelcluster-node/pull/671/files#r2205253483