From 22d1df6389de9ad797225f02474bc35b2b335ca1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20L=C3=B3pez=20Dato?= Date: Fri, 14 Mar 2025 20:45:20 -0300 Subject: [PATCH 1/4] Add liveness and readiness check endpoints, return 503 when not ready --- src/edge_proxy/server.py | 12 ++++-- tests/test_health.py | 87 ++++++++++++++++++++++++++++++++++++++++ tests/test_server.py | 69 +------------------------------ 3 files changed, 98 insertions(+), 70 deletions(-) create mode 100644 tests/test_health.py diff --git a/src/edge_proxy/server.py b/src/edge_proxy/server.py index 0d26b85..7d03c54 100644 --- a/src/edge_proxy/server.py +++ b/src/edge_proxy/server.py @@ -5,7 +5,7 @@ from fastapi import FastAPI, Header from fastapi.middleware.cors import CORSMiddleware from fastapi.middleware.gzip import GZipMiddleware -from fastapi.responses import ORJSONResponse +from fastapi.responses import ORJSONResponse, Response from edge_proxy.health_check.responses import HealthCheckResponse from fastapi_utils.tasks import repeat_every @@ -40,11 +40,12 @@ async def unknown_key_error(request, exc): @app.get("/health", response_class=ORJSONResponse, deprecated=True) @app.get("/proxy/health", response_class=ORJSONResponse) +@app.get("/proxy/health/readiness", response_class=ORJSONResponse) async def health_check(): last_updated_at = environment_service.last_updated_at if not last_updated_at: return HealthCheckResponse( - status_code=500, + status_code=503, status="error", reason="environment document(s) not updated.", last_successful_update=None, @@ -58,7 +59,7 @@ async def health_check(): ) if last_updated_at < threshold: return HealthCheckResponse( - status_code=500, + status_code=503, status="error", reason="environment document(s) stale.", last_successful_update=last_updated_at, @@ -67,6 +68,11 @@ async def health_check(): return HealthCheckResponse(last_successful_update=last_updated_at) +@app.get("/proxy/health/liveness") +async def liveness_check(): + return Response(status_code=200) + + @app.get("/api/v1/flags/", response_class=ORJSONResponse) async def flags(feature: str = None, x_environment_key: str = Header(None)): try: diff --git a/tests/test_health.py b/tests/test_health.py new file mode 100644 index 0000000..4ac7e9a --- /dev/null +++ b/tests/test_health.py @@ -0,0 +1,87 @@ +from datetime import datetime, timedelta + +import pytest +from fastapi.testclient import TestClient +from pytest_mock import MockerFixture + +from edge_proxy.settings import AppSettings, HealthCheckSettings + +pytestmark = [ + pytest.mark.parametrize( + "endpoint", + [ + "/proxy/health/readiness", + "/proxy/health", + "/health", + ], + ) +] + + +def test_health_check_returns_200_if_cache_was_updated_recently( + mocker: MockerFixture, + client: TestClient, + endpoint: str, +) -> None: + mocked_environment_service = mocker.patch("edge_proxy.server.environment_service") + mocked_environment_service.last_updated_at = datetime.now() + + response = client.get(endpoint) + assert response.status_code == 200 + + +def test_health_check_returns_503_if_cache_was_not_updated( + client: TestClient, + endpoint: str, +) -> None: + response = client.get(endpoint) + assert response.status_code == 503 + assert response.json() == { + "status": "error", + "reason": "environment document(s) not updated.", + "last_successful_update": None, + } + + +def test_health_check_returns_503_if_cache_is_stale( + mocker: MockerFixture, + client: TestClient, + endpoint: str, +) -> None: + last_updated_at = datetime.now() - timedelta(days=10) + mocked_environment_service = mocker.patch("edge_proxy.server.environment_service") + mocked_environment_service.last_updated_at = last_updated_at + response = client.get(endpoint) + assert response.status_code == 503 + assert response.json() == { + "status": "error", + "reason": "environment document(s) stale.", + "last_successful_update": last_updated_at.isoformat(), + } + + +def test_health_check_returns_200_if_cache_is_never_stale( + mocker: MockerFixture, + client: TestClient, + endpoint: str, +) -> None: + # Given + settings = AppSettings( + health_check=HealthCheckSettings(environment_update_grace_period_seconds=None) + ) + mocker.patch("edge_proxy.server.settings", settings) + + last_updated_at = datetime.now() - timedelta(days=10) + mocked_environment_service = mocker.patch("edge_proxy.server.environment_service") + mocked_environment_service.last_updated_at = last_updated_at + + # When + response = client.get(endpoint) + + # Then + assert response.status_code == 200 + assert response.json() == { + "status": "ok", + "reason": None, + "last_successful_update": last_updated_at.isoformat(), + } diff --git a/tests/test_server.py b/tests/test_server.py index 3f5d19e..a49eae9 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -1,83 +1,18 @@ -from datetime import datetime, timedelta import typing import orjson -import pytest from fastapi.testclient import TestClient from pytest_mock import MockerFixture -from edge_proxy.settings import AppSettings, HealthCheckSettings from tests.fixtures.response_data import environment_1 if typing.TYPE_CHECKING: from edge_proxy.environments import EnvironmentService -@pytest.mark.parametrize("endpoint", ["/proxy/health", "/health"]) -def test_health_check_returns_200_if_cache_was_updated_recently( - mocker: MockerFixture, - endpoint: str, - client: TestClient, -) -> None: - mocked_environment_service = mocker.patch("edge_proxy.server.environment_service") - mocked_environment_service.last_updated_at = datetime.now() - - response = client.get(endpoint) - assert response.status_code == 200 - - -def test_health_check_returns_500_if_cache_was_not_updated( - client: TestClient, -) -> None: - response = client.get("/proxy/health") - assert response.status_code == 500 - assert response.json() == { - "status": "error", - "reason": "environment document(s) not updated.", - "last_successful_update": None, - } - - -def test_health_check_returns_500_if_cache_is_stale( - mocker: MockerFixture, - client: TestClient, -) -> None: - last_updated_at = datetime.now() - timedelta(days=10) - mocked_environment_service = mocker.patch("edge_proxy.server.environment_service") - mocked_environment_service.last_updated_at = last_updated_at - response = client.get("/proxy/health") - assert response.status_code == 500 - assert response.json() == { - "status": "error", - "reason": "environment document(s) stale.", - "last_successful_update": last_updated_at.isoformat(), - } - - -def test_health_check_returns_200_if_cache_is_stale_and_health_check_configured_correctly( - mocker: MockerFixture, - client: TestClient, -) -> None: - # Given - settings = AppSettings( - health_check=HealthCheckSettings(environment_update_grace_period_seconds=None) - ) - mocker.patch("edge_proxy.server.settings", settings) - - last_updated_at = datetime.now() - timedelta(days=10) - mocked_environment_service = mocker.patch("edge_proxy.server.environment_service") - mocked_environment_service.last_updated_at = last_updated_at - - # When - response = client.get("/proxy/health") - - # Then +def test_liveness_check(client: TestClient) -> None: + response = client.get("/proxy/health/liveness") assert response.status_code == 200 - assert response.json() == { - "status": "ok", - "reason": None, - "last_successful_update": last_updated_at.isoformat(), - } def test_get_flags( From fadb652e1415a4ba8f1e60faa8dc745765f4eec0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20L=C3=B3pez=20Dato?= Date: Mon, 17 Mar 2025 11:24:07 -0300 Subject: [PATCH 2/4] rename test_health to test_readiness --- tests/{test_health.py => test_readiness.py} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/{test_health.py => test_readiness.py} (100%) diff --git a/tests/test_health.py b/tests/test_readiness.py similarity index 100% rename from tests/test_health.py rename to tests/test_readiness.py From daed4006a4a62bff2631167664a555517a8f0d4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20L=C3=B3pez=20Dato?= Date: Mon, 17 Mar 2025 11:26:05 -0300 Subject: [PATCH 3/4] patch healthcheck, not settings --- tests/test_readiness.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/test_readiness.py b/tests/test_readiness.py index 4ac7e9a..d61fa48 100644 --- a/tests/test_readiness.py +++ b/tests/test_readiness.py @@ -4,7 +4,7 @@ from fastapi.testclient import TestClient from pytest_mock import MockerFixture -from edge_proxy.settings import AppSettings, HealthCheckSettings +from edge_proxy.settings import HealthCheckSettings pytestmark = [ pytest.mark.parametrize( @@ -66,10 +66,8 @@ def test_health_check_returns_200_if_cache_is_never_stale( endpoint: str, ) -> None: # Given - settings = AppSettings( - health_check=HealthCheckSettings(environment_update_grace_period_seconds=None) - ) - mocker.patch("edge_proxy.server.settings", settings) + health_check = HealthCheckSettings(environment_update_grace_period_seconds=None) + mocker.patch("edge_proxy.server.settings.health_check", health_check) last_updated_at = datetime.now() - timedelta(days=10) mocked_environment_service = mocker.patch("edge_proxy.server.environment_service") From 91eaa4b7777d73bd7c7b300be9256b3674f6f836 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rodrigo=20L=C3=B3pez=20Dato?= Date: Wed, 2 Apr 2025 18:43:24 -0300 Subject: [PATCH 4/4] move health tests to test_health --- tests/{test_readiness.py => test_health.py} | 24 +++++++++++++-------- tests/test_server.py | 5 ----- 2 files changed, 15 insertions(+), 14 deletions(-) rename tests/{test_readiness.py => test_health.py} (82%) diff --git a/tests/test_readiness.py b/tests/test_health.py similarity index 82% rename from tests/test_readiness.py rename to tests/test_health.py index d61fa48..bfa8570 100644 --- a/tests/test_readiness.py +++ b/tests/test_health.py @@ -6,18 +6,19 @@ from edge_proxy.settings import HealthCheckSettings -pytestmark = [ - pytest.mark.parametrize( - "endpoint", - [ - "/proxy/health/readiness", - "/proxy/health", - "/health", - ], - ) +READINESS_ENDPOINTS = [ + "/proxy/health/readiness", + "/proxy/health", + "/health", ] +def test_liveness_check(client: TestClient) -> None: + response = client.get("/proxy/health/liveness") + assert response.status_code == 200 + + +@pytest.mark.parametrize("endpoint", READINESS_ENDPOINTS) def test_health_check_returns_200_if_cache_was_updated_recently( mocker: MockerFixture, client: TestClient, @@ -30,6 +31,7 @@ def test_health_check_returns_200_if_cache_was_updated_recently( assert response.status_code == 200 +@pytest.mark.parametrize("endpoint", READINESS_ENDPOINTS) def test_health_check_returns_503_if_cache_was_not_updated( client: TestClient, endpoint: str, @@ -43,6 +45,7 @@ def test_health_check_returns_503_if_cache_was_not_updated( } +@pytest.mark.parametrize("endpoint", READINESS_ENDPOINTS) def test_health_check_returns_503_if_cache_is_stale( mocker: MockerFixture, client: TestClient, @@ -51,7 +54,9 @@ def test_health_check_returns_503_if_cache_is_stale( last_updated_at = datetime.now() - timedelta(days=10) mocked_environment_service = mocker.patch("edge_proxy.server.environment_service") mocked_environment_service.last_updated_at = last_updated_at + response = client.get(endpoint) + assert response.status_code == 503 assert response.json() == { "status": "error", @@ -60,6 +65,7 @@ def test_health_check_returns_503_if_cache_is_stale( } +@pytest.mark.parametrize("endpoint", READINESS_ENDPOINTS) def test_health_check_returns_200_if_cache_is_never_stale( mocker: MockerFixture, client: TestClient, diff --git a/tests/test_server.py b/tests/test_server.py index a49eae9..4791b7d 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -10,11 +10,6 @@ from edge_proxy.environments import EnvironmentService -def test_liveness_check(client: TestClient) -> None: - response = client.get("/proxy/health/liveness") - assert response.status_code == 200 - - def test_get_flags( mocker: MockerFixture, environment_1_feature_states_response_list: list[dict],