[DPE-7002] Update opensearch health checks (#215)

akram09 · akram09 · web-flow · commit 11f5c0781523 · 2025-09-24T10:27:40.000+02:00
## Description of issue or feature: This PR addresses the issue mentioned in #177. After initial discussion, it was decided that the opensearch-dashboard charm should stay in a blocked status if the opensearch charm is not healthy or unreachable. This is due to the tight coupling between the two charms. The user is therefore asked to make interventions if the opensearch charm is not in a healthy state. At the current state, we don't make difference between opensearch being sel-healed or restarting and the case of opensearch not working correctly. Therefore this PR makes improvement over the opensearch health checks. ## Solution: - Remove the `app_healthy` function and make calls directly to opensearch_ok to avoid confusions. - Add tenacity to the health checks. - Log the exception happening when json decode error occurs. - Update charm status message when opensearch is in a yellow state. ## How was this change tested? - [X] Manually - [X] Unit tests - [X] Integration tests --------- Co-authored-by: akram09 <akram.boutouchent@eurecom.fr>
diff --git a/src/charm.py b/src/charm.py
@@ -210,9 +210,9 @@ def reconcile(self, event: EventBase) -> None:
 
         # Regular health-check
         # Checks that may modify the 'app' state as well
-        app_healthy, app_msg = self.health_manager.app_healthy()
-        if not app_healthy:
-            set_global_status(self, BlockedStatus(app_msg))
+        opensearch_healthy, opensearch_msg = self.health_manager.opensearch_ok()
+        if not opensearch_healthy:
+            set_global_status(self, BlockedStatus(opensearch_msg))
             return
         else:
             outdated_status += MSG_APP_STATUS
diff --git a/src/literals.py b/src/literals.py
@@ -52,6 +52,7 @@
 MSG_WAITING_FOR_PEER = "waiting for peer relation"
 MSG_STATUS_DB_MISSING = "Opensearch connection is missing"
 MSG_STATUS_DB_DOWN = "Opensearch service is (partially or fully) down"
+MSG_STATUS_DB_UNHEALTHY = "The OpenSearch service health is red"
 MSG_TLS_CONFIG = "Waiting for TLS to be fully configured..."
 MSG_INCOMPATIBLE_UPGRADE = "Incompatible Opensearch and Dashboards versions"
 
@@ -65,6 +66,7 @@
 
 MSG_APP_STATUS = [
     MSG_STATUS_DB_DOWN,
+    MSG_STATUS_DB_UNHEALTHY,
 ]
 
 MSG_UNIT_STATUS = [
diff --git a/src/managers/health.py b/src/managers/health.py
@@ -9,6 +9,8 @@
 
 import requests
 from requests.exceptions import ConnectionError, HTTPError
+from tenacity import Retrying, stop_after_attempt, wait_fixed, RetryCallState
+import urllib3
 
 from core.cluster import SUBSTRATES, ClusterState
 from core.workload import WorkloadBase
@@ -18,6 +20,7 @@
     MSG_STATUS_APP_REMOVED,
     MSG_STATUS_DB_DOWN,
     MSG_STATUS_DB_MISSING,
+    MSG_STATUS_DB_UNHEALTHY,
     MSG_STATUS_ERROR,
     MSG_STATUS_HANGING,
     MSG_STATUS_UNAVAIL,
@@ -78,41 +81,59 @@ def opensearch_ok(self) -> tuple[bool, str]:
         ):
             return False, MSG_STATUS_DB_MISSING
 
+        def log_retry(retry_state: RetryCallState) -> None:
+            """Log retry attempts."""
+            logger.debug(
+                f"Retrying... Attempt {retry_state.attempt_number}"
+                f"\tException: {retry_state.outcome.exception()}"
+            )
+
         for endpoint in self.state.opensearch_server.endpoints:
             full_url = f"https://{endpoint}/{HEALTH_OPENSEARCH_STATUS_URL}"
 
             request_kwargs = {
                 "url": full_url,
                 "method": "GET",
                 "verify": self.workload.paths.opensearch_ca,
-                "headers": None,
+                "headers": {
+                    "Content-Type": "application/json",
+                    "Accept": "application/json",
+                },
                 "timeout": REQUEST_TIMEOUT,
             }
 
-            try:
-                with requests.Session() as s:
+            with requests.Session() as s:
+                try:
                     s.auth = (  # type: ignore [reportAttributeAccessIssue]
                         self.state.opensearch_server.username,
                         self.state.opensearch_server.password,
                     )
-                    resp = s.request(**request_kwargs)
-                    resp.raise_for_status()
-            except requests.exceptions.RequestException:
-                continue
+                    for attempt in Retrying(
+                        stop=stop_after_attempt(3),
+                        wait=wait_fixed(1),
+                        reraise=True,
+                        before_sleep=log_retry,
+                    ):
+                        with attempt:
+                            resp = s.request(**request_kwargs)
+                            resp.raise_for_status()
+                except (requests.RequestException, urllib3.exceptions.HTTPError):
+                    logger.error(f"Failed to connect to {full_url}")
+                    continue
 
             if resp.status_code == 200:
                 try:
                     status = resp.json()
                 except requests.exceptions.JSONDecodeError:
+                    logger.error(f"Failed to decode JSON from {full_url}")
                     continue
-                if status.get("status") == "green":
-                    return True, ""
 
-        return False, MSG_STATUS_DB_DOWN
+                if status.get("status") == "red":
+                    return False, MSG_STATUS_DB_UNHEALTHY
 
-    def app_healthy(self) -> tuple[bool, str]:
-        """Unit-level global healthcheck."""
-        return self.opensearch_ok()
+                if status.get("status") in {"green", "yellow"}:
+                    return True, ""
+        return False, MSG_STATUS_DB_DOWN
 
     def unit_healthy(self) -> tuple[bool, str]:
         """Unit-level global healthcheck."""
diff --git a/tests/integration/application-charm/src/charm.py b/tests/integration/application-charm/src/charm.py
@@ -35,7 +35,7 @@ def __init__(self, *args):
         self.framework.observe(self.on.update_status, self._on_update_status)
 
         # `albums` index is used in integration test
-        self.opensearch = OpenSearchRequires(self, "opensearch-client", "albums", "")
+        self.opensearch = OpenSearchRequires(self, "opensearch-client", "albums", "admin")
 
         self.framework.observe(self.opensearch.on.index_created, self._on_authentication_updated)
         self.framework.observe(
diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py
@@ -11,6 +11,8 @@
 import yaml
 from pytest_operator.plugin import OpsTest
 
+from literals import MSG_STATUS_DB_UNHEALTHY
+
 from .helpers import (
     CONFIG_OPTS,
     DASHBOARD_QUERY_PARAMS,
@@ -316,20 +318,37 @@ async def test_dashboard_status_changes(ops_test: OpsTest):
     opensearch_relation = get_relations(ops_test, OPENSEARCH_RELATION_NAME)[0]
     assert await access_all_dashboards(ops_test, opensearch_relation.id, https=True)
 
-    logger.info("Removing an opensearch unit so Opensearch gets in a 'red' state")
-    await ops_test.model.applications[APP_NAME].destroy_unit(
-        ops_test.model.applications[OPENSEARCH_APP_NAME].units[1].name
+    logger.info(
+        "Adding a new index with shards allocated to a non existent node to make the cluster health red"
     )
-    await ops_test.model.applications[APP_NAME].destroy_unit(
-        ops_test.model.applications[OPENSEARCH_APP_NAME].units[0].name
+    client_relation = get_relations(ops_test, OPENSEARCH_RELATION_NAME, DB_CLIENT_APP_NAME)[0]
+
+    payload = {
+        "settings": {
+            "index.routing.allocation.require._name": "non_existent_node",
+            "index.number_of_shards": 5,
+            "index.number_of_replicas": 0,
+        }
+    }
+
+    payload = json.dumps(payload)
+
+    unit_name = ops_test.model.applications[DB_CLIENT_APP_NAME].units[0].name
+    await client_run_db_request(
+        ops_test,
+        unit_name,
+        client_relation,
+        "PUT",
+        "/bad_index",
+        re.escape(payload),
     )
     async with ops_test.fast_forward("30s"):
         await ops_test.model.wait_for_idle(apps=[APP_NAME], status="blocked")
 
     assert await check_full_status(
         ops_test,
         status="blocked",
-        status_msg="Opensearch service is (partially or fully) down",
+        status_msg=MSG_STATUS_DB_UNHEALTHY,
     )
 
 
diff --git a/tests/unit/test_health.py b/tests/unit/test_health.py
@@ -20,7 +20,7 @@
     OPENSEARCH_REL_NAME,
     SUBSTRATE,
 )
-from src.literals import MSG_STATUS_DB_DOWN, MSG_STATUS_HANGING
+from src.literals import MSG_STATUS_DB_DOWN, MSG_STATUS_HANGING, MSG_STATUS_DB_UNHEALTHY
 from tests.unit.test_charm import MSG_STATUS_UNHEALTHY
 
 logger = logging.getLogger(__name__)
@@ -192,7 +192,11 @@ def test_health_opensearch_not_ok(harness, status):
     )
 
     with patch("os.path.exists", return_value=True), patch("os.path.getsize", return_value=1):
-        assert (False, MSG_STATUS_DB_DOWN) == harness.charm.health_manager.opensearch_ok()
+        # We should make a distinction between unhealthy and down
+        if status == "red":
+            assert (False, MSG_STATUS_DB_UNHEALTHY) == harness.charm.health_manager.opensearch_ok()
+        else:
+            assert (True, "") == harness.charm.health_manager.opensearch_ok()
 
 
 @responses.activate
