🐛Fix save state for non-responsive services (ITISFoundation#2883)

* Does not raise if service failed to start task
* adds flaky tester

Author: pcrespov

services/director-v2/requirements/_test.in

@@ -11,33 +11,36 @@
 
 
 # testing
-asgi_lifespan
+flaky
 pytest
 pytest-aiohttp
 pytest-cov
+pytest-docker
+pytest-icdiff
 pytest-mock
 pytest-runner
-pytest-docker
 pytest-xdist
-pytest-icdiff
-async-asgi-testclient
-minio
-aioboto3
 
 # fixtures
+asgi_lifespan
 Faker
 
+# helpers
+aioboto3
+async-asgi-testclient
+minio
+
+
 # migration due to pytest_simcore.postgres_service2
-alembic
 aio_pika
 aioredis
+alembic
 bokeh
 dask-gateway-server[local]
 docker
 respx
 
 # tools
-pylint
-coveralls
 codecov
-ptvsd
+coveralls
+pylint

services/director-v2/requirements/_test.txt

@@ -4,7 +4,7 @@
 #
 #    pip-compile --output-file=requirements/_test.txt --strip-extras requirements/_test.in
 #
-aio_pika==6.8.0
+aio-pika==6.8.0
     # via
     #   -c requirements/_base.txt
     #   -r requirements/_test.in
@@ -129,6 +129,8 @@ execnet==1.9.0
     # via pytest-xdist
 faker==9.8.3
     # via -r requirements/_test.in
+flaky==3.7.0
+    # via -r requirements/_test.in
 frozenlist==1.3.0
     # via
     #   -c requirements/_base.txt
@@ -235,8 +237,6 @@ psycopg2-binary==2.9.2
     # via
     #   -c requirements/_base.txt
     #   sqlalchemy
-ptvsd==4.3.2
-    # via -r requirements/_test.in
 py==1.11.0
     # via
     #   pytest

services/director-v2/requirements/_tools.txt

@@ -4,7 +4,7 @@
 #
 #    pip-compile --output-file=requirements/_tools.txt --strip-extras requirements/_tools.in
 #
-backports.entry-points-selectable==1.1.1
+backports-entry-points-selectable==1.1.1
     # via virtualenv
 black==21.12b0
     # via -r requirements/../../../requirements/devenv.txt

services/director-v2/tests/unit/test_modules_dask_client.py

@@ -641,6 +641,7 @@ def fake_remote_fct(
     )
 
 
+@pytest.mark.flaky
 async def test_failed_task_returns_exceptions(
     dask_client: DaskClient,
     user_id: UserID,

services/director/src/simcore_service_director/producer.py

@@ -1025,7 +1025,11 @@ async def _save_service_state(service_host_name: str, session: aiohttp.ClientSes
             response.raise_for_status()
 
         except ClientResponseError as err:
-            if err.status in (HTTPStatus.METHOD_NOT_ALLOWED, HTTPStatus.NOT_FOUND):
+            if err.status in (
+                HTTPStatus.METHOD_NOT_ALLOWED,
+                HTTPStatus.NOT_FOUND,
+                HTTPStatus.NOT_IMPLEMENTED,
+            ):
                 # NOTE: Legacy Override. Some old services do not have a state entrypoint defined
                 # therefore we assume there is nothing to be saved and do not raise exception
                 # Responses found so far:
@@ -1038,7 +1042,7 @@ async def _save_service_state(service_host_name: str, session: aiohttp.ClientSes
                     err,
                 )
             else:
-                # re-reaise
+                # upss ... could service had troubles saving, reraise
                 raise
         else:
             log.info(
@@ -1090,7 +1094,7 @@ async def stop_service(app: web.Application, node_uuid: str, save_state: bool) -
             else "",
         )
 
-        # If state save is enforced, it will fail if not completed
+        # If state save is enforced
         if save_state:
             log.debug("saving state of service %s...", service_host_name)
             try:
@@ -1101,13 +1105,15 @@ async def stop_service(app: web.Application, node_uuid: str, save_state: bool) -
                 raise ServiceStateSaveError(
                     node_uuid,
                     reason=f"service {service_host_name} rejected to save state, "
-                    f"responded {err.message} (status {err.status})",
+                    f"responded {err.message} (status {err.status})."
+                    "Aborting stop service to prevent data loss.",
                 ) from err
 
             except ClientError as err:
-                raise ServiceStateSaveError(
-                    node_uuid, reason=f"service {service_host_name} unreachable [{err}]"
-                ) from err
+                log.warning(
+                    "Could not save state because {service_host_name} is unreachable [{err}]."
+                    "Resuming stop_service."
+                )
 
         # remove the services
         try: