From bcd0563f3937e7089dd4861010637e5754a0714e Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 6 Oct 2026 18:45:57 -0400 Subject: [PATCH] feat(docker): uptime from StartedAt, a restart is an event, host totals count physical devices only (#5153) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Agent: started_at comes from inspect's State.StartedAt (nanoseconds trimmed; Docker's zero time is "never started"), not the list's Created, so a restarted container's uptime counts from its restart. - A rising restart_count between two snapshots emits a "restart" event ("restart #3", or "3 restarts, now #5"), said instead of a start; a restart then a death gives both. A count that falls (recreated under the same name) says nothing. The timeline draws it as ↻. - Network and disk I/O count what the kernel calls physical: interfaces not under /sys/devices/virtual/net (bridges, docker0, veth, vxlan, tun), block devices with a device behind them (not dm, md, zram), so a byte is not counted twice. Where /sys calls everything virtual (an agent in LXC) the name-filtered set is kept rather than reporting no traffic. - Agent tests updated for #5152's (status, detail, containers) and (is_manager, topology) shapes; the no-Docker disk test now states the case it was passing for by accident. Co-Authored-By: Claude Opus 5.5 --- plugins/docker/ingest.py | 15 ++++- plugins/docker/mcp_tools.py | 2 +- plugins/docker/models.py | 2 +- plugins/docker/routes.py | 1 + plugins/host_agent/agent.py | 59 +++++++++++++++---- tests/plugins/docker/test_ingest_events.py | 23 ++++++++ tests/plugins/host_agent/test_agent_docker.py | 44 +++++++++----- tests/plugins/host_agent/test_collectors.py | 56 +++++++++++++++++- 8 files changed, 170 insertions(+), 32 deletions(-) diff --git a/plugins/docker/ingest.py b/plugins/docker/ingest.py index 1d9b1e4..c9a0857 100644 --- a/plugins/docker/ingest.py +++ b/plugins/docker/ingest.py @@ -34,12 +34,14 @@ def _derive_events(old_state: dict, new_containers: list) -> list: """Diff a fresh snapshot against stored per-container state → lifecycle events. Pure function (no DB) so it's unit-testable. `old_state` maps name → the - previously stored {status, health, oom_killed, exit_code}; `new_containers` + previously stored {status, health, oom_killed, exit_code, restart_count}; `new_containers` is the newest snapshot's list of container dicts. Returns a list of (container_name, event, detail) tuples: start — a container transitions into running (or a genuinely new container appears already running) + restart — Docker restarted it (its restart count went up); said + instead of a start, since that is what happened stop — running → not-running, or a running container is removed die — same as stop but with a non-zero exit code (abnormal) oom — OOMKilled flips False→True @@ -69,7 +71,13 @@ def _derive_events(old_state: dict, new_containers: list) -> list: continue old_running = (old.get("status") or "").lower() == "running" - if new_running and not old_running: + new_restarts = c.get("restart_count") or 0 + old_restarts = old.get("restart_count") or 0 + if new_restarts > old_restarts: + gained = new_restarts - old_restarts + events.append((name, "restart", f"restart #{new_restarts}" if gained == 1 + else f"{gained} restarts, now #{new_restarts}")) + if new_running and not old_running and new_restarts <= old_restarts: events.append((name, "start", c.get("image") or None)) elif old_running and not new_running: if new_exit not in (None, 0): @@ -338,7 +346,8 @@ async def persist_host_docker(session, host, snapshots, swarm=None, disk=None, )).scalars().all() old_state = { r.name: {"status": r.status, "health": r.health, - "oom_killed": r.oom_killed, "exit_code": r.exit_code} + "oom_killed": r.oom_killed, "exit_code": r.exit_code, + "restart_count": r.restart_count} for r in old_rows } if old_state: diff --git a/plugins/docker/mcp_tools.py b/plugins/docker/mcp_tools.py index 31109d8..d366802 100644 --- a/plugins/docker/mcp_tools.py +++ b/plugins/docker/mcp_tools.py @@ -308,7 +308,7 @@ async def get_container(container: str, host: str = "", range: str = "24h", even range: the window for `cpu_pct_history` and `mem_pct_history` (min, avg, max, p95, last and a ≤60-point series) — "1h", "6h", "24h" (default), "7d" or "30d". `cpu_pct` and `mem_pct` are the latest reported values. - events: how many lifecycle events (start, stop, die, oom, health_change) + events: how many lifecycle events (start, restart, stop, die, oom, health_change) to return, newest first (default 20, max 100). A container Docker has since removed (a dead Swarm task, a `docker rm`) diff --git a/plugins/docker/models.py b/plugins/docker/models.py index 7f79164..f074493 100644 --- a/plugins/docker/models.py +++ b/plugins/docker/models.py @@ -146,7 +146,7 @@ class DockerEvent(Base): ) container_name: Mapped[str] = mapped_column(String(255), nullable=False) event: Mapped[str] = mapped_column(String(16), nullable=False) - # start | stop | die | oom | health_change + # start | restart | stop | die | oom | health_change detail: Mapped[str | None] = mapped_column(String(255), nullable=True) # e.g. "exit 137", "healthy→unhealthy", image on start at: Mapped[datetime] = mapped_column( diff --git a/plugins/docker/routes.py b/plugins/docker/routes.py index eeffd3c..6911b40 100644 --- a/plugins/docker/routes.py +++ b/plugins/docker/routes.py @@ -303,6 +303,7 @@ async def host_panel(host_id: str): # Glyph + colour per lifecycle event, so the timeline reads at a glance. _EVENT_STYLE = { "start": ("▲", "var(--green)"), + "restart": ("↻", "var(--yellow)"), "stop": ("■", "var(--text-muted)"), "die": ("✕", "var(--red)"), "oom": ("☠", "var(--red)"), diff --git a/plugins/host_agent/agent.py b/plugins/host_agent/agent.py index d770333..90b01fc 100644 --- a/plugins/host_agent/agent.py +++ b/plugins/host_agent/agent.py @@ -91,6 +91,10 @@ OS_RELEASE_PATH = "/etc/os-release" NETDEV_PATH = "/proc/net/dev" DISKSTATS_PATH = "/proc/diskstats" HWMON_DIR = "/sys/class/hwmon" +# Where the kernel lists virtual network devices (bridges, veth, docker0, +# vxlan, tun, VLANs, bonds) and the block devices (a disk has a `device`). +SYS_VIRTUAL_NET_DIR = "/sys/devices/virtual/net" +SYS_BLOCK_DIR = "/sys/block" PSI_DIR = "/proc/pressure" # Partition names to drop from disk I/O (we report whole-disk throughput only). @@ -259,10 +263,34 @@ def default_mounts() -> list[str]: return mounts or ["/"] +def _physical_only(found: dict, is_virtual) -> dict: + """The entries the kernel says are physical, so a host total counts each + byte once (a bridge's or an LVM volume's traffic is also counted on the + NIC or disk beneath it). When /sys calls every one of them virtual (an + agent inside LXC, where eth0 is a veth), they are all kept: a host + reporting no traffic at all would be wrong, not filtered.""" + physical = {k: v for k, v in found.items() if not is_virtual(k)} + return physical or found + + +def _virtual_net(name: str) -> bool: + """Whether the kernel lists this interface as virtual; False if /sys can't say.""" + return os.path.isdir(SYS_VIRTUAL_NET_DIR) and os.path.exists( + os.path.join(SYS_VIRTUAL_NET_DIR, name)) + + +def _virtual_disk(name: str) -> bool: + """Whether this block device has no disk behind it (dm, md, zram, nbd); + False if /sys can't say.""" + return os.path.isdir(SYS_BLOCK_DIR) and not os.path.exists( + os.path.join(SYS_BLOCK_DIR, name, "device")) + + def collect_net_raw() -> dict: """Return {iface: (rx_bytes, tx_bytes)} cumulative counters from /proc/net/dev. - Skips loopback and veth* (container) interfaces to cut noise. Counters are + Physical interfaces only (see _physical_only); loopback and veth* are + skipped by name too, for a host whose /sys can't be read. Counters are monotonic; per-second rates are derived from deltas in build_sample(). """ out: dict = {} @@ -284,14 +312,15 @@ def collect_net_raw() -> dict: out[name] = (int(f[0]), int(f[8])) # rx_bytes, tx_bytes except ValueError: continue - return out + return _physical_only(out, _virtual_net) def collect_diskio_raw() -> dict: """Return {device: (read_bytes, write_bytes)} cumulative from /proc/diskstats. - Whole disks only — loop/ram/sr/fd and partitions are skipped. Sectors are - 512 bytes. Counters are monotonic; rates derived from deltas. + Physical whole disks only (see _physical_only): loop/ram/sr/fd and + partitions are skipped by name too. Sectors are 512 bytes. Counters are + monotonic; rates derived from deltas. """ out: dict = {} try: @@ -309,7 +338,7 @@ def collect_diskio_raw() -> dict: out[name] = (int(f[5]) * 512, int(f[9]) * 512) # sectors_read, sectors_written except ValueError: continue - return out + return _physical_only(out, _virtual_disk) def collect_temps() -> list: @@ -563,6 +592,18 @@ def _docker_inspect_fields(insp: dict): return health, restart_count, exit_code, oom_killed +def _docker_started_at(insp: dict): + """State.StartedAt as ISO-8601, or None for a container never started + (Docker's zero time) or an inspect that failed. When the container last + started, not when it was created: a restarted container's uptime counts + from its restart.""" + raw = (((insp or {}).get("State") or {}).get("StartedAt") or "") + if not raw or raw.startswith("0001-"): + return None + ts = _parse_log_ts(raw) + return ts.isoformat() if ts else None + + def _docker_net_io(stats: dict): """Return cumulative (net_rx, net_tx, blk_read, blk_write) bytes from stats. @@ -596,7 +637,7 @@ def _collect_one_container(socket_path: str, c: dict) -> dict: # Inspect every container (incl. stopped) — exit codes + restart counts only # live here, and stopped containers are exactly where exit_code matters. - health = exit_code = None + health = exit_code = insp = None restart_count = 0 oom_killed = False try: @@ -616,11 +657,7 @@ def _collect_one_container(socket_path: str, c: dict) -> dict: except (OSError, ValueError): pass - created = c.get("Created") - started_at = ( - datetime.fromtimestamp(created, tz=timezone.utc).isoformat() - if isinstance(created, (int, float)) else None - ) + started_at = _docker_started_at(insp) record = { "name": name, "container_id": cid[:12], diff --git a/tests/plugins/docker/test_ingest_events.py b/tests/plugins/docker/test_ingest_events.py index 72324c1..6921e37 100644 --- a/tests/plugins/docker/test_ingest_events.py +++ b/tests/plugins/docker/test_ingest_events.py @@ -4,6 +4,8 @@ _derive_events is a pure function (no DB), and ingest.py imports its models lazily inside functions, so importing it here doesn't register ORM tables — safe for the no-DB unit lane. """ +import pytest + from plugins.docker.ingest import _derive_events @@ -69,6 +71,27 @@ def test_no_event_when_unchanged(): # ── container-log row shaping (m79, pure) ───────────────────────────────────── +@pytest.mark.parametrize("old_status, old_n, new, events", [ + # Docker restarted it between samples: a restart, not silence (#5153). + ("running", 2, _c("web", "running", restart_count=3), [("web", "restart", "restart #3")]), + # Caught restarting, now back up: a restart, said instead of a start. + ("restarting", 2, _c("web", "running", restart_count=3), [("web", "restart", "restart #3")]), + ("running", 2, _c("web", "running", restart_count=5), [("web", "restart", "3 restarts, now #5")]), + # Restarted, then died for good: both happened. + ("running", 3, _c("web", "exited", restart_count=4, exit_code=1), + [("web", "restart", "restart #4"), ("web", "die", "exit 1")]), + # Started by hand (the count did not move): a start. + ("exited", 2, _c("web", "running", restart_count=2, image="nginx"), [("web", "start", "nginx")]), + # Recreated under the same name, so the count reset: no restart. + ("running", 5, _c("web", "running", restart_count=0), []), + ("running", 2, _c("web", "running", restart_count=2), []), +]) +def test_restart_events(old_status, old_n, new, events): + old = {"web": {"status": old_status, "health": None, "oom_killed": False, "exit_code": None, + "restart_count": old_n}} + assert _derive_events(old, [new]) == events + + def test_log_rows_shapes_and_filters(): from datetime import datetime, timezone from plugins.docker.ingest import _log_rows diff --git a/tests/plugins/host_agent/test_agent_docker.py b/tests/plugins/host_agent/test_agent_docker.py index 401c93c..9c5667e 100644 --- a/tests/plugins/host_agent/test_agent_docker.py +++ b/tests/plugins/host_agent/test_agent_docker.py @@ -148,13 +148,14 @@ def test_collect_docker_assembles_container(monkeypatch): "Names": ["/web"], "Image": "nginx:latest", "State": "running", - "Created": 1_700_000_000, + "Created": 1_600_000_000, "Ports": [{"PublicPort": 8080, "PrivatePort": 80, "Type": "tcp"}], "Labels": {"com.docker.compose.project": "myproj", "com.docker.swarm.service.name": "web"}, } inspect = {"RestartCount": 2, - "State": {"Health": {"Status": "healthy"}, "ExitCode": 0, "OOMKilled": False}} + "State": {"Health": {"Status": "healthy"}, "ExitCode": 0, "OOMKilled": False, + "StartedAt": "2023-11-14T22:13:20.123456789Z"}} stats = { "cpu_stats": {"cpu_usage": {"total_usage": 200_000_000}, "system_cpu_usage": 2_000_000_000, "online_cpus": 4}, @@ -188,7 +189,8 @@ def test_collect_docker_assembles_container(monkeypatch): assert c["cpu_pct"] == 40.0 assert c["mem_pct"] == 15.0 assert c["ports"] == [{"host_port": 8080, "container_port": 80, "protocol": "tcp"}] - assert c["started_at"].startswith("2023-11-14T") # epoch → ISO-8601 + # When it last started (inspect), not when it was created (2020). + assert c["started_at"] == "2023-11-14T22:13:20.123456+00:00" # enrichment assert c["health"] == "healthy" and c["restart_count"] == 2 assert c["exit_code"] == 0 and c["oom_killed"] is False @@ -418,8 +420,9 @@ def test_build_sample_says_when_a_node_is_not_a_manager(monkeypatch, swarm, flag def test_build_sample_omits_swarm_off_manager(monkeypatch): - monkeypatch.setattr(a, "collect_docker", lambda _s: []) - monkeypatch.setattr(a, "collect_swarm", lambda _s: None) + monkeypatch.setattr(a, "collect_docker", lambda _s: ("ok", None, [])) + monkeypatch.setattr(a, "collect_swarm", lambda _s: (False, None)) + monkeypatch.setattr(a, "collect_disk_usage", lambda _s: None) sample = a.build_sample(["/"], {}, "/var/run/docker.sock") assert "swarm" not in sample @@ -470,18 +473,19 @@ def test_collect_disk_usage_silent_skip_when_no_socket(): def test_build_sample_includes_disk_when_docker_present(monkeypatch): - monkeypatch.setattr(a, "collect_docker", lambda _s: [{"name": "web", "status": "running"}]) - monkeypatch.setattr(a, "collect_swarm", lambda _s: None) + monkeypatch.setattr(a, "collect_docker", lambda _s: ("ok", None, [{"name": "web", "status": "running"}])) + monkeypatch.setattr(a, "collect_swarm", lambda _s: (None, None)) monkeypatch.setattr(a, "collect_disk_usage", lambda _s: {"images_reclaimable": 700}) sample = a.build_sample(["/"], {}, "/var/run/docker.sock") assert sample["docker_disk"] == {"images_reclaimable": 700} def test_build_sample_omits_disk_when_no_docker(monkeypatch): - # No containers → the /system/df call is skipped entirely (cost guard). The + # No Docker → the /system/df call is skipped entirely (cost guard). The # sentinel would land under docker_disk if the gate ever called it. - monkeypatch.setattr(a, "collect_docker", lambda _s: []) - monkeypatch.setattr(a, "collect_swarm", lambda _s: None) + monkeypatch.setattr(a, "collect_docker", + lambda _s: ("unavailable", "no Docker socket at /s", [])) + monkeypatch.setattr(a, "collect_swarm", lambda _s: (None, None)) monkeypatch.setattr(a, "collect_disk_usage", lambda _s: {"sentinel": True}) sample = a.build_sample(["/"], {}, "/var/run/docker.sock") assert "docker_disk" not in sample @@ -632,8 +636,8 @@ def test_collect_docker_logs_forgets_gone_container_cursors(monkeypatch): def test_build_sample_includes_logs_when_present(monkeypatch): monkeypatch.setattr(a, "collect_docker", - lambda _s: [{"name": "web", "status": "running"}]) - monkeypatch.setattr(a, "collect_swarm", lambda _s: None) + lambda _s: ("ok", None, [{"name": "web", "status": "running"}])) + monkeypatch.setattr(a, "collect_swarm", lambda _s: (None, None)) monkeypatch.setattr(a, "collect_disk_usage", lambda _s: None) monkeypatch.setattr( a, "collect_docker_logs", @@ -645,8 +649,8 @@ def test_build_sample_includes_logs_when_present(monkeypatch): def test_build_sample_omits_logs_when_disabled(monkeypatch): monkeypatch.setattr(a, "collect_docker", - lambda _s: [{"name": "web", "status": "running"}]) - monkeypatch.setattr(a, "collect_swarm", lambda _s: None) + lambda _s: ("ok", None, [{"name": "web", "status": "running"}])) + monkeypatch.setattr(a, "collect_swarm", lambda _s: (None, None)) monkeypatch.setattr(a, "collect_disk_usage", lambda _s: None) called = {"logs": False} @@ -690,3 +694,15 @@ def test_read_config_parses_docker_log_exclude(tmp_path): p.write_text("url = x\ntoken = y\ndocker_log_exclude = watchtower, foo\n") cfg = a.read_config(str(p)) assert cfg["docker_log_exclude"] == ["watchtower", "foo"] + + +@pytest.mark.parametrize("insp, started", [ + ({"State": {"StartedAt": "2026-10-06T12:00:01.5Z"}}, "2026-10-06T12:00:01.500000+00:00"), + ({"State": {"StartedAt": "2026-10-06T12:00:01Z"}}, "2026-10-06T12:00:01+00:00"), + ({"State": {"StartedAt": "0001-01-01T00:00:00Z"}}, None), # never started + ({"State": {}}, None), + (None, None), # inspect failed + ({"State": {"StartedAt": "yesterday"}}, None), +]) +def test_docker_started_at(insp, started): + assert a._docker_started_at(insp) == started diff --git a/tests/plugins/host_agent/test_collectors.py b/tests/plugins/host_agent/test_collectors.py index 33370d3..40551b7 100644 --- a/tests/plugins/host_agent/test_collectors.py +++ b/tests/plugins/host_agent/test_collectors.py @@ -54,7 +54,9 @@ def test_rates_skips_names_missing_from_prev(): assert agent._rates({"new": (5, 5)}, {}, dt=1.0) == {} -def test_collect_net_raw_skips_lo_and_veth(monkeypatch): +def test_collect_net_raw_skips_lo_and_veth(monkeypatch, tmp_path): + # /sys can't say (no such tree): the name filter alone decides. + monkeypatch.setattr(agent, "SYS_VIRTUAL_NET_DIR", str(tmp_path / "absent")) fixture = ( "Inter-| Receive\n" " face |bytes\n" @@ -66,7 +68,8 @@ def test_collect_net_raw_skips_lo_and_veth(monkeypatch): assert agent.collect_net_raw() == {"eth0": (1000, 500)} -def test_collect_diskio_raw_whole_disks_only(monkeypatch): +def test_collect_diskio_raw_whole_disks_only(monkeypatch, tmp_path): + monkeypatch.setattr(agent, "SYS_BLOCK_DIR", str(tmp_path / "absent")) # fields: major minor name reads rd_merged sectors_read ms ... sectors_written ... fixture = ( " 8 0 sda 1 0 10 0 1 0 20 0 0 0 0\n" @@ -79,6 +82,55 @@ def test_collect_diskio_raw_whole_disks_only(monkeypatch): assert out == {"sda": (10 * 512, 20 * 512), "nvme0n1": (100 * 512, 200 * 512)} +_NETDEV = ( + "Inter-| Receive\n" + " face |bytes\n" + " lo: 100 1 0 0 0 0 0 0 200 2 0 0 0 0 0 0\n" + " eno1: 1000 5 0 0 0 0 0 0 500 4 0 0 0 0 0 0\n" + "docker0: 70 1 0 0 0 0 0 0 80 1 0 0 0 0 0 0\n" + "br-1a2b: 30 1 0 0 0 0 0 0 40 1 0 0 0 0 0 0\n" + "vxlan0: 10 1 0 0 0 0 0 0 20 1 0 0 0 0 0 0\n" +) + + +def _sys_net(tmp_path, virtual): + d = tmp_path / "virtual-net" + for name in virtual: + (d / name).mkdir(parents=True) + return str(d) + + +def test_collect_net_raw_counts_physical_interfaces_only(monkeypatch, tmp_path): + # Bridge, docker0 and vxlan traffic also crosses eno1: counting them too + # would count those bytes twice in the host total (#5153). + monkeypatch.setattr(agent, "SYS_VIRTUAL_NET_DIR", + _sys_net(tmp_path, ["lo", "docker0", "br-1a2b", "vxlan0"])) + monkeypatch.setattr(agent, "_read_file", lambda p: _NETDEV) + assert agent.collect_net_raw() == {"eno1": (1000, 500)} + + +def test_collect_net_raw_keeps_everything_when_sys_calls_it_all_virtual(monkeypatch, tmp_path): + # Inside LXC eth0 is a veth peer: all virtual. No traffic at all would be wrong. + netdev = "h\nh\n eth0: 1000 5 0 0 0 0 0 0 500 4 0 0 0 0 0 0\n" + monkeypatch.setattr(agent, "SYS_VIRTUAL_NET_DIR", _sys_net(tmp_path, ["lo", "eth0"])) + monkeypatch.setattr(agent, "_read_file", lambda p: netdev) + assert agent.collect_net_raw() == {"eth0": (1000, 500)} + + +def test_collect_diskio_raw_counts_physical_disks_only(monkeypatch, tmp_path): + # dm-0 (LVM on sda) and md0 (RAID over sdb, sdc) repeat their members' I/O. + block = tmp_path / "block" + for name in ("sda", "sdb", "nvme0n1"): + (block / name / "device").mkdir(parents=True) + for name in ("dm-0", "md0", "zram0"): + (block / name).mkdir(parents=True) + diskstats = "".join(f" 8 0 {n} 1 0 10 0 1 0 20 0 0 0 0\n" + for n in ("sda", "sdb", "nvme0n1", "dm-0", "md0", "zram0")) + monkeypatch.setattr(agent, "SYS_BLOCK_DIR", str(block)) + monkeypatch.setattr(agent, "_read_file", lambda p: diskstats) + assert sorted(agent.collect_diskio_raw()) == ["nvme0n1", "sda", "sdb"] + + def test_build_sample_first_call_has_no_rates_but_keeps_state(monkeypatch): monkeypatch.setattr(agent, "collect_cpu", lambda: (5.0, [5.0])) monkeypatch.setattr(agent, "collect_memory", lambda: {"total_bytes": 10, "available_bytes": 5})