From e7acde5d23e65fc3447000f62fe43304fa5950db Mon Sep 17 00:00:00 2001 From: bargajda Date: Mon, 23 Feb 2026 17:27:53 +0000 Subject: [PATCH 1/8] Add ThpPlugin for transparent huge pages collection and analysis Co-authored-by: Cursor --- nodescraper/plugins/inband/thp/__init__.py | 29 +++++ .../plugins/inband/thp/analyzer_args.py | 45 +++++++ .../plugins/inband/thp/thp_analyzer.py | 82 +++++++++++++ .../plugins/inband/thp/thp_collector.py | 110 ++++++++++++++++++ nodescraper/plugins/inband/thp/thp_plugin.py | 40 +++++++ nodescraper/plugins/inband/thp/thpdata.py | 40 +++++++ test/unit/plugin/test_thp_analyzer.py | 76 ++++++++++++ test/unit/plugin/test_thp_collector.py | 101 ++++++++++++++++ 8 files changed, 523 insertions(+) create mode 100644 nodescraper/plugins/inband/thp/__init__.py create mode 100644 nodescraper/plugins/inband/thp/analyzer_args.py create mode 100644 nodescraper/plugins/inband/thp/thp_analyzer.py create mode 100644 nodescraper/plugins/inband/thp/thp_collector.py create mode 100644 nodescraper/plugins/inband/thp/thp_plugin.py create mode 100644 nodescraper/plugins/inband/thp/thpdata.py create mode 100644 test/unit/plugin/test_thp_analyzer.py create mode 100644 test/unit/plugin/test_thp_collector.py diff --git a/nodescraper/plugins/inband/thp/__init__.py b/nodescraper/plugins/inband/thp/__init__.py new file mode 100644 index 0000000..98a7a56 --- /dev/null +++ b/nodescraper/plugins/inband/thp/__init__.py @@ -0,0 +1,29 @@ +############################################################################### +# +# MIT License +# +# Copyright (c) 2025 Advanced Micro Devices, Inc. +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in all +# copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. +# +############################################################################### +from .analyzer_args import ThpAnalyzerArgs +from .thp_plugin import ThpPlugin + +__all__ = ["ThpPlugin", "ThpAnalyzerArgs"] diff --git a/nodescraper/plugins/inband/thp/analyzer_args.py b/nodescraper/plugins/inband/thp/analyzer_args.py new file mode 100644 index 0000000..e173379 --- /dev/null +++ b/nodescraper/plugins/inband/thp/analyzer_args.py @@ -0,0 +1,45 @@ +############################################################################### +# +# MIT License +# +# Copyright (c) 2025 Advanced Micro Devices, Inc. +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in all +# copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. +# +############################################################################### +from typing import Optional + +from nodescraper.models import AnalyzerArgs + +from .thpdata import ThpDataModel + + +class ThpAnalyzerArgs(AnalyzerArgs): + """Expected THP settings for analysis.""" + + exp_enabled: Optional[str] = None # 'always', 'madvise', 'never' + exp_defrag: Optional[str] = None + + @classmethod + def build_from_model(cls, datamodel: ThpDataModel) -> "ThpAnalyzerArgs": + """Build analyzer args from the THP data model.""" + return cls( + exp_enabled=datamodel.enabled, + exp_defrag=datamodel.defrag, + ) diff --git a/nodescraper/plugins/inband/thp/thp_analyzer.py b/nodescraper/plugins/inband/thp/thp_analyzer.py new file mode 100644 index 0000000..2f21901 --- /dev/null +++ b/nodescraper/plugins/inband/thp/thp_analyzer.py @@ -0,0 +1,82 @@ +############################################################################### +# +# MIT License +# +# Copyright (c) 2025 Advanced Micro Devices, Inc. +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in all +# copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. +# +############################################################################### +from typing import Optional + +from nodescraper.enums import EventCategory, EventPriority, ExecutionStatus +from nodescraper.interfaces import DataAnalyzer +from nodescraper.models import TaskResult + +from .analyzer_args import ThpAnalyzerArgs +from .thpdata import ThpDataModel + + +class ThpAnalyzer(DataAnalyzer[ThpDataModel, ThpAnalyzerArgs]): + """Check THP settings against expected values.""" + + DATA_MODEL = ThpDataModel + + def analyze_data( + self, data: ThpDataModel, args: Optional[ThpAnalyzerArgs] = None + ) -> TaskResult: + """Compare THP data to expected settings.""" + mismatches = {} + + if not args: + args = ThpAnalyzerArgs() + + if args.exp_enabled is not None: + actual = (data.enabled or "").strip().lower() + expected = args.exp_enabled.strip().lower() + if actual != expected: + mismatches["enabled"] = {"expected": expected, "actual": data.enabled} + + if args.exp_defrag is not None: + actual = (data.defrag or "").strip().lower() + expected = args.exp_defrag.strip().lower() + if actual != expected: + mismatches["defrag"] = {"expected": expected, "actual": data.defrag} + + if mismatches: + self.result.status = ExecutionStatus.ERROR + self.result.message = "THP setting(s) do not match expected values." + self._log_event( + category=EventCategory.OS, + description="THP mismatch detected", + data=mismatches, + priority=EventPriority.ERROR, + console_log=True, + ) + else: + self._log_event( + category=EventCategory.OS, + description="THP settings match expected (or no expectations set)", + priority=EventPriority.INFO, + console_log=True, + ) + self.result.status = ExecutionStatus.OK + self.result.message = "THP settings as expected." + + return self.result diff --git a/nodescraper/plugins/inband/thp/thp_collector.py b/nodescraper/plugins/inband/thp/thp_collector.py new file mode 100644 index 0000000..fa951a7 --- /dev/null +++ b/nodescraper/plugins/inband/thp/thp_collector.py @@ -0,0 +1,110 @@ +############################################################################### +# +# MIT License +# +# Copyright (c) 2025 Advanced Micro Devices, Inc. +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in all +# copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. +# +############################################################################### +import re +from typing import Optional + +from nodescraper.base import InBandDataCollector +from nodescraper.enums import EventCategory, EventPriority, ExecutionStatus, OSFamily +from nodescraper.models import TaskResult + +from .thpdata import ThpDataModel + +THP_BASE = "/sys/kernel/mm/transparent_hugepage" +# Sysfs format: "[always] madvise never" -> extract bracketed value +BRACKETED_RE = re.compile(r"\[(\w+)\]") + + +def _parse_bracketed_setting(content: str) -> Optional[str]: + """Extract the active setting from sysfs content (value in square brackets).""" + if not content: + return None + match = BRACKETED_RE.search(content.strip()) + return match.group(1).strip() if match else None + + +class ThpCollector(InBandDataCollector[ThpDataModel, None]): + """Collect transparent huge pages (THP) settings from sysfs.""" + + DATA_MODEL = ThpDataModel + SUPPORTED_OS_FAMILY: set[OSFamily] = {OSFamily.LINUX} + + CMD_ENABLED = f"cat {THP_BASE}/enabled" + CMD_DEFRAG = f"cat {THP_BASE}/defrag" + + def collect_data(self, args=None) -> tuple[TaskResult, Optional[ThpDataModel]]: + """Collect THP enabled and defrag settings from the system.""" + if self.system_info.os_family != OSFamily.LINUX: + self._log_event( + category=EventCategory.OS, + description="THP collection is only supported on Linux.", + priority=EventPriority.WARNING, + console_log=True, + ) + return self.result, None + + enabled_raw = self._run_sut_cmd(self.CMD_ENABLED) + defrag_raw = self._run_sut_cmd(self.CMD_DEFRAG) + + enabled: Optional[str] = None + defrag: Optional[str] = None + + if enabled_raw.exit_code == 0 and enabled_raw.stdout: + enabled = _parse_bracketed_setting(enabled_raw.stdout) + else: + self._log_event( + category=EventCategory.OS, + description="Failed to read THP enabled setting", + data={"exit_code": enabled_raw.exit_code}, + priority=EventPriority.WARNING, + console_log=True, + ) + + if defrag_raw.exit_code == 0 and defrag_raw.stdout: + defrag = _parse_bracketed_setting(defrag_raw.stdout) + else: + self._log_event( + category=EventCategory.OS, + description="Failed to read THP defrag setting", + data={"exit_code": defrag_raw.exit_code}, + priority=EventPriority.WARNING, + console_log=True, + ) + + if enabled is None and defrag is None: + self.result.message = "THP settings not read" + self.result.status = ExecutionStatus.ERROR + return self.result, None + + thp_data = ThpDataModel(enabled=enabled, defrag=defrag) + self._log_event( + category=EventCategory.OS, + description="THP settings collected", + data=thp_data.model_dump(), + priority=EventPriority.INFO, + ) + self.result.message = f"THP enabled={enabled}, defrag={defrag}" + self.result.status = ExecutionStatus.OK + return self.result, thp_data diff --git a/nodescraper/plugins/inband/thp/thp_plugin.py b/nodescraper/plugins/inband/thp/thp_plugin.py new file mode 100644 index 0000000..ecbeda2 --- /dev/null +++ b/nodescraper/plugins/inband/thp/thp_plugin.py @@ -0,0 +1,40 @@ +############################################################################### +# +# MIT License +# +# Copyright (c) 2025 Advanced Micro Devices, Inc. +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in all +# copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. +# +############################################################################### +from nodescraper.base import InBandDataPlugin + +from .analyzer_args import ThpAnalyzerArgs +from .thp_analyzer import ThpAnalyzer +from .thp_collector import ThpCollector +from .thpdata import ThpDataModel + + +class ThpPlugin(InBandDataPlugin[ThpDataModel, None, ThpAnalyzerArgs]): + """Plugin to collect and optionally analyze transparent huge pages (THP) settings.""" + + DATA_MODEL = ThpDataModel + COLLECTOR = ThpCollector + ANALYZER = ThpAnalyzer + ANALYZER_ARGS = ThpAnalyzerArgs diff --git a/nodescraper/plugins/inband/thp/thpdata.py b/nodescraper/plugins/inband/thp/thpdata.py new file mode 100644 index 0000000..5c0d440 --- /dev/null +++ b/nodescraper/plugins/inband/thp/thpdata.py @@ -0,0 +1,40 @@ +############################################################################### +# +# MIT License +# +# Copyright (c) 2025 Advanced Micro Devices, Inc. +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in all +# copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. +# +############################################################################### +from typing import Optional + +from nodescraper.models import DataModel + + +class ThpDataModel(DataModel): + """Data model for transparent huge pages (THP) settings. + + Values are parsed from /sys/kernel/mm/transparent_hugepage/ (e.g. enabled, defrag). + The active setting is the one shown in brackets in the sysfs file + (e.g. '[always] madvise never' -> enabled='always'). + """ + + enabled: Optional[str] = None # 'always', 'madvise', or 'never' + defrag: Optional[str] = None # e.g. 'always', 'defer', 'madvise', 'never' diff --git a/test/unit/plugin/test_thp_analyzer.py b/test/unit/plugin/test_thp_analyzer.py new file mode 100644 index 0000000..e68a4cf --- /dev/null +++ b/test/unit/plugin/test_thp_analyzer.py @@ -0,0 +1,76 @@ +############################################################################### +# +# MIT License +# +# Copyright (c) 2025 Advanced Micro Devices, Inc. +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in all +# copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. +# +############################################################################### +import pytest + +from nodescraper.enums import ExecutionStatus +from nodescraper.plugins.inband.thp.analyzer_args import ThpAnalyzerArgs +from nodescraper.plugins.inband.thp.thp_analyzer import ThpAnalyzer +from nodescraper.plugins.inband.thp.thpdata import ThpDataModel + + +@pytest.fixture +def analyzer(system_info): + return ThpAnalyzer(system_info=system_info) + + +@pytest.fixture +def sample_data(): + return ThpDataModel(enabled="always", defrag="madvise") + + +def test_analyzer_no_args_match(analyzer, sample_data): + """No expected values -> OK.""" + result = analyzer.analyze_data(sample_data) + assert result.status == ExecutionStatus.OK + + +def test_analyzer_match(analyzer, sample_data): + """Expected values match -> OK.""" + args = ThpAnalyzerArgs(exp_enabled="always", exp_defrag="madvise") + result = analyzer.analyze_data(sample_data, args) + assert result.status == ExecutionStatus.OK + + +def test_analyzer_enabled_mismatch(analyzer, sample_data): + """Expected enabled differs -> ERROR.""" + args = ThpAnalyzerArgs(exp_enabled="never") + result = analyzer.analyze_data(sample_data, args) + assert result.status == ExecutionStatus.ERROR + assert "do not match" in result.message or "mismatch" in result.message.lower() + + +def test_analyzer_defrag_mismatch(analyzer, sample_data): + """Expected defrag differs -> ERROR.""" + args = ThpAnalyzerArgs(exp_defrag="never") + result = analyzer.analyze_data(sample_data, args) + assert result.status == ExecutionStatus.ERROR + + +def test_build_from_model(sample_data): + """build_from_model populates analyzer args from data model.""" + args = ThpAnalyzerArgs.build_from_model(sample_data) + assert args.exp_enabled == "always" + assert args.exp_defrag == "madvise" diff --git a/test/unit/plugin/test_thp_collector.py b/test/unit/plugin/test_thp_collector.py new file mode 100644 index 0000000..1fdb2fa --- /dev/null +++ b/test/unit/plugin/test_thp_collector.py @@ -0,0 +1,101 @@ +############################################################################### +# +# MIT License +# +# Copyright (c) 2025 Advanced Micro Devices, Inc. +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in all +# copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. +# +############################################################################### +from types import SimpleNamespace + +import pytest + +from nodescraper.enums import ExecutionStatus, OSFamily +from nodescraper.plugins.inband.thp.thp_collector import ThpCollector +from nodescraper.plugins.inband.thp.thpdata import ThpDataModel + + +@pytest.fixture +def linux_thp_collector(system_info, conn_mock): + system_info.os_family = OSFamily.LINUX + return ThpCollector(system_info=system_info, connection=conn_mock) + + +def make_artifact(exit_code, stdout): + return SimpleNamespace(command="", exit_code=exit_code, stdout=stdout, stderr="") + + +def test_collect_data_success(linux_thp_collector, conn_mock): + """Both enabled and defrag read successfully.""" + calls = [] + + def capture_cmd(cmd, **kwargs): + calls.append(cmd) + if "enabled" in cmd: + return make_artifact(0, "[always] madvise never") + return make_artifact(0, "[madvise] always never defer") + + linux_thp_collector._run_sut_cmd = capture_cmd + result, data = linux_thp_collector.collect_data() + + assert result.status == ExecutionStatus.OK + assert data is not None + assert isinstance(data, ThpDataModel) + assert data.enabled == "always" + assert data.defrag == "madvise" + assert "THP enabled=always" in result.message + + +def test_collect_data_enabled_fails(linux_thp_collector): + """Enabled read fails; defrag succeeds -> still get partial data.""" + def run_cmd(cmd, **kwargs): + if "enabled" in cmd: + return make_artifact(1, "") + return make_artifact(0, "[never] always madvise") + + linux_thp_collector._run_sut_cmd = run_cmd + result, data = linux_thp_collector.collect_data() + + assert result.status == ExecutionStatus.OK + assert data is not None + assert data.enabled is None + assert data.defrag == "never" + + +def test_collect_data_both_fail(linux_thp_collector): + """Both reads fail -> error.""" + def run_cmd(cmd, **kwargs): + return make_artifact(1, "") + + linux_thp_collector._run_sut_cmd = run_cmd + result, data = linux_thp_collector.collect_data() + + assert result.status == ExecutionStatus.ERROR + assert data is None + assert "THP settings not read" in result.message + + +def test_collector_raises_on_non_linux(system_info, conn_mock): + """ThpCollector does not support non-Linux; constructor raises.""" + from nodescraper.interfaces.task import SystemCompatibilityError + + system_info.os_family = OSFamily.WINDOWS + with pytest.raises(SystemCompatibilityError, match="not supported"): + ThpCollector(system_info=system_info, connection=conn_mock) From 257081183bdeb1f1fc02ab44fd53149b3d10dd1c Mon Sep 17 00:00:00 2001 From: bargajda Date: Mon, 23 Feb 2026 18:27:50 +0000 Subject: [PATCH 2/8] Fix Black formatting for pre-commit CI Co-authored-by: Cursor --- nodescraper/plugins/inband/thp/thpdata.py | 2 +- test/unit/plugin/test_thp_collector.py | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/nodescraper/plugins/inband/thp/thpdata.py b/nodescraper/plugins/inband/thp/thpdata.py index 5c0d440..51016ce 100644 --- a/nodescraper/plugins/inband/thp/thpdata.py +++ b/nodescraper/plugins/inband/thp/thpdata.py @@ -37,4 +37,4 @@ class ThpDataModel(DataModel): """ enabled: Optional[str] = None # 'always', 'madvise', or 'never' - defrag: Optional[str] = None # e.g. 'always', 'defer', 'madvise', 'never' + defrag: Optional[str] = None # e.g. 'always', 'defer', 'madvise', 'never' diff --git a/test/unit/plugin/test_thp_collector.py b/test/unit/plugin/test_thp_collector.py index 1fdb2fa..3400f32 100644 --- a/test/unit/plugin/test_thp_collector.py +++ b/test/unit/plugin/test_thp_collector.py @@ -65,6 +65,7 @@ def capture_cmd(cmd, **kwargs): def test_collect_data_enabled_fails(linux_thp_collector): """Enabled read fails; defrag succeeds -> still get partial data.""" + def run_cmd(cmd, **kwargs): if "enabled" in cmd: return make_artifact(1, "") @@ -81,6 +82,7 @@ def run_cmd(cmd, **kwargs): def test_collect_data_both_fail(linux_thp_collector): """Both reads fail -> error.""" + def run_cmd(cmd, **kwargs): return make_artifact(1, "") From 1338b8a45bbd260abcc5266214a4c287125fe2e7 Mon Sep 17 00:00:00 2001 From: bargajda Date: Tue, 24 Feb 2026 11:00:40 +0000 Subject: [PATCH 3/8] Copyright 2026, CMD for docs, design proposal (copyright + SysfsPlugin design, remove backward compat and section 3) Co-authored-by: Cursor --- docs/THP_TO_SYSFS_DESIGN_PROPOSAL.md | 87 +++++++++++++++++++ nodescraper/plugins/inband/thp/__init__.py | 2 +- .../plugins/inband/thp/analyzer_args.py | 2 +- .../plugins/inband/thp/thp_analyzer.py | 2 +- .../plugins/inband/thp/thp_collector.py | 4 +- nodescraper/plugins/inband/thp/thp_plugin.py | 2 +- nodescraper/plugins/inband/thp/thpdata.py | 2 +- test/unit/plugin/test_thp_analyzer.py | 2 +- test/unit/plugin/test_thp_collector.py | 2 +- 9 files changed, 97 insertions(+), 8 deletions(-) create mode 100644 docs/THP_TO_SYSFS_DESIGN_PROPOSAL.md diff --git a/docs/THP_TO_SYSFS_DESIGN_PROPOSAL.md b/docs/THP_TO_SYSFS_DESIGN_PROPOSAL.md new file mode 100644 index 0000000..228c63c --- /dev/null +++ b/docs/THP_TO_SYSFS_DESIGN_PROPOSAL.md @@ -0,0 +1,87 @@ +# Proposal: ThpPlugin → Generic SysfsPlugin (design + copyright) + +## 1. Copyright updates (implemented) + +- **Rule:** New files added in 2026 must use copyright year **2026**. +- **Done:** All ThpPlugin-related files updated from `2025` to `2026`: + - `nodescraper/plugins/inband/thp/*.py` (6 files) + - `test/unit/plugin/test_thp_collector.py`, `test_thp_analyzer.py` + +--- + +## 2. Design: From ThpPlugin to a generic SysfsPlugin + +### 2.1 Goals (from review) + +- **Generic plugin:** Rename to something like **SysfsPlugin** (or SysfsSettingsPlugin) so it can check any sysfs paths, not only THP. +- **Config-driven paths:** Collector commands and checks are driven by a list of paths (and optional expected values) from analyzer args, not hardcoded to `/sys/kernel/mm/transparent_hugepage/`. +- **Doc generation:** Use a stable `CMD` (or `CMD_<>`) variable built from the list of paths so the automated doc generator can include the commands in the docs. +- **Flexible expectations:** For each path, support an optional list of allowed values; empty list means “only check that the path exists / value is read”, no value assertion. + +### 2.2 Proposed config shape (`plugin_config.json`) + +```json +{ + "plugins": { + "SysfsPlugin": { + "analysis_args": { + "checks": [ + { + "path": "/sys/kernel/mm/transparent_hugepage/enabled", + "expected": ["always", "[always]"], + "name": "THP enabled" + }, + { + "path": "/sys/kernel/mm/transparent_hugepage/defrag", + "expected": ["always", "[always]"], + "name": "THP defrag" + } + ] + } + } + } +} +``` + +- **`checks`:** List of objects, one per sysfs path. +- **`path`:** Full sysfs path (e.g. `.../enabled`, `.../defrag`). +- **`expected`:** Optional list of accepted values (e.g. raw `always` or as shown in file `[always]`). **Empty list `[]`:** do not assert value, only that the path is readable (or that we got a value). +- **`name`:** Human-readable label for logs/events (e.g. "THP enabled"). + +### 2.3 Data model + +- **Collector output:** One structure per “check”, e.g.: + - `path`, `value_read` (string or None if read failed), optional `name`. +- So the data model is **list-based** (one entry per path) rather than fixed fields like `enabled` / `defrag`. +- Parsing: keep support for “bracketed” sysfs format (e.g. `[always] madvise never`) and optionally store both raw and normalized value. + +### 2.4 Collector behavior + +- **Paths:** Build the list of paths from analyzer args (e.g. from `checks[].path`). If no analyzer args are provided, use a default list (e.g. current THP paths) so the plugin still works without config. +- **Commands:** For each path, run e.g. `cat `. Define a variable so the doc generator can pick it up, e.g.: + - `CMD_READ = "cat {}"` and document that `{}` is replaced by each path from the configured `checks`, or + - A single `CMD` that reflects “one command per path” (e.g. “cat <path> for each path in analysis_args.checks”). +- **Docs:** Use a stable `CMD` / `CMD_<>` pattern as required by the existing doc generator. + +### 2.5 Analyzer behavior + +- For each check: + - If `expected` is non-empty: assert `value_read` is in `expected` (after normalizing if needed). + - If `expected` is empty: only assert that a value was read (path readable, no value check). +- Emit clear events (e.g. by `name`) on mismatch or read failure. + +### 2.6 Naming and packaging + +- **Plugin name:** `ThpPlugin` → **SysfsPlugin** (or SysfsSettingsPlugin). +- **Package:** Either rename `thp` → `sysfs` and keep one plugin, or keep package name and have `SysfsPlugin` live under a renamed module for clarity. Recommendation: **rename to `sysfs`** and have `SysfsPlugin`, `SysfsCollector`, `SysfsAnalyzer`, `SysfsDataModel`, `SysfsAnalyzerArgs` for consistency and future use (many sysfs paths). + +### 2.7 Summary table + +| Area | Current (ThpPlugin) | Proposed (SysfsPlugin) | +|----------------|------------------------|-------------------------------------------------| +| Plugin name | ThpPlugin | SysfsPlugin (or SysfsSettingsPlugin) | +| Paths | Hardcoded THP paths | From `analysis_args.checks[].path` | +| Expected values| Fixed `exp_enabled` / `exp_defrag` | Per-check `expected` list; `[]` = no assertion | +| Data model | `enabled`, `defrag` | List of `{ path, value_read, name? }` | +| Collector CMD | Fixed `cat` for two files | `CMD = "cat {}"` with paths from checks | +| Copyright | 2025 | 2026 (done) | diff --git a/nodescraper/plugins/inband/thp/__init__.py b/nodescraper/plugins/inband/thp/__init__.py index 98a7a56..7de0478 100644 --- a/nodescraper/plugins/inband/thp/__init__.py +++ b/nodescraper/plugins/inband/thp/__init__.py @@ -2,7 +2,7 @@ # # MIT License # -# Copyright (c) 2025 Advanced Micro Devices, Inc. +# Copyright (c) 2026 Advanced Micro Devices, Inc. # # Permission is hereby granted, free of charge, to any person obtaining a copy # of this software and associated documentation files (the "Software"), to deal diff --git a/nodescraper/plugins/inband/thp/analyzer_args.py b/nodescraper/plugins/inband/thp/analyzer_args.py index e173379..e77c8b0 100644 --- a/nodescraper/plugins/inband/thp/analyzer_args.py +++ b/nodescraper/plugins/inband/thp/analyzer_args.py @@ -2,7 +2,7 @@ # # MIT License # -# Copyright (c) 2025 Advanced Micro Devices, Inc. +# Copyright (c) 2026 Advanced Micro Devices, Inc. # # Permission is hereby granted, free of charge, to any person obtaining a copy # of this software and associated documentation files (the "Software"), to deal diff --git a/nodescraper/plugins/inband/thp/thp_analyzer.py b/nodescraper/plugins/inband/thp/thp_analyzer.py index 2f21901..a2f300c 100644 --- a/nodescraper/plugins/inband/thp/thp_analyzer.py +++ b/nodescraper/plugins/inband/thp/thp_analyzer.py @@ -2,7 +2,7 @@ # # MIT License # -# Copyright (c) 2025 Advanced Micro Devices, Inc. +# Copyright (c) 2026 Advanced Micro Devices, Inc. # # Permission is hereby granted, free of charge, to any person obtaining a copy # of this software and associated documentation files (the "Software"), to deal diff --git a/nodescraper/plugins/inband/thp/thp_collector.py b/nodescraper/plugins/inband/thp/thp_collector.py index fa951a7..561a81e 100644 --- a/nodescraper/plugins/inband/thp/thp_collector.py +++ b/nodescraper/plugins/inband/thp/thp_collector.py @@ -2,7 +2,7 @@ # # MIT License # -# Copyright (c) 2025 Advanced Micro Devices, Inc. +# Copyright (c) 2026 Advanced Micro Devices, Inc. # # Permission is hereby granted, free of charge, to any person obtaining a copy # of this software and associated documentation files (the "Software"), to deal @@ -51,6 +51,8 @@ class ThpCollector(InBandDataCollector[ThpDataModel, None]): DATA_MODEL = ThpDataModel SUPPORTED_OS_FAMILY: set[OSFamily] = {OSFamily.LINUX} + # Command template for doc generator: {} is each sysfs path (e.g. from checks). + CMD = "cat {}" CMD_ENABLED = f"cat {THP_BASE}/enabled" CMD_DEFRAG = f"cat {THP_BASE}/defrag" diff --git a/nodescraper/plugins/inband/thp/thp_plugin.py b/nodescraper/plugins/inband/thp/thp_plugin.py index ecbeda2..9e41339 100644 --- a/nodescraper/plugins/inband/thp/thp_plugin.py +++ b/nodescraper/plugins/inband/thp/thp_plugin.py @@ -2,7 +2,7 @@ # # MIT License # -# Copyright (c) 2025 Advanced Micro Devices, Inc. +# Copyright (c) 2026 Advanced Micro Devices, Inc. # # Permission is hereby granted, free of charge, to any person obtaining a copy # of this software and associated documentation files (the "Software"), to deal diff --git a/nodescraper/plugins/inband/thp/thpdata.py b/nodescraper/plugins/inband/thp/thpdata.py index 51016ce..c970c04 100644 --- a/nodescraper/plugins/inband/thp/thpdata.py +++ b/nodescraper/plugins/inband/thp/thpdata.py @@ -2,7 +2,7 @@ # # MIT License # -# Copyright (c) 2025 Advanced Micro Devices, Inc. +# Copyright (c) 2026 Advanced Micro Devices, Inc. # # Permission is hereby granted, free of charge, to any person obtaining a copy # of this software and associated documentation files (the "Software"), to deal diff --git a/test/unit/plugin/test_thp_analyzer.py b/test/unit/plugin/test_thp_analyzer.py index e68a4cf..fe6ccf9 100644 --- a/test/unit/plugin/test_thp_analyzer.py +++ b/test/unit/plugin/test_thp_analyzer.py @@ -2,7 +2,7 @@ # # MIT License # -# Copyright (c) 2025 Advanced Micro Devices, Inc. +# Copyright (c) 2026 Advanced Micro Devices, Inc. # # Permission is hereby granted, free of charge, to any person obtaining a copy # of this software and associated documentation files (the "Software"), to deal diff --git a/test/unit/plugin/test_thp_collector.py b/test/unit/plugin/test_thp_collector.py index 3400f32..93b36d4 100644 --- a/test/unit/plugin/test_thp_collector.py +++ b/test/unit/plugin/test_thp_collector.py @@ -2,7 +2,7 @@ # # MIT License # -# Copyright (c) 2025 Advanced Micro Devices, Inc. +# Copyright (c) 2026 Advanced Micro Devices, Inc. # # Permission is hereby granted, free of charge, to any person obtaining a copy # of this software and associated documentation files (the "Software"), to deal From a47540f2b065a26e448d17bad453046865a005a2 Mon Sep 17 00:00:00 2001 From: Alexandra Bara Date: Tue, 24 Feb 2026 12:02:26 -0600 Subject: [PATCH 4/8] fixes, updates, rename, functional tests --- docs/THP_TO_SYSFS_DESIGN_PROPOSAL.md | 87 ------------ .../inband/{thp => sys_settings}/__init__.py | 6 +- .../inband/sys_settings/analyzer_args.py | 68 +++++++++ .../collector_args.py} | 72 +++++----- .../sys_settings/sys_settings_analyzer.py | 127 +++++++++++++++++ .../sys_settings/sys_settings_collector.py | 132 ++++++++++++++++++ .../sys_settings_data.py} | 78 +++++------ .../sys_settings_plugin.py} | 89 ++++++------ .../plugins/inband/thp/thp_analyzer.py | 82 ----------- .../plugins/inband/thp/thp_collector.py | 112 --------------- test/functional/test_plugin_configs.py | 2 + test/functional/test_sys_settings_plugin.py | 87 ++++++++++++ .../unit/plugin/test_sys_settings_analyzer.py | 107 ++++++++++++++ ...ctor.py => test_sys_settings_collector.py} | 69 +++++---- test/unit/plugin/test_thp_analyzer.py | 76 ---------- 15 files changed, 684 insertions(+), 510 deletions(-) delete mode 100644 docs/THP_TO_SYSFS_DESIGN_PROPOSAL.md rename nodescraper/plugins/inband/{thp => sys_settings}/__init__.py (87%) create mode 100644 nodescraper/plugins/inband/sys_settings/analyzer_args.py rename nodescraper/plugins/inband/{thp/thp_plugin.py => sys_settings/collector_args.py} (72%) create mode 100644 nodescraper/plugins/inband/sys_settings/sys_settings_analyzer.py create mode 100644 nodescraper/plugins/inband/sys_settings/sys_settings_collector.py rename nodescraper/plugins/inband/{thp/thpdata.py => sys_settings/sys_settings_data.py} (73%) rename nodescraper/plugins/inband/{thp/analyzer_args.py => sys_settings/sys_settings_plugin.py} (62%) delete mode 100644 nodescraper/plugins/inband/thp/thp_analyzer.py delete mode 100644 nodescraper/plugins/inband/thp/thp_collector.py create mode 100644 test/functional/test_sys_settings_plugin.py create mode 100644 test/unit/plugin/test_sys_settings_analyzer.py rename test/unit/plugin/{test_thp_collector.py => test_sys_settings_collector.py} (55%) delete mode 100644 test/unit/plugin/test_thp_analyzer.py diff --git a/docs/THP_TO_SYSFS_DESIGN_PROPOSAL.md b/docs/THP_TO_SYSFS_DESIGN_PROPOSAL.md deleted file mode 100644 index 228c63c..0000000 --- a/docs/THP_TO_SYSFS_DESIGN_PROPOSAL.md +++ /dev/null @@ -1,87 +0,0 @@ -# Proposal: ThpPlugin → Generic SysfsPlugin (design + copyright) - -## 1. Copyright updates (implemented) - -- **Rule:** New files added in 2026 must use copyright year **2026**. -- **Done:** All ThpPlugin-related files updated from `2025` to `2026`: - - `nodescraper/plugins/inband/thp/*.py` (6 files) - - `test/unit/plugin/test_thp_collector.py`, `test_thp_analyzer.py` - ---- - -## 2. Design: From ThpPlugin to a generic SysfsPlugin - -### 2.1 Goals (from review) - -- **Generic plugin:** Rename to something like **SysfsPlugin** (or SysfsSettingsPlugin) so it can check any sysfs paths, not only THP. -- **Config-driven paths:** Collector commands and checks are driven by a list of paths (and optional expected values) from analyzer args, not hardcoded to `/sys/kernel/mm/transparent_hugepage/`. -- **Doc generation:** Use a stable `CMD` (or `CMD_<>`) variable built from the list of paths so the automated doc generator can include the commands in the docs. -- **Flexible expectations:** For each path, support an optional list of allowed values; empty list means “only check that the path exists / value is read”, no value assertion. - -### 2.2 Proposed config shape (`plugin_config.json`) - -```json -{ - "plugins": { - "SysfsPlugin": { - "analysis_args": { - "checks": [ - { - "path": "/sys/kernel/mm/transparent_hugepage/enabled", - "expected": ["always", "[always]"], - "name": "THP enabled" - }, - { - "path": "/sys/kernel/mm/transparent_hugepage/defrag", - "expected": ["always", "[always]"], - "name": "THP defrag" - } - ] - } - } - } -} -``` - -- **`checks`:** List of objects, one per sysfs path. -- **`path`:** Full sysfs path (e.g. `.../enabled`, `.../defrag`). -- **`expected`:** Optional list of accepted values (e.g. raw `always` or as shown in file `[always]`). **Empty list `[]`:** do not assert value, only that the path is readable (or that we got a value). -- **`name`:** Human-readable label for logs/events (e.g. "THP enabled"). - -### 2.3 Data model - -- **Collector output:** One structure per “check”, e.g.: - - `path`, `value_read` (string or None if read failed), optional `name`. -- So the data model is **list-based** (one entry per path) rather than fixed fields like `enabled` / `defrag`. -- Parsing: keep support for “bracketed” sysfs format (e.g. `[always] madvise never`) and optionally store both raw and normalized value. - -### 2.4 Collector behavior - -- **Paths:** Build the list of paths from analyzer args (e.g. from `checks[].path`). If no analyzer args are provided, use a default list (e.g. current THP paths) so the plugin still works without config. -- **Commands:** For each path, run e.g. `cat `. Define a variable so the doc generator can pick it up, e.g.: - - `CMD_READ = "cat {}"` and document that `{}` is replaced by each path from the configured `checks`, or - - A single `CMD` that reflects “one command per path” (e.g. “cat <path> for each path in analysis_args.checks”). -- **Docs:** Use a stable `CMD` / `CMD_<>` pattern as required by the existing doc generator. - -### 2.5 Analyzer behavior - -- For each check: - - If `expected` is non-empty: assert `value_read` is in `expected` (after normalizing if needed). - - If `expected` is empty: only assert that a value was read (path readable, no value check). -- Emit clear events (e.g. by `name`) on mismatch or read failure. - -### 2.6 Naming and packaging - -- **Plugin name:** `ThpPlugin` → **SysfsPlugin** (or SysfsSettingsPlugin). -- **Package:** Either rename `thp` → `sysfs` and keep one plugin, or keep package name and have `SysfsPlugin` live under a renamed module for clarity. Recommendation: **rename to `sysfs`** and have `SysfsPlugin`, `SysfsCollector`, `SysfsAnalyzer`, `SysfsDataModel`, `SysfsAnalyzerArgs` for consistency and future use (many sysfs paths). - -### 2.7 Summary table - -| Area | Current (ThpPlugin) | Proposed (SysfsPlugin) | -|----------------|------------------------|-------------------------------------------------| -| Plugin name | ThpPlugin | SysfsPlugin (or SysfsSettingsPlugin) | -| Paths | Hardcoded THP paths | From `analysis_args.checks[].path` | -| Expected values| Fixed `exp_enabled` / `exp_defrag` | Per-check `expected` list; `[]` = no assertion | -| Data model | `enabled`, `defrag` | List of `{ path, value_read, name? }` | -| Collector CMD | Fixed `cat` for two files | `CMD = "cat {}"` with paths from checks | -| Copyright | 2025 | 2026 (done) | diff --git a/nodescraper/plugins/inband/thp/__init__.py b/nodescraper/plugins/inband/sys_settings/__init__.py similarity index 87% rename from nodescraper/plugins/inband/thp/__init__.py rename to nodescraper/plugins/inband/sys_settings/__init__.py index 7de0478..79a10bd 100644 --- a/nodescraper/plugins/inband/thp/__init__.py +++ b/nodescraper/plugins/inband/sys_settings/__init__.py @@ -23,7 +23,7 @@ # SOFTWARE. # ############################################################################### -from .analyzer_args import ThpAnalyzerArgs -from .thp_plugin import ThpPlugin +from .analyzer_args import SysfsCheck, SysSettingsAnalyzerArgs +from .sys_settings_plugin import SysSettingsPlugin -__all__ = ["ThpPlugin", "ThpAnalyzerArgs"] +__all__ = ["SysSettingsPlugin", "SysSettingsAnalyzerArgs", "SysfsCheck"] diff --git a/nodescraper/plugins/inband/sys_settings/analyzer_args.py b/nodescraper/plugins/inband/sys_settings/analyzer_args.py new file mode 100644 index 0000000..e3d4e06 --- /dev/null +++ b/nodescraper/plugins/inband/sys_settings/analyzer_args.py @@ -0,0 +1,68 @@ +############################################################################### +# +# MIT License +# +# Copyright (c) 2026 Advanced Micro Devices, Inc. +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in all +# copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. +# +############################################################################### +from typing import Optional + +from pydantic import BaseModel + +from nodescraper.models import AnalyzerArgs + + +class SysfsCheck(BaseModel): + """One sysfs check: path to read, acceptable values, and display name. + + If expected is an empty list, the check is treated as passing (no constraint). + """ + + path: str + expected: list[str] + name: str + + +class SysSettingsAnalyzerArgs(AnalyzerArgs): + """Sysfs settings for analysis via a list of checks (path, expected values, name). + + The path in each check is the sysfs path to read; the collector uses these paths + when collection_args is derived from analysis_args (e.g. by the plugin). + """ + + checks: Optional[list[SysfsCheck]] = None + + def paths_to_collect(self) -> list[str]: + """Return the unique sysfs paths from checks, for use by the collector. + + Returns: + List of unique path strings from self.checks, preserving order of first occurrence. + """ + if not self.checks: + return [] + seen = set() + out = [] + for c in self.checks: + p = c.path.rstrip("/") + if p not in seen: + seen.add(p) + out.append(c.path) + return out diff --git a/nodescraper/plugins/inband/thp/thp_plugin.py b/nodescraper/plugins/inband/sys_settings/collector_args.py similarity index 72% rename from nodescraper/plugins/inband/thp/thp_plugin.py rename to nodescraper/plugins/inband/sys_settings/collector_args.py index 9e41339..4f321d3 100644 --- a/nodescraper/plugins/inband/thp/thp_plugin.py +++ b/nodescraper/plugins/inband/sys_settings/collector_args.py @@ -1,40 +1,32 @@ -############################################################################### -# -# MIT License -# -# Copyright (c) 2026 Advanced Micro Devices, Inc. -# -# Permission is hereby granted, free of charge, to any person obtaining a copy -# of this software and associated documentation files (the "Software"), to deal -# in the Software without restriction, including without limitation the rights -# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -# copies of the Software, and to permit persons to whom the Software is -# furnished to do so, subject to the following conditions: -# -# The above copyright notice and this permission notice shall be included in all -# copies or substantial portions of the Software. -# -# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -# SOFTWARE. -# -############################################################################### -from nodescraper.base import InBandDataPlugin - -from .analyzer_args import ThpAnalyzerArgs -from .thp_analyzer import ThpAnalyzer -from .thp_collector import ThpCollector -from .thpdata import ThpDataModel - - -class ThpPlugin(InBandDataPlugin[ThpDataModel, None, ThpAnalyzerArgs]): - """Plugin to collect and optionally analyze transparent huge pages (THP) settings.""" - - DATA_MODEL = ThpDataModel - COLLECTOR = ThpCollector - ANALYZER = ThpAnalyzer - ANALYZER_ARGS = ThpAnalyzerArgs +############################################################################### +# +# MIT License +# +# Copyright (c) 2026 Advanced Micro Devices, Inc. +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in all +# copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. +# +############################################################################### +from pydantic import BaseModel + + +class SysSettingsCollectorArgs(BaseModel): + """Collection args for SysSettingsCollector: list of sysfs paths to read.""" + + paths: list[str] = [] diff --git a/nodescraper/plugins/inband/sys_settings/sys_settings_analyzer.py b/nodescraper/plugins/inband/sys_settings/sys_settings_analyzer.py new file mode 100644 index 0000000..06c3811 --- /dev/null +++ b/nodescraper/plugins/inband/sys_settings/sys_settings_analyzer.py @@ -0,0 +1,127 @@ +############################################################################### +# +# MIT License +# +# Copyright (c) 2026 Advanced Micro Devices, Inc. +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in all +# copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. +# +############################################################################### +from typing import Optional, cast + +from nodescraper.enums import EventCategory, EventPriority, ExecutionStatus +from nodescraper.interfaces import DataAnalyzer +from nodescraper.models import TaskResult + +from .analyzer_args import SysSettingsAnalyzerArgs +from .sys_settings_data import SysSettingsDataModel + + +def _get_actual_for_path(data: SysSettingsDataModel, path: str) -> Optional[str]: + """Return the actual value from the data model for the given sysfs path. + + Args: + data: Collected sysfs readings (path -> value). + path: Sysfs path (with or without trailing slash). + + Returns: + Normalized value for that path, or None if not present. + """ + value = data.readings.get(path) or data.readings.get(path.rstrip("/")) + return (value or "").strip().lower() if value is not None else None + + +class SysSettingsAnalyzer(DataAnalyzer[SysSettingsDataModel, SysSettingsAnalyzerArgs]): + """Check sysfs settings against expected values from the checks list.""" + + DATA_MODEL = SysSettingsDataModel + + def analyze_data( + self, data: SysSettingsDataModel, args: Optional[SysSettingsAnalyzerArgs] = None + ) -> TaskResult: + """Compare sysfs data to expected settings from args.checks. + + Args: + data: Collected sysfs readings to check. + args: Analyzer args with checks (path, expected, name). If None or no checks, returns OK. + + Returns: + TaskResult with status OK if all checks pass, ERROR if any mismatch or missing path. + """ + mismatches = {} + + if not args or not args.checks: + self.result.status = ExecutionStatus.OK + self.result.message = "No checks configured." + return self.result + + for check in args.checks: + actual = _get_actual_for_path(data, check.path) + if actual is None: + mismatches[check.name] = { + "path": check.path, + "expected": check.expected, + "actual": None, + "reason": "path not collected by this plugin", + } + continue + + if not check.expected: + continue + expected_normalized = [e.strip().lower() for e in check.expected] + if actual not in expected_normalized: + raw = data.readings.get(check.path) or data.readings.get(check.path.rstrip("/")) + mismatches[check.name] = { + "path": check.path, + "expected": check.expected, + "actual": raw, + } + + if mismatches: + self.result.status = ExecutionStatus.ERROR + parts = [] + for name, info in mismatches.items(): + path = info.get("path", "") + expected = info.get("expected") + actual = cast(Optional[str], info.get("actual")) + reason = info.get("reason") + if reason: + part = f"{name} ({path})" + else: + part = f"{name} ({path}): expected one of {expected}, actual {repr(actual)}" + parts.append(part) + self.result.message = "Sysfs mismatch: " + "; ".join(parts) + self._log_event( + category=EventCategory.OS, + description="Sysfs mismatch detected", + data=mismatches, + priority=EventPriority.ERROR, + console_log=True, + ) + else: + self._log_event( + category=EventCategory.OS, + description="Sysfs settings match expected", + priority=EventPriority.INFO, + console_log=True, + ) + self.result.status = ExecutionStatus.OK + self.result.message = "Sysfs settings as expected." + + return self.result diff --git a/nodescraper/plugins/inband/sys_settings/sys_settings_collector.py b/nodescraper/plugins/inband/sys_settings/sys_settings_collector.py new file mode 100644 index 0000000..da08934 --- /dev/null +++ b/nodescraper/plugins/inband/sys_settings/sys_settings_collector.py @@ -0,0 +1,132 @@ +############################################################################### +# +# MIT License +# +# Copyright (c) 2026 Advanced Micro Devices, Inc. +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in all +# copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. +# +############################################################################### +import re +from typing import Optional + +from nodescraper.base import InBandDataCollector +from nodescraper.enums import EventCategory, EventPriority, ExecutionStatus, OSFamily +from nodescraper.models import TaskResult + +from .collector_args import SysSettingsCollectorArgs +from .sys_settings_data import SysSettingsDataModel + +# Sysfs format: "[always] madvise never" -> extract bracketed value +BRACKETED_RE = re.compile(r"\[(\w+)\]") + + +def _parse_bracketed_setting(content: str) -> Optional[str]: + """Extract the active setting from sysfs content (value in square brackets). + + Args: + content: Raw sysfs file content (e.g. "[always] madvise never"). + + Returns: + The bracketed value if present, else None. + """ + if not content: + return None + match = BRACKETED_RE.search(content.strip()) + return match.group(1).strip() if match else None + + +def _paths_from_args(args: Optional[SysSettingsCollectorArgs]) -> list[str]: + """Extract list of sysfs paths from collection args. + + Args: + args: Collector args containing paths to read, or None. + + Returns: + List of sysfs paths; empty if args is None or args.paths is empty. + """ + if args is None: + return [] + return list(args.paths) if args.paths else [] + + +class SysSettingsCollector(InBandDataCollector[SysSettingsDataModel, SysSettingsCollectorArgs]): + """Collect sysfs settings from user-specified paths (paths come from config/args).""" + + DATA_MODEL = SysSettingsDataModel + SUPPORTED_OS_FAMILY: set[OSFamily] = {OSFamily.LINUX} + + CMD = "cat {}" + + def collect_data( + self, args: Optional[SysSettingsCollectorArgs] = None + ) -> tuple[TaskResult, Optional[SysSettingsDataModel]]: + """Collect sysfs values for each path in args.paths. + + Args: + args: Collector args with paths to read; if None or empty paths, returns NOT_RAN. + + Returns: + Tuple of (TaskResult, SysSettingsDataModel or None). Data is None on NOT_RAN or ERROR. + """ + if self.system_info.os_family != OSFamily.LINUX: + self._log_event( + category=EventCategory.OS, + description="Sysfs collection is only supported on Linux.", + priority=EventPriority.WARNING, + console_log=True, + ) + return self.result, None + + paths = _paths_from_args(args) + if not paths: + self.result.message = "No paths configured for sysfs collection" + self.result.status = ExecutionStatus.NOT_RAN + return self.result, None + + readings: dict[str, str] = {} + for path in paths: + res = self._run_sut_cmd(self.CMD.format(path), sudo=False) + if res.exit_code == 0 and res.stdout: + value = _parse_bracketed_setting(res.stdout) or res.stdout.strip() + readings[path] = value + else: + self._log_event( + category=EventCategory.OS, + description=f"Failed to read sysfs path: {path}", + data={"exit_code": res.exit_code}, + priority=EventPriority.WARNING, + console_log=True, + ) + + if not readings: + self.result.message = "Sysfs settings not read" + self.result.status = ExecutionStatus.ERROR + return self.result, None + + sys_settings_data = SysSettingsDataModel(readings=readings) + self._log_event( + category=EventCategory.OS, + description="Sysfs settings collected", + data=sys_settings_data.model_dump(), + priority=EventPriority.INFO, + ) + self.result.message = f"Sysfs collected {len(readings)} path(s)" + self.result.status = ExecutionStatus.OK + return self.result, sys_settings_data diff --git a/nodescraper/plugins/inband/thp/thpdata.py b/nodescraper/plugins/inband/sys_settings/sys_settings_data.py similarity index 73% rename from nodescraper/plugins/inband/thp/thpdata.py rename to nodescraper/plugins/inband/sys_settings/sys_settings_data.py index c970c04..acec4b4 100644 --- a/nodescraper/plugins/inband/thp/thpdata.py +++ b/nodescraper/plugins/inband/sys_settings/sys_settings_data.py @@ -1,40 +1,38 @@ -############################################################################### -# -# MIT License -# -# Copyright (c) 2026 Advanced Micro Devices, Inc. -# -# Permission is hereby granted, free of charge, to any person obtaining a copy -# of this software and associated documentation files (the "Software"), to deal -# in the Software without restriction, including without limitation the rights -# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -# copies of the Software, and to permit persons to whom the Software is -# furnished to do so, subject to the following conditions: -# -# The above copyright notice and this permission notice shall be included in all -# copies or substantial portions of the Software. -# -# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -# SOFTWARE. -# -############################################################################### -from typing import Optional - -from nodescraper.models import DataModel - - -class ThpDataModel(DataModel): - """Data model for transparent huge pages (THP) settings. - - Values are parsed from /sys/kernel/mm/transparent_hugepage/ (e.g. enabled, defrag). - The active setting is the one shown in brackets in the sysfs file - (e.g. '[always] madvise never' -> enabled='always'). - """ - - enabled: Optional[str] = None # 'always', 'madvise', or 'never' - defrag: Optional[str] = None # e.g. 'always', 'defer', 'madvise', 'never' +############################################################################### +# +# MIT License +# +# Copyright (c) 2026 Advanced Micro Devices, Inc. +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in all +# copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. +# +############################################################################### +from pydantic import Field + +from nodescraper.models import DataModel + + +class SysSettingsDataModel(DataModel): + """Data model for sysfs settings: path -> parsed value. + + Values are parsed from user-specified sysfs paths (bracketed value extracted + when present, e.g. '[always] madvise never' -> 'always'). + """ + + readings: dict[str, str] = Field(default_factory=dict) # sysfs path (as given) -> parsed value diff --git a/nodescraper/plugins/inband/thp/analyzer_args.py b/nodescraper/plugins/inband/sys_settings/sys_settings_plugin.py similarity index 62% rename from nodescraper/plugins/inband/thp/analyzer_args.py rename to nodescraper/plugins/inband/sys_settings/sys_settings_plugin.py index e77c8b0..158ac6f 100644 --- a/nodescraper/plugins/inband/thp/analyzer_args.py +++ b/nodescraper/plugins/inband/sys_settings/sys_settings_plugin.py @@ -1,45 +1,44 @@ -############################################################################### -# -# MIT License -# -# Copyright (c) 2026 Advanced Micro Devices, Inc. -# -# Permission is hereby granted, free of charge, to any person obtaining a copy -# of this software and associated documentation files (the "Software"), to deal -# in the Software without restriction, including without limitation the rights -# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -# copies of the Software, and to permit persons to whom the Software is -# furnished to do so, subject to the following conditions: -# -# The above copyright notice and this permission notice shall be included in all -# copies or substantial portions of the Software. -# -# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -# SOFTWARE. -# -############################################################################### -from typing import Optional - -from nodescraper.models import AnalyzerArgs - -from .thpdata import ThpDataModel - - -class ThpAnalyzerArgs(AnalyzerArgs): - """Expected THP settings for analysis.""" - - exp_enabled: Optional[str] = None # 'always', 'madvise', 'never' - exp_defrag: Optional[str] = None - - @classmethod - def build_from_model(cls, datamodel: ThpDataModel) -> "ThpAnalyzerArgs": - """Build analyzer args from the THP data model.""" - return cls( - exp_enabled=datamodel.enabled, - exp_defrag=datamodel.defrag, - ) +############################################################################### +# +# MIT License +# +# Copyright (c) 2026 Advanced Micro Devices, Inc. +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in all +# copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. +# +############################################################################### +from nodescraper.base import InBandDataPlugin + +from .analyzer_args import SysSettingsAnalyzerArgs +from .collector_args import SysSettingsCollectorArgs +from .sys_settings_analyzer import SysSettingsAnalyzer +from .sys_settings_collector import SysSettingsCollector +from .sys_settings_data import SysSettingsDataModel + + +class SysSettingsPlugin( + InBandDataPlugin[SysSettingsDataModel, SysSettingsCollectorArgs, SysSettingsAnalyzerArgs] +): + """Plugin to collect and analyze sysfs settings from user-specified paths.""" + + DATA_MODEL = SysSettingsDataModel + COLLECTOR = SysSettingsCollector + ANALYZER = SysSettingsAnalyzer + COLLECTOR_ARGS = SysSettingsCollectorArgs + ANALYZER_ARGS = SysSettingsAnalyzerArgs diff --git a/nodescraper/plugins/inband/thp/thp_analyzer.py b/nodescraper/plugins/inband/thp/thp_analyzer.py deleted file mode 100644 index a2f300c..0000000 --- a/nodescraper/plugins/inband/thp/thp_analyzer.py +++ /dev/null @@ -1,82 +0,0 @@ -############################################################################### -# -# MIT License -# -# Copyright (c) 2026 Advanced Micro Devices, Inc. -# -# Permission is hereby granted, free of charge, to any person obtaining a copy -# of this software and associated documentation files (the "Software"), to deal -# in the Software without restriction, including without limitation the rights -# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -# copies of the Software, and to permit persons to whom the Software is -# furnished to do so, subject to the following conditions: -# -# The above copyright notice and this permission notice shall be included in all -# copies or substantial portions of the Software. -# -# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -# SOFTWARE. -# -############################################################################### -from typing import Optional - -from nodescraper.enums import EventCategory, EventPriority, ExecutionStatus -from nodescraper.interfaces import DataAnalyzer -from nodescraper.models import TaskResult - -from .analyzer_args import ThpAnalyzerArgs -from .thpdata import ThpDataModel - - -class ThpAnalyzer(DataAnalyzer[ThpDataModel, ThpAnalyzerArgs]): - """Check THP settings against expected values.""" - - DATA_MODEL = ThpDataModel - - def analyze_data( - self, data: ThpDataModel, args: Optional[ThpAnalyzerArgs] = None - ) -> TaskResult: - """Compare THP data to expected settings.""" - mismatches = {} - - if not args: - args = ThpAnalyzerArgs() - - if args.exp_enabled is not None: - actual = (data.enabled or "").strip().lower() - expected = args.exp_enabled.strip().lower() - if actual != expected: - mismatches["enabled"] = {"expected": expected, "actual": data.enabled} - - if args.exp_defrag is not None: - actual = (data.defrag or "").strip().lower() - expected = args.exp_defrag.strip().lower() - if actual != expected: - mismatches["defrag"] = {"expected": expected, "actual": data.defrag} - - if mismatches: - self.result.status = ExecutionStatus.ERROR - self.result.message = "THP setting(s) do not match expected values." - self._log_event( - category=EventCategory.OS, - description="THP mismatch detected", - data=mismatches, - priority=EventPriority.ERROR, - console_log=True, - ) - else: - self._log_event( - category=EventCategory.OS, - description="THP settings match expected (or no expectations set)", - priority=EventPriority.INFO, - console_log=True, - ) - self.result.status = ExecutionStatus.OK - self.result.message = "THP settings as expected." - - return self.result diff --git a/nodescraper/plugins/inband/thp/thp_collector.py b/nodescraper/plugins/inband/thp/thp_collector.py deleted file mode 100644 index 561a81e..0000000 --- a/nodescraper/plugins/inband/thp/thp_collector.py +++ /dev/null @@ -1,112 +0,0 @@ -############################################################################### -# -# MIT License -# -# Copyright (c) 2026 Advanced Micro Devices, Inc. -# -# Permission is hereby granted, free of charge, to any person obtaining a copy -# of this software and associated documentation files (the "Software"), to deal -# in the Software without restriction, including without limitation the rights -# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -# copies of the Software, and to permit persons to whom the Software is -# furnished to do so, subject to the following conditions: -# -# The above copyright notice and this permission notice shall be included in all -# copies or substantial portions of the Software. -# -# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -# SOFTWARE. -# -############################################################################### -import re -from typing import Optional - -from nodescraper.base import InBandDataCollector -from nodescraper.enums import EventCategory, EventPriority, ExecutionStatus, OSFamily -from nodescraper.models import TaskResult - -from .thpdata import ThpDataModel - -THP_BASE = "/sys/kernel/mm/transparent_hugepage" -# Sysfs format: "[always] madvise never" -> extract bracketed value -BRACKETED_RE = re.compile(r"\[(\w+)\]") - - -def _parse_bracketed_setting(content: str) -> Optional[str]: - """Extract the active setting from sysfs content (value in square brackets).""" - if not content: - return None - match = BRACKETED_RE.search(content.strip()) - return match.group(1).strip() if match else None - - -class ThpCollector(InBandDataCollector[ThpDataModel, None]): - """Collect transparent huge pages (THP) settings from sysfs.""" - - DATA_MODEL = ThpDataModel - SUPPORTED_OS_FAMILY: set[OSFamily] = {OSFamily.LINUX} - - # Command template for doc generator: {} is each sysfs path (e.g. from checks). - CMD = "cat {}" - CMD_ENABLED = f"cat {THP_BASE}/enabled" - CMD_DEFRAG = f"cat {THP_BASE}/defrag" - - def collect_data(self, args=None) -> tuple[TaskResult, Optional[ThpDataModel]]: - """Collect THP enabled and defrag settings from the system.""" - if self.system_info.os_family != OSFamily.LINUX: - self._log_event( - category=EventCategory.OS, - description="THP collection is only supported on Linux.", - priority=EventPriority.WARNING, - console_log=True, - ) - return self.result, None - - enabled_raw = self._run_sut_cmd(self.CMD_ENABLED) - defrag_raw = self._run_sut_cmd(self.CMD_DEFRAG) - - enabled: Optional[str] = None - defrag: Optional[str] = None - - if enabled_raw.exit_code == 0 and enabled_raw.stdout: - enabled = _parse_bracketed_setting(enabled_raw.stdout) - else: - self._log_event( - category=EventCategory.OS, - description="Failed to read THP enabled setting", - data={"exit_code": enabled_raw.exit_code}, - priority=EventPriority.WARNING, - console_log=True, - ) - - if defrag_raw.exit_code == 0 and defrag_raw.stdout: - defrag = _parse_bracketed_setting(defrag_raw.stdout) - else: - self._log_event( - category=EventCategory.OS, - description="Failed to read THP defrag setting", - data={"exit_code": defrag_raw.exit_code}, - priority=EventPriority.WARNING, - console_log=True, - ) - - if enabled is None and defrag is None: - self.result.message = "THP settings not read" - self.result.status = ExecutionStatus.ERROR - return self.result, None - - thp_data = ThpDataModel(enabled=enabled, defrag=defrag) - self._log_event( - category=EventCategory.OS, - description="THP settings collected", - data=thp_data.model_dump(), - priority=EventPriority.INFO, - ) - self.result.message = f"THP enabled={enabled}, defrag={defrag}" - self.result.status = ExecutionStatus.OK - return self.result, thp_data diff --git a/test/functional/test_plugin_configs.py b/test/functional/test_plugin_configs.py index 7f4ea6c..ec83223 100644 --- a/test/functional/test_plugin_configs.py +++ b/test/functional/test_plugin_configs.py @@ -57,6 +57,7 @@ def plugin_config_files(fixtures_dir): "ProcessPlugin": fixtures_dir / "process_plugin_config.json", "RocmPlugin": fixtures_dir / "rocm_plugin_config.json", "StoragePlugin": fixtures_dir / "storage_plugin_config.json", + "SysSettingsPlugin": fixtures_dir / "sys_settings_plugin_config.json", "SysctlPlugin": fixtures_dir / "sysctl_plugin_config.json", "SyslogPlugin": fixtures_dir / "syslog_plugin_config.json", "UptimePlugin": fixtures_dir / "uptime_plugin_config.json", @@ -119,6 +120,7 @@ def test_plugin_config_with_builtin_config(run_cli_command, tmp_path): "ProcessPlugin", "RocmPlugin", "StoragePlugin", + "SysSettingsPlugin", "SysctlPlugin", "SyslogPlugin", "UptimePlugin", diff --git a/test/functional/test_sys_settings_plugin.py b/test/functional/test_sys_settings_plugin.py new file mode 100644 index 0000000..4a482aa --- /dev/null +++ b/test/functional/test_sys_settings_plugin.py @@ -0,0 +1,87 @@ +############################################################################### +# +# MIT License +# +# Copyright (c) 2026 Advanced Micro Devices, Inc. +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in all +# copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. +# +############################################################################### +"""Functional tests for SysSettingsPlugin with --plugin-configs.""" + +from pathlib import Path + +import pytest + + +@pytest.fixture +def fixtures_dir(): + """Return path to fixtures directory.""" + return Path(__file__).parent / "fixtures" + + +@pytest.fixture +def sys_settings_config_file(fixtures_dir): + """Return path to SysSettingsPlugin config file.""" + return fixtures_dir / "sys_settings_plugin_config.json" + + +def test_sys_settings_plugin_with_config_file(run_cli_command, sys_settings_config_file, tmp_path): + """Test SysSettingsPlugin using config file with collection_args and analysis_args.""" + assert sys_settings_config_file.exists(), f"Config file not found: {sys_settings_config_file}" + + log_path = str(tmp_path / "logs_sys_settings") + result = run_cli_command( + ["--log-path", log_path, "--plugin-configs", str(sys_settings_config_file)], check=False + ) + + assert result.returncode in [0, 1, 2] + output = result.stdout + result.stderr + assert len(output) > 0 + assert "SysSettingsPlugin" in output or "syssettings" in output.lower() + + +def test_sys_settings_plugin_with_run_plugins_subcommand(run_cli_command, tmp_path): + """Test SysSettingsPlugin via run-plugins subcommand (no config; collector gets no paths).""" + log_path = str(tmp_path / "logs_sys_settings_subcommand") + result = run_cli_command( + ["--log-path", log_path, "run-plugins", "SysSettingsPlugin"], check=False + ) + + assert result.returncode in [0, 1, 2] + output = result.stdout + result.stderr + assert len(output) > 0 + # Without config, plugin runs with no paths -> NOT_RAN or similar + assert "SysSettings" in output or "sys" in output.lower() + + +def test_sys_settings_plugin_output_contains_plugin_result( + run_cli_command, sys_settings_config_file, tmp_path +): + """On Linux, plugin runs and table shows SysSettingsPlugin with a status.""" + assert sys_settings_config_file.exists() + + log_path = str(tmp_path / "logs_sys_settings_result") + result = run_cli_command( + ["--log-path", log_path, "--plugin-configs", str(sys_settings_config_file)], check=False + ) + + output = result.stdout + result.stderr + # Table or status line should mention the plugin + assert "SysSettingsPlugin" in output diff --git a/test/unit/plugin/test_sys_settings_analyzer.py b/test/unit/plugin/test_sys_settings_analyzer.py new file mode 100644 index 0000000..318093c --- /dev/null +++ b/test/unit/plugin/test_sys_settings_analyzer.py @@ -0,0 +1,107 @@ +############################################################################### +# +# MIT License +# +# Copyright (c) 2026 Advanced Micro Devices, Inc. +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in all +# copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. +# +############################################################################### +import pytest + +from nodescraper.enums import ExecutionStatus +from nodescraper.plugins.inband.sys_settings.analyzer_args import ( + SysfsCheck, + SysSettingsAnalyzerArgs, +) +from nodescraper.plugins.inband.sys_settings.sys_settings_analyzer import ( + SysSettingsAnalyzer, +) +from nodescraper.plugins.inband.sys_settings.sys_settings_data import ( + SysSettingsDataModel, +) + +SYSFS_BASE = "/sys/kernel/mm/transparent_hugepage" + + +@pytest.fixture +def analyzer(system_info): + return SysSettingsAnalyzer(system_info=system_info) + + +@pytest.fixture +def sample_data(): + return SysSettingsDataModel( + readings={ + f"{SYSFS_BASE}/enabled": "always", + f"{SYSFS_BASE}/defrag": "madvise", + } + ) + + +def test_analyzer_no_checks_ok(analyzer, sample_data): + """No checks configured -> OK.""" + result = analyzer.analyze_data(sample_data) + assert result.status == ExecutionStatus.OK + assert "No checks" in result.message + + +def test_analyzer_checks_match(analyzer, sample_data): + """Checks match collected values -> OK.""" + args = SysSettingsAnalyzerArgs( + checks=[ + SysfsCheck( + path=f"{SYSFS_BASE}/enabled", expected=["always", "[always]"], name="enabled" + ), + SysfsCheck( + path=f"{SYSFS_BASE}/defrag", expected=["madvise", "[madvise]"], name="defrag" + ), + ] + ) + result = analyzer.analyze_data(sample_data, args) + assert result.status == ExecutionStatus.OK + assert "as expected" in result.message + + +def test_analyzer_check_mismatch(analyzer, sample_data): + """One check expects wrong value -> ERROR; message enumerates path and expected/actual.""" + args = SysSettingsAnalyzerArgs( + checks=[ + SysfsCheck(path=f"{SYSFS_BASE}/enabled", expected=["never"], name="enabled"), + ] + ) + result = analyzer.analyze_data(sample_data, args) + assert result.status == ExecutionStatus.ERROR + assert "mismatch" in result.message.lower() + assert "enabled" in result.message + assert "never" in result.message + assert "always" in result.message + + +def test_analyzer_unknown_path(analyzer, sample_data): + """Check for path not collected by plugin -> ERROR.""" + args = SysSettingsAnalyzerArgs( + checks=[ + SysfsCheck(path="/sys/unknown/path", expected=["x"], name="unknown"), + ] + ) + result = analyzer.analyze_data(sample_data, args) + assert result.status == ExecutionStatus.ERROR + assert "mismatch" in result.message.lower() + assert "unknown" in result.message diff --git a/test/unit/plugin/test_thp_collector.py b/test/unit/plugin/test_sys_settings_collector.py similarity index 55% rename from test/unit/plugin/test_thp_collector.py rename to test/unit/plugin/test_sys_settings_collector.py index 93b36d4..275cfde 100644 --- a/test/unit/plugin/test_thp_collector.py +++ b/test/unit/plugin/test_sys_settings_collector.py @@ -28,42 +28,61 @@ import pytest from nodescraper.enums import ExecutionStatus, OSFamily -from nodescraper.plugins.inband.thp.thp_collector import ThpCollector -from nodescraper.plugins.inband.thp.thpdata import ThpDataModel +from nodescraper.plugins.inband.sys_settings.sys_settings_collector import ( + SysSettingsCollector, +) +from nodescraper.plugins.inband.sys_settings.sys_settings_data import ( + SysSettingsDataModel, +) + +SYSFS_BASE = "/sys/kernel/mm/transparent_hugepage" +PATH_ENABLED = f"{SYSFS_BASE}/enabled" +PATH_DEFRAG = f"{SYSFS_BASE}/defrag" @pytest.fixture -def linux_thp_collector(system_info, conn_mock): +def linux_sys_settings_collector(system_info, conn_mock): system_info.os_family = OSFamily.LINUX - return ThpCollector(system_info=system_info, connection=conn_mock) + return SysSettingsCollector(system_info=system_info, connection=conn_mock) + + +@pytest.fixture +def collection_args(): + return {"paths": [PATH_ENABLED, PATH_DEFRAG]} def make_artifact(exit_code, stdout): return SimpleNamespace(command="", exit_code=exit_code, stdout=stdout, stderr="") -def test_collect_data_success(linux_thp_collector, conn_mock): +def test_collect_data_success(linux_sys_settings_collector, collection_args): """Both enabled and defrag read successfully.""" - calls = [] - def capture_cmd(cmd, **kwargs): - calls.append(cmd) + def run_cmd(cmd, **kwargs): if "enabled" in cmd: return make_artifact(0, "[always] madvise never") return make_artifact(0, "[madvise] always never defer") - linux_thp_collector._run_sut_cmd = capture_cmd - result, data = linux_thp_collector.collect_data() + linux_sys_settings_collector._run_sut_cmd = run_cmd + result, data = linux_sys_settings_collector.collect_data(collection_args) assert result.status == ExecutionStatus.OK assert data is not None - assert isinstance(data, ThpDataModel) - assert data.enabled == "always" - assert data.defrag == "madvise" - assert "THP enabled=always" in result.message + assert isinstance(data, SysSettingsDataModel) + assert data.readings.get(PATH_ENABLED) == "always" + assert data.readings.get(PATH_DEFRAG) == "madvise" + assert "Sysfs collected 2 path(s)" in result.message + + +def test_collect_data_no_paths_not_ran(linux_sys_settings_collector): + """No paths in args -> NOT_RAN.""" + result, data = linux_sys_settings_collector.collect_data({}) + assert result.status == ExecutionStatus.NOT_RAN + assert "No paths configured" in result.message + assert data is None -def test_collect_data_enabled_fails(linux_thp_collector): +def test_collect_data_enabled_fails(linux_sys_settings_collector, collection_args): """Enabled read fails; defrag succeeds -> still get partial data.""" def run_cmd(cmd, **kwargs): @@ -71,33 +90,33 @@ def run_cmd(cmd, **kwargs): return make_artifact(1, "") return make_artifact(0, "[never] always madvise") - linux_thp_collector._run_sut_cmd = run_cmd - result, data = linux_thp_collector.collect_data() + linux_sys_settings_collector._run_sut_cmd = run_cmd + result, data = linux_sys_settings_collector.collect_data(collection_args) assert result.status == ExecutionStatus.OK assert data is not None - assert data.enabled is None - assert data.defrag == "never" + assert PATH_ENABLED not in data.readings + assert data.readings.get(PATH_DEFRAG) == "never" -def test_collect_data_both_fail(linux_thp_collector): +def test_collect_data_both_fail(linux_sys_settings_collector, collection_args): """Both reads fail -> error.""" def run_cmd(cmd, **kwargs): return make_artifact(1, "") - linux_thp_collector._run_sut_cmd = run_cmd - result, data = linux_thp_collector.collect_data() + linux_sys_settings_collector._run_sut_cmd = run_cmd + result, data = linux_sys_settings_collector.collect_data(collection_args) assert result.status == ExecutionStatus.ERROR assert data is None - assert "THP settings not read" in result.message + assert "Sysfs settings not read" in result.message def test_collector_raises_on_non_linux(system_info, conn_mock): - """ThpCollector does not support non-Linux; constructor raises.""" + """SysSettingsCollector does not support non-Linux; constructor raises.""" from nodescraper.interfaces.task import SystemCompatibilityError system_info.os_family = OSFamily.WINDOWS with pytest.raises(SystemCompatibilityError, match="not supported"): - ThpCollector(system_info=system_info, connection=conn_mock) + SysSettingsCollector(system_info=system_info, connection=conn_mock) diff --git a/test/unit/plugin/test_thp_analyzer.py b/test/unit/plugin/test_thp_analyzer.py deleted file mode 100644 index fe6ccf9..0000000 --- a/test/unit/plugin/test_thp_analyzer.py +++ /dev/null @@ -1,76 +0,0 @@ -############################################################################### -# -# MIT License -# -# Copyright (c) 2026 Advanced Micro Devices, Inc. -# -# Permission is hereby granted, free of charge, to any person obtaining a copy -# of this software and associated documentation files (the "Software"), to deal -# in the Software without restriction, including without limitation the rights -# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -# copies of the Software, and to permit persons to whom the Software is -# furnished to do so, subject to the following conditions: -# -# The above copyright notice and this permission notice shall be included in all -# copies or substantial portions of the Software. -# -# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -# SOFTWARE. -# -############################################################################### -import pytest - -from nodescraper.enums import ExecutionStatus -from nodescraper.plugins.inband.thp.analyzer_args import ThpAnalyzerArgs -from nodescraper.plugins.inband.thp.thp_analyzer import ThpAnalyzer -from nodescraper.plugins.inband.thp.thpdata import ThpDataModel - - -@pytest.fixture -def analyzer(system_info): - return ThpAnalyzer(system_info=system_info) - - -@pytest.fixture -def sample_data(): - return ThpDataModel(enabled="always", defrag="madvise") - - -def test_analyzer_no_args_match(analyzer, sample_data): - """No expected values -> OK.""" - result = analyzer.analyze_data(sample_data) - assert result.status == ExecutionStatus.OK - - -def test_analyzer_match(analyzer, sample_data): - """Expected values match -> OK.""" - args = ThpAnalyzerArgs(exp_enabled="always", exp_defrag="madvise") - result = analyzer.analyze_data(sample_data, args) - assert result.status == ExecutionStatus.OK - - -def test_analyzer_enabled_mismatch(analyzer, sample_data): - """Expected enabled differs -> ERROR.""" - args = ThpAnalyzerArgs(exp_enabled="never") - result = analyzer.analyze_data(sample_data, args) - assert result.status == ExecutionStatus.ERROR - assert "do not match" in result.message or "mismatch" in result.message.lower() - - -def test_analyzer_defrag_mismatch(analyzer, sample_data): - """Expected defrag differs -> ERROR.""" - args = ThpAnalyzerArgs(exp_defrag="never") - result = analyzer.analyze_data(sample_data, args) - assert result.status == ExecutionStatus.ERROR - - -def test_build_from_model(sample_data): - """build_from_model populates analyzer args from data model.""" - args = ThpAnalyzerArgs.build_from_model(sample_data) - assert args.exp_enabled == "always" - assert args.exp_defrag == "madvise" From 5b53a9de2a528b594b3038c14170ed509813111f Mon Sep 17 00:00:00 2001 From: Alexandra Bara Date: Tue, 24 Feb 2026 16:51:24 -0600 Subject: [PATCH 5/8] added fixture --- .../fixtures/sys_settings_plugin_config.json | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 test/functional/fixtures/sys_settings_plugin_config.json diff --git a/test/functional/fixtures/sys_settings_plugin_config.json b/test/functional/fixtures/sys_settings_plugin_config.json new file mode 100644 index 0000000..2a2013c --- /dev/null +++ b/test/functional/fixtures/sys_settings_plugin_config.json @@ -0,0 +1,36 @@ +{ + "name": "SysSettingsPlugin config", + "desc": "Config for testing SysSettingsPlugin (sysfs settings)", + "global_args": {}, + "plugins": { + "SysSettingsPlugin": { + "collection_args": { + "paths": [ + "/sys/kernel/mm/transparent_hugepage/enabled", + "/sys/kernel/mm/transparent_hugepage/defrag", + "/sys/kernel/mm/transparent_hugepage/shmem_enabled" + ] + }, + "analysis_args": { + "checks": [ + { + "path": "/sys/kernel/mm/transparent_hugepage/enabled", + "expected": ["always", "madvise", "never"], + "name": "thp_enabled" + }, + { + "path": "/sys/kernel/mm/transparent_hugepage/defrag", + "expected": ["always", "madvise", "never", "defer"], + "name": "thp_defrag" + }, + { + "path": "/sys/kernel/mm/transparent_hugepage/shmem_enabled", + "expected": [], + "name": "thp_shmem" + } + ] + } + } + }, + "result_collators": {} +} From a843e217d04a5de5eaf20f2e307f0a79d730db56 Mon Sep 17 00:00:00 2001 From: Alexandra Bara Date: Thu, 26 Feb 2026 12:45:32 -0600 Subject: [PATCH 6/8] added /sys{} path restrictions --- .../sys_settings/sys_settings_collector.py | 41 +++++++++++++++--- .../plugin/test_sys_settings_collector.py | 43 +++++++++++++++++++ 2 files changed, 78 insertions(+), 6 deletions(-) diff --git a/nodescraper/plugins/inband/sys_settings/sys_settings_collector.py b/nodescraper/plugins/inband/sys_settings/sys_settings_collector.py index da08934..235202b 100644 --- a/nodescraper/plugins/inband/sys_settings/sys_settings_collector.py +++ b/nodescraper/plugins/inband/sys_settings/sys_settings_collector.py @@ -56,14 +56,32 @@ def _paths_from_args(args: Optional[SysSettingsCollectorArgs]) -> list[str]: """Extract list of sysfs paths from collection args. Args: - args: Collector args containing paths to read, or None. + args: Collector args containing paths to read, or None. May be a dict. Returns: List of sysfs paths; empty if args is None or args.paths is empty. """ if args is None: return [] - return list(args.paths) if args.paths else [] + paths = args.get("paths") if isinstance(args, dict) else getattr(args, "paths", None) + return list(paths) if paths else [] + + +def _path_under_sys(path: str) -> Optional[str]: + """Normalize path to the suffix under /sys/ for use in 'cat /sys/{}'.""" + if ".." in path: + return None + p = path.strip().lstrip("/") + if p.startswith("sys/"): + p = p[4:] + if p.startswith("/"): + return None + return p if p else None + + +def _sysfs_full_path(suffix: str) -> str: + """Return full path /sys/{suffix}.""" + return f"/sys/{suffix}" class SysSettingsCollector(InBandDataCollector[SysSettingsDataModel, SysSettingsCollectorArgs]): @@ -72,7 +90,7 @@ class SysSettingsCollector(InBandDataCollector[SysSettingsDataModel, SysSettings DATA_MODEL = SysSettingsDataModel SUPPORTED_OS_FAMILY: set[OSFamily] = {OSFamily.LINUX} - CMD = "cat {}" + CMD = "cat /sys/{}" def collect_data( self, args: Optional[SysSettingsCollectorArgs] = None @@ -102,14 +120,25 @@ def collect_data( readings: dict[str, str] = {} for path in paths: - res = self._run_sut_cmd(self.CMD.format(path), sudo=False) + suffix = _path_under_sys(path) + if suffix is None: + self._log_event( + category=EventCategory.OS, + description=f"Skipping path not under /sys or invalid: {path!r}", + data={"path": path}, + priority=EventPriority.WARNING, + console_log=True, + ) + continue + full_path = _sysfs_full_path(suffix) + res = self._run_sut_cmd(self.CMD.format(suffix), sudo=False) if res.exit_code == 0 and res.stdout: value = _parse_bracketed_setting(res.stdout) or res.stdout.strip() - readings[path] = value + readings[full_path] = value else: self._log_event( category=EventCategory.OS, - description=f"Failed to read sysfs path: {path}", + description=f"Failed to read sysfs path: {full_path}", data={"exit_code": res.exit_code}, priority=EventPriority.WARNING, console_log=True, diff --git a/test/unit/plugin/test_sys_settings_collector.py b/test/unit/plugin/test_sys_settings_collector.py index 275cfde..88097aa 100644 --- a/test/unit/plugin/test_sys_settings_collector.py +++ b/test/unit/plugin/test_sys_settings_collector.py @@ -120,3 +120,46 @@ def test_collector_raises_on_non_linux(system_info, conn_mock): system_info.os_family = OSFamily.WINDOWS with pytest.raises(SystemCompatibilityError, match="not supported"): SysSettingsCollector(system_info=system_info, connection=conn_mock) + + +def test_collect_data_uses_sys_only_command(linux_sys_settings_collector): + seen_commands = [] + + def run_cmd(cmd, **kwargs): + seen_commands.append(cmd) + return make_artifact(0, "value") + + linux_sys_settings_collector._run_sut_cmd = run_cmd + args = {"paths": [PATH_ENABLED]} + result, data = linux_sys_settings_collector.collect_data(args) + + assert result.status == ExecutionStatus.OK + assert len(seen_commands) == 1 + assert seen_commands[0].startswith("cat /sys/") + assert data.readings.get(PATH_ENABLED) == "value" + + +def test_collect_data_skips_path_with_dotdot(linux_sys_settings_collector): + seen_commands = [] + + def run_cmd(cmd, **kwargs): + seen_commands.append(cmd) + return make_artifact(0, "safe") + + linux_sys_settings_collector._run_sut_cmd = run_cmd + args = { + "paths": [ + "/sys/kernel/mm/transparent_hugepage/enabled", + "/sys/../etc/passwd", + "/sys/something/../../etc", + "sys/kernel/mm/transparent_hugepage/defrag", + ] + } + result, data = linux_sys_settings_collector.collect_data(args) + + assert result.status == ExecutionStatus.OK + assert len(seen_commands) == 2 + assert all(c.startswith("cat /sys/") for c in seen_commands) + assert data.readings.get("/sys/kernel/mm/transparent_hugepage/enabled") == "safe" + assert data.readings.get("/sys/kernel/mm/transparent_hugepage/defrag") == "safe" + assert "/etc" not in str(seen_commands) From aeacaf3818bff8bc87f67274a0eb6245f4422c91 Mon Sep 17 00:00:00 2001 From: Alexandra Bara Date: Thu, 26 Feb 2026 18:19:00 -0600 Subject: [PATCH 7/8] adding ls capability with regex pattern expected values --- nodescraper/interfaces/dataplugin.py | 19 +++++- .../inband/sys_settings/analyzer_args.py | 31 +++++++--- .../inband/sys_settings/collector_args.py | 7 ++- .../sys_settings/sys_settings_analyzer.py | 44 +++++++++++-- .../sys_settings/sys_settings_collector.py | 61 ++++++++++++++++--- test/functional/test_sys_settings_plugin.py | 28 +++++++++ 6 files changed, 166 insertions(+), 24 deletions(-) diff --git a/nodescraper/interfaces/dataplugin.py b/nodescraper/interfaces/dataplugin.py index da7a132..f4aa622 100644 --- a/nodescraper/interfaces/dataplugin.py +++ b/nodescraper/interfaces/dataplugin.py @@ -142,7 +142,7 @@ def collect( Union[SystemInteractionLevel, str] ] = SystemInteractionLevel.INTERACTIVE, preserve_connection: bool = False, - collection_args: Optional[Union[TCollectArg, dict]] = None, + collection_args: Optional[TCollectArg] = None, ) -> TaskResult: """Run data collector task @@ -150,7 +150,7 @@ def collect( max_event_priority_level (Union[EventPriority, str], optional): priority limit for events. Defaults to EventPriority.CRITICAL. system_interaction_level (Union[SystemInteractionLevel, str], optional): system interaction level. Defaults to SystemInteractionLevel.INTERACTIVE. preserve_connection (bool, optional): whether we should close the connection after data collection. Defaults to False. - collection_args (Optional[Union[TCollectArg , dict]], optional): args for data collection. Defaults to None. + collection_args (Optional[TCollectArg], optional): args for data collection (validated model). Defaults to None. Returns: TaskResult: task result for data collection @@ -195,6 +195,13 @@ def collect( message="Connection not available, data collection skipped", ) else: + if ( + collection_args is not None + and isinstance(collection_args, dict) + and hasattr(self, "COLLECTOR_ARGS") + and self.COLLECTOR_ARGS is not None + ): + collection_args = self.COLLECTOR_ARGS.model_validate(collection_args) collection_task = self.COLLECTOR( system_info=self.system_info, @@ -264,6 +271,14 @@ def analyze( ) return self.analysis_result + if ( + analysis_args is not None + and isinstance(analysis_args, dict) + and hasattr(self, "ANALYZER_ARGS") + and self.ANALYZER_ARGS is not None + ): + analysis_args = self.ANALYZER_ARGS.model_validate(analysis_args) + analyzer_task = self.ANALYZER( self.system_info, logger=self.logger, diff --git a/nodescraper/plugins/inband/sys_settings/analyzer_args.py b/nodescraper/plugins/inband/sys_settings/analyzer_args.py index e3d4e06..732398f 100644 --- a/nodescraper/plugins/inband/sys_settings/analyzer_args.py +++ b/nodescraper/plugins/inband/sys_settings/analyzer_args.py @@ -25,20 +25,22 @@ ############################################################################### from typing import Optional -from pydantic import BaseModel +from pydantic import BaseModel, Field from nodescraper.models import AnalyzerArgs class SysfsCheck(BaseModel): - """One sysfs check: path to read, acceptable values, and display name. + """One sysfs check: path to read, acceptable values or pattern, and display name. - If expected is an empty list, the check is treated as passing (no constraint). + For file paths: use expected (list of acceptable values); if empty, check passes. + For directory paths: use pattern (regex); at least one directory entry must match (e.g. ^hsn[0-9]+). """ path: str - expected: list[str] + expected: list[str] = Field(default_factory=list) name: str + pattern: Optional[str] = None class SysSettingsAnalyzerArgs(AnalyzerArgs): @@ -51,16 +53,29 @@ class SysSettingsAnalyzerArgs(AnalyzerArgs): checks: Optional[list[SysfsCheck]] = None def paths_to_collect(self) -> list[str]: - """Return the unique sysfs paths from checks, for use by the collector. + """Return unique sysfs file paths from checks (those without pattern), for use by the collector.""" + if not self.checks: + return [] + seen = set() + out = [] + for c in self.checks: + if c.pattern: + continue + p = c.path.rstrip("/") + if p not in seen: + seen.add(p) + out.append(c.path) + return out - Returns: - List of unique path strings from self.checks, preserving order of first occurrence. - """ + def paths_to_list(self) -> list[str]: + """Return unique sysfs directory paths from checks (those with pattern), for listing (ls).""" if not self.checks: return [] seen = set() out = [] for c in self.checks: + if not c.pattern: + continue p = c.path.rstrip("/") if p not in seen: seen.add(p) diff --git a/nodescraper/plugins/inband/sys_settings/collector_args.py b/nodescraper/plugins/inband/sys_settings/collector_args.py index 4f321d3..b1d2860 100644 --- a/nodescraper/plugins/inband/sys_settings/collector_args.py +++ b/nodescraper/plugins/inband/sys_settings/collector_args.py @@ -27,6 +27,11 @@ class SysSettingsCollectorArgs(BaseModel): - """Collection args for SysSettingsCollector: list of sysfs paths to read.""" + """Collection args for SysSettingsCollector. + + paths: sysfs paths to read (cat). + directory_paths: sysfs paths to list (ls -1); use for checks that match entry names by regex. + """ paths: list[str] = [] + directory_paths: list[str] = [] diff --git a/nodescraper/plugins/inband/sys_settings/sys_settings_analyzer.py b/nodescraper/plugins/inband/sys_settings/sys_settings_analyzer.py index 06c3811..276b73c 100644 --- a/nodescraper/plugins/inband/sys_settings/sys_settings_analyzer.py +++ b/nodescraper/plugins/inband/sys_settings/sys_settings_analyzer.py @@ -23,7 +23,8 @@ # SOFTWARE. # ############################################################################### -from typing import Optional, cast +import re +from typing import List, Optional, Union, cast from nodescraper.enums import EventCategory, EventPriority, ExecutionStatus from nodescraper.interfaces import DataAnalyzer @@ -64,7 +65,7 @@ def analyze_data( Returns: TaskResult with status OK if all checks pass, ERROR if any mismatch or missing path. """ - mismatches = {} + mismatches: dict[str, dict[str, Union[Optional[str], List[str]]]] = {} if not args or not args.checks: self.result.status = ExecutionStatus.OK @@ -72,6 +73,36 @@ def analyze_data( return self.result for check in args.checks: + raw_reading = data.readings.get(check.path) or data.readings.get(check.path.rstrip("/")) + + if check.pattern: + # Directory-listing check: at least one line must match the regex (e.g. ^hsn[0-9]+) + if raw_reading is None: + mismatches[check.name] = { + "path": check.path, + "pattern": check.pattern, + "actual": None, + "reason": "path not collected by this plugin", + } + continue + try: + pat = re.compile(check.pattern) + except re.error: + mismatches[check.name] = { + "path": check.path, + "pattern": check.pattern, + "reason": "invalid regex", + } + continue + lines = [ln.strip() for ln in raw_reading.splitlines() if ln.strip()] + if not any(pat.search(ln) for ln in lines): + mismatches[check.name] = { + "path": check.path, + "pattern": check.pattern, + "actual": lines, + } + continue + actual = _get_actual_for_path(data, check.path) if actual is None: mismatches[check.name] = { @@ -98,12 +129,15 @@ def analyze_data( parts = [] for name, info in mismatches.items(): path = info.get("path", "") - expected = info.get("expected") - actual = cast(Optional[str], info.get("actual")) reason = info.get("reason") + pattern = info.get("pattern") if reason: - part = f"{name} ({path})" + part = f"{name} ({path}): {reason}" + elif pattern is not None: + part = f"{name} ({path}): no entry matching pattern {pattern!r}" else: + expected = info.get("expected") + actual = cast(Optional[str], info.get("actual")) part = f"{name} ({path}): expected one of {expected}, actual {repr(actual)}" parts.append(part) self.result.message = "Sysfs mismatch: " + "; ".join(parts) diff --git a/nodescraper/plugins/inband/sys_settings/sys_settings_collector.py b/nodescraper/plugins/inband/sys_settings/sys_settings_collector.py index 235202b..753abbd 100644 --- a/nodescraper/plugins/inband/sys_settings/sys_settings_collector.py +++ b/nodescraper/plugins/inband/sys_settings/sys_settings_collector.py @@ -38,7 +38,7 @@ def _parse_bracketed_setting(content: str) -> Optional[str]: - """Extract the active setting from sysfs content (value in square brackets). + """Extract the active setting from sysfs content. Args: content: Raw sysfs file content (e.g. "[always] madvise never"). @@ -56,19 +56,30 @@ def _paths_from_args(args: Optional[SysSettingsCollectorArgs]) -> list[str]: """Extract list of sysfs paths from collection args. Args: - args: Collector args containing paths to read, or None. May be a dict. + args: Collector args containing paths to read, or None. Returns: List of sysfs paths; empty if args is None or args.paths is empty. """ if args is None: return [] - paths = args.get("paths") if isinstance(args, dict) else getattr(args, "paths", None) - return list(paths) if paths else [] + return list(args.paths) if args.paths else [] + + +def _directory_paths_from_args(args: Optional[SysSettingsCollectorArgs]) -> list[str]: + """Extract list of sysfs directory paths to list from collection args.""" + if args is None: + return [] + return list(args.directory_paths) if args.directory_paths else [] def _path_under_sys(path: str) -> Optional[str]: - """Normalize path to the suffix under /sys/ for use in 'cat /sys/{}'.""" + """Normalize path to the suffix under /sys/ for use in 'cat /sys/{}'. + + Accepts paths like '/sys/kernel/...' or 'kernel/...'. Returns the relative + part (e.g. 'kernel/mm/transparent_hugepage/enabled'). Returns None if path + contains '..' (e.g. /sys/../etc/passwd, /sys/something/../../etc) or is not under /sys. + """ if ".." in path: return None p = path.strip().lstrip("/") @@ -80,17 +91,18 @@ def _path_under_sys(path: str) -> Optional[str]: def _sysfs_full_path(suffix: str) -> str: - """Return full path /sys/{suffix}.""" + """Return full path /sys/{suffix} for use as readings key.""" return f"/sys/{suffix}" class SysSettingsCollector(InBandDataCollector[SysSettingsDataModel, SysSettingsCollectorArgs]): - """Collect sysfs settings from user-specified paths (paths come from config/args).""" + """Collect sysfs settings from user-specified paths.""" DATA_MODEL = SysSettingsDataModel SUPPORTED_OS_FAMILY: set[OSFamily] = {OSFamily.LINUX} CMD = "cat /sys/{}" + CMD_LS = "ls -1 /sys/{}" def collect_data( self, args: Optional[SysSettingsCollectorArgs] = None @@ -113,7 +125,16 @@ def collect_data( return self.result, None paths = _paths_from_args(args) - if not paths: + directory_paths = _directory_paths_from_args(args) + if directory_paths: + self._log_event( + category=EventCategory.OS, + description=f"Sysfs directory_paths to list: {directory_paths}", + data={"directory_paths": directory_paths}, + priority=EventPriority.INFO, + console_log=True, + ) + if not paths and not directory_paths: self.result.message = "No paths configured for sysfs collection" self.result.status = ExecutionStatus.NOT_RAN return self.result, None @@ -144,6 +165,30 @@ def collect_data( console_log=True, ) + for path in directory_paths: + suffix = _path_under_sys(path) + if suffix is None: + self._log_event( + category=EventCategory.OS, + description=f"Skipping directory path not under /sys or invalid: {path!r}", + data={"path": path}, + priority=EventPriority.WARNING, + console_log=True, + ) + continue + full_path = _sysfs_full_path(suffix) + res = self._run_sut_cmd(self.CMD_LS.format(suffix), sudo=False) + if res.exit_code == 0: + readings[full_path] = res.stdout.strip() if res.stdout else "" + else: + self._log_event( + category=EventCategory.OS, + description=f"Failed to list sysfs path: {full_path}", + data={"exit_code": res.exit_code}, + priority=EventPriority.WARNING, + console_log=True, + ) + if not readings: self.result.message = "Sysfs settings not read" self.result.status = ExecutionStatus.ERROR diff --git a/test/functional/test_sys_settings_plugin.py b/test/functional/test_sys_settings_plugin.py index 4a482aa..70b8449 100644 --- a/test/functional/test_sys_settings_plugin.py +++ b/test/functional/test_sys_settings_plugin.py @@ -30,6 +30,11 @@ import pytest +def _project_root() -> Path: + """Return project root (directory containing plugin_config.json).""" + return Path(__file__).resolve().parent.parent.parent + + @pytest.fixture def fixtures_dir(): """Return path to fixtures directory.""" @@ -42,6 +47,12 @@ def sys_settings_config_file(fixtures_dir): return fixtures_dir / "sys_settings_plugin_config.json" +@pytest.fixture +def plugin_config_json(): + """Return path to project root plugin_config.json (full config: paths + directory_paths + checks).""" + return _project_root() / "plugin_config.json" + + def test_sys_settings_plugin_with_config_file(run_cli_command, sys_settings_config_file, tmp_path): """Test SysSettingsPlugin using config file with collection_args and analysis_args.""" assert sys_settings_config_file.exists(), f"Config file not found: {sys_settings_config_file}" @@ -85,3 +96,20 @@ def test_sys_settings_plugin_output_contains_plugin_result( output = result.stdout + result.stderr # Table or status line should mention the plugin assert "SysSettingsPlugin" in output + + +def test_sys_settings_plugin_with_plugin_config_json(run_cli_command, plugin_config_json, tmp_path): + """Functional test: run node-scraper with project plugin_config.json (paths + directory_paths + checks).""" + assert plugin_config_json.exists(), f"Config not found: {plugin_config_json}" + + log_path = str(tmp_path / "logs_plugin_config") + result = run_cli_command( + ["--log-path", log_path, "--plugin-configs", str(plugin_config_json)], check=False + ) + + assert result.returncode in [0, 1, 2] + output = result.stdout + result.stderr + assert len(output) > 0 + assert "SysSettingsPlugin" in output + # Config exercises paths + directory_paths (/sys/class/net) + pattern check; collector/analyzer run + assert "Sysfs" in output or "sys" in output.lower() From fe8f1d65b523a763680d78d58cbbb820db8c9a00 Mon Sep 17 00:00:00 2001 From: Alexandra Bara Date: Fri, 27 Feb 2026 09:59:44 -0600 Subject: [PATCH 8/8] utest cleanup --- test/functional/fixtures/plugin_config.json | 1 + test/functional/test_sys_settings_plugin.py | 11 +++-------- 2 files changed, 4 insertions(+), 8 deletions(-) create mode 100644 test/functional/fixtures/plugin_config.json diff --git a/test/functional/fixtures/plugin_config.json b/test/functional/fixtures/plugin_config.json new file mode 100644 index 0000000..000b7b2 --- /dev/null +++ b/test/functional/fixtures/plugin_config.json @@ -0,0 +1 @@ +{"name":"plugin_config","desc":"Full config: all collector_args + analyzer_args, values that pass on Ubuntu","global_args":{},"plugins":{"PackagePlugin":{"collection_args":{},"analysis_args":{"exp_package_ver":{"libc|python|apt":null},"regex_match":true,"rocm_regex":null,"enable_rocm_regex":false}},"SysSettingsPlugin":{"collection_args":{"paths":["/sys/kernel/mm/transparent_hugepage/enabled","/sys/kernel/mm/transparent_hugepage/defrag"],"directory_paths":["/sys/class/net"]},"analysis_args":{"checks":[{"path":"/sys/kernel/mm/transparent_hugepage/enabled","expected":["always","madvise","never"],"name":"thp_enabled","pattern":null},{"path":"/sys/kernel/mm/transparent_hugepage/defrag","expected":["always","madvise","never","defer"],"name":"thp_defrag","pattern":null},{"path":"/sys/class/net","expected":[],"name":"net_interfaces","pattern":"^(lo|eth|enp|wl|br-|docker|ens)"}]}}},"result_collators":{}} diff --git a/test/functional/test_sys_settings_plugin.py b/test/functional/test_sys_settings_plugin.py index 70b8449..85b1bc0 100644 --- a/test/functional/test_sys_settings_plugin.py +++ b/test/functional/test_sys_settings_plugin.py @@ -30,11 +30,6 @@ import pytest -def _project_root() -> Path: - """Return project root (directory containing plugin_config.json).""" - return Path(__file__).resolve().parent.parent.parent - - @pytest.fixture def fixtures_dir(): """Return path to fixtures directory.""" @@ -48,9 +43,9 @@ def sys_settings_config_file(fixtures_dir): @pytest.fixture -def plugin_config_json(): - """Return path to project root plugin_config.json (full config: paths + directory_paths + checks).""" - return _project_root() / "plugin_config.json" +def plugin_config_json(fixtures_dir): + """Return path to fixture plugin_config.json.""" + return fixtures_dir / "plugin_config.json" def test_sys_settings_plugin_with_config_file(run_cli_command, sys_settings_config_file, tmp_path):