🐛 Bug/autoscaling datetime failure (#3976)

packages/service-library/tests/test_docker_utils.py

@@ -1,12 +1,65 @@
-from datetime import datetime
+from datetime import datetime, timezone
 
 import pytest
 from servicelib.docker_utils import to_datetime
 
+NOW = datetime.now(timezone.utc)
+
 
 @pytest.mark.parametrize(
     "docker_time, expected_datetime",
-    [("2020-10-09T12:28:14.771034099Z", datetime(2020, 10, 9, 12, 28, 14, 771034))],
+    [
+        (
+            "2020-10-09T12:28:14.771034099Z",
+            datetime(2020, 10, 9, 12, 28, 14, 771034),
+        ),
+        (NOW.strftime("%Y-%m-%dT%H:%M:%S.%f"), NOW),
+    ],
 )
 def test_to_datetime(docker_time: str, expected_datetime: datetime):
-    assert to_datetime(docker_time) == expected_datetime
+    got_dt = to_datetime(docker_time)
+
+    assert got_dt.replace(tzinfo=timezone.utc) == expected_datetime.replace(
+        tzinfo=timezone.utc
+    )
+
+
+def test_to_strptime_errors():
+    # When “Z” (Zulu) is tacked on the end of a time, it indicates that that time is UTC,
+    # so really the literal Z is part of the time. What is T between date and time?
+    # The T is just a literal to separate the date from the time,
+    # and the Z means “zero hour offset” also known as “Zulu time” (UTC)
+    with pytest.raises(ValueError) as err_info:
+        # ValueError: unconverted data remains: Z
+        # Z at the end (represnting ZULU = UTC) should be removed
+        datetime.strptime("2020-10-09T12:28:14.123456Z", "%Y-%m-%dT%H:%M:%S.%f")
+
+    error_msg = err_info.value.args[0]
+    assert error_msg == "unconverted data remains: Z"
+
+
+def test_to_datetime_known_errors():
+    """
+    Keeps an overview of formatting errors produced by 'to_datetime'
+    """
+    # this works
+    to_datetime("2020-10-09T12:28:14.123456099Z")
+
+    # %f (milliseconds) has here 5 digits (can have at most 6) but to_datetime truncates after 6!
+    # therefore it cannot understand Z
+    with pytest.raises(ValueError) as err_info:
+        # ValueError: unconverted data remains: Z
+        to_datetime("2020-10-09T12:28:14.12345Z")
+
+    error_msg = err_info.value.args[0]
+    assert error_msg == "unconverted data remains: Z"
+
+    # This was the error in pending_service_tasks_with_insufficient_resources
+    # The 'T' is missing between the date and the time stamp
+    with pytest.raises(ValueError) as err_info:
+        # "time data '2023-03-15 13:01:21.774501' does not match format '%Y-%m-%dT%H:%M:%S.%f'"
+        to_datetime(f"{datetime.now(timezone.utc)}")
+
+    error_msg = err_info.value.args[0]
+    assert "time data" in error_msg
+    assert "does not match" in error_msg

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

@@ -7,6 +7,7 @@
 import datetime
 import logging
 import re
+from contextlib import suppress
 from pathlib import Path
 from typing import Final, Optional, cast
 
@@ -135,6 +136,18 @@ async def _associated_service_has_no_node_placement_contraints(
     return True
 
 
+def _by_created_dt(task: Task) -> datetime.datetime:
+    # NOTE: SAFE implementation to extract task.CreatedAt as datetime for comparison
+    if task.CreatedAt:
+        with suppress(ValueError):
+            created_at = to_datetime(task.CreatedAt)
+            created_at_utc: datetime.datetime = created_at.replace(
+                tzinfo=datetime.timezone.utc
+            )
+            return created_at_utc
+    return datetime.datetime.now(datetime.timezone.utc)
+
+
 async def pending_service_tasks_with_insufficient_resources(
     docker_client: AutoscalingDocker,
     service_labels: list[DockerLabelKey],
@@ -157,17 +170,7 @@ async def pending_service_tasks_with_insufficient_resources(
         ),
     )
 
-    sorted_tasks = sorted(
-        tasks,
-        key=lambda task: cast(  # NOTE: some mypy fun here
-            datetime.datetime,
-            (
-                to_datetime(
-                    task.CreatedAt or f"{datetime.datetime.now(datetime.timezone.utc)}"
-                )
-            ),
-        ),
-    )
+    sorted_tasks = sorted(tasks, key=_by_created_dt)
 
     pending_tasks = [
         task

services/autoscaling/tests/unit/test_utils_docker.py

@@ -27,6 +27,7 @@
 from simcore_service_autoscaling.modules.docker import AutoscalingDocker
 from simcore_service_autoscaling.utils.utils_docker import (
     Node,
+    _by_created_dt,
     compute_cluster_total_resources,
     compute_cluster_used_resources,
     compute_node_used_resources,
@@ -397,14 +398,35 @@ async def test_pending_service_task_with_insufficient_resources_properly_sorts_t
         days=1
     )
     for task in pending_tasks:
-        assert task.CreatedAt
+        assert task.CreatedAt  # NOTE: in this case they are but they might be None
         assert (
             to_datetime(task.CreatedAt).replace(tzinfo=datetime.timezone.utc)
             > last_date
         )
         last_date = to_datetime(task.CreatedAt).replace(tzinfo=datetime.timezone.utc)
 
 
+def test_safe_sort_key_callback():
+    tasks_with_faulty_timestamp = [
+        Task(ID=n, CreatedAt=value)  # type: ignore
+        for n, value in enumerate(
+            [
+                # SEE test_to_datetime_conversion_known_errors
+                None,
+                "2023-03-15 09:20:58.123456",
+                "2023-03-15T09:20:58.123456",
+                "2023-03-15T09:20:58.123456Z",
+                f"{datetime.datetime.now(datetime.timezone.utc)}",
+                "corrupted string",
+            ]
+        )
+    ]
+    sorted_tasks = sorted(tasks_with_faulty_timestamp, key=_by_created_dt)
+
+    assert len(sorted_tasks) == len(tasks_with_faulty_timestamp)
+    assert {t.ID for t in sorted_tasks} == {t.ID for t in tasks_with_faulty_timestamp}
+
+
 def test_get_node_total_resources(host_node: Node):
     resources = get_node_total_resources(host_node)
     assert host_node.Description