fix(node-agent): R1/R2/R3 — stop deleting deliberately stopped containers
R1 (critical): _prune_stopped_containers no longer calls containers.prune().
The unfiltered API call removes EVERY non-running container, ignoring
RestartPolicy and compose labels — that is what deleted ollama@solaria 19 s
after an operator `docker stop`. No filter argument can fix it (`until` filters
on creation time, not stop time). Replaced with explicit enumeration over
containers.list(all=True, filters={"status": "exited"}), skipping anything with
restart policy unless-stopped/always/on-failure or a com.docker.compose.project
label. A stopped managed service is recorded operator intent and now belongs to
the module's NEVER TOUCHED list; only one-off leftovers are removed. Dangling
image and build cache prune are unchanged.
R2 (high): _sd_card_rate_ok → _cleanup_rate_ok, applied to every cleanup-
eligible node type. ai_node and standard pruned every 60 s (1440/day, "0 MB
reclaimed" in practically every cycle) with no rate limit at all; they now share
sd_card's CLEANUP_INTERVAL_SECS (24 h) and mark the cleanup timestamp.
R3 (medium): removals are named in the log — WARNING with the container names
when the list is non-empty, INFO otherwise. The old line reported megabytes
only, which is why ollama's deletion looked identical to every no-op cycle.
Tests: services/node-agent/tests/test_safe_cleanup.py (22 cases) — the ollama
regression (exited + unless-stopped survives, prune() never called), all three
restart policies, compose-label protection, disposable-leftover removal, sweep
continues past a failed remove, WARNING-level naming, and the rate limit for
ai_node/standard/sd_card plus lte_node still doing nothing.
Refs docs/incidents/2026-07-30-ollama-solaria-vanish.md §7 R1-R3.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-04 14:55:26 +02:00
|
|
|
"""Tests for NodeAgent Docker cleanup — regression cover for the 2026-07-30
|
2026-08-04 17:02:12 +02:00
|
|
|
unfiltered-prune incident (kb/incidents/2026-07-30-ollama-solaria-vanish.md).
|
fix(node-agent): R1/R2/R3 — stop deleting deliberately stopped containers
R1 (critical): _prune_stopped_containers no longer calls containers.prune().
The unfiltered API call removes EVERY non-running container, ignoring
RestartPolicy and compose labels — that is what deleted ollama@solaria 19 s
after an operator `docker stop`. No filter argument can fix it (`until` filters
on creation time, not stop time). Replaced with explicit enumeration over
containers.list(all=True, filters={"status": "exited"}), skipping anything with
restart policy unless-stopped/always/on-failure or a com.docker.compose.project
label. A stopped managed service is recorded operator intent and now belongs to
the module's NEVER TOUCHED list; only one-off leftovers are removed. Dangling
image and build cache prune are unchanged.
R2 (high): _sd_card_rate_ok → _cleanup_rate_ok, applied to every cleanup-
eligible node type. ai_node and standard pruned every 60 s (1440/day, "0 MB
reclaimed" in practically every cycle) with no rate limit at all; they now share
sd_card's CLEANUP_INTERVAL_SECS (24 h) and mark the cleanup timestamp.
R3 (medium): removals are named in the log — WARNING with the container names
when the list is non-empty, INFO otherwise. The old line reported megabytes
only, which is why ollama's deletion looked identical to every no-op cycle.
Tests: services/node-agent/tests/test_safe_cleanup.py (22 cases) — the ollama
regression (exited + unless-stopped survives, prune() never called), all three
restart policies, compose-label protection, disposable-leftover removal, sweep
continues past a failed remove, WARNING-level naming, and the rate limit for
ai_node/standard/sd_card plus lte_node still doing nothing.
Refs docs/incidents/2026-07-30-ollama-solaria-vanish.md §7 R1-R3.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-04 14:55:26 +02:00
|
|
|
|
|
|
|
|
`containers.prune()` removes EVERY non-running container, ignoring restart
|
|
|
|
|
policy and compose labels; that is what deleted `ollama` on SOLARIA 19 s after
|
|
|
|
|
an operator stopped it. These tests pin down the replacement:
|
|
|
|
|
|
|
|
|
|
R1 — explicit enumeration; a stopped container with `restart: unless-stopped`
|
|
|
|
|
or a compose project label is never removed, and prune() is never called.
|
|
|
|
|
R2 — the cleanup rate limit applies to ai_node / standard, not sd_card only.
|
|
|
|
|
R3 — removals are named in a WARNING log line.
|
|
|
|
|
"""
|
|
|
|
|
from __future__ import annotations
|
|
|
|
|
|
|
|
|
|
import time
|
|
|
|
|
from unittest.mock import MagicMock
|
|
|
|
|
|
|
|
|
|
import pytest
|
|
|
|
|
|
|
|
|
|
import node_agent
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
# Fake Docker container helper
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
|
|
|
|
def make_stopped(name, *, restart_policy="no", compose_project=None):
|
|
|
|
|
c = MagicMock()
|
|
|
|
|
c.name = name
|
|
|
|
|
c.status = "exited"
|
|
|
|
|
c.attrs = {"HostConfig": {"RestartPolicy": {"Name": restart_policy}}}
|
|
|
|
|
c.labels = {"com.docker.compose.project": compose_project} if compose_project else {}
|
|
|
|
|
return c
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
def with_containers(agent, containers):
|
|
|
|
|
client = MagicMock()
|
|
|
|
|
client.containers.list.return_value = containers
|
|
|
|
|
agent.docker_client = client
|
|
|
|
|
return client
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
# R1 — the incident itself
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
|
|
|
|
def test_unless_stopped_container_survives(agent):
|
|
|
|
|
"""THE regression: ollama@solaria — exited, restart=unless-stopped, must live."""
|
|
|
|
|
ollama = make_stopped("ollama", restart_policy="unless-stopped")
|
|
|
|
|
client = with_containers(agent, [ollama])
|
|
|
|
|
|
|
|
|
|
agent._prune_stopped_containers()
|
|
|
|
|
|
|
|
|
|
ollama.remove.assert_not_called()
|
|
|
|
|
client.containers.prune.assert_not_called()
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
@pytest.mark.parametrize("policy", ["unless-stopped", "always", "on-failure"])
|
|
|
|
|
def test_every_restart_policy_is_operator_intent(agent, policy):
|
|
|
|
|
c = make_stopped("svc", restart_policy=policy)
|
|
|
|
|
with_containers(agent, [c])
|
|
|
|
|
agent._prune_stopped_containers()
|
|
|
|
|
c.remove.assert_not_called()
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
def test_compose_managed_container_survives(agent):
|
|
|
|
|
"""No restart policy, but compose owns it → still off limits."""
|
|
|
|
|
c = make_stopped("outline-redis-1", restart_policy="no", compose_project="outline")
|
|
|
|
|
with_containers(agent, [c])
|
|
|
|
|
agent._prune_stopped_containers()
|
|
|
|
|
c.remove.assert_not_called()
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
def test_disposable_leftover_is_removed(agent):
|
|
|
|
|
"""restart=no and no compose project → a one-off leftover, safe to remove."""
|
|
|
|
|
c = make_stopped("nervous-shell-42", restart_policy="no")
|
|
|
|
|
with_containers(agent, [c])
|
|
|
|
|
agent._prune_stopped_containers()
|
|
|
|
|
c.remove.assert_called_once()
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
def test_only_exited_containers_are_enumerated(agent):
|
|
|
|
|
"""Running containers must never even enter the candidate list."""
|
|
|
|
|
client = with_containers(agent, [])
|
|
|
|
|
agent._prune_stopped_containers()
|
|
|
|
|
_args, kwargs = client.containers.list.call_args
|
|
|
|
|
assert kwargs["all"] is True
|
|
|
|
|
assert kwargs["filters"] == {"status": "exited"}
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
def test_mixed_set_removes_only_the_disposable_one(agent):
|
|
|
|
|
keep_policy = make_stopped("ollama", restart_policy="unless-stopped")
|
|
|
|
|
keep_compose = make_stopped("umami-db", compose_project="umami")
|
|
|
|
|
drop = make_stopped("tmp-build", restart_policy="no")
|
|
|
|
|
with_containers(agent, [keep_policy, keep_compose, drop])
|
|
|
|
|
|
|
|
|
|
agent._prune_stopped_containers()
|
|
|
|
|
|
|
|
|
|
keep_policy.remove.assert_not_called()
|
|
|
|
|
keep_compose.remove.assert_not_called()
|
|
|
|
|
drop.remove.assert_called_once()
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
def test_remove_failure_does_not_abort_the_sweep(agent):
|
|
|
|
|
boom = make_stopped("boom", restart_policy="no")
|
|
|
|
|
boom.remove.side_effect = RuntimeError("device or resource busy")
|
|
|
|
|
later = make_stopped("later", restart_policy="no")
|
|
|
|
|
with_containers(agent, [boom, later])
|
|
|
|
|
|
|
|
|
|
agent._prune_stopped_containers()
|
|
|
|
|
|
|
|
|
|
later.remove.assert_called_once()
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
def test_no_docker_client_is_noop(agent):
|
|
|
|
|
agent.docker_client = None
|
|
|
|
|
agent._prune_stopped_containers() # must not raise
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
# R3 — say what was deleted
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
|
|
|
|
def test_removal_is_logged_at_warning_with_names(agent, caplog):
|
|
|
|
|
with_containers(agent, [make_stopped("tmp-build", restart_policy="no")])
|
|
|
|
|
|
|
|
|
|
with caplog.at_level("INFO", logger="node-agent"):
|
|
|
|
|
agent._prune_stopped_containers()
|
|
|
|
|
|
|
|
|
|
warnings = [r for r in caplog.records if r.levelname == "WARNING"]
|
|
|
|
|
assert len(warnings) == 1
|
|
|
|
|
assert "tmp-build" in warnings[0].message
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
def test_nothing_removed_stays_at_info(agent, caplog):
|
|
|
|
|
with_containers(agent, [make_stopped("ollama", restart_policy="unless-stopped")])
|
|
|
|
|
|
|
|
|
|
with caplog.at_level("INFO", logger="node-agent"):
|
|
|
|
|
agent._prune_stopped_containers()
|
|
|
|
|
|
|
|
|
|
assert [r for r in caplog.records if r.levelname == "WARNING"] == []
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
# R2 — rate limit covers every node type
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
|
|
|
|
@pytest.fixture
|
|
|
|
|
def cleanup_calls(agent, monkeypatch):
|
|
|
|
|
"""Record which prune helpers run, without touching Docker."""
|
|
|
|
|
calls = []
|
|
|
|
|
for helper in ("_prune_dangling_images", "_prune_stopped_containers",
|
|
|
|
|
"_prune_build_cache"):
|
|
|
|
|
monkeypatch.setattr(agent, helper, lambda h=helper: calls.append(h))
|
|
|
|
|
return calls
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
@pytest.fixture
|
|
|
|
|
def fresh_cleanup_marker(tmp_path, monkeypatch):
|
|
|
|
|
monkeypatch.setattr(node_agent, "LAST_CLEANUP_FILE", tmp_path / "last-docker-cleanup")
|
|
|
|
|
return node_agent.LAST_CLEANUP_FILE
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
@pytest.mark.parametrize("node_type", ["ai_node", "standard", "sd_card"])
|
|
|
|
|
def test_recent_cleanup_blocks_next_run(agent, cleanup_calls, fresh_cleanup_marker,
|
|
|
|
|
node_type):
|
|
|
|
|
"""Was true for sd_card only; after R2 it holds for ai_node and standard too."""
|
|
|
|
|
agent.node_type = node_type
|
|
|
|
|
fresh_cleanup_marker.write_text(str(int(time.time())))
|
|
|
|
|
|
|
|
|
|
agent.run_safe_cleanup()
|
|
|
|
|
|
|
|
|
|
assert cleanup_calls == []
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
@pytest.mark.parametrize("node_type,expected", [
|
|
|
|
|
("ai_node", ["_prune_dangling_images", "_prune_stopped_containers",
|
|
|
|
|
"_prune_build_cache"]),
|
|
|
|
|
("standard", ["_prune_dangling_images", "_prune_stopped_containers",
|
|
|
|
|
"_prune_build_cache"]),
|
|
|
|
|
("sd_card", ["_prune_dangling_images", "_prune_stopped_containers"]),
|
|
|
|
|
])
|
|
|
|
|
def test_stale_marker_allows_cleanup_and_is_refreshed(agent, cleanup_calls,
|
|
|
|
|
fresh_cleanup_marker,
|
|
|
|
|
node_type, expected):
|
|
|
|
|
agent.node_type = node_type
|
|
|
|
|
stale = int(time.time()) - node_agent.CLEANUP_INTERVAL_SECS - 1
|
|
|
|
|
fresh_cleanup_marker.write_text(str(stale))
|
|
|
|
|
|
|
|
|
|
agent.run_safe_cleanup()
|
|
|
|
|
|
|
|
|
|
assert cleanup_calls == expected
|
|
|
|
|
# Marker refreshed for every type, otherwise the guard never engages.
|
|
|
|
|
assert int(fresh_cleanup_marker.read_text()) > stale
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
def test_lte_node_still_does_nothing(agent, cleanup_calls, fresh_cleanup_marker):
|
|
|
|
|
agent.node_type = "lte_node"
|
|
|
|
|
agent.run_safe_cleanup()
|
|
|
|
|
assert cleanup_calls == []
|
|
|
|
|
assert not fresh_cleanup_marker.exists()
|