🐛Autoscaling: fix listing of nodes with common prefix (#4410)

Author: sanderegg

services/autoscaling/src/simcore_service_autoscaling/utils/utils_docker.py

@@ -414,7 +414,14 @@ async def find_node_with_name(
     list_of_nodes = await docker_client.nodes.list(filters={"name": name})
     if not list_of_nodes:
         return None
-    return parse_obj_as(Node, list_of_nodes[0])
+    # note that there might be several nodes with a common_prefixed name. so now we want exact matching
+    list_of_nodes = parse_obj_as(list[Node], list_of_nodes)
+    for node in list_of_nodes:
+        assert node.Description  # nosec
+        if node.Description.Hostname == name:
+            return node
+
+    return None
 
 
 async def tag_node(

services/autoscaling/tests/unit/conftest.py

@@ -7,17 +7,7 @@
 import random
 from datetime import timezone
 from pathlib import Path
-from typing import (
-    Any,
-    AsyncIterator,
-    Awaitable,
-    Callable,
-    Final,
-    Iterator,
-    Optional,
-    Union,
-    cast,
-)
+from typing import Any, AsyncIterator, Awaitable, Callable, Final, Iterator, cast
 
 import aiodocker
 import httpx
@@ -186,7 +176,6 @@ def service_monitored_labels(
 
 @pytest.fixture
 async def async_client(initialized_app: FastAPI) -> AsyncIterator[httpx.AsyncClient]:
-
     async with httpx.AsyncClient(
         app=initialized_app,
         base_url=f"http://{initialized_app.title}.testserver.io",
@@ -218,25 +207,35 @@ async def host_node(
 
 
 @pytest.fixture
-def fake_node(faker: Faker) -> Node:
-    return Node(
-        ID=faker.uuid4(),
-        Version=ObjectVersion(Index=faker.pyint()),
-        CreatedAt=faker.date_time(tzinfo=timezone.utc).isoformat(),
-        UpdatedAt=faker.date_time(tzinfo=timezone.utc).isoformat(),
-        Description=NodeDescription(
-            Hostname=faker.pystr(),
-            Resources=ResourceObject(
-                NanoCPUs=int(9 * 1e9), MemoryBytes=256 * 1024 * 1024 * 1024
+def create_fake_node(faker: Faker) -> Callable[..., Node]:
+    def _creator(**node_overrides) -> Node:
+        default_config = dict(
+            ID=faker.uuid4(),
+            Version=ObjectVersion(Index=faker.pyint()),
+            CreatedAt=faker.date_time(tzinfo=timezone.utc).isoformat(),
+            UpdatedAt=faker.date_time(tzinfo=timezone.utc).isoformat(),
+            Description=NodeDescription(
+                Hostname=faker.pystr(),
+                Resources=ResourceObject(
+                    NanoCPUs=int(9 * 1e9), MemoryBytes=256 * 1024 * 1024 * 1024
+                ),
             ),
-        ),
-        Spec=NodeSpec(
-            Name=None,
-            Labels=None,
-            Role=None,
-            Availability=Availability.drain,
-        ),
-    )
+            Spec=NodeSpec(
+                Name=None,
+                Labels=None,
+                Role=None,
+                Availability=Availability.drain,
+            ),
+        )
+        default_config.update(**node_overrides)
+        return Node(**default_config)
+
+    return _creator
+
+
+@pytest.fixture
+def fake_node(create_fake_node: Callable[..., Node]) -> Node:
+    return create_fake_node()
 
 
 @pytest.fixture
@@ -254,7 +253,7 @@ def task_template() -> dict[str, Any]:
 
 @pytest.fixture
 def create_task_reservations() -> Callable[[NUM_CPUS, int], dict[str, Any]]:
-    def _creator(num_cpus: NUM_CPUS, memory: Union[ByteSize, int]) -> dict[str, Any]:
+    def _creator(num_cpus: NUM_CPUS, memory: ByteSize | int) -> dict[str, Any]:
         return {
             "Resources": {
                 "Reservations": {
@@ -269,7 +268,7 @@ def _creator(num_cpus: NUM_CPUS, memory: Union[ByteSize, int]) -> dict[str, Any]
 
 @pytest.fixture
 def create_task_limits() -> Callable[[NUM_CPUS, int], dict[str, Any]]:
-    def _creator(num_cpus: NUM_CPUS, memory: Union[ByteSize, int]) -> dict[str, Any]:
+    def _creator(num_cpus: NUM_CPUS, memory: ByteSize | int) -> dict[str, Any]:
         return {
             "Resources": {
                 "Limits": {
@@ -288,13 +287,13 @@ async def create_service(
     docker_swarm: None,
     faker: Faker,
 ) -> AsyncIterator[
-    Callable[[dict[str, Any], Optional[dict[str, str]]], Awaitable[Service]]
+    Callable[[dict[str, Any], dict[str, str] | None], Awaitable[Service]]
 ]:
     created_services = []
 
     async def _creator(
         task_template: dict[str, Any],
-        labels: Optional[dict[str, str]] = None,
+        labels: dict[str, str] | None = None,
         wait_for_service_state="running",
     ) -> Service:
         service_name = f"pytest_{faker.pystr()}"
@@ -353,6 +352,7 @@ async def _creator(
     await asyncio.gather(
         *(async_docker_client.services.delete(s.ID) for s in created_services)
     )
+
     # wait until all tasks are gone
     @retry(
         retry=retry_if_exception_type(AssertionError),

services/autoscaling/tests/unit/test_utils_docker.py

@@ -16,6 +16,7 @@
 from models_library.docker import DockerGenericTag, DockerLabelKey
 from models_library.generated_models.docker_rest_api import (
     Availability,
+    NodeDescription,
     NodeState,
     Service,
     Task,
@@ -790,6 +791,44 @@ async def test_try_get_node_with_name_fake(
     assert received_node is None
 
 
+async def test_find_node_with_name_with_common_prefixed_nodes(
+    autoscaling_docker: AutoscalingDocker,
+    mocker: MockerFixture,
+    create_fake_node: Callable[..., Node],
+):
+    common_prefix = "ip-10-0-1-"
+    mocked_aiodocker = mocker.patch.object(autoscaling_docker, "nodes", autospec=True)
+    mocked_aiodocker.list.return_value = [
+        create_fake_node(
+            Description=NodeDescription(Hostname=f"{common_prefix}{'1'*(i+1)}")
+        )
+        for i in range(3)
+    ]
+    needed_host_name = f"{common_prefix}11"
+    found_node = await find_node_with_name(autoscaling_docker, needed_host_name)
+    assert found_node
+    assert found_node.Description
+    assert found_node.Description.Hostname == needed_host_name
+
+
+async def test_find_node_with_smaller_name_with_common_prefixed_nodes_returns_none(
+    autoscaling_docker: AutoscalingDocker,
+    mocker: MockerFixture,
+    create_fake_node: Callable[..., Node],
+):
+    common_prefix = "ip-10-0-1-"
+    mocked_aiodocker = mocker.patch.object(autoscaling_docker, "nodes", autospec=True)
+    mocked_aiodocker.list.return_value = [
+        create_fake_node(
+            Description=NodeDescription(Hostname=f"{common_prefix}{'1'*(i+1)}")
+        )
+        for i in range(3)
+    ]
+    needed_host_name = f"{common_prefix}"
+    found_node = await find_node_with_name(autoscaling_docker, needed_host_name)
+    assert found_node is None
+
+
 async def test_tag_node(
     autoscaling_docker: AutoscalingDocker, host_node: Node, faker: Faker
 ):

services/autoscaling/tests/unit/test_utils_dynamic_scaling.py

@@ -56,13 +56,24 @@ def _creator(**overrides) -> Task:
     return _creator
 
 
+@pytest.mark.parametrize(
+    "aws_private_dns, expected_host_name",
+    [
+        ("ip-10-12-32-3.internal-data", "ip-10-12-32-3"),
+        ("ip-10-12-32-32.internal-data", "ip-10-12-32-32"),
+        ("ip-10-0-3-129.internal-data", "ip-10-0-3-129"),
+        ("ip-10-0-3-12.internal-data", "ip-10-0-3-12"),
+    ],
+)
 def test_node_host_name_from_ec2_private_dns(
-    fake_ec2_instance_data: Callable[..., EC2InstanceData]
+    fake_ec2_instance_data: Callable[..., EC2InstanceData],
+    aws_private_dns: str,
+    expected_host_name: str,
 ):
     instance = fake_ec2_instance_data(
-        aws_private_dns="ip-10-12-32-3.internal-data",
+        aws_private_dns=aws_private_dns,
     )
-    assert node_host_name_from_ec2_private_dns(instance) == "ip-10-12-32-3"
+    assert node_host_name_from_ec2_private_dns(instance) == expected_host_name
 
 
 def test_node_host_name_from_ec2_private_dns_raises_with_invalid_name(