reading journal logs with journalctl only

alexandraBara · alexandraBara · commit bca73c090a19 · 2025-09-08T15:36:51.000-05:00
diff --git a/nodescraper/plugins/inband/journal/journal_collector.py b/nodescraper/plugins/inband/journal/journal_collector.py
@@ -24,8 +24,7 @@
 #
 ###############################################################################
 from nodescraper.base import InBandDataCollector
-from nodescraper.connection.inband import TextFileArtifact
-from nodescraper.enums import EventCategory, EventPriority, OSFamily
+from nodescraper.enums import OSFamily
 from nodescraper.models import TaskResult
 
 from .journaldata import JournalData
@@ -37,102 +36,20 @@ class JournalCollector(InBandDataCollector[JournalData, None]):
     SUPPORTED_OS_FAMILY = {OSFamily.LINUX}
     DATA_MODEL = JournalData
 
-    CMD = "ls -1 /var/log/journal/*/system* 2>/dev/null || true"
-
-    def _shell_quote(self, s: str) -> str:
-        """single-quote fix.
-
-        Args:
-            s (str): path
-
-        Returns:
-            str: escaped path
-        """
-        return "'" + s.replace("'", "'\"'\"'") + "'"
-
-    def _flat_name(self, path: str) -> str:
-        """Flatten path name
-
-        Args:
-            path (str): path
-
-        Returns:
-            str: flattened path name
-        """
-        return "journalctl__" + path.lstrip("/").replace("/", "__") + ".json"
-
-    def _read_with_journalctl(self, path: str):
+    def _read_with_journalctl(self):
         """Read journal logs using journalctl
 
-        Args:
-            path (str): path for log to read
-
         Returns:
-            str|None: name of local journal log filed, or None if log was not read
+            str|None: system journal read
         """
-        qp = self._shell_quote(path)
-        cmd = f"journalctl --no-pager --system --all --file={qp} --output=json"
+        cmd = "journalctl --no-pager --system --all -o short-iso --output=json"
         res = self._run_sut_cmd(cmd, sudo=True, log_artifact=False, strip=False)
 
         if res.exit_code == 0:
-            text = (
-                res.stdout.decode("utf-8", "replace")
-                if isinstance(res.stdout, (bytes, bytearray))
-                else res.stdout
-            )
-            fname = self._flat_name(path)
-            self.result.artifacts.append(TextFileArtifact(filename=fname, contents=text))
-            self.logger.info("Collected journal: %s", path)
-            return fname
+            return res.stdout
 
         return None
 
-    def _get_journals(self) -> list[str]:
-        """Read journal log files on remote system
-
-        Returns:
-            list[str]: List of names of read logs
-        """
-        list_res = self._run_sut_cmd(self.CMD, sudo=True)
-        paths = [p.strip() for p in (list_res.stdout or "").splitlines() if p.strip()]
-
-        if not paths:
-            self._log_event(
-                category=EventCategory.OS,
-                description="No /var/log/journal files found (including rotations).",
-                data={"list_exit_code": list_res.exit_code},
-                priority=EventPriority.WARNING,
-            )
-            return []
-
-        collected, failed = [], []
-        for p in paths:
-            self.logger.debug("Reading journal file: %s", p)
-            fname = self._read_with_journalctl(p)
-            if fname:
-                collected.append(fname)
-            else:
-                failed.append(fname)
-
-        if collected:
-            self._log_event(
-                category=EventCategory.OS,
-                description="Collected journal logs.",
-                data={"collected": collected},
-                priority=EventPriority.INFO,
-            )
-            self.result.message = self.result.message or "journalctl logs collected"
-
-        if failed:
-            self._log_event(
-                category=EventCategory.OS,
-                description="Some journal files could not be read with journalctl.",
-                data={"failed": failed},
-                priority=EventPriority.WARNING,
-            )
-
-        return collected
-
     def collect_data(self, args=None) -> tuple[TaskResult, JournalData | None]:
         """Collect journal lofs
 
@@ -142,9 +59,9 @@ def collect_data(self, args=None) -> tuple[TaskResult, JournalData | None]:
         Returns:
             tuple[TaskResult, JournalData | None]: Tuple of results and data model or none.
         """
-        collected = self._get_journals()
-        if collected:
-            data = JournalData(journal_logs=collected)
+        journal_log = self._read_with_journalctl()
+        if journal_log:
+            data = JournalData(journal_log=journal_log)
             self.result.message = self.result.message or "Journal data collected"
             return self.result, data
         return self.result, None
diff --git a/nodescraper/plugins/inband/journal/journaldata.py b/nodescraper/plugins/inband/journal/journaldata.py
@@ -29,4 +29,4 @@
 class JournalData(DataModel):
     """Data model for journal logs"""
 
-    journal_logs: list[str] = []
+    journal_log: str
diff --git a/test/unit/plugin/test_journal_collector.py b/test/unit/plugin/test_journal_collector.py
@@ -59,134 +59,13 @@ def _run_sut_cmd(cmd, *args, **kwargs):
     return c
 
 
-def test_get_journals_happy_path(monkeypatch, system_info, conn_mock):
-    paths = [
-        "/var/log/journal/m1/system.journal",
-        "/var/log/journal/m1/system@0000000000000001-0000000000000002.journal",
-        "/var/log/journal/m2/system.journal",
-    ]
-    ls_out = "\n".join(paths) + "\n"
-
-    def run_map(cmd, **kwargs):
-        if cmd.startswith("ls -1 /var/log/journal"):
-            return DummyRes(command=cmd, stdout=ls_out, exit_code=0)
-
-        if cmd.startswith("journalctl ") and "--file=" in cmd:
-            if paths[0] in cmd:
-                return DummyRes(cmd, stdout='{"MESSAGE":"a"}\n', exit_code=0)
-            if paths[1] in cmd:
-                return DummyRes(cmd, stdout=b'{"MESSAGE":"b"}\n', exit_code=0)
-            if paths[2] in cmd:
-                return DummyRes(cmd, stdout='{"MESSAGE":"c"}\n', exit_code=0)
-
-        return DummyRes(command=cmd, stdout="", exit_code=1, stderr="unexpected")
-
-    c = get_collector(monkeypatch, run_map, system_info, conn_mock)
-
-    collected = c._get_journals()
-    assert len(collected) == 3
-
-    expected_names = {
-        "journalctl__var__log__journal__m1__system.journal.json",
-        "journalctl__var__log__journal__m1__system@0000000000000001-0000000000000002.journal.json",
-        "journalctl__var__log__journal__m2__system.journal.json",
-    }
-    names = {a.filename for a in c.result.artifacts}
-    assert names == expected_names
-
-    contents = {a.filename: a.contents for a in c.result.artifacts}
-    assert (
-        contents["journalctl__var__log__journal__m1__system.journal.json"].strip()
-        == '{"MESSAGE":"a"}'
-    )
-    assert (
-        contents[
-            "journalctl__var__log__journal__m1__system@0000000000000001-0000000000000002.journal.json"
-        ].strip()
-        == '{"MESSAGE":"b"}'
-    )
-    assert (
-        contents["journalctl__var__log__journal__m2__system.journal.json"].strip()
-        == '{"MESSAGE":"c"}'
-    )
-
-    assert any(
-        evt.get("description") == "Collected journal logs."
-        and getattr(evt.get("priority"), "name", str(evt.get("priority"))) == "INFO"
-        for evt in c._events
-    )
-    assert c.result.message == "journalctl logs collected"
-
-
-def test_get_journals_no_files(monkeypatch, system_info, conn_mock):
-    def run_map(cmd, **kwargs):
-        if cmd.startswith("ls -1 /var/log/journal"):
-            return DummyRes(command=cmd, stdout="", exit_code=0)
-        return DummyRes(command=cmd, stdout="", exit_code=1)
-
-    c = get_collector(monkeypatch, run_map, system_info, conn_mock)
-
-    collected = c._get_journals()
-    assert collected == []
-    assert c.result.artifacts == []
-
-    assert any(
-        evt.get("description", "").startswith("No /var/log/journal files found")
-        and getattr(evt.get("priority"), "name", str(evt.get("priority"))) == "WARNING"
-        for evt in c._events
-    )
-
-
-def test_get_journals_partial_failure(monkeypatch, system_info, conn_mock):
-    ok_path = "/var/log/journal/m1/system.journal"
-    bad_path = "/var/log/journal/m1/system@bad.journal"
-    ls_out = ok_path + "\n" + bad_path + "\n"
-
-    def run_map(cmd, **kwargs):
-        if cmd.startswith("ls -1 /var/log/journal"):
-            return DummyRes(command=cmd, stdout=ls_out, exit_code=0)
-
-        if cmd.startswith("journalctl ") and "--file=" in cmd:
-            if ok_path in cmd:
-                return DummyRes(cmd, stdout='{"MESSAGE":"ok"}\n', exit_code=0)
-            if bad_path in cmd:
-                return DummyRes(cmd, stdout="", exit_code=1, stderr="cannot read")
-
-        return DummyRes(command=cmd, stdout="", exit_code=1)
-
-    c = get_collector(monkeypatch, run_map, system_info, conn_mock)
-
-    collected = c._get_journals()
-    assert collected == ["journalctl__var__log__journal__m1__system.journal.json"]
-    assert [a.filename for a in c.result.artifacts] == [
-        "journalctl__var__log__journal__m1__system.journal.json"
-    ]
-
-    assert any(
-        evt.get("description") == "Some journal files could not be read with journalctl."
-        and getattr(evt.get("priority"), "name", str(evt.get("priority"))) == "WARNING"
-        for evt in c._events
-    )
-
-
 def test_collect_data_integration(monkeypatch, system_info, conn_mock):
-    dummy_path = "/var/log/journal/m1/system.journal"
-    ls_out = dummy_path + "\n"
-
     def run_map(cmd, **kwargs):
-        if cmd.startswith("ls -1 /var/log/journal"):
-            return DummyRes(command=cmd, stdout=ls_out, exit_code=0)
-        if cmd.startswith("journalctl ") and "--file=" in cmd and dummy_path in cmd:
-            return DummyRes(command=cmd, stdout='{"MESSAGE":"hello"}\n', exit_code=0)
-        return DummyRes(command=cmd, stdout="", exit_code=1)
+        return DummyRes(command=cmd, stdout='{"MESSAGE":"hello"}\n', exit_code=0)
 
     c = get_collector(monkeypatch, run_map, system_info, conn_mock)
 
     result, data = c.collect_data()
     assert isinstance(data, JournalData)
 
-    expected_name = "journalctl__var__log__journal__m1__system.journal.json"
-    assert data.journal_logs == [expected_name]
-    assert c.result.message == "journalctl logs collected"
-
-    assert [a.filename for a in c.result.artifacts] == [expected_name]
+    assert data.journal_log == '{"MESSAGE":"hello"}\n'
