From bcb3346c77ffb7c9cd0175f16857121978f0bd93 Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Tue, 3 Jun 2025 14:56:29 -0400 Subject: [PATCH 01/29] [Subnet Prioritization] Add prioritized|capacity-optimized-prioritized to AllocationStrategy configuration; Add new configuration of enable_single_availability_zone; Signed-off-by: Hanxuan Zhang --- cli/src/pcluster/config/cluster_config.py | 5 ++++- cli/src/pcluster/schemas/cluster_schema.py | 1 + cli/src/pcluster/validators/instances_validators.py | 13 +++++++++++-- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/cli/src/pcluster/config/cluster_config.py b/cli/src/pcluster/config/cluster_config.py index cdac31d589..b87e2a0b96 100644 --- a/cli/src/pcluster/config/cluster_config.py +++ b/cli/src/pcluster/config/cluster_config.py @@ -813,10 +813,11 @@ def __init__(self, subnet_ids: List[str], assign_public_ip: str = None, **kwargs class SlurmQueueNetworking(_QueueNetworking): """Represent the networking configuration for the slurm Queue.""" - def __init__(self, placement_group: PlacementGroup = None, proxy: Proxy = None, **kwargs): + def __init__(self, placement_group: PlacementGroup = None, proxy: Proxy = None, enable_single_availability_zone: bool = None, **kwargs): super().__init__(**kwargs) self.placement_group = placement_group or PlacementGroup(implied=True) self.proxy = proxy + self.enable_single_availability_zone = enable_single_availability_zone class AwsBatchQueueNetworking(_QueueNetworking): @@ -2572,6 +2573,8 @@ class AllocationStrategy(Enum): LOWEST_PRICE = "lowest-price" CAPACITY_OPTIMIZED = "capacity-optimized" PRICE_CAPACITY_OPTIMIZED = "price-capacity-optimized" + PRIORITIZED = "prioritized" + CAPACITY_OPTIMIZED_PRIORITIZED = "capacity-optimized-prioritized" class SlurmQueue(_CommonQueue): diff --git a/cli/src/pcluster/schemas/cluster_schema.py b/cli/src/pcluster/schemas/cluster_schema.py index e14e345767..2ac736b880 100644 --- a/cli/src/pcluster/schemas/cluster_schema.py +++ b/cli/src/pcluster/schemas/cluster_schema.py @@ -752,6 +752,7 @@ class SlurmQueueNetworkingSchema(QueueNetworkingSchema): PlacementGroupSchema, metadata={"update_policy": UpdatePolicy.MANAGED_PLACEMENT_GROUP} ) proxy = fields.Nested(QueueProxySchema, metadata={"update_policy": UpdatePolicy.QUEUE_UPDATE_STRATEGY}) + enable_single_availability_zone = fields.Bool(metadata={"update_policy": UpdatePolicy.MANAGED_FSX}) @post_load def make_resource(self, data, **kwargs): diff --git a/cli/src/pcluster/validators/instances_validators.py b/cli/src/pcluster/validators/instances_validators.py index 80215aa3d2..9c84609bfd 100644 --- a/cli/src/pcluster/validators/instances_validators.py +++ b/cli/src/pcluster/validators/instances_validators.py @@ -221,17 +221,18 @@ 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.""" if ( capacity_type == cluster_config.CapacityType.ONDEMAND and allocation_strategy and allocation_strategy != cluster_config.AllocationStrategy.LOWEST_PRICE + and allocation_strategy != cluster_config.AllocationStrategy.PRIORITIZED ): 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"{cluster_config.AllocationStrategy.LOWEST_PRICE.value}' or '{cluster_config.AllocationStrategy.PRIORITIZED.value}' allocation strategy.", FailureLevel.ERROR, ) if capacity_type == cluster_config.CapacityType.CAPACITY_BLOCK and allocation_strategy: @@ -241,6 +242,14 @@ 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: + 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 should use " + f"'{cluster_config.AllocationStrategy.CAPACITY_OPTIMIZED_PRIORITIZED.value}' instead of '{cluster_config.AllocationStrategy.PRIORITIZED.value}' " + f"allocation strategy.", + FailureLevel.ERROR, + ) class InstancesMemorySchedulingWarningValidator(Validator): From 136056084d283b8e6c80502bd87c1cce84018cd4 Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Thu, 5 Jun 2025 16:10:33 -0400 Subject: [PATCH 02/29] [Subnet Prioritization] Add test cases for instance allocation strategy validator --- .../validators/instances_validators.py | 5 +-- .../validators/test_instances_validators.py | 41 +++++++++++++++---- 2 files changed, 36 insertions(+), 10 deletions(-) diff --git a/cli/src/pcluster/validators/instances_validators.py b/cli/src/pcluster/validators/instances_validators.py index 9c84609bfd..aa42714e6b 100644 --- a/cli/src/pcluster/validators/instances_validators.py +++ b/cli/src/pcluster/validators/instances_validators.py @@ -245,9 +245,8 @@ def _validate(self, compute_resource_name: str, capacity_type: Enum, allocation_ if capacity_type == cluster_config.CapacityType.SPOT and allocation_strategy == cluster_config.AllocationStrategy.PRIORITIZED: 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 should use " - f"'{cluster_config.AllocationStrategy.CAPACITY_OPTIMIZED_PRIORITIZED.value}' instead of '{cluster_config.AllocationStrategy.PRIORITIZED.value}' " - f"allocation strategy.", + f"Allocation Strategy specified is {allocation_strategy.value}. SPOT CapacityType cannot use " + f"'{cluster_config.AllocationStrategy.PRIORITIZED.value}' allocation strategy.", FailureLevel.ERROR, ) diff --git a/cli/tests/pcluster/validators/test_instances_validators.py b/cli/tests/pcluster/validators/test_instances_validators.py index e35d571fa6..6c3698b25a 100644 --- a/cli/tests/pcluster/validators/test_instances_validators.py +++ b/cli/tests/pcluster/validators/test_instances_validators.py @@ -591,31 +591,46 @@ def test_instances_networking_validator( CapacityType.ONDEMAND, AllocationStrategy.CAPACITY_OPTIMIZED, "Compute Resource TestComputeResource is using an OnDemand CapacityType but the Allocation Strategy " - "specified is capacity-optimized. OnDemand CapacityType can only use 'lowest-price' allocation strategy.", + "specified is capacity-optimized. " + "OnDemand CapacityType can only use 'lowest-price' or 'prioritized' allocation strategy.", ), ( CapacityType.ONDEMAND, AllocationStrategy.PRICE_CAPACITY_OPTIMIZED, "Compute Resource TestComputeResource is using an OnDemand CapacityType but the Allocation Strategy " "specified is price-capacity-optimized. " - "OnDemand CapacityType can only use 'lowest-price' allocation strategy.", + "OnDemand CapacityType can only use 'lowest-price' or 'prioritized' allocation strategy.", ), + ( + CapacityType.ONDEMAND, + AllocationStrategy.CAPACITY_OPTIMIZED_PRIORITIZED, + f"Compute Resource TestComputeResource is using an OnDemand CapacityType but the Allocation " + f"Strategy specified is capacity-optimized-prioritized. " + f"OnDemand CapacityType can only use 'lowest-price' or 'prioritized' allocation strategy.", + ), + (CapacityType.ONDEMAND, AllocationStrategy.PRIORITIZED, ""), (CapacityType.ONDEMAND, AllocationStrategy.LOWEST_PRICE, ""), (CapacityType.ONDEMAND, None, ""), # Spot Capacity type supports both "lowest-price" and "capacity-optimized" allocation strategy (CapacityType.SPOT, AllocationStrategy.LOWEST_PRICE, ""), (CapacityType.SPOT, AllocationStrategy.CAPACITY_OPTIMIZED, ""), (CapacityType.SPOT, AllocationStrategy.PRICE_CAPACITY_OPTIMIZED, ""), + (CapacityType.SPOT, AllocationStrategy.CAPACITY_OPTIMIZED_PRIORITIZED, ""), (CapacityType.SPOT, None, ""), + ( + CapacityType.SPOT, + AllocationStrategy.PRIORITIZED, + f"Compute Resource TestComputeResource is using a SPOT CapacityType but the " + f"Allocation Strategy specified is prioritized. SPOT CapacityType cannot use " + f"'prioritized' allocation strategy.", + ), # Capacity Block type supports does not support any allocation strategy ( CapacityType.CAPACITY_BLOCK, AllocationStrategy.CAPACITY_OPTIMIZED, - ( - "Compute Resource TestComputeResource is using a CAPACITY_BLOCK CapacityType but the Allocation " - "Strategy specified is capacity-optimized. When using CAPACITY_BLOCK CapacityType, " - "allocation strategy should not be set." - ), + "Compute Resource TestComputeResource is using a CAPACITY_BLOCK CapacityType but the Allocation " + "Strategy specified is capacity-optimized. When using CAPACITY_BLOCK CapacityType, " + "allocation strategy should not be set." ), ( CapacityType.CAPACITY_BLOCK, @@ -632,6 +647,18 @@ def test_instances_networking_validator( "Compute Resource TestComputeResource is using a CAPACITY_BLOCK CapacityType but the Allocation Strategy " "specified is lowest-price. When using CAPACITY_BLOCK CapacityType, allocation strategy should not be set.", ), + ( + CapacityType.CAPACITY_BLOCK, + AllocationStrategy.PRIORITIZED, + "Compute Resource TestComputeResource is using a CAPACITY_BLOCK CapacityType but the Allocation Strategy " + "specified is prioritized. When using CAPACITY_BLOCK CapacityType, allocation strategy should not be set.", + ), + ( + CapacityType.CAPACITY_BLOCK, + AllocationStrategy.CAPACITY_OPTIMIZED_PRIORITIZED, + "Compute Resource TestComputeResource is using a CAPACITY_BLOCK CapacityType but the Allocation Strategy " + "specified is capacity-optimized-prioritized. When using CAPACITY_BLOCK CapacityType, allocation strategy should not be set.", + ), (CapacityType.CAPACITY_BLOCK, None, ""), ], ) From 8cf10aacf0925267fcfd68a67098300f4e947527 Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Thu, 5 Jun 2025 16:13:08 -0400 Subject: [PATCH 03/29] [Subnet Prioritization] Update the default value and update policy of enable_single_availability_zone --- cli/src/pcluster/config/cluster_config.py | 2 +- cli/src/pcluster/schemas/cluster_schema.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cli/src/pcluster/config/cluster_config.py b/cli/src/pcluster/config/cluster_config.py index 461b31ba8d..132fc192b5 100644 --- a/cli/src/pcluster/config/cluster_config.py +++ b/cli/src/pcluster/config/cluster_config.py @@ -813,7 +813,7 @@ def __init__(self, subnet_ids: List[str], assign_public_ip: str = None, **kwargs class SlurmQueueNetworking(_QueueNetworking): """Represent the networking configuration for the slurm Queue.""" - def __init__(self, placement_group: PlacementGroup = None, proxy: Proxy = None, enable_single_availability_zone: bool = None, **kwargs): + def __init__(self, placement_group: PlacementGroup = None, proxy: Proxy = None, enable_single_availability_zone: bool = False, **kwargs): super().__init__(**kwargs) self.placement_group = placement_group or PlacementGroup(implied=True) self.proxy = proxy diff --git a/cli/src/pcluster/schemas/cluster_schema.py b/cli/src/pcluster/schemas/cluster_schema.py index 2ac736b880..efd62c2cab 100644 --- a/cli/src/pcluster/schemas/cluster_schema.py +++ b/cli/src/pcluster/schemas/cluster_schema.py @@ -752,7 +752,7 @@ class SlurmQueueNetworkingSchema(QueueNetworkingSchema): PlacementGroupSchema, metadata={"update_policy": UpdatePolicy.MANAGED_PLACEMENT_GROUP} ) proxy = fields.Nested(QueueProxySchema, metadata={"update_policy": UpdatePolicy.QUEUE_UPDATE_STRATEGY}) - enable_single_availability_zone = fields.Bool(metadata={"update_policy": UpdatePolicy.MANAGED_FSX}) + enable_single_availability_zone = fields.Bool(metadata={"update_policy": UpdatePolicy.QUEUE_UPDATE_STRATEGY}) @post_load def make_resource(self, data, **kwargs): From 7f303ddc30ee6e0c4a661d79247a8cb3199fcfce Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Fri, 6 Jun 2025 16:13:56 -0400 Subject: [PATCH 04/29] [Subnet Prioritization] Move AllocationStrategy Enum from pcluster.config.cluster_config to pcluster.config.common --- cli/src/pcluster/config/cluster_config.py | 16 ++++++++-------- cli/src/pcluster/config/common.py | 8 ++++++++ cli/src/pcluster/schemas/cluster_schema.py | 3 +-- .../pcluster/validators/instances_validators.py | 12 ++++++------ .../validators/test_instances_validators.py | 3 ++- 5 files changed, 25 insertions(+), 17 deletions(-) diff --git a/cli/src/pcluster/config/cluster_config.py b/cli/src/pcluster/config/cluster_config.py index 132fc192b5..4d10e355f7 100644 --- a/cli/src/pcluster/config/cluster_config.py +++ b/cli/src/pcluster/config/cluster_config.py @@ -2566,14 +2566,14 @@ def _register_validators(self, context: ValidatorContext = None): ) -class AllocationStrategy(Enum): - """Define supported allocation strategies.""" - - LOWEST_PRICE = "lowest-price" - CAPACITY_OPTIMIZED = "capacity-optimized" - PRICE_CAPACITY_OPTIMIZED = "price-capacity-optimized" - PRIORITIZED = "prioritized" - CAPACITY_OPTIMIZED_PRIORITIZED = "capacity-optimized-prioritized" +# class AllocationStrategy(Enum): +# """Define supported allocation strategies.""" +# +# LOWEST_PRICE = "lowest-price" +# CAPACITY_OPTIMIZED = "capacity-optimized" +# PRICE_CAPACITY_OPTIMIZED = "price-capacity-optimized" +# PRIORITIZED = "prioritized" +# CAPACITY_OPTIMIZED_PRIORITIZED = "capacity-optimized-prioritized" class SlurmQueue(_CommonQueue): diff --git a/cli/src/pcluster/config/common.py b/cli/src/pcluster/config/common.py index cf7f54323c..3e8557728e 100644 --- a/cli/src/pcluster/config/common.py +++ b/cli/src/pcluster/config/common.py @@ -27,6 +27,14 @@ LOGGER = logging.getLogger(__name__) +class AllocationStrategy(Enum): + """Define supported allocation strategies.""" + + LOWEST_PRICE = "lowest-price" + CAPACITY_OPTIMIZED = "capacity-optimized" + PRICE_CAPACITY_OPTIMIZED = "price-capacity-optimized" + PRIORITIZED = "prioritized" + CAPACITY_OPTIMIZED_PRIORITIZED = "capacity-optimized-prioritized" class CapacityType(Enum): """Enum to identify the type compute supported by the queues.""" diff --git a/cli/src/pcluster/schemas/cluster_schema.py b/cli/src/pcluster/schemas/cluster_schema.py index efd62c2cab..c9c4279262 100644 --- a/cli/src/pcluster/schemas/cluster_schema.py +++ b/cli/src/pcluster/schemas/cluster_schema.py @@ -24,7 +24,6 @@ from pcluster.config.cluster_config import ( AdditionalPackages, Alarms, - AllocationStrategy, AmiSearchFilters, AwsBatchClusterConfig, AwsBatchComputeResource, @@ -95,7 +94,7 @@ SlurmSettings, Timeouts, ) -from pcluster.config.common import BaseTag, CapacityType, DefaultUserHomeType +from pcluster.config.common import BaseTag, CapacityType, DefaultUserHomeType, AllocationStrategy from pcluster.config.update_policy import UpdatePolicy from pcluster.constants import ( DELETION_POLICIES, diff --git a/cli/src/pcluster/validators/instances_validators.py b/cli/src/pcluster/validators/instances_validators.py index aa42714e6b..80ba21f3e7 100644 --- a/cli/src/pcluster/validators/instances_validators.py +++ b/cli/src/pcluster/validators/instances_validators.py @@ -12,7 +12,7 @@ from typing import Callable, Dict from pcluster.aws.aws_resources import InstanceTypeInfo -from pcluster.config import cluster_config +from pcluster.config import cluster_config, common from pcluster.constants import MIN_MEMORY_ABSOLUTE_DIFFERENCE, MIN_MEMORY_PRECENTAGE_DIFFERENCE from pcluster.validators.common import FailureLevel, Validator @@ -225,14 +225,14 @@ def _validate(self, compute_resource_name: str, capacity_type: Enum, allocation_ if ( capacity_type == cluster_config.CapacityType.ONDEMAND and allocation_strategy - and allocation_strategy != cluster_config.AllocationStrategy.LOWEST_PRICE - and allocation_strategy != cluster_config.AllocationStrategy.PRIORITIZED + and allocation_strategy != common.AllocationStrategy.LOWEST_PRICE + and allocation_strategy != common.AllocationStrategy.PRIORITIZED ): 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}' or '{cluster_config.AllocationStrategy.PRIORITIZED.value}' allocation strategy.", + f"{common.AllocationStrategy.LOWEST_PRICE.value}' or '{common.AllocationStrategy.PRIORITIZED.value}' allocation strategy.", FailureLevel.ERROR, ) if capacity_type == cluster_config.CapacityType.CAPACITY_BLOCK and allocation_strategy: @@ -242,11 +242,11 @@ 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: + if capacity_type == cluster_config.CapacityType.SPOT and allocation_strategy == common.AllocationStrategy.PRIORITIZED: 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 cannot use " - f"'{cluster_config.AllocationStrategy.PRIORITIZED.value}' allocation strategy.", + f"'{common.AllocationStrategy.PRIORITIZED.value}' allocation strategy.", FailureLevel.ERROR, ) diff --git a/cli/tests/pcluster/validators/test_instances_validators.py b/cli/tests/pcluster/validators/test_instances_validators.py index 6c3698b25a..93c5f206bc 100644 --- a/cli/tests/pcluster/validators/test_instances_validators.py +++ b/cli/tests/pcluster/validators/test_instances_validators.py @@ -14,7 +14,8 @@ import pytest from pcluster.aws.aws_resources import InstanceTypeInfo -from pcluster.config.cluster_config import AllocationStrategy, CapacityType +from pcluster.config.cluster_config import CapacityType +from pcluster.config.common import AllocationStrategy from pcluster.validators.instances_validators import ( InstancesAcceleratorsValidator, InstancesAllocationStrategyValidator, From 26ddfec4417b3a6e8232f8e9fd3468536980217f Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Fri, 6 Jun 2025 16:20:58 -0400 Subject: [PATCH 05/29] [Subnet Prioritization] Add validator and validator test for enable_single_availability_zone --- .../validators/networking_validators.py | 18 +++++ .../validators/test_networking_validators.py | 71 +++++++++++++++++++ 2 files changed, 89 insertions(+) diff --git a/cli/src/pcluster/validators/networking_validators.py b/cli/src/pcluster/validators/networking_validators.py index d5c9b59b45..2cbf360a23 100644 --- a/cli/src/pcluster/validators/networking_validators.py +++ b/cli/src/pcluster/validators/networking_validators.py @@ -8,12 +8,14 @@ # or in the "LICENSE.txt" file accompanying this file. This file is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES # OR CONDITIONS OF ANY KIND, express or implied. See the License for the specific language governing permissions and # limitations under the License. +from enum import Enum from collections import Counter from typing import List, Union from pcluster.aws.aws_api import AWSApi from pcluster.aws.common import AWSClientError from pcluster.validators.common import FailureLevel, Validator +from pcluster.config import common class SecurityGroupsValidator(Validator): @@ -63,6 +65,22 @@ def _validate(self, subnet_ids: List[str]): except AWSClientError as e: self._add_failure(str(e), FailureLevel.ERROR) +class EnableSingleAvailabilityZoneValidator(Validator): + """ + Single Availability Zone validator. + + Check that enable_single_availability_zone should be used with prioritized + or capacity-optimized-prioritized Allocation Strategy + """ + def _validate(self, allocation_strategy: Enum, enable_single_availability_zone: bool): + if enable_single_availability_zone == True: + if (allocation_strategy != common.AllocationStrategy.PRIORITIZED and + allocation_strategy != common.AllocationStrategy.CAPACITY_OPTIMIZED_PRIORITIZED): + self._add_failure(f"Enable_single_availability_zone is specified as " + f"'{enable_single_availability_zone}' while allocation_strategy is specified as " + f"'{allocation_strategy.value}'. Enable_single_availability_zone should only be used with " + f"prioritized or capacity-optimized-prioritized Allocation Strategy.", + FailureLevel.ERROR) class QueueSubnetsValidator(Validator): """ diff --git a/cli/tests/pcluster/validators/test_networking_validators.py b/cli/tests/pcluster/validators/test_networking_validators.py index ce3f7624c3..90600725a1 100644 --- a/cli/tests/pcluster/validators/test_networking_validators.py +++ b/cli/tests/pcluster/validators/test_networking_validators.py @@ -22,9 +22,11 @@ SecurityGroupsValidator, SingleInstanceTypeSubnetValidator, SubnetsValidator, + EnableSingleAvailabilityZoneValidator, ) from tests.pcluster.aws.dummy_aws_api import mock_aws_api from tests.pcluster.validators.utils import assert_failure_messages +from pcluster.config.common import AllocationStrategy def test_ec2_security_group_validator(mocker): @@ -64,6 +66,75 @@ def test_ec2_subnet_id_validator(mocker): actual_failures = SubnetsValidator().execute(["subnet-12345678", "subnet-23456789"]) assert_failure_messages(actual_failures, None) +@pytest.mark.parametrize( + "allocation_strategy, enable_single_availability_zone, failure_message", + [ + ( + AllocationStrategy.LOWEST_PRICE, + False, + None + ), + ( + AllocationStrategy.LOWEST_PRICE, + True, + f"Enable_single_availability_zone is specified as " + f"'True' while allocation_strategy is specified as " + f"'lowest-price'. Enable_single_availability_zone should only be used with " + f"prioritized or capacity-optimized-prioritized Allocation Strategy." + ), + ( + AllocationStrategy.CAPACITY_OPTIMIZED, + False, + None + ), + ( + AllocationStrategy.CAPACITY_OPTIMIZED, + True, + f"Enable_single_availability_zone is specified as " + f"'True' while allocation_strategy is specified as " + f"'capacity-optimized'. Enable_single_availability_zone should only be used with " + f"prioritized or capacity-optimized-prioritized Allocation Strategy." + ), + ( + AllocationStrategy.PRICE_CAPACITY_OPTIMIZED, + False, + None + ), + ( + AllocationStrategy.PRICE_CAPACITY_OPTIMIZED, + True, + f"Enable_single_availability_zone is specified as " + f"'True' while allocation_strategy is specified as " + f"'price-capacity-optimized'. Enable_single_availability_zone should only be used with " + f"prioritized or capacity-optimized-prioritized Allocation Strategy." + ), + ( + AllocationStrategy.PRIORITIZED, + False, + None + ), + ( + AllocationStrategy.PRIORITIZED, + True, + None + ), + ( + AllocationStrategy.CAPACITY_OPTIMIZED_PRIORITIZED, + False, + None + ), + ( + AllocationStrategy.CAPACITY_OPTIMIZED_PRIORITIZED, + True, + None + ), + ] +) +def test_enable_single_availability_zone_validator(allocation_strategy, enable_single_availability_zone, failure_message): + actual_failures = EnableSingleAvailabilityZoneValidator().execute( + allocation_strategy, enable_single_availability_zone + ) + assert_failure_messages(actual_failures, failure_message) @pytest.mark.parametrize( "queue_name, queue_subnets, subnet_id_az_mapping, failure_message", From 13d8cfae7f7b5f6a95b4fa9cc6a6787e528fc835 Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Fri, 6 Jun 2025 16:31:10 -0400 Subject: [PATCH 06/29] [Subnet Prioritization] Move AllocationStrategy Enum from cluster_config.py to common.py --- cli/src/pcluster/config/cluster_config.py | 12 +----------- cli/src/pcluster/config/common.py | 2 ++ 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/cli/src/pcluster/config/cluster_config.py b/cli/src/pcluster/config/cluster_config.py index 4d10e355f7..2d8d8fd625 100644 --- a/cli/src/pcluster/config/cluster_config.py +++ b/cli/src/pcluster/config/cluster_config.py @@ -29,7 +29,7 @@ BaseDevSettings, BaseTag, CapacityType, - DefaultUserHomeType, + DefaultUserHomeType, AllocationStrategy, ) from pcluster.config.common import Imds as TopLevelImds from pcluster.config.common import ( @@ -2566,16 +2566,6 @@ def _register_validators(self, context: ValidatorContext = None): ) -# class AllocationStrategy(Enum): -# """Define supported allocation strategies.""" -# -# LOWEST_PRICE = "lowest-price" -# CAPACITY_OPTIMIZED = "capacity-optimized" -# PRICE_CAPACITY_OPTIMIZED = "price-capacity-optimized" -# PRIORITIZED = "prioritized" -# CAPACITY_OPTIMIZED_PRIORITIZED = "capacity-optimized-prioritized" - - class SlurmQueue(_CommonQueue): """Represents a Slurm Queue that has Compute Resources with both Single and Multiple Instance Types.""" diff --git a/cli/src/pcluster/config/common.py b/cli/src/pcluster/config/common.py index 3e8557728e..ddb7c4e65b 100644 --- a/cli/src/pcluster/config/common.py +++ b/cli/src/pcluster/config/common.py @@ -27,6 +27,7 @@ LOGGER = logging.getLogger(__name__) + class AllocationStrategy(Enum): """Define supported allocation strategies.""" @@ -36,6 +37,7 @@ class AllocationStrategy(Enum): PRIORITIZED = "prioritized" CAPACITY_OPTIMIZED_PRIORITIZED = "capacity-optimized-prioritized" + class CapacityType(Enum): """Enum to identify the type compute supported by the queues.""" From f8de4a5d86ca67b3c08933aec42aa4ecf44125ed Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Fri, 6 Jun 2025 17:20:05 -0400 Subject: [PATCH 07/29] Revert "[Subnet Prioritization] Move AllocationStrategy Enum from cluster_config.py to common.py" This reverts commit 13d8cfae7f7b5f6a95b4fa9cc6a6787e528fc835. --- cli/src/pcluster/config/cluster_config.py | 12 +++++++++++- cli/src/pcluster/config/common.py | 2 -- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/cli/src/pcluster/config/cluster_config.py b/cli/src/pcluster/config/cluster_config.py index 2d8d8fd625..4d10e355f7 100644 --- a/cli/src/pcluster/config/cluster_config.py +++ b/cli/src/pcluster/config/cluster_config.py @@ -29,7 +29,7 @@ BaseDevSettings, BaseTag, CapacityType, - DefaultUserHomeType, AllocationStrategy, + DefaultUserHomeType, ) from pcluster.config.common import Imds as TopLevelImds from pcluster.config.common import ( @@ -2566,6 +2566,16 @@ def _register_validators(self, context: ValidatorContext = None): ) +# class AllocationStrategy(Enum): +# """Define supported allocation strategies.""" +# +# LOWEST_PRICE = "lowest-price" +# CAPACITY_OPTIMIZED = "capacity-optimized" +# PRICE_CAPACITY_OPTIMIZED = "price-capacity-optimized" +# PRIORITIZED = "prioritized" +# CAPACITY_OPTIMIZED_PRIORITIZED = "capacity-optimized-prioritized" + + class SlurmQueue(_CommonQueue): """Represents a Slurm Queue that has Compute Resources with both Single and Multiple Instance Types.""" diff --git a/cli/src/pcluster/config/common.py b/cli/src/pcluster/config/common.py index ddb7c4e65b..3e8557728e 100644 --- a/cli/src/pcluster/config/common.py +++ b/cli/src/pcluster/config/common.py @@ -27,7 +27,6 @@ LOGGER = logging.getLogger(__name__) - class AllocationStrategy(Enum): """Define supported allocation strategies.""" @@ -37,7 +36,6 @@ class AllocationStrategy(Enum): PRIORITIZED = "prioritized" CAPACITY_OPTIMIZED_PRIORITIZED = "capacity-optimized-prioritized" - class CapacityType(Enum): """Enum to identify the type compute supported by the queues.""" From c7889cd9beb303cab8581ecafba4ea689cf00f74 Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Fri, 6 Jun 2025 17:47:02 -0400 Subject: [PATCH 08/29] [Subnet Prioritization] Register enable_single_availability_zone_validator in cluster_config.py and add registration tests --- cli/src/pcluster/config/cluster_config.py | 16 +++++++++++++++- .../pcluster/validators/test_all_validators.py | 11 ++++++++++- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/cli/src/pcluster/config/cluster_config.py b/cli/src/pcluster/config/cluster_config.py index 4d10e355f7..0b8bfe99be 100644 --- a/cli/src/pcluster/config/cluster_config.py +++ b/cli/src/pcluster/config/cluster_config.py @@ -29,7 +29,7 @@ BaseDevSettings, BaseTag, CapacityType, - DefaultUserHomeType, + DefaultUserHomeType, AllocationStrategy, ) from pcluster.config.common import Imds as TopLevelImds from pcluster.config.common import ( @@ -190,6 +190,7 @@ SecurityGroupsValidator, SingleInstanceTypeSubnetValidator, SubnetsValidator, + EnableSingleAvailabilityZoneValidator ) from pcluster.validators.s3_validators import ( S3BucketRegionValidator, @@ -2636,6 +2637,14 @@ def _register_validators(self, context: ValidatorContext = None): max_length=MAX_COMPUTE_RESOURCES_PER_QUEUE, resource_name="ComputeResources per Queue", ) + if any( + isinstance(compute_resource, SlurmFlexibleComputeResource) for compute_resource in self.compute_resources + ): + self._register_validator( + EnableSingleAvailabilityZoneValidator, + allocation_strategy=self.allocation_strategy, + enable_single_availability_zone=self.networking.enable_single_availability_zone, + ) self._register_validator( QueueSubnetsValidator, queue_name=self.name, @@ -3182,6 +3191,11 @@ def _register_validators(self, context: ValidatorContext = None): # noqa: C901 ] for validator in flexible_instance_types_validators: self._register_validator(validator, **validator_args) + self._register_validator( + EnableSingleAvailabilityZoneValidator, + allocation_strategy=queue.allocation_strategy, + enable_single_availability_zone=queue.networking.enable_single_availability_zone, + ) self._register_validator( ComputeResourceTagsValidator, queue_name=queue.name, diff --git a/cli/tests/pcluster/validators/test_all_validators.py b/cli/tests/pcluster/validators/test_all_validators.py index 9e5c413bab..9269fff9d8 100644 --- a/cli/tests/pcluster/validators/test_all_validators.py +++ b/cli/tests/pcluster/validators/test_all_validators.py @@ -13,7 +13,7 @@ from assertpy import assert_that from pcluster.aws.aws_resources import ImageInfo -from pcluster.config.common import CapacityType +from pcluster.config.common import CapacityType, AllocationStrategy from pcluster.schemas.cluster_schema import ClusterSchema from pcluster.utils import load_yaml_dict from pcluster.validators import ( @@ -224,6 +224,9 @@ def test_slurm_validators_are_called_with_correct_argument(test_datadir, mocker) single_instance_type_subnet_validator = mocker.patch( networking_validators + ".SingleInstanceTypeSubnetValidator._validate", return_value=[] ) + enable_single_availability_zone_validator = mocker.patch( + networking_validators + ".EnableSingleAvailabilityZoneValidator._validate", return_value=[] + ) fsx_validators = validators_path + ".fsx_validators" fsx_s3_validator = mocker.patch(fsx_validators + ".FsxS3Validator._validate", return_value=[]) @@ -334,6 +337,12 @@ def test_slurm_validators_are_called_with_correct_argument(test_datadir, mocker) any_order=True, ) subnets_validator.assert_has_calls([call(subnet_ids=["subnet-12345678", "subnet-23456789", "subnet-12345678"])]) + enable_single_availability_zone_validator.assert_has_calls( + [ + call(allocation_strategy=AllocationStrategy.LOWEST_PRICE, enable_single_availability_zone=False) + ], + any_order=True, + ) single_instance_type_subnet_validator.assert_has_calls( [ call(queue_name="queue1", subnet_ids=["subnet-23456789"]), From b6506cff47fe400652cb903b733d36c2fd01bf66 Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Mon, 9 Jun 2025 14:36:49 -0400 Subject: [PATCH 09/29] [Subnet Prioritization] Change default value of enable_single_availability_zone from False to None --- cli/src/pcluster/config/cluster_config.py | 2 +- cli/tests/pcluster/validators/test_all_validators.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cli/src/pcluster/config/cluster_config.py b/cli/src/pcluster/config/cluster_config.py index 0b8bfe99be..2b959c5ede 100644 --- a/cli/src/pcluster/config/cluster_config.py +++ b/cli/src/pcluster/config/cluster_config.py @@ -814,7 +814,7 @@ def __init__(self, subnet_ids: List[str], assign_public_ip: str = None, **kwargs class SlurmQueueNetworking(_QueueNetworking): """Represent the networking configuration for the slurm Queue.""" - def __init__(self, placement_group: PlacementGroup = None, proxy: Proxy = None, enable_single_availability_zone: bool = False, **kwargs): + def __init__(self, placement_group: PlacementGroup = None, proxy: Proxy = None, enable_single_availability_zone: bool = None, **kwargs): super().__init__(**kwargs) self.placement_group = placement_group or PlacementGroup(implied=True) self.proxy = proxy diff --git a/cli/tests/pcluster/validators/test_all_validators.py b/cli/tests/pcluster/validators/test_all_validators.py index 9269fff9d8..79e93fce50 100644 --- a/cli/tests/pcluster/validators/test_all_validators.py +++ b/cli/tests/pcluster/validators/test_all_validators.py @@ -339,7 +339,7 @@ def test_slurm_validators_are_called_with_correct_argument(test_datadir, mocker) subnets_validator.assert_has_calls([call(subnet_ids=["subnet-12345678", "subnet-23456789", "subnet-12345678"])]) enable_single_availability_zone_validator.assert_has_calls( [ - call(allocation_strategy=AllocationStrategy.LOWEST_PRICE, enable_single_availability_zone=False) + call(allocation_strategy=AllocationStrategy.LOWEST_PRICE, enable_single_availability_zone=None) ], any_order=True, ) From a3b7672a2ca5cf3c1cae9c72c5d82a6aaacb8b3c Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Mon, 9 Jun 2025 14:37:27 -0400 Subject: [PATCH 10/29] [Subnet Prioritization] Add enable_single_availability_zone parameter to slurm.full_config.snapshot.yaml --- .../slurm.full_config.snapshot.yaml | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/cli/tests/pcluster/templates/test_cluster_stack/test_cluster_config_limits/slurm.full_config.snapshot.yaml b/cli/tests/pcluster/templates/test_cluster_stack/test_cluster_config_limits/slurm.full_config.snapshot.yaml index eddb82b155..d6580bef26 100644 --- a/cli/tests/pcluster/templates/test_cluster_stack/test_cluster_config_limits/slurm.full_config.snapshot.yaml +++ b/cli/tests/pcluster/templates/test_cluster_stack/test_cluster_config_limits/slurm.full_config.snapshot.yaml @@ -219,6 +219,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -297,6 +298,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -375,6 +377,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -453,6 +456,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -531,6 +535,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -609,6 +614,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -687,6 +693,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -765,6 +772,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -843,6 +851,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -921,6 +930,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -999,6 +1009,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -1077,6 +1088,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -1155,6 +1167,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -1233,6 +1246,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -1311,6 +1325,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -1389,6 +1404,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -1467,6 +1483,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -1545,6 +1562,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -1623,6 +1641,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -1701,6 +1720,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -1779,6 +1799,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -1857,6 +1878,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -1935,6 +1957,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -2013,6 +2036,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -2091,6 +2115,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -2158,6 +2183,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -2226,6 +2252,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -2294,6 +2321,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -2362,6 +2390,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -2430,6 +2459,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -2498,6 +2528,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -2566,6 +2597,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -2634,6 +2666,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -2702,6 +2735,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -2770,6 +2804,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -2838,6 +2873,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -2906,6 +2942,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -2974,6 +3011,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -3042,6 +3080,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -3110,6 +3149,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -3178,6 +3218,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -3246,6 +3287,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -3314,6 +3356,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -3382,6 +3425,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -3450,6 +3494,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -3518,6 +3563,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -3586,6 +3632,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -3654,6 +3701,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -3722,6 +3770,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -3790,6 +3839,7 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true + EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String From 406d5e8c88cc323edbe75bbdeef8a5c887cd0aad Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Wed, 11 Jun 2025 15:55:52 -0400 Subject: [PATCH 11/29] [Subnet Prioritization] Fix format issues --- cli/src/pcluster/config/cluster_config.py | 15 +++- cli/src/pcluster/config/common.py | 2 + cli/src/pcluster/schemas/cluster_schema.py | 2 +- .../validators/instances_validators.py | 12 ++- .../validators/networking_validators.py | 27 +++--- .../validators/test_all_validators.py | 6 +- .../validators/test_instances_validators.py | 25 +++--- .../validators/test_networking_validators.py | 90 +++++++------------ 8 files changed, 87 insertions(+), 92 deletions(-) diff --git a/cli/src/pcluster/config/cluster_config.py b/cli/src/pcluster/config/cluster_config.py index 2b959c5ede..168032ee84 100644 --- a/cli/src/pcluster/config/cluster_config.py +++ b/cli/src/pcluster/config/cluster_config.py @@ -25,11 +25,12 @@ from pcluster.aws.common import AWSClientError, get_region from pcluster.config.common import ( AdditionalIamPolicy, + AllocationStrategy, BaseDeploymentSettings, BaseDevSettings, BaseTag, CapacityType, - DefaultUserHomeType, AllocationStrategy, + DefaultUserHomeType, ) from pcluster.config.common import Imds as TopLevelImds from pcluster.config.common import ( @@ -185,12 +186,12 @@ from pcluster.validators.monitoring_validators import DetailedMonitoringValidator, LogRotationValidator from pcluster.validators.networking_validators import ( ElasticIpValidator, + EnableSingleAvailabilityZoneValidator, MultiAzPlacementGroupValidator, QueueSubnetsValidator, SecurityGroupsValidator, SingleInstanceTypeSubnetValidator, SubnetsValidator, - EnableSingleAvailabilityZoneValidator ) from pcluster.validators.s3_validators import ( S3BucketRegionValidator, @@ -814,7 +815,13 @@ def __init__(self, subnet_ids: List[str], assign_public_ip: str = None, **kwargs class SlurmQueueNetworking(_QueueNetworking): """Represent the networking configuration for the slurm Queue.""" - def __init__(self, placement_group: PlacementGroup = None, proxy: Proxy = None, enable_single_availability_zone: bool = None, **kwargs): + def __init__( + self, + placement_group: PlacementGroup = None, + proxy: Proxy = None, + enable_single_availability_zone: bool = None, + **kwargs, + ): super().__init__(**kwargs) self.placement_group = placement_group or PlacementGroup(implied=True) self.proxy = proxy @@ -2638,7 +2645,7 @@ def _register_validators(self, context: ValidatorContext = None): resource_name="ComputeResources per Queue", ) if any( - isinstance(compute_resource, SlurmFlexibleComputeResource) for compute_resource in self.compute_resources + isinstance(compute_resource, SlurmFlexibleComputeResource) for compute_resource in self.compute_resources ): self._register_validator( EnableSingleAvailabilityZoneValidator, diff --git a/cli/src/pcluster/config/common.py b/cli/src/pcluster/config/common.py index 3e8557728e..ddb7c4e65b 100644 --- a/cli/src/pcluster/config/common.py +++ b/cli/src/pcluster/config/common.py @@ -27,6 +27,7 @@ LOGGER = logging.getLogger(__name__) + class AllocationStrategy(Enum): """Define supported allocation strategies.""" @@ -36,6 +37,7 @@ class AllocationStrategy(Enum): PRIORITIZED = "prioritized" CAPACITY_OPTIMIZED_PRIORITIZED = "capacity-optimized-prioritized" + class CapacityType(Enum): """Enum to identify the type compute supported by the queues.""" diff --git a/cli/src/pcluster/schemas/cluster_schema.py b/cli/src/pcluster/schemas/cluster_schema.py index c9c4279262..0d21c8f927 100644 --- a/cli/src/pcluster/schemas/cluster_schema.py +++ b/cli/src/pcluster/schemas/cluster_schema.py @@ -94,7 +94,7 @@ SlurmSettings, Timeouts, ) -from pcluster.config.common import BaseTag, CapacityType, DefaultUserHomeType, AllocationStrategy +from pcluster.config.common import AllocationStrategy, BaseTag, CapacityType, DefaultUserHomeType from pcluster.config.update_policy import UpdatePolicy from pcluster.constants import ( DELETION_POLICIES, diff --git a/cli/src/pcluster/validators/instances_validators.py b/cli/src/pcluster/validators/instances_validators.py index 80ba21f3e7..33bcc2ef4d 100644 --- a/cli/src/pcluster/validators/instances_validators.py +++ b/cli/src/pcluster/validators/instances_validators.py @@ -230,9 +230,10 @@ def _validate(self, compute_resource_name: str, capacity_type: Enum, allocation_ ): 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"{common.AllocationStrategy.LOWEST_PRICE.value}' or '{common.AllocationStrategy.PRIORITIZED.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"{common.AllocationStrategy.LOWEST_PRICE.value}' or '{common.AllocationStrategy.PRIORITIZED.value}' " + "allocation strategy.", FailureLevel.ERROR, ) if capacity_type == cluster_config.CapacityType.CAPACITY_BLOCK and allocation_strategy: @@ -242,7 +243,10 @@ 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 == common.AllocationStrategy.PRIORITIZED: + if ( + capacity_type == cluster_config.CapacityType.SPOT + and allocation_strategy == common.AllocationStrategy.PRIORITIZED + ): 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 cannot use " diff --git a/cli/src/pcluster/validators/networking_validators.py b/cli/src/pcluster/validators/networking_validators.py index 2cbf360a23..997b9ed009 100644 --- a/cli/src/pcluster/validators/networking_validators.py +++ b/cli/src/pcluster/validators/networking_validators.py @@ -8,14 +8,14 @@ # or in the "LICENSE.txt" file accompanying this file. This file is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES # OR CONDITIONS OF ANY KIND, express or implied. See the License for the specific language governing permissions and # limitations under the License. -from enum import Enum from collections import Counter +from enum import Enum from typing import List, Union from pcluster.aws.aws_api import AWSApi from pcluster.aws.common import AWSClientError -from pcluster.validators.common import FailureLevel, Validator from pcluster.config import common +from pcluster.validators.common import FailureLevel, Validator class SecurityGroupsValidator(Validator): @@ -65,6 +65,7 @@ def _validate(self, subnet_ids: List[str]): except AWSClientError as e: self._add_failure(str(e), FailureLevel.ERROR) + class EnableSingleAvailabilityZoneValidator(Validator): """ Single Availability Zone validator. @@ -72,15 +73,21 @@ class EnableSingleAvailabilityZoneValidator(Validator): Check that enable_single_availability_zone should be used with prioritized or capacity-optimized-prioritized Allocation Strategy """ + def _validate(self, allocation_strategy: Enum, enable_single_availability_zone: bool): - if enable_single_availability_zone == True: - if (allocation_strategy != common.AllocationStrategy.PRIORITIZED and - allocation_strategy != common.AllocationStrategy.CAPACITY_OPTIMIZED_PRIORITIZED): - self._add_failure(f"Enable_single_availability_zone is specified as " - f"'{enable_single_availability_zone}' while allocation_strategy is specified as " - f"'{allocation_strategy.value}'. Enable_single_availability_zone should only be used with " - f"prioritized or capacity-optimized-prioritized Allocation Strategy.", - FailureLevel.ERROR) + if enable_single_availability_zone: + if ( + allocation_strategy != common.AllocationStrategy.PRIORITIZED + and allocation_strategy != common.AllocationStrategy.CAPACITY_OPTIMIZED_PRIORITIZED + ): + self._add_failure( + "Enable_single_availability_zone is specified as " + f"'{enable_single_availability_zone}' while allocation_strategy is specified as " + f"'{allocation_strategy.value}'. Enable_single_availability_zone should only be used with " + "prioritized or capacity-optimized-prioritized Allocation Strategy.", + FailureLevel.ERROR, + ) + class QueueSubnetsValidator(Validator): """ diff --git a/cli/tests/pcluster/validators/test_all_validators.py b/cli/tests/pcluster/validators/test_all_validators.py index 79e93fce50..59c4c8210f 100644 --- a/cli/tests/pcluster/validators/test_all_validators.py +++ b/cli/tests/pcluster/validators/test_all_validators.py @@ -13,7 +13,7 @@ from assertpy import assert_that from pcluster.aws.aws_resources import ImageInfo -from pcluster.config.common import CapacityType, AllocationStrategy +from pcluster.config.common import AllocationStrategy, CapacityType from pcluster.schemas.cluster_schema import ClusterSchema from pcluster.utils import load_yaml_dict from pcluster.validators import ( @@ -338,9 +338,7 @@ def test_slurm_validators_are_called_with_correct_argument(test_datadir, mocker) ) subnets_validator.assert_has_calls([call(subnet_ids=["subnet-12345678", "subnet-23456789", "subnet-12345678"])]) enable_single_availability_zone_validator.assert_has_calls( - [ - call(allocation_strategy=AllocationStrategy.LOWEST_PRICE, enable_single_availability_zone=None) - ], + [call(allocation_strategy=AllocationStrategy.LOWEST_PRICE, enable_single_availability_zone=None)], any_order=True, ) single_instance_type_subnet_validator.assert_has_calls( diff --git a/cli/tests/pcluster/validators/test_instances_validators.py b/cli/tests/pcluster/validators/test_instances_validators.py index 93c5f206bc..0d017c4751 100644 --- a/cli/tests/pcluster/validators/test_instances_validators.py +++ b/cli/tests/pcluster/validators/test_instances_validators.py @@ -603,11 +603,11 @@ def test_instances_networking_validator( "OnDemand CapacityType can only use 'lowest-price' or 'prioritized' allocation strategy.", ), ( - CapacityType.ONDEMAND, - AllocationStrategy.CAPACITY_OPTIMIZED_PRIORITIZED, - f"Compute Resource TestComputeResource is using an OnDemand CapacityType but the Allocation " - f"Strategy specified is capacity-optimized-prioritized. " - f"OnDemand CapacityType can only use 'lowest-price' or 'prioritized' allocation strategy.", + CapacityType.ONDEMAND, + AllocationStrategy.CAPACITY_OPTIMIZED_PRIORITIZED, + "Compute Resource TestComputeResource is using an OnDemand CapacityType but the Allocation " + "Strategy specified is capacity-optimized-prioritized. " + "OnDemand CapacityType can only use 'lowest-price' or 'prioritized' allocation strategy.", ), (CapacityType.ONDEMAND, AllocationStrategy.PRIORITIZED, ""), (CapacityType.ONDEMAND, AllocationStrategy.LOWEST_PRICE, ""), @@ -619,11 +619,11 @@ def test_instances_networking_validator( (CapacityType.SPOT, AllocationStrategy.CAPACITY_OPTIMIZED_PRIORITIZED, ""), (CapacityType.SPOT, None, ""), ( - CapacityType.SPOT, - AllocationStrategy.PRIORITIZED, - f"Compute Resource TestComputeResource is using a SPOT CapacityType but the " - f"Allocation Strategy specified is prioritized. SPOT CapacityType cannot use " - f"'prioritized' allocation strategy.", + CapacityType.SPOT, + AllocationStrategy.PRIORITIZED, + "Compute Resource TestComputeResource is using a SPOT CapacityType but the " + "Allocation Strategy specified is prioritized. SPOT CapacityType cannot use " + "'prioritized' allocation strategy.", ), # Capacity Block type supports does not support any allocation strategy ( @@ -631,7 +631,7 @@ def test_instances_networking_validator( AllocationStrategy.CAPACITY_OPTIMIZED, "Compute Resource TestComputeResource is using a CAPACITY_BLOCK CapacityType but the Allocation " "Strategy specified is capacity-optimized. When using CAPACITY_BLOCK CapacityType, " - "allocation strategy should not be set." + "allocation strategy should not be set.", ), ( CapacityType.CAPACITY_BLOCK, @@ -658,7 +658,8 @@ def test_instances_networking_validator( CapacityType.CAPACITY_BLOCK, AllocationStrategy.CAPACITY_OPTIMIZED_PRIORITIZED, "Compute Resource TestComputeResource is using a CAPACITY_BLOCK CapacityType but the Allocation Strategy " - "specified is capacity-optimized-prioritized. When using CAPACITY_BLOCK CapacityType, allocation strategy should not be set.", + "specified is capacity-optimized-prioritized. When using CAPACITY_BLOCK CapacityType, " + "allocation strategy should not be set.", ), (CapacityType.CAPACITY_BLOCK, None, ""), ], diff --git a/cli/tests/pcluster/validators/test_networking_validators.py b/cli/tests/pcluster/validators/test_networking_validators.py index 90600725a1..624737a286 100644 --- a/cli/tests/pcluster/validators/test_networking_validators.py +++ b/cli/tests/pcluster/validators/test_networking_validators.py @@ -14,19 +14,19 @@ import pytest from pcluster.aws.common import AWSClientError +from pcluster.config.common import AllocationStrategy from pcluster.validators.networking_validators import ( ElasticIpValidator, + EnableSingleAvailabilityZoneValidator, LambdaFunctionsVpcConfigValidator, MultiAzPlacementGroupValidator, QueueSubnetsValidator, SecurityGroupsValidator, SingleInstanceTypeSubnetValidator, SubnetsValidator, - EnableSingleAvailabilityZoneValidator, ) from tests.pcluster.aws.dummy_aws_api import mock_aws_api from tests.pcluster.validators.utils import assert_failure_messages -from pcluster.config.common import AllocationStrategy def test_ec2_security_group_validator(mocker): @@ -66,76 +66,52 @@ def test_ec2_subnet_id_validator(mocker): actual_failures = SubnetsValidator().execute(["subnet-12345678", "subnet-23456789"]) assert_failure_messages(actual_failures, None) + @pytest.mark.parametrize( "allocation_strategy, enable_single_availability_zone, failure_message", [ + (AllocationStrategy.LOWEST_PRICE, False, None), ( - AllocationStrategy.LOWEST_PRICE, - False, - None - ), - ( - AllocationStrategy.LOWEST_PRICE, - True, - f"Enable_single_availability_zone is specified as " - f"'True' while allocation_strategy is specified as " - f"'lowest-price'. Enable_single_availability_zone should only be used with " - f"prioritized or capacity-optimized-prioritized Allocation Strategy." - ), - ( - AllocationStrategy.CAPACITY_OPTIMIZED, - False, - None - ), - ( - AllocationStrategy.CAPACITY_OPTIMIZED, - True, - f"Enable_single_availability_zone is specified as " - f"'True' while allocation_strategy is specified as " - f"'capacity-optimized'. Enable_single_availability_zone should only be used with " - f"prioritized or capacity-optimized-prioritized Allocation Strategy." - ), - ( - AllocationStrategy.PRICE_CAPACITY_OPTIMIZED, - False, - None - ), - ( - AllocationStrategy.PRICE_CAPACITY_OPTIMIZED, - True, - f"Enable_single_availability_zone is specified as " - f"'True' while allocation_strategy is specified as " - f"'price-capacity-optimized'. Enable_single_availability_zone should only be used with " - f"prioritized or capacity-optimized-prioritized Allocation Strategy." - ), - ( - AllocationStrategy.PRIORITIZED, - False, - None - ), - ( - AllocationStrategy.PRIORITIZED, - True, - None + AllocationStrategy.LOWEST_PRICE, + True, + "Enable_single_availability_zone is specified as " + "'True' while allocation_strategy is specified as " + "'lowest-price'. Enable_single_availability_zone should only be used with " + "prioritized or capacity-optimized-prioritized Allocation Strategy.", ), + (AllocationStrategy.CAPACITY_OPTIMIZED, False, None), ( - AllocationStrategy.CAPACITY_OPTIMIZED_PRIORITIZED, - False, - None + AllocationStrategy.CAPACITY_OPTIMIZED, + True, + "Enable_single_availability_zone is specified as " + "'True' while allocation_strategy is specified as " + "'capacity-optimized'. Enable_single_availability_zone should only be used with " + "prioritized or capacity-optimized-prioritized Allocation Strategy.", ), + (AllocationStrategy.PRICE_CAPACITY_OPTIMIZED, False, None), ( - AllocationStrategy.CAPACITY_OPTIMIZED_PRIORITIZED, - True, - None + AllocationStrategy.PRICE_CAPACITY_OPTIMIZED, + True, + "Enable_single_availability_zone is specified as " + "'True' while allocation_strategy is specified as " + "'price-capacity-optimized'. Enable_single_availability_zone should only be used with " + "prioritized or capacity-optimized-prioritized Allocation Strategy.", ), - ] + (AllocationStrategy.PRIORITIZED, False, None), + (AllocationStrategy.PRIORITIZED, True, None), + (AllocationStrategy.CAPACITY_OPTIMIZED_PRIORITIZED, False, None), + (AllocationStrategy.CAPACITY_OPTIMIZED_PRIORITIZED, True, None), + ], ) -def test_enable_single_availability_zone_validator(allocation_strategy, enable_single_availability_zone, failure_message): +def test_enable_single_availability_zone_validator( + allocation_strategy, enable_single_availability_zone, failure_message +): actual_failures = EnableSingleAvailabilityZoneValidator().execute( allocation_strategy, enable_single_availability_zone ) assert_failure_messages(actual_failures, failure_message) + @pytest.mark.parametrize( "queue_name, queue_subnets, subnet_id_az_mapping, failure_message", [ From f2a0c7efb8f87ee9985333ddcdbabc971f48ac17 Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Wed, 11 Jun 2025 15:58:51 -0400 Subject: [PATCH 12/29] [Subnet Prioritization] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d81f8e12e..68ba1a5c9f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ CHANGELOG **CHANGES** - Ubuntu 20.04 is no longer supported. +- Support prioritized and capacity-optimized-prioritized Allocation Strategy **BUG FIXES** - Fix an issue where Security Group validation failed when a rule contained both IPv4 ranges (IpRanges) and security group references (UserIdGroupPairs). From 2e9448561ff2b5596357b25197f88768f5d9bd96 Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Wed, 11 Jun 2025 16:06:04 -0400 Subject: [PATCH 13/29] [Subnet Prioritization] Remove duplicated AllocationStrategy Enum --- cli/src/pcluster/config/cluster_config.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/cli/src/pcluster/config/cluster_config.py b/cli/src/pcluster/config/cluster_config.py index 168032ee84..aee9c4d753 100644 --- a/cli/src/pcluster/config/cluster_config.py +++ b/cli/src/pcluster/config/cluster_config.py @@ -2574,16 +2574,6 @@ def _register_validators(self, context: ValidatorContext = None): ) -# class AllocationStrategy(Enum): -# """Define supported allocation strategies.""" -# -# LOWEST_PRICE = "lowest-price" -# CAPACITY_OPTIMIZED = "capacity-optimized" -# PRICE_CAPACITY_OPTIMIZED = "price-capacity-optimized" -# PRIORITIZED = "prioritized" -# CAPACITY_OPTIMIZED_PRIORITIZED = "capacity-optimized-prioritized" - - class SlurmQueue(_CommonQueue): """Represents a Slurm Queue that has Compute Resources with both Single and Multiple Instance Types.""" From d2c2ba6a1530226dc37e65aa1e878f2f34e0bdd2 Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Wed, 11 Jun 2025 16:06:51 -0400 Subject: [PATCH 14/29] [Subnet Prioritization] Remove duplicated EnableSingleAvailabilityZoneValidator registration --- cli/src/pcluster/config/cluster_config.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/cli/src/pcluster/config/cluster_config.py b/cli/src/pcluster/config/cluster_config.py index aee9c4d753..7f4c7165d5 100644 --- a/cli/src/pcluster/config/cluster_config.py +++ b/cli/src/pcluster/config/cluster_config.py @@ -3188,11 +3188,6 @@ def _register_validators(self, context: ValidatorContext = None): # noqa: C901 ] for validator in flexible_instance_types_validators: self._register_validator(validator, **validator_args) - self._register_validator( - EnableSingleAvailabilityZoneValidator, - allocation_strategy=queue.allocation_strategy, - enable_single_availability_zone=queue.networking.enable_single_availability_zone, - ) self._register_validator( ComputeResourceTagsValidator, queue_name=queue.name, From db7a9c54c4533bde089c82183cfc3bf41223092c Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Wed, 11 Jun 2025 16:17:16 -0400 Subject: [PATCH 15/29] [Subnet Prioritization] Update the failure message of InstancesAllocationStrategyValidator --- cli/src/pcluster/validators/instances_validators.py | 7 +++++-- cli/tests/pcluster/validators/test_instances_validators.py | 7 +++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/cli/src/pcluster/validators/instances_validators.py b/cli/src/pcluster/validators/instances_validators.py index 33bcc2ef4d..4bcc8fe45f 100644 --- a/cli/src/pcluster/validators/instances_validators.py +++ b/cli/src/pcluster/validators/instances_validators.py @@ -249,8 +249,11 @@ def _validate(self, compute_resource_name: str, capacity_type: Enum, allocation_ ): 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 cannot use " - f"'{common.AllocationStrategy.PRIORITIZED.value}' allocation strategy.", + f"Allocation Strategy specified is {allocation_strategy.value}. SPOT CapacityType can only use " + f"'{common.AllocationStrategy.LOWEST_PRICE.value}', " + f"'{common.AllocationStrategy.CAPACITY_OPTIMIZED.value}', " + f"'{common.AllocationStrategy.PRICE_CAPACITY_OPTIMIZED.value}' " + f"or '{common.AllocationStrategy.CAPACITY_OPTIMIZED_PRIORITIZED.value}' allocation strategy.", FailureLevel.ERROR, ) diff --git a/cli/tests/pcluster/validators/test_instances_validators.py b/cli/tests/pcluster/validators/test_instances_validators.py index 0d017c4751..3ffaa0adb5 100644 --- a/cli/tests/pcluster/validators/test_instances_validators.py +++ b/cli/tests/pcluster/validators/test_instances_validators.py @@ -622,8 +622,11 @@ def test_instances_networking_validator( CapacityType.SPOT, AllocationStrategy.PRIORITIZED, "Compute Resource TestComputeResource is using a SPOT CapacityType but the " - "Allocation Strategy specified is prioritized. SPOT CapacityType cannot use " - "'prioritized' allocation strategy.", + "Allocation Strategy specified is prioritized. SPOT CapacityType can only use " + "'lowest-price', " + "'capacity-optimized', " + "'price-capacity-optimized' " + "or 'capacity-optimized-prioritized' allocation strategy.", ), # Capacity Block type supports does not support any allocation strategy ( From 99773c41220e39292a516481b220b75d2545f498 Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Wed, 11 Jun 2025 17:05:40 -0400 Subject: [PATCH 16/29] [Subnet Prioritization] Update enable_single_availability_zone_validator registration test --- cli/tests/pcluster/validators/test_all_validators.py | 5 ++++- .../slurm.yaml | 3 +++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/cli/tests/pcluster/validators/test_all_validators.py b/cli/tests/pcluster/validators/test_all_validators.py index 59c4c8210f..10f0f68a1b 100644 --- a/cli/tests/pcluster/validators/test_all_validators.py +++ b/cli/tests/pcluster/validators/test_all_validators.py @@ -338,7 +338,10 @@ def test_slurm_validators_are_called_with_correct_argument(test_datadir, mocker) ) subnets_validator.assert_has_calls([call(subnet_ids=["subnet-12345678", "subnet-23456789", "subnet-12345678"])]) enable_single_availability_zone_validator.assert_has_calls( - [call(allocation_strategy=AllocationStrategy.LOWEST_PRICE, enable_single_availability_zone=None)], + [ + call(allocation_strategy=AllocationStrategy.PRIORITIZED, enable_single_availability_zone=True), + call(allocation_strategy=None, enable_single_availability_zone=False), + ], any_order=True, ) single_instance_type_subnet_validator.assert_has_calls( diff --git a/cli/tests/pcluster/validators/test_all_validators/test_slurm_validators_are_called_with_correct_argument/slurm.yaml b/cli/tests/pcluster/validators/test_all_validators/test_slurm_validators_are_called_with_correct_argument/slurm.yaml index 2f70a826d2..3a552d7fe0 100644 --- a/cli/tests/pcluster/validators/test_all_validators/test_slurm_validators_are_called_with_correct_argument/slurm.yaml +++ b/cli/tests/pcluster/validators/test_all_validators/test_slurm_validators_are_called_with_correct_argument/slurm.yaml @@ -39,9 +39,11 @@ Scheduling: - Name: compute_resource2 InstanceType: c4.2xlarge - Name: queue2 + AllocationStrategy: "prioritized" Networking: SubnetIds: - subnet-23456789 + EnableSingleAvailabilityZone: true ComputeResources: - Name: compute_resource1 InstanceType: c5.4xlarge @@ -68,6 +70,7 @@ Scheduling: Networking: SubnetIds: - subnet-23456789 + EnableSingleAvailabilityZone: false ComputeResources: - Name: compute_resource1 Instances: From ce299af3a4b59a0c4158bf8dda319a07b5de80ad Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Thu, 12 Jun 2025 14:59:27 -0400 Subject: [PATCH 17/29] [Subnet Prioritization] Update format --- .../validators/networking_validators.py | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/cli/src/pcluster/validators/networking_validators.py b/cli/src/pcluster/validators/networking_validators.py index 997b9ed009..20db9436a7 100644 --- a/cli/src/pcluster/validators/networking_validators.py +++ b/cli/src/pcluster/validators/networking_validators.py @@ -75,18 +75,18 @@ class EnableSingleAvailabilityZoneValidator(Validator): """ def _validate(self, allocation_strategy: Enum, enable_single_availability_zone: bool): - if enable_single_availability_zone: - if ( - allocation_strategy != common.AllocationStrategy.PRIORITIZED - and allocation_strategy != common.AllocationStrategy.CAPACITY_OPTIMIZED_PRIORITIZED - ): - self._add_failure( - "Enable_single_availability_zone is specified as " - f"'{enable_single_availability_zone}' while allocation_strategy is specified as " - f"'{allocation_strategy.value}'. Enable_single_availability_zone should only be used with " - "prioritized or capacity-optimized-prioritized Allocation Strategy.", - FailureLevel.ERROR, - ) + prioritized_allocation_strategies = ( + common.AllocationStrategy.PRIORITIZED, + common.AllocationStrategy.CAPACITY_OPTIMIZED_PRIORITIZED, + ) + if enable_single_availability_zone in prioritized_allocation_strategies: + self._add_failure( + "Enable_single_availability_zone is specified as " + f"'{enable_single_availability_zone}' while allocation_strategy is specified as " + f"'{allocation_strategy.value}'. Enable_single_availability_zone should only be used with " + "prioritized or capacity-optimized-prioritized Allocation Strategy.", + FailureLevel.ERROR, + ) class QueueSubnetsValidator(Validator): From aa81737a19fc8368272fd3f97442f1e5b658637f Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Thu, 12 Jun 2025 15:08:44 -0400 Subject: [PATCH 18/29] [Subnet Prioritization] Fix EnableSingleAvailabilityZoneValidator --- cli/src/pcluster/validators/networking_validators.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/src/pcluster/validators/networking_validators.py b/cli/src/pcluster/validators/networking_validators.py index 20db9436a7..e63c590ba6 100644 --- a/cli/src/pcluster/validators/networking_validators.py +++ b/cli/src/pcluster/validators/networking_validators.py @@ -79,7 +79,7 @@ def _validate(self, allocation_strategy: Enum, enable_single_availability_zone: common.AllocationStrategy.PRIORITIZED, common.AllocationStrategy.CAPACITY_OPTIMIZED_PRIORITIZED, ) - if enable_single_availability_zone in prioritized_allocation_strategies: + if enable_single_availability_zone == True and allocation_strategy not in prioritized_allocation_strategies: self._add_failure( "Enable_single_availability_zone is specified as " f"'{enable_single_availability_zone}' while allocation_strategy is specified as " From 24129fff328abe66668e808e6e2c5eb9454c71a9 Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Thu, 12 Jun 2025 15:12:07 -0400 Subject: [PATCH 19/29] [Subnet Prioritization] Fix format issue --- cli/src/pcluster/validators/networking_validators.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/src/pcluster/validators/networking_validators.py b/cli/src/pcluster/validators/networking_validators.py index e63c590ba6..39b4cf5221 100644 --- a/cli/src/pcluster/validators/networking_validators.py +++ b/cli/src/pcluster/validators/networking_validators.py @@ -79,7 +79,7 @@ def _validate(self, allocation_strategy: Enum, enable_single_availability_zone: common.AllocationStrategy.PRIORITIZED, common.AllocationStrategy.CAPACITY_OPTIMIZED_PRIORITIZED, ) - if enable_single_availability_zone == True and allocation_strategy not in prioritized_allocation_strategies: + if enable_single_availability_zone and allocation_strategy not in prioritized_allocation_strategies: self._add_failure( "Enable_single_availability_zone is specified as " f"'{enable_single_availability_zone}' while allocation_strategy is specified as " From 444859ad53bc83bedff2ee44bd47f6270bf0c63a Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Mon, 30 Jun 2025 12:03:06 -0400 Subject: [PATCH 20/29] [Subnet Prioritization] Add integration test for subnet prioritization --- .../networking/test_cluster_networking.py | 21 +++++++ .../pcluster.config.yaml | 55 +++++++++++++++++++ 2 files changed, 76 insertions(+) create mode 100644 tests/integration-tests/tests/networking/test_cluster_networking/test_cluster_with_subnet_prioritization/pcluster.config.yaml diff --git a/tests/integration-tests/tests/networking/test_cluster_networking.py b/tests/integration-tests/tests/networking/test_cluster_networking.py index c339e9e0d0..681daf02e8 100644 --- a/tests/integration-tests/tests/networking/test_cluster_networking.py +++ b/tests/integration-tests/tests/networking/test_cluster_networking.py @@ -31,6 +31,7 @@ from tests.common.assertions import ( assert_lambda_vpc_settings_are_correct, + assert_msg_in_log, assert_no_errors_in_logs, assert_no_msg_in_logs, wait_for_num_instances_in_cluster, @@ -238,3 +239,23 @@ def _run_mpi_jobs(mpi_variants, remote_command_executor, test_datadir, slurm_com slurm_commands.assert_job_succeeded(job_id) logging.info("Checking cluster has two nodes after running MPI jobs") # 1 static node + 1 dynamic node assert_that(len(get_compute_nodes_instance_ids(cluster.cfn_name, region))).is_equal_to(2) + + +@pytest.mark.usefixtures("instance") +def test_cluster_with_subnet_prioritization( + region, os, pcluster_config_reader, clusters_factory, vpc_stack, scheduler_commands_factory +): + # Create cluster with subnet prioritization + init_config_file = pcluster_config_reader(config_file="pcluster.config.yaml") + cluster = clusters_factory(init_config_file) + + remote_command_executor = RemoteCommandExecutor(cluster) + scheduler_commands = scheduler_commands_factory(remote_command_executor) + + scheduler_commands.submit_command("sleep 60", nodes=10) + wait_for_num_instances_in_cluster(cluster.cfn_name, cluster.region, desired=11) + + slurm_resume_log = "/var/log/parallelcluster/slurm_resume.log" + public_subnets = vpc_stack.get_all_public_subnets() + assert_msg_in_log(remote_command_executor, slurm_resume_log, f"'SubnetId': '{public_subnets[0]}', 'Priority': 0.0") + assert_msg_in_log(remote_command_executor, slurm_resume_log, f"'SubnetId': '{public_subnets[1]}', 'Priority': 1.0") diff --git a/tests/integration-tests/tests/networking/test_cluster_networking/test_cluster_with_subnet_prioritization/pcluster.config.yaml b/tests/integration-tests/tests/networking/test_cluster_networking/test_cluster_with_subnet_prioritization/pcluster.config.yaml new file mode 100644 index 0000000000..aa5b03f930 --- /dev/null +++ b/tests/integration-tests/tests/networking/test_cluster_networking/test_cluster_with_subnet_prioritization/pcluster.config.yaml @@ -0,0 +1,55 @@ +Image: + Os: {{ os }} +HeadNode: + SharedStorageType: {{ shared_headnode_storage_type }} + InstanceType: {{ instance }} + Networking: + SubnetId: {{ public_subnet_id }} + Ssh: + KeyName: {{ key_name }} + Iam: + S3Access: + - BucketName: sa-east-1-pcluster +Scheduling: + Scheduler: slurm + SlurmQueues: + - Name: queue1 + CapacityType: ONDEMAND + AllocationStrategy: prioritized + Iam: + S3Access: + - BucketName: sa-east-1-pcluster + ComputeResources: + - Name: queue1-i1 + Instances: + - InstanceType: {{ instance }} + MinCount: 1 + MaxCount: 10 + Networking: + SubnetIds: + - {{ public_subnet_ids[0] }} + - {{ public_subnet_ids[1] }} + EnableSingleAvailabilityZone: true + - Name: queue2 + CapacityType: SPOT + AllocationStrategy: capacity-optimized-prioritized + Iam: + S3Access: + - BucketName: sa-east-1-pcluster + ComputeResources: + - Name: queue2-i1 + Instances: + - InstanceType: {{ instance }} + MinCount: 1 + MaxCount: 10 + Networking: + SubnetIds: + - {{ public_subnet_ids[0] }} + - {{ public_subnet_ids[1] }} + EnableSingleAvailabilityZone: true +DevSettings: + AmiSearchFilters: + Owner: "447714826191" + NodePackage: s3://sa-east-1-pcluster/parallelcluster/3.14.0/node/aws-parallelcluster-node-3.14.0.tgz + Cookbook: + ChefCookbook: s3://sa-east-1-pcluster/parallelcluster/3.14.0/cookbooks/aws-parallelcluster-cookbook-3.14.0.tgz \ No newline at end of file From 09c0170f6718c1aa378959779d8aabc884bb631c Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Tue, 1 Jul 2025 16:56:05 -0400 Subject: [PATCH 21/29] [Subnet Prioritization] Update integration test for subnet prioritization --- .../tests/networking/test_cluster_networking.py | 11 ++++++----- .../pcluster.config.yaml | 4 ++-- tests/integration-tests/utils.py | 16 ++++++++++++++++ 3 files changed, 24 insertions(+), 7 deletions(-) diff --git a/tests/integration-tests/tests/networking/test_cluster_networking.py b/tests/integration-tests/tests/networking/test_cluster_networking.py index 681daf02e8..ba005d374e 100644 --- a/tests/integration-tests/tests/networking/test_cluster_networking.py +++ b/tests/integration-tests/tests/networking/test_cluster_networking.py @@ -24,6 +24,7 @@ check_pcluster_list_cluster_log_streams, generate_stack_name, get_compute_nodes_instance_ids, + get_compute_nodes_subnet_ids, get_username_for_os, is_dcv_supported, render_jinja_template, @@ -252,10 +253,10 @@ def test_cluster_with_subnet_prioritization( remote_command_executor = RemoteCommandExecutor(cluster) scheduler_commands = scheduler_commands_factory(remote_command_executor) - scheduler_commands.submit_command("sleep 60", nodes=10) - wait_for_num_instances_in_cluster(cluster.cfn_name, cluster.region, desired=11) + scheduler_commands.submit_command("sleep 60", nodes=5) + wait_for_num_instances_in_cluster(cluster.cfn_name, cluster.region, desired=5) - slurm_resume_log = "/var/log/parallelcluster/slurm_resume.log" public_subnets = vpc_stack.get_all_public_subnets() - assert_msg_in_log(remote_command_executor, slurm_resume_log, f"'SubnetId': '{public_subnets[0]}', 'Priority': 0.0") - assert_msg_in_log(remote_command_executor, slurm_resume_log, f"'SubnetId': '{public_subnets[1]}', 'Priority': 1.0") + subnet_ids = get_compute_nodes_subnet_ids(cluster.cfn_name, region) + for subnet_id in subnet_ids: + assert_that(subnet_id).is_equal_to(public_subnets[0]) diff --git a/tests/integration-tests/tests/networking/test_cluster_networking/test_cluster_with_subnet_prioritization/pcluster.config.yaml b/tests/integration-tests/tests/networking/test_cluster_networking/test_cluster_with_subnet_prioritization/pcluster.config.yaml index aa5b03f930..350e90ce97 100644 --- a/tests/integration-tests/tests/networking/test_cluster_networking/test_cluster_with_subnet_prioritization/pcluster.config.yaml +++ b/tests/integration-tests/tests/networking/test_cluster_networking/test_cluster_with_subnet_prioritization/pcluster.config.yaml @@ -23,7 +23,7 @@ Scheduling: - Name: queue1-i1 Instances: - InstanceType: {{ instance }} - MinCount: 1 + MinCount: 0 MaxCount: 10 Networking: SubnetIds: @@ -40,7 +40,7 @@ Scheduling: - Name: queue2-i1 Instances: - InstanceType: {{ instance }} - MinCount: 1 + MinCount: 0 MaxCount: 10 Networking: SubnetIds: diff --git a/tests/integration-tests/utils.py b/tests/integration-tests/utils.py index 17d2d2273a..01c52012f5 100644 --- a/tests/integration-tests/utils.py +++ b/tests/integration-tests/utils.py @@ -364,6 +364,22 @@ def get_cluster_nodes_instance_ids(stack_name, region, instance_types=None, node logging.error("Failed retrieving instance ids with exception: %s", e) raise +def get_compute_nodes_subnet_ids(stack_name, region, instance_types=None, queue_name=None): + """Return a list of cluster Instances Subnet Ids.""" + try: + instances = describe_cluster_instances( + stack_name, + region, + filter_by_node_type="Compute", + filter_by_instance_types=instance_types, + filter_by_queue_name=queue_name, + ) + logging.info("Instance Information: ") + logging.info(instances) + return [instance["SubnetId"] for instance in instances] + except Exception as e: + logging.error("Failed retrieving instance ids with exception: %s", e) + raise def get_compute_nodes_instance_ips(stack_name, region): """Return a list of compute Instances Ip's.""" From 51172703cf284448690af4ec779d03e869f3e922 Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Mon, 7 Jul 2025 14:48:35 -0400 Subject: [PATCH 22/29] [Subnet Prioritization] Remove EnableSingleAvailabilityZone parameter from configuration --- cli/src/pcluster/config/cluster_config.py | 18 +------ cli/src/pcluster/schemas/cluster_schema.py | 1 - .../validators/networking_validators.py | 25 ---------- .../slurm.full_config.snapshot.yaml | 50 ------------------- .../validators/test_all_validators.py | 10 ---- .../slurm.yaml | 2 - .../validators/test_networking_validators.py | 46 ----------------- 7 files changed, 1 insertion(+), 151 deletions(-) diff --git a/cli/src/pcluster/config/cluster_config.py b/cli/src/pcluster/config/cluster_config.py index 7f4c7165d5..adba698199 100644 --- a/cli/src/pcluster/config/cluster_config.py +++ b/cli/src/pcluster/config/cluster_config.py @@ -186,7 +186,6 @@ from pcluster.validators.monitoring_validators import DetailedMonitoringValidator, LogRotationValidator from pcluster.validators.networking_validators import ( ElasticIpValidator, - EnableSingleAvailabilityZoneValidator, MultiAzPlacementGroupValidator, QueueSubnetsValidator, SecurityGroupsValidator, @@ -815,17 +814,10 @@ def __init__(self, subnet_ids: List[str], assign_public_ip: str = None, **kwargs class SlurmQueueNetworking(_QueueNetworking): """Represent the networking configuration for the slurm Queue.""" - def __init__( - self, - placement_group: PlacementGroup = None, - proxy: Proxy = None, - enable_single_availability_zone: bool = None, - **kwargs, - ): + def __init__(self, placement_group: PlacementGroup = None, proxy: Proxy = None, **kwargs): super().__init__(**kwargs) self.placement_group = placement_group or PlacementGroup(implied=True) self.proxy = proxy - self.enable_single_availability_zone = enable_single_availability_zone class AwsBatchQueueNetworking(_QueueNetworking): @@ -2634,14 +2626,6 @@ def _register_validators(self, context: ValidatorContext = None): max_length=MAX_COMPUTE_RESOURCES_PER_QUEUE, resource_name="ComputeResources per Queue", ) - if any( - isinstance(compute_resource, SlurmFlexibleComputeResource) for compute_resource in self.compute_resources - ): - self._register_validator( - EnableSingleAvailabilityZoneValidator, - allocation_strategy=self.allocation_strategy, - enable_single_availability_zone=self.networking.enable_single_availability_zone, - ) self._register_validator( QueueSubnetsValidator, queue_name=self.name, diff --git a/cli/src/pcluster/schemas/cluster_schema.py b/cli/src/pcluster/schemas/cluster_schema.py index 0d21c8f927..00c1d8a749 100644 --- a/cli/src/pcluster/schemas/cluster_schema.py +++ b/cli/src/pcluster/schemas/cluster_schema.py @@ -751,7 +751,6 @@ class SlurmQueueNetworkingSchema(QueueNetworkingSchema): PlacementGroupSchema, metadata={"update_policy": UpdatePolicy.MANAGED_PLACEMENT_GROUP} ) proxy = fields.Nested(QueueProxySchema, metadata={"update_policy": UpdatePolicy.QUEUE_UPDATE_STRATEGY}) - enable_single_availability_zone = fields.Bool(metadata={"update_policy": UpdatePolicy.QUEUE_UPDATE_STRATEGY}) @post_load def make_resource(self, data, **kwargs): diff --git a/cli/src/pcluster/validators/networking_validators.py b/cli/src/pcluster/validators/networking_validators.py index 39b4cf5221..d5c9b59b45 100644 --- a/cli/src/pcluster/validators/networking_validators.py +++ b/cli/src/pcluster/validators/networking_validators.py @@ -9,12 +9,10 @@ # OR CONDITIONS OF ANY KIND, express or implied. See the License for the specific language governing permissions and # limitations under the License. from collections import Counter -from enum import Enum from typing import List, Union from pcluster.aws.aws_api import AWSApi from pcluster.aws.common import AWSClientError -from pcluster.config import common from pcluster.validators.common import FailureLevel, Validator @@ -66,29 +64,6 @@ def _validate(self, subnet_ids: List[str]): self._add_failure(str(e), FailureLevel.ERROR) -class EnableSingleAvailabilityZoneValidator(Validator): - """ - Single Availability Zone validator. - - Check that enable_single_availability_zone should be used with prioritized - or capacity-optimized-prioritized Allocation Strategy - """ - - def _validate(self, allocation_strategy: Enum, enable_single_availability_zone: bool): - prioritized_allocation_strategies = ( - common.AllocationStrategy.PRIORITIZED, - common.AllocationStrategy.CAPACITY_OPTIMIZED_PRIORITIZED, - ) - if enable_single_availability_zone and allocation_strategy not in prioritized_allocation_strategies: - self._add_failure( - "Enable_single_availability_zone is specified as " - f"'{enable_single_availability_zone}' while allocation_strategy is specified as " - f"'{allocation_strategy.value}'. Enable_single_availability_zone should only be used with " - "prioritized or capacity-optimized-prioritized Allocation Strategy.", - FailureLevel.ERROR, - ) - - class QueueSubnetsValidator(Validator): """ Queue Subnets validator. diff --git a/cli/tests/pcluster/templates/test_cluster_stack/test_cluster_config_limits/slurm.full_config.snapshot.yaml b/cli/tests/pcluster/templates/test_cluster_stack/test_cluster_config_limits/slurm.full_config.snapshot.yaml index d6580bef26..eddb82b155 100644 --- a/cli/tests/pcluster/templates/test_cluster_stack/test_cluster_config_limits/slurm.full_config.snapshot.yaml +++ b/cli/tests/pcluster/templates/test_cluster_stack/test_cluster_config_limits/slurm.full_config.snapshot.yaml @@ -219,7 +219,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -298,7 +297,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -377,7 +375,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -456,7 +453,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -535,7 +531,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -614,7 +609,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -693,7 +687,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -772,7 +765,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -851,7 +843,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -930,7 +921,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -1009,7 +999,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -1088,7 +1077,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -1167,7 +1155,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -1246,7 +1233,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -1325,7 +1311,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -1404,7 +1389,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -1483,7 +1467,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -1562,7 +1545,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -1641,7 +1623,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -1720,7 +1701,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -1799,7 +1779,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -1878,7 +1857,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -1957,7 +1935,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -2036,7 +2013,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -2115,7 +2091,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: null - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: null Id: null @@ -2183,7 +2158,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -2252,7 +2226,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -2321,7 +2294,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -2390,7 +2362,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -2459,7 +2430,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -2528,7 +2498,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -2597,7 +2566,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -2666,7 +2634,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -2735,7 +2702,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -2804,7 +2770,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -2873,7 +2838,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -2942,7 +2906,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -3011,7 +2974,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -3080,7 +3042,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -3149,7 +3110,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -3218,7 +3178,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -3287,7 +3246,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -3356,7 +3314,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -3425,7 +3382,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -3494,7 +3450,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -3563,7 +3518,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -3632,7 +3586,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -3701,7 +3654,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -3770,7 +3722,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String @@ -3839,7 +3790,6 @@ Scheduling: Networking: AdditionalSecurityGroups: null AssignPublicIp: true - EnableSingleAvailabilityZone: null PlacementGroup: Enabled: true Id: String diff --git a/cli/tests/pcluster/validators/test_all_validators.py b/cli/tests/pcluster/validators/test_all_validators.py index 10f0f68a1b..59a5761741 100644 --- a/cli/tests/pcluster/validators/test_all_validators.py +++ b/cli/tests/pcluster/validators/test_all_validators.py @@ -224,9 +224,6 @@ def test_slurm_validators_are_called_with_correct_argument(test_datadir, mocker) single_instance_type_subnet_validator = mocker.patch( networking_validators + ".SingleInstanceTypeSubnetValidator._validate", return_value=[] ) - enable_single_availability_zone_validator = mocker.patch( - networking_validators + ".EnableSingleAvailabilityZoneValidator._validate", return_value=[] - ) fsx_validators = validators_path + ".fsx_validators" fsx_s3_validator = mocker.patch(fsx_validators + ".FsxS3Validator._validate", return_value=[]) @@ -337,13 +334,6 @@ def test_slurm_validators_are_called_with_correct_argument(test_datadir, mocker) any_order=True, ) subnets_validator.assert_has_calls([call(subnet_ids=["subnet-12345678", "subnet-23456789", "subnet-12345678"])]) - enable_single_availability_zone_validator.assert_has_calls( - [ - call(allocation_strategy=AllocationStrategy.PRIORITIZED, enable_single_availability_zone=True), - call(allocation_strategy=None, enable_single_availability_zone=False), - ], - any_order=True, - ) single_instance_type_subnet_validator.assert_has_calls( [ call(queue_name="queue1", subnet_ids=["subnet-23456789"]), diff --git a/cli/tests/pcluster/validators/test_all_validators/test_slurm_validators_are_called_with_correct_argument/slurm.yaml b/cli/tests/pcluster/validators/test_all_validators/test_slurm_validators_are_called_with_correct_argument/slurm.yaml index 3a552d7fe0..367507f2ae 100644 --- a/cli/tests/pcluster/validators/test_all_validators/test_slurm_validators_are_called_with_correct_argument/slurm.yaml +++ b/cli/tests/pcluster/validators/test_all_validators/test_slurm_validators_are_called_with_correct_argument/slurm.yaml @@ -43,7 +43,6 @@ Scheduling: Networking: SubnetIds: - subnet-23456789 - EnableSingleAvailabilityZone: true ComputeResources: - Name: compute_resource1 InstanceType: c5.4xlarge @@ -70,7 +69,6 @@ Scheduling: Networking: SubnetIds: - subnet-23456789 - EnableSingleAvailabilityZone: false ComputeResources: - Name: compute_resource1 Instances: diff --git a/cli/tests/pcluster/validators/test_networking_validators.py b/cli/tests/pcluster/validators/test_networking_validators.py index 624737a286..06d136bce6 100644 --- a/cli/tests/pcluster/validators/test_networking_validators.py +++ b/cli/tests/pcluster/validators/test_networking_validators.py @@ -17,7 +17,6 @@ from pcluster.config.common import AllocationStrategy from pcluster.validators.networking_validators import ( ElasticIpValidator, - EnableSingleAvailabilityZoneValidator, LambdaFunctionsVpcConfigValidator, MultiAzPlacementGroupValidator, QueueSubnetsValidator, @@ -67,51 +66,6 @@ def test_ec2_subnet_id_validator(mocker): assert_failure_messages(actual_failures, None) -@pytest.mark.parametrize( - "allocation_strategy, enable_single_availability_zone, failure_message", - [ - (AllocationStrategy.LOWEST_PRICE, False, None), - ( - AllocationStrategy.LOWEST_PRICE, - True, - "Enable_single_availability_zone is specified as " - "'True' while allocation_strategy is specified as " - "'lowest-price'. Enable_single_availability_zone should only be used with " - "prioritized or capacity-optimized-prioritized Allocation Strategy.", - ), - (AllocationStrategy.CAPACITY_OPTIMIZED, False, None), - ( - AllocationStrategy.CAPACITY_OPTIMIZED, - True, - "Enable_single_availability_zone is specified as " - "'True' while allocation_strategy is specified as " - "'capacity-optimized'. Enable_single_availability_zone should only be used with " - "prioritized or capacity-optimized-prioritized Allocation Strategy.", - ), - (AllocationStrategy.PRICE_CAPACITY_OPTIMIZED, False, None), - ( - AllocationStrategy.PRICE_CAPACITY_OPTIMIZED, - True, - "Enable_single_availability_zone is specified as " - "'True' while allocation_strategy is specified as " - "'price-capacity-optimized'. Enable_single_availability_zone should only be used with " - "prioritized or capacity-optimized-prioritized Allocation Strategy.", - ), - (AllocationStrategy.PRIORITIZED, False, None), - (AllocationStrategy.PRIORITIZED, True, None), - (AllocationStrategy.CAPACITY_OPTIMIZED_PRIORITIZED, False, None), - (AllocationStrategy.CAPACITY_OPTIMIZED_PRIORITIZED, True, None), - ], -) -def test_enable_single_availability_zone_validator( - allocation_strategy, enable_single_availability_zone, failure_message -): - actual_failures = EnableSingleAvailabilityZoneValidator().execute( - allocation_strategy, enable_single_availability_zone - ) - assert_failure_messages(actual_failures, failure_message) - - @pytest.mark.parametrize( "queue_name, queue_subnets, subnet_id_az_mapping, failure_message", [ From ca4f2e7ec9c9ee13420922f1cdecd09301feca42 Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Mon, 7 Jul 2025 14:58:15 -0400 Subject: [PATCH 23/29] [Subnet Prioritization] Update Integration Test --- .../tests/networking/test_cluster_networking.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/integration-tests/tests/networking/test_cluster_networking.py b/tests/integration-tests/tests/networking/test_cluster_networking.py index ba005d374e..8d5e95e635 100644 --- a/tests/integration-tests/tests/networking/test_cluster_networking.py +++ b/tests/integration-tests/tests/networking/test_cluster_networking.py @@ -256,7 +256,13 @@ def test_cluster_with_subnet_prioritization( scheduler_commands.submit_command("sleep 60", nodes=5) wait_for_num_instances_in_cluster(cluster.cfn_name, cluster.region, desired=5) + # Check that the CreateFleet request contains priorities for each subnet + slurm_resume_log = "/var/log/parallelcluster/slurm_resume.log" public_subnets = vpc_stack.get_all_public_subnets() + assert_msg_in_log(remote_command_executor, slurm_resume_log, f"'SubnetId': '{public_subnets[0]}', 'Priority': 0.0") + assert_msg_in_log(remote_command_executor, slurm_resume_log, f"'SubnetId': '{public_subnets[1]}', 'Priority': 1.0") + + # Check that all instances are launched in the subnet with the highest priority subnet_ids = get_compute_nodes_subnet_ids(cluster.cfn_name, region) for subnet_id in subnet_ids: assert_that(subnet_id).is_equal_to(public_subnets[0]) From c033811d6308107b97763519bde3dc55d0fbc9c9 Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Mon, 7 Jul 2025 15:20:27 -0400 Subject: [PATCH 24/29] [Subnet Prioritization] Fix format issues --- cli/tests/pcluster/validators/test_all_validators.py | 2 +- cli/tests/pcluster/validators/test_networking_validators.py | 1 - tests/integration-tests/utils.py | 2 ++ 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/cli/tests/pcluster/validators/test_all_validators.py b/cli/tests/pcluster/validators/test_all_validators.py index 59a5761741..9e5c413bab 100644 --- a/cli/tests/pcluster/validators/test_all_validators.py +++ b/cli/tests/pcluster/validators/test_all_validators.py @@ -13,7 +13,7 @@ from assertpy import assert_that from pcluster.aws.aws_resources import ImageInfo -from pcluster.config.common import AllocationStrategy, CapacityType +from pcluster.config.common import CapacityType from pcluster.schemas.cluster_schema import ClusterSchema from pcluster.utils import load_yaml_dict from pcluster.validators import ( diff --git a/cli/tests/pcluster/validators/test_networking_validators.py b/cli/tests/pcluster/validators/test_networking_validators.py index 06d136bce6..ce3f7624c3 100644 --- a/cli/tests/pcluster/validators/test_networking_validators.py +++ b/cli/tests/pcluster/validators/test_networking_validators.py @@ -14,7 +14,6 @@ import pytest from pcluster.aws.common import AWSClientError -from pcluster.config.common import AllocationStrategy from pcluster.validators.networking_validators import ( ElasticIpValidator, LambdaFunctionsVpcConfigValidator, diff --git a/tests/integration-tests/utils.py b/tests/integration-tests/utils.py index 01c52012f5..bd79c5b66d 100644 --- a/tests/integration-tests/utils.py +++ b/tests/integration-tests/utils.py @@ -364,6 +364,7 @@ def get_cluster_nodes_instance_ids(stack_name, region, instance_types=None, node logging.error("Failed retrieving instance ids with exception: %s", e) raise + def get_compute_nodes_subnet_ids(stack_name, region, instance_types=None, queue_name=None): """Return a list of cluster Instances Subnet Ids.""" try: @@ -381,6 +382,7 @@ def get_compute_nodes_subnet_ids(stack_name, region, instance_types=None, queue_ logging.error("Failed retrieving instance ids with exception: %s", e) raise + def get_compute_nodes_instance_ips(stack_name, region): """Return a list of compute Instances Ip's.""" try: From 48ccdd02e6f6d2dece5a9067d8d769c6288f69ef Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Mon, 7 Jul 2025 15:25:40 -0400 Subject: [PATCH 25/29] [Subnet Prioritization] Remove EnableSingleAvailabilityZone from integration test --- .../pcluster.config.yaml | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/integration-tests/tests/networking/test_cluster_networking/test_cluster_with_subnet_prioritization/pcluster.config.yaml b/tests/integration-tests/tests/networking/test_cluster_networking/test_cluster_with_subnet_prioritization/pcluster.config.yaml index 350e90ce97..eb9281aed7 100644 --- a/tests/integration-tests/tests/networking/test_cluster_networking/test_cluster_with_subnet_prioritization/pcluster.config.yaml +++ b/tests/integration-tests/tests/networking/test_cluster_networking/test_cluster_with_subnet_prioritization/pcluster.config.yaml @@ -29,7 +29,6 @@ Scheduling: SubnetIds: - {{ public_subnet_ids[0] }} - {{ public_subnet_ids[1] }} - EnableSingleAvailabilityZone: true - Name: queue2 CapacityType: SPOT AllocationStrategy: capacity-optimized-prioritized @@ -46,7 +45,6 @@ Scheduling: SubnetIds: - {{ public_subnet_ids[0] }} - {{ public_subnet_ids[1] }} - EnableSingleAvailabilityZone: true DevSettings: AmiSearchFilters: Owner: "447714826191" From 80ab65bbd12470ee8fb7d2c3706d897297f168f8 Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Tue, 8 Jul 2025 11:54:01 -0400 Subject: [PATCH 26/29] [Subnet Prioritization] Move AllocationStrategy Enum from common.py back to cluster_config.py --- cli/src/pcluster/config/cluster_config.py | 11 ++++++++++- cli/src/pcluster/config/common.py | 10 ---------- cli/src/pcluster/schemas/cluster_schema.py | 3 ++- .../validators/instances_validators.py | 18 +++++++++--------- .../validators/test_instances_validators.py | 3 +-- tests/integration-tests/configs/installer.yaml | 10 +++++----- 6 files changed, 27 insertions(+), 28 deletions(-) diff --git a/cli/src/pcluster/config/cluster_config.py b/cli/src/pcluster/config/cluster_config.py index adba698199..39edc2f66b 100644 --- a/cli/src/pcluster/config/cluster_config.py +++ b/cli/src/pcluster/config/cluster_config.py @@ -25,7 +25,6 @@ from pcluster.aws.common import AWSClientError, get_region from pcluster.config.common import ( AdditionalIamPolicy, - AllocationStrategy, BaseDeploymentSettings, BaseDevSettings, BaseTag, @@ -2566,6 +2565,16 @@ def _register_validators(self, context: ValidatorContext = None): ) +class AllocationStrategy(Enum): + """Define supported allocation strategies.""" + + LOWEST_PRICE = "lowest-price" + CAPACITY_OPTIMIZED = "capacity-optimized" + PRICE_CAPACITY_OPTIMIZED = "price-capacity-optimized" + PRIORITIZED = "prioritized" + CAPACITY_OPTIMIZED_PRIORITIZED = "capacity-optimized-prioritized" + + class SlurmQueue(_CommonQueue): """Represents a Slurm Queue that has Compute Resources with both Single and Multiple Instance Types.""" diff --git a/cli/src/pcluster/config/common.py b/cli/src/pcluster/config/common.py index ddb7c4e65b..cf7f54323c 100644 --- a/cli/src/pcluster/config/common.py +++ b/cli/src/pcluster/config/common.py @@ -28,16 +28,6 @@ LOGGER = logging.getLogger(__name__) -class AllocationStrategy(Enum): - """Define supported allocation strategies.""" - - LOWEST_PRICE = "lowest-price" - CAPACITY_OPTIMIZED = "capacity-optimized" - PRICE_CAPACITY_OPTIMIZED = "price-capacity-optimized" - PRIORITIZED = "prioritized" - CAPACITY_OPTIMIZED_PRIORITIZED = "capacity-optimized-prioritized" - - class CapacityType(Enum): """Enum to identify the type compute supported by the queues.""" diff --git a/cli/src/pcluster/schemas/cluster_schema.py b/cli/src/pcluster/schemas/cluster_schema.py index 00c1d8a749..e14e345767 100644 --- a/cli/src/pcluster/schemas/cluster_schema.py +++ b/cli/src/pcluster/schemas/cluster_schema.py @@ -24,6 +24,7 @@ from pcluster.config.cluster_config import ( AdditionalPackages, Alarms, + AllocationStrategy, AmiSearchFilters, AwsBatchClusterConfig, AwsBatchComputeResource, @@ -94,7 +95,7 @@ SlurmSettings, Timeouts, ) -from pcluster.config.common import AllocationStrategy, BaseTag, CapacityType, DefaultUserHomeType +from pcluster.config.common import BaseTag, CapacityType, DefaultUserHomeType from pcluster.config.update_policy import UpdatePolicy from pcluster.constants import ( DELETION_POLICIES, diff --git a/cli/src/pcluster/validators/instances_validators.py b/cli/src/pcluster/validators/instances_validators.py index 4bcc8fe45f..9ce227041b 100644 --- a/cli/src/pcluster/validators/instances_validators.py +++ b/cli/src/pcluster/validators/instances_validators.py @@ -12,7 +12,7 @@ from typing import Callable, Dict from pcluster.aws.aws_resources import InstanceTypeInfo -from pcluster.config import cluster_config, common +from pcluster.config import cluster_config from pcluster.constants import MIN_MEMORY_ABSOLUTE_DIFFERENCE, MIN_MEMORY_PRECENTAGE_DIFFERENCE from pcluster.validators.common import FailureLevel, Validator @@ -225,14 +225,14 @@ def _validate(self, compute_resource_name: str, capacity_type: Enum, allocation_ if ( capacity_type == cluster_config.CapacityType.ONDEMAND and allocation_strategy - and allocation_strategy != common.AllocationStrategy.LOWEST_PRICE - and allocation_strategy != common.AllocationStrategy.PRIORITIZED + and allocation_strategy != cluster_config.AllocationStrategy.LOWEST_PRICE + and allocation_strategy != cluster_config.AllocationStrategy.PRIORITIZED ): 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 " f"the Allocation Strategy specified is {alloc_strategy_msg}. OnDemand CapacityType can only use '" - f"{common.AllocationStrategy.LOWEST_PRICE.value}' or '{common.AllocationStrategy.PRIORITIZED.value}' " + f"{cluster_config.AllocationStrategy.LOWEST_PRICE.value}' or '{cluster_config.AllocationStrategy.PRIORITIZED.value}' " "allocation strategy.", FailureLevel.ERROR, ) @@ -245,15 +245,15 @@ def _validate(self, compute_resource_name: str, capacity_type: Enum, allocation_ ) if ( capacity_type == cluster_config.CapacityType.SPOT - and allocation_strategy == common.AllocationStrategy.PRIORITIZED + and allocation_strategy == cluster_config.AllocationStrategy.PRIORITIZED ): 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"'{common.AllocationStrategy.LOWEST_PRICE.value}', " - f"'{common.AllocationStrategy.CAPACITY_OPTIMIZED.value}', " - f"'{common.AllocationStrategy.PRICE_CAPACITY_OPTIMIZED.value}' " - f"or '{common.AllocationStrategy.CAPACITY_OPTIMIZED_PRIORITIZED.value}' allocation strategy.", + f"'{cluster_config.AllocationStrategy.LOWEST_PRICE.value}', " + 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, ) diff --git a/cli/tests/pcluster/validators/test_instances_validators.py b/cli/tests/pcluster/validators/test_instances_validators.py index 3ffaa0adb5..d4d459bd0b 100644 --- a/cli/tests/pcluster/validators/test_instances_validators.py +++ b/cli/tests/pcluster/validators/test_instances_validators.py @@ -14,8 +14,7 @@ import pytest from pcluster.aws.aws_resources import InstanceTypeInfo -from pcluster.config.cluster_config import CapacityType -from pcluster.config.common import AllocationStrategy +from pcluster.config.cluster_config import AllocationStrategy, CapacityType from pcluster.validators.instances_validators import ( InstancesAcceleratorsValidator, InstancesAllocationStrategyValidator, diff --git a/tests/integration-tests/configs/installer.yaml b/tests/integration-tests/configs/installer.yaml index 8d754d20b5..474011989f 100644 --- a/tests/integration-tests/configs/installer.yaml +++ b/tests/integration-tests/configs/installer.yaml @@ -1,10 +1,10 @@ {%- import 'common.jinja2' as common with context -%} --- test-suites: - cli_commands: - test_cli_commands.py::test_slurm_cli_commands: + networking: + test_cluster_networking.py::test_cluster_with_subnet_prioritization: dimensions: - - regions: [ "ap-northeast-2" ] - instances: {{ common.INSTANCES_DEFAULT_X86 }} - oss: [ "ubuntu2404" ] + - regions: [ "sa-east-1" ] + instances: [ "t3.large" ] + oss: [ "alinux2" ] schedulers: [ "slurm" ] From aafcfdfcbf464e196098f84a08070021868ca0f2 Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Tue, 8 Jul 2025 14:54:09 -0400 Subject: [PATCH 27/29] [Subnet Prioritization] Update integration test and format --- .../validators/instances_validators.py | 4 ++-- .../integration-tests/configs/installer.yaml | 10 ++++---- .../networking/test_cluster_networking.py | 23 +++++++++++-------- tests/integration-tests/utils.py | 6 ++--- 4 files changed, 23 insertions(+), 20 deletions(-) diff --git a/cli/src/pcluster/validators/instances_validators.py b/cli/src/pcluster/validators/instances_validators.py index 9ce227041b..daa01dc99d 100644 --- a/cli/src/pcluster/validators/instances_validators.py +++ b/cli/src/pcluster/validators/instances_validators.py @@ -232,8 +232,8 @@ def _validate(self, compute_resource_name: str, capacity_type: Enum, allocation_ self._add_failure( 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 '{cluster_config.AllocationStrategy.PRIORITIZED.value}' " - "allocation strategy.", + 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: diff --git a/tests/integration-tests/configs/installer.yaml b/tests/integration-tests/configs/installer.yaml index 474011989f..8d754d20b5 100644 --- a/tests/integration-tests/configs/installer.yaml +++ b/tests/integration-tests/configs/installer.yaml @@ -1,10 +1,10 @@ {%- import 'common.jinja2' as common with context -%} --- test-suites: - networking: - test_cluster_networking.py::test_cluster_with_subnet_prioritization: + cli_commands: + test_cli_commands.py::test_slurm_cli_commands: dimensions: - - regions: [ "sa-east-1" ] - instances: [ "t3.large" ] - oss: [ "alinux2" ] + - regions: [ "ap-northeast-2" ] + instances: {{ common.INSTANCES_DEFAULT_X86 }} + oss: [ "ubuntu2404" ] schedulers: [ "slurm" ] diff --git a/tests/integration-tests/tests/networking/test_cluster_networking.py b/tests/integration-tests/tests/networking/test_cluster_networking.py index 8d5e95e635..174b9f869c 100644 --- a/tests/integration-tests/tests/networking/test_cluster_networking.py +++ b/tests/integration-tests/tests/networking/test_cluster_networking.py @@ -13,7 +13,7 @@ import boto3 import pytest -from assertpy import assert_that +from assertpy import assert_that, soft_assertions from cfn_stacks_factory import CfnStack from constants import OSU_BENCHMARK_VERSION, UNSUPPORTED_OSES_FOR_DCV from fabric import Connection @@ -36,6 +36,7 @@ assert_no_errors_in_logs, assert_no_msg_in_logs, wait_for_num_instances_in_cluster, + wait_for_num_instances_in_queue, ) from tests.common.osu_common import compile_osu from tests.common.schedulers_common import SlurmCommands @@ -252,17 +253,21 @@ def test_cluster_with_subnet_prioritization( remote_command_executor = RemoteCommandExecutor(cluster) scheduler_commands = scheduler_commands_factory(remote_command_executor) + public_subnets = vpc_stack.get_all_public_subnets() + queues = ["queue1", "queue2"] + logging.info(f"Public subnets: {public_subnets}") + # Check that all instances are launched in the subnet with the highest priority + with soft_assertions(): + for queue in queues: + scheduler_commands.submit_command("sleep 60", nodes=5, partition=queue) + wait_for_num_instances_in_queue(cluster.cfn_name, cluster.region, desired=5, queue=queue) - scheduler_commands.submit_command("sleep 60", nodes=5) - wait_for_num_instances_in_cluster(cluster.cfn_name, cluster.region, desired=5) + subnet_ids = get_compute_nodes_subnet_ids(cluster.cfn_name, region, node_type="Compute", queue_name=queue) + logging.info(f"Subnets: {subnet_ids}") + for subnet_id in subnet_ids: + assert_that(subnet_id).is_equal_to(public_subnets[0]) # Check that the CreateFleet request contains priorities for each subnet slurm_resume_log = "/var/log/parallelcluster/slurm_resume.log" - public_subnets = vpc_stack.get_all_public_subnets() assert_msg_in_log(remote_command_executor, slurm_resume_log, f"'SubnetId': '{public_subnets[0]}', 'Priority': 0.0") assert_msg_in_log(remote_command_executor, slurm_resume_log, f"'SubnetId': '{public_subnets[1]}', 'Priority': 1.0") - - # Check that all instances are launched in the subnet with the highest priority - subnet_ids = get_compute_nodes_subnet_ids(cluster.cfn_name, region) - for subnet_id in subnet_ids: - assert_that(subnet_id).is_equal_to(public_subnets[0]) diff --git a/tests/integration-tests/utils.py b/tests/integration-tests/utils.py index bd79c5b66d..2ad7bc3bde 100644 --- a/tests/integration-tests/utils.py +++ b/tests/integration-tests/utils.py @@ -365,18 +365,16 @@ def get_cluster_nodes_instance_ids(stack_name, region, instance_types=None, node raise -def get_compute_nodes_subnet_ids(stack_name, region, instance_types=None, queue_name=None): +def get_compute_nodes_subnet_ids(stack_name, region, instance_types=None, node_type=None, queue_name=None): """Return a list of cluster Instances Subnet Ids.""" try: instances = describe_cluster_instances( stack_name, region, - filter_by_node_type="Compute", + filter_by_node_type=node_type, filter_by_instance_types=instance_types, filter_by_queue_name=queue_name, ) - logging.info("Instance Information: ") - logging.info(instances) return [instance["SubnetId"] for instance in instances] except Exception as e: logging.error("Failed retrieving instance ids with exception: %s", e) From 08e19010685051d6e008e6a7137744a6d845191d Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Tue, 8 Jul 2025 15:15:05 -0400 Subject: [PATCH 28/29] [Subnet Prioritization] Remove custom hardcode settings from integration test config file --- .../pcluster.config.yaml | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/tests/integration-tests/tests/networking/test_cluster_networking/test_cluster_with_subnet_prioritization/pcluster.config.yaml b/tests/integration-tests/tests/networking/test_cluster_networking/test_cluster_with_subnet_prioritization/pcluster.config.yaml index eb9281aed7..4be538f180 100644 --- a/tests/integration-tests/tests/networking/test_cluster_networking/test_cluster_with_subnet_prioritization/pcluster.config.yaml +++ b/tests/integration-tests/tests/networking/test_cluster_networking/test_cluster_with_subnet_prioritization/pcluster.config.yaml @@ -7,18 +7,12 @@ HeadNode: SubnetId: {{ public_subnet_id }} Ssh: KeyName: {{ key_name }} - Iam: - S3Access: - - BucketName: sa-east-1-pcluster Scheduling: Scheduler: slurm SlurmQueues: - Name: queue1 CapacityType: ONDEMAND AllocationStrategy: prioritized - Iam: - S3Access: - - BucketName: sa-east-1-pcluster ComputeResources: - Name: queue1-i1 Instances: @@ -32,9 +26,6 @@ Scheduling: - Name: queue2 CapacityType: SPOT AllocationStrategy: capacity-optimized-prioritized - Iam: - S3Access: - - BucketName: sa-east-1-pcluster ComputeResources: - Name: queue2-i1 Instances: @@ -44,10 +35,4 @@ Scheduling: Networking: SubnetIds: - {{ public_subnet_ids[0] }} - - {{ public_subnet_ids[1] }} -DevSettings: - AmiSearchFilters: - Owner: "447714826191" - NodePackage: s3://sa-east-1-pcluster/parallelcluster/3.14.0/node/aws-parallelcluster-node-3.14.0.tgz - Cookbook: - ChefCookbook: s3://sa-east-1-pcluster/parallelcluster/3.14.0/cookbooks/aws-parallelcluster-cookbook-3.14.0.tgz \ No newline at end of file + - {{ public_subnet_ids[1] }} \ No newline at end of file From aed81d69291d3c23c9f37e547e15328b59ff62df Mon Sep 17 00:00:00 2001 From: Hanxuan Zhang Date: Wed, 9 Jul 2025 16:28:22 -0400 Subject: [PATCH 29/29] [Subnet Prioritization] Update format in instances_validators.py --- cli/src/pcluster/validators/instances_validators.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/cli/src/pcluster/validators/instances_validators.py b/cli/src/pcluster/validators/instances_validators.py index daa01dc99d..ab67d6752c 100644 --- a/cli/src/pcluster/validators/instances_validators.py +++ b/cli/src/pcluster/validators/instances_validators.py @@ -222,11 +222,14 @@ class InstancesAllocationStrategyValidator(Validator, _FlexibleInstanceTypesVali def _validate(self, compute_resource_name: str, capacity_type: Enum, allocation_strategy: Enum, **kwargs): """On-demand Capacity type only supports "lowest-price" and "prioritized" allocation strategy.""" + valid_on_demand_allocation_strategy = { + 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 != cluster_config.AllocationStrategy.PRIORITIZED + 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(