From 8641f35be9adaa91de0fc8342de76ce44bc345af Mon Sep 17 00:00:00 2001 From: Chris Makin Date: Fri, 16 Aug 2024 10:43:12 -0400 Subject: [PATCH] [LoginNodes] Update API to support multiple login node pools (#6395) --- api/client/src/docs/ClusterInstance.md | 1 + .../docs/DescribeClusterResponseContent.md | 2 +- api/client/src/docs/LoginNodesPool.md | 1 + .../pcluster_client/model/cluster_instance.py | 4 + .../describe_cluster_response_content.py | 6 +- .../pcluster_client/model/login_nodes_pool.py | 4 + api/spec/openapi/ParallelCluster.openapi.yaml | 8 +- .../model/operations/DescribeCluster.smithy | 2 +- .../DescribeClusterInstances.smithy | 3 +- .../smithy/model/types/LoginNodesPool.smithy | 5 + .../cluster_instances_controller.py | 1 + .../cluster_operations_controller.py | 9 +- .../pcluster/api/models/cluster_instance.py | 27 +++ .../describe_cluster_response_content.py | 8 +- .../pcluster/api/models/login_nodes_pool.py | 30 ++- cli/src/pcluster/api/openapi/openapi.yaml | 26 ++- cli/src/pcluster/aws/aws_resources.py | 6 + .../test_cluster_operations_controller.py | 219 +++++++++++------- .../tests/cli_commands/test_cli_commands.py | 2 +- .../pcluster.config.with.warnings.yaml | 8 + .../pcluster.config.yaml | 8 + 21 files changed, 273 insertions(+), 107 deletions(-) diff --git a/api/client/src/docs/ClusterInstance.md b/api/client/src/docs/ClusterInstance.md index 059b7f28ed..9f7d7cd5d8 100644 --- a/api/client/src/docs/ClusterInstance.md +++ b/api/client/src/docs/ClusterInstance.md @@ -12,6 +12,7 @@ Name | Type | Description | Notes **node_type** | [**NodeType**](NodeType.md) | | **public_ip_address** | **str** | | [optional] **queue_name** | **str** | | [optional] +**pool_name** | **str** | | [optional] **any string name** | **bool, date, datetime, dict, float, int, list, str, none_type** | any string name can be used but the value must be the correct type | [optional] [[Back to Model list]](../README.md#documentation-for-models) [[Back to API list]](../README.md#documentation-for-api-endpoints) [[Back to README]](../README.md) diff --git a/api/client/src/docs/DescribeClusterResponseContent.md b/api/client/src/docs/DescribeClusterResponseContent.md index a3e6d104f9..f7cb1e902f 100644 --- a/api/client/src/docs/DescribeClusterResponseContent.md +++ b/api/client/src/docs/DescribeClusterResponseContent.md @@ -17,7 +17,7 @@ Name | Type | Description | Notes **tags** | [**[Tag]**](Tag.md) | Tags associated with the cluster. | **scheduler** | [**Scheduler**](Scheduler.md) | | [optional] **head_node** | [**EC2Instance**](EC2Instance.md) | | [optional] -**login_nodes** | [**LoginNodesPool**](LoginNodesPool.md) | | [optional] +**login_nodes** | [**[LoginNodesPool]**](LoginNodesPool.md) | | [optional] **failures** | [**[Failure]**](Failure.md) | Failures array containing failures reason and code when the stack is in CREATE_FAILED status. | [optional] **any string name** | **bool, date, datetime, dict, float, int, list, str, none_type** | any string name can be used but the value must be the correct type | [optional] diff --git a/api/client/src/docs/LoginNodesPool.md b/api/client/src/docs/LoginNodesPool.md index 4bdb8e1d6c..828e543a2c 100644 --- a/api/client/src/docs/LoginNodesPool.md +++ b/api/client/src/docs/LoginNodesPool.md @@ -5,6 +5,7 @@ Name | Type | Description | Notes ------------ | ------------- | ------------- | ------------- **status** | [**LoginNodesState**](LoginNodesState.md) | | +**pool_name** | **str** | | [optional] **address** | **str** | | [optional] **scheme** | **str** | | [optional] **healthy_nodes** | **int** | | [optional] diff --git a/api/client/src/pcluster_client/model/cluster_instance.py b/api/client/src/pcluster_client/model/cluster_instance.py index 3dc25c9b33..c8f30c5640 100644 --- a/api/client/src/pcluster_client/model/cluster_instance.py +++ b/api/client/src/pcluster_client/model/cluster_instance.py @@ -97,6 +97,7 @@ def openapi_types(): 'node_type': (NodeType,), # noqa: E501 'public_ip_address': (str,), # noqa: E501 'queue_name': (str,), # noqa: E501 + 'pool_name': (str,), # noqa: E501 } @cached_property @@ -113,6 +114,7 @@ def discriminator(): 'node_type': 'nodeType', # noqa: E501 'public_ip_address': 'publicIpAddress', # noqa: E501 'queue_name': 'queueName', # noqa: E501 + 'pool_name': 'poolName', # noqa: E501 } read_only_vars = { @@ -166,6 +168,7 @@ def _from_openapi_data(cls, instance_id, instance_type, launch_time, private_ip_ _visited_composed_classes = (Animal,) public_ip_address (str): [optional] # noqa: E501 queue_name (str): [optional] # noqa: E501 + pool_name (str): [optional] # noqa: E501 """ _check_type = kwargs.pop('_check_type', True) @@ -267,6 +270,7 @@ def __init__(self, instance_id, instance_type, launch_time, private_ip_address, _visited_composed_classes = (Animal,) public_ip_address (str): [optional] # noqa: E501 queue_name (str): [optional] # noqa: E501 + pool_name (str): [optional] # noqa: E501 """ _check_type = kwargs.pop('_check_type', True) diff --git a/api/client/src/pcluster_client/model/describe_cluster_response_content.py b/api/client/src/pcluster_client/model/describe_cluster_response_content.py index 54c4949499..b78428614c 100644 --- a/api/client/src/pcluster_client/model/describe_cluster_response_content.py +++ b/api/client/src/pcluster_client/model/describe_cluster_response_content.py @@ -121,7 +121,7 @@ def openapi_types(): 'tags': ([Tag],), # noqa: E501 'scheduler': (Scheduler,), # noqa: E501 'head_node': (EC2Instance,), # noqa: E501 - 'login_nodes': (LoginNodesPool,), # noqa: E501 + 'login_nodes': ([LoginNodesPool],), # noqa: E501 'failures': ([Failure],), # noqa: E501 } @@ -204,7 +204,7 @@ def _from_openapi_data(cls, cluster_name, region, version, cloud_formation_stack _visited_composed_classes = (Animal,) scheduler (Scheduler): [optional] # noqa: E501 head_node (EC2Instance): [optional] # noqa: E501 - login_nodes (LoginNodesPool): [optional] # noqa: E501 + login_nodes ([LoginNodesPool]): [optional] # noqa: E501 failures ([Failure]): Failures array containing failures reason and code when the stack is in CREATE_FAILED status.. [optional] # noqa: E501 """ @@ -317,7 +317,7 @@ def __init__(self, cluster_name, region, version, cloud_formation_stack_status, _visited_composed_classes = (Animal,) scheduler (Scheduler): [optional] # noqa: E501 head_node (EC2Instance): [optional] # noqa: E501 - login_nodes (LoginNodesPool): [optional] # noqa: E501 + login_nodes ([LoginNodesPool]): [optional] # noqa: E501 failures ([Failure]): Failures array containing failures reason and code when the stack is in CREATE_FAILED status.. [optional] # noqa: E501 """ diff --git a/api/client/src/pcluster_client/model/login_nodes_pool.py b/api/client/src/pcluster_client/model/login_nodes_pool.py index c00a3e3f7a..e08d798814 100644 --- a/api/client/src/pcluster_client/model/login_nodes_pool.py +++ b/api/client/src/pcluster_client/model/login_nodes_pool.py @@ -88,6 +88,7 @@ def openapi_types(): lazy_import() return { 'status': (LoginNodesState,), # noqa: E501 + 'pool_name': (str,), # noqa: E501 'address': (str,), # noqa: E501 'scheme': (str,), # noqa: E501 'healthy_nodes': (int,), # noqa: E501 @@ -101,6 +102,7 @@ def discriminator(): attribute_map = { 'status': 'status', # noqa: E501 + 'pool_name': 'poolName', # noqa: E501 'address': 'address', # noqa: E501 'scheme': 'scheme', # noqa: E501 'healthy_nodes': 'healthyNodes', # noqa: E501 @@ -151,6 +153,7 @@ def _from_openapi_data(cls, status, *args, **kwargs): # noqa: E501 Animal class but this time we won't travel through its discriminator because we passed in _visited_composed_classes = (Animal,) + pool_name (str): [optional] # noqa: E501 address (str): [optional] # noqa: E501 scheme (str): [optional] # noqa: E501 healthy_nodes (int): [optional] # noqa: E501 @@ -244,6 +247,7 @@ def __init__(self, status, *args, **kwargs): # noqa: E501 Animal class but this time we won't travel through its discriminator because we passed in _visited_composed_classes = (Animal,) + pool_name (str): [optional] # noqa: E501 address (str): [optional] # noqa: E501 scheme (str): [optional] # noqa: E501 healthy_nodes (int): [optional] # noqa: E501 diff --git a/api/spec/openapi/ParallelCluster.openapi.yaml b/api/spec/openapi/ParallelCluster.openapi.yaml index 24cde280ba..6900fd40ca 100644 --- a/api/spec/openapi/ParallelCluster.openapi.yaml +++ b/api/spec/openapi/ParallelCluster.openapi.yaml @@ -1748,6 +1748,8 @@ components: $ref: '#/components/schemas/NodeType' queueName: type: string + poolName: + type: string required: - instanceId - instanceType @@ -1914,7 +1916,9 @@ components: headNode: $ref: '#/components/schemas/EC2Instance' loginNodes: - $ref: '#/components/schemas/LoginNodesPool' + type: array + items: + $ref: '#/components/schemas/LoginNodesPool' failures: type: array items: @@ -2323,6 +2327,8 @@ components: properties: status: $ref: '#/components/schemas/LoginNodesState' + poolName: + type: string address: type: string scheme: diff --git a/api/spec/smithy/model/operations/DescribeCluster.smithy b/api/spec/smithy/model/operations/DescribeCluster.smithy index d72a56d78c..3f6678c30b 100644 --- a/api/spec/smithy/model/operations/DescribeCluster.smithy +++ b/api/spec/smithy/model/operations/DescribeCluster.smithy @@ -61,7 +61,7 @@ structure DescribeClusterResponse { @documentation("Tags associated with the cluster.") tags: Tags, headNode: EC2Instance, - loginNodes: LoginNodesPool, + loginNodes: LoginNodes, @documentation("Failures array containing failures reason and code when the stack is in CREATE_FAILED status.") failures: Failures } diff --git a/api/spec/smithy/model/operations/DescribeClusterInstances.smithy b/api/spec/smithy/model/operations/DescribeClusterInstances.smithy index 9bfd866677..d77ec68b68 100644 --- a/api/spec/smithy/model/operations/DescribeClusterInstances.smithy +++ b/api/spec/smithy/model/operations/DescribeClusterInstances.smithy @@ -53,7 +53,8 @@ structure ClusterInstance { state: InstanceState, @required nodeType: NodeType, - queueName: String + queueName: String, + poolName: String } list InstanceSummaries { diff --git a/api/spec/smithy/model/types/LoginNodesPool.smithy b/api/spec/smithy/model/types/LoginNodesPool.smithy index 1046a1a266..0f48d7d760 100644 --- a/api/spec/smithy/model/types/LoginNodesPool.smithy +++ b/api/spec/smithy/model/types/LoginNodesPool.smithy @@ -7,9 +7,14 @@ namespace parallelcluster ]) string LoginNodesState +list LoginNodes { + member: LoginNodesPool +} + structure LoginNodesPool { @required status: LoginNodesState, + poolName: String address: String, scheme: String, healthyNodes: Integer, diff --git a/cli/src/pcluster/api/controllers/cluster_instances_controller.py b/cli/src/pcluster/api/controllers/cluster_instances_controller.py index 55a83983e1..31004a4581 100644 --- a/cli/src/pcluster/api/controllers/cluster_instances_controller.py +++ b/cli/src/pcluster/api/controllers/cluster_instances_controller.py @@ -97,6 +97,7 @@ def describe_cluster_instances(cluster_name, region=None, next_token=None, node_ private_ip_address=instance.private_ip, node_type=node_type, queue_name=instance.queue_name, + pool_name=instance.pool_name, ) ) return DescribeClusterInstancesResponseContent(instances=ec2_instances, next_token=next_token) diff --git a/cli/src/pcluster/api/controllers/cluster_operations_controller.py b/cli/src/pcluster/api/controllers/cluster_operations_controller.py index 461a6e2e36..5616c93b44 100644 --- a/cli/src/pcluster/api/controllers/cluster_operations_controller.py +++ b/cli/src/pcluster/api/controllers/cluster_operations_controller.py @@ -266,26 +266,23 @@ def describe_cluster(cluster_name, region=None): def _get_login_nodes(cluster): login_nodes_status = cluster.login_nodes_status - # TODO Fix once the API models are updated to support multiple pools in describe-cluster response if login_nodes_status.get_login_nodes_pool_available(): login_nodes = [] - - for _pool_name, pool_status in login_nodes_status.get_pool_status_dict().items(): + for pool_name, pool_status in login_nodes_status.get_pool_status_dict().items(): status = LoginNodesState.FAILED if pool_status.get_status() == LoginNodesPoolState.ACTIVE: status = LoginNodesState.ACTIVE elif pool_status.get_status() == LoginNodesPoolState.PENDING: status = LoginNodesState.PENDING pool = LoginNodesPool(status=status) - # pool.name = pool_name + pool.pool_name = pool_name pool.address = pool_status.get_address() pool.scheme = pool_status.get_scheme() pool.healthy_nodes = pool_status.get_healthy_nodes() pool.unhealthy_nodes = pool_status.get_unhealthy_nodes() login_nodes.append(pool) - break - return login_nodes[0] + return login_nodes return None diff --git a/cli/src/pcluster/api/models/cluster_instance.py b/cli/src/pcluster/api/models/cluster_instance.py index 1711f0ef22..c30a8c4992 100644 --- a/cli/src/pcluster/api/models/cluster_instance.py +++ b/cli/src/pcluster/api/models/cluster_instance.py @@ -28,6 +28,7 @@ def __init__( launch_time=None, instance_id=None, queue_name=None, + pool_name=None, public_ip_address=None, instance_type=None, state=None, @@ -42,6 +43,8 @@ def __init__( :type instance_id: str :param queue_name: The queue_name of this ClusterInstance. # noqa: E501 :type queue_name: str + :param pool_name: The pool_name of this ClusterInstance. # noqa: E501 + :type pool_name: str :param public_ip_address: The public_ip_address of this ClusterInstance. # noqa: E501 :type public_ip_address: str :param instance_type: The instance_type of this ClusterInstance. # noqa: E501 @@ -57,6 +60,7 @@ def __init__( "launch_time": datetime, "instance_id": str, "queue_name": str, + "pool_name": str, "public_ip_address": str, "instance_type": str, "state": InstanceState, @@ -68,6 +72,7 @@ def __init__( "launch_time": "launchTime", "instance_id": "instanceId", "queue_name": "queueName", + "pool_name": "poolName", "public_ip_address": "publicIpAddress", "instance_type": "instanceType", "state": "state", @@ -78,6 +83,7 @@ def __init__( self._launch_time = launch_time self._instance_id = instance_id self._queue_name = queue_name + self._pool_name = pool_name self._public_ip_address = public_ip_address self._instance_type = instance_type self._state = state @@ -274,3 +280,24 @@ def private_ip_address(self, private_ip_address): raise ValueError("Invalid value for `private_ip_address`, must not be `None`") # noqa: E501 self._private_ip_address = private_ip_address + + @property + def pool_name(self): + """Gets the pool_name of this ClusterInstance. + + + :return: The pool_name of this ClusterInstance. + :rtype: str + """ + return self._pool_name + + @pool_name.setter + def pool_name(self, pool_name): + """Sets the pool_name of this ClusterInstance. + + + :param pool_name: The pool_name of this ClusterInstance. + :type pool_name: str + """ + + self._pool_name = pool_name diff --git a/cli/src/pcluster/api/models/describe_cluster_response_content.py b/cli/src/pcluster/api/models/describe_cluster_response_content.py index 21d3275c53..73dbd534ac 100644 --- a/cli/src/pcluster/api/models/describe_cluster_response_content.py +++ b/cli/src/pcluster/api/models/describe_cluster_response_content.py @@ -79,7 +79,7 @@ def __init__( :param scheduler: The scheduler of this DescribeClusterResponseContent. # noqa: E501 :type scheduler: Scheduler :param login_nodes: The login_nodes of this DescribeClusterResponseContent. # noqa: E501 - :type login_nodes: LoginNodesPool + :type login_nodes: List[LoginNodesPool] :param failures: The failures of this DescribeClusterResponseContent. # noqa: E501 :type failures: List[Failure] """ @@ -97,7 +97,7 @@ def __init__( "region": str, "cluster_status": ClusterStatus, "scheduler": Scheduler, - "login_nodes": LoginNodesPool, + "login_nodes": List[LoginNodesPool], "failures": List[Failure], } @@ -469,7 +469,7 @@ def login_nodes(self): :return: The login_nodes of this DescribeClusterResponseContent. - :rtype: LoginNodesPool + :rtype: List[LoginNodesPool] """ return self._login_nodes @@ -479,7 +479,7 @@ def login_nodes(self, login_nodes): :param login_nodes: The login_nodes of this DescribeClusterResponseContent. - :type login_nodes: LoginNodesPool + :type login_nodes: List[LoginNodesPool] """ self._login_nodes = login_nodes diff --git a/cli/src/pcluster/api/models/login_nodes_pool.py b/cli/src/pcluster/api/models/login_nodes_pool.py index dc89852932..34a02381dc 100644 --- a/cli/src/pcluster/api/models/login_nodes_pool.py +++ b/cli/src/pcluster/api/models/login_nodes_pool.py @@ -22,11 +22,15 @@ class LoginNodesPool(Model): Do not edit the class manually. """ - def __init__(self, status=None, address=None, scheme=None, healthy_nodes=None, unhealthy_nodes=None): # noqa: E501 + def __init__( + self, status=None, pool_name=None, address=None, scheme=None, healthy_nodes=None, unhealthy_nodes=None + ): # noqa: E501 """LoginNodesPool - a model defined in OpenAPI :param status: The status of this LoginNodesPool. # noqa: E501 :type status: LoginNodesState + :param pool_name: The pool_name of this LoginNodesPool. # noqa: E501 + :type pool_name: str :param address: The address of this LoginNodesPool. # noqa: E501 :type address: str :param scheme: The scheme of this LoginNodesPool. # noqa: E501 @@ -38,6 +42,7 @@ def __init__(self, status=None, address=None, scheme=None, healthy_nodes=None, u """ self.openapi_types = { "status": LoginNodesState, + "pool_name": str, "address": str, "scheme": str, "healthy_nodes": int, @@ -46,6 +51,7 @@ def __init__(self, status=None, address=None, scheme=None, healthy_nodes=None, u self.attribute_map = { "status": "status", + "pool_name": "poolName", "address": "address", "scheme": "scheme", "healthy_nodes": "healthyNodes", @@ -53,6 +59,7 @@ def __init__(self, status=None, address=None, scheme=None, healthy_nodes=None, u } self._status = status + self._pool_name = pool_name self._address = address self._scheme = scheme self._healthy_nodes = healthy_nodes @@ -92,6 +99,27 @@ def status(self, status): self._status = status + @property + def pool_name(self): + """Gets the pool_name of this LoginNodesPool. + + + :return: The pool_name of this LoginNodesPool. + :rtype: str + """ + return self._pool_name + + @pool_name.setter + def pool_name(self, pool_name): + """Sets the pool_name of this LoginNodesPool. + + + :param pool_name: The pool_name of this LoginNodesPool. + :type pool_name: str + """ + + self._pool_name = pool_name + @property def address(self): """Gets the address of this LoginNodesPool. diff --git a/cli/src/pcluster/api/openapi/openapi.yaml b/cli/src/pcluster/api/openapi/openapi.yaml index d3df3bcf19..ec42930128 100644 --- a/cli/src/pcluster/api/openapi/openapi.yaml +++ b/cli/src/pcluster/api/openapi/openapi.yaml @@ -2362,11 +2362,18 @@ components: failureReason: failureReason creationTime: 2000-01-23T04:56:07.000+00:00 loginNodes: - unhealthyNodes: 6 - address: address - scheme: scheme - healthyNodes: 0 - status: null + - unhealthyNodes: 6 + address: address + scheme: scheme + healthyNodes: 0 + status: null + poolName: poolName + - unhealthyNodes: 6 + address: address + scheme: scheme + healthyNodes: 0 + status: null + poolName: poolName version: version clusterConfiguration: url: url @@ -2442,7 +2449,10 @@ components: headNode: $ref: '#/components/schemas/EC2Instance' loginNodes: - $ref: '#/components/schemas/LoginNodesPool' + items: + $ref: '#/components/schemas/LoginNodesPool' + title: loginNodes + type: array failures: description: Failures array containing failures reason and code when the stack is in CREATE_FAILED status. @@ -3187,9 +3197,13 @@ components: scheme: scheme healthyNodes: 0 status: null + poolName: poolName properties: status: $ref: '#/components/schemas/LoginNodesState' + poolName: + title: poolName + type: string address: title: address type: string diff --git a/cli/src/pcluster/aws/aws_resources.py b/cli/src/pcluster/aws/aws_resources.py index a43a9b583b..61e0faf343 100644 --- a/cli/src/pcluster/aws/aws_resources.py +++ b/cli/src/pcluster/aws/aws_resources.py @@ -15,6 +15,7 @@ PCLUSTER_IMAGE_CONFIG_TAG, PCLUSTER_IMAGE_ID_TAG, PCLUSTER_IMAGE_OS_TAG, + PCLUSTER_LOGIN_NODES_POOL_NAME_TAG, PCLUSTER_NODE_TYPE_TAG, PCLUSTER_QUEUE_NAME_TAG, PCLUSTER_S3_BUCKET_TAG, @@ -169,6 +170,11 @@ def queue_name(self) -> str: """Return queue name of the instance.""" return self._get_tag(PCLUSTER_QUEUE_NAME_TAG) + @property + def pool_name(self) -> str: + """Return pool name of the instance.""" + return self._get_tag(PCLUSTER_LOGIN_NODES_POOL_NAME_TAG) + def _get_tag(self, tag_key): return next(iter([tag["Value"] for tag in self._tags if tag["Key"] == tag_key]), None) diff --git a/cli/tests/pcluster/api/controllers/test_cluster_operations_controller.py b/cli/tests/pcluster/api/controllers/test_cluster_operations_controller.py index ade39a79ab..7509b3cef5 100644 --- a/cli/tests/pcluster/api/controllers/test_cluster_operations_controller.py +++ b/cli/tests/pcluster/api/controllers/test_cluster_operations_controller.py @@ -57,6 +57,25 @@ def cfn_describe_stack_mock_response(edits=None): return stack_data +def get_mock_pool_status(mocker, pool_name, status, scheme, address, healthy_nodes, unhealthy_nodes): + """Return a mocked pool status for describe-cluster command.""" + pool_status = PoolStatus("clustername", pool_name) + + if status: + mocker.patch.object(pool_status, "get_status", return_value=status) + if scheme: + mocker.patch.object(pool_status, "get_scheme", return_value=scheme) + if address: + mocker.patch.object(pool_status, "get_address", return_value=address) + if healthy_nodes is not None: + mocker.patch.object(pool_status, "get_healthy_nodes", return_value=healthy_nodes) + if unhealthy_nodes is not None: + mocker.patch.object(pool_status, "get_unhealthy_nodes", return_value=unhealthy_nodes) + mocker.patch.object(pool_status, "get_unhealthy_nodes", return_value=unhealthy_nodes) + + return pool_status + + class TestCreateCluster: url = "/v3/clusters" method = "POST" @@ -1167,15 +1186,15 @@ def test_error_conversion(self, client, mocker, error_type, error_code, http_cod assert_that(response.get_json()).is_equal_to(expected_response) @pytest.mark.parametrize( - "login_nodes_pool_available, status, scheme, address, healthy_nodes, unhealthy_nodes, expected_response", + "login_nodes_pool_available, statuses, schemes, addresses, healthy_nodes, unhealthy_nodes, expected_response", [ ( False, - None, - None, - None, - 0, - 0, + [None, None], + [None, None], + [None, None], + [0, 0], + [0, 0], { "cloudFormationStackStatus": "CREATE_COMPLETE", "cloudformationStackArn": "arn:aws:cloudformation:us-east-1:123:stack/pcluster3-2/123", @@ -1208,11 +1227,11 @@ def test_error_conversion(self, client, mocker, error_type, error_code, http_cod ), ( True, - LoginNodesPoolState.PENDING, - None, - None, - 0, - 0, + [LoginNodesPoolState.PENDING, LoginNodesPoolState.PENDING], + [None, None], + [None, None], + [None, None], + [None, None], { "cloudFormationStackStatus": "CREATE_COMPLETE", "cloudformationStackArn": "arn:aws:cloudformation:us-east-1:123:stack/pcluster3-2/123", @@ -1222,7 +1241,10 @@ def test_error_conversion(self, client, mocker, error_type, error_code, http_cod "computeFleetStatus": "RUNNING", "creationTime": to_iso_timestr(datetime(2021, 4, 30)), "lastUpdatedTime": to_iso_timestr(datetime(2021, 4, 30)), - "loginNodes": {"status": "pending"}, + "loginNodes": [ + {"poolName": "pool1", "status": "pending"}, + {"poolName": "pool2", "status": "pending"}, + ], "region": "us-east-1", "tags": [ {"key": "parallelcluster:version", "value": get_installed_version()}, @@ -1246,11 +1268,11 @@ def test_error_conversion(self, client, mocker, error_type, error_code, http_cod ), ( True, - LoginNodesPoolState.ACTIVE, - "external", - "load.balancer.com", - 5, - 2, + [LoginNodesPoolState.ACTIVE, LoginNodesPoolState.ACTIVE], + ["external", "internet-facing"], + ["pool1.load.balancer.com", "pool2.load.balancer.com"], + [5, 0], + [2, 0], { "cloudFormationStackStatus": "CREATE_COMPLETE", "cloudformationStackArn": "arn:aws:cloudformation:us-east-1:123:stack/pcluster3-2/123", @@ -1260,13 +1282,24 @@ def test_error_conversion(self, client, mocker, error_type, error_code, http_cod "computeFleetStatus": "RUNNING", "creationTime": to_iso_timestr(datetime(2021, 4, 30)), "lastUpdatedTime": to_iso_timestr(datetime(2021, 4, 30)), - "loginNodes": { - "address": "load.balancer.com", - "healthyNodes": 5, - "scheme": "external", - "status": "active", - "unhealthyNodes": 2, - }, + "loginNodes": [ + { + "address": "pool1.load.balancer.com", + "poolName": "pool1", + "healthyNodes": 5, + "scheme": "external", + "status": "active", + "unhealthyNodes": 2, + }, + { + "address": "pool2.load.balancer.com", + "poolName": "pool2", + "healthyNodes": 0, + "scheme": "internet-facing", + "status": "active", + "unhealthyNodes": 0, + }, + ], "region": "us-east-1", "tags": [ {"key": "parallelcluster:version", "value": get_installed_version()}, @@ -1290,11 +1323,11 @@ def test_error_conversion(self, client, mocker, error_type, error_code, http_cod ), ( True, - LoginNodesPoolState.ACTIVE, - "external", - "load.balancer.com", - 0, - 2, + [LoginNodesPoolState.ACTIVE, LoginNodesPoolState.ACTIVE], + ["external", "external"], + ["pool1.load.balancer.com", "pool2.load.balancer.com"], + [0, 2], + [2, 0], { "cloudFormationStackStatus": "CREATE_COMPLETE", "cloudformationStackArn": "arn:aws:cloudformation:us-east-1:123:stack/pcluster3-2/123", @@ -1304,12 +1337,24 @@ def test_error_conversion(self, client, mocker, error_type, error_code, http_cod "computeFleetStatus": "RUNNING", "creationTime": to_iso_timestr(datetime(2021, 4, 30)), "lastUpdatedTime": to_iso_timestr(datetime(2021, 4, 30)), - "loginNodes": { - "address": "load.balancer.com", - "scheme": "external", - "status": "active", - "unhealthyNodes": 2, - }, + "loginNodes": [ + { + "address": "pool1.load.balancer.com", + "poolName": "pool1", + "scheme": "external", + "status": "active", + "unhealthyNodes": 2, + "healthyNodes": 0, + }, + { + "address": "pool2.load.balancer.com", + "poolName": "pool2", + "scheme": "external", + "status": "active", + "healthyNodes": 2, + "unhealthyNodes": 0, + }, + ], "region": "us-east-1", "tags": [ {"key": "parallelcluster:version", "value": get_installed_version()}, @@ -1333,11 +1378,11 @@ def test_error_conversion(self, client, mocker, error_type, error_code, http_cod ), ( True, - LoginNodesPoolState.ACTIVE, - "external", - "load.balancer.com", - 5, - 0, + [LoginNodesPoolState.ACTIVE, LoginNodesPoolState.ACTIVE], + ["external", "private"], + ["pool1.load.balancer.com", "pool2.load.balancer.com"], + [5, 2], + [2, 5], { "cloudFormationStackStatus": "CREATE_COMPLETE", "cloudformationStackArn": "arn:aws:cloudformation:us-east-1:123:stack/pcluster3-2/123", @@ -1347,12 +1392,24 @@ def test_error_conversion(self, client, mocker, error_type, error_code, http_cod "computeFleetStatus": "RUNNING", "creationTime": to_iso_timestr(datetime(2021, 4, 30)), "lastUpdatedTime": to_iso_timestr(datetime(2021, 4, 30)), - "loginNodes": { - "address": "load.balancer.com", - "healthyNodes": 5, - "scheme": "external", - "status": "active", - }, + "loginNodes": [ + { + "address": "pool1.load.balancer.com", + "poolName": "pool1", + "healthyNodes": 5, + "unhealthyNodes": 2, + "scheme": "external", + "status": "active", + }, + { + "address": "pool2.load.balancer.com", + "poolName": "pool2", + "healthyNodes": 2, + "unhealthyNodes": 5, + "scheme": "private", + "status": "active", + }, + ], "region": "us-east-1", "tags": [ {"key": "parallelcluster:version", "value": get_installed_version()}, @@ -1376,11 +1433,11 @@ def test_error_conversion(self, client, mocker, error_type, error_code, http_cod ), ( True, - LoginNodesPoolState.FAILED, - "internal", - "load.balancer.com", - 0, - 0, + [LoginNodesPoolState.FAILED, LoginNodesPoolState.FAILED], + ["internal", "internal"], + ["pool1.load.balancer.com", "pool2.load.balancer.com"], + [None, None], + [None, None], { "cloudFormationStackStatus": "CREATE_COMPLETE", "cloudformationStackArn": "arn:aws:cloudformation:us-east-1:123:stack/pcluster3-2/123", @@ -1390,11 +1447,20 @@ def test_error_conversion(self, client, mocker, error_type, error_code, http_cod "computeFleetStatus": "RUNNING", "creationTime": to_iso_timestr(datetime(2021, 4, 30)), "lastUpdatedTime": to_iso_timestr(datetime(2021, 4, 30)), - "loginNodes": { - "address": "load.balancer.com", - "scheme": "internal", - "status": "failed", - }, + "loginNodes": [ + { + "address": "pool1.load.balancer.com", + "poolName": "pool1", + "scheme": "internal", + "status": "failed", + }, + { + "address": "pool2.load.balancer.com", + "poolName": "pool2", + "scheme": "internal", + "status": "failed", + }, + ], "region": "us-east-1", "tags": [ {"key": "parallelcluster:version", "value": get_installed_version()}, @@ -1424,9 +1490,9 @@ def test_login_nodes_pool_information( mocker, client, login_nodes_pool_available, - status, - scheme, - address, + statuses, + schemes, + addresses, healthy_nodes, unhealthy_nodes, expected_response, @@ -1482,34 +1548,23 @@ def test_login_nodes_pool_information( config_mock.return_value.scheduling.settings.scheduler_definition.metadata = "" config_mock.return_value.scheduling.scheduler = "slurm" - # TODO Update once multiple pools supported in describe-cluster API response - mocker.patch.object(PoolStatus, "_retrieve_data") - pool_status_1 = PoolStatus("clustername", "pool1") - pool_status_dict = {"pool1": pool_status_1} - + # Mock the LoginNodesStatus mocker.patch("pcluster.models.login_nodes_status.LoginNodesStatus.retrieve_data") - mocker.patch( - "pcluster.models.login_nodes_status.LoginNodesStatus.get_pool_status_dict", return_value=pool_status_dict - ) + mocker.patch("pcluster.models.login_nodes_status.PoolStatus._retrieve_data") mocker.patch( "pcluster.models.login_nodes_status.LoginNodesStatus.get_login_nodes_pool_available", return_value=login_nodes_pool_available, ) - mocker.patch("pcluster.models.login_nodes_status.PoolStatus.get_status", return_value=status) - if scheme: - mocker.patch("pcluster.models.login_nodes_status.PoolStatus.get_scheme", return_value=scheme) - if address: - mocker.patch("pcluster.models.login_nodes_status.PoolStatus.get_address", return_value=address) - if healthy_nodes: - mocker.patch( - "pcluster.models.login_nodes_status.PoolStatus.get_healthy_nodes", - return_value=healthy_nodes, - ) - if unhealthy_nodes: - mocker.patch( - "pcluster.models.login_nodes_status.PoolStatus.get_unhealthy_nodes", - return_value=unhealthy_nodes, - ) + pool_status_1 = get_mock_pool_status( + mocker, "pool1", statuses[0], schemes[0], addresses[0], healthy_nodes[0], unhealthy_nodes[0] + ) + pool_status_2 = get_mock_pool_status( + mocker, "pool2", statuses[1], schemes[1], addresses[1], healthy_nodes[1], unhealthy_nodes[1] + ) + pool_status_dict = {"pool1": pool_status_1, "pool2": pool_status_2} + mocker.patch( + "pcluster.models.login_nodes_status.LoginNodesStatus.get_pool_status_dict", return_value=pool_status_dict + ) response = self._send_test_request(client) diff --git a/tests/integration-tests/tests/cli_commands/test_cli_commands.py b/tests/integration-tests/tests/cli_commands/test_cli_commands.py index e1f1b4ed3b..cb839d8548 100644 --- a/tests/integration-tests/tests/cli_commands/test_cli_commands.py +++ b/tests/integration-tests/tests/cli_commands/test_cli_commands.py @@ -194,7 +194,6 @@ def _check_response(actual_response, expected_response, validation_key): def _test_describe_cluster(cluster): - # TODO Add section for login nodes once describe-cluster updated to support multiple pools cluster_info = cluster.describe_cluster() assert_that(cluster_info).is_not_none() assert_that(cluster_info).contains("clusterName") @@ -205,6 +204,7 @@ def _test_describe_cluster(cluster): assert_that(cluster_info).contains("creationTime") assert_that(cluster_info).contains("clusterConfiguration") assert_that(cluster_info).contains("scheduler") + assert_that(cluster_info).contains("loginNodes") def _test_list_cluster(cluster_name, expected_status): diff --git a/tests/integration-tests/tests/cli_commands/test_cli_commands/test_slurm_cli_commands/pcluster.config.with.warnings.yaml b/tests/integration-tests/tests/cli_commands/test_cli_commands/test_slurm_cli_commands/pcluster.config.with.warnings.yaml index 40687ce603..15762c4c75 100644 --- a/tests/integration-tests/tests/cli_commands/test_cli_commands/test_slurm_cli_commands/pcluster.config.with.warnings.yaml +++ b/tests/integration-tests/tests/cli_commands/test_cli_commands/test_slurm_cli_commands/pcluster.config.with.warnings.yaml @@ -7,6 +7,14 @@ HeadNode: # Head Node section without a KeyName will generate an warning SubnetId: {{ public_subnet_id }} Imds: Secured: {{ imds_secured }} +LoginNodes: + Pools: + - Name: pool + InstanceType: {{ instance }} + Count: 1 + Networking: + SubnetIds: + - {{ public_subnet_id }} Scheduling: Scheduler: {{ scheduler }} {{ scheduler_prefix }}Queues: diff --git a/tests/integration-tests/tests/cli_commands/test_cli_commands/test_slurm_cli_commands/pcluster.config.yaml b/tests/integration-tests/tests/cli_commands/test_cli_commands/test_slurm_cli_commands/pcluster.config.yaml index 694c8c1bcd..7242f11b0d 100644 --- a/tests/integration-tests/tests/cli_commands/test_cli_commands/test_slurm_cli_commands/pcluster.config.yaml +++ b/tests/integration-tests/tests/cli_commands/test_cli_commands/test_slurm_cli_commands/pcluster.config.yaml @@ -8,6 +8,14 @@ HeadNode: KeyName: {{ key_name }} Imds: Secured: {{ imds_secured }} +LoginNodes: + Pools: + - Name: pool + InstanceType: {{ instance }} + Count: 1 + Networking: + SubnetIds: + - {{ public_subnet_id }} Scheduling: Scheduler: {{ scheduler }} {{ scheduler_prefix }}Settings: