🐛 Fixes catalog's synchronization background task continues errors due to faulty service info (#6344)

Author: pcrespov

packages/service-library/src/servicelib/rabbitmq/rpc_interfaces/catalog/services.py

@@ -54,7 +54,7 @@ async def _call(
             user_id=user_id,
             limit=limit,
             offset=offset,
-            timeout_s=20 * RPC_REQUEST_DEFAULT_TIMEOUT_S,
+            timeout_s=40 * RPC_REQUEST_DEFAULT_TIMEOUT_S,
         )
 
     result = await _call(

services/catalog/src/simcore_service_catalog/core/background_tasks.py

@@ -15,12 +15,14 @@
 from pprint import pformat
 from typing import Final
 
-from fastapi import FastAPI
+from fastapi import FastAPI, HTTPException
 from models_library.services import ServiceMetaDataPublished
 from models_library.services_types import ServiceKey, ServiceVersion
 from packaging.version import Version
+from pydantic import ValidationError
 from simcore_service_catalog.api.dependencies.director import get_director_api
 from simcore_service_catalog.services import manifest
+from sqlalchemy.exc import SQLAlchemyError
 from sqlalchemy.ext.asyncio import AsyncEngine
 
 from ..db.repositories.groups import GroupsRepository
@@ -62,32 +64,45 @@ def _by_version(t: tuple[ServiceKey, ServiceVersion]) -> Version:
     sorted_services = sorted(service_keys, key=_by_version)
 
     for service_key, service_version in sorted_services:
+
         service_metadata: ServiceMetaDataPublished = services_in_registry[
             (service_key, service_version)
         ]
-        ## Set deprecation date to null (is valid date value for postgres)
+        try:
+            ## Set deprecation date to null (is valid date value for postgres)
 
-        # DEFAULT policies
-        (
-            owner_gid,
-            service_access_rights,
-        ) = await access_rights.evaluate_default_policy(app, service_metadata)
+            # DEFAULT policies
+            (
+                owner_gid,
+                service_access_rights,
+            ) = await access_rights.evaluate_default_policy(app, service_metadata)
 
-        # AUTO-UPGRADE PATCH policy
-        inherited_access_rights = await access_rights.evaluate_auto_upgrade_policy(
-            service_metadata, services_repo
-        )
+            # AUTO-UPGRADE PATCH policy
+            inherited_access_rights = await access_rights.evaluate_auto_upgrade_policy(
+                service_metadata, services_repo
+            )
 
-        service_access_rights += inherited_access_rights
-        service_access_rights = access_rights.reduce_access_rights(
-            service_access_rights
-        )
+            service_access_rights += inherited_access_rights
+            service_access_rights = access_rights.reduce_access_rights(
+                service_access_rights
+            )
 
-        # set the service in the DB
-        await services_repo.create_or_update_service(
-            ServiceMetaDataAtDB(**service_metadata.dict(), owner=owner_gid),
-            service_access_rights,
-        )
+            # set the service in the DB
+            await services_repo.create_or_update_service(
+                ServiceMetaDataAtDB(**service_metadata.dict(), owner=owner_gid),
+                service_access_rights,
+            )
+
+        except (HTTPException, ValidationError, SQLAlchemyError) as err:
+            # Resilient to single failures: errors in individual (service,key) should not prevent the evaluation of the rest
+            # and stop the background task from running.
+            # SEE https://github.com/ITISFoundation/osparc-simcore/issues/6318
+            _logger.warning(
+                "Skipping '%s:%s' due to %s",
+                service_key,
+                service_version,
+                err,
+            )
 
 
 async def _ensure_registry_and_database_are_synced(app: FastAPI) -> None:

services/catalog/src/simcore_service_catalog/exceptions/errors.py

@@ -16,5 +16,13 @@ class UninitializedGroupError(RepositoryError):
     msg_tempalte = "{group} groups was never initialized"
 
 
-class DirectorUnresponsiveError(CatalogBaseError):
+class BaseDirectorError(CatalogBaseError):
+    ...
+
+
+class DirectorUnresponsiveError(BaseDirectorError):
     msg_template = "Director-v0 is not responsive"
+
+
+class DirectorStatusError(BaseDirectorError):
+    ...

services/catalog/src/simcore_service_catalog/services/access_rights.py

@@ -5,7 +5,7 @@
 import logging
 import operator
 from collections.abc import Callable
-from datetime import datetime, timezone
+from datetime import UTC, datetime
 from typing import Any, cast
 from urllib.parse import quote_plus
 
@@ -25,9 +25,7 @@
 
 _logger = logging.getLogger(__name__)
 
-_LEGACY_SERVICES_DATE: datetime = datetime(
-    year=2020, month=8, day=19, tzinfo=timezone.utc
-)
+_LEGACY_SERVICES_DATE: datetime = datetime(year=2020, month=8, day=19, tzinfo=UTC)
 
 
 def _is_frontend_service(service: ServiceMetaDataPublished) -> bool:
@@ -46,9 +44,6 @@ async def _is_old_service(app: FastAPI, service: ServiceMetaDataPublished) -> bo
     )
     if not data or "build_date" not in data:
         return True
-
-    _logger.debug("retrieved service extras are %s", data)
-
     service_build_data = arrow.get(data["build_date"]).datetime
     return bool(service_build_data < _LEGACY_SERVICES_DATE)
 
@@ -63,6 +58,12 @@ async def evaluate_default_policy(
         1. All services published in osparc prior 19.08.2020 will be visible to everyone (refered as 'old service').
         2. Services published after 19.08.2020 will be visible ONLY to his/her owner
         3. Front-end services are have execute-access to everyone
+
+
+    Raises:
+        HTTPException: from calls to director's rest API. Maps director errors into catalog's server error
+        SQLAlchemyError: from access to pg database
+        ValidationError: from pydantic model errors
     """
     db_engine: AsyncEngine = app.state.engine
 

services/catalog/tests/unit/with_dbs/test_core_background_task__sync.py

@@ -9,7 +9,8 @@
 from typing import Any
 
 import pytest
-from fastapi import FastAPI
+from fastapi import FastAPI, HTTPException, status
+from pytest_mock import MockerFixture
 from respx.router import MockRouter
 from simcore_postgres_database.models.services import services_meta_data
 from simcore_service_catalog.core.background_tasks import _run_sync_services
@@ -40,6 +41,7 @@ async def cleanup_service_meta_data_db_content(sqlalchemy_async_engine: AsyncEng
         await conn.execute(services_meta_data.delete())
 
 
+@pytest.mark.parametrize("director_fails", [False, True])
 async def test_registry_sync_task(
     background_tasks_setup_disabled: None,
     rabbitmq_and_rpc_setup_disabled: None,
@@ -49,10 +51,20 @@ async def test_registry_sync_task(
     app: FastAPI,
     services_repo: ServicesRepository,
     cleanup_service_meta_data_db_content: None,
+    mocker: MockerFixture,
+    director_fails: bool,
 ):
-
     assert app.state
 
+    if director_fails:
+        # Emulates issue https://github.com/ITISFoundation/osparc-simcore/issues/6318
+        mocker.patch(
+            "simcore_service_catalog.services.access_rights._is_old_service",
+            side_effect=HTTPException(
+                status_code=status.HTTP_404_NOT_FOUND, detail="fake director error"
+            ),
+        )
+
     service_key = expected_director_list_services[0]["key"]
     service_version = expected_director_list_services[0]["version"]
 
@@ -75,6 +87,10 @@ async def test_registry_sync_task(
         key=service_key,
         version=service_version,
     )
-    assert got_from_db
-    assert got_from_db.key == service_key
-    assert got_from_db.version == service_version
+
+    if director_fails:
+        assert not got_from_db
+    else:
+        assert got_from_db
+        assert got_from_db.key == service_key
+        assert got_from_db.version == service_version

services/web/server/docker/boot.sh

@@ -38,9 +38,14 @@ echo "$INFO" "Selected config ${APP_CONFIG}"
 echo "$INFO" "Log-level app/server: $APP_LOG_LEVEL/$SERVER_LOG_LEVEL"
 
 # NOTE: the number of workers ```(2 x $num_cores) + 1``` is
-# the official recommendation [https://docs.gunicorn.org/en/latest/design.html#how-many-workers]
+# the official recommendation https://docs.gunicorn.org/en/latest/design.html#how-many-workers
 # For now we set it to 1 to check what happens with websockets
+#
 # SEE also https://docs.aiohttp.org/en/stable/deployment.html#start-gunicorn
+#
+# NOTE: GUNICORN_CMD_ARGS is affecting as well gunicorn
+# SEE https://docs.gunicorn.org/en/latest/settings.html#settings
+echo "$INFO" "GUNICORN_CMD_ARGS: $GUNICORN_CMD_ARGS"
 
 if [ "${SC_BOOT_MODE}" = "debug" ]; then
   # NOTE: ptvsd is programmatically enabled inside of the service