maintenance: preserve active pod logs during cleanup

services/maintenance/node-ops/kustomization.yaml

@@ -43,6 +43,7 @@ configMapGenerator:
     namespace: maintenance
     files:
       - node_image_sweeper.sh=scripts/node_image_sweeper.sh
+      - node_pod_log_cleanup.py=scripts/node_pod_log_cleanup.py
     options:
       disableNameSuffixHash: true
   - name: titan-24-docker-script

services/maintenance/node-ops/node-image-sweeper-daemonset.yaml

@@ -11,9 +11,11 @@ spec:
   updateStrategy:
     type: RollingUpdate
     rollingUpdate:
-      maxUnavailable: 100%
+      maxUnavailable: 1
   template:
     metadata:
+      annotations:
+        atlas.bstein.dev/config-revision: "2026-10-04-preserve-active-pod-logs"
       labels:
         app: node-image-sweeper
     spec:

services/maintenance/node-ops/scripts/node_image_sweeper.sh

@@ -9,90 +9,6 @@ LOG_RETENTION_DAYS=${LOG_RETENTION_DAYS:-7}
 ORPHAN_POD_RETENTION_DAYS=${ORPHAN_POD_RETENTION_DAYS:-3}
 JOURNAL_MAX_SIZE=${JOURNAL_MAX_SIZE:-200M}
 
-cleanup_orphaned_hdd_pod_logs() {
-  if [ ! -d /host/var/log.hdd/pods ]; then
-    return 0
-  fi
-
-  ORPHAN_POD_RETENTION_DAYS="${ORPHAN_POD_RETENTION_DAYS}" python3 - <<'PY'
-import os
-import shutil
-import time
-
-hdd_pods = "/host/var/log.hdd/pods"
-active_pods = "/host/var/log/pods"
-retention_days = int(os.environ.get("ORPHAN_POD_RETENTION_DAYS", "3"))
-cutoff = time.time() - (retention_days * 86400)
-
-try:
-    active_names = set(os.listdir(active_pods))
-except Exception:
-    active_names = set()
-
-try:
-    hdd_names = os.listdir(hdd_pods)
-except Exception:
-    hdd_names = []
-
-for name in hdd_names:
-    path = os.path.join(hdd_pods, name)
-    if not os.path.isdir(path):
-        continue
-    if name in active_names:
-        continue
-    try:
-        mtime = os.path.getmtime(path)
-    except Exception:
-        continue
-    if mtime > cutoff:
-        continue
-    print(path)
-    shutil.rmtree(path, ignore_errors=True)
-PY
-}
-
-cleanup_orphaned_root_pod_logs() {
-  if [ ! -d /host/var/log/pods ] || [ ! -d /host/var/lib/kubelet/pods ]; then
-    return 0
-  fi
-
-  ORPHAN_POD_RETENTION_DAYS="${ORPHAN_POD_RETENTION_DAYS}" python3 - <<'PY'
-import os
-import shutil
-import time
-
-root_pods = "/host/var/log/pods"
-active_pods = "/host/var/lib/kubelet/pods"
-retention_days = int(os.environ.get("ORPHAN_POD_RETENTION_DAYS", "3"))
-cutoff = time.time() - (retention_days * 86400)
-
-try:
-    active_names = set(os.listdir(active_pods))
-except Exception:
-    active_names = set()
-
-try:
-    root_names = os.listdir(root_pods)
-except Exception:
-    root_names = []
-
-for name in root_names:
-    path = os.path.join(root_pods, name)
-    if not os.path.isdir(path):
-        continue
-    if name in active_names:
-        continue
-    try:
-        mtime = os.path.getmtime(path)
-    except Exception:
-        continue
-    if mtime > cutoff:
-        continue
-    print(path)
-    shutil.rmtree(path, ignore_errors=True)
-PY
-}
-
 sweep_once() {
   usage=$(df -P /host | awk 'NR==2 {gsub(/%/,"",$5); print $5}') || usage=""
 
@@ -102,12 +18,9 @@ sweep_once() {
     chroot /host /bin/sh -c "crictl rmi --prune >/dev/null 2>&1 || true"
   fi
 
-  cleanup_orphaned_hdd_pod_logs
-  cleanup_orphaned_root_pod_logs
-
-  if [ -d /host/var/log.hdd/pods ]; then
-    find /host/var/log.hdd/pods -type f -name "*.log" -mtime +"${LOG_RETENTION_DAYS}" -print -delete 2>/dev/null || true
-  fi
+  # Kubelet owns active logs; retain every active UID, including quiet pods.
+  python3 /scripts/node_pod_log_cleanup.py --host-root /host \
+    --retention-days "${ORPHAN_POD_RETENTION_DAYS}"
 
   if [ -d /host/var/log.hdd/containers ]; then
     find /host/var/log.hdd/containers -xtype l -print -delete 2>/dev/null || true
@@ -120,9 +33,9 @@ sweep_once() {
     # Emergency pass for rootfs pressure on SD-backed nodes.
     chroot /host /bin/sh -c "crictl rmi --prune >/dev/null 2>&1 || true"
     chroot /host /bin/sh -c "journalctl --vacuum-size='${JOURNAL_MAX_SIZE}' >/dev/null 2>&1 || true"
-    find /host/var/log -type f -name "*.gz" -mtime +"${LOG_RETENTION_DAYS}" -print -delete 2>/dev/null || true
-    find /host/var/log/pods -type f -name "*.log" -mtime +"${LOG_RETENTION_DAYS}" -print -delete 2>/dev/null || true
-    find /host/var/log.hdd -type f -name "*.gz" -mtime +"${LOG_RETENTION_DAYS}" -print -delete 2>/dev/null || true
+    # Pod logs are handled only by the UID-aware check above.
+    find /host/var/log -path /host/var/log/pods -prune -o -type f -name "*.gz" -mtime +"${LOG_RETENTION_DAYS}" -print -exec rm -f {} \; 2>/dev/null || true
+    find /host/var/log.hdd -path /host/var/log.hdd/pods -prune -o -type f -name "*.gz" -mtime +"${LOG_RETENTION_DAYS}" -print -exec rm -f {} \; 2>/dev/null || true
     chroot /host /bin/sh -c "if command -v apt-get >/dev/null 2>&1; then apt-get clean >/dev/null 2>&1 || true; fi"
   fi
 }

services/maintenance/node-ops/scripts/node_pod_log_cleanup.py

@@ -0,0 +1,96 @@
+#!/usr/bin/env python3
+"""Remove only expired orphan pod-log directories; never read log contents."""
+
+from __future__ import annotations
+
+import argparse
+import os
+from pathlib import Path
+import re
+import shutil
+import time
+
+
+UID = re.compile(r"[0-9a-f]{8}(?:-[0-9a-f]{4}){3}-[0-9a-f]{12}")
+
+
+def walk_error(error: OSError) -> None:
+    """A failed directory scan cannot establish that all logs are expired."""
+    raise error
+
+
+def newest_mtime(path: Path) -> float:
+    """Inspect timestamps without following symlinks or reading file contents."""
+    newest = path.stat().st_mtime
+    for root, directories, files in os.walk(path, onerror=walk_error, followlinks=False):
+        for name in directories + files:
+            newest = max(newest, (Path(root) / name).lstat().st_mtime)
+    return newest
+
+
+def cleanup(host_root: Path, retention_days: int, dry_run: bool = False) -> dict[str, int]:
+    """Return counts after pruning inactive UID directories older than retention.
+
+    An unreadable or empty kubelet inventory cannot authorize any deletion.
+    Recent files preserve a directory even when its own timestamp is old.
+    """
+    if retention_days < 1:
+        raise ValueError("Pod-log retention must be at least one day")
+    result = {"removed": 0, "eligible": 0, "skipped": 0, "inventory_unavailable": 0}
+    try:
+        active = {p.name for p in (host_root / "var/lib/kubelet/pods").iterdir()
+                  if UID.fullmatch(p.name)}
+    except OSError:
+        active = set()
+    if not active:
+        result["inventory_unavailable"] = 1
+        return result
+    cutoff = time.time() - retention_days * 86400
+    for relative in ("var/log/pods", "var/log.hdd/pods"):
+        try:
+            candidates = list((host_root / relative).iterdir())
+        except OSError:
+            continue
+        for path in candidates:
+            uid = path.name.rsplit("_", 1)[-1]
+            if not UID.fullmatch(uid) or uid in active or path.is_symlink():
+                result["skipped"] += 1
+                continue
+            try:
+                if not path.is_dir():
+                    continue
+                if newest_mtime(path) >= cutoff:
+                    result["skipped"] += 1
+                    continue
+                # Recheck the exact UID immediately before the destructive step.
+                current = {p.name for p in (host_root / "var/lib/kubelet/pods").iterdir()
+                           if UID.fullmatch(p.name)}
+                if not current:
+                    result["inventory_unavailable"] = 1
+                    return result
+                if uid in current:
+                    result["skipped"] += 1
+                    continue
+                result["eligible"] += 1
+                if not dry_run:
+                    shutil.rmtree(path)
+                    result["removed"] += 1
+            except OSError:
+                result["skipped"] += 1
+    return result
+
+
+def main() -> None:
+    """Run the configured host-root cleanup and print counts, not log paths."""
+    import json
+
+    parser = argparse.ArgumentParser(description=__doc__)
+    parser.add_argument("--host-root", type=Path, default=Path("/host"))
+    parser.add_argument("--retention-days", type=int, default=3)
+    parser.add_argument("--dry-run", action="store_true")
+    args = parser.parse_args()
+    print(json.dumps(cleanup(args.host_root, args.retention_days, args.dry_run)))
+
+
+if __name__ == "__main__":
+    main()

testing/tests/test_node_pod_log_cleanup.py

@@ -0,0 +1,134 @@
+"""Exercise orphan cleanup against temporary kubelet and log inventories."""
+
+import importlib.util
+import json
+import os
+from pathlib import Path
+import time
+
+import pytest
+
+SOURCE = Path(__file__).resolve().parents[2] / 'services/maintenance/node-ops/scripts/node_pod_log_cleanup.py'
+spec = importlib.util.spec_from_file_location('node_pod_log_cleanup', SOURCE)
+cleaner = importlib.util.module_from_spec(spec)
+spec.loader.exec_module(cleaner)
+ACTIVE = '11111111-1111-1111-1111-111111111111'
+ORPHAN = '22222222-2222-2222-2222-222222222222'
+
+
+def inventory(root):
+    """Create one current pod with a UID-only kubelet directory."""
+    (root / 'var/lib/kubelet/pods' / ACTIVE).mkdir(parents=True)
+
+
+def logs(root, uid, relative='var/log/pods', old=True):
+    """Create the real namespace_name_UID directory layout with a synthetic log."""
+    path = root / relative / ('namespace_name_' + uid)
+    (path / 'container').mkdir(parents=True)
+    (path / 'container/0.log').write_text('SYNTHETIC LOG CONTENT')
+    if old:
+        stamp = time.time() - 10 * 86400
+        for entry in [*path.rglob('*'), path]:
+            os.utime(entry, (stamp, stamp))
+    return path
+
+
+def test_active_uid_preserves_old_current_and_hdd_logs(tmp_path):
+    """Old quiet containers remain protected in both log locations."""
+    inventory(tmp_path)
+    paths = [logs(tmp_path, ACTIVE, relative=r) for r in ('var/log/pods', 'var/log.hdd/pods')]
+    assert cleaner.cleanup(tmp_path, 3)['removed'] == 0
+    assert all((p / 'container/0.log').exists() for p in paths)
+
+
+def test_only_expired_orphans_are_removed(tmp_path):
+    """Dry-run reports the same eligible directory without deleting it."""
+    inventory(tmp_path)
+    path = logs(tmp_path, ORPHAN)
+    assert cleaner.cleanup(tmp_path, 3, dry_run=True)['eligible'] == 1
+    assert path.exists()
+    assert cleaner.cleanup(tmp_path, 3)['removed'] == 1
+    assert not path.exists()
+
+
+def test_recent_log_preserves_old_parent_directory(tmp_path):
+    """Writing a log does not update the pod directory's own mtime."""
+    inventory(tmp_path)
+    path = logs(tmp_path, ORPHAN)
+    os.utime(path / 'container/0.log', None)
+    assert cleaner.cleanup(tmp_path, 3)['skipped'] == 1
+    assert path.exists()
+
+
+@pytest.mark.parametrize('empty', [False, True])
+def test_unavailable_inventory_cannot_authorize_deletion(tmp_path, empty):
+    """Missing and empty inventories both preserve all candidate logs."""
+    if empty:
+        (tmp_path / 'var/lib/kubelet/pods').mkdir(parents=True)
+    path = logs(tmp_path, ORPHAN)
+    assert cleaner.cleanup(tmp_path, 3)['inventory_unavailable'] == 1
+    assert path.exists()
+
+
+def test_symlinks_unknown_names_and_nondirectories_are_preserved(tmp_path):
+    """Only recognized pod directories enter deletion; links never escape root."""
+    inventory(tmp_path)
+    parent = tmp_path / 'var/log/pods'
+    parent.mkdir(parents=True)
+    outside = tmp_path / 'outside'
+    outside.mkdir()
+    (parent / ('ns_link_' + ORPHAN)).symlink_to(outside)
+    (parent / 'unrecognized').mkdir()
+    (parent / ('ns_file_' + ORPHAN)).write_text('metadata')
+    assert cleaner.cleanup(tmp_path, 3)['removed'] == 0
+    assert outside.exists()
+    assert (parent / 'unrecognized').exists()
+
+
+def test_scan_failure_preserves_candidate(tmp_path, monkeypatch):
+    """An unreadable log subtree must not look like an old empty directory."""
+    inventory(tmp_path)
+    path = logs(tmp_path, ORPHAN)
+
+    def fail_scan(*args, **kwargs):
+        """Simulate the walker reporting a storage error."""
+        kwargs['onerror'](OSError('synthetic scan failure'))
+
+    monkeypatch.setattr(cleaner.os, 'walk', fail_scan)
+    assert cleaner.cleanup(tmp_path, 3)['skipped'] == 1
+    assert path.exists()
+
+
+@pytest.mark.parametrize('new_active', [True, False])
+def test_inventory_change_before_deletion_is_rechecked(tmp_path, monkeypatch, new_active):
+    """A newly active UID or vanished inventory invalidates the initial decision."""
+    inventory(tmp_path)
+    path = logs(tmp_path, ORPHAN)
+    original = cleaner.newest_mtime
+
+    def change_inventory(candidate):
+        """Alter only inventory metadata after the age scan."""
+        newest = original(candidate)
+        pods = tmp_path / 'var/lib/kubelet/pods'
+        if new_active:
+            (pods / ORPHAN).mkdir()
+        else:
+            (pods / ACTIVE).rmdir()
+        return newest
+
+    monkeypatch.setattr(cleaner, 'newest_mtime', change_inventory)
+    assert cleaner.cleanup(tmp_path, 3)['removed'] == 0
+    assert path.exists()
+
+
+def test_cli_reports_counts_and_rejects_zero_retention(tmp_path, monkeypatch, capsys):
+    """No content or candidate path enters the operator's cleanup summary."""
+    inventory(tmp_path)
+    path = logs(tmp_path, ORPHAN)
+    monkeypatch.setattr('sys.argv', ['clean', '--host-root', str(tmp_path), '--dry-run'])
+    cleaner.main()
+    result = json.loads(capsys.readouterr().out)
+    assert result['eligible'] == 1 and result['removed'] == 0
+    assert path.exists()
+    with pytest.raises(ValueError, match='at least one day'):
+        cleaner.cleanup(tmp_path, 0)