🐛Autoscaling: Fix lost stopped EC2 instances and missing error logs (#7493)

Author: sanderegg

packages/aws-library/src/aws_library/ec2/_error_handler.py

@@ -59,13 +59,8 @@ def ec2_exception_handler(
     [Callable[Concatenate[Self, P], Coroutine[Any, Any, R]]],
     Callable[Concatenate[Self, P], Coroutine[Any, Any, R]],
 ]:
-    """
-    Raises:
-        SSMAccessError:
-    """
-
     def decorator(
-        func: Callable[Concatenate[Self, P], Coroutine[Any, Any, R]]
+        func: Callable[Concatenate[Self, P], Coroutine[Any, Any, R]],
     ) -> Callable[Concatenate[Self, P], Coroutine[Any, Any, R]]:
         @functools.wraps(func)
         async def wrapper(self: Self, *args: P.args, **kwargs: P.kwargs) -> R:

packages/service-library/src/servicelib/background_task.py

@@ -10,7 +10,7 @@
 from tenacity.wait import wait_fixed
 
 from .async_utils import cancel_wait_task, delayed_start
-from .logging_utils import log_context
+from .logging_utils import log_catch, log_context
 
 _logger = logging.getLogger(__name__)
 
@@ -84,7 +84,8 @@ class _InternalTryAgain(TryAgain):
         )
         @functools.wraps(func)
         async def _wrapper(*args: P.args, **kwargs: P.kwargs) -> None:
-            await func(*args, **kwargs)
+            with log_catch(_logger, reraise=True):
+                await func(*args, **kwargs)
             raise _InternalTryAgain
 
         return _wrapper

packages/service-library/tests/test_background_task.py

@@ -6,6 +6,7 @@
 
 import asyncio
 import datetime
+import logging
 from collections.abc import AsyncIterator, Awaitable, Callable
 from typing import Final
 from unittest import mock
@@ -204,3 +205,19 @@ async def _func() -> None:
         await task
 
     assert mock_func.call_count > 1
+
+
+async def test_periodic_task_logs_error(
+    mock_background_task: mock.AsyncMock,
+    task_interval: datetime.timedelta,
+    caplog: pytest.LogCaptureFixture,
+):
+    mock_background_task.side_effect = RuntimeError("Test error")
+
+    with caplog.at_level(logging.ERROR):
+        async with periodic_task(
+            mock_background_task, interval=task_interval, task_name="test_task"
+        ):
+            await asyncio.sleep(2 * task_interval.total_seconds())
+
+    assert "Test error" in caplog.text

services/autoscaling/src/simcore_service_autoscaling/modules/auto_scaling_core.py

@@ -338,10 +338,10 @@ async def _sorted_allowed_instance_types(app: FastAPI) -> list[EC2InstanceType]:
         allowed_instance_type_names
     ), "EC2_INSTANCES_ALLOWED_TYPES cannot be empty!"
 
-    allowed_instance_types: list[
-        EC2InstanceType
-    ] = await ec2_client.get_ec2_instance_capabilities(
-        cast(set[InstanceTypeType], set(allowed_instance_type_names))
+    allowed_instance_types: list[EC2InstanceType] = (
+        await ec2_client.get_ec2_instance_capabilities(
+            cast(set[InstanceTypeType], set(allowed_instance_type_names))
+        )
     )
 
     def _as_selection(instance_type: EC2InstanceType) -> int:
@@ -470,14 +470,14 @@ async def _start_warm_buffer_instances(
     with log_context(
         _logger, logging.INFO, f"start {len(instances_to_start)} buffer machines"
     ):
-        await get_ec2_client(app).set_instances_tags(
-            instances_to_start,
-            tags=get_activated_buffer_ec2_tags(app, auto_scaling_mode),
-        )
-
         started_instances = await get_ec2_client(app).start_instances(
             instances_to_start
         )
+        # NOTE: first start the instance and then set the tags in case the instance cannot start (e.g. InsufficientInstanceCapacity)
+        await get_ec2_client(app).set_instances_tags(
+            started_instances,
+            tags=get_activated_buffer_ec2_tags(app, auto_scaling_mode),
+        )
     started_instance_ids = [i.id for i in started_instances]
 
     return dataclasses.replace(
@@ -669,7 +669,7 @@ async def _find_needed_instances(
         "found following %s needed instances: %s",
         len(needed_new_instance_types_for_tasks),
         [
-            f"{i.instance_type.name}:{i.instance_type.resources} takes {len(i.assigned_tasks)} task{'s' if len(i.assigned_tasks)>1 else ''}"
+            f"{i.instance_type.name}:{i.instance_type.resources} takes {len(i.assigned_tasks)} task{'s' if len(i.assigned_tasks) > 1 else ''}"
             for i in needed_new_instance_types_for_tasks
         ],
     )