From 2b8ca45fb4026829041383aa1047fd0f3185e441 Mon Sep 17 00:00:00 2001 From: Alexandra Bara Date: Tue, 9 Jun 2026 10:39:38 -0500 Subject: [PATCH 01/13] generic collection plugin --- .../inband/generic_collection/__init__.py | 25 +++ .../generic_collection/collector_args.py | 83 +++++++++ .../generic_collection_collector.py | 107 ++++++++++++ .../generic_collection_data.py | 46 +++++ .../generic_collection_plugin.py | 42 +++++ .../generic_collection_plugin_config.json | 26 +++ .../test_generic_collection_plugin.py | 118 +++++++++++++ test/functional/test_plugin_configs.py | 2 + .../test_generic_collection_collector.py | 157 ++++++++++++++++++ 9 files changed, 606 insertions(+) create mode 100644 nodescraper/plugins/inband/generic_collection/__init__.py create mode 100644 nodescraper/plugins/inband/generic_collection/collector_args.py create mode 100644 nodescraper/plugins/inband/generic_collection/generic_collection_collector.py create mode 100644 nodescraper/plugins/inband/generic_collection/generic_collection_data.py create mode 100644 nodescraper/plugins/inband/generic_collection/generic_collection_plugin.py create mode 100644 test/functional/fixtures/generic_collection_plugin_config.json create mode 100644 test/functional/test_generic_collection_plugin.py create mode 100644 test/unit/plugin/test_generic_collection_collector.py diff --git a/nodescraper/plugins/inband/generic_collection/__init__.py b/nodescraper/plugins/inband/generic_collection/__init__.py new file mode 100644 index 00000000..eced675e --- /dev/null +++ b/nodescraper/plugins/inband/generic_collection/__init__.py @@ -0,0 +1,25 @@ +############################################################################### +# +# 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. +# +############################################################################### diff --git a/nodescraper/plugins/inband/generic_collection/collector_args.py b/nodescraper/plugins/inband/generic_collection/collector_args.py new file mode 100644 index 00000000..be7f3c61 --- /dev/null +++ b/nodescraper/plugins/inband/generic_collection/collector_args.py @@ -0,0 +1,83 @@ +############################################################################### +# +# 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, Union + +from pydantic import BaseModel, Field, field_validator + +from nodescraper.models import CollectorArgs + + +class CommandSpec(BaseModel): + """One shell command and optional per-command overrides.""" + + model_config = {"extra": "forbid"} + + command: str = Field(description="Shell command to run on the target system.") + sudo: Optional[bool] = Field( + default=None, + description="Run with sudo. When omitted, uses collection_args.sudo.", + ) + timeout: Optional[int] = Field( + default=None, + ge=1, + description="Command timeout in seconds. When omitted, uses collection_args.timeout.", + ) + + +class GenericCollectionCollectorArgs(CollectorArgs): + commands: list[CommandSpec] = Field( + default_factory=list, + description=( + "Commands to run. Each entry may be a plain string or an object with " + "'command' and optional 'sudo' / 'timeout' overrides." + ), + ) + sudo: bool = Field( + default=False, + description="Default sudo setting for commands that do not specify sudo.", + ) + timeout: int = Field( + default=300, + ge=1, + description="Default per-command timeout in seconds.", + ) + + @field_validator("commands", mode="before") + @classmethod + def _normalize_commands( + cls, value: Optional[list[Union[str, dict, CommandSpec]]] + ) -> list[Union[str, dict, CommandSpec]]: + if not value: + return [] + normalized: list[Union[str, dict, CommandSpec]] = [] + for item in value: + if isinstance(item, str): + command = item.strip() + if command: + normalized.append({"command": command}) + else: + normalized.append(item) + return normalized diff --git a/nodescraper/plugins/inband/generic_collection/generic_collection_collector.py b/nodescraper/plugins/inband/generic_collection/generic_collection_collector.py new file mode 100644 index 00000000..618a64d8 --- /dev/null +++ b/nodescraper/plugins/inband/generic_collection/generic_collection_collector.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. +# +############################################################################### +from typing import Optional + +from nodescraper.base import InBandDataCollector +from nodescraper.enums import EventCategory, EventPriority, ExecutionStatus +from nodescraper.models import TaskResult + +from .collector_args import GenericCollectionCollectorArgs +from .generic_collection_data import CommandCollectionResult, GenericCollectionDataModel + + +class GenericCollectionCollector( + InBandDataCollector[GenericCollectionDataModel, GenericCollectionCollectorArgs] +): + """Run user-configured shell commands and report per-command success.""" + + DATA_MODEL = GenericCollectionDataModel + + def collect_data( + self, args: Optional[GenericCollectionCollectorArgs] = None + ) -> tuple[TaskResult, Optional[GenericCollectionDataModel]]: + if args is None: + args = GenericCollectionCollectorArgs() + + if not args.commands: + self.result.message = "No commands configured" + self.result.status = ExecutionStatus.NOT_RAN + return self.result, None + + results: list[CommandCollectionResult] = [] + for cmd_spec in args.commands: + command = cmd_spec.command.strip() + if not command: + continue + + sudo = cmd_spec.sudo if cmd_spec.sudo is not None else args.sudo + timeout = cmd_spec.timeout if cmd_spec.timeout is not None else args.timeout + res = self._run_sut_cmd( + command, + sudo=sudo, + timeout=timeout, + ) + success = res.exit_code == 0 + cmd_result = CommandCollectionResult( + command=command, + success=success, + exit_code=res.exit_code, + sudo=sudo, + stderr=res.stderr or None, + ) + results.append(cmd_result) + + if success: + self._log_event( + category=EventCategory.RUNTIME, + description=f"Command succeeded: {command!r}", + data={"command": command, "exit_code": res.exit_code, "sudo": sudo}, + priority=EventPriority.INFO, + ) + else: + self._log_event( + category=EventCategory.RUNTIME, + description=f"Command failed: {command!r}", + data={ + "command": command, + "exit_code": res.exit_code, + "sudo": sudo, + "stderr": res.stderr, + }, + priority=EventPriority.ERROR, + console_log=True, + ) + + if not results: + self.result.message = "No commands configured" + self.result.status = ExecutionStatus.NOT_RAN + return self.result, None + + success_count = sum(1 for result in results if result.success) + total = len(results) + self.result.message = f"Generic collection: {success_count}/{total} commands succeeded" + self.result.status = ExecutionStatus.OK if success_count == total else ExecutionStatus.ERROR + return self.result, GenericCollectionDataModel(results=results) diff --git a/nodescraper/plugins/inband/generic_collection/generic_collection_data.py b/nodescraper/plugins/inband/generic_collection/generic_collection_data.py new file mode 100644 index 00000000..c14a20ad --- /dev/null +++ b/nodescraper/plugins/inband/generic_collection/generic_collection_data.py @@ -0,0 +1,46 @@ +############################################################################### +# +# 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 Field + +from nodescraper.models import DataModel + + +class CommandCollectionResult(DataModel): + """Outcome of running one configured shell command.""" + + command: str + success: bool + exit_code: int + sudo: bool = False + stderr: Optional[str] = None + + +class GenericCollectionDataModel(DataModel): + """Results for each command configured in collection_args.""" + + results: list[CommandCollectionResult] = Field(default_factory=list) diff --git a/nodescraper/plugins/inband/generic_collection/generic_collection_plugin.py b/nodescraper/plugins/inband/generic_collection/generic_collection_plugin.py new file mode 100644 index 00000000..b299e474 --- /dev/null +++ b/nodescraper/plugins/inband/generic_collection/generic_collection_plugin.py @@ -0,0 +1,42 @@ +############################################################################### +# +# 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 .collector_args import GenericCollectionCollectorArgs +from .generic_collection_collector import GenericCollectionCollector +from .generic_collection_data import GenericCollectionDataModel + + +class GenericCollectionPlugin( + InBandDataPlugin[GenericCollectionDataModel, GenericCollectionCollectorArgs, None] +): + """Run arbitrary shell commands configured via collection_args.""" + + DATA_MODEL = GenericCollectionDataModel + + COLLECTOR = GenericCollectionCollector + + COLLECTOR_ARGS = GenericCollectionCollectorArgs diff --git a/test/functional/fixtures/generic_collection_plugin_config.json b/test/functional/fixtures/generic_collection_plugin_config.json new file mode 100644 index 00000000..adbf46b6 --- /dev/null +++ b/test/functional/fixtures/generic_collection_plugin_config.json @@ -0,0 +1,26 @@ +{ + "global_args": {}, + "plugins": { + "GenericCollectionPlugin": { + "collection_args": { + "commands": [ + "uname -s", + "uname -m", + "hostname", + { + "command": "echo generic-collection-ok", + "timeout": 60 + }, + { + "command": "id -u", + "sudo": false + }, + "test -d /proc && echo proc-ok" + ] + } + } + }, + "result_collators": {}, + "name": "GenericCollectionPlugin config", + "desc": "Config for testing GenericCollectionPlugin with plain and per-command options" +} diff --git a/test/functional/test_generic_collection_plugin.py b/test/functional/test_generic_collection_plugin.py new file mode 100644 index 00000000..74be8d07 --- /dev/null +++ b/test/functional/test_generic_collection_plugin.py @@ -0,0 +1,118 @@ +############################################################################### +# +# 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 GenericCollectionPlugin with --plugin-configs.""" + +import csv +from pathlib import Path + +import pytest + + +@pytest.fixture +def fixtures_dir(): + """Return path to fixtures directory.""" + return Path(__file__).parent / "fixtures" + + +@pytest.fixture +def generic_collection_config_file(fixtures_dir): + """Return path to GenericCollectionPlugin config file.""" + return fixtures_dir / "generic_collection_plugin_config.json" + + +def test_generic_collection_plugin_with_config_file( + run_cli_command, generic_collection_config_file, tmp_path +): + """Run GenericCollectionPlugin using collection_args.commands from config file.""" + assert ( + generic_collection_config_file.exists() + ), f"Config file not found: {generic_collection_config_file}" + + log_path = str(tmp_path / "logs_generic_collection") + result = run_cli_command( + [ + "--log-path", + log_path, + f"--plugin-configs={generic_collection_config_file}", + "run-plugins", + "GenericCollectionPlugin", + ], + check=False, + ) + + output = result.stdout + result.stderr + assert result.returncode in [0, 1, 2] + assert "GenericCollectionPlugin" in output + assert "Generic collection" in output or "genericcollection" in output.lower() + + +def test_generic_collection_plugin_csv_status_ok( + run_cli_command, generic_collection_config_file, tmp_path +): + """Configured commands should collect successfully on the local system.""" + log_path = str(tmp_path / "logs_generic_collection_csv") + result = run_cli_command( + [ + "--log-path", + log_path, + f"--plugin-configs={generic_collection_config_file}", + "run-plugins", + "GenericCollectionPlugin", + ], + check=False, + ) + + output = result.stdout + result.stderr + assert "GenericCollectionPlugin" in output + + csv_files = list(Path(log_path).glob("**/nodescraper.csv")) + if not csv_files: + pytest.skip("CSV output not written; cannot verify collection status") + + with open(csv_files[0], "r", encoding="utf-8") as csv_file: + rows = [ + row + for row in csv.DictReader(csv_file) + if row.get("plugin") == "GenericCollectionPlugin" + ] + + assert len(rows) >= 1, "GenericCollectionPlugin should appear in CSV results" + assert rows[0].get("status") == "OK", rows[0].get("message") + assert "6/6 commands succeeded" in (rows[0].get("message") or "") + + +def test_generic_collection_plugin_without_commands_not_ran(run_cli_command, tmp_path): + """Running without collection_args.commands should report no commands configured.""" + log_path = str(tmp_path / "logs_generic_collection_empty") + result = run_cli_command( + ["--log-path", log_path, "run-plugins", "GenericCollectionPlugin"], + check=False, + ) + + output = result.stdout + result.stderr + assert result.returncode in [0, 1, 2] + assert "GenericCollectionPlugin" in output + assert "No commands configured" in output or "NOT_RAN" in output diff --git a/test/functional/test_plugin_configs.py b/test/functional/test_plugin_configs.py index 6ee1a004..ce06b057 100644 --- a/test/functional/test_plugin_configs.py +++ b/test/functional/test_plugin_configs.py @@ -48,6 +48,7 @@ def plugin_config_files(fixtures_dir): "DimmPlugin": fixtures_dir / "dimm_plugin_config.json", "DkmsPlugin": fixtures_dir / "dkms_plugin_config.json", "DmesgPlugin": fixtures_dir / "dmesg_plugin_config.json", + "GenericCollectionPlugin": fixtures_dir / "generic_collection_plugin_config.json", "JournalPlugin": fixtures_dir / "journal_plugin_config.json", "KernelPlugin": fixtures_dir / "kernel_plugin_config.json", "KernelModulePlugin": fixtures_dir / "kernel_module_plugin_config.json", @@ -111,6 +112,7 @@ def test_plugin_config_with_builtin_config(run_cli_command, tmp_path): "DimmPlugin", "DkmsPlugin", "DmesgPlugin", + "GenericCollectionPlugin", "JournalPlugin", "KernelPlugin", "KernelModulePlugin", diff --git a/test/unit/plugin/test_generic_collection_collector.py b/test/unit/plugin/test_generic_collection_collector.py new file mode 100644 index 00000000..b8447076 --- /dev/null +++ b/test/unit/plugin/test_generic_collection_collector.py @@ -0,0 +1,157 @@ +############################################################################### +# +# 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 unittest.mock import MagicMock + +import pytest + +from nodescraper.connection.inband.inband import CommandArtifact +from nodescraper.enums.executionstatus import ExecutionStatus +from nodescraper.enums.systeminteraction import SystemInteractionLevel +from nodescraper.plugins.inband.generic_collection.collector_args import ( + CommandSpec, + GenericCollectionCollectorArgs, +) +from nodescraper.plugins.inband.generic_collection.generic_collection_collector import ( + GenericCollectionCollector, +) +from nodescraper.plugins.inband.generic_collection.generic_collection_data import ( + CommandCollectionResult, + GenericCollectionDataModel, +) +from nodescraper.plugins.inband.generic_collection.generic_collection_plugin import ( + GenericCollectionPlugin, +) + + +@pytest.fixture +def collector(system_info, conn_mock): + return GenericCollectionCollector( + system_info=system_info, + system_interaction_level=SystemInteractionLevel.PASSIVE, + connection=conn_mock, + ) + + +def test_collect_all_commands_success(collector): + collector._run_sut_cmd = MagicMock( + side_effect=[ + CommandArtifact(exit_code=0, stdout="linux\n", stderr="", command="uname -s"), + CommandArtifact(exit_code=0, stdout="ok\n", stderr="", command="echo ok"), + ] + ) + args = GenericCollectionCollectorArgs(commands=["uname -s", "echo ok"]) + + result, data = collector.collect_data(args) + + assert result.status == ExecutionStatus.OK + assert data == GenericCollectionDataModel( + results=[ + CommandCollectionResult(command="uname -s", success=True, exit_code=0, sudo=False), + CommandCollectionResult(command="echo ok", success=True, exit_code=0, sudo=False), + ] + ) + assert collector._run_sut_cmd.call_count == 2 + + +def test_collect_reports_partial_failure(collector): + collector._run_sut_cmd = MagicMock( + side_effect=[ + CommandArtifact(exit_code=0, stdout="linux\n", stderr="", command="uname -s"), + CommandArtifact(exit_code=1, stdout="", stderr="failed", command="false"), + ] + ) + args = GenericCollectionCollectorArgs(commands=["uname -s", "false"]) + + result, data = collector.collect_data(args) + + assert result.status == ExecutionStatus.ERROR + assert data.results[0].success is True + assert data.results[1].success is False + assert data.results[1].exit_code == 1 + assert data.results[1].stderr == "failed" + + +def test_collect_no_commands(collector): + result, data = collector.collect_data(GenericCollectionCollectorArgs()) + + assert result.status == ExecutionStatus.NOT_RAN + assert data is None + + +def test_collect_passes_global_sudo_and_timeout(collector): + collector._run_sut_cmd = MagicMock( + return_value=CommandArtifact(exit_code=0, stdout="", stderr="", command="id") + ) + args = GenericCollectionCollectorArgs(commands=["id"], sudo=True, timeout=60) + + collector.collect_data(args) + + collector._run_sut_cmd.assert_called_once_with("id", sudo=True, timeout=60) + + +def test_collect_per_command_sudo_overrides(collector): + collector._run_sut_cmd = MagicMock( + side_effect=[ + CommandArtifact(exit_code=0, stdout="", stderr="", command="id"), + CommandArtifact(exit_code=0, stdout="", stderr="", command="cat /var/log/messages"), + ] + ) + args = GenericCollectionCollectorArgs( + commands=[ + "id", + CommandSpec(command="cat /var/log/messages", sudo=True), + ], + sudo=False, + timeout=300, + ) + + result, data = collector.collect_data(args) + + assert result.status == ExecutionStatus.OK + assert collector._run_sut_cmd.call_args_list[0].kwargs == {"sudo": False, "timeout": 300} + assert collector._run_sut_cmd.call_args_list[1].kwargs == {"sudo": True, "timeout": 300} + assert data.results[0].sudo is False + assert data.results[1].sudo is True + + +def test_collect_per_command_timeout_override(collector): + collector._run_sut_cmd = MagicMock( + return_value=CommandArtifact(exit_code=0, stdout="", stderr="", command="sleep 1") + ) + args = GenericCollectionCollectorArgs( + commands=[CommandSpec(command="sleep 1", timeout=10)], + timeout=300, + ) + + collector.collect_data(args) + + collector._run_sut_cmd.assert_called_once_with("sleep 1", sudo=False, timeout=10) + + +def test_generic_collection_plugin_wiring(): + assert GenericCollectionPlugin.DATA_MODEL is GenericCollectionDataModel + assert GenericCollectionPlugin.get_collector_classes() == (GenericCollectionCollector,) + assert GenericCollectionPlugin.COLLECTOR_ARGS is GenericCollectionCollectorArgs From d8af0fae9bb7413b3ef19bea041947b6ae409427 Mon Sep 17 00:00:00 2001 From: Alexandra Bara Date: Tue, 9 Jun 2026 12:57:40 -0500 Subject: [PATCH 02/13] generic plugin --- nodescraper/interfaces/dataplugin.py | 276 +++++++++++++----- .../generic_collection/analyzer_args.py | 135 +++++++++ .../generic_collection/collector_args.py | 60 ++-- .../generic_collection/generic_analyzer.py | 266 +++++++++++++++++ .../generic_collection_collector.py | 15 +- .../generic_collection_data.py | 2 + .../generic_collection_plugin.py | 14 +- .../generic_collection_plugin_config.json | 26 -- .../test_generic_collection_plugin.py | 47 ++- test/functional/test_plugin_configs.py | 3 +- test/unit/framework/test_dataplugin.py | 129 +++++++- test/unit/plugin/test_generic_analyzer.py | 243 +++++++++++++++ .../test_generic_collection_collector.py | 90 +++++- 13 files changed, 1171 insertions(+), 135 deletions(-) create mode 100644 nodescraper/plugins/inband/generic_collection/analyzer_args.py create mode 100644 nodescraper/plugins/inband/generic_collection/generic_analyzer.py delete mode 100644 test/functional/fixtures/generic_collection_plugin_config.json create mode 100644 test/unit/plugin/test_generic_analyzer.py diff --git a/nodescraper/interfaces/dataplugin.py b/nodescraper/interfaces/dataplugin.py index 8448dff3..19820b31 100644 --- a/nodescraper/interfaces/dataplugin.py +++ b/nodescraper/interfaces/dataplugin.py @@ -39,6 +39,7 @@ from nodescraper.interfaces.plugin import PluginInterface from nodescraper.models import ( AnalyzerArgs, + CollectorArgs, DataModel, DataPluginResult, PluginResult, @@ -51,6 +52,17 @@ from .task import SystemCompatibilityError from .taskresulthook import TaskResultHook +CollectorClasses = Union[ + Type[DataCollector], + tuple[Type[DataCollector], ...], + list[Type[DataCollector]], +] + +CollectorArgsClasses = Union[ + Type[CollectorArgs], + dict[str, Type[CollectorArgs]], +] + class DataPlugin( PluginInterface, Generic[TConnectionManager, TConnectArg, TDataModel, TCollectArg, TAnalyzeArg] @@ -61,7 +73,9 @@ class DataPlugin( CONNECTION_TYPE: Optional[Type[TConnectionManager]] - COLLECTOR: Optional[Type[DataCollector]] = None + COLLECTOR: Optional[CollectorClasses] = None + + COLLECTOR_ARGS: Optional[CollectorArgsClasses] = None ANALYZER: Optional[Type[DataAnalyzer]] = None @@ -101,6 +115,43 @@ def __init__( ) self._data: Optional[TDataModel] = None + @classmethod + def get_collector_classes(cls) -> tuple[Type[DataCollector], ...]: + """Return all collector classes configured on this plugin.""" + collector = cls.COLLECTOR + if collector is None: + return () + if isinstance(collector, (tuple, list)): + return tuple(collector) + return (collector,) + + @classmethod + def _collector_args_class( + cls, collector_cls: Type[DataCollector] + ) -> Optional[Type[CollectorArgs]]: + collector_args = cls.COLLECTOR_ARGS + if isinstance(collector_args, dict): + return collector_args.get(collector_cls.__name__) + return collector_args + + @classmethod + def _validate_collector_args(cls) -> None: + collector_args = cls.COLLECTOR_ARGS + if collector_args is None: + return + if isinstance(collector_args, dict): + for collector_name, args_cls in collector_args.items(): + if not isinstance(args_cls, type) or not issubclass(args_cls, CollectorArgs): + raise TypeError( + f"COLLECTOR_ARGS[{collector_name!r}] must be a CollectorArgs subclass, " + f"got {args_cls!r}" + ) + return + if not isinstance(collector_args, type) or not issubclass(collector_args, CollectorArgs): + raise TypeError( + f"COLLECTOR_ARGS must be a CollectorArgs subclass or dict, got {collector_args!r}" + ) + @classmethod def _validate_class_var(cls): if not hasattr(cls, "DATA_MODEL"): @@ -109,12 +160,96 @@ def _validate_class_var(cls): if cls.DATA_MODEL is None: raise TypeError("DATA_MODEL class variable not defined") - if not cls.COLLECTOR and not cls.ANALYZER: + if not cls.get_collector_classes() and not cls.ANALYZER: raise TypeError("No collector or analyzer task defined") - if cls.COLLECTOR and not cls.CONNECTION_TYPE: + if cls.get_collector_classes() and not cls.CONNECTION_TYPE: raise TypeError("CONNECTION_TYPE must be defined for collector") + for collector_cls in cls.get_collector_classes(): + if not isinstance(collector_cls, type) or not issubclass(collector_cls, DataCollector): + raise TypeError( + f"COLLECTOR entries must be DataCollector subclasses, got {collector_cls!r}" + ) + + cls._validate_collector_args() + + @classmethod + def _merge_collected_data( + cls, + existing: Optional[TDataModel], + new_data: Optional[TDataModel], + ) -> Optional[TDataModel]: + if new_data is None: + return existing + if existing is None: + return new_data + if not isinstance(new_data, existing.__class__): + raise TypeError( + f"Collector returned {new_data.__class__.__name__}, " + f"expected {existing.__class__.__name__}" + ) + merged = { + **existing.model_dump(exclude_unset=True), + **new_data.model_dump(exclude_unset=True), + } + return existing.__class__.model_validate(merged) + + @classmethod + def _aggregate_collection_results( + cls, + plugin_name: str, + results: list[TaskResult], + ) -> TaskResult: + if not results: + return TaskResult( + parent=plugin_name, + status=ExecutionStatus.NOT_RAN, + message=f"Data collection not ran for {plugin_name}", + ) + if len(results) == 1: + return results[0] + + aggregated = TaskResult( + parent=plugin_name, + status=max(result.status for result in results), + task=",".join(result.task for result in results if result.task), + ) + messages = [result.message for result in results if result.message] + if messages: + aggregated.message = "; ".join(messages) + for result in results: + aggregated.artifacts.extend(result.artifacts) + aggregated.events.extend(result.events) + aggregated.details["collector_results"] = [ + result.model_dump(exclude={"artifacts", "events"}) for result in results + ] + return aggregated + + def _resolve_collector_args( + self, + collector_cls: Type[DataCollector], + collection_args: Optional[Union[TCollectArg, dict]], + ) -> Optional[Union[TCollectArg, dict]]: + if collection_args is None: + return None + + collector_name = collector_cls.__name__ + collector_names = {cls.__name__ for cls in self.get_collector_classes()} + raw_args: Optional[Union[TCollectArg, dict]] = collection_args + + if isinstance(collection_args, dict) and collector_names.intersection( + collection_args.keys() + ): + raw_args = collection_args.get(collector_name) + if raw_args is None: + return None + + args_cls = self._collector_args_class(collector_cls) + if args_cls is not None and isinstance(raw_args, dict): + return args_cls.model_validate(raw_args) + return raw_args + @classmethod def is_valid(cls) -> bool: """Check that all required class variables are set @@ -167,7 +302,8 @@ def collect( Returns: TaskResult: task result for data collection """ - if not self.COLLECTOR: + collector_classes = self.get_collector_classes() + if not collector_classes: self.collection_result = TaskResult( parent=self.__class__.__name__, status=ExecutionStatus.NOT_RAN, @@ -175,11 +311,13 @@ def collect( ) return self.collection_result + primary_collector = collector_classes[0] + try: if not self.connection_manager: if not self.CONNECTION_TYPE: self.collection_result = TaskResult( - task=self.COLLECTOR.__name__, + task=primary_collector.__name__, parent=self.__class__.__name__, status=ExecutionStatus.NOT_RAN, message=f"No connection manager type provided for {self.__class__.__name__}", @@ -203,49 +341,53 @@ def collect( if self.connection_manager.result.status != ExecutionStatus.OK: self.collection_result = TaskResult( - task=self.COLLECTOR.__name__, + task=primary_collector.__name__, parent=self.__class__.__name__, status=ExecutionStatus.NOT_RAN, 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) + collector_results: list[TaskResult] = [] + merged_data: Optional[TDataModel] = None + + for collector_cls in collector_classes: + collector_args = self._resolve_collector_args(collector_cls, collection_args) + collection_task = collector_cls( + system_info=self.system_info, + logger=self.logger, + system_interaction_level=system_interaction_level, + connection=self.connection_manager.connection, + max_event_priority_level=max_event_priority_level, + parent=self.__class__.__name__, + task_result_hooks=self.task_result_hooks, + log_path=self.log_path, + event_reporter=self.event_reporter, + session_id=self.session_id, + ) + result, data = collection_task.collect_data(collector_args) + collector_results.append(result) + merged_data = self._merge_collected_data(merged_data, data) - collection_task = self.COLLECTOR( - system_info=self.system_info, - logger=self.logger, - system_interaction_level=system_interaction_level, - connection=self.connection_manager.connection, - max_event_priority_level=max_event_priority_level, - parent=self.__class__.__name__, - task_result_hooks=self.task_result_hooks, - log_path=self.log_path, - event_reporter=self.event_reporter, - session_id=self.session_id, + self.collection_result = self._aggregate_collection_results( + self.__class__.__name__, + collector_results, ) - self.collection_result, self._data = collection_task.collect_data(collection_args) + self._data = merged_data except SystemCompatibilityError as e: self.collection_result = TaskResult( - task=self.COLLECTOR.__name__, + task=primary_collector.__name__, parent=self.__class__.__name__, status=ExecutionStatus.NOT_RAN, message=str(e), ) except Exception as e: self.logger.exception( - "Unhandled exception running collector %s for plugin %s", - self.COLLECTOR.__name__, + "Unhandled exception running collectors for plugin %s", self.__class__.__name__, ) self.collection_result = TaskResult( - task=self.COLLECTOR.__name__, + task=primary_collector.__name__, parent=self.__class__.__name__, status=ExecutionStatus.EXECUTION_FAILURE, message=f"Unhandled exception running data collector: {str(e)}", @@ -382,19 +524,19 @@ def run( ExecutionStatus.EXECUTION_FAILURE, ExecutionStatus.WARNING, ]: - if self.analysis_result.status > self.collection_result.status: - message = ( - f"Analysis warning: {self.analysis_result.message}" - if self.analysis_result.status == ExecutionStatus.WARNING - else f"Analysis error: {self.analysis_result.message}" - ) - else: - - message = ( - f"Collection warning: {self.collection_result.message}" - if self.collection_result.status == ExecutionStatus.WARNING - else f"Collection error: {self.collection_result.message}" - ) + failure_parts: list[str] = [] + for label, task_result in ( + ("Collection", self.collection_result), + ("Analysis", self.analysis_result), + ): + if task_result.status == ExecutionStatus.WARNING: + failure_parts.append(f"{label} warning: {task_result.message}") + elif task_result.status in ( + ExecutionStatus.ERROR, + ExecutionStatus.EXECUTION_FAILURE, + ): + failure_parts.append(f"{label} error: {task_result.message}") + message = "; ".join(failure_parts) else: message = "Plugin tasks completed successfully" @@ -422,33 +564,33 @@ def find_datamodel_path_in_run(cls, run_path: str) -> Optional[str]: run_path = os.path.abspath(run_path) if not os.path.isdir(run_path): return None - collector_cls = getattr(cls, "COLLECTOR", None) data_model_cls = getattr(cls, "DATA_MODEL", None) - if not collector_cls or not data_model_cls: - return None - collector_dir = os.path.join( - run_path, - pascal_to_snake(cls.__name__), - pascal_to_snake(collector_cls.__name__), - ) - if not os.path.isdir(collector_dir): - return None - result_path = os.path.join(collector_dir, "result.json") - if not os.path.isfile(result_path): - return None - try: - res_payload = json.loads(Path(result_path).read_text(encoding="utf-8")) - if res_payload.get("parent") != cls.__name__: - return None - except (json.JSONDecodeError, OSError): + if not data_model_cls: return None - want_json = data_model_cls.__name__.lower() + ".json" - for fname in os.listdir(collector_dir): - low = fname.lower() - if low.endswith("datamodel.json") or low == want_json: - return os.path.join(collector_dir, fname) - if low.endswith(".log"): - return os.path.join(collector_dir, fname) + for collector_cls in cls.get_collector_classes(): + collector_dir = os.path.join( + run_path, + pascal_to_snake(cls.__name__), + pascal_to_snake(collector_cls.__name__), + ) + if not os.path.isdir(collector_dir): + continue + result_path = os.path.join(collector_dir, "result.json") + if not os.path.isfile(result_path): + continue + try: + res_payload = json.loads(Path(result_path).read_text(encoding="utf-8")) + if res_payload.get("parent") != cls.__name__: + continue + except (json.JSONDecodeError, OSError): + continue + want_json = data_model_cls.__name__.lower() + ".json" + for fname in os.listdir(collector_dir): + low = fname.lower() + if low.endswith("datamodel.json") or low == want_json: + return os.path.join(collector_dir, fname) + if low.endswith(".log"): + return os.path.join(collector_dir, fname) return None @classmethod diff --git a/nodescraper/plugins/inband/generic_collection/analyzer_args.py b/nodescraper/plugins/inband/generic_collection/analyzer_args.py new file mode 100644 index 00000000..c7a13312 --- /dev/null +++ b/nodescraper/plugins/inband/generic_collection/analyzer_args.py @@ -0,0 +1,135 @@ +############################################################################### +# +# 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 Literal, Optional, Union + +from pydantic import BaseModel, Field, field_validator, model_validator + +from nodescraper.models import AnalyzerArgs + +CompareOp = Literal["==", "!=", ">", ">=", "<", "<="] +MatchMode = Literal["full", "any_line", "all_lines"] +ValueType = Literal["int", "float", "str"] + + +class CommandCheck(BaseModel): + """Validation rule for one collected command result, matched by collector command name.""" + + model_config = {"extra": "forbid"} + + name: str = Field( + description="Name of the collected command to validate (must match collection_args.commands[].name).", + ) + allow_failure: bool = Field( + default=False, + description="When True, a collection failure for this command does not fail the check.", + ) + expected_exit_code: Optional[int] = Field( + default=None, + description="Expected exit code from collection. Defaults to 0 when other content checks are set.", + ) + must_contain: Optional[Union[str, list[str]]] = Field( + default=None, + description="Stdout must contain this text or all texts in the list.", + ) + must_not_contain: Optional[Union[str, list[str]]] = Field( + default=None, + description="Stdout must not contain this text or any texts in the list.", + ) + expected: Optional[str] = Field( + default=None, + description="Exact stdout match after strip.", + ) + expected_in: Optional[list[str]] = Field( + default=None, + description="Stripped stdout must be one of these values.", + ) + expected_regex: Optional[str] = Field( + default=None, + description="Stdout must match this regex.", + ) + forbidden_regex: Optional[str] = Field( + default=None, + description="Stdout must not match this regex.", + ) + ignore_case: bool = Field( + default=False, + description="Case-insensitive matching for substring and regex checks.", + ) + match_mode: MatchMode = Field( + default="full", + description="How to apply regex checks: full output, any line, or all non-empty lines.", + ) + min_lines: Optional[int] = Field(default=None, ge=0) + max_lines: Optional[int] = Field(default=None, ge=0) + exact_lines: Optional[int] = Field(default=None, ge=0) + value_type: ValueType = Field( + default="int", + description="Type used when parsing stdout for expected_value checks.", + ) + compare_op: CompareOp = Field( + default="==", + description="Comparison operator for expected_value checks.", + ) + expected_value: Optional[Union[int, float, str]] = Field( + default=None, + description="Value to compare against parsed stdout.", + ) + capture_regex: Optional[str] = Field( + default=None, + description="Optional regex with a capture group used before expected_value comparison.", + ) + + @field_validator("name", mode="before") + @classmethod + def _strip_name(cls, value: object) -> object: + if isinstance(value, str): + return value.strip() + return value + + @model_validator(mode="after") + def _validate_name(self) -> "CommandCheck": + if not self.name: + raise ValueError("name must not be empty") + return self + + +class GenericAnalyzerArgs(AnalyzerArgs): + checks: list[CommandCheck] = Field( + default_factory=list, + description="Per-command validation rules keyed by collected command name.", + ) + + @model_validator(mode="after") + def _validate_unique_check_names(self) -> "GenericAnalyzerArgs": + seen: set[str] = set() + duplicates: set[str] = set() + for check in self.checks: + if check.name in seen: + duplicates.add(check.name) + seen.add(check.name) + if duplicates: + raise ValueError(f"Duplicate check name(s): {sorted(duplicates)}") + return self diff --git a/nodescraper/plugins/inband/generic_collection/collector_args.py b/nodescraper/plugins/inband/generic_collection/collector_args.py index be7f3c61..deb04b0a 100644 --- a/nodescraper/plugins/inband/generic_collection/collector_args.py +++ b/nodescraper/plugins/inband/generic_collection/collector_args.py @@ -23,18 +23,19 @@ # SOFTWARE. # ############################################################################### -from typing import Optional, Union +from typing import Optional -from pydantic import BaseModel, Field, field_validator +from pydantic import BaseModel, Field, field_validator, model_validator from nodescraper.models import CollectorArgs class CommandSpec(BaseModel): - """One shell command and optional per-command overrides.""" + """One named shell command and optional per-command overrides.""" model_config = {"extra": "forbid"} + name: str = Field(description="Stable name for this command, used by analysis checks.") command: str = Field(description="Shell command to run on the target system.") sudo: Optional[bool] = Field( default=None, @@ -45,15 +46,31 @@ class CommandSpec(BaseModel): ge=1, description="Command timeout in seconds. When omitted, uses collection_args.timeout.", ) + include_stdout: Optional[bool] = Field( + default=None, + description="Store stdout in the data model. When omitted, uses collection_args.include_stdout.", + ) + + @field_validator("name", "command", mode="before") + @classmethod + def _strip_required_text(cls, value: object) -> object: + if isinstance(value, str): + return value.strip() + return value + + @model_validator(mode="after") + def _validate_required_fields(self) -> "CommandSpec": + if not self.name: + raise ValueError("name must not be empty") + if not self.command: + raise ValueError("command must not be empty") + return self class GenericCollectionCollectorArgs(CollectorArgs): commands: list[CommandSpec] = Field( default_factory=list, - description=( - "Commands to run. Each entry may be a plain string or an object with " - "'command' and optional 'sudo' / 'timeout' overrides." - ), + description="Named commands to run. Each entry must include 'name' and 'command'.", ) sudo: bool = Field( default=False, @@ -64,20 +81,29 @@ class GenericCollectionCollectorArgs(CollectorArgs): ge=1, description="Default per-command timeout in seconds.", ) + include_stdout: bool = Field( + default=True, + description="Default setting for storing stdout in the data model for analysis.", + ) @field_validator("commands", mode="before") @classmethod - def _normalize_commands( - cls, value: Optional[list[Union[str, dict, CommandSpec]]] - ) -> list[Union[str, dict, CommandSpec]]: + def _reject_plain_string_commands(cls, value: Optional[list[object]]) -> object: if not value: return [] - normalized: list[Union[str, dict, CommandSpec]] = [] for item in value: if isinstance(item, str): - command = item.strip() - if command: - normalized.append({"command": command}) - else: - normalized.append(item) - return normalized + raise ValueError("Each command must be an object with 'name' and 'command' fields") + return value + + @model_validator(mode="after") + def _validate_unique_command_names(self) -> "GenericCollectionCollectorArgs": + seen: set[str] = set() + duplicates: set[str] = set() + for cmd in self.commands: + if cmd.name in seen: + duplicates.add(cmd.name) + seen.add(cmd.name) + if duplicates: + raise ValueError(f"Duplicate command name(s): {sorted(duplicates)}") + return self diff --git a/nodescraper/plugins/inband/generic_collection/generic_analyzer.py b/nodescraper/plugins/inband/generic_collection/generic_analyzer.py new file mode 100644 index 00000000..96f7b8a4 --- /dev/null +++ b/nodescraper/plugins/inband/generic_collection/generic_analyzer.py @@ -0,0 +1,266 @@ +############################################################################### +# +# 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 operator +import re +from typing import Callable, Optional, Union + +from nodescraper.enums import EventCategory, EventPriority, ExecutionStatus +from nodescraper.interfaces import DataAnalyzer +from nodescraper.models import TaskResult + +from .analyzer_args import CommandCheck, CompareOp, GenericAnalyzerArgs +from .generic_collection_data import CommandCollectionResult, GenericCollectionDataModel + +_COMPARE_OPS: dict[CompareOp, Callable[[Union[int, float, str], Union[int, float, str]], bool]] = { + "==": operator.eq, + "!=": operator.ne, + ">": operator.gt, + ">=": operator.ge, + "<": operator.lt, + "<=": operator.le, +} + + +def _as_list(value: Union[str, list[str]]) -> list[str]: + if isinstance(value, str): + return [value] + return list(value) + + +def _non_empty_lines(stdout: str) -> list[str]: + return [line for line in stdout.splitlines() if line.strip()] + + +def _needs_stdout(check: CommandCheck) -> bool: + return any( + [ + check.must_contain is not None, + check.must_not_contain is not None, + check.expected is not None, + check.expected_in is not None, + check.expected_regex is not None, + check.forbidden_regex is not None, + check.min_lines is not None, + check.max_lines is not None, + check.exact_lines is not None, + check.expected_value is not None, + ] + ) + + +def _check_label(check: CommandCheck) -> str: + return check.name + + +def _find_result( + data: GenericCollectionDataModel, check: CommandCheck +) -> Optional[CommandCollectionResult]: + for result in data.results: + if result.name == check.name: + return result + return None + + +def _regex_flags(ignore_case: bool) -> int: + return re.IGNORECASE if ignore_case else 0 + + +def _regex_matches(pattern: str, text: str, mode: str, ignore_case: bool) -> bool: + flags = _regex_flags(ignore_case) + compiled = re.compile(pattern, flags) + if mode == "full": + return compiled.search(text) is not None + lines = _non_empty_lines(text) + if not lines: + return False + if mode == "any_line": + return any(compiled.search(line) for line in lines) + return all(compiled.search(line) for line in lines) + + +def _contains(text: str, needle: str, ignore_case: bool) -> bool: + if ignore_case: + return needle.lower() in text.lower() + return needle in text + + +def _parse_value(raw: str, value_type: str, capture_regex: Optional[str]) -> Union[int, float, str]: + value = raw.strip() + if capture_regex: + match = re.search(capture_regex, value) + if not match: + raise ValueError(f"capture_regex {capture_regex!r} did not match stdout") + value = match.group(1) if match.groups() else match.group(0) + value = value.strip() + if value_type == "int": + return int(value) + if value_type == "float": + return float(value) + return value + + +class GenericAnalyzer(DataAnalyzer[GenericCollectionDataModel, GenericAnalyzerArgs]): + """Validate GenericCollectionPlugin command results against analysis_args checks.""" + + DATA_MODEL = GenericCollectionDataModel + + def analyze_data( + self, + data: GenericCollectionDataModel, + args: Optional[GenericAnalyzerArgs] = None, + ) -> TaskResult: + if args is None: + args = GenericAnalyzerArgs() + + if not data.results: + self.result.message = "No command results to analyze" + self.result.status = ExecutionStatus.NOT_RAN + return self.result + + if not args.checks: + success_count = sum(1 for result in data.results if result.success) + self.result.message = ( + f"Generic analysis: {success_count}/{len(data.results)} commands collected" + ) + self.result.status = ExecutionStatus.OK + return self.result + + failures: list[str] = [] + failed_check_count = 0 + for check in args.checks: + label = _check_label(check) + result = _find_result(data, check) + if result is None: + failures.append(f"{label}: no matching collected command") + failed_check_count += 1 + continue + + check_failures = self._evaluate_check(check, result) + if check_failures: + failed_check_count += 1 + failures.extend(f"{label}: {msg}" for msg in check_failures) + self._log_event( + category=EventCategory.RUNTIME, + description=f"Check failed: {label}", + data={ + "failures": check_failures, + "name": result.name, + "command": result.command, + }, + priority=EventPriority.ERROR, + console_log=True, + ) + else: + self._log_event( + category=EventCategory.RUNTIME, + description=f"Check passed: {label}", + data={"name": result.name, "command": result.command}, + priority=EventPriority.INFO, + ) + + if failed_check_count: + passed = len(args.checks) - failed_check_count + self.result.message = f"Generic analysis: {passed}/{len(args.checks)} checks passed" + self.result.status = ExecutionStatus.ERROR + return self.result + + self.result.message = ( + f"Generic analysis: {len(args.checks)}/{len(args.checks)} checks passed" + ) + self.result.status = ExecutionStatus.OK + return self.result + + def _evaluate_check(self, check: CommandCheck, result: CommandCollectionResult) -> list[str]: + failures: list[str] = [] + + if not result.success and not check.allow_failure: + failures.append(f"command failed with exit code {result.exit_code}") + if not _needs_stdout(check) and check.expected_exit_code is None: + return failures + + expected_exit_code = check.expected_exit_code + if expected_exit_code is None and _needs_stdout(check): + expected_exit_code = 0 + if expected_exit_code is not None and result.exit_code != expected_exit_code: + failures.append(f"expected exit code {expected_exit_code}, got {result.exit_code}") + + if not _needs_stdout(check): + return failures + + if result.stdout is None: + failures.append("stdout not collected; set include_stdout on the command") + return failures + + stdout = result.stdout + stripped = stdout.strip() + + for needle in _as_list(check.must_contain or []): + if not _contains(stdout, needle, check.ignore_case): + failures.append(f"must_contain {needle!r} not found") + + for needle in _as_list(check.must_not_contain or []): + if _contains(stdout, needle, check.ignore_case): + failures.append(f"must_not_contain {needle!r} found") + + if check.expected is not None and stripped != check.expected: + failures.append(f"expected exact stdout {check.expected!r}, got {stripped!r}") + + if check.expected_in is not None and stripped not in check.expected_in: + failures.append(f"stdout {stripped!r} not in expected_in") + + if check.expected_regex is not None and not _regex_matches( + check.expected_regex, stdout, check.match_mode, check.ignore_case + ): + failures.append(f"expected_regex {check.expected_regex!r} did not match") + + if check.forbidden_regex is not None and _regex_matches( + check.forbidden_regex, stdout, check.match_mode, check.ignore_case + ): + failures.append(f"forbidden_regex {check.forbidden_regex!r} matched") + + line_count = len(_non_empty_lines(stdout)) + if check.exact_lines is not None and line_count != check.exact_lines: + failures.append(f"expected {check.exact_lines} non-empty lines, got {line_count}") + if check.min_lines is not None and line_count < check.min_lines: + failures.append( + f"expected at least {check.min_lines} non-empty lines, got {line_count}" + ) + if check.max_lines is not None and line_count > check.max_lines: + failures.append(f"expected at most {check.max_lines} non-empty lines, got {line_count}") + + if check.expected_value is not None: + try: + parsed = _parse_value(stripped, check.value_type, check.capture_regex) + except (TypeError, ValueError) as exc: + failures.append(str(exc)) + else: + compare = _COMPARE_OPS[check.compare_op] + if not compare(parsed, check.expected_value): + failures.append( + f"expected parsed value {parsed!r} {check.compare_op} {check.expected_value!r}" + ) + + return failures diff --git a/nodescraper/plugins/inband/generic_collection/generic_collection_collector.py b/nodescraper/plugins/inband/generic_collection/generic_collection_collector.py index 618a64d8..e2a3e0b7 100644 --- a/nodescraper/plugins/inband/generic_collection/generic_collection_collector.py +++ b/nodescraper/plugins/inband/generic_collection/generic_collection_collector.py @@ -59,6 +59,11 @@ def collect_data( sudo = cmd_spec.sudo if cmd_spec.sudo is not None else args.sudo timeout = cmd_spec.timeout if cmd_spec.timeout is not None else args.timeout + include_stdout = ( + cmd_spec.include_stdout + if cmd_spec.include_stdout is not None + else args.include_stdout + ) res = self._run_sut_cmd( command, sudo=sudo, @@ -66,10 +71,12 @@ def collect_data( ) success = res.exit_code == 0 cmd_result = CommandCollectionResult( + name=cmd_spec.name, command=command, success=success, exit_code=res.exit_code, sudo=sudo, + stdout=res.stdout if include_stdout else None, stderr=res.stderr or None, ) results.append(cmd_result) @@ -78,7 +85,12 @@ def collect_data( self._log_event( category=EventCategory.RUNTIME, description=f"Command succeeded: {command!r}", - data={"command": command, "exit_code": res.exit_code, "sudo": sudo}, + data={ + "name": cmd_spec.name, + "command": command, + "exit_code": res.exit_code, + "sudo": sudo, + }, priority=EventPriority.INFO, ) else: @@ -86,6 +98,7 @@ def collect_data( category=EventCategory.RUNTIME, description=f"Command failed: {command!r}", data={ + "name": cmd_spec.name, "command": command, "exit_code": res.exit_code, "sudo": sudo, diff --git a/nodescraper/plugins/inband/generic_collection/generic_collection_data.py b/nodescraper/plugins/inband/generic_collection/generic_collection_data.py index c14a20ad..3421df96 100644 --- a/nodescraper/plugins/inband/generic_collection/generic_collection_data.py +++ b/nodescraper/plugins/inband/generic_collection/generic_collection_data.py @@ -34,9 +34,11 @@ class CommandCollectionResult(DataModel): """Outcome of running one configured shell command.""" command: str + name: str success: bool exit_code: int sudo: bool = False + stdout: Optional[str] = None stderr: Optional[str] = None diff --git a/nodescraper/plugins/inband/generic_collection/generic_collection_plugin.py b/nodescraper/plugins/inband/generic_collection/generic_collection_plugin.py index b299e474..e706f84d 100644 --- a/nodescraper/plugins/inband/generic_collection/generic_collection_plugin.py +++ b/nodescraper/plugins/inband/generic_collection/generic_collection_plugin.py @@ -25,18 +25,28 @@ ############################################################################### from nodescraper.base import InBandDataPlugin +from .analyzer_args import GenericAnalyzerArgs from .collector_args import GenericCollectionCollectorArgs +from .generic_analyzer import GenericAnalyzer from .generic_collection_collector import GenericCollectionCollector from .generic_collection_data import GenericCollectionDataModel class GenericCollectionPlugin( - InBandDataPlugin[GenericCollectionDataModel, GenericCollectionCollectorArgs, None] + InBandDataPlugin[ + GenericCollectionDataModel, + GenericCollectionCollectorArgs, + GenericAnalyzerArgs, + ] ): - """Run arbitrary shell commands configured via collection_args.""" + """Run arbitrary shell commands configured via collection_args and validate via analysis_args.""" DATA_MODEL = GenericCollectionDataModel COLLECTOR = GenericCollectionCollector COLLECTOR_ARGS = GenericCollectionCollectorArgs + + ANALYZER = GenericAnalyzer + + ANALYZER_ARGS = GenericAnalyzerArgs diff --git a/test/functional/fixtures/generic_collection_plugin_config.json b/test/functional/fixtures/generic_collection_plugin_config.json deleted file mode 100644 index adbf46b6..00000000 --- a/test/functional/fixtures/generic_collection_plugin_config.json +++ /dev/null @@ -1,26 +0,0 @@ -{ - "global_args": {}, - "plugins": { - "GenericCollectionPlugin": { - "collection_args": { - "commands": [ - "uname -s", - "uname -m", - "hostname", - { - "command": "echo generic-collection-ok", - "timeout": 60 - }, - { - "command": "id -u", - "sudo": false - }, - "test -d /proc && echo proc-ok" - ] - } - } - }, - "result_collators": {}, - "name": "GenericCollectionPlugin config", - "desc": "Config for testing GenericCollectionPlugin with plain and per-command options" -} diff --git a/test/functional/test_generic_collection_plugin.py b/test/functional/test_generic_collection_plugin.py index 74be8d07..3bc52747 100644 --- a/test/functional/test_generic_collection_plugin.py +++ b/test/functional/test_generic_collection_plugin.py @@ -30,17 +30,13 @@ import pytest - -@pytest.fixture -def fixtures_dir(): - """Return path to fixtures directory.""" - return Path(__file__).parent / "fixtures" +REPO_ROOT = Path(__file__).resolve().parent.parent.parent @pytest.fixture -def generic_collection_config_file(fixtures_dir): - """Return path to GenericCollectionPlugin config file.""" - return fixtures_dir / "generic_collection_plugin_config.json" +def generic_collection_config_file(): + """Return path to GenericCollectionPlugin config at repo root.""" + return REPO_ROOT / "generic_collection_plugin_config.json" def test_generic_collection_plugin_with_config_file( @@ -69,10 +65,10 @@ def test_generic_collection_plugin_with_config_file( assert "Generic collection" in output or "genericcollection" in output.lower() -def test_generic_collection_plugin_csv_status_ok( +def test_generic_collection_plugin_demo_config_runs_end_to_end( run_cli_command, generic_collection_config_file, tmp_path ): - """Configured commands should collect successfully on the local system.""" + """Repo-root demo config runs collection and analysis with expected partial failures.""" log_path = str(tmp_path / "logs_generic_collection_csv") result = run_cli_command( [ @@ -87,6 +83,11 @@ def test_generic_collection_plugin_csv_status_ok( output = result.stdout + result.stderr assert "GenericCollectionPlugin" in output + assert "14/14 commands succeeded" not in output + assert "14/14 checks passed" not in output + assert "/14 commands succeeded" in output + assert "/14 checks passed" in output + assert "Check failed: kernel_os" in output csv_files = list(Path(log_path).glob("**/nodescraper.csv")) if not csv_files: @@ -100,8 +101,30 @@ def test_generic_collection_plugin_csv_status_ok( ] assert len(rows) >= 1, "GenericCollectionPlugin should appear in CSV results" - assert rows[0].get("status") == "OK", rows[0].get("message") - assert "6/6 commands succeeded" in (rows[0].get("message") or "") + assert rows[0].get("status") == "ERROR", rows[0].get("message") + + +def test_generic_collection_plugin_runs_analyzer( + run_cli_command, generic_collection_config_file, tmp_path +): + """Analysis checks from the repo-root config should run after collection.""" + log_path = str(tmp_path / "logs_generic_collection_analysis") + result = run_cli_command( + [ + "--log-path", + log_path, + f"--plugin-configs={generic_collection_config_file}", + "run-plugins", + "GenericCollectionPlugin", + ], + check=False, + ) + + output = result.stdout + result.stderr + assert "Running data analyzer: GenericAnalyzer" in output + assert "Generic analysis:" in output + assert "14/14 checks passed" not in output + assert "Check failed: kernel_os" in output def test_generic_collection_plugin_without_commands_not_ran(run_cli_command, tmp_path): diff --git a/test/functional/test_plugin_configs.py b/test/functional/test_plugin_configs.py index ce06b057..ed8bc979 100644 --- a/test/functional/test_plugin_configs.py +++ b/test/functional/test_plugin_configs.py @@ -41,6 +41,7 @@ def fixtures_dir(): @pytest.fixture def plugin_config_files(fixtures_dir): """Return dict mapping plugin names to their config file paths.""" + repo_root = Path(__file__).resolve().parent.parent.parent return { "AmdSmiPlugin": fixtures_dir / "amdsmi_plugin_config.json", "BiosPlugin": fixtures_dir / "bios_plugin_config.json", @@ -48,7 +49,7 @@ def plugin_config_files(fixtures_dir): "DimmPlugin": fixtures_dir / "dimm_plugin_config.json", "DkmsPlugin": fixtures_dir / "dkms_plugin_config.json", "DmesgPlugin": fixtures_dir / "dmesg_plugin_config.json", - "GenericCollectionPlugin": fixtures_dir / "generic_collection_plugin_config.json", + "GenericCollectionPlugin": repo_root / "generic_collection_plugin_config.json", "JournalPlugin": fixtures_dir / "journal_plugin_config.json", "KernelPlugin": fixtures_dir / "kernel_plugin_config.json", "KernelModulePlugin": fixtures_dir / "kernel_module_plugin_config.json", diff --git a/test/unit/framework/test_dataplugin.py b/test/unit/framework/test_dataplugin.py index e88f8cc5..ce7612e6 100644 --- a/test/unit/framework/test_dataplugin.py +++ b/test/unit/framework/test_dataplugin.py @@ -25,6 +25,7 @@ ############################################################################### import json from pathlib import Path +from typing import Optional from unittest.mock import MagicMock, patch import pytest @@ -34,7 +35,7 @@ from nodescraper.interfaces.dataanalyzertask import DataAnalyzer from nodescraper.interfaces.datacollectortask import DataCollector from nodescraper.interfaces.dataplugin import DataPlugin -from nodescraper.models import DataModel, TaskResult +from nodescraper.models import CollectorArgs, DataModel, TaskResult class StandardDataModel(DataModel): @@ -261,6 +262,28 @@ def test_run_execution_modes(self, plugin_with_conn, collection, analysis, expec assert mock_collect.call_count == expected_calls[0] assert mock_analyze.call_count == expected_calls[1] + def test_run_reports_collection_and_analysis_errors(self, plugin_with_conn): + plugin_with_conn.data = StandardDataModel() + + with ( + patch.object(CoreDataPlugin, "collect") as mock_collect, + patch.object(CoreDataPlugin, "analyze") as mock_analyze, + ): + mock_collect.return_value = TaskResult( + status=ExecutionStatus.ERROR, + message="Generic collection: 2/3 commands succeeded", + ) + mock_analyze.return_value = TaskResult( + status=ExecutionStatus.ERROR, + message="Generic analysis: 1/3 checks passed", + ) + + result = plugin_with_conn.run(collection=True, analysis=True) + + assert result.status == ExecutionStatus.ERROR + assert "Collection error: Generic collection: 2/3 commands succeeded" in result.message + assert "Analysis error: Generic analysis: 1/3 checks passed" in result.message + def test_run_with_parameters(self, plugin_with_conn): collection_args = {"param": "value"} analysis_args = {"threshold": 0.5} @@ -543,3 +566,107 @@ def test_load_run_data_direct_file(self, tmp_path: Path) -> None: loaded = ExtractPlugin.load_run_data(str(p)) assert loaded is not None assert loaded["value"] == "direct" + + +class MultiPartDataModel(DataModel): + alpha: Optional[str] = None + beta: Optional[str] = None + + +class AlphaCollector(DataCollector): + DATA_MODEL = MultiPartDataModel + + def collect_data(self, args=None): + return TaskResult(status=ExecutionStatus.OK, task="AlphaCollector"), MultiPartDataModel( + alpha="alpha-value" + ) + + +class BetaCollector(DataCollector): + DATA_MODEL = MultiPartDataModel + + def collect_data(self, args=None): + return TaskResult(status=ExecutionStatus.OK, task="BetaCollector"), MultiPartDataModel( + beta="beta-value" + ) + + +class AlphaCollectorArgs(CollectorArgs): + alpha_path: str = "/alpha" + + +class BetaCollectorArgs(CollectorArgs): + beta_path: str = "/beta" + + +class MultiCollectorPlugin(DataPlugin): + DATA_MODEL = MultiPartDataModel + CONNECTION_TYPE = MockConnectionManager + COLLECTOR = (AlphaCollector, BetaCollector) + ANALYZER = StandardAnalyzer + + +class TestMultiCollectorDataPlugin: + def test_get_collector_classes_accepts_collector_tuple(self): + assert MultiCollectorPlugin.get_collector_classes() == (AlphaCollector, BetaCollector) + + def test_get_collector_classes_accepts_single_collector(self): + assert CoreDataPlugin.get_collector_classes() == (BaseDataCollector,) + + def test_collector_args_class_accepts_args_map(self): + class MappedArgsPlugin(DataPlugin): + DATA_MODEL = MultiPartDataModel + CONNECTION_TYPE = MockConnectionManager + COLLECTOR = (AlphaCollector, BetaCollector) + COLLECTOR_ARGS = { + "AlphaCollector": AlphaCollectorArgs, + "BetaCollector": BetaCollectorArgs, + } + ANALYZER = StandardAnalyzer + + assert MappedArgsPlugin._collector_args_class(AlphaCollector) is AlphaCollectorArgs + assert MappedArgsPlugin._collector_args_class(BetaCollector) is BetaCollectorArgs + + def test_collect_runs_all_collectors_and_merges_data(self, plugin_with_conn): + multi_plugin = MultiCollectorPlugin( + system_info=plugin_with_conn.system_info, + logger=plugin_with_conn.logger, + connection_manager=plugin_with_conn.connection_manager, + ) + + with ( + patch.object(AlphaCollector, "collect_data") as alpha_collect, + patch.object(BetaCollector, "collect_data") as beta_collect, + ): + alpha_collect.return_value = ( + TaskResult(status=ExecutionStatus.OK, task="AlphaCollector"), + MultiPartDataModel(alpha="alpha-value"), + ) + beta_collect.return_value = ( + TaskResult(status=ExecutionStatus.OK, task="BetaCollector"), + MultiPartDataModel(beta="beta-value"), + ) + + result = multi_plugin.collect() + + alpha_collect.assert_called_once() + beta_collect.assert_called_once() + assert result.status == ExecutionStatus.OK + assert multi_plugin.data.alpha == "alpha-value" + assert multi_plugin.data.beta == "beta-value" + assert "AlphaCollector" in result.task + assert "BetaCollector" in result.task + + def test_find_datamodel_path_in_run_checks_all_collectors(self, tmp_path: Path) -> None: + beta_dir = tmp_path / "multi_collector_plugin" / "beta_collector" + beta_dir.mkdir(parents=True) + (beta_dir / "result.json").write_text( + json.dumps({"parent": "MultiCollectorPlugin"}), encoding="utf-8" + ) + (beta_dir / "multipartdatamodel.json").write_text( + json.dumps({"alpha": "a", "beta": "b"}), encoding="utf-8" + ) + + found = MultiCollectorPlugin.find_datamodel_path_in_run(str(tmp_path)) + assert found is not None + assert found.endswith("multipartdatamodel.json") diff --git a/test/unit/plugin/test_generic_analyzer.py b/test/unit/plugin/test_generic_analyzer.py new file mode 100644 index 00000000..1156106f --- /dev/null +++ b/test/unit/plugin/test_generic_analyzer.py @@ -0,0 +1,243 @@ +############################################################################### +# +# 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 pydantic import ValidationError + +from nodescraper.enums.executionstatus import ExecutionStatus +from nodescraper.plugins.inband.generic_collection.analyzer_args import ( + CommandCheck, + GenericAnalyzerArgs, +) +from nodescraper.plugins.inband.generic_collection.generic_analyzer import ( + GenericAnalyzer, +) +from nodescraper.plugins.inband.generic_collection.generic_collection_data import ( + CommandCollectionResult, + GenericCollectionDataModel, +) + + +@pytest.fixture +def analyzer(system_info): + return GenericAnalyzer(system_info=system_info) + + +def _data(*results: CommandCollectionResult) -> GenericCollectionDataModel: + return GenericCollectionDataModel(results=list(results)) + + +def test_evaluates_each_check_independently(analyzer): + data = _data( + CommandCollectionResult( + name="kernel_os", + command="uname -s", + success=True, + exit_code=0, + stdout="Linux\n", + ), + CommandCollectionResult( + name="messages", + command="cat /var/log/messages", + success=False, + exit_code=1, + stdout="", + stderr="No such file", + ), + CommandCollectionResult( + name="uid", + command="id -u", + success=True, + exit_code=0, + stdout="1000\n", + ), + ) + args = GenericAnalyzerArgs( + checks=[ + CommandCheck(name="kernel_os", must_contain="TEST"), + CommandCheck(name="messages", must_not_contain="error"), + CommandCheck(name="uid", expected_regex=r"^\d+$"), + ], + ) + + result = analyzer.analyze_data(data, args) + + assert result.status == ExecutionStatus.ERROR + assert "1/3 checks passed" in result.message + + +def test_must_contain_passes(analyzer): + data = _data( + CommandCollectionResult( + name="kernel_os", + command="uname -s", + success=True, + exit_code=0, + stdout="Linux\n", + ) + ) + args = GenericAnalyzerArgs(checks=[CommandCheck(name="kernel_os", must_contain="Linux")]) + + result = analyzer.analyze_data(data, args) + + assert result.status == ExecutionStatus.OK + assert "1/1 checks passed" in result.message + + +def test_expected_value_numeric_compare(analyzer): + data = _data( + CommandCollectionResult( + name="gpu_count", + command="echo 8", + success=True, + exit_code=0, + stdout="8\n", + ) + ) + args = GenericAnalyzerArgs( + checks=[ + CommandCheck( + name="gpu_count", + expected_value=8, + compare_op="==", + value_type="int", + ) + ], + ) + + result = analyzer.analyze_data(data, args) + + assert result.status == ExecutionStatus.OK + + +def test_expected_value_numeric_compare_fails(analyzer): + data = _data( + CommandCollectionResult( + name="gpu_count", + command="echo 4", + success=True, + exit_code=0, + stdout="4\n", + ) + ) + args = GenericAnalyzerArgs( + checks=[ + CommandCheck( + name="gpu_count", + expected_value=8, + compare_op="==", + value_type="int", + ) + ], + ) + + result = analyzer.analyze_data(data, args) + + assert result.status == ExecutionStatus.ERROR + + +def test_line_count_checks(analyzer): + data = _data( + CommandCollectionResult( + name="devices", + command="lspci", + success=True, + exit_code=0, + stdout="dev1\n\ndev2\n", + ) + ) + args = GenericAnalyzerArgs(checks=[CommandCheck(name="devices", min_lines=2, max_lines=2)]) + + result = analyzer.analyze_data(data, args) + + assert result.status == ExecutionStatus.OK + + +def test_stdout_required_for_content_check(analyzer): + data = _data( + CommandCollectionResult( + name="kernel_os", + command="uname -s", + success=True, + exit_code=0, + stdout=None, + ) + ) + args = GenericAnalyzerArgs(checks=[CommandCheck(name="kernel_os", must_contain="Linux")]) + + result = analyzer.analyze_data(data, args) + + assert result.status == ExecutionStatus.ERROR + + +def test_allow_failure_passes_failed_command_check(analyzer): + data = _data( + CommandCollectionResult( + name="optional", + command="false", + success=False, + exit_code=1, + stdout="", + ) + ) + args = GenericAnalyzerArgs( + checks=[CommandCheck(name="optional", allow_failure=True, expected_exit_code=1)], + ) + + result = analyzer.analyze_data(data, args) + + assert result.status == ExecutionStatus.OK + + +def test_no_checks_reports_collection_summary(analyzer): + data = _data( + CommandCollectionResult( + name="false_cmd", + command="false", + success=False, + exit_code=1, + stdout="", + ) + ) + + result = analyzer.analyze_data(data, GenericAnalyzerArgs(checks=[])) + + assert result.status == ExecutionStatus.OK + assert "0/1 commands collected" in result.message + + +def test_analyzer_args_require_unique_check_names(): + with pytest.raises(ValidationError, match="Duplicate check name"): + GenericAnalyzerArgs( + checks=[ + CommandCheck(name="kernel_os", must_contain="Linux"), + CommandCheck(name="kernel_os", expected="Linux"), + ] + ) + + +def test_analyzer_args_require_check_name(): + with pytest.raises(ValidationError): + GenericAnalyzerArgs(checks=[CommandCheck(name="", must_contain="Linux")]) diff --git a/test/unit/plugin/test_generic_collection_collector.py b/test/unit/plugin/test_generic_collection_collector.py index b8447076..e77fe310 100644 --- a/test/unit/plugin/test_generic_collection_collector.py +++ b/test/unit/plugin/test_generic_collection_collector.py @@ -26,6 +26,7 @@ from unittest.mock import MagicMock import pytest +from pydantic import ValidationError from nodescraper.connection.inband.inband import CommandArtifact from nodescraper.enums.executionstatus import ExecutionStatus @@ -62,15 +63,34 @@ def test_collect_all_commands_success(collector): CommandArtifact(exit_code=0, stdout="ok\n", stderr="", command="echo ok"), ] ) - args = GenericCollectionCollectorArgs(commands=["uname -s", "echo ok"]) + args = GenericCollectionCollectorArgs( + commands=[ + CommandSpec(name="kernel_os", command="uname -s"), + CommandSpec(name="echo_ok", command="echo ok"), + ] + ) result, data = collector.collect_data(args) assert result.status == ExecutionStatus.OK assert data == GenericCollectionDataModel( results=[ - CommandCollectionResult(command="uname -s", success=True, exit_code=0, sudo=False), - CommandCollectionResult(command="echo ok", success=True, exit_code=0, sudo=False), + CommandCollectionResult( + name="kernel_os", + command="uname -s", + success=True, + exit_code=0, + sudo=False, + stdout="linux\n", + ), + CommandCollectionResult( + name="echo_ok", + command="echo ok", + success=True, + exit_code=0, + sudo=False, + stdout="ok\n", + ), ] ) assert collector._run_sut_cmd.call_count == 2 @@ -83,7 +103,12 @@ def test_collect_reports_partial_failure(collector): CommandArtifact(exit_code=1, stdout="", stderr="failed", command="false"), ] ) - args = GenericCollectionCollectorArgs(commands=["uname -s", "false"]) + args = GenericCollectionCollectorArgs( + commands=[ + CommandSpec(name="kernel_os", command="uname -s"), + CommandSpec(name="false_cmd", command="false"), + ] + ) result, data = collector.collect_data(args) @@ -105,7 +130,11 @@ def test_collect_passes_global_sudo_and_timeout(collector): collector._run_sut_cmd = MagicMock( return_value=CommandArtifact(exit_code=0, stdout="", stderr="", command="id") ) - args = GenericCollectionCollectorArgs(commands=["id"], sudo=True, timeout=60) + args = GenericCollectionCollectorArgs( + commands=[CommandSpec(name="user_id", command="id")], + sudo=True, + timeout=60, + ) collector.collect_data(args) @@ -121,8 +150,8 @@ def test_collect_per_command_sudo_overrides(collector): ) args = GenericCollectionCollectorArgs( commands=[ - "id", - CommandSpec(command="cat /var/log/messages", sudo=True), + CommandSpec(name="user_id", command="id"), + CommandSpec(name="messages", command="cat /var/log/messages", sudo=True), ], sudo=False, timeout=300, @@ -142,7 +171,7 @@ def test_collect_per_command_timeout_override(collector): return_value=CommandArtifact(exit_code=0, stdout="", stderr="", command="sleep 1") ) args = GenericCollectionCollectorArgs( - commands=[CommandSpec(command="sleep 1", timeout=10)], + commands=[CommandSpec(name="sleep_one", command="sleep 1", timeout=10)], timeout=300, ) @@ -151,7 +180,52 @@ def test_collect_per_command_timeout_override(collector): collector._run_sut_cmd.assert_called_once_with("sleep 1", sudo=False, timeout=10) +def test_collect_stores_stdout_when_disabled(collector): + collector._run_sut_cmd = MagicMock( + return_value=CommandArtifact( + exit_code=0, stdout="secret\n", stderr="", command="echo secret" + ) + ) + args = GenericCollectionCollectorArgs( + commands=[CommandSpec(name="secret", command="echo secret", include_stdout=False)], + include_stdout=True, + ) + + _, data = collector.collect_data(args) + + assert data.results[0].stdout is None + + +def test_collector_args_reject_plain_string_commands(): + with pytest.raises(ValidationError, match="name' and 'command'"): + GenericCollectionCollectorArgs(commands=["uname -s"]) + + +def test_collector_args_require_name(): + with pytest.raises(ValidationError): + GenericCollectionCollectorArgs(commands=[CommandSpec(name="", command="uname -s")]) + + +def test_collector_args_require_unique_names(): + with pytest.raises(ValidationError, match="Duplicate command name"): + GenericCollectionCollectorArgs( + commands=[ + CommandSpec(name="dup", command="uname -s"), + CommandSpec(name="dup", command="uname -m"), + ] + ) + + def test_generic_collection_plugin_wiring(): assert GenericCollectionPlugin.DATA_MODEL is GenericCollectionDataModel assert GenericCollectionPlugin.get_collector_classes() == (GenericCollectionCollector,) assert GenericCollectionPlugin.COLLECTOR_ARGS is GenericCollectionCollectorArgs + from nodescraper.plugins.inband.generic_collection.analyzer_args import ( + GenericAnalyzerArgs, + ) + from nodescraper.plugins.inband.generic_collection.generic_analyzer import ( + GenericAnalyzer, + ) + + assert GenericCollectionPlugin.ANALYZER is GenericAnalyzer + assert GenericCollectionPlugin.ANALYZER_ARGS is GenericAnalyzerArgs From d638ffab6e14be659185820a69955154adbbe9c2 Mon Sep 17 00:00:00 2001 From: Alexandra Bara Date: Tue, 9 Jun 2026 19:14:47 -0500 Subject: [PATCH 03/13] added oobgenericcollectionplugin --- .../plugins/generic_collection/__init__.py | 24 +++++++ .../generic_collection/analyzer_args.py | 13 +--- .../generic_collection/collector_args.py | 6 +- .../generic_collection/generic_analyzer.py | 2 +- .../generic_collection_collector.py | 0 .../generic_collection_data.py | 0 .../generic_collection_plugin_mixin.py | 52 ++++++++++++++ .../inband_plugin.py} | 18 ++--- .../oob_plugin.py} | 17 +++++ nodescraper/typeutils.py | 9 +-- pyproject.toml | 4 +- test/unit/framework/test_type_utils.py | 12 ++++ test/unit/plugin/test_generic_analyzer.py | 10 +-- .../test_generic_collection_collector.py | 24 +++---- .../test_oob_generic_collection_plugin.py | 72 +++++++++++++++++++ 15 files changed, 205 insertions(+), 58 deletions(-) create mode 100644 nodescraper/plugins/generic_collection/__init__.py rename nodescraper/plugins/{inband => }/generic_collection/analyzer_args.py (93%) rename nodescraper/plugins/{inband => }/generic_collection/collector_args.py (96%) rename nodescraper/plugins/{inband => }/generic_collection/generic_analyzer.py (99%) rename nodescraper/plugins/{inband => }/generic_collection/generic_collection_collector.py (100%) rename nodescraper/plugins/{inband => }/generic_collection/generic_collection_data.py (100%) create mode 100644 nodescraper/plugins/generic_collection/generic_collection_plugin_mixin.py rename nodescraper/plugins/{inband/generic_collection/generic_collection_plugin.py => generic_collection/inband_plugin.py} (79%) rename nodescraper/plugins/{inband/generic_collection/__init__.py => generic_collection/oob_plugin.py} (68%) create mode 100644 test/unit/plugin/test_oob_generic_collection_plugin.py diff --git a/nodescraper/plugins/generic_collection/__init__.py b/nodescraper/plugins/generic_collection/__init__.py new file mode 100644 index 00000000..659fbbd9 --- /dev/null +++ b/nodescraper/plugins/generic_collection/__init__.py @@ -0,0 +1,24 @@ +"""Generic command collection plugins (in-band and OOB SSH).""" + +from .analyzer_args import CommandCheck, GenericAnalyzerArgs +from .collector_args import CommandSpec, GenericCollectionCollectorArgs +from .generic_analyzer import GenericAnalyzer +from .generic_collection_collector import GenericCollectionCollector +from .generic_collection_data import CommandCollectionResult, GenericCollectionDataModel +from .generic_collection_plugin_mixin import GenericCollectionPluginMixin +from .inband_plugin import GenericCollectionPlugin +from .oob_plugin import OOBGenericCollectionPlugin + +__all__ = [ + "CommandCheck", + "CommandCollectionResult", + "CommandSpec", + "GenericAnalyzer", + "GenericAnalyzerArgs", + "GenericCollectionCollector", + "GenericCollectionCollectorArgs", + "GenericCollectionDataModel", + "GenericCollectionPlugin", + "GenericCollectionPluginMixin", + "OOBGenericCollectionPlugin", +] diff --git a/nodescraper/plugins/inband/generic_collection/analyzer_args.py b/nodescraper/plugins/generic_collection/analyzer_args.py similarity index 93% rename from nodescraper/plugins/inband/generic_collection/analyzer_args.py rename to nodescraper/plugins/generic_collection/analyzer_args.py index c7a13312..a68e0fb2 100644 --- a/nodescraper/plugins/inband/generic_collection/analyzer_args.py +++ b/nodescraper/plugins/generic_collection/analyzer_args.py @@ -25,7 +25,7 @@ ############################################################################### from typing import Literal, Optional, Union -from pydantic import BaseModel, Field, field_validator, model_validator +from pydantic import Field, model_validator from nodescraper.models import AnalyzerArgs @@ -34,11 +34,9 @@ ValueType = Literal["int", "float", "str"] -class CommandCheck(BaseModel): +class CommandCheck(AnalyzerArgs): """Validation rule for one collected command result, matched by collector command name.""" - model_config = {"extra": "forbid"} - name: str = Field( description="Name of the collected command to validate (must match collection_args.commands[].name).", ) @@ -102,13 +100,6 @@ class CommandCheck(BaseModel): description="Optional regex with a capture group used before expected_value comparison.", ) - @field_validator("name", mode="before") - @classmethod - def _strip_name(cls, value: object) -> object: - if isinstance(value, str): - return value.strip() - return value - @model_validator(mode="after") def _validate_name(self) -> "CommandCheck": if not self.name: diff --git a/nodescraper/plugins/inband/generic_collection/collector_args.py b/nodescraper/plugins/generic_collection/collector_args.py similarity index 96% rename from nodescraper/plugins/inband/generic_collection/collector_args.py rename to nodescraper/plugins/generic_collection/collector_args.py index deb04b0a..fb0015d2 100644 --- a/nodescraper/plugins/inband/generic_collection/collector_args.py +++ b/nodescraper/plugins/generic_collection/collector_args.py @@ -25,16 +25,14 @@ ############################################################################### from typing import Optional -from pydantic import BaseModel, Field, field_validator, model_validator +from pydantic import Field, field_validator, model_validator from nodescraper.models import CollectorArgs -class CommandSpec(BaseModel): +class CommandSpec(CollectorArgs): """One named shell command and optional per-command overrides.""" - model_config = {"extra": "forbid"} - name: str = Field(description="Stable name for this command, used by analysis checks.") command: str = Field(description="Shell command to run on the target system.") sudo: Optional[bool] = Field( diff --git a/nodescraper/plugins/inband/generic_collection/generic_analyzer.py b/nodescraper/plugins/generic_collection/generic_analyzer.py similarity index 99% rename from nodescraper/plugins/inband/generic_collection/generic_analyzer.py rename to nodescraper/plugins/generic_collection/generic_analyzer.py index 96f7b8a4..ce911276 100644 --- a/nodescraper/plugins/inband/generic_collection/generic_analyzer.py +++ b/nodescraper/plugins/generic_collection/generic_analyzer.py @@ -123,7 +123,7 @@ def _parse_value(raw: str, value_type: str, capture_regex: Optional[str]) -> Uni class GenericAnalyzer(DataAnalyzer[GenericCollectionDataModel, GenericAnalyzerArgs]): - """Validate GenericCollectionPlugin command results against analysis_args checks.""" + """Validate generic collection command results against analysis_args checks.""" DATA_MODEL = GenericCollectionDataModel diff --git a/nodescraper/plugins/inband/generic_collection/generic_collection_collector.py b/nodescraper/plugins/generic_collection/generic_collection_collector.py similarity index 100% rename from nodescraper/plugins/inband/generic_collection/generic_collection_collector.py rename to nodescraper/plugins/generic_collection/generic_collection_collector.py diff --git a/nodescraper/plugins/inband/generic_collection/generic_collection_data.py b/nodescraper/plugins/generic_collection/generic_collection_data.py similarity index 100% rename from nodescraper/plugins/inband/generic_collection/generic_collection_data.py rename to nodescraper/plugins/generic_collection/generic_collection_data.py diff --git a/nodescraper/plugins/generic_collection/generic_collection_plugin_mixin.py b/nodescraper/plugins/generic_collection/generic_collection_plugin_mixin.py new file mode 100644 index 00000000..2a91c644 --- /dev/null +++ b/nodescraper/plugins/generic_collection/generic_collection_plugin_mixin.py @@ -0,0 +1,52 @@ +############################################################################### +# +# 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. +# +############################################################################### +"""Shared plugin wiring for in-band and OOB generic command collection plugins.""" + +from typing import Optional, Type + +from nodescraper.interfaces.dataanalyzertask import DataAnalyzer +from nodescraper.interfaces.dataplugin import CollectorArgsClasses, CollectorClasses +from nodescraper.models import AnalyzerArgs + +from .analyzer_args import GenericAnalyzerArgs +from .collector_args import GenericCollectionCollectorArgs +from .generic_analyzer import GenericAnalyzer +from .generic_collection_collector import GenericCollectionCollector +from .generic_collection_data import GenericCollectionDataModel + + +class GenericCollectionPluginMixin: + """Collector, analyzer, and args shared by GenericCollectionPlugin variants.""" + + DATA_MODEL: Type[GenericCollectionDataModel] = GenericCollectionDataModel + + COLLECTOR: Optional[CollectorClasses] = GenericCollectionCollector + + COLLECTOR_ARGS: Optional[CollectorArgsClasses] = GenericCollectionCollectorArgs + + ANALYZER: Optional[Type[DataAnalyzer]] = GenericAnalyzer + + ANALYZER_ARGS: Optional[Type[AnalyzerArgs]] = GenericAnalyzerArgs diff --git a/nodescraper/plugins/inband/generic_collection/generic_collection_plugin.py b/nodescraper/plugins/generic_collection/inband_plugin.py similarity index 79% rename from nodescraper/plugins/inband/generic_collection/generic_collection_plugin.py rename to nodescraper/plugins/generic_collection/inband_plugin.py index e706f84d..f00ee138 100644 --- a/nodescraper/plugins/inband/generic_collection/generic_collection_plugin.py +++ b/nodescraper/plugins/generic_collection/inband_plugin.py @@ -27,26 +27,16 @@ from .analyzer_args import GenericAnalyzerArgs from .collector_args import GenericCollectionCollectorArgs -from .generic_analyzer import GenericAnalyzer -from .generic_collection_collector import GenericCollectionCollector from .generic_collection_data import GenericCollectionDataModel +from .generic_collection_plugin_mixin import GenericCollectionPluginMixin class GenericCollectionPlugin( + GenericCollectionPluginMixin, InBandDataPlugin[ GenericCollectionDataModel, GenericCollectionCollectorArgs, GenericAnalyzerArgs, - ] + ], ): - """Run arbitrary shell commands configured via collection_args and validate via analysis_args.""" - - DATA_MODEL = GenericCollectionDataModel - - COLLECTOR = GenericCollectionCollector - - COLLECTOR_ARGS = GenericCollectionCollectorArgs - - ANALYZER = GenericAnalyzer - - ANALYZER_ARGS = GenericAnalyzerArgs + """Run arbitrary shell commands on the host via in-band SSH and validate results.""" diff --git a/nodescraper/plugins/inband/generic_collection/__init__.py b/nodescraper/plugins/generic_collection/oob_plugin.py similarity index 68% rename from nodescraper/plugins/inband/generic_collection/__init__.py rename to nodescraper/plugins/generic_collection/oob_plugin.py index eced675e..e68f0f25 100644 --- a/nodescraper/plugins/inband/generic_collection/__init__.py +++ b/nodescraper/plugins/generic_collection/oob_plugin.py @@ -23,3 +23,20 @@ # SOFTWARE. # ############################################################################### +from nodescraper.base import OOBSSHDataPlugin + +from .analyzer_args import GenericAnalyzerArgs +from .collector_args import GenericCollectionCollectorArgs +from .generic_collection_data import GenericCollectionDataModel +from .generic_collection_plugin_mixin import GenericCollectionPluginMixin + + +class OOBGenericCollectionPlugin( + GenericCollectionPluginMixin, + OOBSSHDataPlugin[ + GenericCollectionDataModel, + GenericCollectionCollectorArgs, + GenericAnalyzerArgs, + ], +): + """Run arbitrary shell commands on the BMC via OOB SSH and validate results.""" diff --git a/nodescraper/typeutils.py b/nodescraper/typeutils.py index cd7a4650..777108a8 100644 --- a/nodescraper/typeutils.py +++ b/nodescraper/typeutils.py @@ -57,13 +57,14 @@ def get_generic_map(cls, class_type: Type[Any]) -> dict: Returns: dict: map of generic type parameters to their actual types """ - if class_type.__orig_bases__ and len(class_type.__orig_bases__) > 0: - gen_base = class_type.__orig_bases__[0] + generic_map: dict = {} + for gen_base in getattr(class_type, "__orig_bases__", ()) or (): class_org = get_origin(gen_base) + if class_org is None: + continue args = get_args(gen_base) generic_map = dict(zip(class_org.__parameters__, args)) - else: - generic_map = {} + break return generic_map diff --git a/pyproject.toml b/pyproject.toml index c5495ffc..1d40c1a8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -29,7 +29,9 @@ dev = [ "pre-commit", "pytest", "pytest-cov", - "mypy" + "mypy", + "types-paramiko", + "types-setuptools", ] [project.urls] diff --git a/test/unit/framework/test_type_utils.py b/test/unit/framework/test_type_utils.py index be14d7ee..a6bb1012 100644 --- a/test/unit/framework/test_type_utils.py +++ b/test/unit/framework/test_type_utils.py @@ -45,6 +45,14 @@ class TestGenericImpl(TestGenericBase[str]): pass +class WiringMixin: + pass + + +class TestMixinFirstImpl(WiringMixin, TestGenericBase[str]): + pass + + class TestModel(BaseModel): str_attr: str int_attr: int @@ -57,6 +65,10 @@ def test_generic_map(): assert TypeUtils.get_generic_map(TestGenericImpl) == {T: str} +def test_generic_map_skips_non_generic_mixin_base(): + assert TypeUtils.get_generic_map(TestMixinFirstImpl) == {T: str} + + def test_func_arg_types(): res = TypeUtils.get_func_arg_types(TestGenericImpl.test_func, TestGenericImpl) assert list(res.keys()) == ["arg", "arg2", "arg3"] diff --git a/test/unit/plugin/test_generic_analyzer.py b/test/unit/plugin/test_generic_analyzer.py index 1156106f..63c507b2 100644 --- a/test/unit/plugin/test_generic_analyzer.py +++ b/test/unit/plugin/test_generic_analyzer.py @@ -27,15 +27,11 @@ from pydantic import ValidationError from nodescraper.enums.executionstatus import ExecutionStatus -from nodescraper.plugins.inband.generic_collection.analyzer_args import ( +from nodescraper.plugins.generic_collection import ( CommandCheck, - GenericAnalyzerArgs, -) -from nodescraper.plugins.inband.generic_collection.generic_analyzer import ( - GenericAnalyzer, -) -from nodescraper.plugins.inband.generic_collection.generic_collection_data import ( CommandCollectionResult, + GenericAnalyzer, + GenericAnalyzerArgs, GenericCollectionDataModel, ) diff --git a/test/unit/plugin/test_generic_collection_collector.py b/test/unit/plugin/test_generic_collection_collector.py index e77fe310..c7b3c802 100644 --- a/test/unit/plugin/test_generic_collection_collector.py +++ b/test/unit/plugin/test_generic_collection_collector.py @@ -31,18 +31,12 @@ from nodescraper.connection.inband.inband import CommandArtifact from nodescraper.enums.executionstatus import ExecutionStatus from nodescraper.enums.systeminteraction import SystemInteractionLevel -from nodescraper.plugins.inband.generic_collection.collector_args import ( +from nodescraper.plugins.generic_collection import ( + CommandCollectionResult, CommandSpec, - GenericCollectionCollectorArgs, -) -from nodescraper.plugins.inband.generic_collection.generic_collection_collector import ( GenericCollectionCollector, -) -from nodescraper.plugins.inband.generic_collection.generic_collection_data import ( - CommandCollectionResult, + GenericCollectionCollectorArgs, GenericCollectionDataModel, -) -from nodescraper.plugins.inband.generic_collection.generic_collection_plugin import ( GenericCollectionPlugin, ) @@ -217,15 +211,13 @@ def test_collector_args_require_unique_names(): def test_generic_collection_plugin_wiring(): - assert GenericCollectionPlugin.DATA_MODEL is GenericCollectionDataModel - assert GenericCollectionPlugin.get_collector_classes() == (GenericCollectionCollector,) - assert GenericCollectionPlugin.COLLECTOR_ARGS is GenericCollectionCollectorArgs - from nodescraper.plugins.inband.generic_collection.analyzer_args import ( - GenericAnalyzerArgs, - ) - from nodescraper.plugins.inband.generic_collection.generic_analyzer import ( + from nodescraper.plugins.generic_collection import ( GenericAnalyzer, + GenericAnalyzerArgs, ) + assert GenericCollectionPlugin.DATA_MODEL is GenericCollectionDataModel + assert GenericCollectionPlugin.get_collector_classes() == (GenericCollectionCollector,) + assert GenericCollectionPlugin.COLLECTOR_ARGS is GenericCollectionCollectorArgs assert GenericCollectionPlugin.ANALYZER is GenericAnalyzer assert GenericCollectionPlugin.ANALYZER_ARGS is GenericAnalyzerArgs diff --git a/test/unit/plugin/test_oob_generic_collection_plugin.py b/test/unit/plugin/test_oob_generic_collection_plugin.py new file mode 100644 index 00000000..acea5862 --- /dev/null +++ b/test/unit/plugin/test_oob_generic_collection_plugin.py @@ -0,0 +1,72 @@ +############################################################################### +# +# 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, OOBSSHDataPlugin +from nodescraper.connection.inband.inbandmanager import InBandConnectionManager +from nodescraper.pluginregistry import PluginRegistry +from nodescraper.plugins.generic_collection import ( + GenericAnalyzer, + GenericAnalyzerArgs, + GenericCollectionCollector, + GenericCollectionCollectorArgs, + GenericCollectionDataModel, + GenericCollectionPlugin, + GenericCollectionPluginMixin, + OOBGenericCollectionPlugin, +) + + +def test_generic_collection_plugins_are_valid(): + assert GenericCollectionPlugin.is_valid() + assert OOBGenericCollectionPlugin.is_valid() + + +def test_generic_collection_plugins_register(): + registry = PluginRegistry() + assert "GenericCollectionPlugin" in registry.plugins + assert "OOBGenericCollectionPlugin" in registry.plugins + + +def test_generic_collection_plugin_mixin_wiring(): + for plugin_cls in (GenericCollectionPlugin, OOBGenericCollectionPlugin): + assert plugin_cls.DATA_MODEL is GenericCollectionDataModel + assert plugin_cls.get_collector_classes() == (GenericCollectionCollector,) + assert plugin_cls.COLLECTOR_ARGS is GenericCollectionCollectorArgs + assert plugin_cls.ANALYZER is GenericAnalyzer + assert plugin_cls.ANALYZER_ARGS is GenericAnalyzerArgs + + +def test_generic_collection_plugin_uses_inband_base(): + assert issubclass(GenericCollectionPlugin, InBandDataPlugin) + assert issubclass(GenericCollectionPlugin, GenericCollectionPluginMixin) + assert GenericCollectionPlugin.CONNECTION_TYPE is InBandConnectionManager + + +def test_oob_generic_collection_plugin_uses_oob_ssh_base(): + assert issubclass(OOBGenericCollectionPlugin, OOBSSHDataPlugin) + assert issubclass(OOBGenericCollectionPlugin, GenericCollectionPluginMixin) + assert OOBGenericCollectionPlugin.CONNECTION_TYPE is InBandConnectionManager + assert GenericCollectionPlugin.COLLECTOR is OOBGenericCollectionPlugin.COLLECTOR + assert GenericCollectionPlugin.ANALYZER is OOBGenericCollectionPlugin.ANALYZER From f80ccbd794f62230ff6d1b58807faa3005c19b57 Mon Sep 17 00:00:00 2001 From: Alexandra Bara Date: Wed, 10 Jun 2026 09:54:32 -0500 Subject: [PATCH 04/13] utest fix --- test/unit/framework/test_dataplugin.py | 28 ++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/test/unit/framework/test_dataplugin.py b/test/unit/framework/test_dataplugin.py index ce7612e6..67b92fb9 100644 --- a/test/unit/framework/test_dataplugin.py +++ b/test/unit/framework/test_dataplugin.py @@ -265,18 +265,30 @@ def test_run_execution_modes(self, plugin_with_conn, collection, analysis, expec def test_run_reports_collection_and_analysis_errors(self, plugin_with_conn): plugin_with_conn.data = StandardDataModel() + collection_error = TaskResult( + status=ExecutionStatus.ERROR, + message="Generic collection: 2/3 commands succeeded", + ) + analysis_error = TaskResult( + status=ExecutionStatus.ERROR, + message="Generic analysis: 1/3 checks passed", + ) + with ( patch.object(CoreDataPlugin, "collect") as mock_collect, patch.object(CoreDataPlugin, "analyze") as mock_analyze, ): - mock_collect.return_value = TaskResult( - status=ExecutionStatus.ERROR, - message="Generic collection: 2/3 commands succeeded", - ) - mock_analyze.return_value = TaskResult( - status=ExecutionStatus.ERROR, - message="Generic analysis: 1/3 checks passed", - ) + + def collect_side_effect(*args, **kwargs): + plugin_with_conn.collection_result = collection_error + return collection_error + + def analyze_side_effect(*args, **kwargs): + plugin_with_conn.analysis_result = analysis_error + return analysis_error + + mock_collect.side_effect = collect_side_effect + mock_analyze.side_effect = analyze_side_effect result = plugin_with_conn.run(collection=True, analysis=True) From 1442069de59d8a58cdc69f523936227464d82216 Mon Sep 17 00:00:00 2001 From: Alexandra Bara Date: Wed, 10 Jun 2026 10:36:43 -0500 Subject: [PATCH 05/13] rename plugin to camel case --- .../plugins/generic_collection/__init__.py | 4 ++-- .../plugins/generic_collection/oob_plugin.py | 2 +- .../test_oob_generic_collection_plugin.py | 18 +++++++++--------- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/nodescraper/plugins/generic_collection/__init__.py b/nodescraper/plugins/generic_collection/__init__.py index 659fbbd9..c2cdee75 100644 --- a/nodescraper/plugins/generic_collection/__init__.py +++ b/nodescraper/plugins/generic_collection/__init__.py @@ -7,7 +7,7 @@ from .generic_collection_data import CommandCollectionResult, GenericCollectionDataModel from .generic_collection_plugin_mixin import GenericCollectionPluginMixin from .inband_plugin import GenericCollectionPlugin -from .oob_plugin import OOBGenericCollectionPlugin +from .oob_plugin import OobGenericCollectionPlugin __all__ = [ "CommandCheck", @@ -20,5 +20,5 @@ "GenericCollectionDataModel", "GenericCollectionPlugin", "GenericCollectionPluginMixin", - "OOBGenericCollectionPlugin", + "OobGenericCollectionPlugin", ] diff --git a/nodescraper/plugins/generic_collection/oob_plugin.py b/nodescraper/plugins/generic_collection/oob_plugin.py index e68f0f25..e2451600 100644 --- a/nodescraper/plugins/generic_collection/oob_plugin.py +++ b/nodescraper/plugins/generic_collection/oob_plugin.py @@ -31,7 +31,7 @@ from .generic_collection_plugin_mixin import GenericCollectionPluginMixin -class OOBGenericCollectionPlugin( +class OobGenericCollectionPlugin( GenericCollectionPluginMixin, OOBSSHDataPlugin[ GenericCollectionDataModel, diff --git a/test/unit/plugin/test_oob_generic_collection_plugin.py b/test/unit/plugin/test_oob_generic_collection_plugin.py index acea5862..7c206f29 100644 --- a/test/unit/plugin/test_oob_generic_collection_plugin.py +++ b/test/unit/plugin/test_oob_generic_collection_plugin.py @@ -34,23 +34,23 @@ GenericCollectionDataModel, GenericCollectionPlugin, GenericCollectionPluginMixin, - OOBGenericCollectionPlugin, + OobGenericCollectionPlugin, ) def test_generic_collection_plugins_are_valid(): assert GenericCollectionPlugin.is_valid() - assert OOBGenericCollectionPlugin.is_valid() + assert OobGenericCollectionPlugin.is_valid() def test_generic_collection_plugins_register(): registry = PluginRegistry() assert "GenericCollectionPlugin" in registry.plugins - assert "OOBGenericCollectionPlugin" in registry.plugins + assert "OobGenericCollectionPlugin" in registry.plugins def test_generic_collection_plugin_mixin_wiring(): - for plugin_cls in (GenericCollectionPlugin, OOBGenericCollectionPlugin): + for plugin_cls in (GenericCollectionPlugin, OobGenericCollectionPlugin): assert plugin_cls.DATA_MODEL is GenericCollectionDataModel assert plugin_cls.get_collector_classes() == (GenericCollectionCollector,) assert plugin_cls.COLLECTOR_ARGS is GenericCollectionCollectorArgs @@ -65,8 +65,8 @@ def test_generic_collection_plugin_uses_inband_base(): def test_oob_generic_collection_plugin_uses_oob_ssh_base(): - assert issubclass(OOBGenericCollectionPlugin, OOBSSHDataPlugin) - assert issubclass(OOBGenericCollectionPlugin, GenericCollectionPluginMixin) - assert OOBGenericCollectionPlugin.CONNECTION_TYPE is InBandConnectionManager - assert GenericCollectionPlugin.COLLECTOR is OOBGenericCollectionPlugin.COLLECTOR - assert GenericCollectionPlugin.ANALYZER is OOBGenericCollectionPlugin.ANALYZER + assert issubclass(OobGenericCollectionPlugin, OOBSSHDataPlugin) + assert issubclass(OobGenericCollectionPlugin, GenericCollectionPluginMixin) + assert OobGenericCollectionPlugin.CONNECTION_TYPE is InBandConnectionManager + assert GenericCollectionPlugin.COLLECTOR is OobGenericCollectionPlugin.COLLECTOR + assert GenericCollectionPlugin.ANALYZER is OobGenericCollectionPlugin.ANALYZER From f7cb25a2e5f8662613a2f9eabcc6383849dea80d Mon Sep 17 00:00:00 2001 From: Alexandra Bara Date: Wed, 10 Jun 2026 14:04:47 -0500 Subject: [PATCH 06/13] updated connection manger to RedfishConnectionManager to ssh into BMC --- nodescraper/base/oobsshdataplugin.py | 57 ++++++++++++-- nodescraper/connection/redfish/__init__.py | 2 + nodescraper/pluginexecutor.py | 74 ++++++++++++------- .../test_oob_generic_collection_plugin.py | 3 +- 4 files changed, 101 insertions(+), 35 deletions(-) diff --git a/nodescraper/base/oobsshdataplugin.py b/nodescraper/base/oobsshdataplugin.py index 170cd3ff..7a08e17c 100644 --- a/nodescraper/base/oobsshdataplugin.py +++ b/nodescraper/base/oobsshdataplugin.py @@ -23,21 +23,64 @@ # SOFTWARE. # ############################################################################### -from typing import Generic +from __future__ import annotations -from nodescraper.connection.inband import InBandConnectionManager, SSHConnectionParams +from logging import Logger +from typing import Generic, Optional, Union + +from nodescraper.connection.inband.inbandmanager import InBandConnectionManager +from nodescraper.connection.redfish import ( + RedfishConnectionManager, + RedfishConnectionParams, + redfish_params_to_ssh, +) +from nodescraper.enums import EventPriority from nodescraper.generictypes import TAnalyzeArg, TCollectArg, TDataModel from nodescraper.interfaces import DataPlugin +from nodescraper.interfaces.taskresulthook import TaskResultHook +from nodescraper.models import SystemInfo + + +class _OobSshConnectionManager(InBandConnectionManager): + """Internal SSH transport for OOB plugins; uses Redfish BMC credentials.""" + + def __init__( + self, + system_info: SystemInfo, + logger: Optional[Logger] = None, + max_event_priority_level: Union[EventPriority, str] = EventPriority.CRITICAL, + parent: Optional[str] = None, + task_result_hooks: Optional[list[TaskResultHook]] = None, + connection_args: Optional[Union[RedfishConnectionParams, dict]] = None, + **kwargs, + ): + if connection_args is not None: + connection_args = redfish_params_to_ssh(connection_args) + super().__init__( + system_info=system_info, + logger=logger, + max_event_priority_level=max_event_priority_level, + parent=parent, + task_result_hooks=task_result_hooks, + connection_args=connection_args, + **kwargs, + ) class OOBSSHDataPlugin( - DataPlugin[InBandConnectionManager, SSHConnectionParams, TDataModel, TCollectArg, TAnalyzeArg], + DataPlugin[ + RedfishConnectionManager, + RedfishConnectionParams, + TDataModel, + TCollectArg, + TAnalyzeArg, + ], Generic[TDataModel, TCollectArg, TAnalyzeArg], ): - """Base class for out-of-band (OOB) plugins that use BMC SSH. + """Base class for out-of-band (OOB) plugins that run shell commands on the BMC. - Mirrors OOBandDataPlugin but wires InBandConnectionManager so collectors can - inherit from InBandDataCollector and use _run_sut_cmd() unchanged. + Configure the BMC using ``RedfishConnectionManager`` in the connection config. + Commands are executed over SSH (port 22) using the same host/username/password. """ - CONNECTION_TYPE = InBandConnectionManager + CONNECTION_TYPE = RedfishConnectionManager diff --git a/nodescraper/connection/redfish/__init__.py b/nodescraper/connection/redfish/__init__.py index f98faaac..8db61470 100644 --- a/nodescraper/connection/redfish/__init__.py +++ b/nodescraper/connection/redfish/__init__.py @@ -41,6 +41,7 @@ ) from .redfish_params import RedfishConnectionParams from .redfish_path import RedfishPath +from .redfish_ssh import redfish_params_to_ssh __all__ = [ "RedfishConnection", @@ -48,6 +49,7 @@ "RedfishGetResult", "RedfishConnectionManager", "RedfishConnectionParams", + "redfish_params_to_ssh", "RedfishPath", "collect_oem_diagnostic_data", "get_oem_diagnostic_allowable_values", diff --git a/nodescraper/pluginexecutor.py b/nodescraper/pluginexecutor.py index 92a8d770..a53b9c6b 100644 --- a/nodescraper/pluginexecutor.py +++ b/nodescraper/pluginexecutor.py @@ -34,6 +34,7 @@ from pydantic import BaseModel +from nodescraper.base.oobsshdataplugin import OOBSSHDataPlugin, _OobSshConnectionManager from nodescraper.constants import DEFAULT_LOGGER from nodescraper.interfaces import ConnectionManager, DataPlugin, PluginInterface from nodescraper.models import PluginConfig, SystemInfo @@ -81,6 +82,9 @@ def __init__( self.plugin_config = self.merge_configs(plugin_configs) self.connection_library: dict[type[ConnectionManager], ConnectionManager] = {} + self.connection_configs: dict[str, Union[dict, BaseModel]] = ( + dict(connections) if connections else {} + ) self.log_path = log_path @@ -175,40 +179,56 @@ def run_queue(self) -> list[PluginResult]: } if plugin_class.CONNECTION_TYPE: - connection_manager_class: Type[ConnectionManager] = plugin_class.CONNECTION_TYPE - if ( - connection_manager_class.__name__ - in self.plugin_registry.connection_managers - ): - mgr_impl = self.plugin_registry.connection_managers[ - connection_manager_class.__name__ - ] - elif ( - inspect.isclass(connection_manager_class) - and issubclass(connection_manager_class, ConnectionManager) - and not inspect.isabstract(connection_manager_class) - ): - # External packages set CONNECTION_TYPE on the plugin; - # use it when not listed under nodescraper.connection_managers entry points. - mgr_impl = connection_manager_class + if issubclass(plugin_class, OOBSSHDataPlugin): + mgr_impl = _OobSshConnectionManager + connection_args = self.connection_configs.get("RedfishConnectionManager") + if connection_args is None: + self.logger.error( + "%s requires RedfishConnectionManager in the connection config", + plugin_name, + ) + continue else: - self.logger.error( - "Unable to find registered connection manager class for %s that is required by", - connection_manager_class.__name__, + connection_manager_class: Type[ConnectionManager] = ( + plugin_class.CONNECTION_TYPE ) - continue + if ( + connection_manager_class.__name__ + in self.plugin_registry.connection_managers + ): + mgr_impl = self.plugin_registry.connection_managers[ + connection_manager_class.__name__ + ] + elif ( + inspect.isclass(connection_manager_class) + and issubclass(connection_manager_class, ConnectionManager) + and not inspect.isabstract(connection_manager_class) + ): + # External packages set CONNECTION_TYPE on the plugin; + # use it when not listed under nodescraper.connection_managers entry points. + mgr_impl = connection_manager_class + else: + self.logger.error( + "Unable to find registered connection manager class for %s that is required by", + connection_manager_class.__name__, + ) + continue + connection_args = None if mgr_impl not in self.connection_library: self.logger.info( - "Initializing connection manager for %s with default args", + "Initializing connection manager for %s", mgr_impl.__name__, ) - self.connection_library[mgr_impl] = mgr_impl( - system_info=self.system_info, - logger=self.logger, - task_result_hooks=self.connection_result_hooks, - session_id=self.session_id, - ) + init_kwargs = { + "system_info": self.system_info, + "logger": self.logger, + "task_result_hooks": self.connection_result_hooks, + "session_id": self.session_id, + } + if connection_args is not None: + init_kwargs["connection_args"] = connection_args + self.connection_library[mgr_impl] = mgr_impl(**init_kwargs) init_payload["connection_manager"] = self.connection_library[mgr_impl] diff --git a/test/unit/plugin/test_oob_generic_collection_plugin.py b/test/unit/plugin/test_oob_generic_collection_plugin.py index 7c206f29..8f7698cb 100644 --- a/test/unit/plugin/test_oob_generic_collection_plugin.py +++ b/test/unit/plugin/test_oob_generic_collection_plugin.py @@ -25,6 +25,7 @@ ############################################################################### from nodescraper.base import InBandDataPlugin, OOBSSHDataPlugin from nodescraper.connection.inband.inbandmanager import InBandConnectionManager +from nodescraper.connection.redfish import RedfishConnectionManager from nodescraper.pluginregistry import PluginRegistry from nodescraper.plugins.generic_collection import ( GenericAnalyzer, @@ -67,6 +68,6 @@ def test_generic_collection_plugin_uses_inband_base(): def test_oob_generic_collection_plugin_uses_oob_ssh_base(): assert issubclass(OobGenericCollectionPlugin, OOBSSHDataPlugin) assert issubclass(OobGenericCollectionPlugin, GenericCollectionPluginMixin) - assert OobGenericCollectionPlugin.CONNECTION_TYPE is InBandConnectionManager + assert OobGenericCollectionPlugin.CONNECTION_TYPE is RedfishConnectionManager assert GenericCollectionPlugin.COLLECTOR is OobGenericCollectionPlugin.COLLECTOR assert GenericCollectionPlugin.ANALYZER is OobGenericCollectionPlugin.ANALYZER From 9c3a98fc2478f77714a7f21be35752a3f473c200 Mon Sep 17 00:00:00 2001 From: Alexandra Bara Date: Thu, 11 Jun 2026 10:10:05 -0500 Subject: [PATCH 07/13] moved connection to its own file --- nodescraper/base/oobsshdataplugin.py | 34 +--------------------------- nodescraper/pluginexecutor.py | 5 ++-- 2 files changed, 4 insertions(+), 35 deletions(-) diff --git a/nodescraper/base/oobsshdataplugin.py b/nodescraper/base/oobsshdataplugin.py index 7a08e17c..b383d7aa 100644 --- a/nodescraper/base/oobsshdataplugin.py +++ b/nodescraper/base/oobsshdataplugin.py @@ -25,46 +25,14 @@ ############################################################################### from __future__ import annotations -from logging import Logger -from typing import Generic, Optional, Union +from typing import Generic -from nodescraper.connection.inband.inbandmanager import InBandConnectionManager from nodescraper.connection.redfish import ( RedfishConnectionManager, RedfishConnectionParams, - redfish_params_to_ssh, ) -from nodescraper.enums import EventPriority from nodescraper.generictypes import TAnalyzeArg, TCollectArg, TDataModel from nodescraper.interfaces import DataPlugin -from nodescraper.interfaces.taskresulthook import TaskResultHook -from nodescraper.models import SystemInfo - - -class _OobSshConnectionManager(InBandConnectionManager): - """Internal SSH transport for OOB plugins; uses Redfish BMC credentials.""" - - def __init__( - self, - system_info: SystemInfo, - logger: Optional[Logger] = None, - max_event_priority_level: Union[EventPriority, str] = EventPriority.CRITICAL, - parent: Optional[str] = None, - task_result_hooks: Optional[list[TaskResultHook]] = None, - connection_args: Optional[Union[RedfishConnectionParams, dict]] = None, - **kwargs, - ): - if connection_args is not None: - connection_args = redfish_params_to_ssh(connection_args) - super().__init__( - system_info=system_info, - logger=logger, - max_event_priority_level=max_event_priority_level, - parent=parent, - task_result_hooks=task_result_hooks, - connection_args=connection_args, - **kwargs, - ) class OOBSSHDataPlugin( diff --git a/nodescraper/pluginexecutor.py b/nodescraper/pluginexecutor.py index a53b9c6b..4f3febed 100644 --- a/nodescraper/pluginexecutor.py +++ b/nodescraper/pluginexecutor.py @@ -34,7 +34,8 @@ from pydantic import BaseModel -from nodescraper.base.oobsshdataplugin import OOBSSHDataPlugin, _OobSshConnectionManager +from nodescraper.base.oobsshdataplugin import OOBSSHDataPlugin +from nodescraper.connection.oob_ssh import OobSshConnectionManager from nodescraper.constants import DEFAULT_LOGGER from nodescraper.interfaces import ConnectionManager, DataPlugin, PluginInterface from nodescraper.models import PluginConfig, SystemInfo @@ -180,7 +181,7 @@ def run_queue(self) -> list[PluginResult]: if plugin_class.CONNECTION_TYPE: if issubclass(plugin_class, OOBSSHDataPlugin): - mgr_impl = _OobSshConnectionManager + mgr_impl = OobSshConnectionManager connection_args = self.connection_configs.get("RedfishConnectionManager") if connection_args is None: self.logger.error( From fec6f929ec91cc066c34e6ecf866f360abec9a5b Mon Sep 17 00:00:00 2001 From: Alexandra Bara Date: Thu, 11 Jun 2026 12:25:39 -0500 Subject: [PATCH 08/13] added missing function --- nodescraper/connection/redfish/__init__.py | 3 +- .../connection/redfish/redfish_params.py | 29 +++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/nodescraper/connection/redfish/__init__.py b/nodescraper/connection/redfish/__init__.py index 8db61470..12b5af16 100644 --- a/nodescraper/connection/redfish/__init__.py +++ b/nodescraper/connection/redfish/__init__.py @@ -39,9 +39,8 @@ collect_oem_diagnostic_data, get_oem_diagnostic_allowable_values, ) -from .redfish_params import RedfishConnectionParams +from .redfish_params import RedfishConnectionParams, redfish_params_to_ssh from .redfish_path import RedfishPath -from .redfish_ssh import redfish_params_to_ssh __all__ = [ "RedfishConnection", diff --git a/nodescraper/connection/redfish/redfish_params.py b/nodescraper/connection/redfish/redfish_params.py index 7d9b5d5f..4eb70a96 100644 --- a/nodescraper/connection/redfish/redfish_params.py +++ b/nodescraper/connection/redfish/redfish_params.py @@ -23,11 +23,15 @@ # SOFTWARE. # ############################################################################### +from __future__ import annotations + from typing import Optional, Union from pydantic import BaseModel, ConfigDict, Field, SecretStr from pydantic.networks import IPvAnyAddress +from nodescraper.connection.inband.sshparams import SSHConnectionParams + from .redfish_connection import DEFAULT_REDFISH_API_ROOT @@ -51,3 +55,28 @@ class RedfishConnectionParams(BaseModel): default=DEFAULT_REDFISH_API_ROOT, description="Redfish API path (e.g. 'redfish/v1'). Override for a different API version.", ) + + +def redfish_params_to_ssh( + params: Union[RedfishConnectionParams, dict], + *, + ssh_port: Optional[int] = None, + key_filename: Optional[str] = None, +) -> SSHConnectionParams: + """Map Redfish BMC credentials to SSH connection params for shell access.""" + if isinstance(params, dict): + data = dict(params) + ssh_port = data.pop("ssh_port", ssh_port if ssh_port is not None else 22) + key_filename = data.pop("key_filename", key_filename) + params = RedfishConnectionParams.model_validate(data) + else: + if ssh_port is None: + ssh_port = 22 + + return SSHConnectionParams( + hostname=str(params.host), + username=params.username, + password=params.password, + port=ssh_port, + key_filename=key_filename, + ) From 537af53f0bc06fb698093d7c43cad3fb17cede59 Mon Sep 17 00:00:00 2001 From: Alexandra Bara Date: Thu, 11 Jun 2026 14:01:46 -0500 Subject: [PATCH 09/13] added missing folder --- nodescraper/connection/oob_ssh/__init__.py | 28 ++++ .../oob_ssh/oob_ssh_connection_manager.py | 124 ++++++++++++++++++ 2 files changed, 152 insertions(+) create mode 100644 nodescraper/connection/oob_ssh/__init__.py create mode 100644 nodescraper/connection/oob_ssh/oob_ssh_connection_manager.py diff --git a/nodescraper/connection/oob_ssh/__init__.py b/nodescraper/connection/oob_ssh/__init__.py new file mode 100644 index 00000000..15e619da --- /dev/null +++ b/nodescraper/connection/oob_ssh/__init__.py @@ -0,0 +1,28 @@ +############################################################################### +# +# 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 .oob_ssh_connection_manager import OobSshConnectionManager + +__all__ = ["OobSshConnectionManager"] diff --git a/nodescraper/connection/oob_ssh/oob_ssh_connection_manager.py b/nodescraper/connection/oob_ssh/oob_ssh_connection_manager.py new file mode 100644 index 00000000..823c00cb --- /dev/null +++ b/nodescraper/connection/oob_ssh/oob_ssh_connection_manager.py @@ -0,0 +1,124 @@ +############################################################################### +# +# 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 __future__ import annotations + +from logging import Logger +from typing import Optional, Union + +from nodescraper.enums import EventCategory, EventPriority, ExecutionStatus +from nodescraper.interfaces.connectionmanager import ConnectionManager +from nodescraper.interfaces.taskresulthook import TaskResultHook +from nodescraper.models import SystemInfo, TaskResult +from nodescraper.utils import get_exception_traceback + +from ..inband.inband import InBandConnection +from ..inband.inbandremote import RemoteShell, SSHConnectionError +from ..redfish.redfish_params import RedfishConnectionParams, redfish_params_to_ssh + + +class OobSshConnectionManager(ConnectionManager[InBandConnection, RedfishConnectionParams]): + """SSH to the BMC using the same host and credentials as Redfish (OOB shell).""" + + def __init__( + self, + system_info: SystemInfo, + logger: Optional[Logger] = None, + max_event_priority_level: Union[EventPriority, str] = EventPriority.CRITICAL, + parent: Optional[str] = None, + task_result_hooks: Optional[list[TaskResultHook]] = None, + connection_args: Optional[RedfishConnectionParams] = None, + **kwargs, + ): + super().__init__( + system_info, + logger, + max_event_priority_level, + parent, + task_result_hooks, + connection_args, + **kwargs, + ) + + def connect(self) -> TaskResult: + if not self.connection_args: + self._log_event( + category=EventCategory.RUNTIME, + description="No Redfish connection parameters provided for OOB SSH", + priority=EventPriority.CRITICAL, + console_log=True, + ) + self.result.status = ExecutionStatus.EXECUTION_FAILURE + return self.result + + raw = self.connection_args + if isinstance(raw, dict): + params = RedfishConnectionParams.model_validate(raw) + elif isinstance(raw, RedfishConnectionParams): + params = raw + else: + self._log_event( + category=EventCategory.RUNTIME, + description="Redfish connection_args must be dict or RedfishConnectionParams", + priority=EventPriority.CRITICAL, + console_log=True, + ) + self.result.status = ExecutionStatus.EXECUTION_FAILURE + return self.result + + try: + ssh_params = redfish_params_to_ssh(params) + self.logger.info("Initializing OOB SSH to BMC host %s", ssh_params.hostname) + self.connection = RemoteShell(ssh_params) + self.connection.connect_ssh() + except SSHConnectionError as exception: + self._log_event( + category=EventCategory.SSH, + description=str(exception), + priority=EventPriority.CRITICAL, + console_log=True, + ) + self.result.status = ExecutionStatus.EXECUTION_FAILURE + self.connection = None + except Exception as exception: + self._log_event( + category=EventCategory.SSH, + description=f"Exception during OOB SSH: {exception!s}", + data=get_exception_traceback(exception), + priority=EventPriority.CRITICAL, + console_log=True, + ) + self.result.status = ExecutionStatus.EXECUTION_FAILURE + self.connection = None + return self.result + + def disconnect(self) -> None: + conn = self.connection + super().disconnect() + if isinstance(conn, RemoteShell): + try: + conn.client.close() + except Exception: + pass From 6975cf2764ffccabd9c1620a3b3dd94a1a35823c Mon Sep 17 00:00:00 2001 From: Alexandra Bara Date: Thu, 11 Jun 2026 14:13:50 -0500 Subject: [PATCH 10/13] fix for fixtures --- .../generic_collection_plugin_config.json | 150 ++++++++++++++++++ .../test_generic_collection_plugin.py | 10 +- test/functional/test_plugin_configs.py | 3 +- 3 files changed, 156 insertions(+), 7 deletions(-) create mode 100644 test/functional/fixtures/generic_collection_plugin_config.json diff --git a/test/functional/fixtures/generic_collection_plugin_config.json b/test/functional/fixtures/generic_collection_plugin_config.json new file mode 100644 index 00000000..9941393d --- /dev/null +++ b/test/functional/fixtures/generic_collection_plugin_config.json @@ -0,0 +1,150 @@ +{ + "global_args": {}, + "plugins": { + "GenericCollectionPlugin": { + "collection_args": { + "sudo": false, + "include_stdout": true, + "commands": [ + { + "name": "kernel_os", + "command": "uname -s" + }, + { + "name": "messages", + "command": "cat /var/log/messages", + "sudo": true + }, + { + "name": "uid", + "command": "id -u", + "sudo": false + }, + { + "name": "count_eq", + "command": "echo 8" + }, + { + "name": "count_ne", + "command": "echo 7" + }, + { + "name": "count_gt", + "command": "echo 10" + }, + { + "name": "count_gte", + "command": "echo 5" + }, + { + "name": "count_lt", + "command": "echo 3" + }, + { + "name": "count_lte", + "command": "echo 10" + }, + { + "name": "count_capture", + "command": "printf 'count: 42\\n'" + }, + { + "name": "exact_match", + "command": "echo exact-value" + }, + { + "name": "expected_in_check", + "command": "echo beta" + }, + { + "name": "line_count", + "command": "printf 'one\\ntwo\\n'" + }, + { + "name": "optional_fail", + "command": "false" + } + ] + }, + "analysis_args": { + "checks": [ + { + "name": "kernel_os", + "must_contain": "TEST" + }, + { + "name": "messages", + "must_not_contain": "error" + }, + { + "name": "uid", + "expected_regex": "^\\d+$" + }, + { + "name": "count_eq", + "value_type": "int", + "compare_op": "==", + "expected_value": 8 + }, + { + "name": "count_ne", + "value_type": "int", + "compare_op": "!=", + "expected_value": 0 + }, + { + "name": "count_gt", + "value_type": "int", + "compare_op": ">", + "expected_value": 5 + }, + { + "name": "count_gte", + "value_type": "int", + "compare_op": ">=", + "expected_value": 5 + }, + { + "name": "count_lt", + "value_type": "int", + "compare_op": "<", + "expected_value": 10 + }, + { + "name": "count_lte", + "value_type": "int", + "compare_op": "<=", + "expected_value": 10 + }, + { + "name": "count_capture", + "value_type": "int", + "compare_op": "==", + "expected_value": 42, + "capture_regex": "count:\\s+(\\d+)" + }, + { + "name": "exact_match", + "expected": "exact-value" + }, + { + "name": "expected_in_check", + "expected_in": ["alpha", "beta", "gamma"] + }, + { + "name": "line_count", + "exact_lines": 2 + }, + { + "name": "optional_fail", + "allow_failure": true, + "expected_exit_code": 1 + } + ] + } + } + }, + "result_collators": {}, + "name": "GenericCollectionPlugin config", + "desc": "Demo config: per-command sudo, text checks, all compare_op values, and optional allow_failure" +} diff --git a/test/functional/test_generic_collection_plugin.py b/test/functional/test_generic_collection_plugin.py index 3bc52747..3f37bc1b 100644 --- a/test/functional/test_generic_collection_plugin.py +++ b/test/functional/test_generic_collection_plugin.py @@ -30,13 +30,13 @@ import pytest -REPO_ROOT = Path(__file__).resolve().parent.parent.parent +_FIXTURES = Path(__file__).resolve().parent / "fixtures" @pytest.fixture def generic_collection_config_file(): - """Return path to GenericCollectionPlugin config at repo root.""" - return REPO_ROOT / "generic_collection_plugin_config.json" + """Return path to GenericCollectionPlugin demo config (functional fixtures).""" + return _FIXTURES / "generic_collection_plugin_config.json" def test_generic_collection_plugin_with_config_file( @@ -68,7 +68,7 @@ def test_generic_collection_plugin_with_config_file( def test_generic_collection_plugin_demo_config_runs_end_to_end( run_cli_command, generic_collection_config_file, tmp_path ): - """Repo-root demo config runs collection and analysis with expected partial failures.""" + """Demo fixture config runs collection and analysis with expected partial failures.""" log_path = str(tmp_path / "logs_generic_collection_csv") result = run_cli_command( [ @@ -107,7 +107,7 @@ def test_generic_collection_plugin_demo_config_runs_end_to_end( def test_generic_collection_plugin_runs_analyzer( run_cli_command, generic_collection_config_file, tmp_path ): - """Analysis checks from the repo-root config should run after collection.""" + """Analysis checks from the demo fixture config should run after collection.""" log_path = str(tmp_path / "logs_generic_collection_analysis") result = run_cli_command( [ diff --git a/test/functional/test_plugin_configs.py b/test/functional/test_plugin_configs.py index ed8bc979..ce06b057 100644 --- a/test/functional/test_plugin_configs.py +++ b/test/functional/test_plugin_configs.py @@ -41,7 +41,6 @@ def fixtures_dir(): @pytest.fixture def plugin_config_files(fixtures_dir): """Return dict mapping plugin names to their config file paths.""" - repo_root = Path(__file__).resolve().parent.parent.parent return { "AmdSmiPlugin": fixtures_dir / "amdsmi_plugin_config.json", "BiosPlugin": fixtures_dir / "bios_plugin_config.json", @@ -49,7 +48,7 @@ def plugin_config_files(fixtures_dir): "DimmPlugin": fixtures_dir / "dimm_plugin_config.json", "DkmsPlugin": fixtures_dir / "dkms_plugin_config.json", "DmesgPlugin": fixtures_dir / "dmesg_plugin_config.json", - "GenericCollectionPlugin": repo_root / "generic_collection_plugin_config.json", + "GenericCollectionPlugin": fixtures_dir / "generic_collection_plugin_config.json", "JournalPlugin": fixtures_dir / "journal_plugin_config.json", "KernelPlugin": fixtures_dir / "kernel_plugin_config.json", "KernelModulePlugin": fixtures_dir / "kernel_module_plugin_config.json", From 97dfa1c527e89b9f03b1e5b80bd0175057826bf7 Mon Sep 17 00:00:00 2001 From: Alexandra Bara Date: Thu, 11 Jun 2026 14:29:48 -0500 Subject: [PATCH 11/13] fixed for running on unknown sku --- .../plugins/generic_collection/generic_collection_collector.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/nodescraper/plugins/generic_collection/generic_collection_collector.py b/nodescraper/plugins/generic_collection/generic_collection_collector.py index e2a3e0b7..873f572a 100644 --- a/nodescraper/plugins/generic_collection/generic_collection_collector.py +++ b/nodescraper/plugins/generic_collection/generic_collection_collector.py @@ -26,7 +26,7 @@ from typing import Optional from nodescraper.base import InBandDataCollector -from nodescraper.enums import EventCategory, EventPriority, ExecutionStatus +from nodescraper.enums import EventCategory, EventPriority, ExecutionStatus, OSFamily from nodescraper.models import TaskResult from .collector_args import GenericCollectionCollectorArgs @@ -39,6 +39,7 @@ class GenericCollectionCollector( """Run user-configured shell commands and report per-command success.""" DATA_MODEL = GenericCollectionDataModel + SUPPORTED_OS_FAMILY: set[OSFamily] = {OSFamily.WINDOWS, OSFamily.LINUX, OSFamily.UNKNOWN} def collect_data( self, args: Optional[GenericCollectionCollectorArgs] = None From c93fdbd68a85b51ae2e9958ea241bf46b7855cf9 Mon Sep 17 00:00:00 2001 From: Alexandra Bara Date: Thu, 11 Jun 2026 15:37:04 -0500 Subject: [PATCH 12/13] updated description for fields --- .../generic_collection/collector_args.py | 37 +++++++++++++++++-- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/nodescraper/plugins/generic_collection/collector_args.py b/nodescraper/plugins/generic_collection/collector_args.py index fb0015d2..fae58edf 100644 --- a/nodescraper/plugins/generic_collection/collector_args.py +++ b/nodescraper/plugins/generic_collection/collector_args.py @@ -34,7 +34,15 @@ class CommandSpec(CollectorArgs): """One named shell command and optional per-command overrides.""" name: str = Field(description="Stable name for this command, used by analysis checks.") - command: str = Field(description="Shell command to run on the target system.") + command: str = Field( + description=( + "Shell command to run on the target (non-interactive SSH exec). " + "Intended for small, text-oriented output. Avoid commands that stream large or " + "binary data to stdout (e.g. ``tar czf - ``): stdout is read fully into " + "memory and decoded as UTF-8 when captured. For BMC directory archives over SSH, " + "use ``OobBmcArchivePlugin`` instead." + ), + ) sudo: Optional[bool] = Field( default=None, description="Run with sudo. When omitted, uses collection_args.sudo.", @@ -46,7 +54,11 @@ class CommandSpec(CollectorArgs): ) include_stdout: Optional[bool] = Field( default=None, - description="Store stdout in the data model. When omitted, uses collection_args.include_stdout.", + description=( + "Store stdout in the data model. When omitted, uses collection_args.include_stdout. " + "When false, stdout is omitted from stored results (the SSH layer may still read it " + "for that command)." + ), ) @field_validator("name", "command", mode="before") @@ -66,9 +78,22 @@ def _validate_required_fields(self) -> "CommandSpec": class GenericCollectionCollectorArgs(CollectorArgs): + """Arguments for :class:`GenericCollectionCollector` / ``GenericCollectionPlugin``. + + Commands are run over SSH; **stdout and stderr are read fully into memory** on the + controller and decoded as UTF-8 (with replacement) when stdout is captured. This plugin + is meant for **short, text-oriented diagnostics** (logs, version strings, small file + snippets). It is **not** appropriate for large or binary streams—for example + ``tar czf - ``, which writes a tarball to stdout: you risk huge memory use, timeouts, + and corrupt binary data. For archiving BMC paths over SSH, use **``OobBmcArchivePlugin``**. + """ + commands: list[CommandSpec] = Field( default_factory=list, - description="Named commands to run. Each entry must include 'name' and 'command'.", + description=( + "Named commands to run. Each entry must include 'name' and 'command'. " + "Prefer small textual stdout; see class docstring for streaming/binary limitations." + ), ) sudo: bool = Field( default=False, @@ -81,7 +106,11 @@ class GenericCollectionCollectorArgs(CollectorArgs): ) include_stdout: bool = Field( default=True, - description="Default setting for storing stdout in the data model for analysis.", + description=( + "Default: include each command's stdout in collected results for analysis. " + "When false, stdout is omitted from stored results (not a substitute for avoiding " + "large/binary streams; see class docstring)." + ), ) @field_validator("commands", mode="before") From 042328b6efebfccd3ac8a028f7d22451740f1796 Mon Sep 17 00:00:00 2001 From: Alexandra Bara Date: Thu, 11 Jun 2026 17:29:29 -0500 Subject: [PATCH 13/13] added missing license --- .../plugins/generic_collection/__init__.py | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/nodescraper/plugins/generic_collection/__init__.py b/nodescraper/plugins/generic_collection/__init__.py index c2cdee75..06dbda9f 100644 --- a/nodescraper/plugins/generic_collection/__init__.py +++ b/nodescraper/plugins/generic_collection/__init__.py @@ -1,3 +1,28 @@ +############################################################################### +# +# 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. +# +############################################################################### """Generic command collection plugins (in-band and OOB SSH).""" from .analyzer_args import CommandCheck, GenericAnalyzerArgs