From 3aff4106970ede6db51ad163fff8b9d01fc4db1a Mon Sep 17 00:00:00 2001 From: Sylvain <35365065+sanderegg@users.noreply.github.com> Date: Thu, 28 Oct 2021 16:29:29 +0200 Subject: [PATCH] ensure director has started before getting services (#2609) --- .../core/background_tasks.py | 4 +++- .../simcore_service_catalog/services/director.py | 16 ++++++++++------ .../catalog/tests/unit/test_services_director.py | 15 +++++++++++---- services/catalog/tests/unit/with_dbs/conftest.py | 6 +++--- 4 files changed, 27 insertions(+), 14 deletions(-) diff --git a/services/catalog/src/simcore_service_catalog/core/background_tasks.py b/services/catalog/src/simcore_service_catalog/core/background_tasks.py index 143a9b039b0..cc6cb407882 100644 --- a/services/catalog/src/simcore_service_catalog/core/background_tasks.py +++ b/services/catalog/src/simcore_service_catalog/core/background_tasks.py @@ -211,7 +211,9 @@ async def sync_registry_task(app: FastAPI) -> None: "Unexpected error while syncing registry entries, restarting now..." ) # wait a bit before retrying, so it does not block everything until the director is up - await asyncio.sleep(app.state.settings.BACKGROUND_TASK_WAIT_AFTER_FAILURE) + await asyncio.sleep( + app.state.settings.CATALOG_BACKGROUND_TASK_WAIT_AFTER_FAILURE + ) async def start_registry_sync_task(app: FastAPI) -> None: diff --git a/services/catalog/src/simcore_service_catalog/services/director.py b/services/catalog/src/simcore_service_catalog/services/director.py index db8e678ba45..1b3f6f3a257 100644 --- a/services/catalog/src/simcore_service_catalog/services/director.py +++ b/services/catalog/src/simcore_service_catalog/services/director.py @@ -6,27 +6,30 @@ import httpx from fastapi import FastAPI, HTTPException from starlette import status -from tenacity import retry +from tenacity._asyncio import AsyncRetrying from tenacity.before_sleep import before_sleep_log from tenacity.wait import wait_random logger = logging.getLogger(__name__) director_statup_retry_policy = dict( - wait=wait_random(5, 10), + wait=wait_random(1, 10), before_sleep=before_sleep_log(logger, logging.WARNING), reraise=True, ) -@retry(**director_statup_retry_policy) async def setup_director(app: FastAPI) -> None: if settings := app.state.settings.CATALOG_DIRECTOR: # init client-api logger.debug("Setup director at %s...", settings.base_url) director_client = DirectorApi(base_url=settings.base_url, app=app) # check that the director is accessible - await director_client.is_responsive() + async for attempt in AsyncRetrying(**director_statup_retry_policy): + with attempt: + if not await director_client.is_responsive(): + raise ValueError("Director-v0 is not responsive") + logger.info("Connection with director-v0 established") app.state.director_api = director_client @@ -130,9 +133,10 @@ async def put(self, path: str, body: Dict) -> Optional[Dict]: async def is_responsive(self) -> bool: try: + logger.debug("checking director-v0 is responsive") health_check_path: str = "/" - result = await self.client.head(health_check_path) + result = await self.client.head(health_check_path, timeout=1.0) result.raise_for_status() return True - except (httpx.HTTPStatusError, httpx.RequestError): + except (httpx.HTTPStatusError, httpx.RequestError, httpx.TimeoutException): return False diff --git a/services/catalog/tests/unit/test_services_director.py b/services/catalog/tests/unit/test_services_director.py index 9ec55548216..d92938de63a 100644 --- a/services/catalog/tests/unit/test_services_director.py +++ b/services/catalog/tests/unit/test_services_director.py @@ -5,7 +5,7 @@ import asyncio -from typing import Dict, Iterator +from typing import Dict, Iterable, Iterator import pytest import respx @@ -28,10 +28,15 @@ def minimal_app( app = init_app() + yield app + + +@pytest.fixture() +def client(minimal_app: FastAPI) -> Iterable[TestClient]: # NOTE: this way we ensure the events are run in the application # since it starts the app on a test server - with TestClient(app): - yield app + with TestClient(minimal_app) as client: + yield client @pytest.fixture @@ -41,6 +46,7 @@ def mocked_director_service_api(minimal_app: FastAPI) -> MockRouter: assert_all_called=False, assert_all_mocked=True, ) as respx_mock: + respx_mock.head("/", name="healthcheck").respond(200, json={"health": "OK"}) respx_mock.get("/services", name="list_services").respond( 200, json={"data": ["one", "two"]} ) @@ -50,8 +56,9 @@ def mocked_director_service_api(minimal_app: FastAPI) -> MockRouter: async def test_director_client_setup( loop: asyncio.AbstractEventLoop, - minimal_app: FastAPI, mocked_director_service_api: MockRouter, + minimal_app: FastAPI, + client: TestClient, ): # gets director client as used in handlers diff --git a/services/catalog/tests/unit/with_dbs/conftest.py b/services/catalog/tests/unit/with_dbs/conftest.py index 7503147aa52..513582e0b7b 100644 --- a/services/catalog/tests/unit/with_dbs/conftest.py +++ b/services/catalog/tests/unit/with_dbs/conftest.py @@ -5,7 +5,7 @@ import itertools import random -from typing import Any, Callable, Dict, Iterator, List, Tuple +from typing import Any, Callable, Dict, Iterable, Iterator, List, Tuple import pytest import respx @@ -34,14 +34,14 @@ def app( service_test_environ: None, postgres_db: sa.engine.Engine, postgres_host_config: Dict[str, str], -) -> FastAPI: +) -> Iterable[FastAPI]: monkeypatch.setenv("CATALOG_TRACING", "null") app = init_app() yield app @pytest.fixture -def client(app: FastAPI) -> TestClient: +def client(app: FastAPI) -> Iterable[TestClient]: with TestClient(app) as cli: # Note: this way we ensure the events are run in the application yield cli