Skip to content

Commit b7988dd

Browse files
alexandraBaraalexandraBara
committed
addressed reviews
1 parent 47cacb2 commit b7988dd

File tree

5 files changed

+59
-95
lines changed

5 files changed

+59
-95
lines changed

‎nodescraper/plugins/inband/dmesg/dmesg_collector.py‎

Lines changed: 4 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,13 @@
2323
# SOFTWARE.
2424
#
2525
###############################################################################
26-
import re
2726
from typing import Optional
2827

2928
from nodescraper.base import InBandDataCollector
3029
from nodescraper.connection.inband import TextFileArtifact
3130
from nodescraper.enums import EventCategory, EventPriority, ExecutionStatus, OSFamily
3231
from nodescraper.models import TaskResult
32+
from nodescraper.utils import nice_rotated_name, shell_quote
3333

3434
from .collector_args import DmesgCollectorArgs
3535
from .dmesgdata import DmesgData
@@ -48,43 +48,6 @@ class DmesgCollector(InBandDataCollector[DmesgData, DmesgCollectorArgs]):
4848
r"ls -1 /var/log/dmesg* 2>/dev/null | grep -E '^/var/log/dmesg(\.[0-9]+(\.gz)?)?$' || true"
4949
)
5050

51-
def _shell_quote(self, s: str) -> str:
52-
"""Single quote fix
53-
54-
Args:
55-
s (str): path to be converted
56-
57-
Returns:
58-
str: path to be returned
59-
"""
60-
return "'" + s.replace("'", "'\"'\"'") + "'"
61-
62-
def _nice_dmesg_name(self, path: str) -> str:
63-
"""Map path to filename
64-
65-
Args:
66-
path (str): file path
67-
68-
Returns:
69-
str: new local filename
70-
"""
71-
prefix = "rotated_"
72-
base = path.rstrip("/").rsplit("/", 1)[-1]
73-
74-
if base == "dmesg":
75-
return f"{prefix}dmesg_log.log"
76-
77-
m = re.fullmatch(r"dmesg\.(\d+)\.gz", base)
78-
if m:
79-
return f"{prefix}dmesg.{m.group(1)}.gz.log"
80-
81-
m = re.fullmatch(r"dmesg\.(\d+)", base)
82-
if m:
83-
return f"{prefix}dmesg.{m.group(1)}.log"
84-
85-
middle = base[:-3] if base.endswith(".gz") else base
86-
return f"{prefix}{middle}.log"
87-
8851
def _collect_dmesg_rotations(self):
8952
"""Collect dmesg logs"""
9053
list_res = self._run_sut_cmd(self.DMESG_LOGS_CMD, sudo=True)
@@ -100,12 +63,12 @@ def _collect_dmesg_rotations(self):
10063

10164
collected_logs, failed_logs = [], []
10265
for p in paths:
103-
qp = self._shell_quote(p)
66+
qp = shell_quote(p)
10467
if p.endswith(".gz"):
10568
cmd = f"gzip -dc {qp} 2>/dev/null || zcat {qp} 2>/dev/null"
10669
res = self._run_sut_cmd(cmd, sudo=True, log_artifact=False)
10770
if res.exit_code == 0 and res.stdout is not None:
108-
fname = self._nice_dmesg_name(p)
71+
fname = nice_rotated_name(p, "dmesg")
10972
self.logger.info("Collected dmesg log: %s", fname)
11073
self.result.artifacts.append(
11174
TextFileArtifact(filename=fname, contents=res.stdout)
@@ -121,7 +84,7 @@ def _collect_dmesg_rotations(self):
12184
cmd = f"cat {qp}"
12285
res = self._run_sut_cmd(cmd, sudo=True, log_artifact=False)
12386
if res.exit_code == 0 and res.stdout is not None:
124-
fname = self._nice_dmesg_name(p)
87+
fname = nice_rotated_name(p, "dmesg")
12588
self.logger.info("Collected dmesg log: %s", fname)
12689
self.result.artifacts.append(
12790
TextFileArtifact(filename=fname, contents=res.stdout)

‎nodescraper/plugins/inband/syslog/syslog_collector.py‎

Lines changed: 12 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,12 @@
2323
# SOFTWARE.
2424
#
2525
###############################################################################
26-
import re
2726

2827
from nodescraper.base import InBandDataCollector
2928
from nodescraper.connection.inband.inband import TextFileArtifact
3029
from nodescraper.enums import EventCategory, EventPriority, OSFamily
3130
from nodescraper.models import TaskResult
31+
from nodescraper.utils import nice_rotated_name, shell_quote
3232

3333
from .syslogdata import SyslogData
3434

@@ -42,36 +42,8 @@ class SyslogCollector(InBandDataCollector[SyslogData, None]):
4242

4343
SYSLOG_CMD = r"ls -1 /var/log/syslog* 2>/dev/null | grep -E '^/var/log/syslog(\.[0-9]+(\.gz)?)?$' || true"
4444

45-
def _shell_quote(self, s: str) -> str:
46-
"""single-quote fix."""
47-
return "'" + s.replace("'", "'\"'\"'") + "'"
48-
49-
def _nice_syslog_name(self, path: str) -> str:
50-
"""Map path to filename
51-
Args:
52-
path (str): file path
53-
Returns:
54-
str: new local filename
55-
"""
56-
prefix = "rotated_"
57-
base = path.rstrip("/").rsplit("/", 1)[-1]
58-
59-
if base == "syslog":
60-
return f"{prefix}syslog.log"
61-
62-
m = re.fullmatch(r"syslog\.(\d+)\.gz", base)
63-
if m:
64-
return f"{prefix}syslog.{m.group(1)}.gz.log"
65-
66-
m = re.fullmatch(r"syslog\.(\d+)", base)
67-
if m:
68-
return f"{prefix}syslog.{m.group(1)}.log"
69-
70-
middle = base[:-3] if base.endswith(".gz") else base
71-
return f"{prefix}{middle}.log"
72-
73-
def _collect_syslog_rotations(self) -> int:
74-
ret = 0
45+
def _collect_syslog_rotations(self) -> list[str]:
46+
ret = []
7547
list_res = self._run_sut_cmd(self.SYSLOG_CMD, sudo=True)
7648
paths = [p.strip() for p in (list_res.stdout or "").splitlines() if p.strip()]
7749
if not paths:
@@ -81,43 +53,35 @@ def _collect_syslog_rotations(self) -> int:
8153
data={"list_exit_code": list_res.exit_code},
8254
priority=EventPriority.WARNING,
8355
)
84-
return 0
56+
return []
8557

8658
collected_logs, failed_logs = [], []
8759
for p in paths:
88-
qp = self._shell_quote(p)
60+
qp = shell_quote(p)
8961
if p.endswith(".gz"):
9062
cmd = f"gzip -dc {qp} 2>/dev/null || zcat {qp} 2>/dev/null"
9163
res = self._run_sut_cmd(cmd, sudo=True, log_artifact=False)
9264
if res.exit_code == 0 and res.stdout is not None:
93-
fname = self._nice_syslog_name(p)
65+
fname = nice_rotated_name(p, "syslog")
9466
self.logger.info("Collected syslog log: %s", fname)
9567
self.result.artifacts.append(
9668
TextFileArtifact(filename=fname, contents=res.stdout)
9769
)
98-
collected_logs.append(
99-
{"path": p, "as": fname, "bytes": len(res.stdout.encode("utf-8", "ignore"))}
100-
)
70+
collected_logs.append(fname)
10171
else:
102-
failed_logs.append(
103-
{"path": p, "exit_code": res.exit_code, "stderr": res.stderr, "cmd": cmd}
104-
)
72+
failed_logs.append(p)
10573
else:
10674
cmd = f"cat {qp}"
10775
res = self._run_sut_cmd(cmd, sudo=True, log_artifact=False)
10876
if res.exit_code == 0 and res.stdout is not None:
109-
fname = self._nice_syslog_name(p)
77+
fname = nice_rotated_name(p, "syslog")
11078
self.logger.info("Collected syslog log: %s", fname)
11179
self.result.artifacts.append(
11280
TextFileArtifact(filename=fname, contents=res.stdout)
11381
)
114-
collected_logs.append(
115-
{"path": p, "as": fname, "bytes": len(res.stdout.encode("utf-8", "ignore"))}
116-
)
82+
collected_logs.append(fname)
11783
else:
118-
failed_logs.append(
119-
{"path": p, "exit_code": res.exit_code, "stderr": res.stderr, "cmd": cmd}
120-
)
84+
failed_logs.append(p)
12185

12286
if collected_logs:
12387
self._log_event(
@@ -137,7 +101,7 @@ def _collect_syslog_rotations(self) -> int:
137101
)
138102

139103
if collected_logs:
140-
ret = len(collected_logs)
104+
ret = collected_logs
141105
return ret
142106

143107
def collect_data(

‎nodescraper/plugins/inband/syslog/syslogdata.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,4 +29,4 @@
2929
class SyslogData(DataModel):
3030
"""Data model for in band syslog logs"""
3131

32-
syslog_logs: int = 0
32+
syslog_logs: list[str] = []

‎nodescraper/utils.py‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,3 +169,35 @@ def bytes_to_human_readable(input_bytes: int) -> str:
169169

170170
gb = round(mb / 1000, 2)
171171
return f"{gb}GB"
172+
173+
174+
def shell_quote(s: str) -> str:
175+
"""Single quote fix
176+
177+
Args:
178+
s (str): path to be converted
179+
180+
Returns:
181+
str: path to be returned
182+
"""
183+
return "'" + s.replace("'", "'\"'\"'") + "'"
184+
185+
186+
def nice_rotated_name(path: str, stem: str, prefix: str = "rotated_") -> str:
187+
"""Map path to a new local filename, generalized for any stem."""
188+
base = path.rstrip("/").rsplit("/", 1)[-1]
189+
s = re.escape(stem)
190+
191+
if base == stem:
192+
return f"{prefix}{stem}.log"
193+
194+
m = re.fullmatch(rf"{s}\.(\d+)\.gz", base)
195+
if m:
196+
return f"{prefix}{stem}.{m.group(1)}.gz.log"
197+
198+
m = re.fullmatch(rf"{s}\.(\d+)", base)
199+
if m:
200+
return f"{prefix}{stem}.{m.group(1)}.log"
201+
202+
middle = base[:-3] if base.endswith(".gz") else base
203+
return f"{prefix}{middle}.log"

‎test/unit/plugin/test_syslog_collector.py‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,12 @@ def run_map(cmd, **kwargs):
9191
c = get_collector(monkeypatch, run_map, system_info, conn_mock)
9292

9393
n = c._collect_syslog_rotations()
94-
assert n == 4
94+
assert n == [
95+
"rotated_syslog.log",
96+
"rotated_syslog.1.log",
97+
"rotated_syslog.2.gz.log",
98+
"rotated_syslog.10.gz.log",
99+
]
95100

96101
names = {a.filename for a in c.result.artifacts}
97102
assert names == {
@@ -114,7 +119,7 @@ def run_map(cmd, **kwargs):
114119
c = get_collector(monkeypatch, run_map, system_info, conn_mock)
115120

116121
n = c._collect_syslog_rotations()
117-
assert n == 0
122+
assert n == []
118123
assert c.result.artifacts == []
119124

120125
assert any(
@@ -137,15 +142,15 @@ def run_map(cmd, **kwargs):
137142
c = get_collector(monkeypatch, run_map, system_info, conn_mock)
138143

139144
n = c._collect_syslog_rotations()
140-
assert n == 0
145+
assert n == []
141146
assert c.result.artifacts == []
142147

143148
fail_events = [
144149
e for e in c._events if e["description"] == "Some syslog files could not be collected."
145150
]
146151
assert fail_events, "Expected a failure event"
147152
failed = fail_events[-1]["data"]["failed"]
148-
assert any(item["path"].endswith("/var/log/syslog.2.gz") for item in failed)
153+
assert failed == ["/var/log/syslog.2.gz"]
149154

150155

151156
def test_collect_data_integration(monkeypatch, system_info, conn_mock):
@@ -162,5 +167,5 @@ def run_map(cmd, **kwargs):
162167

163168
result, data = c.collect_data()
164169
assert isinstance(data, SyslogData)
165-
assert data.syslog_logs == 1
170+
assert data.syslog_logs == ["rotated_syslog.log"]
166171
assert c.result.message == "Syslog data collected"

0 commit comments

Comments
 (0)