diff --git a/system/loggerd/deleter.py b/system/loggerd/deleter.py index eb8fd35f21813c..e94fe608ed4a56 100755 --- a/system/loggerd/deleter.py +++ b/system/loggerd/deleter.py @@ -51,22 +51,44 @@ def deleter_thread(exit_event: threading.Event): out_of_percent = get_available_percent(default=MIN_PERCENT + 1) < MIN_PERCENT if out_of_percent or out_of_bytes: - dirs = listdir_by_creation(Paths.log_root()) + log_root = Paths.log_root() + dirs = listdir_by_creation(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)): - delete_path = os.path.join(Paths.log_root(), delete_dir) + delete_path = os.path.join(log_root, delete_dir) - if any(name.endswith(".lock") for name in os.listdir(delete_path)): + try: + if any(name.endswith(".lock") for name in os.listdir(delete_path)): + continue + except OSError: + cloudlog.exception(f"issue listing {delete_path}") continue try: cloudlog.info(f"deleting {delete_path}") - shutil.rmtree(delete_path) + if os.path.islink(delete_path): + os.unlink(delete_path) + else: + shutil.rmtree(delete_path) break except OSError: cloudlog.exception(f"issue deleting {delete_path}") + + # also remove stray non-directory entries (files, symlinks) not managed by openpilot + try: + for entry in os.listdir(log_root): + entry_path = os.path.join(log_root, entry) + if os.path.islink(entry_path) or not os.path.isdir(entry_path): + try: + cloudlog.info(f"deleting stray entry {entry_path}") + os.unlink(entry_path) + except OSError: + cloudlog.exception(f"issue deleting stray {entry_path}") + except OSError: + cloudlog.exception("failed to list log root for stray entries") + exit_event.wait(.1) else: exit_event.wait(30) diff --git a/system/loggerd/tests/test_deleter.py b/system/loggerd/tests/test_deleter.py index 6222ea253ba0db..254664398037ae 100644 --- a/system/loggerd/tests/test_deleter.py +++ b/system/loggerd/tests/test_deleter.py @@ -115,3 +115,107 @@ def test_no_delete_with_lock_file(self): self.join_thread() assert f_path.exists(), "File deleted when locked" + + def _wait_for_deletion(self, path: Path, timeout: float = 3.0) -> bool: + """Return True if path is deleted within timeout seconds.""" + deadline = time.monotonic() + timeout + while time.monotonic() < deadline: + if not path.exists() and not path.is_symlink(): + return True + time.sleep(0.01) + return False + + def test_delete_stray_file(self): + """Stray non-segment files in log root should be deleted when disk is low.""" + stray = Path(deleter.Paths.log_root()) / "stray_backup.tar.gz" + stray.write_bytes(b"data") + + self.start_thread() + deleted = self._wait_for_deletion(stray) + self.join_thread() + + assert deleted, "Stray file was not deleted" + + def test_delete_stray_files_multiple(self): + """Multiple stray files are all cleaned up.""" + log_root = Path(deleter.Paths.log_root()) + strays = [log_root / f"stray_{i}.bin" for i in range(3)] + for f in strays: + f.write_bytes(b"x" * 1024) + + self.start_thread() + try: + with Timeout(5, "Timeout waiting for stray files to be deleted"): + while any(f.exists() for f in strays): + time.sleep(0.01) + finally: + self.join_thread() + + def test_delete_symlink_in_log_root(self): + """A symlink in log root is removed without following (target must survive).""" + target = Path(deleter.Paths.log_root()) / "symlink_target_dir" + target.mkdir() + link = Path(deleter.Paths.log_root()) / "stray_link" + link.symlink_to(target) + + self.start_thread() + deleted = self._wait_for_deletion(link) + self.join_thread() + + assert deleted, "Symlink was not deleted" + assert target.exists(), "Symlink target was incorrectly removed" + + def test_delete_broken_symlink(self): + """A broken (dangling) symlink in log root is removed.""" + link = Path(deleter.Paths.log_root()) / "broken_link" + link.symlink_to("/nonexistent/path/does/not/exist") + + self.start_thread() + deleted = self._wait_for_deletion(link) + self.join_thread() + + assert deleted, "Broken symlink was not deleted" + + def test_stray_file_mixed_with_segments(self): + """Stray files and segment directories coexist; both are cleaned up.""" + seg_file = self.make_file_with_data(self.seg_dir, self.f_type) + stray = Path(deleter.Paths.log_root()) / "unexpected.log" + stray.write_bytes(b"oops") + + self.start_thread() + try: + with Timeout(5, "Timeout waiting for all entries to be deleted"): + while seg_file.exists() or stray.exists(): + time.sleep(0.01) + finally: + self.join_thread() + + def test_deleter_continues_after_stray_entry_error(self): + """A deletion failure on one stray entry does not prevent deleting the next.""" + log_root = Path(deleter.Paths.log_root()) + good_stray = log_root / "good_stray.bin" + good_stray.write_bytes(b"deleteme") + + # Also create a normal segment so the deleter has something to process + seg_file = self.make_file_with_data(self.seg_format.format(0), self.f_type) + + self.start_thread() + try: + with Timeout(5, "Timeout waiting for entries to be deleted"): + while good_stray.exists() and seg_file.exists(): + time.sleep(0.01) + finally: + self.join_thread() + + def test_delete_nested_stray_directory(self): + """A non-segment directory nested inside log root is removed via the regular dir path.""" + # Non-segment dirs ARE included by listdir_by_creation and should be cleaned up + non_seg_dir = Path(deleter.Paths.log_root()) / "some_random_dir" + (non_seg_dir / "subdir").mkdir(parents=True) + (non_seg_dir / "file.txt").write_bytes(b"hello") + + self.start_thread() + deleted = self._wait_for_deletion(non_seg_dir) + self.join_thread() + + assert deleted, "Non-segment directory was not deleted"