fix(scan): reconcile scan runs orphaned by a backend restart
A scan runs on a background thread inside the API process. If that process dies mid-scan — an OOM kill, docker stop, a crash — the ScanRun row stays "running" for ever, because nothing is left alive to finish it. That row is not just cosmetic clutter in Scan History: the trigger endpoints reject a new scan while one is "running" for the same target, so a single kill locks that range out permanently. Nothing can legitimately be "running" the moment we boot, so lifespan() now marks every such row "error" — the same word run_scan and run_device_scan write when they fail themselves — with finished_at and an explanatory message. Reported alongside the OOM itself in #374, which is what produced the orphans. Fixes #374 ha-relevant: maybe
This commit is contained in:
committed by
Pouzor - Rémy Jardient
parent
c29680a75f
commit
05c0da53e6
+6
-1
@@ -28,7 +28,8 @@ from app.api.routes import settings as settings_routes
|
||||
from app.core.config import settings
|
||||
from app.core.scheduler import start_scheduler, stop_scheduler
|
||||
from app.core.security import OIDCCSRFMiddleware
|
||||
from app.db.database import init_db
|
||||
from app.db.database import AsyncSessionLocal, init_db
|
||||
from app.services.scanner import reconcile_orphan_runs
|
||||
|
||||
|
||||
@asynccontextmanager
|
||||
@@ -45,6 +46,10 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]:
|
||||
logging.getLogger("app.services.scanner").setLevel(logging.INFO)
|
||||
await init_db()
|
||||
settings.load_overrides()
|
||||
# A scan runs on a background thread in this process, so anything still
|
||||
# flagged "running" now was orphaned by a previous one that died mid-scan.
|
||||
async with AsyncSessionLocal() as db:
|
||||
await reconcile_orphan_runs(db)
|
||||
start_scheduler()
|
||||
yield
|
||||
stop_scheduler()
|
||||
|
||||
@@ -947,3 +947,33 @@ async def run_device_scan(
|
||||
finally:
|
||||
with _cancelled_lock:
|
||||
_cancelled_runs.discard(run_id)
|
||||
|
||||
|
||||
async def reconcile_orphan_runs(db: AsyncSession) -> int:
|
||||
"""
|
||||
Mark every scan run still flagged `running` at startup as errored.
|
||||
|
||||
Scans live on a background thread inside this process, so nothing can
|
||||
legitimately be `running` the moment we boot: any such row belongs to a
|
||||
previous process that died mid-scan (an OOM kill, `docker stop`, a crash).
|
||||
Left alone the row is immortal, and it also locks the target out — the
|
||||
trigger endpoints reject a new scan while one is `running` for the same
|
||||
range, so a single kill blocks re-scanning for good.
|
||||
|
||||
Returns the number of rows reconciled.
|
||||
"""
|
||||
orphans = (
|
||||
await db.execute(select(ScanRun).where(ScanRun.status == "running"))
|
||||
).scalars().all()
|
||||
if not orphans:
|
||||
return 0
|
||||
now = datetime.now(timezone.utc)
|
||||
for run in orphans:
|
||||
# "error" is the word run_scan and run_device_scan already write when
|
||||
# they fail; Scan History filters and colours that one.
|
||||
run.status = "error"
|
||||
run.error = "Interrupted: the backend restarted while this scan was running"
|
||||
run.finished_at = now
|
||||
await db.commit()
|
||||
logger.warning("Reconciled %d scan run(s) left running by a previous process", len(orphans))
|
||||
return len(orphans)
|
||||
|
||||
@@ -8,7 +8,12 @@ from sqlalchemy import select
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
|
||||
from app.db.models import InventoryDevice, Node, ScanRun
|
||||
from app.services.scanner import _cancelled_runs, request_cancel, run_scan
|
||||
from app.services.scanner import (
|
||||
_cancelled_runs,
|
||||
reconcile_orphan_runs,
|
||||
request_cancel,
|
||||
run_scan,
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -468,3 +473,101 @@ async def test_background_device_scan_marks_run_errored_on_exception(mem_db):
|
||||
refreshed = await session.get(ScanRun, run_id)
|
||||
assert refreshed is not None
|
||||
assert refreshed.status == "error"
|
||||
|
||||
|
||||
# --- reconcile_orphan_runs (issue #374) --------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_reconcile_orphan_runs_marks_running_as_error(mem_db):
|
||||
"""A run left "running" by a process that died mid-scan is immortal, and it
|
||||
also locks its range out of future scans. Startup must reconcile it."""
|
||||
async with mem_db() as session:
|
||||
orphan = ScanRun(status="running", ranges=["10.0.0.0/24"])
|
||||
session.add(orphan)
|
||||
await session.commit()
|
||||
orphan_id = orphan.id
|
||||
|
||||
assert await reconcile_orphan_runs(session) == 1
|
||||
|
||||
reconciled = await session.get(ScanRun, orphan_id)
|
||||
# "error", not "failed" — one condition, one name, and the only status
|
||||
# Scan History filters and colours.
|
||||
assert reconciled.status == "error"
|
||||
assert reconciled.finished_at is not None
|
||||
assert "restarted" in reconciled.error
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_reconcile_orphan_runs_leaves_finished_runs_alone(mem_db):
|
||||
"""Only "running" is orphaned. Terminal rows must not be rewritten."""
|
||||
async with mem_db() as session:
|
||||
done = ScanRun(status="done", ranges=["10.0.0.0/24"], devices_found=7)
|
||||
cancelled = ScanRun(status="cancelled", ranges=["10.0.1.0/24"])
|
||||
errored = ScanRun(status="error", ranges=["10.0.2.0/24"], error="nmap missing")
|
||||
session.add_all([done, cancelled, errored])
|
||||
await session.commit()
|
||||
ids = (done.id, cancelled.id, errored.id)
|
||||
|
||||
assert await reconcile_orphan_runs(session) == 0
|
||||
|
||||
for run_id, expected in zip(ids, ("done", "cancelled", "error"), strict=True):
|
||||
assert (await session.get(ScanRun, run_id)).status == expected
|
||||
# The pre-existing error message must survive untouched.
|
||||
assert (await session.get(ScanRun, ids[2])).error == "nmap missing"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_reconcile_orphan_runs_handles_several_and_an_empty_table(mem_db):
|
||||
async with mem_db() as session:
|
||||
# Nothing to do on a fresh database.
|
||||
assert await reconcile_orphan_runs(session) == 0
|
||||
|
||||
session.add_all([
|
||||
ScanRun(status="running", ranges=["10.0.0.0/24"]),
|
||||
ScanRun(status="running", kind="device", ranges=["10.0.0.5"]),
|
||||
ScanRun(status="done", ranges=["10.0.1.0/24"]),
|
||||
])
|
||||
await session.commit()
|
||||
|
||||
assert await reconcile_orphan_runs(session) == 2
|
||||
remaining = (
|
||||
await session.execute(select(ScanRun).where(ScanRun.status == "running"))
|
||||
).scalars().all()
|
||||
assert remaining == []
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_reconcile_orphan_runs_unblocks_a_new_scan(mem_db):
|
||||
"""The point of the reconcile: the trigger endpoints reject a scan while one
|
||||
is "running" for the same target, so an orphan blocks that range for good."""
|
||||
async with mem_db() as session:
|
||||
session.add(ScanRun(status="running", kind="device", ranges=["10.0.0.5"]))
|
||||
await session.commit()
|
||||
|
||||
await reconcile_orphan_runs(session)
|
||||
|
||||
blocking = (
|
||||
await session.execute(
|
||||
select(ScanRun).where(
|
||||
ScanRun.status == "running", ScanRun.kind == "device"
|
||||
)
|
||||
)
|
||||
).scalars().all()
|
||||
assert not any("10.0.0.5" in (r.ranges or []) for r in blocking)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_lifespan_reconciles_orphan_runs_at_startup():
|
||||
"""The reconcile is worthless unless startup actually calls it."""
|
||||
from app.main import app as fastapi_app
|
||||
from app.main import lifespan
|
||||
|
||||
with patch("app.main.init_db", new=AsyncMock()), \
|
||||
patch("app.main.start_scheduler"), \
|
||||
patch("app.main.stop_scheduler"), \
|
||||
patch("app.main.reconcile_orphan_runs", new=AsyncMock()) as reconcile:
|
||||
async with lifespan(fastapi_app):
|
||||
pass
|
||||
|
||||
reconcile.assert_awaited_once()
|
||||
|
||||
Reference in New Issue
Block a user