diff --git a/system/loggerd/deleter.py b/system/loggerd/deleter.py index eb8fd35f21813c..6600b536fe1052 100755 --- a/system/loggerd/deleter.py +++ b/system/loggerd/deleter.py @@ -54,16 +54,28 @@ def deleter_thread(exit_event: threading.Event): dirs = listdir_by_creation(Paths.log_root()) preserved_dirs = get_preserved_segments(dirs) - # remove the earliest directory we can - for delete_dir in sorted(dirs, key=lambda d: (d in DELETE_LAST, d in preserved_dirs)): + # listdir_by_creation only returns directories; also collect stray + # files and symlinks so they get cleaned up instead of crashing + dir_set = set(dirs) + try: + stray = [e for e in os.listdir(Paths.log_root()) if e not in dir_set] + except OSError: + stray = [] + all_entries = stray + sorted(dirs, key=lambda d: (d in DELETE_LAST, d in preserved_dirs)) + + # remove the earliest entry we can + for delete_dir in all_entries: delete_path = os.path.join(Paths.log_root(), delete_dir) - if any(name.endswith(".lock") for name in os.listdir(delete_path)): - continue - try: - cloudlog.info(f"deleting {delete_path}") - shutil.rmtree(delete_path) + if os.path.isdir(delete_path) and not os.path.islink(delete_path): + if any(name.endswith(".lock") for name in os.listdir(delete_path)): + continue + cloudlog.info(f"deleting {delete_path}") + shutil.rmtree(delete_path) + else: + cloudlog.info(f"deleting {delete_path}") + os.remove(delete_path) break except OSError: cloudlog.exception(f"issue deleting {delete_path}") diff --git a/system/loggerd/tests/test_deleter.py b/system/loggerd/tests/test_deleter.py index 6222ea253ba0db..896b869c8c7de6 100644 --- a/system/loggerd/tests/test_deleter.py +++ b/system/loggerd/tests/test_deleter.py @@ -1,3 +1,5 @@ +import os +import shutil import time import threading from collections import namedtuple @@ -6,7 +8,8 @@ import openpilot.system.loggerd.deleter as deleter from openpilot.common.timeout import Timeout, TimeoutException -from openpilot.system.loggerd.tests.loggerd_tests_common import UploaderTestCase +from openpilot.system.loggerd.tests.loggerd_tests_common import UploaderTestCase, create_random_file +from openpilot.system.hardware.hw import Paths Stats = namedtuple("Stats", ['f_bavail', 'f_blocks', 'f_frsize']) @@ -115,3 +118,97 @@ def test_no_delete_with_lock_file(self): self.join_thread() assert f_path.exists(), "File deleted when locked" + + def test_delete_stray_file(self): + """Stray files (e.g. tar.gz) in log_root should be deleted without crashing.""" + root = Path(Paths.log_root()) + stray = root / "2023-09-23.tar.gz" + create_random_file(stray, 0.01) + + seg_file = self.make_file_with_data(self.seg_dir, self.f_type, 0.1) + + self.start_thread() + try: + with Timeout(5, "Timeout waiting for stray file to be deleted"): + while stray.exists() or seg_file.exists(): + time.sleep(0.01) + finally: + self.join_thread() + + def test_delete_stray_symlink(self): + """Symlinks in log_root should be removed (unlinked), not followed.""" + root = Path(Paths.log_root()) + target = root.parent / "symlink_target.bin" + create_random_file(target, 0.01) + + link = root / "stray_symlink" + os.symlink(target, link) + + self.start_thread() + try: + with Timeout(5, "Timeout waiting for symlink to be deleted"): + while os.path.lexists(link): + time.sleep(0.01) + finally: + self.join_thread() + assert target.exists(), "Symlink target should not be deleted" + target.unlink() + + def test_delete_mixed_entries(self): + """Deleter handles a mix of segment dirs, stray files, and nested dirs.""" + root = Path(Paths.log_root()) + paths = [] + + # stray files + for name in ["backup.tar.gz", "notes.txt", ".hidden_file"]: + p = root / name + create_random_file(p, 0.01) + paths.append(p) + + # stray nested directory + nested = root / "stray_logs" / "sub" / "deep" + nested.mkdir(parents=True, exist_ok=True) + create_random_file(nested / "data.bin", 0.01) + paths.append(root / "stray_logs") + + # normal segment directory + seg_file = self.make_file_with_data(self.seg_dir, self.f_type, 0.1) + paths.append(seg_file) + + self.start_thread() + try: + with Timeout(10, "Timeout waiting for mixed entries to be deleted"): + while any(os.path.lexists(p) for p in paths): + time.sleep(0.01) + finally: + self.join_thread() + + def test_delete_continues_after_error(self, monkeypatch): + """Deleter should continue to the next entry if one fails to delete.""" + root = Path(Paths.log_root()) + + bad = root / "0_bad_entry" + bad.mkdir(parents=True, exist_ok=True) + create_random_file(bad / "file.bin", 0.01) + + good_file = root / "1_good_file.bin" + create_random_file(good_file, 0.01) + + original_rmtree = shutil.rmtree + + def failing_rmtree(path, *args, **kwargs): + if str(path) == str(bad): + raise PermissionError("simulated permission error") + return original_rmtree(path, *args, **kwargs) + + monkeypatch.setattr(shutil, "rmtree", failing_rmtree) + + self.start_thread() + try: + with Timeout(5, "Timeout waiting for good file to be deleted"): + while good_file.exists(): + time.sleep(0.01) + finally: + self.join_thread() + + assert bad.exists(), "Bad entry should survive the error"