From b4f0f6b48c2f9950c362ef5c2aa93efb01302492 Mon Sep 17 00:00:00 2001 From: jenkins Date: Sun, 4 Oct 2026 07:13:27 -0500 Subject: [PATCH] maintenance: preserve active pod logs during cleanup --- .../maintenance/node-ops/kustomization.yaml | 1 + .../node-image-sweeper-daemonset.yaml | 4 +- .../node-ops/scripts/node_image_sweeper.sh | 99 +------------ .../node-ops/scripts/node_pod_log_cleanup.py | 96 +++++++++++++ testing/tests/test_node_pod_log_cleanup.py | 134 ++++++++++++++++++ 5 files changed, 240 insertions(+), 94 deletions(-) create mode 100644 services/maintenance/node-ops/scripts/node_pod_log_cleanup.py create mode 100644 testing/tests/test_node_pod_log_cleanup.py diff --git a/services/maintenance/node-ops/kustomization.yaml b/services/maintenance/node-ops/kustomization.yaml index acaf19b6..c694e5b1 100644 --- a/services/maintenance/node-ops/kustomization.yaml +++ b/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 diff --git a/services/maintenance/node-ops/node-image-sweeper-daemonset.yaml b/services/maintenance/node-ops/node-image-sweeper-daemonset.yaml index 05b4d71a..bbe837c8 100644 --- a/services/maintenance/node-ops/node-image-sweeper-daemonset.yaml +++ b/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: diff --git a/services/maintenance/node-ops/scripts/node_image_sweeper.sh b/services/maintenance/node-ops/scripts/node_image_sweeper.sh index 38aa3a48..5e7b1ea6 100644 --- a/services/maintenance/node-ops/scripts/node_image_sweeper.sh +++ b/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 } diff --git a/services/maintenance/node-ops/scripts/node_pod_log_cleanup.py b/services/maintenance/node-ops/scripts/node_pod_log_cleanup.py new file mode 100644 index 00000000..2176b0e8 --- /dev/null +++ b/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() diff --git a/testing/tests/test_node_pod_log_cleanup.py b/testing/tests/test_node_pod_log_cleanup.py new file mode 100644 index 00000000..6e201b69 --- /dev/null +++ b/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)