mirror of
https://github.com/zvx-echo6/navi.git
synced 2026-08-26 17:31:37 +00:00
decouple: drop navi-admin → recon /api/health coupling
Per Matt's directive that navi-* should not call any /api/* on recon. navi-admin was the only navi service doing so (polling recon's /api/health and surfacing it in /api/admin/recon/info + the /api/admin/fleet fan-out). navi-admin is now the navi-only fleet view; recon has its own dashboard for recon-pipeline health. - admin_route.py: delete the /api/admin/recon/info handler; drop the recon config entry + RECON_HEALTH_URL/RECON_REPO_PATH env entries from /api/admin/navi-admin/info; refresh docstrings. - fleet.py: remove recon constants, recon_health_url/recon_repo_path/ recon_git_sha/wrap_recon_health, the now-unused shared.git_sha import, and the recon arms in build_fleet + dependency_summaries. /api/admin/fleet now reports only the 6 navi-* services. - tests: drop the 2 recon/info tests + recon scaffolding; strip recon assertions from fleet + self-info tests. 12 relevant tests pass (10 admin + 2 git_sha). shared/git_sha.py KEPT unchanged — it's a generic git_short_sha(path) helper used by every service's create_app(), not recon-specific. RECON_HEALTH_URL + RECON_REPO_PATH in /etc/navi-backend/navi-admin.env are now dead — flagged for out-of-band post-merge cleanup. Co-authored-by: Matt Johnson <mj@k7zvx.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
GitHub
parent
ae82cee46a
commit
767818b88e
3 changed files with 24 additions and 139 deletions
|
|
@ -1,11 +1,10 @@
|
|||
"""navi-admin routes — the fleet admin front door (extraction #7).
|
||||
|
||||
Net-new (recon has no admin-info to port — Phase A §3). Three routes, all
|
||||
``@require_auth`` (auth is also enforced at the Caddy edge in prod; the Flask
|
||||
gate matches every other navi-* service and protects the localhost :8427 path):
|
||||
Two routes, all ``@require_auth`` (auth is also enforced at the Caddy edge in
|
||||
prod; the Flask gate matches every other navi-* service and protects the
|
||||
localhost :8427 path):
|
||||
|
||||
GET /api/admin/recon/info recon's /api/health wrapped in the info shape
|
||||
GET /api/admin/fleet fan-out aggregator across all navi-* + recon
|
||||
GET /api/admin/fleet fan-out aggregator across all navi-* services
|
||||
GET /api/admin/navi-admin/info navi-admin's own admin-info (self-describe)
|
||||
|
||||
The per-service /api/admin/<svc>/info endpoints stay localhost-only (Phase A
|
||||
|
|
@ -26,19 +25,10 @@ bp = Blueprint('navi_admin', __name__)
|
|||
PORT = 8427
|
||||
|
||||
|
||||
@bp.route('/api/admin/recon/info')
|
||||
@require_auth
|
||||
def recon_info():
|
||||
"""recon's pipeline /api/health, wrapped into the uniform info shape.
|
||||
recon down → a degraded info dict, not a 5xx."""
|
||||
_, health, error = fleet.probe('recon', fleet.recon_health_url(), request.user_id)
|
||||
return jsonify(fleet.wrap_recon_health(health, error))
|
||||
|
||||
|
||||
@bp.route('/api/admin/fleet')
|
||||
@require_auth
|
||||
def fleet_info():
|
||||
"""Fan out to every navi-* service + recon over localhost, merged. Forwards
|
||||
"""Fan out to every navi-* service over localhost, merged. Forwards
|
||||
the caller's X-Authentik-Username so the @require_auth upstreams accept it.
|
||||
Never 5xx — per-service failures land in `errors`."""
|
||||
return jsonify(fleet.build_fleet(request.user_id))
|
||||
|
|
@ -57,12 +47,8 @@ def navi_admin_info():
|
|||
# What it aggregates — non-secret, documents the fleet membership.
|
||||
config={
|
||||
'fanned_services': [{'name': n, 'port': p} for n, p in fleet.SERVICES],
|
||||
'recon': {'name': fleet.RECON_SERVICE_NAME, 'port': fleet.RECON_PORT,
|
||||
'git_sha': fleet.recon_git_sha()},
|
||||
},
|
||||
env=[
|
||||
{'name': 'RECON_HEALTH_URL', 'value': fleet.recon_health_url()},
|
||||
{'name': 'RECON_REPO_PATH', 'value': fleet.recon_repo_path()},
|
||||
{'name': 'NAVI_ADMIN_FANOUT_TIMEOUT_S', 'value': str(fleet.fanout_timeout())},
|
||||
],
|
||||
dependencies=fleet.dependency_summaries(request.user_id),
|
||||
|
|
|
|||
|
|
@ -1,10 +1,10 @@
|
|||
"""Fleet fan-out + recon-health wrapping for navi-admin.
|
||||
"""Fleet fan-out for navi-admin.
|
||||
|
||||
navi-admin is a stateless aggregator: it fans out over localhost to each
|
||||
navi-* service's ``/api/admin/<svc>/info`` endpoint (and recon's pipeline
|
||||
``/api/health``), merging them into one fleet response. Every per-service admin
|
||||
endpoint is ``@require_auth``, so the fan-out forwards the caller's validated
|
||||
``X-Authentik-Username`` header — otherwise the upstreams would 401.
|
||||
navi-* service's ``/api/admin/<svc>/info`` endpoint, merging them into one
|
||||
fleet response. Every per-service admin endpoint is ``@require_auth``, so the
|
||||
fan-out forwards the caller's validated ``X-Authentik-Username`` header —
|
||||
otherwise the upstreams would 401.
|
||||
|
||||
Service discovery: a hardcoded module-level list (Option B). The set of navi-*
|
||||
services changes only when we ship a new extraction — the same moment we'd be
|
||||
|
|
@ -17,8 +17,6 @@ from concurrent.futures import ThreadPoolExecutor
|
|||
|
||||
import requests
|
||||
|
||||
from shared.git_sha import git_short_sha
|
||||
|
||||
# (service-name, port) for every shipped navi-* service. The admin-info path is
|
||||
# always /api/admin/<service-name>/info. Add a row when a new extraction ships.
|
||||
SERVICES = [
|
||||
|
|
@ -30,22 +28,9 @@ SERVICES = [
|
|||
('navi-geo', 8426), # #6 geocode + reverse + reverse bundle
|
||||
]
|
||||
|
||||
RECON_SERVICE_NAME = 'recon'
|
||||
RECON_PORT = 8420
|
||||
|
||||
DEFAULT_RECON_HEALTH_URL = 'http://127.0.0.1:8420/api/health'
|
||||
DEFAULT_RECON_REPO_PATH = '/opt/recon' # actual deploy path on VM 1130 (a git repo)
|
||||
DEFAULT_FANOUT_TIMEOUT_S = 3.0
|
||||
|
||||
|
||||
def recon_health_url():
|
||||
return os.environ.get('RECON_HEALTH_URL', DEFAULT_RECON_HEALTH_URL)
|
||||
|
||||
|
||||
def recon_repo_path():
|
||||
return os.environ.get('RECON_REPO_PATH', DEFAULT_RECON_REPO_PATH)
|
||||
|
||||
|
||||
def fanout_timeout():
|
||||
try:
|
||||
return float(os.environ.get('NAVI_ADMIN_FANOUT_TIMEOUT_S', DEFAULT_FANOUT_TIMEOUT_S))
|
||||
|
|
@ -57,12 +42,6 @@ def service_info_url(name, port):
|
|||
return f'http://127.0.0.1:{port}/api/admin/{name}/info'
|
||||
|
||||
|
||||
def recon_git_sha():
|
||||
"""recon's deployed git SHA, or 'unknown'. Reads from recon_repo_path()
|
||||
(default /opt/recon on VM 1130, Phase A-verified)."""
|
||||
return git_short_sha(recon_repo_path())
|
||||
|
||||
|
||||
def _get_json(url, auth_user, timeout):
|
||||
"""GET url, forwarding the auth header. Returns (json_or_None, latency_ms,
|
||||
error_or_None) where error is 'timeout' | 'HTTP <code>' | exception name."""
|
||||
|
|
@ -98,8 +77,8 @@ def probe(name, url, auth_user, timeout=None):
|
|||
def _degraded_entry(name, port, error):
|
||||
"""Uniform 'service was probed but failed' entry — matches the
|
||||
build_info_response shape so callers see the same keys whether the service is
|
||||
healthy or down. Used for every failure path (navi-* AND recon), so there is
|
||||
exactly one degraded shape: runtime.status == 'unreachable'."""
|
||||
healthy or down. Used for every failure path, so there is exactly one
|
||||
degraded shape: runtime.status == 'unreachable'."""
|
||||
return {
|
||||
'service': name, 'version': 'unknown', 'port': port,
|
||||
'config': {}, 'env': [],
|
||||
|
|
@ -109,43 +88,14 @@ def _degraded_entry(name, port, error):
|
|||
}
|
||||
|
||||
|
||||
def wrap_recon_health(health, error):
|
||||
"""Wrap recon's /api/health into an admin-info-shaped dict (service:'recon').
|
||||
|
||||
recon has no admin-info endpoint (Phase A §3); /api/health is the closest
|
||||
input. On success its components become `dependencies` and its pipeline/status
|
||||
become recon-specific `runtime` fields (meaningful only when recon is up). On
|
||||
failure → the same uniform _degraded_entry shape as every other service (no
|
||||
recon-specific data exists to preserve when recon is unreachable)."""
|
||||
if health is None:
|
||||
return _degraded_entry(RECON_SERVICE_NAME, RECON_PORT, error or 'unreachable')
|
||||
version = recon_git_sha()
|
||||
components = health.get('components', {})
|
||||
dependencies = [
|
||||
{'name': cname, 'status': cval.get('status'), **{k: v for k, v in cval.items() if k != 'status'}}
|
||||
for cname, cval in components.items()
|
||||
]
|
||||
return {
|
||||
'service': RECON_SERVICE_NAME, 'version': version, 'port': RECON_PORT,
|
||||
'config': {}, 'env': [],
|
||||
'dependencies': dependencies,
|
||||
'filesystem': [],
|
||||
'runtime': {
|
||||
'recon_status': health.get('status'),
|
||||
'recon_uptime': health.get('uptime'),
|
||||
'pipeline': health.get('pipeline', {}),
|
||||
},
|
||||
}
|
||||
|
||||
|
||||
def build_fleet(auth_user):
|
||||
"""Fan out to all navi-* services + recon in parallel; merge. Never raises.
|
||||
"""Fan out to all navi-* services in parallel; merge. Never raises.
|
||||
|
||||
Returns {services: {<name>: <info>}, fetched_at: ISO8601, errors: [{service, error}]}.
|
||||
Invariant: EVERY probed service appears in `services` — a full info dict when
|
||||
healthy, the uniform `_degraded_entry` shape (runtime.status == 'unreachable')
|
||||
when it fails — and `errors` is a parallel listing of which ones failed and
|
||||
why. No special cases: recon failures use the same degraded shape."""
|
||||
why."""
|
||||
timeout = fanout_timeout()
|
||||
targets = [(name, port, service_info_url(name, port)) for name, port in SERVICES]
|
||||
|
||||
|
|
@ -155,18 +105,13 @@ def build_fleet(auth_user):
|
|||
|
||||
services = {}
|
||||
errors = []
|
||||
with ThreadPoolExecutor(max_workers=len(targets) + 1) as ex:
|
||||
with ThreadPoolExecutor(max_workers=len(targets)) as ex:
|
||||
futures = [ex.submit(_fetch, name, port, url) for name, port, url in targets]
|
||||
recon_future = ex.submit(probe, RECON_SERVICE_NAME, recon_health_url(), auth_user, timeout)
|
||||
for fut in futures:
|
||||
name, port, full, error = fut.result()
|
||||
services[name] = full if full is not None else _degraded_entry(name, port, error)
|
||||
if error:
|
||||
errors.append({'service': name, 'error': error})
|
||||
_, recon_health, recon_error = recon_future.result()
|
||||
services[RECON_SERVICE_NAME] = wrap_recon_health(recon_health, recon_error)
|
||||
if recon_error:
|
||||
errors.append({'service': RECON_SERVICE_NAME, 'error': recon_error})
|
||||
|
||||
return {
|
||||
'services': services,
|
||||
|
|
@ -176,13 +121,11 @@ def build_fleet(auth_user):
|
|||
|
||||
|
||||
def dependency_summaries(auth_user):
|
||||
"""{name, status, latency_ms[, error]} for recon-health + each navi-* admin
|
||||
endpoint — the same probes the fleet runs, reused for navi-admin's own
|
||||
/info `dependencies`. Sequential (it's a rare, auth-gated call)."""
|
||||
"""{name, status, latency_ms[, error]} for each navi-* admin endpoint — the
|
||||
same probes the fleet runs, reused for navi-admin's own /info
|
||||
`dependencies`. Sequential (it's a rare, auth-gated call)."""
|
||||
timeout = fanout_timeout()
|
||||
summaries = []
|
||||
s, _, _ = probe('recon-health', recon_health_url(), auth_user, timeout)
|
||||
summaries.append(s)
|
||||
for name, port in SERVICES:
|
||||
s, _, _ = probe(name, service_info_url(name, port), auth_user, timeout)
|
||||
summaries.append(s)
|
||||
|
|
|
|||
|
|
@ -3,7 +3,6 @@
|
|||
No live calls: services.navi_admin.fleet.requests.get is mocked to a router
|
||||
keyed on the port in the URL, so we exercise the real _get_json mapping
|
||||
(timeout -> 'timeout', non-200 -> 'HTTP <code>') and the real fan-out/merge.
|
||||
recon_git_sha is stubbed so tests don't depend on /opt/recon.
|
||||
"""
|
||||
import pytest
|
||||
import requests
|
||||
|
|
@ -21,14 +20,6 @@ def _svc_info(name, port):
|
|||
'runtime': {'uptime_s': 1.0, 'request_count': 0, 'last_error_at': None}}
|
||||
|
||||
|
||||
def _recon_health(status='healthy'):
|
||||
return {'status': status, 'uptime': '2026-05-23T00:00:00Z',
|
||||
'components': {'qdrant': {'status': 'up', 'vectors': 42},
|
||||
'tei': {'status': 'up'},
|
||||
'nfs': {'status': 'up'}},
|
||||
'pipeline': {'total': 100, 'done': 90}}
|
||||
|
||||
|
||||
class _FakeResp:
|
||||
def __init__(self, status_code, json_data):
|
||||
self.status_code = status_code
|
||||
|
|
@ -57,15 +48,12 @@ def _router(behaviors, captured_headers):
|
|||
|
||||
|
||||
def _all_ok():
|
||||
"""Every navi-* service 200 + recon health 200."""
|
||||
b = {port: ('ok', _svc_info(name, port)) for name, port in fleet.SERVICES}
|
||||
b[fleet.RECON_PORT] = ('ok', _recon_health())
|
||||
return b
|
||||
"""Every navi-* service 200."""
|
||||
return {port: ('ok', _svc_info(name, port)) for name, port in fleet.SERVICES}
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def client(monkeypatch):
|
||||
monkeypatch.setattr(fleet, 'recon_git_sha', lambda: 'recon99')
|
||||
return create_app().test_client()
|
||||
|
||||
|
||||
|
|
@ -83,12 +71,11 @@ def _wire(monkeypatch, behaviors, captured):
|
|||
def test_fleet_happy_path(client, monkeypatch, captured):
|
||||
_wire(monkeypatch, _all_ok(), captured)
|
||||
data = client.get('/api/admin/fleet', headers=AUTH).get_json()
|
||||
expected = {name for name, _ in fleet.SERVICES} | {'recon'}
|
||||
expected = {name for name, _ in fleet.SERVICES}
|
||||
assert set(data['services'].keys()) == expected
|
||||
assert data['errors'] == []
|
||||
assert data['fetched_at'].endswith('Z')
|
||||
assert data['services']['navi-geo']['port'] == 8426
|
||||
assert data['services']['recon']['runtime']['recon_status'] == 'healthy'
|
||||
|
||||
|
||||
def test_fleet_service_timeout_lands_in_errors(client, monkeypatch, captured):
|
||||
|
|
@ -118,18 +105,14 @@ def test_fleet_forwards_auth_header(client, monkeypatch, captured):
|
|||
assert captured and all(h.get('X-Authentik-Username') == 'matt' for h in captured)
|
||||
|
||||
|
||||
def test_fleet_never_5xx_when_recon_down_and_a_service_errors(client, monkeypatch, captured):
|
||||
def test_fleet_never_5xx_when_a_service_errors(client, monkeypatch, captured):
|
||||
b = _all_ok()
|
||||
b[fleet.RECON_PORT] = 'timeout' # recon health down
|
||||
b[8421] = ('http', 502) # navi-traffic 502
|
||||
_wire(monkeypatch, b, captured)
|
||||
resp = client.get('/api/admin/fleet', headers=AUTH)
|
||||
assert resp.status_code == 200 # never 5xx
|
||||
data = resp.get_json()
|
||||
# recon still present as a degraded entry (same uniform shape), AND in errors
|
||||
assert data['services']['recon']['runtime']['status'] == 'unreachable'
|
||||
assert {'service': 'recon', 'error': 'timeout'} in data['errors']
|
||||
# navi-traffic also present (degraded) AND in errors — the uniform invariant
|
||||
# navi-traffic present (degraded) AND in errors — the uniform invariant
|
||||
assert data['services']['navi-traffic']['runtime']['status'] == 'unreachable'
|
||||
assert {'service': 'navi-traffic', 'error': 'HTTP 502'} in data['errors']
|
||||
|
||||
|
|
@ -157,31 +140,6 @@ def test_fleet_service_returns_html_lands_as_invalid_json(client, monkeypatch, c
|
|||
assert data['services']['navi-places']['runtime']['status'] == 'unreachable'
|
||||
|
||||
|
||||
# ── recon/info wrapper ──────────────────────────────────────────────────────
|
||||
|
||||
def test_recon_info_wraps_health(client, monkeypatch, captured):
|
||||
_wire(monkeypatch, {fleet.RECON_PORT: ('ok', _recon_health())}, captured)
|
||||
data = client.get('/api/admin/recon/info', headers=AUTH).get_json()
|
||||
assert data['service'] == 'recon' and data['port'] == 8420
|
||||
assert data['version'] == 'recon99'
|
||||
dep_names = {d['name'] for d in data['dependencies']}
|
||||
assert {'qdrant', 'tei', 'nfs'} <= dep_names
|
||||
assert data['runtime']['recon_status'] == 'healthy'
|
||||
assert data['runtime']['pipeline'] == {'total': 100, 'done': 90}
|
||||
|
||||
|
||||
def test_recon_info_recon_down_is_degraded_not_5xx(client, monkeypatch, captured):
|
||||
_wire(monkeypatch, {fleet.RECON_PORT: 'timeout'}, captured)
|
||||
resp = client.get('/api/admin/recon/info', headers=AUTH)
|
||||
assert resp.status_code == 200 # degraded, not 5xx
|
||||
data = resp.get_json()
|
||||
assert data['service'] == 'recon'
|
||||
# Uniform degraded shape (same as every other service) — no recon_status special case.
|
||||
assert data['runtime']['status'] == 'unreachable'
|
||||
assert data['dependencies'][0]['status'] == 'error'
|
||||
assert data['dependencies'][0]['name'] == 'recon-info'
|
||||
|
||||
|
||||
# ── self-info ───────────────────────────────────────────────────────────────
|
||||
|
||||
def test_self_info_no_secrets_and_lists_deps(client, monkeypatch, captured):
|
||||
|
|
@ -192,7 +150,6 @@ def test_self_info_no_secrets_and_lists_deps(client, monkeypatch, captured):
|
|||
# No masked secrets anywhere in env (Phase A §9 — none exist).
|
||||
assert all('...' not in str(e['value']) and e['value'] != '****' for e in data['env'])
|
||||
dep_names = {d['name'] for d in data['dependencies']}
|
||||
assert 'recon-health' in dep_names
|
||||
assert {name for name, _ in fleet.SERVICES} <= dep_names
|
||||
|
||||
|
||||
|
|
@ -201,12 +158,11 @@ def test_self_info_config_lists_fanned_services(client, monkeypatch, captured):
|
|||
data = client.get('/api/admin/navi-admin/info', headers=AUTH).get_json()
|
||||
fanned = {s['name']: s['port'] for s in data['config']['fanned_services']}
|
||||
assert fanned == {name: port for name, port in fleet.SERVICES}
|
||||
assert data['config']['recon']['git_sha'] == 'recon99'
|
||||
|
||||
|
||||
# ── auth gating ─────────────────────────────────────────────────────────────
|
||||
|
||||
@pytest.mark.parametrize('path', [
|
||||
'/api/admin/fleet', '/api/admin/recon/info', '/api/admin/navi-admin/info'])
|
||||
'/api/admin/fleet', '/api/admin/navi-admin/info'])
|
||||
def test_auth_required(client, path):
|
||||
assert client.get(path).status_code == 401
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue