fix(docker): move dedup helper to a model-free module (CI green)
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Jg27rgypiW2efULXJDtMC
This commit is contained in:
@@ -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
|
||||||
@@ -10,6 +10,7 @@ from steward.auth.middleware import require_role
|
|||||||
from steward.models.users import UserRole
|
from steward.models.users import UserRole
|
||||||
from steward.models.hosts import Host
|
from steward.models.hosts import Host
|
||||||
from steward.core.time_range import parse_range, DEFAULT_RANGE
|
from steward.core.time_range import parse_range, DEFAULT_RANGE
|
||||||
|
from .dedup import dedup_by_container_id
|
||||||
from .models import (
|
from .models import (
|
||||||
DockerContainer, DockerDiskUsage, DockerEvent, DockerImage, DockerMetric,
|
DockerContainer, DockerDiskUsage, DockerEvent, DockerImage, DockerMetric,
|
||||||
DockerSwarmNode, DockerSwarmService,
|
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")
|
@docker_bp.get("/widget")
|
||||||
@require_role(UserRole.viewer)
|
@require_role(UserRole.viewer)
|
||||||
async def widget():
|
async def widget():
|
||||||
@@ -233,7 +215,7 @@ async def widget():
|
|||||||
since_24h = datetime.now(timezone.utc) - timedelta(hours=24)
|
since_24h = datetime.now(timezone.utc) - timedelta(hours=24)
|
||||||
|
|
||||||
async with current_app.db_sessionmaker() as db:
|
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(
|
select(DockerContainer).order_by(
|
||||||
(DockerContainer.status == "running").desc(),
|
(DockerContainer.status == "running").desc(),
|
||||||
DockerContainer.name,
|
DockerContainer.name,
|
||||||
@@ -284,7 +266,7 @@ async def widget_resources():
|
|||||||
async with current_app.db_sessionmaker() as db:
|
async with current_app.db_sessionmaker() as db:
|
||||||
# Dedup before the limit, else swarm tasks reported by both managers can
|
# Dedup before the limit, else swarm tasks reported by both managers can
|
||||||
# fill the list with duplicates of the same container.
|
# 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)
|
select(DockerContainer)
|
||||||
.where(DockerContainer.status == "running")
|
.where(DockerContainer.status == "running")
|
||||||
.order_by(DockerContainer.cpu_pct.desc().nullslast())
|
.order_by(DockerContainer.cpu_pct.desc().nullslast())
|
||||||
|
|||||||
@@ -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,
|
the dashboard widget must count it once. Older agents send no container_id,
|
||||||
so those rows can't be matched and are all kept."""
|
so those rows can't be matched and are all kept."""
|
||||||
from types import SimpleNamespace
|
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"):
|
def c(cid, name, status="running"):
|
||||||
return SimpleNamespace(container_id=cid, name=name, status=status)
|
return SimpleNamespace(container_id=cid, name=name, status=status)
|
||||||
|
|
||||||
rows = [c("abc", "web@m1"), c("abc", "web@m2"), # same task, two managers
|
rows = [c("abc", "web@m1"), c("abc", "web@m2"), # same task, two managers
|
||||||
c("def", "db"), c("", "noid-1"), c("", "noid-2")]
|
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
|
assert len(out) == 4 # one "abc" dropped
|
||||||
abc = [r for r in out if r.container_id == "abc"]
|
abc = [r for r in out if r.container_id == "abc"]
|
||||||
|
|||||||
Reference in New Issue
Block a user