fix: probe remote gateway via HERMES_API_URL before local fallback (#3312)
Rebased onto master + removed an unused 'io' import flagged by the ruff gate. Co-authored-by: Sanjays2402 <Sanjays2402@users.noreply.github.com>
This commit is contained in:
parent
b6e4899887
commit
329aae6979
2 changed files with 235 additions and 0 deletions
|
|
@ -24,9 +24,14 @@ from __future__ import annotations
|
|||
|
||||
import importlib
|
||||
import json
|
||||
import os
|
||||
import threading
|
||||
import time
|
||||
from datetime import datetime, timezone
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
from urllib import error as urllib_error
|
||||
from urllib import request as urllib_request
|
||||
|
||||
_GATEWAY_PID_FILE = "gateway.pid"
|
||||
_GATEWAY_RUNTIME_STATUS_FILE = "gateway_state.json"
|
||||
|
|
@ -285,6 +290,125 @@ def _runtime_detail_subset(runtime_status: dict[str, Any] | None) -> dict[str, A
|
|||
return details
|
||||
|
||||
|
||||
# Remote-gateway probe (#3281)
|
||||
# ------------------------------------------------------------------
|
||||
# In multi-container Docker deployments the WebUI container does not ship the
|
||||
# ``gateway`` Python package. The lazy ``importlib.import_module("gateway.status")``
|
||||
# therefore raises ``ModuleNotFoundError`` and the payload falls through to
|
||||
# ``gateway_not_configured`` even though ``HERMES_API_URL`` points at a perfectly
|
||||
# reachable remote gateway. The Tasks/Cron banner then shows a spurious amber
|
||||
# "Gateway not configured" warning.
|
||||
#
|
||||
# When ``HERMES_API_URL`` is set we treat that as an explicit declaration that
|
||||
# the gateway lives elsewhere, and probe it over HTTP before touching any local
|
||||
# filesystem / module signal. The probe result is cached briefly so a dashboard
|
||||
# rerender that fans out to multiple panels does not hammer the gateway.
|
||||
|
||||
_REMOTE_PROBE_TIMEOUT_S: float = 2.0
|
||||
_REMOTE_PROBE_CACHE_TTL_S: float = 5.0
|
||||
_REMOTE_PROBE_PATHS: tuple[str, ...] = ("/health", "/status", "/api/gateway/status")
|
||||
|
||||
_remote_probe_lock = threading.Lock()
|
||||
_remote_probe_cache: dict[str, Any] = {"url": None, "expires_at": 0.0, "result": None}
|
||||
|
||||
|
||||
def _remote_gateway_base_url() -> str | None:
|
||||
raw = os.environ.get("HERMES_API_URL")
|
||||
if not isinstance(raw, str):
|
||||
return None
|
||||
url = raw.strip()
|
||||
if not url:
|
||||
return None
|
||||
return url.rstrip("/")
|
||||
|
||||
|
||||
def _http_probe(url: str, timeout_s: float) -> tuple[bool, int | None, str | None]:
|
||||
"""GET ``url`` and return (ok, status_code, error_name).
|
||||
|
||||
``ok`` is True only for a 2xx response. 5xx and network errors are not OK.
|
||||
4xx is also treated as "responded" (the gateway is up, just answering 404
|
||||
on this particular path) so the caller can move on to the next path.
|
||||
"""
|
||||
req = urllib_request.Request(url, method="GET")
|
||||
try:
|
||||
with urllib_request.urlopen(req, timeout=timeout_s) as resp: # noqa: S310 - trusted env var URL
|
||||
status = getattr(resp, "status", None) or resp.getcode()
|
||||
return (200 <= int(status) < 300, int(status), None)
|
||||
except urllib_error.HTTPError as exc:
|
||||
return (False, int(exc.code), "HTTPError")
|
||||
except Exception as exc: # urllib_error.URLError, socket.timeout, ssl, etc.
|
||||
return (False, None, type(exc).__name__)
|
||||
|
||||
|
||||
def _probe_remote_gateway(base_url: str, *, now: float | None = None) -> dict[str, Any]:
|
||||
"""Return an agent-health payload dict for a remote gateway base URL.
|
||||
|
||||
Result is cached for ``_REMOTE_PROBE_CACHE_TTL_S`` seconds per base_url.
|
||||
"""
|
||||
current = time.monotonic() if now is None else now
|
||||
with _remote_probe_lock:
|
||||
if (
|
||||
_remote_probe_cache.get("url") == base_url
|
||||
and _remote_probe_cache.get("expires_at", 0.0) > current
|
||||
and _remote_probe_cache.get("result") is not None
|
||||
):
|
||||
cached = _remote_probe_cache["result"]
|
||||
# Refresh checked_at so the UI shows a current timestamp without
|
||||
# actually re-hitting the gateway.
|
||||
return {**cached, "checked_at": _checked_at()}
|
||||
|
||||
last_status: int | None = None
|
||||
last_error: str | None = None
|
||||
for path in _REMOTE_PROBE_PATHS:
|
||||
ok, status, err = _http_probe(base_url + path, _REMOTE_PROBE_TIMEOUT_S)
|
||||
if ok:
|
||||
payload = {
|
||||
"alive": True,
|
||||
"checked_at": _checked_at(),
|
||||
"details": {
|
||||
"state": "alive",
|
||||
"reason": "remote_gateway",
|
||||
"endpoint": base_url + path,
|
||||
"status_code": status,
|
||||
},
|
||||
}
|
||||
break
|
||||
# Remember the most informative failure signal we saw.
|
||||
if status is not None:
|
||||
last_status = status
|
||||
if err is not None:
|
||||
last_error = err
|
||||
else:
|
||||
details: dict[str, Any] = {
|
||||
"state": "down",
|
||||
"reason": "remote_gateway_unreachable",
|
||||
"endpoint": base_url,
|
||||
}
|
||||
if last_status is not None:
|
||||
details["status_code"] = last_status
|
||||
if last_error is not None:
|
||||
details["error"] = last_error
|
||||
payload = {
|
||||
"alive": False,
|
||||
"checked_at": _checked_at(),
|
||||
"details": details,
|
||||
}
|
||||
|
||||
with _remote_probe_lock:
|
||||
_remote_probe_cache["url"] = base_url
|
||||
_remote_probe_cache["expires_at"] = current + _REMOTE_PROBE_CACHE_TTL_S
|
||||
_remote_probe_cache["result"] = payload
|
||||
return payload
|
||||
|
||||
|
||||
def _reset_remote_probe_cache_for_tests() -> None:
|
||||
"""Test hook: clear the in-process remote-probe cache."""
|
||||
with _remote_probe_lock:
|
||||
_remote_probe_cache["url"] = None
|
||||
_remote_probe_cache["expires_at"] = 0.0
|
||||
_remote_probe_cache["result"] = None
|
||||
|
||||
|
||||
def build_agent_health_payload() -> dict[str, Any]:
|
||||
"""Return `{alive, checked_at, details}` for the Hermes gateway/agent.
|
||||
|
||||
|
|
@ -295,6 +419,16 @@ def build_agent_health_payload() -> dict[str, Any]:
|
|||
probably not configured with a separate gateway process.
|
||||
"""
|
||||
checked_at = _checked_at()
|
||||
|
||||
# Multi-container deployments (#3281): when HERMES_API_URL is set the
|
||||
# gateway lives in another container/host. Probe it over HTTP before
|
||||
# touching local module/pid/state-file signals, otherwise a missing
|
||||
# ``gateway`` Python package in this image masquerades as
|
||||
# "gateway_not_configured" and produces a spurious banner.
|
||||
remote_base = _remote_gateway_base_url()
|
||||
if remote_base is not None:
|
||||
return _probe_remote_gateway(remote_base)
|
||||
|
||||
try:
|
||||
gateway_status = _gateway_status_module()
|
||||
except Exception as exc:
|
||||
|
|
|
|||
101
tests/test_agent_health_remote.py
Normal file
101
tests/test_agent_health_remote.py
Normal file
|
|
@ -0,0 +1,101 @@
|
|||
"""Tests for HERMES_API_URL remote gateway probe (#3281)."""
|
||||
from __future__ import annotations
|
||||
|
||||
from unittest import mock
|
||||
|
||||
import pytest
|
||||
|
||||
from api import agent_health
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _clear_cache():
|
||||
agent_health._reset_remote_probe_cache_for_tests()
|
||||
yield
|
||||
agent_health._reset_remote_probe_cache_for_tests()
|
||||
|
||||
|
||||
class _FakeResp:
|
||||
def __init__(self, status: int = 200):
|
||||
self.status = status
|
||||
|
||||
def getcode(self) -> int:
|
||||
return self.status
|
||||
|
||||
def read(self) -> bytes:
|
||||
return b""
|
||||
|
||||
def __enter__(self):
|
||||
return self
|
||||
|
||||
def __exit__(self, *_a):
|
||||
return False
|
||||
|
||||
|
||||
def test_remote_gateway_healthy_when_200(monkeypatch):
|
||||
monkeypatch.setenv("HERMES_API_URL", "http://gateway:8080")
|
||||
calls: list[str] = []
|
||||
|
||||
def fake_urlopen(req, timeout=None):
|
||||
calls.append(req.full_url)
|
||||
return _FakeResp(200)
|
||||
|
||||
with mock.patch.object(agent_health.urllib_request, "urlopen", fake_urlopen):
|
||||
payload = agent_health.build_agent_health_payload()
|
||||
|
||||
assert payload["alive"] is True
|
||||
assert payload["details"]["reason"] == "remote_gateway"
|
||||
assert payload["details"]["status_code"] == 200
|
||||
assert calls and calls[0].startswith("http://gateway:8080/")
|
||||
|
||||
|
||||
def test_remote_gateway_unreachable_when_network_error(monkeypatch):
|
||||
monkeypatch.setenv("HERMES_API_URL", "http://gateway:8080/")
|
||||
|
||||
def fake_urlopen(req, timeout=None):
|
||||
raise OSError("connection refused")
|
||||
|
||||
with mock.patch.object(agent_health.urllib_request, "urlopen", fake_urlopen):
|
||||
payload = agent_health.build_agent_health_payload()
|
||||
|
||||
assert payload["alive"] is False
|
||||
assert payload["details"]["reason"] == "remote_gateway_unreachable"
|
||||
assert payload["details"]["endpoint"] == "http://gateway:8080"
|
||||
assert "error" in payload["details"]
|
||||
|
||||
|
||||
def test_falls_back_to_local_when_no_env(monkeypatch):
|
||||
monkeypatch.delenv("HERMES_API_URL", raising=False)
|
||||
|
||||
# Force the local importlib path to fail so we hit the well-known
|
||||
# "gateway_not_configured" terminal state — proving the remote probe was
|
||||
# NOT invoked and the legacy local path ran.
|
||||
def boom(name):
|
||||
raise ModuleNotFoundError(name)
|
||||
|
||||
with mock.patch.object(agent_health.importlib, "import_module", boom):
|
||||
payload = agent_health.build_agent_health_payload()
|
||||
|
||||
assert payload["alive"] is None
|
||||
assert payload["details"]["reason"] == "gateway_status_unavailable"
|
||||
|
||||
|
||||
def test_remote_probe_result_cached_for_5s(monkeypatch):
|
||||
monkeypatch.setenv("HERMES_API_URL", "http://gateway:8080")
|
||||
call_count = {"n": 0}
|
||||
|
||||
def fake_urlopen(req, timeout=None):
|
||||
call_count["n"] += 1
|
||||
return _FakeResp(200)
|
||||
|
||||
with mock.patch.object(agent_health.urllib_request, "urlopen", fake_urlopen):
|
||||
first = agent_health.build_agent_health_payload()
|
||||
second = agent_health.build_agent_health_payload()
|
||||
|
||||
assert first["alive"] is True
|
||||
assert second["alive"] is True
|
||||
assert second["details"]["reason"] == "remote_gateway"
|
||||
# Second call must NOT have hit the network.
|
||||
assert call_count["n"] == 1
|
||||
# checked_at is refreshed even on cache hit so the UI shows a current time.
|
||||
assert "checked_at" in second
|
||||
Loading…
Reference in a new issue