From 431f804037740c89bf13c0c181d841a3c1f2ed07 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sat, 20 Jun 2026 23:07:33 -0400 Subject: [PATCH] fix(docker): move dedup helper to a model-free module (CI green) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The widget-dedup test imported plugins.docker.routes, which re-imports plugins.docker.models after the app already loaded the docker plugin — "Table 'docker_containers' is already defined". Same plugin-loader gotcha the host_agent query helpers avoid. Extract dedup_by_container_id into plugins/docker/dedup.py (imports no ORM models, so it's safe to import in either environment); routes.py imports it from there and the test targets the model-free module. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_016Jg27rgypiW2efULXJDtMC --- plugins/docker/dedup.py | 27 +++++++++++++++++++++++++++ plugins/docker/routes.py | 24 +++--------------------- tests/integration/test_docker.py | 4 ++-- 3 files changed, 32 insertions(+), 23 deletions(-) create mode 100644 plugins/docker/dedup.py diff --git a/plugins/docker/dedup.py b/plugins/docker/dedup.py new file mode 100644 index 0000000..5a1e5b7 --- /dev/null +++ b/plugins/docker/dedup.py @@ -0,0 +1,27 @@ +"""Model-free Docker view helpers. + +Kept separate from routes.py/models.py so tests can import it directly: importing +a module that re-imports plugins.docker.models AFTER the app has loaded the docker +plugin raises "Table 'docker_containers' is already defined". This module touches +no ORM models, so it's safe to import in either environment. +""" +from __future__ import annotations + + +def dedup_by_container_id(containers: list) -> list: + """Collapse containers double-reported across hosts. Swarm-aware agents on + every manager list cluster-wide tasks, so the same container (identical + container_id) arrives once per manager and would otherwise be counted N times. + Keep the first occurrence per non-empty container_id — callers order + running-first, so a running report wins over a stale stopped one. Rows without + a container_id (older agents) can't be matched, so they're kept as-is.""" + seen: set[str] = set() + out = [] + for c in containers: + cid = (c.container_id or "").strip() + if cid: + if cid in seen: + continue + seen.add(cid) + out.append(c) + return out diff --git a/plugins/docker/routes.py b/plugins/docker/routes.py index 6b1b9ee..37ab55c 100644 --- a/plugins/docker/routes.py +++ b/plugins/docker/routes.py @@ -10,6 +10,7 @@ from steward.auth.middleware import require_role from steward.models.users import UserRole from steward.models.hosts import Host from steward.core.time_range import parse_range, DEFAULT_RANGE +from .dedup import dedup_by_container_id from .models import ( DockerContainer, DockerDiskUsage, DockerEvent, DockerImage, DockerMetric, DockerSwarmNode, DockerSwarmService, @@ -205,25 +206,6 @@ async def rows(): ) -def _dedup_by_container_id(containers: list) -> list: - """Collapse containers double-reported across hosts. Swarm-aware agents on - every manager list cluster-wide tasks, so the same container (identical - container_id) arrives once per manager and would otherwise be counted N times. - Keep the first occurrence per non-empty container_id — callers order - running-first, so a running report wins over a stale stopped one. Rows without - a container_id (older agents) can't be matched, so they're kept as-is.""" - seen: set[str] = set() - out = [] - for c in containers: - cid = (c.container_id or "").strip() - if cid: - if cid in seen: - continue - seen.add(cid) - out.append(c) - return out - - @docker_bp.get("/widget") @require_role(UserRole.viewer) async def widget(): @@ -233,7 +215,7 @@ async def widget(): since_24h = datetime.now(timezone.utc) - timedelta(hours=24) async with current_app.db_sessionmaker() as db: - all_containers = _dedup_by_container_id(list((await db.execute( + all_containers = dedup_by_container_id(list((await db.execute( select(DockerContainer).order_by( (DockerContainer.status == "running").desc(), DockerContainer.name, @@ -284,7 +266,7 @@ async def widget_resources(): async with current_app.db_sessionmaker() as db: # Dedup before the limit, else swarm tasks reported by both managers can # fill the list with duplicates of the same container. - containers = _dedup_by_container_id(list((await db.execute( + containers = dedup_by_container_id(list((await db.execute( select(DockerContainer) .where(DockerContainer.status == "running") .order_by(DockerContainer.cpu_pct.desc().nullslast()) diff --git a/tests/integration/test_docker.py b/tests/integration/test_docker.py index b54da02..38bc9db 100644 --- a/tests/integration/test_docker.py +++ b/tests/integration/test_docker.py @@ -456,14 +456,14 @@ def test_widget_dedup_collapses_cross_manager_duplicates(): the dashboard widget must count it once. Older agents send no container_id, so those rows can't be matched and are all kept.""" from types import SimpleNamespace - from plugins.docker.routes import _dedup_by_container_id + from plugins.docker.dedup import dedup_by_container_id def c(cid, name, status="running"): return SimpleNamespace(container_id=cid, name=name, status=status) rows = [c("abc", "web@m1"), c("abc", "web@m2"), # same task, two managers c("def", "db"), c("", "noid-1"), c("", "noid-2")] - out = _dedup_by_container_id(rows) + out = dedup_by_container_id(rows) assert len(out) == 4 # one "abc" dropped abc = [r for r in out if r.container_id == "abc"]