diff --git a/backend/app/main.py b/backend/app/main.py index e04695c..addc14c 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -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() diff --git a/backend/app/services/scanner.py b/backend/app/services/scanner.py index b437cdc..f067414 100644 --- a/backend/app/services/scanner.py +++ b/backend/app/services/scanner.py @@ -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) diff --git a/backend/tests/scan/test_scan_run.py b/backend/tests/scan/test_scan_run.py index 37793cb..1755a83 100644 --- a/backend/tests/scan/test_scan_run.py +++ b/backend/tests/scan/test_scan_run.py @@ -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()