diff --git a/README.md b/README.md index 52bdfbef..94702804 100644 --- a/README.md +++ b/README.md @@ -269,17 +269,21 @@ Redfish plugins can collect from multiple BMCs concurrently. In-band plugins and } ``` -Multi-target mode applies to Redfish plugins (`RedfishEndpointPlugin`, `RedfishOemDiagPlugin`, and other plugins based on `OOBandDataPlugin`). Targets are collected concurrently. Wall-clock time follows the slowest target. +List each Redfish plugin once. Put the hosts on that plugin's connection manager `targets` list. `RedfishConnectionManager` is the direct BMC session used by `RedfishEndpointPlugin`, `RedfishInventoryPlugin`, and `RedfishOemDiagPlugin`. `RedfishSshProxyConnectionManager` is the SSH proxy used by `AmcRedfishInventoryPlugin`, `AmcRedfishEndpointPlugin`, and `AmcRedfishDiagPlugin`. Those two managers are separate lists, so a plugin compares and collects only the targets it connected. -A target that fails to connect or collect does not fail the run when another target succeeds. That plugin result is a warning, and analysis still runs for the targets that returned data. The run fails when every target fails, or when analysis of collected data reports an error. +Targets are collected concurrently. Connect failures stay on the connection result. The plugin result shows collection and analysis for the targets that connected, and the run fails when every target fails to connect or analysis reports an error. - `targets`: list of per-target connection parameters. Each entry accepts the same fields as the single-target config plus an optional `target_key` (used as the result key; defaults to the host string). - `max_workers` (optional): maximum concurrent collection threads. Defaults to `min(len(targets), 32)` and is capped at 32. -Per-target results are written to `///`. Single-target results stay in `//`. The same layout is used for the analyzer directory and for the AMC SSH-proxy plugin. +Per-target results are written to `///`. Single-target results stay in `//`. The same layout is used for the analyzer directory and for the AMC SSH-proxy plugin. Inventory collection writes `redfishinventorydatamodel.json` in the collector directory. A ready-to-edit sample is in `config/connection-config_redfish_multi_target.example.json`. Replace the example hosts and password, then pass it with `--connection-config`. +**Firmware comparison:** `RedfishInventoryPlugin` and `AmcRedfishInventoryPlugin` compare `FirmwareInventory` across the targets that plugin connected. `analysis_args.compare_firmware` defaults to `true`. Each member is compared by its published `Id`. A missing or empty `Version` is recorded as `N/A`. The warning also records `Status.Health`, `Status.State`, `SoftwareId`, `Updateable`, and `Oem.SVN`. A different version, or a member present on one connected target and absent on another, logs one `FW` warning per component. BMC firmware is not compared with AMC firmware. A target that failed to connect is excluded. One connected target does not warn. Set `compare_firmware` to `false` to skip the check. + +The console summary reports the warning count. Each component is in the analyzer `events.json`, and `result.json` has the summary. Collection profiles and the other inventory switches are in the [plugin reference](docs/PLUGIN_DOC.md). + **Notes:** - If using SSH keys, specify `key_filename` instead of `password`. - The remote user must have permissions to run the requested plugins and access required files. If needed, use the `--skip-sudo` argument to skip plugins requiring sudo. diff --git a/docs/PLUGIN_DOC.md b/docs/PLUGIN_DOC.md index ebac0133..34489431 100644 --- a/docs/PLUGIN_DOC.md +++ b/docs/PLUGIN_DOC.md @@ -44,6 +44,8 @@ | OobGenericCollectionPlugin | Runs each command from collection_args.commands on the target (in-band host or BMC over OOB SSH).
Commands are user-configured; there are no fixed CMD_\* class fields. | **Analyzer Args:**
- `checks`: list[nodescraper.plugins.generic_collection.analyzer_args.CommandCheck] — Per-command validation rules keyed by collected command name. | **Collection Args:**
- `html_view`: bool — When true, include logged command artifacts in command_artifacts.html using human-readable output.
- `commands`: list[nodescraper.plugins.generic_collection.collector_args.CommandSpec] — Named commands to run. Each entry must include 'name' and 'command'. Prefer small textual stdout; see class docstring...
- `sudo`: bool — Default sudo setting for commands that do not specify sudo.
- `timeout`: int — Default per-command timeout in seconds.
- `include_stdout`: bool — Default: include each command's stdout in collected results for analysis. When false, stdout is omitted from stored r... | [GenericCollectionDataModel](#GenericCollectionDataModel-Model) | [GenericCollectionCollector](#Collector-Class-GenericCollectionCollector) | [GenericAnalyzer](#Data-Analyzer-Class-GenericAnalyzer) | | OobBmcArchivePlugin | SSH (BMC) shell: tar+gzip archives for each path in collection_args (see PathSpec entries).
Uses sudo on the BMC when collection_args paths require elevated access. | - | **Collection Args:**
- `html_view`: bool — When true, include logged command artifacts in command_artifacts.html using human-readable output.
- `paths`: list[nodescraper.plugins.ooband.bmc_archive.collector_args.PathSpec] — Named BMC paths to archive with tar czf -. Configure in plugin config under plugins.OobBmcArchivePlugin.collection_ar...
- `sudo`: bool — Default sudo setting for paths that do not specify sudo.
- `timeout`: int — Default per-path tar timeout in seconds.
- `skip_if_missing`: bool — Skip paths that do not exist on the BMC instead of failing collection.
- `ignore_failed_read`: bool — When true, pass GNU tar's --ignore-failed-read when the remote tar supports it. | [BmcArchiveDataModel](#BmcArchiveDataModel-Model) | [BmcArchiveCollector](#Collector-Class-BmcArchiveCollector) | - | | RedfishEndpointPlugin | Redfish GET: explicit paths from collection_args.uris (parallel when max_workers>1).
Optional paged GET following the Members collection OData nextLink field when follow_next_link is true.
Redfish GET tree: when discover_tree is true, walks from api_root using OData resource id links and Members navigation (depth and endpoint caps from collection_args). | For each entry in analysis_args.checks, reads JSON paths in collected responses and compares values to constraints (eq, min/max, anyOf, regex, etc.).
URI key `*` runs checks against every collected response body.
**Analyzer Args:**
- `checks`: dict[str, dict[str, int | float | str | bool | dict[str, Any]]] — Map: URI or `*` -> { property_path: constraint }. URI keys must match a key in the collected responses (exact match).... | **Collection Args:**
- `html_view`: bool — When true, include logged command artifacts in command_artifacts.html using human-readable output.
- `uris`: list[str] — Redfish URIs to GET. Ignored when discover_tree is True.
- `discover_tree`: bool — If True, discover endpoints from the BMC Redfish tree (service root and links) instead of using uris.
- `tree_max_depth`: int — When discover_tree is True: max traversal depth (1=service root only, 2=root + collections, 3=+ members).
- `tree_max_endpoints`: int — When discover_tree is True: max endpoints to discover (0=no limit).
- `max_workers`: int — Max concurrent GETs (1=sequential). Use >1 for async endpoint fetches.
- `follow_next_link`: bool — If True, follow Redfish Members collection OData nextLink pagination for each URI and merge all pages into a single r...
- `max_pages`: int — When follow_next_link is True: safety cap on the number of pages to follow per URI (default 200).
- `clear_log_uris`: list[str] — Redfish LogService.ClearLog action URIs to POST {} to after successful collection. Each URI is posted with an empty J... | [RedfishEndpointDataModel](#RedfishEndpointDataModel-Model) | [RedfishEndpointCollector](#Collector-Class-RedfishEndpointCollector) | [RedfishEndpointAnalyzer](#Data-Analyzer-Class-RedfishEndpointAnalyzer) | +| AmcRedfishInventoryPlugin | GET the Redfish service root and follow standard collection links such as Chassis, Managers, Systems, TelemetryService, and UpdateService.
Expand collection members and follow Assembly, PowerSubsystem, FruDevices, Memory, Processors, FirmwareInventory, and metric-report links when the service publishes them.
collection_args.collections and collection_args.member_links append extra property names to that profile set. Names already in the set are ignored.
Uses the active Redfish connection, including a direct session or the SSH proxy.
An SSH-proxy HTTP 404 for a listed collection member is an empty slot and is not a failure. | analysis_args.compare_firmware defaults to true and compares FirmwareInventory across the targets this plugin connected.
RedfishInventoryPlugin compares its direct Redfish targets. AmcRedfishInventoryPlugin compares its SSH-proxy targets. The two are not mixed.
A different Version, a member missing on a connected target, or a missing or empty Version (recorded as N/A) logs one FW warning per component.
Each warning records Version, Status Health, Status State, SoftwareId, Updateable, and Oem SVN. The component id is the published Id and is not normalized.
Warnings are written to the analyzer events.json. result.json has the summary count. The console summary is that count, not the per-component table.
Fewer than two connected targets, a target that failed to connect, or compare_firmware false does not warn. --data loads one model and does not compare nodes.
When no FirmwareInventory members were collected, one warning says the comparison did not run.
**Analyzer Args:**
- `compare_firmware`: bool — Compare FirmwareInventory across this plugin's connected targets. Default true. A different Version, a member missing... | **Collection Args:**
- `html_view`: bool — When true, include logged command artifacts in command_artifacts.html using human-readable output.
- `profile`: Literal['bmc', 'smc', 'all'] — bmc reads chassis, managers, telemetry, firmware inventory, and systems. smc reads systems, memory, processors, and f...
- `collections`: list[str] — Extra service-root property names appended to the profile set. Duplicates are ignored. Empty leaves the profile uncha...
- `member_links`: list[str] — Extra property names appended to the profile link set. Duplicates are ignored. Empty leaves the profile unchanged.
- `max_pages`: int — Maximum pages to keep for each collection, including the first page.
- `enabled`: dict[str, bool] — Turn inventory areas on or off. Omitted areas stay at the profile default, and the default profile enables every area... | [RedfishInventoryDataModel](#RedfishInventoryDataModel-Model) | [RedfishInventoryCollector](#Collector-Class-RedfishInventoryCollector) | [RedfishInventoryAnalyzer](#Data-Analyzer-Class-RedfishInventoryAnalyzer) | +| RedfishInventoryPlugin | GET the Redfish service root and follow standard collection links such as Chassis, Managers, Systems, TelemetryService, and UpdateService.
Expand collection members and follow Assembly, PowerSubsystem, FruDevices, Memory, Processors, FirmwareInventory, and metric-report links when the service publishes them.
collection_args.collections and collection_args.member_links append extra property names to that profile set. Names already in the set are ignored.
Uses the active Redfish connection, including a direct session or the SSH proxy.
An SSH-proxy HTTP 404 for a listed collection member is an empty slot and is not a failure. | analysis_args.compare_firmware defaults to true and compares FirmwareInventory across the targets this plugin connected.
RedfishInventoryPlugin compares its direct Redfish targets. AmcRedfishInventoryPlugin compares its SSH-proxy targets. The two are not mixed.
A different Version, a member missing on a connected target, or a missing or empty Version (recorded as N/A) logs one FW warning per component.
Each warning records Version, Status Health, Status State, SoftwareId, Updateable, and Oem SVN. The component id is the published Id and is not normalized.
Warnings are written to the analyzer events.json. result.json has the summary count. The console summary is that count, not the per-component table.
Fewer than two connected targets, a target that failed to connect, or compare_firmware false does not warn. --data loads one model and does not compare nodes.
When no FirmwareInventory members were collected, one warning says the comparison did not run.
**Analyzer Args:**
- `compare_firmware`: bool — Compare FirmwareInventory across this plugin's connected targets. Default true. A different Version, a member missing... | **Collection Args:**
- `html_view`: bool — When true, include logged command artifacts in command_artifacts.html using human-readable output.
- `profile`: Literal['bmc', 'smc', 'all'] — bmc reads chassis, managers, telemetry, firmware inventory, and systems. smc reads systems, memory, processors, and f...
- `collections`: list[str] — Extra service-root property names appended to the profile set. Duplicates are ignored. Empty leaves the profile uncha...
- `member_links`: list[str] — Extra property names appended to the profile link set. Duplicates are ignored. Empty leaves the profile unchanged.
- `max_pages`: int — Maximum pages to keep for each collection, including the first page.
- `enabled`: dict[str, bool] — Turn inventory areas on or off. Omitted areas stay at the profile default, and the default profile enables every area... | [RedfishInventoryDataModel](#RedfishInventoryDataModel-Model) | [RedfishInventoryCollector](#Collector-Class-RedfishInventoryCollector) | [RedfishInventoryAnalyzer](#Data-Analyzer-Class-RedfishInventoryAnalyzer) | | RedfishOemDiagPlugin | Redfish LogService.CollectDiagnosticData for each entry in collection_args.oem_diagnostic_types (collection_args.log_service_path selects the LogService).
Optional binary archives under the plugin log path when log_path is set. | Summarizes success/failure per OEM diagnostic type from collected results.
When analysis_args.require_all_success is true, fails the run if any type failed collection.
**Analyzer Args:**
- `require_all_success`: bool — If True, analysis fails when any OEM type collection failed. | **Collection Args:**
- `html_view`: bool — When true, include logged command artifacts in command_artifacts.html using human-readable output.
- `log_service_path`: str — Redfish path to the LogService (e.g. DiagLogs).
- `oem_diagnostic_types_allowable`: list[str] | None — Allowable OEM diagnostic types for this architecture/BMC. When set, used for validation and as default for oem_diagno...
- `oem_diagnostic_types`: list[str] — OEM diagnostic types to collect. When empty and oem_diagnostic_types_allowable is set, defaults to that list.
- `task_timeout_s`: int — Max seconds to wait for each BMC task.
- `clear_logs_after_collection`: bool — If True, POST LogService.ClearLog to the configured log_service_path after a successful collection. Requires system_i... | [RedfishOemDiagDataModel](#RedfishOemDiagDataModel-Model) | [RedfishOemDiagCollector](#Collector-Class-RedfishOemDiagCollector) | [RedfishOemDiagAnalyzer](#Data-Analyzer-Class-RedfishOemDiagAnalyzer) | | ServiceabilityPluginMI3XX | Redfish GET: BMC event log Entries (collection_args.rf_event_log_uri; optional uri alias).
Paginated Members collection and optional top, reference_time/time_operator filters.
Redfish GET: CPER AdditionalDataURI binaries for DiagnosticDataType=CPER events (base64 in data model).
Optional chassis Assembly GETs (rf_assembly_uri_template + rf_chassis_devices).
Optional firmware bundle inventory GET (rf_firmware_bundle_uri) for component details. | Builds AFID events from collected Redfish event log members (and optional assembly metadata).
Optionally decodes CPER attachments via analysis_args.cper_decode_module before hub analysis.
Runs the configured Python service hub (hub_python_module) to produce service recommendations.
When analysis_args.skip_hub is true, only builds AFID events without running the hub.
**Analyzer Args:**
- `hub_python_module`: str | None — Import path for the hub module (class implements hub_analyze_method); hub_options forwards kwargs.
- `hub_entry_point`: str | None — Registered hub entry point name when hub_python_module is omitted (required in analysis_args).
- `hub_raise_on_error`: bool — When True, entry-point hub analyze() exceptions raise instead of status:error.
- `hub_prefer_rf_events`: bool — When True and Redfish rf_events are available without decoded CPER data, pass raw rf_events to the entry-point hub fo...
- `hub_display_name`: str | None — Optional label for analyzer status messages.
- `afid_sag_path`: str | None — Path to AFID_SAG.json. When omitted, uses /opt/amd/afid/AFID_SAG.json.
- `hub_init_path_kwarg`: str — Hub __init__ keyword that receives afid_sag_path.
- `hub_analyze_method`: str — Hub method called with rf_events first (default get_service_info).
- `skip_hub`: bool — If True, only build afid_events without running the service hub.
- `cper_decode_module`: str | None — Module import path for CPER decoding when events include CPER attachments.
- `cper_decode_method`: str — Callable on cper_decode_module: file-like CPER in, (return_code, decode_dict) out.
- `hub_options`: dict[str, Any] | None — Extra kwargs for hub __init__ and analyze; collected cper_data overrides cper_data key.
- `from_ac_cycle`: int — from_ac_cycle kwarg for the hub analyze call (merged after hub_options).
- `from_date`: str | None — Optional from_date for the hub analyze call (merged after hub_options).
- `designation_serials`: dict[str, str] | None — Optional designation_serials for the hub analyze call (merged after hub_options).
- `suppress_service_actions`: list[str] | None — Optional suppress_service_actions for the hub analyze call (merged after hub_options). | **Collection Args:**
- `html_view`: bool — When true, include logged command artifacts in command_artifacts.html using human-readable output.
- `uri`: str | None — Optional alias for ``rf_event_log_uri``. When both ``uri`` and ``rf_event_log_uri`` are explicitly set to non-empty v...
- `rf_event_log_uri`: str — Redfish URI for the event log ``Entries`` collection.
- `rf_chassis_devices`: List[str] | None — Chassis designations for Assembly GETs; required with ``rf_assembly_uri_template``.
- `rf_assembly_uri_template`: str | None — Redfish URI template containing ``{device}`` for each chassis Assembly resource.
- `rf_firmware_bundle_uri`: str | None — Redfish URI for firmware bundle inventory when subclasses extract component details.
- `follow_next_link`: bool — If True, follow Members@odata.nextLink up to max_pages; else single GET.
- `max_pages`: int — Safety cap on the number of pages when following event log pagination.
- `top`: int | None — Most recent N entries via $skip after count probe; None collects full window.
- `reference_time`: str | None — Optional ISO-8601 date or date-time used with time_operator (e.g. 2026-05-17 or 2026-05-17T13:01:00).
- `time_operator`: Literal['>', '>=', '<', '<=', '=='] | None — Comparison operator applied when reference_time is set. | [ServiceabilityDataModel](#ServiceabilityDataModel-Model) | [MI3XXCollector](#Collector-Class-MI3XXCollector) | [MI3XXAnalyzer](#Data-Analyzer-Class-MI3XXAnalyzer) | | Mi4xxServiceabilityPlugin | Redfish GET: Instinct accelerator event log Entries (collection_args.rf_event_log_uri).
MI4xx-only pagination: follows Members@odata.nextLink and falls back to $skip when the BMC reports more entries than one page returns.
Paginated Members collection and optional top, reference_time/time_operator filters.
Optional chassis Assembly GETs (rf_assembly_uri_template + rf_chassis_devices).
Optional firmware bundle inventory GET (rf_firmware_bundle_uri) for component details.
Optional AFID_SAG-backed FRU grouping summary when collection_args.afid_sag_path is set. | Builds AFID events from collected Redfish event log members (and optional assembly metadata).
Runs the configured entry-point service hub (hub_entry_point in analysis_args) to produce service recommendations.
When analysis_args.skip_hub is true, only builds AFID events without running the hub.
Supports offline analysis from a prior collection via --data with --collection False.
**Analyzer Args:**
- `hub_python_module`: str | None — Not used for MI4XX; Hub is selected via hub_entry_point afse.
- `hub_entry_point`: str — Registered Hub entry point name (default afse).
- `hub_raise_on_error`: bool — When True, entry-point hub analyze() exceptions raise instead of status:error.
- `hub_prefer_rf_events`: bool — When True and Redfish rf_events are available without decoded CPER data, pass raw rf_events to the entry-point hub fo...
- `hub_display_name`: str | None — Label for analyzer status messages.
- `afid_sag_path`: str | None — Path to AFID_SAG.json. When omitted, uses /opt/amd/afid/AFID_SAG.json.
- `hub_init_path_kwarg`: str — Hub __init__ keyword that receives afid_sag_path.
- `hub_analyze_method`: str — Hub method called with rf_events first (default get_service_info).
- `skip_hub`: bool — If True, only build afid_events without running the service hub.
- `cper_decode_module`: str | None — Module import path for CPER decoding when events include CPER attachments.
- `cper_decode_method`: str — Callable on cper_decode_module: file-like CPER in, (return_code, decode_dict) out.
- `hub_options`: dict[str, Any] | None — Extra kwargs for hub __init__ and analyze; collected cper_data overrides cper_data key.
- `from_ac_cycle`: int — from_ac_cycle kwarg for the hub analyze call (merged after hub_options).
- `from_date`: str | None — Optional from_date for the hub analyze call (merged after hub_options).
- `designation_serials`: dict[str, str] | None — Optional designation_serials for the hub analyze call (merged after hub_options).
- `suppress_service_actions`: list[str] | None — Optional suppress_service_actions for the hub analyze call (merged after hub_options).
- `rf_event_log_uri`: str — Redfish URI for the Instinct accelerator event log Entries collection. | **Collection Args:**
- `html_view`: bool — When true, include logged command artifacts in command_artifacts.html using human-readable output.
- `rf_event_log_uri`: str — Redfish URI for the Instinct accelerator event log Entries collection.
- `follow_next_link`: bool — If True, follow Members@odata.nextLink up to max_pages; else single GET.
- `max_pages`: int — Safety cap on the number of pages when following event log pagination.
- `top`: int | None — Most recent N entries via $skip after count probe; None collects full window.
- `reference_time`: str | None — Optional ISO-8601 date or date-time used with time_operator (e.g. 2026-05-17 or 2026-05-17T13:01:00).
- `time_operator`: Literal['>', '>=', '<', '<=', '=='] | None — Comparison operator applied when reference_time is set.
- `rf_assembly_uri_template`: str | None — Optional Redfish URI template containing {device} for chassis Assembly GETs.
- `rf_chassis_devices`: List[str] | None — Optional chassis designations paired with rf_assembly_uri_template.
- `rf_firmware_bundle_uri`: str | None — Optional Redfish URI for firmware bundle inventory.
- `afid_sag_path`: str | None — Optional AFID_SAG.json path for collector FRU grouping logs. When omitted, FRU summary logging is skipped. | [ServiceabilityDataModel](#ServiceabilityDataModel-Model) | [MI4XXCollector](#Collector-Class-MI4XXCollector) | [MI4XXAnalyzer](#Data-Analyzer-Class-MI4XXAnalyzer) | @@ -1671,6 +1673,28 @@ RedfishEndpointDataModel - Optional paged GET following the Members collection OData nextLink field when follow_next_link is true. - Redfish GET tree: when discover_tree is true, walks from api_root using OData resource id links and Members navigation (depth and endpoint caps from collection_args). +## Collector Class RedfishInventoryCollector + +### Description + +Collect standard Redfish inventory resources without product-specific validation. + +**Bases**: ['RedfishDataCollector'] + +**Link to code**: [inventory_collector.py](https://github.com/amd/node-scraper/blob/HEAD/nodescraper/plugins/ooband/redfish_inventory/inventory_collector.py) + +### Provides Data + +RedfishInventoryDataModel + +### Documented collection + +- GET the Redfish service root and follow standard collection links such as Chassis, Managers, Systems, TelemetryService, and UpdateService. +- Expand collection members and follow Assembly, PowerSubsystem, FruDevices, Memory, Processors, FirmwareInventory, and metric-report links when the service publishes them. +- collection_args.collections and collection_args.member_links append extra property names to that profile set. Names already in the set are ignored. +- Uses the active Redfish connection, including a direct session or the SSH proxy. +- An SSH-proxy HTTP 404 for a listed collection member is an empty slot and is not a failure. + ## Collector Class RedfishOemDiagCollector ### Description @@ -2320,6 +2344,20 @@ Collected Redfish endpoint responses: URI -> JSON body. - **responses**: `dict[str, dict]` +## RedfishInventoryDataModel Model + +### Description + +Collected standard Redfish inventory responses, keyed by resource path. + +**Link to code**: [inventory_data.py](https://github.com/amd/node-scraper/blob/HEAD/nodescraper/plugins/ooband/redfish_inventory/inventory_data.py) + +**Bases**: ['DataModel'] + +### Model annotations and fields + +- **responses**: `dict[str, dict]` + ## RedfishOemDiagDataModel Model ### Description @@ -2846,6 +2884,26 @@ Checks Redfish endpoint responses against configured thresholds and key/value ru - For each entry in analysis_args.checks, reads JSON paths in collected responses and compares values to constraints (eq, min/max, anyOf, regex, etc.). - URI key `*` runs checks against every collected response body. +## Data Analyzer Class RedfishInventoryAnalyzer + +### Description + +Compare FirmwareInventory members across the targets this plugin collected. + +**Bases**: ['DataAnalyzer'] + +**Link to code**: [inventory_analyzer.py](https://github.com/amd/node-scraper/blob/HEAD/nodescraper/plugins/ooband/redfish_inventory/inventory_analyzer.py) + +### Documented analysis + +- analysis_args.compare_firmware defaults to true and compares FirmwareInventory across the targets this plugin connected. +- RedfishInventoryPlugin compares its direct Redfish targets. AmcRedfishInventoryPlugin compares its SSH-proxy targets. The two are not mixed. +- A different Version, a member missing on a connected target, or a missing or empty Version (recorded as N/A) logs one FW warning per component. +- Each warning records Version, Status Health, Status State, SoftwareId, Updateable, and Oem SVN. The component id is the published Id and is not normalized. +- Warnings are written to the analyzer events.json. result.json has the summary count. The console summary is that count, not the per-component table. +- Fewer than two connected targets, a target that failed to connect, or compare_firmware false does not warn. --data loads one model and does not compare nodes. +- When no FirmwareInventory members were collected, one warning says the comparison did not run. + ## Data Analyzer Class RedfishOemDiagAnalyzer ### Description @@ -3315,6 +3373,20 @@ Analyzer args for config-driven Redfish checks. - **checks**: `dict[str, dict[str, int | float | str | bool | dict[str, Any]]]` — Map: URI or `*` -> { property_path: constraint }. URI keys must match a key in the collected responses (exact match). Use `*` as the key to apply the inner constraints to every collected response body. Property paths use '/' for nesting and indices, e.g. 'Status/Health', 'PowerControl/0/PowerConsumedWatts'. Constraints: 'eq' — value must equal the given literal (int, float, str, bool). 'min' — value must be numeric and >= the given number. 'max' — value must be numeric and <= the given number. 'anyOf' — value must be in the given list (OR; any match passes). Example: { "/redfish/v1/Systems/1": { "Status/Health": { "anyOf": ["OK", "Warning"] }, "PowerState": "On" }, "\*": { "Status/Health": { "anyOf": ["OK"] } } }. +## Analyzer Args Class RedfishInventoryAnalyzerArgs + +### Description + +Analyzer args for comparing Redfish firmware inventory across targets. + +**Bases**: ['AnalyzerArgs'] + +**Link to code**: [analyzer_args.py](https://github.com/amd/node-scraper/blob/HEAD/nodescraper/plugins/ooband/redfish_inventory/analyzer_args.py) + +### Annotations / fields + +- **compare_firmware**: `bool` — Compare FirmwareInventory across this plugin's connected targets. Default true. A different Version, a member missing on a connected target, or a missing Version (recorded as N/A) logs one FW warning per component. The warning records Version, Status Health, Status State, SoftwareId, Updateable, and Oem SVN. Ids are compared as published. One connected target, a failed connection, or false skips the comparison. --data loads one model and does not compare nodes. + ## Analyzer Args Class RedfishOemDiagAnalyzerArgs ### Description diff --git a/nodescraper/base/redfishcollectortask.py b/nodescraper/base/redfishcollectortask.py index 478ca6d2..2e8b4463 100644 --- a/nodescraper/base/redfishcollectortask.py +++ b/nodescraper/base/redfishcollectortask.py @@ -98,6 +98,7 @@ def combine_target_results(parent: str, results: list[TaskResult]) -> TaskResult aggregated.message = prefix + "\n".join(messages) if hard_fail and succeeded: aggregated.status = ExecutionStatus.WARNING + aggregated.details["per_target_messages"] = True return aggregated @@ -172,14 +173,7 @@ def _multi_target_wrapper( multi_conn: MultiTargetRedfishConnection = collector.connection # type: ignore[assignment] parent_name = collector.parent or type(collector).__name__ - all_results: list[TaskResult] = [ - TaskResult( - status=ExecutionStatus.WARNING, - parent=f"{parent_name}[{target_key}]", - message=message, - ) - for target_key, message in multi_conn.failed_targets.items() - ] + all_results: list[TaskResult] = [] target_count = len(multi_conn.target_connections) configured_workers = multi_conn.max_workers or target_count max_workers = min(configured_workers, target_count, 32) if target_count else 1 diff --git a/nodescraper/connection/inband/inbandremote.py b/nodescraper/connection/inband/inbandremote.py index eebc728d..76f9fb4e 100644 --- a/nodescraper/connection/inband/inbandremote.py +++ b/nodescraper/connection/inband/inbandremote.py @@ -26,7 +26,7 @@ import logging import os import socket -from typing import Type, Union +from typing import Any, Type, Union import paramiko from paramiko.ssh_exception import ( @@ -47,10 +47,14 @@ class SSHConnectionError(Exception): """A general exception for ssh connection failures""" +CHANNEL_OPEN_TIMEOUT = 60 + + class RemoteShell(InBandConnection): """Utility class for running shell commands""" host_key_policy: Type[paramiko.MissingHostKeyPolicy] = paramiko.RejectPolicy + _proxy_ssh_down: bool = False def __init__( self, @@ -62,10 +66,31 @@ def __init__( ssh_params (SSHConnectionParams): The parameters used to initiate the SSH connection username/password/ect. """ self.ssh_params = ssh_params + self._new_client() + + def _new_client(self) -> None: + """Replace the SSH client with a new unconnected client. + + Returns: + None. + """ self.client = paramiko.SSHClient() self.client.load_system_host_keys() self.client.set_missing_host_key_policy(self.host_key_policy()) + def _arm_channel_timeout(self) -> None: + """Bound channel opens that do not pass their own timeout. + + Returns: + None. + """ + transport = self.client.get_transport() + if transport is None: + return + runtime_transport: Any = transport + if runtime_transport.channel_timeout is None: + runtime_transport.channel_timeout = CHANNEL_OPEN_TIMEOUT + def connect_ssh(self): """Connect to the remote host via SSH""" try: @@ -85,6 +110,7 @@ def connect_ssh(self): auth_timeout=60, banner_timeout=200, ) + self._arm_channel_timeout() except socket.timeout: raise SSHConnectionError("SSH Request timeout") from socket.timeout except socket.gaierror as e: @@ -104,6 +130,19 @@ def connect_ssh(self): except Exception as e: raise SSHConnectionError(f"Unexpected error during SSH connection: {str(e)}") from e + def reconnect_ssh(self) -> None: + """Close the current SSH session and open a new one. + + Returns: + None. + """ + try: + self.client.close() + except (OSError, SSHException): + pass + self._new_client() + self.connect_ssh() + def read_file( self, filename: str, @@ -120,8 +159,7 @@ def read_file( Returns: BaseFileArtifact: Object representing file contents """ - with self.client.open_sftp().open(filename, "rb") as remote_file: - raw_contents = remote_file.read() + raw_contents = self.read_files([filename])[0] return BaseFileArtifact.from_bytes( filename=os.path.basename(filename), raw_contents=raw_contents, @@ -129,6 +167,26 @@ def read_file( strip=strip, ) + def read_files(self, filenames: list[str]) -> list[bytes]: + """Read remote files through one SFTP session. + + Args: + filenames: Remote paths to read. + + Returns: + File contents in the same order as filenames. + """ + self._arm_channel_timeout() + sftp = self.client.open_sftp() + try: + contents: list[bytes] = [] + for filename in filenames: + with sftp.open(filename, "rb") as remote_file: + contents.append(remote_file.read()) + return contents + finally: + sftp.close() + def run_command( self, command: str, @@ -155,8 +213,10 @@ def run_command( elif sudo: cmd_str = f"sudo {cmd_str}" + channel = None try: stdin, stdout, stderr = self.client.exec_command(cmd_str, timeout=timeout) + channel = stdout.channel if write_password: stdin.write( ( @@ -184,6 +244,12 @@ def run_command( stderr_str = f"{type(exc).__name__}: {exc}" stdout_str = "" exit_code = 124 + finally: + if channel is not None: + try: + channel.close() + except (OSError, SSHException): + pass return CommandArtifact( command=cmd_str, diff --git a/nodescraper/connection/redfish/redfish_connection.py b/nodescraper/connection/redfish/redfish_connection.py index 1cfc1667..2fa92236 100644 --- a/nodescraper/connection/redfish/redfish_connection.py +++ b/nodescraper/connection/redfish/redfish_connection.py @@ -239,6 +239,21 @@ def _login_session(self) -> None: else: session.auth = HTTPBasicAuth(self.username, self.password) + def _drop_session(self) -> None: + """Discard the HTTP session without logging out of Redfish. + + Args: + None + + Returns: + None + """ + if self._session is not None: + self._session.close() + self._session = None + self._session_token = None + self._session_uri = None + def get(self, path: RedfishPath) -> dict[str, Any]: """GET a Redfish path and return the JSON body. path must be a RedfishPath.""" path_str = str(path) @@ -256,10 +271,22 @@ def get(self, path: RedfishPath) -> dict[str, Any]: ) from exc def get_response(self, path: Union[str, "RedfishPath"]) -> RedfishHttpResponse: - """GET a Redfish path and return the raw Response. path may be a string or RedfishPath.""" + """GET a Redfish path and refresh the session once after HTTP 401. + + Args: + path: Redfish path or absolute URL. + + Returns: + The HTTP response, including a second 401 when reauthentication fails. + """ path = str(path) session = self._ensure_session() url = path if path.startswith("http") else urljoin(self.base_url + "/", path.lstrip("/")) + response = self._execute_request(lambda: session.get(url, timeout=self.timeout)) + if response.status_code != 401 or not self.use_session_auth or not self.password: + return response + self._drop_session() + session = self._ensure_session() return self._execute_request(lambda: session.get(url, timeout=self.timeout)) def post( diff --git a/nodescraper/connection/redfish/ssh_proxy_connection.py b/nodescraper/connection/redfish/ssh_proxy_connection.py index a58f86da..1d47a963 100644 --- a/nodescraper/connection/redfish/ssh_proxy_connection.py +++ b/nodescraper/connection/redfish/ssh_proxy_connection.py @@ -26,16 +26,17 @@ from __future__ import annotations import json as json_lib +import logging +import secrets import shlex from http import HTTPStatus from typing import Any, Optional, Union from urllib.parse import urljoin, urlparse -from paramiko.ssh_exception import SSHException from requests.structures import CaseInsensitiveDict -from nodescraper.connection.inband.inband import BinaryFileArtifact, CommandArtifact -from nodescraper.connection.inband.inbandremote import RemoteShell +from nodescraper.connection.inband.inband import CommandArtifact +from nodescraper.connection.inband.inbandremote import RemoteShell, SSHConnectionError from .redfish_connection import RedfishConnection, RedfishConnectionError from .redfish_path import RedfishPath @@ -62,6 +63,70 @@ def parse_curl_headers(header_text: str) -> CaseInsensitiveDict: return headers +_DEAD_SESSION_MARKERS = ( + "server connection dropped", + "ssh session not active", + "session not active", + "timeout opening channel", + "socket is closed", + "connection reset", + "not connected", + "ssh channel failed", +) + + +def _is_dead_session(detail: str) -> bool: + """Return whether an SSH error means the BMC session is gone. + + Args: + detail: Error text from the SSH client. + + Returns: + True when the session should be opened again. + """ + text = detail.lower() + return any(marker in text for marker in _DEAD_SESSION_MARKERS) + + +def split_curl_stdout(raw: str, marker: str) -> tuple[int, str, bytes]: + """Split curl stdout into an HTTP status, headers, and body. + + Args: + raw: Combined curl stdout. + marker: Trailer token written by curl -w. + + Returns: + Status code, header text, and body bytes. + """ + payload, found, tail = raw.rpartition(marker) + if not found: + raise RedfishConnectionError("curl stdout is missing the status trailer") + if payload.endswith("\n"): + payload = payload[:-1] + try: + status_code = int(tail.strip()) + except (TypeError, ValueError) as exc: + raise RedfishConnectionError(f"curl status is not numeric: {tail.strip()}") from exc + header_text, body_text = _split_header_and_body(payload) + return status_code, header_text, body_text.encode("utf-8") + + +def _split_header_and_body(payload: str) -> tuple[str, str]: + """Split a curl -D - payload into header text and body text. + + Args: + payload: Headers and body with the status trailer removed. + + Returns: + Header text and body text. Header text is empty when no header block is present. + """ + for separator in ("\r\n\r\n", "\n\n"): + index = payload.find(separator) + if index != -1: + return payload[:index], payload[index + len(separator) :] + return "", payload + + def _url_path(url: str) -> str: """Return the URL path for log messages (no host). @@ -137,63 +202,78 @@ def __init__( def _cmd_timeout(self) -> int: return max(int(self.timeout) + 15, 30) - def _mktemp(self) -> str: - artifact = self._shell.run_command("mktemp", timeout=self._cmd_timeout()) - path = (artifact.stdout or "").strip() - if artifact.exit_code != 0 or not path: - detail = (artifact.stderr or "").strip() - raise RedfishConnectionError(detail or "Failed to create remote temp file via mktemp") - return path + def _curl(self, url: str, extra_flags: str = "") -> CurlResponse: + """Run curl over SSH, and open the session again once if it has dropped. - def _rm(self, *paths: str) -> None: - quoted = " ".join(shlex.quote(p) for p in paths if p) - if quoted: - self._shell.run_command(f"rm -f {quoted}", timeout=self._cmd_timeout()) + Args: + url: Absolute URL to request. + extra_flags: Extra curl arguments, including a trailing space when set. - def _curl(self, url: str, extra_flags: str = "") -> CurlResponse: - header_path = "" - body_path = "" + Returns: + CurlResponse with status, headers, and body. + """ + if self._shell._proxy_ssh_down: + raise RedfishConnectionError("SSH session not active") try: - header_path = self._mktemp() - body_path = self._mktemp() - max_time = max(int(self.timeout), 1) - cmd = ( - f"curl -sS --max-time {max_time} -D {shlex.quote(header_path)} " - f"-o {shlex.quote(body_path)} -w '%{{http_code}}' {extra_flags}" - f"{shlex.quote(url)}" - ) - result: CommandArtifact = self._shell.run_command( - cmd, timeout=self._cmd_timeout(), strip=True + return self._curl_once(url, extra_flags) + except RedfishConnectionError as exc: + if not _is_dead_session(str(exc)): + raise + logging.getLogger("nodescraper").info( + "SSH proxy session dropped; reconnecting and retrying %s", + _url_path(url), ) - status_str = (result.stdout or "").strip() try: - status_code = int(status_str) - except (TypeError, ValueError) as exc: - detail = result.stderr or result.stdout or str(exc) - raise RedfishConnectionError(f"curl to {_url_path(url)} failed: {detail}") from exc - if result.exit_code != 0 and status_code == 0: + self._shell.reconnect_ssh() + except (SSHConnectionError, OSError) as reconnect_exc: + self._shell._proxy_ssh_down = True raise RedfishConnectionError( - f"curl to {_url_path(url)} failed: {result.stderr or result.stdout}" - ) - header_art = self._shell.read_file(header_path, encoding="utf-8", strip=False) - body_art = self._shell.read_file(body_path, encoding=None) - if isinstance(header_art, BinaryFileArtifact): - header_text = header_art.contents.decode("utf-8", errors="replace") - else: - header_text = header_art.contents_str() - if isinstance(body_art, BinaryFileArtifact): - body = body_art.contents - else: - body = body_art.contents_str().encode("utf-8") - return CurlResponse( - status_code=status_code, - content=body, - headers=parse_curl_headers(header_text), - ) - except SSHException as exc: - raise RedfishConnectionError(f"SSH channel failed: {exc}") from exc - finally: - self._rm(header_path, body_path) + f"SSH proxy reconnect failed: {reconnect_exc}" + ) from reconnect_exc + try: + return self._curl_once(url, extra_flags) + except RedfishConnectionError as retry_exc: + if _is_dead_session(str(retry_exc)): + self._shell._proxy_ssh_down = True + raise + + def _curl_once(self, url: str, extra_flags: str = "") -> CurlResponse: + """Run one curl on the BMC and read the response from that command. + + Args: + url: Absolute URL to request. + extra_flags: Extra curl arguments, including a trailing space when set. + + Returns: + CurlResponse with status, headers, and body. + """ + marker = f"NODESCRAPER{secrets.token_hex(16)}" + max_time = max(int(self.timeout), 1) + trailer = f"\n{marker}%{{http_code}}\n" + cmd = ( + f"curl -sS --max-time {max_time} -D - -o - " + f"-w {shlex.quote(trailer)} {extra_flags}{shlex.quote(url)}" + ) + result: CommandArtifact = self._shell.run_command( + cmd, timeout=self._cmd_timeout(), strip=False + ) + if _is_dead_session(result.stderr or ""): + raise RedfishConnectionError((result.stderr or "").strip() or "SSH session not active") + raw = result.stdout or "" + detail = (result.stderr or result.stdout or "").strip() + if marker not in raw: + raise RedfishConnectionError(f"curl to {_url_path(url)} failed: {detail}") + try: + status_code, header_text, body = split_curl_stdout(raw, marker) + except RedfishConnectionError as exc: + raise RedfishConnectionError(f"curl to {_url_path(url)} failed: {exc}") from exc + if result.exit_code != 0 and status_code == 0: + raise RedfishConnectionError(f"curl to {_url_path(url)} failed: {detail}") + return CurlResponse( + status_code=status_code, + content=body, + headers=parse_curl_headers(header_text), + ) def _ensure_session(self): return None diff --git a/nodescraper/interfaces/dataplugin.py b/nodescraper/interfaces/dataplugin.py index 44f9d3dc..758d5424 100644 --- a/nodescraper/interfaces/dataplugin.py +++ b/nodescraper/interfaces/dataplugin.py @@ -606,6 +606,10 @@ def run( ) status = max(self.collection_result.status, self.analysis_result.status) + collection_message = self.collection_result.message or "" + show_targets = bool( + self.collection_result.details.get("per_target_messages") and collection_message + ) message = "" if status == ExecutionStatus.NOT_RAN: @@ -616,11 +620,19 @@ def run( ExecutionStatus.WARNING, ]: failure_parts: list[str] = [] + if show_targets and self.collection_result.status == ExecutionStatus.OK: + failure_parts.append(collection_message) for label, task_result in ( ("Collection", self.collection_result), ("Analysis", self.analysis_result), ): - if task_result.status == ExecutionStatus.WARNING: + if ( + show_targets + and label == "Collection" + and task_result.status == ExecutionStatus.WARNING + ): + failure_parts.append(collection_message) + elif task_result.status == ExecutionStatus.WARNING: failure_parts.append(f"{label} warning: {task_result.message}") elif task_result.status in ( ExecutionStatus.ERROR, @@ -628,6 +640,8 @@ def run( ): failure_parts.append(f"{label} error: {task_result.message}") message = "; ".join(failure_parts) + elif show_targets: + message = collection_message else: message = "Plugin tasks completed successfully" diff --git a/nodescraper/pluginexecutor.py b/nodescraper/pluginexecutor.py index 01737615..b79c9df6 100644 --- a/nodescraper/pluginexecutor.py +++ b/nodescraper/pluginexecutor.py @@ -40,6 +40,10 @@ from nodescraper.connection.inband import InBandConnectionManager from nodescraper.connection.inband.osdetection import discover_and_write_os_family from nodescraper.connection.oob_ssh import OobSshConnectionManager +from nodescraper.connection.redfish import ( + RedfishConnectionManager, + RedfishSshProxyConnectionManager, +) from nodescraper.constants import DEFAULT_LOGGER from nodescraper.enums import ExecutionStatus, OSFamily from nodescraper.helpers.plugin_execution_target import ( @@ -268,6 +272,35 @@ def discover_os_info(self) -> None: finally: inband_connection.safe_disconnect() + def _resolve_redfish_manager(self, plugin_name: str) -> Optional[ConnectionManager]: + """Pick the Redfish transport for a plugin declaring ``RedfishConnectionManager``. + + The direct ``RedfishConnectionManager`` wins when configured; otherwise + ``RedfishSshProxyConnectionManager`` is used. With neither configured the plugin is skipped. + + Args: + plugin_name: Registry key for the plugin (for logging). + + Returns: + The connection manager instance, or None if no Redfish connection is configured. + """ + for name in ( + RedfishConnectionManager.__name__, + RedfishSshProxyConnectionManager.__name__, + ): + if name not in self.connection_configs: + continue + mgr_impl = self.plugin_registry.connection_managers.get(name) + conn_mgr = self.connection_library.get(mgr_impl) if mgr_impl else None + if conn_mgr is not None: + return conn_mgr + self.logger.error( + "%s requires RedfishConnectionManager or RedfishSshProxyConnectionManager in the " + "connection config", + plugin_name, + ) + return None + def _get_connection_manager_for_plugin( self, plugin_class: type, @@ -281,6 +314,11 @@ def _get_connection_manager_for_plugin( The resolved manager is stored in ``self.connection_library`` so that it can be shared across plugins that require the same type. + + Plugins declaring ``RedfishConnectionManager`` fall back to + ``RedfishSshProxyConnectionManager`` when only the proxy is configured + (see :meth:`_resolve_redfish_manager`). Plugins declaring + ``RedfishSshProxyConnectionManager`` always use the proxy. """ if not plugin_class.CONNECTION_TYPE: return None @@ -296,6 +334,8 @@ def _get_connection_manager_for_plugin( return None else: connection_manager_class: Type[ConnectionManager] = plugin_class.CONNECTION_TYPE + if connection_manager_class is RedfishConnectionManager: + return self._resolve_redfish_manager(plugin_name) if connection_manager_class.__name__ in self.plugin_registry.connection_managers: mgr_impl = self.plugin_registry.connection_managers[ connection_manager_class.__name__ diff --git a/nodescraper/plugins/ooband/redfish_endpoint/__init__.py b/nodescraper/plugins/ooband/redfish_endpoint/__init__.py index 1407be8c..6c769742 100644 --- a/nodescraper/plugins/ooband/redfish_endpoint/__init__.py +++ b/nodescraper/plugins/ooband/redfish_endpoint/__init__.py @@ -28,9 +28,10 @@ from .endpoint_analyzer import RedfishEndpointAnalyzer from .endpoint_collector import RedfishEndpointCollector from .endpoint_data import RedfishEndpointDataModel -from .endpoint_plugin import RedfishEndpointPlugin +from .endpoint_plugin import AmcRedfishEndpointPlugin, RedfishEndpointPlugin __all__ = [ + "AmcRedfishEndpointPlugin", "RedfishEndpointAnalyzer", "RedfishEndpointAnalyzerArgs", "RedfishEndpointCollector", diff --git a/nodescraper/plugins/ooband/redfish_endpoint/analyzer_args.py b/nodescraper/plugins/ooband/redfish_endpoint/analyzer_args.py index 28f115e5..5b495bb5 100644 --- a/nodescraper/plugins/ooband/redfish_endpoint/analyzer_args.py +++ b/nodescraper/plugins/ooband/redfish_endpoint/analyzer_args.py @@ -42,6 +42,8 @@ class ConstraintKey(str, Enum): MIN = "min" MAX = "max" ANY_OF = "anyOf" + REGEX = "regex" + EXISTS = "exists" RedfishConstraint = Union[int, float, str, bool, dict[str, Any]] @@ -62,6 +64,8 @@ class RedfishEndpointAnalyzerArgs(AnalyzerArgs): "'min' — value must be numeric and >= the given number. " "'max' — value must be numeric and <= the given number. " "'anyOf' — value must be in the given list (OR; any match passes). " + "'regex' — value must be a string matching the pattern (Python re.search; anchor with ^ and $ for a full match). " + "'exists' — true: property must be present and not null; false: property must be absent or null. " 'Example: { "/redfish/v1/Systems/1": { "Status/Health": { "anyOf": ["OK", "Warning"] }, "PowerState": "On" }, "*": { "Status/Health": { "anyOf": ["OK"] } } }.' ), ) diff --git a/nodescraper/plugins/ooband/redfish_endpoint/collector_args.py b/nodescraper/plugins/ooband/redfish_endpoint/collector_args.py index 2f5a4f47..b4a38aba 100644 --- a/nodescraper/plugins/ooband/redfish_endpoint/collector_args.py +++ b/nodescraper/plugins/ooband/redfish_endpoint/collector_args.py @@ -42,7 +42,7 @@ class RedfishEndpointCollectorArgs(CollectorArgs): tree_max_depth: int = Field( default=2, ge=1, - le=10, + le=20, description="When discover_tree is True: max traversal depth (1=service root only, 2=root + collections, 3=+ members).", ) tree_max_endpoints: int = Field( @@ -51,6 +51,21 @@ class RedfishEndpointCollectorArgs(CollectorArgs): le=10_000, description="When discover_tree is True: max endpoints to discover (0=no limit).", ) + tree_max_collection_pages: int = Field( + default=1, + ge=1, + le=10_000, + description="When discover_tree is True: max pages to collect for each paginated collection.", + ) + tree_max_log_entry_members: int = Field( + default=0, + ge=0, + le=10_000, + description=( + "When discover_tree is True: max individual LogService entries to collect per collection. " + "Zero keeps every referenced entry within the depth limit." + ), + ) max_workers: int = Field( default=1, ge=1, diff --git a/nodescraper/plugins/ooband/redfish_endpoint/endpoint_analyzer.py b/nodescraper/plugins/ooband/redfish_endpoint/endpoint_analyzer.py index 3b783fe3..d0e51b1b 100644 --- a/nodescraper/plugins/ooband/redfish_endpoint/endpoint_analyzer.py +++ b/nodescraper/plugins/ooband/redfish_endpoint/endpoint_analyzer.py @@ -23,6 +23,7 @@ # SOFTWARE. # ############################################################################### +import re from typing import Any, Optional from nodescraper.enums import EventCategory, EventPriority, ExecutionStatus @@ -59,6 +60,19 @@ def _get_by_path(obj: Any, path: str) -> Any: def _check_constraint(actual: Any, constraint: RedfishConstraint) -> tuple[bool, str]: """Compare actual value to constraint.""" if isinstance(constraint, dict): + if ConstraintKey.EXISTS in constraint: + want = bool(constraint[ConstraintKey.EXISTS]) + ok = (actual is not None) == want + return ok, f"expected {'present' if want else 'absent'}, got {actual!r}" + if ConstraintKey.REGEX in constraint: + pattern = constraint[ConstraintKey.REGEX] + if not isinstance(actual, str): + return False, f"expected string matching {pattern!r}, got {actual!r}" + try: + ok = re.search(pattern, actual) is not None + except re.error as exc: + return False, f"invalid regex {pattern!r}: {exc}" + return ok, f"expected match for {pattern!r}, got {actual!r}" if ConstraintKey.EQ in constraint: ok = actual == constraint[ConstraintKey.EQ] return ok, f"expected eq {constraint[ConstraintKey.EQ]}, got {actual!r}" @@ -91,7 +105,7 @@ class RedfishEndpointAnalyzer(DataAnalyzer[RedfishEndpointDataModel, RedfishEndp DOCUMENTATION_ANALYSIS_ITEMS: tuple[str, ...] = ( "For each entry in analysis_args.checks, reads JSON paths in collected responses and " - "compares values to constraints (eq, min/max, anyOf, regex, etc.).", + "compares values to constraints (eq, min/max, anyOf, regex, exists).", "URI key `*` runs checks against every collected response body.", ) diff --git a/nodescraper/plugins/ooband/redfish_endpoint/endpoint_collector.py b/nodescraper/plugins/ooband/redfish_endpoint/endpoint_collector.py index b139396a..e9076069 100644 --- a/nodescraper/plugins/ooband/redfish_endpoint/endpoint_collector.py +++ b/nodescraper/plugins/ooband/redfish_endpoint/endpoint_collector.py @@ -23,6 +23,7 @@ # SOFTWARE. # ############################################################################### +import re from collections import deque from concurrent.futures import ThreadPoolExecutor, as_completed from typing import Any, Optional @@ -31,6 +32,7 @@ from nodescraper.base import RedfishDataCollector from nodescraper.connection.redfish import ( RF_MEMBERS, + RF_MEMBERS_NEXT_LINK, RF_ODATA_ID, RedfishConnection, RedfishGetResult, @@ -51,6 +53,19 @@ from .collector_args import RedfishEndpointCollectorArgs from .endpoint_data import RedfishEndpointDataModel +_REFERENCE_KEYS = frozenset( + { + RF_ODATA_ID, + "Uri", + "@Redfish.ActionInfo", + "DataSourceUri", + "TargetComponentURI", + } +) +_SKIPPED_REFERENCE_SUBTREES = frozenset({"Oem", "OriginOfCondition"}) +# Individual LogServices entry resources, not the Entries collection. +_LOG_ENTRY_URI_RE = re.compile(r"LogServices/.*/Entries/[^/]+$") + def _normalize_path(odata_id: str, api_root: str) -> str: """Convert @odata.id value (URL or path) to a normalized path under api_root.""" @@ -60,6 +75,8 @@ def _normalize_path(odata_id: str, api_root: str) -> str: if s.startswith(("http://", "https://")): parsed = urlparse(s) s = parsed.path or "/" + if parsed.query: + s = f"{s}?{parsed.query}" if not s.startswith("/"): s = "/" + s s = s.rstrip("/") or "/" @@ -69,44 +86,147 @@ def _normalize_path(odata_id: str, api_root: str) -> str: return s +def _is_fetchable_reference(value: Any) -> bool: + """Return whether a value points at a separate Redfish resource. + + Args: + value: A candidate Redfish reference. + + Returns: + True when value is an absolute resource path without a fragment. + """ + return isinstance(value, str) and value.startswith("/") and "#" not in value + + def _extract_odata_ids(obj: Any) -> list[str]: - """Recursively extract all @odata.id values from a Redfish JSON body.""" + """Extract Redfish resource references from a JSON body. + + Args: + obj: A Redfish JSON value. + + Returns: + Resource paths and resource ids referenced by the body. + """ out: list[str] = [] - if isinstance(obj, dict): - if RF_ODATA_ID in obj and isinstance(obj[RF_ODATA_ID], str): - out.append(obj[RF_ODATA_ID]) - for k, v in obj.items(): - if k == RF_MEMBERS and isinstance(v, list): - for item in v: - if ( - isinstance(item, dict) - and RF_ODATA_ID in item - and isinstance(item[RF_ODATA_ID], str) - ): - out.append(item[RF_ODATA_ID]) - elif isinstance(v, dict): - out.extend(_extract_odata_ids(v)) - elif isinstance(v, list): - for item in v: - if isinstance(item, dict): - out.extend(_extract_odata_ids(item)) + if not isinstance(obj, dict): + return out + for key, value in obj.items(): + if key in _SKIPPED_REFERENCE_SUBTREES: + continue + if key == RF_ODATA_ID and isinstance(value, str): + out.append(value) + elif key in _REFERENCE_KEYS and _is_fetchable_reference(value): + out.append(value) + for key, value in obj.items(): + if key in _SKIPPED_REFERENCE_SUBTREES: + continue + if key == RF_MEMBERS and isinstance(value, list): + for item in value: + if isinstance(item, dict) and isinstance(item.get(RF_ODATA_ID), str): + out.append(item[RF_ODATA_ID]) + elif isinstance(value, dict): + out.extend(_extract_odata_ids(value)) + elif isinstance(value, list): + for item in value: + if isinstance(item, dict): + out.extend(_extract_odata_ids(item)) return out +def _normalize_next_link(odata_id: Any, api_root: str) -> str: + """Normalize a collection next link and keep its query string. + + Args: + odata_id: A Members nextLink value. + api_root: Redfish service root. + + Returns: + The path and query under the service root, or an empty string. + """ + if not isinstance(odata_id, str) or not odata_id.strip(): + return "" + source = odata_id.strip() + query = "" + if source.startswith(("http://", "https://")): + parsed = urlparse(source) + source = parsed.path or "/" + query = f"?{parsed.query}" if parsed.query else "" + elif "?" in source: + source, raw_query = source.split("?", 1) + query = "?" + raw_query + path = _normalize_path(source, api_root) + if not path: + return "" + return path + query + + +def _enqueue_tree_links( + payload: Any, + depth: int, + api_root: str, + max_depth: int, + seen: set[str], + to_visit: deque[tuple[str, int]], + queued: set[str], + max_log_entry_members: int, +) -> None: + """Queue child resources linked from one Redfish payload. + + Args: + payload: Redfish JSON body. + depth: Depth of the payload that contains the links. + api_root: Redfish service root. + max_depth: Maximum resource depth. + seen: Paths already fetched. + to_visit: Paths waiting to be fetched. + queued: Paths already queued in this crawl. + max_log_entry_members: Maximum log entries from this payload. Zero means no limit. + + Returns: + None + """ + candidates: list[str] = [] + for odata_id in _extract_odata_ids(payload): + if isinstance(odata_id, str) and "#" in odata_id: + continue + link_path = _normalize_path(odata_id, api_root) + if not link_path or link_path in seen or link_path in queued or depth + 1 >= max_depth: + continue + candidates.append(link_path) + unique = list(dict.fromkeys(candidates)) + log_entries = sorted(path for path in unique if _LOG_ENTRY_URI_RE.search(path)) + others = [path for path in unique if not _LOG_ENTRY_URI_RE.search(path)] + if max_log_entry_members: + log_entries = log_entries[:max_log_entry_members] + for link_path in others + log_entries: + queued.add(link_path) + to_visit.append((link_path, depth + 1)) + + def _discover_tree( connection: RedfishConnection, api_root: str, max_depth: int, max_endpoints: int, + max_collection_pages: int = 1, + max_log_entry_members: int = 0, ) -> tuple[list[str], dict[str, dict], list[RedfishGetResult]]: - """ - Traverse the Redfish resource tree from the service root. - - max_depth matches collection_args.tree_max_depth: 1 = service root only; 2 = root + one link - level; child links are only enqueued when depth + 1 < max_depth (root is depth 0). + """Traverse the Redfish resource tree from the service root. + + Args: + connection: Connection used to fetch resources. + api_root: Redfish service-root path. + max_depth: Maximum resource depth, where one is only the service root. + max_endpoints: Maximum resource count, where zero means unlimited. + max_collection_pages: Maximum pages to retain for each paginated collection. + max_log_entry_members: Maximum individual log entries taken from each payload. + + Returns: + Visited paths, successful response bodies, and every GET result. """ root_path = _normalize_path(api_root, api_root) or ("/" + api_root.strip("/")) seen: set[str] = set() + queued: set[str] = {root_path} to_visit: deque[tuple[str, int]] = deque([(root_path, 0)]) responses: dict[str, dict] = {} results: list[RedfishGetResult] = [] @@ -119,13 +239,45 @@ def _discover_tree( seen.add(path) res = connection.run_get(path) results.append(res) - if res.success and res.data is not None: - responses[path] = res.data - for odata_id in _extract_odata_ids(res.data): - link_path = _normalize_path(odata_id, api_root) - # Follow only if the child depth stays strictly below max_depth (1 = root only). - if link_path and link_path not in seen and depth + 1 < max_depth: - to_visit.append((link_path, depth + 1)) + if not res.success or not isinstance(res.data, dict): + continue + responses[path] = res.data + _enqueue_tree_links( + res.data, + depth, + api_root, + max_depth, + seen, + to_visit, + queued, + max_log_entry_members, + ) + if max_collection_pages <= 1: + continue + next_path = _normalize_next_link(res.data.get(RF_MEMBERS_NEXT_LINK), api_root) + pages = 1 + while next_path and pages < max_collection_pages: + if next_path in seen or (max_endpoints and len(seen) >= max_endpoints): + break + seen.add(next_path) + queued.add(next_path) + page_res = connection.run_get(next_path) + results.append(page_res) + pages += 1 + if not page_res.success or not isinstance(page_res.data, dict): + break + responses[next_path] = page_res.data + _enqueue_tree_links( + page_res.data, + depth, + api_root, + max_depth, + seen, + to_visit, + queued, + max_log_entry_members, + ) + next_path = _normalize_next_link(page_res.data.get(RF_MEMBERS_NEXT_LINK), api_root) return sorted(seen), responses, results @@ -165,8 +317,9 @@ class RedfishEndpointCollector( DOCUMENTATION_COLLECTION_ITEMS: tuple[str, ...] = ( "Redfish GET: explicit paths from collection_args.uris (parallel when max_workers>1).", "Optional paged GET following the Members collection OData nextLink field when follow_next_link is true.", - "Redfish GET tree: when discover_tree is true, walks from api_root using OData resource id links and " - "Members navigation (depth and endpoint caps from collection_args).", + "Redfish GET tree: when discover_tree is true, walks from api_root using Redfish resource links. " + "Oem and OriginOfCondition subtrees are not followed. Collection pages and individual log entries " + "are capped by collection_args.", ) def collect_data( @@ -181,11 +334,15 @@ def collect_data( api_root = getattr(self.connection, "api_root", "redfish/v1") max_depth = getattr(args, "tree_max_depth", 2) if args else 2 max_endpoints = (getattr(args, "tree_max_endpoints", 0) or 0) if args else 0 + max_collection_pages = getattr(args, "tree_max_collection_pages", 1) if args else 1 + max_log_entry_members = getattr(args, "tree_max_log_entry_members", 0) if args else 0 _paths, responses, results = _discover_tree( self.connection, api_root, max_depth=max_depth, max_endpoints=max_endpoints, + max_collection_pages=max_collection_pages, + max_log_entry_members=max_log_entry_members, ) for res in results: self._append_redfish_artifact(res) diff --git a/nodescraper/plugins/ooband/redfish_endpoint/endpoint_plugin.py b/nodescraper/plugins/ooband/redfish_endpoint/endpoint_plugin.py index 8732888f..ad8decf4 100644 --- a/nodescraper/plugins/ooband/redfish_endpoint/endpoint_plugin.py +++ b/nodescraper/plugins/ooband/redfish_endpoint/endpoint_plugin.py @@ -24,6 +24,7 @@ # ############################################################################### from nodescraper.base import OOBandDataPlugin +from nodescraper.connection.redfish import RedfishSshProxyConnectionManager from .analyzer_args import RedfishEndpointAnalyzerArgs from .collector_args import RedfishEndpointCollectorArgs @@ -46,3 +47,14 @@ class RedfishEndpointPlugin( ANALYZER = RedfishEndpointAnalyzer COLLECTOR_ARGS = RedfishEndpointCollectorArgs ANALYZER_ARGS = RedfishEndpointAnalyzerArgs + + +class AmcRedfishEndpointPlugin(RedfishEndpointPlugin): + """Collect the same Redfish endpoint checks through the BMC SSH proxy. + + Pair this with RedfishEndpointPlugin so one run reads the BMC on the direct + Redfish connection and the AMC on RedfishSshProxyConnectionManager. collection_args + and analysis_args are the same for both plugins. + """ + + CONNECTION_TYPE = RedfishSshProxyConnectionManager # type: ignore[assignment] diff --git a/nodescraper/plugins/ooband/redfish_inventory/__init__.py b/nodescraper/plugins/ooband/redfish_inventory/__init__.py new file mode 100644 index 00000000..c3644f93 --- /dev/null +++ b/nodescraper/plugins/ooband/redfish_inventory/__init__.py @@ -0,0 +1,41 @@ +############################################################################### +# +# 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 .analyzer_args import RedfishInventoryAnalyzerArgs +from .collector_args import RedfishInventoryCollectorArgs +from .inventory_analyzer import RedfishInventoryAnalyzer +from .inventory_collector import RedfishInventoryCollector +from .inventory_data import RedfishInventoryDataModel +from .inventory_plugin import AmcRedfishInventoryPlugin, RedfishInventoryPlugin + +__all__ = [ + "AmcRedfishInventoryPlugin", + "RedfishInventoryAnalyzer", + "RedfishInventoryAnalyzerArgs", + "RedfishInventoryCollector", + "RedfishInventoryCollectorArgs", + "RedfishInventoryDataModel", + "RedfishInventoryPlugin", +] diff --git a/nodescraper/plugins/ooband/redfish_inventory/analyzer_args.py b/nodescraper/plugins/ooband/redfish_inventory/analyzer_args.py new file mode 100644 index 00000000..a3d0bcb7 --- /dev/null +++ b/nodescraper/plugins/ooband/redfish_inventory/analyzer_args.py @@ -0,0 +1,45 @@ +############################################################################### +# +# MIT License +# +# Copyright (c) 2026 Advanced Micro Devices, Inc. +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in all +# copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. +# +############################################################################### +from pydantic import Field + +from nodescraper.models import AnalyzerArgs + + +class RedfishInventoryAnalyzerArgs(AnalyzerArgs): + """Analyzer args for comparing Redfish firmware inventory across targets.""" + + compare_firmware: bool = Field( + default=True, + description=( + "Compare FirmwareInventory across this plugin's connected targets. " + "Default true. A different Version, a member missing on a connected target, " + "or a missing Version (recorded as N/A) logs one FW warning per component. " + "The warning records Version, Status Health, Status State, SoftwareId, " + "Updateable, and Oem SVN. Ids are compared as published. " + "One connected target, a failed connection, or false skips the comparison. " + "--data loads one model and does not compare nodes." + ), + ) diff --git a/nodescraper/plugins/ooband/redfish_inventory/collector_args.py b/nodescraper/plugins/ooband/redfish_inventory/collector_args.py new file mode 100644 index 00000000..0659e873 --- /dev/null +++ b/nodescraper/plugins/ooband/redfish_inventory/collector_args.py @@ -0,0 +1,233 @@ +############################################################################### +# +# 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 + +from pydantic import Field, field_validator + +from nodescraper.models import CollectorArgs + +BMC_COLLECTIONS = ("Chassis", "Managers", "TelemetryService", "UpdateService", "Systems") +BMC_LINKS = ( + "Assembly", + "PowerSubsystem", + "Power", + "FruDevices", + "MetricReports", + "MetricReportDefinitions", + "FirmwareInventory", +) +SMC_COLLECTIONS = ("Systems", "UpdateService") +SMC_LINKS = ("Memory", "Processors", "FirmwareInventory") +INVENTORY_SWITCHES = ( + "chassis", + "managers", + "assembly", + "power", + "fru", + "metrics", + "update_service", + "systems", + "memory", + "processors", +) +_ROOT_SWITCH = { + "Chassis": "chassis", + "Managers": "managers", + "TelemetryService": "metrics", + "UpdateService": "update_service", + "Systems": "systems", +} +_LINK_SWITCH = { + "Assembly": "assembly", + "PowerSubsystem": "power", + "Power": "power", + "FruDevices": "fru", + "MetricReports": "metrics", + "MetricReportDefinitions": "metrics", + "FirmwareInventory": "update_service", + "Memory": "memory", + "Processors": "processors", +} +_SWITCH_ROOTS: dict[str, tuple[str, ...]] = { + "chassis": ("Chassis",), + "managers": ("Managers",), + "metrics": ("TelemetryService",), + "update_service": ("UpdateService",), + "systems": ("Systems",), +} +_SWITCH_LINKS = { + "assembly": ("Assembly",), + "power": ("PowerSubsystem", "Power"), + "fru": ("FruDevices",), + "metrics": ("MetricReports", "MetricReportDefinitions"), + "update_service": ("FirmwareInventory",), + "memory": ("Memory",), + "processors": ("Processors",), +} + + +def _ordered_unique(names: tuple[str, ...]) -> list[str]: + """Return names with duplicates removed and the first occurrence kept. + + Args: + names: Collection or link names. + + Returns: + Names in first-seen order. + """ + return list(dict.fromkeys(names)) + + +def _apply_enabled( + names: list[str], + name_switches: dict[str, str], + forced: dict[str, tuple[str, ...]], + enabled: dict[str, bool], +) -> list[str]: + """Drop disabled inventory areas and add areas explicitly turned on. + + Args: + names: Profile or caller-selected Redfish property names. + name_switches: Property name to inventory switch. + forced: Property names added when a switch is true. + enabled: Switch overrides. A missing switch is left unchanged. + + Returns: + Property names to follow. + """ + kept: list[str] = [] + for name in names: + switch = name_switches.get(name) + if switch is not None and enabled.get(switch) is False: + continue + kept.append(name) + for switch, extra in forced.items(): + if enabled.get(switch) is True: + kept.extend(extra) + return _ordered_unique(tuple(kept)) + + +class RedfishInventoryCollectorArgs(CollectorArgs): + """Select standard Redfish inventory collections to read from the service root.""" + + profile: Literal["bmc", "smc", "all"] = Field( + default="all", + description="bmc reads chassis, managers, telemetry, firmware inventory, and systems. " + "smc reads systems, memory, processors, and firmware inventory. all reads both.", + ) + collections: list[str] = Field( + default_factory=list, + description="Extra service-root property names appended to the profile set. " + "Duplicates are ignored. Empty leaves the profile unchanged.", + ) + member_links: list[str] = Field( + default_factory=list, + description="Extra property names appended to the profile link set. " + "Duplicates are ignored. Empty leaves the profile unchanged.", + ) + max_pages: int = Field( + default=1, + ge=1, + le=10_000, + description="Maximum pages to keep for each collection, including the first page.", + ) + enabled: dict[str, bool] = Field( + default_factory=dict, + description="Turn inventory areas on or off. Omitted areas stay at the profile default, " + "and the default profile enables every area. update_service also follows " + "FirmwareInventory members. Keys: chassis, managers, assembly, power, " + "fru, metrics, update_service, systems, memory, processors.", + ) + + @field_validator("enabled") + @classmethod + def reject_unknown_switches(cls, value: dict[str, bool]) -> dict[str, bool]: + """Reject inventory switch names that this collector does not know. + + Args: + value: Requested enable flags. + + Returns: + The same flags when every key is known. + """ + unknown = sorted(set(value) - set(INVENTORY_SWITCHES)) + if unknown: + raise ValueError(f"unknown inventory switches: {', '.join(unknown)}") + return value + + def _profile_collections(self) -> list[str]: + """Return the service-root collections selected by the profile. + + Args: + None + + Returns: + Service-root property names. + """ + if self.profile == "bmc": + return list(BMC_COLLECTIONS) + if self.profile == "smc": + return list(SMC_COLLECTIONS) + return _ordered_unique(BMC_COLLECTIONS + SMC_COLLECTIONS) + + def _profile_links(self) -> list[str]: + """Return the resource links selected by the profile. + + Args: + None + + Returns: + Property names followed from collected resources. + """ + if self.profile == "bmc": + return list(BMC_LINKS) + if self.profile == "smc": + return list(SMC_LINKS) + return _ordered_unique(BMC_LINKS + SMC_LINKS) + + def selected_collections(self) -> list[str]: + """Return the service-root collections this run will follow. + + Args: + None + + Returns: + Profile names with collections appended. Duplicates are dropped. + """ + names = _ordered_unique(tuple(self._profile_collections() + list(self.collections))) + return _apply_enabled(names, _ROOT_SWITCH, _SWITCH_ROOTS, self.enabled) + + def selected_links(self) -> list[str]: + """Return the resource links this run will follow. + + Args: + None + + Returns: + Profile names with member_links appended. Duplicates are dropped. + """ + names = _ordered_unique(tuple(self._profile_links() + list(self.member_links))) + return _apply_enabled(names, _LINK_SWITCH, _SWITCH_LINKS, self.enabled) diff --git a/nodescraper/plugins/ooband/redfish_inventory/firmware_compare.py b/nodescraper/plugins/ooband/redfish_inventory/firmware_compare.py new file mode 100644 index 00000000..03bb34bb --- /dev/null +++ b/nodescraper/plugins/ooband/redfish_inventory/firmware_compare.py @@ -0,0 +1,183 @@ +############################################################################### +# +# 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 Any, Optional, TypedDict + +from .inventory_data import RedfishInventoryDataModel + +_FIRMWARE_MEMBER = "/FirmwareInventory/" + + +class FirmwareComponent(TypedDict): + """Firmware fields recorded for one FirmwareInventory member.""" + + version: str + health: str + state: str + software_id: str + updateable: str + svn: str + + +class FirmwareMismatch(TypedDict): + """One firmware id whose version or presence is not the same on every target.""" + + firmware_id: str + targets: dict[str, Optional[FirmwareComponent]] + + +def _text_or_na(value: Any) -> str: + """Return a display string, using N/A when the value is missing. + + Args: + value: Field value from a firmware member. + + Returns: + The string value, or N/A when value is None or empty. + """ + if value is None or value == "": + return "N/A" + return str(value) + + +def _member_name(path: str) -> str: + """Return the FirmwareInventory member name from a Redfish path. + + Args: + path: Collected resource path. + + Returns: + The member name, or an empty string for collections and nested resources. + """ + clean = path.split("?", 1)[0].rstrip("/") + if _FIRMWARE_MEMBER not in clean: + return "" + tail = clean.split(_FIRMWARE_MEMBER, 1)[1] + if not tail or "/" in tail: + return "" + return tail + + +def _component(body: dict) -> FirmwareComponent: + """Read the firmware fields stored for one member. + + Args: + body: FirmwareInventory member payload. + + Returns: + Version, health, state, software id, updateable, and SVN. + """ + status = body.get("Status") + if not isinstance(status, dict): + status = {} + oem = body.get("Oem") + if not isinstance(oem, dict): + oem = {} + updateable = body.get("Updateable") + svn = oem.get("SVN") + return { + "version": _text_or_na(body.get("Version")), + "health": _text_or_na(status.get("Health")), + "state": _text_or_na(status.get("State")), + "software_id": _text_or_na(body.get("SoftwareId")), + "updateable": "N/A" if updateable is None else str(updateable), + "svn": "" if svn is None else str(svn), + } + + +def firmware_components( + model: RedfishInventoryDataModel, +) -> dict[str, FirmwareComponent]: + """Return FirmwareInventory members keyed by their raw id. + + Args: + model: Inventory collected from one target. + + Returns: + Firmware id to the recorded fields. A missing version is N/A. + """ + components: dict[str, FirmwareComponent] = {} + for path, body in model.responses.items(): + if not isinstance(path, str) or not isinstance(body, dict): + continue + member = _member_name(path) + if not member: + continue + raw_id = body.get("Id") + if isinstance(raw_id, str) and raw_id.strip(): + fw_id = raw_id.strip() + else: + fw_id = member + components[fw_id] = _component(body) + return components + + +def firmware_mismatches( + by_target: dict[str, dict[str, FirmwareComponent]], +) -> list[FirmwareMismatch]: + """Find firmware ids whose version or presence differs across targets. + + Args: + by_target: Target key to that target's firmware members. + + Returns: + Mismatches in firmware-id order. An absent member is included. + """ + names: set[str] = set() + for components in by_target.values(): + names.update(components) + mismatches: list[FirmwareMismatch] = [] + for fw_id in sorted(names): + rows: dict[str, Optional[FirmwareComponent]] = {} + versions: list[str] = [] + missing = False + for target in sorted(by_target): + record = by_target[target].get(fw_id) + rows[target] = record + if record is None: + missing = True + else: + versions.append(record["version"]) + if missing or len(set(versions)) > 1: + mismatches.append({"firmware_id": fw_id, "targets": rows}) + return mismatches + + +def firmware_target_text(record: Optional[FirmwareComponent]) -> str: + """Format one target's firmware fields for a warning. + + Args: + record: Fields for this target, or None when the member is absent. + + Returns: + missing, or the version and the other recorded fields. + """ + if record is None: + return "missing" + return ( + f"version={record['version']} health={record['health']} " + f"state={record['state']} software_id={record['software_id']} " + f"updateable={record['updateable']} svn={record['svn']}" + ) diff --git a/nodescraper/plugins/ooband/redfish_inventory/inventory_analyzer.py b/nodescraper/plugins/ooband/redfish_inventory/inventory_analyzer.py new file mode 100644 index 00000000..4a47eb04 --- /dev/null +++ b/nodescraper/plugins/ooband/redfish_inventory/inventory_analyzer.py @@ -0,0 +1,166 @@ +############################################################################### +# +# MIT License +# +# Copyright (c) 2026 Advanced Micro Devices, Inc. +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in all +# copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. +# +############################################################################### +from typing import Optional + +from nodescraper.enums import EventCategory, EventPriority, ExecutionStatus +from nodescraper.interfaces import DataAnalyzer +from nodescraper.models import TaskResult + +from .analyzer_args import RedfishInventoryAnalyzerArgs +from .firmware_compare import ( + firmware_components, + firmware_mismatches, + firmware_target_text, +) +from .inventory_data import RedfishInventoryDataModel + + +class RedfishInventoryAnalyzer( + DataAnalyzer[RedfishInventoryDataModel, RedfishInventoryAnalyzerArgs] +): + """Compare FirmwareInventory members across the targets this plugin collected.""" + + DATA_MODEL = RedfishInventoryDataModel + + DOCUMENTATION_ANALYSIS_ITEMS: tuple[str, ...] = ( + "analysis_args.compare_firmware defaults to true and compares FirmwareInventory " + "across the targets this plugin connected.", + "RedfishInventoryPlugin compares its direct Redfish targets. " + "AmcRedfishInventoryPlugin compares its SSH-proxy targets. The two are not mixed.", + "A different Version, a member missing on a connected target, or a missing or empty " + "Version (recorded as N/A) logs one FW warning per component.", + "Each warning records Version, Status Health, Status State, SoftwareId, Updateable, " + "and Oem SVN. The component id is the published Id and is not normalized.", + "Warnings are written to the analyzer events.json. result.json has the summary count. " + "The console summary is that count, not the per-component table.", + "Fewer than two connected targets, a target that failed to connect, or " + "compare_firmware false does not warn. --data loads one model and does not compare nodes.", + "When no FirmwareInventory members were collected, one warning says the comparison did not run.", + ) + + def analyze_data( + self, + data: RedfishInventoryDataModel, + args: Optional[RedfishInventoryAnalyzerArgs] = None, + ) -> TaskResult: + """Compare firmware for a single collected inventory model. + + Args: + data: Inventory collected from one target. + args: Analyzer arguments. Defaults to comparing firmware. + + Returns: + TaskResult: OK because one model has no other target to compare. + """ + self._apply_firmware_comparison({"target": data}, args or RedfishInventoryAnalyzerArgs()) + return self.result + + def compare_firmware( + self, + targets: dict[str, RedfishInventoryDataModel], + args: Optional[RedfishInventoryAnalyzerArgs] = None, + ) -> TaskResult: + """Compare firmware across every target collected by one plugin. + + Args: + targets: Target key to the inventory collected from that target. + args: Analyzer arguments. Defaults to comparing firmware. + + Returns: + TaskResult: WARNING when firmware differs, otherwise OK or NOT_RAN. + """ + self.result = self._init_result() + self._apply_firmware_comparison(targets, args or RedfishInventoryAnalyzerArgs()) + result = self.result + result.finalize(self.logger) + sample = next(iter(targets.values()), None) + self._run_hooks(result, data=sample) + return result + + def _apply_firmware_comparison( + self, + targets: dict[str, RedfishInventoryDataModel], + args: RedfishInventoryAnalyzerArgs, + ) -> None: + """Set the analysis result for a firmware comparison. + + Args: + targets: Target key to the inventory collected from that target. + args: Analyzer arguments. + + Returns: + None. + """ + if not args.compare_firmware: + self.result.status = ExecutionStatus.OK + self.result.message = "Firmware comparison disabled" + return + if not targets: + self.result.status = ExecutionStatus.NOT_RAN + self.result.message = "No Redfish inventory available to compare" + return + if len(targets) < 2: + self.result.status = ExecutionStatus.OK + self.result.message = "Firmware comparison needs at least two connected targets" + return + + by_target = {key: firmware_components(model) for key, model in targets.items()} + if not any(by_target.values()): + self._log_event( + category=EventCategory.FW, + description=( + "Firmware comparison did not run: no FirmwareInventory members were collected" + ), + priority=EventPriority.WARNING, + console_log=True, + ) + self.result.status = ExecutionStatus.WARNING + self.result.message = "No FirmwareInventory members were collected" + return + + mismatches = firmware_mismatches(by_target) + for mismatch in mismatches: + parts = [ + f"{target}={firmware_target_text(record)}" + for target, record in mismatch["targets"].items() + ] + description = f"Firmware {mismatch['firmware_id']} differs: " + ", ".join(parts) + self._log_event( + category=EventCategory.FW, + description=description, + data={ + "firmware_id": mismatch["firmware_id"], + "targets": mismatch["targets"], + }, + priority=EventPriority.WARNING, + console_log=True, + ) + if mismatches: + self.result.status = ExecutionStatus.WARNING + self.result.message = f"Firmware differs for {len(mismatches)} component(s)" + return + self.result.status = ExecutionStatus.OK + self.result.message = f"Firmware versions match across {len(targets)} targets" diff --git a/nodescraper/plugins/ooband/redfish_inventory/inventory_collector.py b/nodescraper/plugins/ooband/redfish_inventory/inventory_collector.py new file mode 100644 index 00000000..5efb8a95 --- /dev/null +++ b/nodescraper/plugins/ooband/redfish_inventory/inventory_collector.py @@ -0,0 +1,258 @@ +############################################################################### +# +# 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 Any, Optional + +from nodescraper.base import RedfishDataCollector +from nodescraper.connection.redfish import RF_MEMBERS, RF_MEMBERS_NEXT_LINK, RF_ODATA_ID +from nodescraper.connection.redfish.ssh_proxy_connection import ( + SshProxyRedfishConnection, +) +from nodescraper.enums import EventCategory, EventPriority, ExecutionStatus +from nodescraper.models import TaskResult + +from .collector_args import RedfishInventoryCollectorArgs +from .inventory_data import RedfishInventoryDataModel + + +def _reference_path(value: Any) -> str: + """Return a fetchable Redfish path from a link value. + + Args: + value: An OData id string or a dict containing one. + + Returns: + The absolute path, or an empty string when the value is not fetchable. + """ + if isinstance(value, dict): + value = value.get(RF_ODATA_ID) + if not isinstance(value, str): + return "" + text = value.strip() + if not text or "#" in text or not text.startswith("/"): + return "" + return text.rstrip("/") or "/" + + +def _member_paths(body: dict) -> list[str]: + """Return member resource paths from a collection body. + + Args: + body: A Redfish collection payload. + + Returns: + Absolute member paths. + """ + paths: list[str] = [] + for item in body.get(RF_MEMBERS) or []: + if isinstance(item, dict): + path = _reference_path(item.get(RF_ODATA_ID)) + if path: + paths.append(path) + return paths + + +def _named_path(body: dict, name: str) -> str: + """Return one named link from a Redfish resource. + + Args: + body: A Redfish resource payload. + name: Property that may hold an OData link. + + Returns: + The linked path, or an empty string when the property is absent. + """ + return _reference_path(body.get(name)) + + +class RedfishInventoryCollector( + RedfishDataCollector[RedfishInventoryDataModel, RedfishInventoryCollectorArgs] +): + """Collect standard Redfish inventory resources without product-specific validation.""" + + DATA_MODEL = RedfishInventoryDataModel + + DOCUMENTATION_COLLECTION_ITEMS: tuple[str, ...] = ( + "GET the Redfish service root and follow standard collection links such as Chassis, " + "Managers, Systems, TelemetryService, and UpdateService.", + "Expand collection members and follow Assembly, PowerSubsystem, FruDevices, Memory, " + "Processors, FirmwareInventory, and metric-report links when the service publishes them.", + "collection_args.collections and collection_args.member_links append extra property " + "names to that profile set. Names already in the set are ignored.", + "Uses the active Redfish connection, including a direct session or the SSH proxy.", + "An SSH-proxy HTTP 404 for a listed collection member is an empty slot and is not a failure.", + ) + + def collect_data( + self, args: Optional[RedfishInventoryCollectorArgs] = None + ) -> tuple[TaskResult, Optional[RedfishInventoryDataModel]]: + """Collect the selected standard Redfish inventory resources. + + Args: + args: Profile and optional collection overrides. + + Returns: + The task result and collected responses, or None when the service root cannot be read. + """ + collect_args = args or RedfishInventoryCollectorArgs() + api_root = getattr(self.connection, "api_root", "redfish/v1") + root_path = "/" + str(api_root).strip("/") + responses: dict[str, dict] = {} + seen: set[str] = set() + found_collections = 0 + missing: list[str] = [] + + root = self._fetch(root_path, responses, seen) + if root is None: + self.result.message = "No Redfish inventory could be read" + self.result.status = ExecutionStatus.ERROR + return self.result, None + + for name in collect_args.selected_collections(): + collection_path = _named_path(root, name) + if not collection_path: + missing.append(name) + continue + found_collections += 1 + for body in self._fetch_pages(collection_path, responses, seen, collect_args.max_pages): + self._follow_links(body, responses, seen, collect_args) + for member_path in _member_paths(body): + member = self._fetch(member_path, responses, seen, allow_missing=True) + if member is not None: + self._follow_links(member, responses, seen, collect_args) + + for name in missing: + self._log_event( + category=EventCategory.RUNTIME, + description=f"Redfish service root has no {name} link", + priority=EventPriority.WARNING, + console_log=False, + ) + + data = RedfishInventoryDataModel(responses=responses) + if found_collections == 0: + self.result.message = "No requested Redfish inventory collections were present" + self.result.status = ExecutionStatus.WARNING + return self.result, data + + self.result.message = f"Collected {len(responses)} Redfish inventory resource(s)" + self.result.status = ExecutionStatus.OK + return self.result, data + + def _fetch( + self, + path: str, + responses: dict[str, dict], + seen: set[str], + allow_missing: bool = False, + ) -> Optional[dict]: + """GET one Redfish path and store a successful object body. + + Args: + path: Absolute Redfish path. + responses: Collected bodies keyed by path. + seen: Paths already requested. + allow_missing: When True, an SSH-proxy HTTP 404 is an empty slot and is not a warning. + + Returns: + The JSON object, or None when the GET did not return one. + """ + if path in seen: + return responses.get(path) + seen.add(path) + res = self._run_redfish_get(path, log_artifact=True) + if res.success and isinstance(res.data, dict): + responses[path] = res.data + self.logger.info("Collected Redfish resource %s", path) + return res.data + if ( + allow_missing + and res.status_code == 404 + and isinstance(self.connection, SshProxyRedfishConnection) + ): + return None + self._log_event( + category=EventCategory.RUNTIME, + description=f"Redfish GET failed for {path}: {res.error or 'unknown'}", + priority=EventPriority.WARNING, + console_log=True, + ) + return None + + def _fetch_pages( + self, + path: str, + responses: dict[str, dict], + seen: set[str], + max_pages: int, + ) -> list[dict]: + """GET a resource and additional collection pages up to max_pages. + + Args: + path: First page path. + responses: Collected bodies keyed by path. + seen: Paths already requested. + max_pages: Maximum pages to keep. + + Returns: + Bodies that were read, in page order. + """ + bodies: list[dict] = [] + current = path + pages = 0 + while current and pages < max_pages: + body = self._fetch(current, responses, seen) + pages += 1 + if body is None: + break + bodies.append(body) + current = _reference_path(body.get(RF_MEMBERS_NEXT_LINK)) + return bodies + + def _follow_links( + self, + body: dict, + responses: dict[str, dict], + seen: set[str], + args: RedfishInventoryCollectorArgs, + ) -> None: + """Follow configured links and expand one level of their members. + + Args: + body: Resource that may contain the configured links. + responses: Collected bodies keyed by path. + seen: Paths already requested. + args: Collector args selecting which links to follow. + + Returns: + None + """ + for name in args.selected_links(): + link = _named_path(body, name) + if not link: + continue + for linked in self._fetch_pages(link, responses, seen, args.max_pages): + for member_path in _member_paths(linked): + self._fetch(member_path, responses, seen, allow_missing=True) diff --git a/nodescraper/plugins/ooband/redfish_inventory/inventory_data.py b/nodescraper/plugins/ooband/redfish_inventory/inventory_data.py new file mode 100644 index 00000000..a9cb930f --- /dev/null +++ b/nodescraper/plugins/ooband/redfish_inventory/inventory_data.py @@ -0,0 +1,34 @@ +############################################################################### +# +# MIT License +# +# Copyright (c) 2026 Advanced Micro Devices, Inc. +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in all +# copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. +# +############################################################################### +from pydantic import Field + +from nodescraper.models import DataModel + + +class RedfishInventoryDataModel(DataModel): + """Collected standard Redfish inventory responses, keyed by resource path.""" + + responses: dict[str, dict] = Field(default_factory=dict) diff --git a/nodescraper/plugins/ooband/redfish_inventory/inventory_plugin.py b/nodescraper/plugins/ooband/redfish_inventory/inventory_plugin.py new file mode 100644 index 00000000..3f119ab8 --- /dev/null +++ b/nodescraper/plugins/ooband/redfish_inventory/inventory_plugin.py @@ -0,0 +1,170 @@ +############################################################################### +# +# 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 Any, Optional, Union + +from nodescraper.base import OOBandDataPlugin +from nodescraper.connection.redfish import ( + RedfishSshProxyConnectionManager, + collected_multi_target_data, +) +from nodescraper.enums import EventPriority +from nodescraper.models import TaskResult + +from .analyzer_args import RedfishInventoryAnalyzerArgs +from .collector_args import RedfishInventoryCollectorArgs +from .inventory_analyzer import RedfishInventoryAnalyzer +from .inventory_collector import RedfishInventoryCollector +from .inventory_data import RedfishInventoryDataModel + + +class RedfishInventoryPlugin( + OOBandDataPlugin[ + RedfishInventoryDataModel, + RedfishInventoryCollectorArgs, + RedfishInventoryAnalyzerArgs, + ] +): + """Collect portable BMC and SMC Redfish inventory over the native Redfish connection. + + BMC coverage is Chassis, Managers, Assembly, PowerSubsystem, FruDevices, telemetry + metric reports, UpdateService, FirmwareInventory, and Systems. SMC coverage is + ComputerSystem, Memory, Processors, and FirmwareInventory. Every area is enabled + unless collection_args.enabled turns it off. + Collection follows links the service publishes and does not apply product member expectations. + When two or more targets connect, analysis compares FirmwareInventory across those + targets. compare_firmware defaults to true. A different version or a member missing on + a connected target logs an FW warning. AmcRedfishInventoryPlugin runs that comparison + on its own targets. + + Keep these outside node-scraper core: IPMI, SSH commands and file capture, PLDM, + manufacturing-tool access, vendor-only BMC actions, BIOS and diagnostic logs, CPLD, + post codes, SOL, CPER, and product checks for chassis topology, ports, PCIe, sensors, + certificates, and firmware integrity. Non-Redfish fallbacks stay with the product + collector that owns them. + """ + + DATA_MODEL = RedfishInventoryDataModel + COLLECTOR = RedfishInventoryCollector + COLLECTOR_ARGS = RedfishInventoryCollectorArgs + ANALYZER = RedfishInventoryAnalyzer + ANALYZER_ARGS = RedfishInventoryAnalyzerArgs + + def analyze( + self, + max_event_priority_level: Optional[Union[EventPriority, str]] = EventPriority.CRITICAL, + analysis_args: Optional[Union[RedfishInventoryAnalyzerArgs, dict]] = None, + data: Optional[Any] = None, + ) -> TaskResult: + """Compare firmware once across every target this plugin collected. + + Args: + max_event_priority_level: Priority limit for events. + analysis_args: Analyzer arguments. Defaults to comparing firmware. + data: Pre-collected inventory for a single-target run. + + Returns: + TaskResult: WARNING when firmware differs across connected targets. + """ + if data: + self.data = data + args = self._analysis_args(analysis_args) + analyzer = RedfishInventoryAnalyzer( + system_info=self.system_info.model_copy(), + logger=self.logger, + max_event_priority_level=max_event_priority_level or EventPriority.CRITICAL, + parent=self.__class__.__name__, + task_result_hooks=self.task_result_hooks, + event_reporter=self.event_reporter, + session_id=self.session_id, + ) + self.analysis_result = analyzer.compare_firmware(self._firmware_targets(), args) + return self.analysis_result + + def _analysis_args( + self, analysis_args: Optional[Union[RedfishInventoryAnalyzerArgs, dict]] + ) -> RedfishInventoryAnalyzerArgs: + """Return analyzer args, applying the compare_firmware default when omitted. + + Args: + analysis_args: Caller-supplied analyzer arguments. + + Returns: + Validated analyzer arguments. + """ + if analysis_args is None: + return RedfishInventoryAnalyzerArgs() + if isinstance(analysis_args, dict): + return RedfishInventoryAnalyzerArgs.model_validate(analysis_args) + return analysis_args + + def _firmware_targets(self) -> dict[str, RedfishInventoryDataModel]: + """Return inventory models for the connected targets, or the single collected model. + + Args: + None + + Returns: + Target key to inventory model. + """ + models: dict[str, RedfishInventoryDataModel] = {} + for key, value in collected_multi_target_data(self.connection_manager).items(): + model = _as_inventory_model(value) + if model is not None: + models[str(key)] = model + if models: + return models + model = _as_inventory_model(self.data) + if model is None: + return {} + return {"target": model} + + +def _as_inventory_model(value: Any) -> Optional[RedfishInventoryDataModel]: + """Return an inventory model when value is one or a dumped responses dict. + + Args: + value: Collected data for one target. + + Returns: + The inventory model, or None when value is not inventory. + """ + if isinstance(value, RedfishInventoryDataModel): + return value + if isinstance(value, dict) and isinstance(value.get("responses"), dict): + return RedfishInventoryDataModel.model_validate(value) + return None + + +class AmcRedfishInventoryPlugin(RedfishInventoryPlugin): + """Collect the same Redfish inventory through the BMC SSH proxy. + + Pair this with RedfishInventoryPlugin so one run reads the BMC on the direct + Redfish connection and the AMC on RedfishSshProxyConnectionManager. collection_args + and analysis_args are the same for both plugins. Firmware comparison uses only the + targets this plugin connected. + """ + + CONNECTION_TYPE = RedfishSshProxyConnectionManager # type: ignore[assignment] diff --git a/nodescraper/plugins/serviceability/afid_events.py b/nodescraper/plugins/serviceability/afid_events.py index 645d9177..6261f262 100644 --- a/nodescraper/plugins/serviceability/afid_events.py +++ b/nodescraper/plugins/serviceability/afid_events.py @@ -98,12 +98,32 @@ def _afid_event_from_cper_slot(unit: str, payload: Any) -> Optional[AfidEvent]: ) +def _oem_mappings(payload: dict[str, Any]) -> list[dict[str, Any]]: + """Return Oem objects on a log entry, including one nested under CPER. + + Args: + payload: Redfish log member or switch event. + + Returns: + Oem mappings that may contain AMDFieldIdentifiers. + """ + found: list[dict[str, Any]] = [] + oem = payload.get("Oem") + if isinstance(oem, dict): + found.append(oem) + cper = payload.get("CPER") + if isinstance(cper, dict): + nested = cper.get("Oem") + if isinstance(nested, dict): + found.append(nested) + return found + + def _extract_afid(payload: dict[str, Any]) -> Optional[int]: for key in _AFID_KEYS: if key in payload and payload[key] is not None: return int(payload[key]) - oem = payload.get("Oem") - if isinstance(oem, dict): + for oem in _oem_mappings(payload): for vendor_payload in oem.values(): found = _extract_afid_from_oem_fragment(vendor_payload) if found is not None: @@ -148,6 +168,148 @@ def _origin_dict_to_unit(value: Any) -> Optional[str]: return None +def _unit_odata_id(unit: Any) -> Optional[str]: + if not isinstance(unit, dict): + return None + for key in ("@odata.id", "odata.id"): + value = unit.get(key) + if isinstance(value, str) and value.strip(): + return value + return None + + +def usable_serviceable_units(units: Any) -> list[Any]: + """Return ServiceableUnits entries that carry an @odata.id or odata.id string. + + Args: + units: ServiceableUnits value from an AFID identifier entry. + + Returns: + Unit objects that name a location. Empty when none qualify. + """ + if not isinstance(units, list): + return [] + return [unit for unit in units if _unit_odata_id(unit) is not None] + + +def promote_serviceable_unit_odata_ids(member: dict[str, Any]) -> dict[str, Any]: + """Copy odata.id onto @odata.id so the hub can read the location. + + Args: + member: Redfish log member. + + Returns: + The same member, or a copy when a ServiceableUnits entry needed @odata.id. + """ + oem = member.get("Oem") + if not isinstance(oem, dict): + return member + amd = oem.get("AMD") + if not isinstance(amd, dict): + return member + identifiers = amd.get("AMDFieldIdentifiers") + if not isinstance(identifiers, list): + return member + new_identifiers: list[Any] = [] + changed = False + for entry in identifiers: + if not isinstance(entry, dict) or not isinstance(entry.get("ServiceableUnits"), list): + new_identifiers.append(entry) + continue + new_units: list[Any] = [] + entry_changed = False + for unit in entry["ServiceableUnits"]: + if ( + isinstance(unit, dict) + and not isinstance(unit.get("@odata.id"), str) + and isinstance(unit.get("odata.id"), str) + and unit["odata.id"].strip() + ): + copied = dict(unit) + copied["@odata.id"] = unit["odata.id"] + new_units.append(copied) + entry_changed = True + else: + new_units.append(unit) + if entry_changed: + changed = True + new_identifiers.append({**entry, "ServiceableUnits": new_units}) + else: + new_identifiers.append(entry) + if not changed: + return member + return { + **member, + "Oem": {**oem, "AMD": {**amd, "AMDFieldIdentifiers": new_identifiers}}, + } + + +def amd_field_identifiers_with_units( + oem: Any, +) -> Optional[tuple[list[dict[str, Any]], list[Any]]]: + """Split Oem.AMD.AMDFieldIdentifiers by whether ServiceableUnits is usable. + + Args: + oem: Redfish Oem object from a log member. + + Returns: + (usable entries, unusable entries) when Oem.AMD.AMDFieldIdentifiers is a list. + None when that shape is absent so another source shape can be tried. + """ + if not isinstance(oem, dict): + return None + amd = oem.get("AMD") + if not isinstance(amd, dict): + return None + identifiers = amd.get("AMDFieldIdentifiers") + if not isinstance(identifiers, list): + return None + usable: list[dict[str, Any]] = [] + unusable: list[Any] = [] + for entry in identifiers: + units = entry.get("ServiceableUnits") if isinstance(entry, dict) else None + if isinstance(entry, dict) and usable_serviceable_units(units): + usable.append(entry) + else: + unusable.append(entry) + return usable, unusable + + +_LOCATION_EXTRACTORS = (amd_field_identifiers_with_units,) + + +def split_afid_location_entries( + oem: Any, +) -> Optional[tuple[list[dict[str, Any]], list[Any]]]: + """Return usable and unusable AFID entries for the first matching source shape. + + Args: + oem: Redfish Oem object from a log member. + + Returns: + (usable, unusable) for a recognized shape, or None when no shape applies. + """ + for extractor in _LOCATION_EXTRACTORS: + split = extractor(oem) + if split is not None: + return split + return None + + +def _first_unit_from_identifiers(identifiers: Any) -> Optional[str]: + if not isinstance(identifiers, list): + return None + for item in identifiers: + if not isinstance(item, dict): + continue + units = item.get("ServiceableUnits") or item.get("serviceable_units") + for unit in usable_serviceable_units(units): + name = _origin_dict_to_unit(unit) + if name: + return name + return None + + def _extract_serviceable_unit(payload: dict[str, Any]) -> Optional[str]: for key in ("serviceable_unit", "ServiceableUnit", "OriginOfCondition", "Device"): value = payload.get(key) @@ -172,45 +334,78 @@ def _extract_serviceable_unit(payload: dict[str, Any]) -> Optional[str]: if unit: return unit - oem = payload.get("Oem") - if isinstance(oem, dict): - for vendor_payload in oem.values(): + for oem in _oem_mappings(payload): + for vendor_name, vendor_payload in oem.items(): if isinstance(vendor_payload, dict): unit = vendor_payload.get("serviceable_unit") or vendor_payload.get( "ServiceableUnit" ) if unit is not None and str(unit).strip(): return str(unit).strip() - identifiers = vendor_payload.get("AMDFieldIdentifiers") - if isinstance(identifiers, list): - for item in identifiers: - if not isinstance(item, dict): - continue - su = item.get("ServiceableUnits") or item.get("serviceable_units") - if isinstance(su, list) and su: - u = _origin_dict_to_unit(su[0]) - if u: - return u - elif isinstance(vendor_payload, list): - for item in vendor_payload: - if not isinstance(item, dict): - continue - su = item.get("ServiceableUnits") or item.get("serviceable_units") - if isinstance(su, list) and su: - u = _origin_dict_to_unit(su[0]) - if u: - return u + if vendor_name != "AMD": + unit = _first_unit_from_identifiers(vendor_payload.get("AMDFieldIdentifiers")) + if unit: + return unit + else: + unit = _first_unit_from_identifiers(vendor_payload) + if unit: + return unit + split = amd_field_identifiers_with_units(oem) + if split is not None: + unit = _first_unit_from_identifiers(split[0]) + if unit: + return unit + return _switch_serviceable_unit(payload) + + +def _switch_serviceable_unit(payload: dict[str, Any]) -> Optional[str]: + """Return the component name carried by one switch event. + + Args: + payload: Switch event object. + + Returns: + object-ref name, unit id, or node name, or None when this is not a switch event. + """ + if payload.get("kind") != "Event": + return None + object_ref = payload.get("object-ref") + if not isinstance(object_ref, dict): + object_ref = payload.get("object_ref") + if isinstance(object_ref, dict): + name = object_ref.get("name") + if isinstance(name, str) and name.strip(): + return name.strip() + source = payload.get("source") + if not isinstance(source, dict): + return None + for key in ("unit-id", "node-name"): + value = source.get(key) + if isinstance(value, str) and value.strip(): + return value.strip() return None def _extract_timestamp(payload: dict[str, Any]) -> Optional[str]: for key in _EVENT_TIMESTAMP_KEYS: - value = payload.get(key) - if value is not None and str(value).strip(): - return str(value).strip() + text = _timestamp_text(payload.get(key)) + if text: + return text + meta = payload.get("meta") + if isinstance(meta, dict): + for key in ("creation-time", "mod-time"): + text = _timestamp_text(meta.get(key)) + if text: + return text return None +def _timestamp_text(value: Any) -> str: + if value is None: + return "" + return str(value).replace("\x00", "").strip() + + def _unit_from_odata_id(odata_id: str) -> str: segment = odata_id.rstrip("/").split("/")[-1] return segment or odata_id diff --git a/nodescraper/plugins/serviceability/afid_sag_lookup.py b/nodescraper/plugins/serviceability/afid_sag_lookup.py index 52957ce5..a13bee20 100644 --- a/nodescraper/plugins/serviceability/afid_sag_lookup.py +++ b/nodescraper/plugins/serviceability/afid_sag_lookup.py @@ -48,19 +48,46 @@ def load_afid_sag_data(path: Optional[str]) -> Optional[dict[str, Any]]: def normalize_fru_name(name: str) -> str: - """Normalize FRU labels for comparison (SAG serviceable_fru vs afid.fru).""" + """Normalize a FRU or action subject for comparison. + + Args: + name: SAG fru or action_subject label. + + Returns: + Uppercase label with hyphens replaced by underscores. + """ return str(name).strip().upper().replace("-", "_") -def known_fru_names_from_sag(sag: Optional[dict[str, Any]]) -> list[str]: - """Return normalized FRU names declared under serviceable_fru in the SAG.""" +def _serviceable_catalog(sag: Optional[dict[str, Any]]) -> list[Any]: + """Return the SAG list of serviceable groups. + + Args: + sag: Loaded AFID SAG object. + + Returns: + serviceable_fru when that list is present, otherwise serviceable_action_subject. + """ if not sag: return [] - raw = sag.get("serviceable_fru") - if not isinstance(raw, list): - return [] + for key in ("serviceable_fru", "serviceable_action_subject"): + raw = sag.get(key) + if isinstance(raw, list) and raw: + return raw + return [] + + +def known_fru_names_from_sag(sag: Optional[dict[str, Any]]) -> list[str]: + """Return normalized serviceable group names from the SAG catalog. + + Args: + sag: Loaded AFID SAG object. + + Returns: + Normalized fru or action_subject names declared in the SAG catalog. + """ names: list[str] = [] - for item in raw: + for item in _serviceable_catalog(sag): if not isinstance(item, dict): continue for key in item: @@ -71,14 +98,16 @@ def known_fru_names_from_sag(sag: Optional[dict[str, Any]]) -> list[str]: def sag_fru_display_names(sag: Optional[dict[str, Any]]) -> list[str]: - """Return sorted FRU labels from SAG serviceable_fru using the original key text.""" - if not sag: - return [] - raw = sag.get("serviceable_fru") - if not isinstance(raw, list): - return [] + """Return serviceable group labels from the SAG catalog. + + Args: + sag: Loaded AFID SAG object. + + Returns: + Original fru or action_subject key text from the SAG catalog. + """ names: list[str] = [] - for item in raw: + for item in _serviceable_catalog(sag): if not isinstance(item, dict): continue for key in item: @@ -100,14 +129,23 @@ def afid_entry_from_sag(afid: int, sag: Optional[dict[str, Any]]) -> Optional[di def afid_fru_from_sag(afid: int, sag: Optional[dict[str, Any]]) -> Optional[str]: - """Return the FRU string for an AFID from the SAG afid table.""" + """Return the FRU or action subject for an AFID from the SAG afid table. + + Args: + afid: AMD Fault ID. + sag: Loaded AFID SAG object. + + Returns: + afid.fru when set, otherwise afid.action_subject. None when neither is set. + """ entry = afid_entry_from_sag(afid, sag) if not entry: return None - fru = entry.get("fru") - if fru is None or not str(fru).strip(): - return None - return str(fru).strip() + for key in ("fru", "action_subject"): + value = entry.get(key) + if value is not None and str(value).strip(): + return str(value).strip() + return None def afid_summary_from_sag(afid: int, sag: Optional[dict[str, Any]]) -> Optional[str]: @@ -182,7 +220,15 @@ def group_afid_events_by_fru( events: list[AfidEvent], sag: Optional[dict[str, Any]], ) -> dict[str, list[AfidEvent]]: - """Group parsed AFID events by SAG FRU (unknown FRU bucket when lookup fails).""" + """Group parsed AFID events by SAG fru or action subject. + + Args: + events: Parsed AFID events. + sag: Loaded AFID SAG object. + + Returns: + Events keyed by normalized fru or action subject. Missing labels use UNKNOWN_FRU. + """ grouped: dict[str, list[AfidEvent]] = defaultdict(list) for event in events: fru = afid_fru_from_sag(event.afid, sag) or "UNKNOWN_FRU" @@ -215,13 +261,6 @@ def format_collected_afid_fru_summary_lines( unit_bits = ", ".join(f"{unit} x{count}" for unit, count in sorted(unit_counts.items())) lines.append(f" {fru}: AFIDs [{afid_bits}]") lines.append(f" units: {unit_bits}") - known = known_fru_names_from_sag(sag) - if known: - missing = sorted(set(known) - set(grouped.keys())) - if missing: - lines.append(f" FRUs in SAG with no collected AFID events: {', '.join(missing)}") - else: - lines.append(" All SAG serviceable FRUs have at least one collected AFID event.") return lines diff --git a/nodescraper/plugins/serviceability/mi4xx/mi4xx_analyzer.py b/nodescraper/plugins/serviceability/mi4xx/mi4xx_analyzer.py index 6676a370..91613bcb 100644 --- a/nodescraper/plugins/serviceability/mi4xx/mi4xx_analyzer.py +++ b/nodescraper/plugins/serviceability/mi4xx/mi4xx_analyzer.py @@ -25,10 +25,18 @@ ############################################################################### from __future__ import annotations +import json +from datetime import datetime, timezone + +from nodescraper.connection.inband import TextFileArtifact +from nodescraper.plugins.serviceability.se_models import ServiceabilityBlock +from nodescraper.plugins.serviceability.serviceability_api import top_action_summary from nodescraper.plugins.serviceability.serviceability_hub_analyzer import ( ServiceabilityHubAnalyzer, ) +SUMMARY_FILENAME = "summary.json" + class MI4XXAnalyzer(ServiceabilityHubAnalyzer): """Build AFID events from collected data and run the configured entry-point hub.""" @@ -38,4 +46,21 @@ class MI4XXAnalyzer(ServiceabilityHubAnalyzer): "Runs the configured entry-point service hub (hub_entry_point in analysis_args) to produce service recommendations.", "When analysis_args.skip_hub is true, only builds AFID events without running the hub.", "Supports offline analysis from a prior collection via --data with --collection False.", + f"Writes the top service action to {SUMMARY_FILENAME} in the analyzer log directory.", ) + + def _append_hub_artifacts(self, block: ServiceabilityBlock) -> None: + super()._append_hub_artifacts(block) + self.result.artifacts.append( + TextFileArtifact( + filename=SUMMARY_FILENAME, + contents=json.dumps( + top_action_summary( + block, + system_name=self.system_info.name, + analyzed_at=datetime.now(timezone.utc).isoformat(), + ), + indent=2, + ), + ) + ) diff --git a/nodescraper/plugins/serviceability/mi4xx/serviceability_plugin_mi4xx.py b/nodescraper/plugins/serviceability/mi4xx/serviceability_plugin_mi4xx.py index 464b7b88..ab542dd3 100644 --- a/nodescraper/plugins/serviceability/mi4xx/serviceability_plugin_mi4xx.py +++ b/nodescraper/plugins/serviceability/mi4xx/serviceability_plugin_mi4xx.py @@ -25,12 +25,12 @@ ############################################################################### from __future__ import annotations -from typing import Annotated, Any, Optional, Union +from typing import Annotated, Any, Optional, Union, cast from pydantic import Field -from nodescraper.enums import EventPriority, ExecutionStatus, SystemInteractionLevel -from nodescraper.models import DataPluginResult, PluginResult, TaskResult +from nodescraper.enums import EventPriority, SystemInteractionLevel +from nodescraper.models import PluginResult from nodescraper.plugins.serviceability.afid_sag_lookup import log_afid_fru_summary from nodescraper.plugins.serviceability.serviceability_data import ( ServiceabilityDataLoadError, @@ -140,25 +140,22 @@ def _merge_mi4xx_collection_args( return collection_args.model_copy(update={"rf_event_log_uri": uri_text}) return collection_args - def _plugin_error_result(self, message: str) -> PluginResult: - self.logger.error("(%s) %s", self.__class__.__name__, message) - return PluginResult( - status=ExecutionStatus.ERROR, - source=self.__class__.__name__, - message=message, - result_data=DataPluginResult( - collection_result=TaskResult( - status=ExecutionStatus.NOT_RAN, - parent=self.__class__.__name__, - message="Data collection skipped", - ), - analysis_result=TaskResult( - status=ExecutionStatus.ERROR, - parent=self.__class__.__name__, - message=message, - ), - ), - ) + def _apply_afid_sag_path_to_collection_args( + self, + afid_sag_path: str, + collection_args: Any, + ) -> Any: + """Copy a resolved AFID_SAG path onto MI4xx collector args. + + Args: + afid_sag_path: Absolute path returned by preflight. + collection_args: MI4xx collector args for this run, or None. + + Returns: + Collector args with afid_sag_path set. + """ + merged_collection, _ = self._merge_afid_sag_path(afid_sag_path, collection_args, None) + return merged_collection def run( # type: ignore[override] self, @@ -198,6 +195,16 @@ def run( # type: ignore[override] analysis_args, ) + if analysis: + preflight = self._preflight_hub_sag(collection_args, analysis_args) + if isinstance(preflight, PluginResult): + return preflight + collection_args, updated_analysis = preflight + analysis_args = cast( + Optional[Union[Mi4xxServiceabilityAnalyzerArgs, dict[str, Any]]], + updated_analysis, + ) + if analysis and not collection and data is not None: try: loaded = ( diff --git a/nodescraper/plugins/serviceability/se_runner.py b/nodescraper/plugins/serviceability/se_runner.py index dc7b74de..1d1682cb 100644 --- a/nodescraper/plugins/serviceability/se_runner.py +++ b/nodescraper/plugins/serviceability/se_runner.py @@ -31,17 +31,30 @@ import importlib import importlib.metadata import inspect +import logging +import re from pathlib import Path -from typing import Any, Callable, Optional, Protocol, Type, Union, cast +from typing import TYPE_CHECKING, Any, Callable, Optional, Protocol, Type, Union, cast -from .afid_sag_paths import validate_afid_sag_path +from nodescraper.constants import DEFAULT_LOGGER + +from .afid_events import promote_serviceable_unit_odata_ids, split_afid_location_entries +from .afid_sag_paths import default_afid_sag_path, validate_afid_sag_path from .se_adapter import ( serviceability_block_from_entry_point_hub, serviceability_block_from_service_result, ) from .se_models import AfidEvent, ServiceabilityBlock +if TYPE_CHECKING: + from .analyzer_args import ServiceabilityAnalyzerArgs + HUB_ENTRY_POINT_GROUP = "amd.serviceability_engines" +_MIN_AFSE_VERSION = (0, 2) +# Leading major.minor. A patch, pre-release, or local .dev suffix is ignored. +_AFSE_VERSION_RE = re.compile(r"^v?(?P\d+)\.(?P\d+)") +# setuptools_scm no-guess-dev with no ancestor tag: 0.0.post1.devN+gHASH +_UNTAGGED_AFSE_RE = re.compile(r"^v?0\.0(?:\.post\d+)?\.dev\d+(?:\+[0-9A-Za-z.]+)?$") def normalize_hub_entry_point(hub_name: str) -> str: @@ -292,15 +305,78 @@ def events_payload_for_hub_analyze( raise HubRunError("rf_events or afid_events is required") +def _redfish_severity_is_ok(member: dict[str, Any]) -> bool: + """Return whether a Redfish log member is an OK event. + + Args: + member: Redfish log member. + + Returns: + True when Severity is OK. + """ + severity = member.get("Severity") + return isinstance(severity, str) and severity.strip().upper() == "OK" + + +def drop_afids_without_serviceable_units( + rf_events: list[Any], + logger: Optional[logging.Logger] = None, +) -> list[Any]: + """Waive AMDFieldIdentifiers entries the hub would reject for lacking ServiceableUnits. + + The entry-point hub rejects the whole event list when any Oem.AMD.AMDFieldIdentifiers + entry has no ServiceableUnits, so drop those entries before analysis. OK events are + dropped quietly. Any other severity is logged. + + Args: + rf_events: Raw Redfish log members. + logger: Logger for the per-entry warning. Defaults to the nodescraper logger. + + Returns: + Members with unusable AFID entries removed. Members left with no AFID entries are dropped. + """ + log = logger or logging.getLogger(DEFAULT_LOGGER) + kept: list[Any] = [] + for member in rf_events: + oem = member.get("Oem") if isinstance(member, dict) else None + split = split_afid_location_entries(oem) + if split is None: + kept.append(member) + continue + usable, unusable = split + if not _redfish_severity_is_ok(member): + for entry in unusable: + afid = entry.get("AFID") if isinstance(entry, dict) else None + log.warning( + "Waiving AFID %s from %s: AMDFieldIdentifiers entry has no ServiceableUnits", + afid, + member.get("@odata.id") or member.get("Id") or "", + ) + if not unusable: + kept.append(promote_serviceable_unit_odata_ids(member)) + elif usable: + amd = cast(dict, oem).get("AMD") + amd_copy = {**cast(dict, amd), "AMDFieldIdentifiers": usable} + kept.append( + promote_serviceable_unit_odata_ids( + {**member, "Oem": {**cast(dict, oem), "AMD": amd_copy}} + ) + ) + return kept + + def resolve_hub_events_payload( *, afid_events: list[AfidEvent], rf_events: Optional[list[Any]] = None, prefer_rf_events: bool = True, + logger: Optional[logging.Logger] = None, ) -> list[Any]: """Choose rf_events or aggregated afid_events for hub analyze().""" if prefer_rf_events and rf_events: - return events_payload_for_hub_analyze(rf_events=rf_events) + usable_rf_events = drop_afids_without_serviceable_units(rf_events, logger) + if usable_rf_events: + return events_payload_for_hub_analyze(rf_events=usable_rf_events) return events_payload_for_hub_analyze( afid_events_payload=afid_events_to_entry_point_payload(afid_events) ) @@ -444,6 +520,252 @@ def _hub_analyze_response_from_raw(raw_result: Any) -> dict[str, Any]: return dataclasses.asdict(raw_result) +def _response_field(result: Any, name: str) -> Any: + """Read a field from an AFSE TriageResponse dataclass or a dict payload.""" + if isinstance(result, dict): + return result.get(name) + return getattr(result, name, None) + + +def validate_afid_sag_with_hub(hub: Any, sag_path: str) -> bool: + """Ask the loaded hub to validate the AFID SAG file via its validate() API. + + Args: + hub: Loaded entry-point hub (AFSE). + sag_path: AFID SAG file to validate. + + Returns: + True when the hub validated the file, False when the hub has no validate(). + + Raises: + HubRunError: When validate() raises or reports a status other than ok. + """ + validate = getattr(hub, "validate", None) + if not callable(validate): + return False + try: + result = validate(sag_path) + except Exception as exc: # noqa: BLE001 + raise HubRunError(f"AFSE SAG validation failed: {exc}") from exc + status = str(_response_field(result, "status") or "").strip().lower() + if status == "ok": + return True + error = _response_field(result, "error") + code = _response_field(error, "code") if error is not None else None + message = _response_field(error, "message") if error is not None else None + detail = str(message or code or status or "unknown error") + raise HubRunError(f"AFSE SAG validation failed: {detail}") + + +def resolve_runtime_afid_sag_path( + args: ServiceabilityAnalyzerArgs, + logger: Optional[logging.Logger] = None, +) -> str: + """Return the AFID SAG file analysis should open. + + A configured afid_sag_path wins. Otherwise the loaded entry-point hub is asked for its + default_sag_path(), falling back to the built-in /opt/amd/afid/AFID_SAG.json. + + Args: + args: Analyzer args for this run. + logger: Logger for the "path omitted" line. Defaults to the nodescraper logger. + + Returns: + SAG path to use for analysis. + """ + if args.afid_sag_path and str(args.afid_sag_path).strip(): + return str(args.afid_sag_path).strip() + hub: Any = None + if args.uses_entry_point_hub(): + try: + hub = load_hub_from_entry_point(args.resolved_hub_entry_point()) + except (HubRunError, ValueError): + hub = None + path = default_afid_sag_path() + lookup = getattr(hub, "default_sag_path", None) + if callable(lookup): + found = lookup() + if found is not None and str(found).strip(): + path = str(found).strip() + (logger or logging.getLogger(DEFAULT_LOGGER)).info( + "AFID_SAG path omitted; using default: %s", path + ) + return path + + +def _untagged_afse_has_redfish_and_switch(version: str) -> bool: + """Return whether an untagged AFSE build can parse Redfish and switch events. + + Args: + version: Version reported by the AFSE package or module hub. + + Returns: + True when version has no release tag and both parsers import. + """ + if _UNTAGGED_AFSE_RE.match(version.strip()) is None: + return False + try: + redfish = importlib.import_module("serviceability_engine.transformers.redfish") + switch = importlib.import_module("serviceability_engine.transformers.switch") + except ImportError: + return False + return callable(getattr(redfish, "parse", None)) and callable(getattr(switch, "parse", None)) + + +def _afse_version_error( + version: str, + logger: Optional[logging.Logger] = None, +) -> Optional[str]: + """Return an error when AFSE is older than 0.2. + + Args: + version: Version reported by the AFSE package or module hub. + logger: Logger used when an untagged build is accepted. + + Returns: + Error text when the major.minor is below 0.2 or missing, otherwise None. + """ + if _untagged_afse_has_redfish_and_switch(version): + (logger or logging.getLogger(DEFAULT_LOGGER)).info( + "AFSE %s is an untagged build with Redfish and switch event support", + version, + ) + return None + match = _AFSE_VERSION_RE.match(version.strip()) + found = (int(match.group("major")), int(match.group("minor"))) if match else None + if found is None or found < _MIN_AFSE_VERSION: + return ( + f"AFSE {_MIN_AFSE_VERSION[0]}.{_MIN_AFSE_VERSION[1]} or newer is required " + f"(Redfish log entries and switch events); found {version}" + ) + return None + + +def _entry_point_distribution_version(hub_name: str) -> Optional[str]: + """Return the installed package version for a registered AFSE entry point. + + Args: + hub_name: Entry point name such as afse. + + Returns: + Distribution version string, or None when it cannot be resolved. + """ + wanted = normalize_hub_entry_point(hub_name) + for ep in _entry_points_for_group(HUB_ENTRY_POINT_GROUP): + if getattr(ep, "name", None) != wanted: + continue + dist = getattr(ep, "dist", None) + version = getattr(dist, "version", None) if dist is not None else None + if version: + return str(version) + value = str(getattr(ep, "value", "") or getattr(ep, "module", "") or "") + top = value.split(":")[0].split(".")[0].strip() + if not top: + continue + try: + return importlib.metadata.version(top) + except importlib.metadata.PackageNotFoundError: + continue + return None + + +def _probe_entry_point_hub( + args: ServiceabilityAnalyzerArgs, +) -> tuple[Any, str, str]: + """Load the configured AFSE entry point and return the hub, label, and version. + + Args: + args: Analyzer args for this run. + + Returns: + Loaded hub instance, display label, and version string. + """ + name = args.resolved_hub_entry_point() + hub = load_hub_from_entry_point(name) + label = args.hub_display_name or getattr(hub, "name", None) or name + version = _entry_point_distribution_version(name) or _hub_engine_version(hub) + return hub, str(label), version + + +def preflight_module_hub( + args: ServiceabilityAnalyzerArgs, + logger: Optional[logging.Logger] = None, +) -> None: + """Check that a Python-module hub is installed and new enough. + + Args: + args: Analyzer args that select hub_python_module. + logger: Logger for the install line. Defaults to the nodescraper logger. + + Returns: + None. + + Raises: + HubRunError: When the module cannot be imported or AFSE is older than 0.2. + """ + log = logger or logging.getLogger(DEFAULT_LOGGER) + module_name = str(args.hub_python_module) + try: + mod = importlib.import_module(module_name) + except ImportError as exc: + raise HubRunError(f"AFSE hub module {module_name!r} is not installed: {exc}") from exc + hub_cls = _resolve_hub_class(mod, args.hub_analyze_method) + label = args.hub_display_name or getattr(hub_cls, "name", None) or module_name + version = getattr(mod, "__version__", None) or _hub_engine_version(hub_cls) + if callable(version): + version = version() + version_text = version.strip() if isinstance(version, str) and version.strip() else "unknown" + log.info("AFSE install found: %s version %s", label, version_text) + version_error = _afse_version_error(version_text, log) + if version_error: + raise HubRunError(version_error) + + +def preflight_afid_sag( + args: ServiceabilityAnalyzerArgs, + logger: Optional[logging.Logger] = None, +) -> str: + """Resolve the AFID SAG file and check it with AFSE before any collection runs. + + Args: + args: Analyzer args for this run (entry-point hub). + logger: Logger for the OK lines. Defaults to the nodescraper logger. + + Returns: + Resolved absolute SAG path. + + Raises: + HubRunError: When the file is missing, the hub cannot load, AFSE is too old, or AFSE rejects the file. + """ + log = logger or logging.getLogger(DEFAULT_LOGGER) + errors: list[str] = [] + hub: Any = None + hub_label = "" + try: + hub, hub_label, hub_version = _probe_entry_point_hub(args) + log.info("AFSE install found: %s version %s", hub_label, hub_version) + version_error = _afse_version_error(hub_version, log) + if version_error: + errors.append(version_error) + except (HubRunError, ValueError) as exc: + errors.append(str(exc)) + + resolved = "" + try: + sag_path = resolve_runtime_afid_sag_path(args, log) + validate_afid_sag_path(sag_path) + resolved = str(Path(sag_path).expanduser().resolve()) + log.info("AFID_SAG file OK: %s", resolved) + if hub is not None and validate_afid_sag_with_hub(hub, resolved): + log.info("AFID_SAG validated by %s: %s", getattr(hub, "name", hub_label), resolved) + except HubRunError as exc: + errors.append(str(exc)) + + if errors: + raise HubRunError("; ".join(errors)) + return resolved + + def run_entry_point_hub( *, hub_entry_point: str, @@ -454,20 +776,27 @@ def run_entry_point_hub( rf_event_count: int = 0, raise_on_error: bool = False, prefer_rf_events: bool = True, + logger: Optional[logging.Logger] = None, ) -> ServiceabilityBlock: """Run a registered entry-point service hub and return a :class:`ServiceabilityBlock`.""" if not afid_events and not (prefer_rf_events and rf_events): raise HubRunError("No AFID events to analyze") + log = logger or logging.getLogger(DEFAULT_LOGGER) validate_afid_sag_path(afid_sag_path) + resolved_sag = Path(afid_sag_path).expanduser().resolve() + log.debug("AFID_SAG file OK: %s", resolved_sag) label = hub_display_name or normalize_hub_entry_point(hub_entry_point) hub = load_hub_from_entry_point(hub_entry_point) + if validate_afid_sag_with_hub(hub, str(resolved_sag)): + log.debug("AFID_SAG validated by %s: %s", getattr(hub, "name", label), resolved_sag) hub_label = getattr(hub, "name", label) engine_version = _hub_engine_version(hub) events = resolve_hub_events_payload( afid_events=afid_events, rf_events=rf_events, prefer_rf_events=prefer_rf_events, + logger=logger, ) try: raw_result = hub.analyze(events, afid_sag_path) diff --git a/nodescraper/plugins/serviceability/serviceability_api.py b/nodescraper/plugins/serviceability/serviceability_api.py index 30f1ea2d..5cfc8ea1 100644 --- a/nodescraper/plugins/serviceability/serviceability_api.py +++ b/nodescraper/plugins/serviceability/serviceability_api.py @@ -25,7 +25,7 @@ ############################################################################### from __future__ import annotations -from typing import Any +from typing import Any, Optional from .se_models import ( HubTriageResult, @@ -89,8 +89,210 @@ def _solution_to_action(rank: int, solution: ServiceabilitySolution) -> Prioriti ) +def _optional_int(value: Any) -> Optional[int]: + """Return value as an int when it is numeric. + + Args: + value: Field from an AFSE triage row. + + Returns: + The integer value, or None when value is missing or not numeric. + """ + if value is None: + return None + try: + return int(value) + except (TypeError, ValueError): + return None + + +def _afse_service_action(row: dict[str, Any]) -> dict[str, Any]: + """Return the service_action object on an AFSE triage row. + + Args: + row: One AFSE triage.top or triage.results entry. + + Returns: + The service_action mapping, or an empty mapping when it is absent. + """ + service_action = row.get("service_action") + if isinstance(service_action, dict): + return service_action + return {} + + +def _afse_action_title(row: dict[str, Any]) -> Optional[str]: + """Return the service action title from an AFSE triage row. + + Args: + row: One AFSE triage.top or triage.results entry. + + Returns: + The title text, or None when the row has no title. + """ + service_action = _afse_service_action(row) + for candidate in (service_action.get("title"), row.get("service_action_title")): + if candidate is None: + continue + text = str(candidate).strip() + if text: + return text + return None + + +def _afse_action_steps(row: dict[str, Any]) -> list[str]: + """Return service action step descriptions from an AFSE triage row. + + Args: + row: One AFSE triage.top or triage.results entry. + + Returns: + Step description strings in AFSE order. + """ + steps = _afse_service_action(row).get("steps") + if not isinstance(steps, list): + return [] + descriptions: list[str] = [] + for step in steps: + if not isinstance(step, dict): + continue + description = step.get("description") + if description is None: + continue + text = str(description).strip() + if text: + descriptions.append(text) + return descriptions + + +def _action_from_afse_row(rank: int, row: dict[str, Any]) -> Optional[PrioritizedServiceAction]: + """Map one AFSE triage row to a table action. + + Args: + rank: 1-based position of this row inside triage.top. + row: One AFSE triage.top entry. + + Returns: + The table action, or None when the row has no AFID, action number, or location. + """ + afid_raw = row.get("afid", row.get("afid_num")) + san_raw = row.get("service_action_num") + location = row.get("location") or row.get("serviceable_unit") + if afid_raw is None or san_raw is None or location is None: + return None + try: + afid = int(afid_raw) + service_action_num = int(san_raw) + except (TypeError, ValueError): + return None + location_text = str(location).strip() + if not location_text: + return None + service_action = _afse_service_action(row) + tier_label = row.get("tier_label") + if tier_label is not None: + tier_label = str(tier_label).strip() or None + category = service_action.get("category") + category_text = ( + str(category).strip() if category is not None and str(category).strip() else None + ) + return PrioritizedServiceAction( + rank=rank, + afid=afid, + location=location_text, + count=max(1, _optional_int(row.get("count")) or 1), + service_action_num=service_action_num, + service_action_title=_afse_action_title(row), + service_action_category=category_text, + priority=_optional_int(row.get("priority")), + sa_severity=_optional_int(row.get("sa_severity")) + or _optional_int(service_action.get("severity")), + tier=_optional_int(row.get("tier")), + tier_label=tier_label, + hub_sort_priority=_optional_int( + row.get("se_sort_priority") or row.get("hub_sort_priority") + ), + multi_mask=_optional_int(row.get("multi_mask")), + service_action_steps=_afse_action_steps(row), + ) + + +def actions_from_afse_triage_top(block: ServiceabilityBlock) -> list[PrioritizedServiceAction]: + """Return table rows for AFSE triage.top. + + Args: + block: Serviceability block that may hold the raw hub analyze response. + + Returns: + One action per AFSE triage.top entry. Empty when that list is absent. + """ + response = block.hub_analyze_response + if not isinstance(response, dict): + return [] + triage = response.get("triage") + if not isinstance(triage, dict): + return [] + top = triage.get("top") + if not isinstance(top, list): + return [] + actions: list[PrioritizedServiceAction] = [] + for index, row in enumerate(top, start=1): + if not isinstance(row, dict): + continue + action = _action_from_afse_row(index, row) + if action is not None: + actions.append(action) + return actions + + +def _afse_triage_section(block: ServiceabilityBlock) -> Optional[dict[str, Any]]: + """Return the AFSE triage object when the hub response includes one. + + Args: + block: Serviceability block populated by hub analysis. + + Returns: + The triage mapping, or None when the response has no triage section. + """ + response = block.hub_analyze_response + if not isinstance(response, dict): + return None + triage = response.get("triage") + if not isinstance(triage, dict): + return None + return triage + + +def afse_top_action_missing(block: ServiceabilityBlock) -> bool: + """Return whether an AFSE triage section has no top action. + + Args: + block: Serviceability block populated by hub analysis. + + Returns: + True when triage is present and triage.top has no usable action. + """ + if _afse_triage_section(block) is None: + return False + return not actions_from_afse_triage_top(block) + + def build_top_service_actions(block: ServiceabilityBlock) -> list[PrioritizedServiceAction]: - """Return Hub triage.top service actions with ranks from triage.results order.""" + """Return the AFSE triage.top service actions for display. + + Args: + block: Serviceability block populated by hub analysis. + + Returns: + Actions from AFSE triage.top when that list has entries. Empty when triage + is present but triage.top is empty. Parsed hub rows are used when the + response has no triage section. + """ + afse_top = actions_from_afse_triage_top(block) + if afse_top: + return afse_top + if _afse_triage_section(block) is not None: + return [] if block.hub_top_results: rank_by_identity = { _hub_triage_identity(row): rank @@ -131,6 +333,8 @@ def split_recommendation_actions( if not prioritized: return top, [] if not top: + if _afse_triage_section(block) is not None: + return [], prioritized return [prioritized[0]], prioritized[1:] top_sort_priorities = { @@ -149,17 +353,44 @@ def split_recommendation_actions( return top, additional -def prepare_serviceability_block_for_export( +def top_action_summary( block: ServiceabilityBlock, -) -> ServiceabilityBlock: - """Return a JSON-safe serviceability block without duplicate hub triage rows.""" - return block.model_copy(update={"hub_triage_results": [], "hub_top_results": []}) + system_name: str, + analyzed_at: str, +) -> dict[str, Any]: + """Build a top-service-action summary payload from a hub result. + Args: + block: Serviceability block returned by the hub. + system_name: Name of the analyzed system. + analyzed_at: ISO timestamp for when analysis ran. -def export_serviceability_json(block: ServiceabilityBlock) -> dict[str, Any]: - """Serialize serviceability.json with raw Hub output and no duplicate triage rows.""" - exported = prepare_serviceability_block_for_export(block) - return exported.model_dump( - mode="json", - exclude={"hub_triage_results", "hub_top_results"}, - ) + Returns: + System name, analysis time, hub and SAG versions, AFID event count, and the + top service action(s). Ties for the top rank are all included. + """ + top, _ = split_recommendation_actions(block) + return { + "system_name": system_name, + "analyzed_at": analyzed_at, + "hub_version": block.hub_version, + "afid_sag_file_version": block.afid_sag_file_version, + "afid_event_count": len(block.afid_events), + "top_service_actions": [ + action.model_dump(mode="json", exclude_none=True) for action in top + ], + } + + +def export_serviceability_json(block: ServiceabilityBlock) -> Optional[dict[str, Any]]: + """Return the AFSE analyze payload written to serviceability.json. + + Args: + block: Serviceability block that may hold the raw hub analyze response. + + Returns: + The hub analyze response, or None when AFSE did not return one. + """ + if block.hub_analyze_response is None: + return None + return dict(block.hub_analyze_response) diff --git a/nodescraper/plugins/serviceability/serviceability_plugin_base.py b/nodescraper/plugins/serviceability/serviceability_plugin_base.py index 67ff45ca..edb76ac8 100644 --- a/nodescraper/plugins/serviceability/serviceability_plugin_base.py +++ b/nodescraper/plugins/serviceability/serviceability_plugin_base.py @@ -23,10 +23,14 @@ # SOFTWARE. # ############################################################################### +from typing import Any, Optional, Union + from nodescraper.base import OOBandDataPlugin -from nodescraper.models import CollectorArgs +from nodescraper.enums import ExecutionStatus +from nodescraper.models import CollectorArgs, DataPluginResult, PluginResult, TaskResult from .analyzer_args import ServiceabilityAnalyzerArgs +from .se_runner import HubRunError, preflight_afid_sag, preflight_module_hub from .serviceability_collector import ServiceabilityCollectorBase from .serviceability_data import ServiceabilityDataModel @@ -44,3 +48,92 @@ class ServiceabilityPluginBase( COLLECTOR = ServiceabilityCollectorBase COLLECTOR_ARGS = CollectorArgs ANALYZER_ARGS = ServiceabilityAnalyzerArgs + + def _apply_afid_sag_path_to_collection_args( + self, + afid_sag_path: str, + collection_args: Any, + ) -> Any: + """Copy a resolved AFID_SAG path onto collector args when the collector stores one. + + Args: + afid_sag_path: Absolute path returned by preflight. + collection_args: Collector args for this run, or None. + + Returns: + Collector args unchanged. Subclasses that store the path override this. + """ + return collection_args + + def _plugin_error_result(self, message: str) -> PluginResult: + """Build a plugin result that skips collection and fails analysis. + + Args: + message: Error recorded on the plugin and analysis results. + + Returns: + Plugin result with collection NOT_RAN and analysis ERROR. + """ + self.logger.error("(%s) %s", self.__class__.__name__, message) + return PluginResult( + status=ExecutionStatus.ERROR, + source=self.__class__.__name__, + message=message, + result_data=DataPluginResult( + collection_result=TaskResult( + status=ExecutionStatus.NOT_RAN, + parent=self.__class__.__name__, + message="Data collection skipped", + ), + analysis_result=TaskResult( + status=ExecutionStatus.ERROR, + parent=self.__class__.__name__, + message=message, + ), + ), + ) + + def _preflight_hub_sag( + self, + collection_args: Any, + analysis_args: Optional[Union[ServiceabilityAnalyzerArgs, dict[str, Any]]], + ) -> Union[ + tuple[Any, Optional[Union[ServiceabilityAnalyzerArgs, dict[str, Any]]]], + PluginResult, + ]: + """Resolve and validate AFID_SAG before collection when an entry-point hub will run. + + Args: + collection_args: Collector args for this run, or None. + analysis_args: Analyzer args for this run, or None. + + Returns: + Updated collector and analyzer args, or a plugin error when preflight fails. + """ + try: + parsed = ( + analysis_args + if isinstance(analysis_args, self.ANALYZER_ARGS) + else self.ANALYZER_ARGS(**(analysis_args or {})) + ) + if parsed.skip_hub: + return collection_args, analysis_args + if not parsed.uses_entry_point_hub(): + preflight_module_hub(parsed, self.logger) + return collection_args, analysis_args + sag_path = preflight_afid_sag(parsed, self.logger) + except (HubRunError, ValueError) as exc: + return self._plugin_error_result(f"AFID_SAG preflight failed: {exc}") + if isinstance(analysis_args, dict): + updated_analysis: Optional[Union[ServiceabilityAnalyzerArgs, dict[str, Any]]] = { + **analysis_args, + "afid_sag_path": sag_path, + } + elif analysis_args is None: + updated_analysis = {"afid_sag_path": sag_path} + else: + updated_analysis = parsed.model_copy(update={"afid_sag_path": sag_path}) + return ( + self._apply_afid_sag_path_to_collection_args(sag_path, collection_args), + updated_analysis, + ) diff --git a/nodescraper/plugins/serviceability/serviceability_recommendations_table.py b/nodescraper/plugins/serviceability/serviceability_recommendations_table.py index 2721e764..0383cf0a 100644 --- a/nodescraper/plugins/serviceability/serviceability_recommendations_table.py +++ b/nodescraper/plugins/serviceability/serviceability_recommendations_table.py @@ -25,14 +25,16 @@ ############################################################################### from __future__ import annotations +import logging import sys from textwrap import wrap from typing import Any, Optional +from nodescraper.constants import DEFAULT_LOGGER from nodescraper.models import PluginResult from .se_models import PrioritizedServiceAction, ServiceabilityBlock -from .serviceability_api import split_recommendation_actions +from .serviceability_api import afse_top_action_missing, split_recommendation_actions RECOMMENDATION_TABLE_HEADERS: tuple[str, ...] = ( "Rank", @@ -45,10 +47,6 @@ ) PRIMARY_RECOMMENDATION_TITLE = "Top Hub service action" -ADDITIONAL_RECOMMENDATIONS_TITLE = "Additional Hub service actions" -ADDITIONAL_RECOMMENDATIONS_NOTE = ( - "Lower-priority follow-on actions if the top service action does not resolve the issue." -) RECOMMENDATION_TABLE_MAX_WIDTHS: dict[str, int] = { "Rank": 4, @@ -176,20 +174,37 @@ def _serviceability_block_from_plugin_result( if isinstance(serviceability, dict): serviceability = ServiceabilityBlock.model_validate(serviceability) - if not (serviceability.hub_triage_results or serviceability.hub_top_results): + if not ( + serviceability.hub_triage_results + or serviceability.hub_top_results + or isinstance(serviceability.hub_analyze_response, dict) + ): return None return serviceability def render_serviceability_recommendation_tables_for_plugin_results( plugin_results: list[PluginResult], + logger: Optional[logging.Logger] = None, ) -> str: - """Render Hub recommendation tables for all plugins that produced serviceability output.""" + """Render Hub recommendation tables for plugins that produced serviceability output. + + Args: + plugin_results: Plugin results from the run. + logger: Logger for the missing-top warning. Defaults to the nodescraper logger. + + Returns: + Joined top-action tables. Empty when AFSE returned no triage.top action. + """ + log = logger or logging.getLogger(DEFAULT_LOGGER) sections: list[str] = [] for plugin_result in plugin_results: block = _serviceability_block_from_plugin_result(plugin_result) if block is None: continue + if afse_top_action_missing(block): + log.warning("AFSE did not return a top action item") + continue rendered = render_serviceability_recommendation_tables(block) if rendered: sections.append(rendered) @@ -197,29 +212,33 @@ def render_serviceability_recommendation_tables_for_plugin_results( def render_serviceability_recommendation_tables(block: ServiceabilityBlock) -> str: - """Render primary and additional Hub recommendation tables.""" - primary, additional = split_recommendation_actions(block) - sections: list[str] = [] - primary_section = render_recommendations_section( + """Render the AFSE triage.top service action table. + + Args: + block: Serviceability block populated by hub analysis. + + Returns: + The top service action table. Lower-ranked triage.results rows are omitted. + """ + primary, _ = split_recommendation_actions(block) + return render_recommendations_section( _recommendation_section_title(PRIMARY_RECOMMENDATION_TITLE, primary), primary, ) - if primary_section: - sections.append(primary_section) - additional_section = render_recommendations_section( - _recommendation_section_title(ADDITIONAL_RECOMMENDATIONS_TITLE, additional), - additional, - note=ADDITIONAL_RECOMMENDATIONS_NOTE, - ) - if additional_section: - sections.append(additional_section) - if not sections: - return "" - return "\n\n".join(sections) def emit_serviceability_recommendation_tables(block: ServiceabilityBlock) -> None: - """Print Hub recommendation tables to stdout.""" + """Print the AFSE top service action table to stdout. + + Args: + block: Serviceability block populated by hub analysis. + + Returns: + None. Logs a warning and prints nothing when AFSE returned no top action. + """ + if afse_top_action_missing(block): + logging.getLogger(DEFAULT_LOGGER).warning("AFSE did not return a top action item") + return output = render_serviceability_recommendation_tables(block) if not output: return diff --git a/nodescraper/resultcollators/tablesummary.py b/nodescraper/resultcollators/tablesummary.py index cde7dc25..e8b5b2f3 100644 --- a/nodescraper/resultcollators/tablesummary.py +++ b/nodescraper/resultcollators/tablesummary.py @@ -143,7 +143,8 @@ def render_physical_row(parts: list[str]) -> str: table_sections.append(table) serviceability_tables = render_serviceability_recommendation_tables_for_plugin_results( - plugin_results + plugin_results, + logger=self.logger, ) if serviceability_tables: table_sections.append(serviceability_tables) diff --git a/pyproject.toml b/pyproject.toml index 35c755b6..5f771458 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -54,6 +54,8 @@ node-scraper = "nodescraper.cli:cli_entry" [project.entry-points."nodescraper.plugins"] AmcRedfishDiagPlugin = "nodescraper.plugins.ooband.amc_redfish_diag.amc_diag_plugin:AmcRedfishDiagPlugin" +AmcRedfishEndpointPlugin = "nodescraper.plugins.ooband.redfish_endpoint.endpoint_plugin:AmcRedfishEndpointPlugin" +AmcRedfishInventoryPlugin = "nodescraper.plugins.ooband.redfish_inventory.inventory_plugin:AmcRedfishInventoryPlugin" AmdSmiFlavorPlugin = "nodescraper.plugins.inband.amdsmi.amdsmi_flavor_plugin:AmdSmiFlavorPlugin" AmdSmiPlugin = "nodescraper.plugins.inband.amdsmi.amdsmi_plugin:AmdSmiPlugin" BiosPlugin = "nodescraper.plugins.inband.bios.bios_plugin:BiosPlugin" @@ -80,6 +82,7 @@ PciePlugin = "nodescraper.plugins.inband.pcie.pcie_plugin:PciePlugin" ProcessPlugin = "nodescraper.plugins.inband.process.process_plugin:ProcessPlugin" RdmaPlugin = "nodescraper.plugins.inband.rdma.rdma_plugin:RdmaPlugin" RedfishEndpointPlugin = "nodescraper.plugins.ooband.redfish_endpoint.endpoint_plugin:RedfishEndpointPlugin" +RedfishInventoryPlugin = "nodescraper.plugins.ooband.redfish_inventory.inventory_plugin:RedfishInventoryPlugin" RedfishOemDiagPlugin = "nodescraper.plugins.ooband.redfish_oem_diag.oem_diag_plugin:RedfishOemDiagPlugin" RegexSearchPlugin = "nodescraper.plugins.inband.regex_search.regex_search_plugin:RegexSearchPlugin" RocmPlugin = "nodescraper.plugins.inband.rocm.rocm_plugin:RocmPlugin" diff --git a/test/unit/connection/inband/test_shellcommand.py b/test/unit/connection/inband/test_shellcommand.py index e969c1bd..3b4fc390 100644 --- a/test/unit/connection/inband/test_shellcommand.py +++ b/test/unit/connection/inband/test_shellcommand.py @@ -81,6 +81,7 @@ def test_remoteshell_sudo_password_is_newline_terminated(mock_client_cls): shell.run_command("cat /etc/shadow", sudo=True) stdin.write.assert_called_once_with("hunter2\n") + stdout.channel.close.assert_called_once() @patch("nodescraper.connection.inband.inbandremote.paramiko.SSHClient") @@ -92,3 +93,50 @@ def test_remoteshell_ssh_exception_is_a_failed_command(mock_client_cls): artifact = shell.run_command("mktemp") assert artifact.exit_code == 124 assert artifact.stderr == "Timeout opening channel." + + +def _sftp_file(data: bytes) -> MagicMock: + handle = MagicMock() + handle.read.return_value = data + handle.__enter__.return_value = handle + handle.__exit__.return_value = False + return handle + + +@patch("nodescraper.connection.inband.inbandremote.paramiko.SSHClient") +def test_reconnect_ssh_opens_a_new_client(mock_client_cls): + clients = [] + + def factory(): + client = MagicMock() + client.get_transport.return_value.channel_timeout = None + clients.append(client) + return client + + mock_client_cls.side_effect = factory + shell = RemoteShell( + SSHConnectionParams(hostname="127.0.0.1", username="user", password="secret") + ) + shell.reconnect_ssh() + assert len(clients) == 2 + clients[0].close.assert_called_once() + clients[1].connect.assert_called_once() + assert shell.client is clients[1] + assert clients[1].get_transport.return_value.channel_timeout == 60 + + +@patch("nodescraper.connection.inband.inbandremote.paramiko.SSHClient") +def test_read_files_uses_one_sftp_session(mock_client_cls): + sftp = MagicMock() + sftp.open.side_effect = [ + _sftp_file(b"HTTP/1.1 200 OK\n\n"), + _sftp_file(b"{}"), + ] + mock_client_cls.return_value.open_sftp.return_value = sftp + shell = RemoteShell( + SSHConnectionParams(hostname="127.0.0.1", username="user", password="secret") + ) + contents = shell.read_files(["/tmp/headers", "/tmp/body"]) + assert contents == [b"HTTP/1.1 200 OK\n\n", b"{}"] + mock_client_cls.return_value.open_sftp.assert_called_once() + sftp.close.assert_called_once() diff --git a/test/unit/connection/redfish/test_redfish_multi_target.py b/test/unit/connection/redfish/test_redfish_multi_target.py index 60510ff8..450f3910 100644 --- a/test/unit/connection/redfish/test_redfish_multi_target.py +++ b/test/unit/connection/redfish/test_redfish_multi_target.py @@ -40,7 +40,7 @@ ) from nodescraper.enums import ExecutionStatus from nodescraper.interfaces import DataAnalyzer -from nodescraper.models import DataModel, SystemInfo +from nodescraper.models import DataModel, SystemInfo, TaskResult from nodescraper.taskresulthooks.filesystemloghook import FileSystemLogHook @@ -155,7 +155,7 @@ def test_every_target_down_fails_connect(mock_conn_cls, system_info: SystemInfo) def test_one_bmc_collection_failure_stays_warning(system_info: SystemInfo) -> None: - """A collect failure and a missed connect stay a warning when another target succeeds.""" + """A collect failure stays a warning when another target succeeds.""" good = MagicMock() good.marker = "good" bad = MagicMock() @@ -179,7 +179,37 @@ def test_one_bmc_collection_failure_stays_warning(system_info: SystemInfo) -> No assert result.status <= ExecutionStatus.WARNING assert multi.multi_target_data["good"].label == "good" assert "bad" not in multi.multi_target_data - assert "offline" in result.message + assert "offline" not in result.message + assert "[good] good" in result.message + assert "bad" in result.message + + +def test_missed_connect_stays_off_the_plugin_result(system_info: SystemInfo) -> None: + """A target that never connected is omitted from the plugin result.""" + good = MagicMock() + good.marker = "good" + multi = MultiTargetRedfishConnection( + {"good": good}, + failed_targets={"offline": "Redfish connection failed for target 'offline'"}, + ) + connection_manager = MagicMock() + connection_manager.connection = multi + connection_manager.result = TaskResult( + status=ExecutionStatus.WARNING, + message="Some Redfish targets failed to connect\n[offline] Redfish connection failed", + ) + plugin = SamplePlugin( + system_info=system_info, + logger=logging.getLogger("multi-target-test"), + connection_manager=connection_manager, + ) + + result = plugin.run() + + assert result.status == ExecutionStatus.OK + assert "offline" not in result.message + assert "failed to connect" not in result.message + assert result.message == "[good] good" def test_multi_target_logs_under_collector_directory(system_info: SystemInfo, tmp_path) -> None: diff --git a/test/unit/connection/redfish/test_redfish_session_refresh.py b/test/unit/connection/redfish/test_redfish_session_refresh.py new file mode 100644 index 00000000..a32fff77 --- /dev/null +++ b/test/unit/connection/redfish/test_redfish_session_refresh.py @@ -0,0 +1,134 @@ +############################################################################### +# +# 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, patch + +import pytest + +from nodescraper.connection.redfish import RedfishConnection, RedfishConnectionError + + +def _response(status_code: int) -> MagicMock: + response = MagicMock() + response.status_code = status_code + response.ok = status_code < 400 + return response + + +@pytest.fixture +def session_conn() -> RedfishConnection: + return RedfishConnection( + base_url="https://bmc.example", + username="root", + password="pass", + use_session_auth=True, + verify_ssl=False, + ) + + +def test_get_response_refreshes_session_after_401(session_conn: RedfishConnection) -> None: + stale = MagicMock() + fresh = MagicMock() + stale.get.return_value = _response(401) + fresh.get.return_value = _response(200) + sessions = [stale, fresh] + + def ensure_session() -> MagicMock: + session_conn._session = sessions.pop(0) + return session_conn._session + + with patch.object(session_conn, "_ensure_session", side_effect=ensure_session): + response = session_conn.get_response("/redfish/v1/Systems") + + assert response.status_code == 200 + stale.get.assert_called_once() + fresh.get.assert_called_once() + assert session_conn._session is fresh + + +def test_get_response_returns_second_401_after_refresh(session_conn: RedfishConnection) -> None: + stale = MagicMock() + fresh = MagicMock() + stale.get.return_value = _response(401) + second = _response(401) + fresh.get.return_value = second + sessions = [stale, fresh] + + def ensure_session() -> MagicMock: + session_conn._session = sessions.pop(0) + return session_conn._session + + with patch.object(session_conn, "_ensure_session", side_effect=ensure_session): + response = session_conn.get_response("/redfish/v1/Systems") + + assert response is second + assert fresh.get.call_count == 1 + + +def test_get_response_does_not_refresh_basic_auth() -> None: + conn = RedfishConnection( + base_url="https://bmc.example", + username="root", + password="pass", + use_session_auth=False, + verify_ssl=False, + ) + session = MagicMock() + session.get.return_value = _response(401) + + with patch.object(conn, "_ensure_session", return_value=session) as ensure_session: + response = conn.get_response("/redfish/v1/Systems") + + assert response.status_code == 401 + ensure_session.assert_called_once() + session.get.assert_called_once() + + +def test_get_response_does_not_refresh_other_status(session_conn: RedfishConnection) -> None: + session = MagicMock() + session.get.return_value = _response(404) + + with patch.object(session_conn, "_ensure_session", return_value=session) as ensure_session: + response = session_conn.get_response("/redfish/v1/Systems") + + assert response.status_code == 404 + ensure_session.assert_called_once() + session.get.assert_called_once() + + +def test_get_response_propagates_failed_relogin(session_conn: RedfishConnection) -> None: + stale = MagicMock() + stale.get.return_value = _response(401) + + def ensure_session() -> MagicMock: + session = session_conn._session + if not isinstance(session, MagicMock): + raise RedfishConnectionError("Session login failed: 401 Unauthorized") + return session + + session_conn._session = stale + with patch.object(session_conn, "_ensure_session", side_effect=ensure_session): + with pytest.raises(RedfishConnectionError, match="Session login failed"): + session_conn.get_response("/redfish/v1/Systems") diff --git a/test/unit/connection/redfish/test_ssh_proxy_connection.py b/test/unit/connection/redfish/test_ssh_proxy_connection.py index 9c5b9289..bbe75c6a 100644 --- a/test/unit/connection/redfish/test_ssh_proxy_connection.py +++ b/test/unit/connection/redfish/test_ssh_proxy_connection.py @@ -30,7 +30,8 @@ import pytest from pydantic import ValidationError -from nodescraper.connection.inband.inband import BaseFileArtifact, CommandArtifact +from nodescraper.connection.inband.inband import CommandArtifact +from nodescraper.connection.inband.inbandremote import SSHConnectionError from nodescraper.connection.inband.sshparams import SSHConnectionParams from nodescraper.connection.redfish.redfish_connection import RedfishConnectionError from nodescraper.connection.redfish.redfish_manager import MultiTargetRedfishConnection @@ -38,6 +39,7 @@ CurlResponse, SshProxyRedfishConnection, parse_curl_headers, + split_curl_stdout, ) from nodescraper.connection.redfish.ssh_proxy_manager import ( RedfishSshProxyConnectionManager, @@ -49,40 +51,39 @@ class FakeRemoteShell: - def __init__(self, status=200, headers="", body=b"{}"): + def __init__(self, status=200, headers="", body=b"{}", fail_times=0, reconnect_error=None): self.status = status self.headers = headers self.body = body - self.files = {} self.commands = [] - self._n = 0 self.client = MagicMock() + self.fail_times = fail_times + self.reconnect_error = reconnect_error + self.reconnects = 0 + self._curls = 0 + self._proxy_ssh_down = False def run_command(self, command, sudo=False, timeout=30, strip=True): self.commands.append(command) - if command == "mktemp": - self._n += 1 - path = f"/tmp/ns-proxy-{self._n}" - self.files[path] = b"" - return CommandArtifact(command=command, stdout=path, stderr="", exit_code=0) if command.startswith("curl"): + self._curls += 1 + if self._curls <= self.fail_times: + return CommandArtifact( + command=command, + stdout="", + stderr="Server connection dropped:", + exit_code=124, + ) parts = shlex.split(command) - hdr = parts[parts.index("-D") + 1] - body_path = parts[parts.index("-o") + 1] - self.files[hdr] = self.headers.encode("utf-8") - self.files[body_path] = self.body - return CommandArtifact(command=command, stdout=str(self.status), stderr="", exit_code=0) - if command.startswith("rm -f"): - return CommandArtifact(command=command, stdout="", stderr="", exit_code=0) + trailer = parts[parts.index("-w") + 1].replace("%{http_code}", str(self.status)) + stdout = f"{self.headers}{self.body.decode('utf-8')}{trailer}" + return CommandArtifact(command=command, stdout=stdout, stderr="", exit_code=0) return CommandArtifact(command=command, stdout="", stderr="unexpected", exit_code=1) - def read_file(self, filename, encoding="utf-8", strip=True): - return BaseFileArtifact.from_bytes( - filename=filename, - raw_contents=self.files[filename], - encoding=encoding, - strip=strip, - ) + def reconnect_ssh(self): + self.reconnects += 1 + if self.reconnect_error is not None: + raise self.reconnect_error def test_parse_curl_headers_last_hop_wins(): @@ -118,6 +119,72 @@ def test_ssh_proxy_get_uses_quoted_internal_url(): curl_cmds = [c for c in shell.commands if c.startswith("curl")] assert len(curl_cmds) == 1 assert "http://192.0.2.10:80/redfish/v1" in curl_cmds[0] + assert "-D -" in curl_cmds[0] + assert "-o -" in curl_cmds[0] + assert all(not command.startswith("mktemp") for command in shell.commands) + assert shell.reconnects == 0 + + +def test_split_curl_stdout_keeps_crlf_body(): + marker = "NODESCRAPERdeadbeef" + raw = ( + "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\n\r\n" + '{"Id": "card"}\n' + f"{marker}200\n" + ) + status, headers, body = split_curl_stdout(raw, marker) + assert status == 200 + assert "Content-Type: application/json" in headers + assert body == b'{"Id": "card"}' + + +def test_ssh_proxy_reconnects_once_after_dropped_session(): + shell = FakeRemoteShell( + headers="HTTP/1.1 200 OK\nContent-Type: application/json\n\n", + body=b'{"RedfishVersion": "1.15.0"}', + fail_times=1, + ) + conn = SshProxyRedfishConnection(shell, "http://192.0.2.10:80", api_root="redfish/v1") + data = conn.get_service_root() + assert data["RedfishVersion"] == "1.15.0" + assert shell.reconnects == 1 + assert shell._curls == 2 + + +def test_ssh_proxy_does_not_retry_after_the_second_drop(): + shell = FakeRemoteShell(fail_times=5) + conn = SshProxyRedfishConnection(shell, "http://192.0.2.10:80", api_root="redfish/v1") + with pytest.raises(RedfishConnectionError, match="Server connection dropped"): + conn.get_service_root() + with pytest.raises(RedfishConnectionError, match="SSH session not active"): + conn.get_service_root() + assert shell.reconnects == 1 + assert shell._curls == 2 + + +def test_ssh_proxy_reports_reconnect_failure(): + shell = FakeRemoteShell(fail_times=1, reconnect_error=SSHConnectionError("SSH Request timeout")) + conn = SshProxyRedfishConnection(shell, "http://192.0.2.10:80", api_root="redfish/v1") + with pytest.raises(RedfishConnectionError, match="SSH proxy reconnect failed"): + conn.get_service_root() + assert shell.reconnects == 1 + assert shell._curls == 1 + + +def test_ssh_proxy_curl_timeout_does_not_reconnect(): + shell = FakeRemoteShell() + + def timed_out(command, sudo=False, timeout=30, strip=True): + shell.commands.append(command) + return CommandArtifact( + command=command, stdout="", stderr="Command timed out", exit_code=124 + ) + + shell.run_command = timed_out + conn = SshProxyRedfishConnection(shell, "http://192.0.2.10:80", api_root="redfish/v1") + with pytest.raises(RedfishConnectionError, match="Command timed out"): + conn.get_service_root() + assert shell.reconnects == 0 def test_ssh_proxy_post_includes_json_body(): diff --git a/test/unit/framework/test_redfish_connection_selection.py b/test/unit/framework/test_redfish_connection_selection.py new file mode 100644 index 00000000..21d08614 --- /dev/null +++ b/test/unit/framework/test_redfish_connection_selection.py @@ -0,0 +1,128 @@ +############################################################################### +# +# 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 unittest +from typing import Any, Optional + +from nodescraper.connection.redfish import ( + RedfishConnectionManager, + RedfishSshProxyConnectionManager, +) +from nodescraper.enums import OSFamily, SystemLocation +from nodescraper.interfaces import ConnectionManager +from nodescraper.models import PluginConfig, SystemInfo +from nodescraper.pluginexecutor import PluginExecutor + +REDFISH_PLUGIN = "RedfishEndpointPlugin" +PROXY_PLUGIN = "AmcRedfishDiagPlugin" + +DIRECT_CONFIG: dict[str, Any] = { + "RedfishConnectionManager": {"host": "bmc.example.com", "username": "root", "password": "x"} +} +PROXY_CONFIG: dict[str, Any] = { + "RedfishSshProxyConnectionManager": { + "host": "192.0.2.10", + "ssh": {"hostname": "bmc.example.com", "username": "root", "password": "x"}, + } +} +BOTH_CONFIG: dict[str, Any] = {**DIRECT_CONFIG, **PROXY_CONFIG} + + +def _executor(connections: dict) -> PluginExecutor: + return PluginExecutor( + plugin_configs=[PluginConfig(plugins={REDFISH_PLUGIN: {}})], + connections=connections, + system_info=SystemInfo( + name="test", location=SystemLocation.REMOTE, os_family=OSFamily.LINUX + ), + ) + + +def _resolve(ex: PluginExecutor, plugin_name: str = REDFISH_PLUGIN) -> Optional[ConnectionManager]: + plugin_cls = ex.plugin_registry.plugins[plugin_name] + return ex._get_connection_manager_for_plugin(plugin_cls, plugin_name) + + +class TestRedfishConnectionSelection(unittest.TestCase): + def test_plugin_declares_direct_redfish(self): + ex = _executor({}) + plugin_cls = ex.plugin_registry.plugins[REDFISH_PLUGIN] + self.assertIs(plugin_cls.CONNECTION_TYPE, RedfishConnectionManager) + + def test_direct_only_uses_direct(self): + ex = _executor(DIRECT_CONFIG) + self.assertIs(_resolve(ex), ex.connection_library[RedfishConnectionManager]) + + def test_proxy_only_falls_back_to_proxy(self): + ex = _executor(PROXY_CONFIG) + self.assertIs(_resolve(ex), ex.connection_library[RedfishSshProxyConnectionManager]) + + def test_both_prefers_direct(self): + ex = _executor(BOTH_CONFIG) + self.assertIs(_resolve(ex), ex.connection_library[RedfishConnectionManager]) + + def test_neither_skips_plugin(self): + ex = _executor({}) + self.assertIsNone(_resolve(ex)) + self.assertNotIn(RedfishConnectionManager, ex.connection_library) + self.assertFalse( + ex._run_single_plugin( + REDFISH_PLUGIN, ex.plugin_config.plugins[REDFISH_PLUGIN], [], queue_callback=None + ) + ) + + def test_plugin_declaring_proxy_keeps_proxy_when_both_configured(self): + ex = _executor(BOTH_CONFIG) + plugin_cls = ex.plugin_registry.plugins[PROXY_PLUGIN] + self.assertIs(plugin_cls.CONNECTION_TYPE, RedfishSshProxyConnectionManager) + self.assertIs( + _resolve(ex, PROXY_PLUGIN), ex.connection_library[RedfishSshProxyConnectionManager] + ) + + def test_inventory_plugins_split_bmc_and_amc_when_both_configured(self): + ex = _executor(BOTH_CONFIG) + self.assertIs( + _resolve(ex, "RedfishInventoryPlugin"), + ex.connection_library[RedfishConnectionManager], + ) + self.assertIs( + _resolve(ex, "AmcRedfishInventoryPlugin"), + ex.connection_library[RedfishSshProxyConnectionManager], + ) + + def test_endpoint_plugins_split_bmc_and_amc_when_both_configured(self): + ex = _executor(BOTH_CONFIG) + self.assertIs( + _resolve(ex, "RedfishEndpointPlugin"), + ex.connection_library[RedfishConnectionManager], + ) + self.assertIs( + _resolve(ex, "AmcRedfishEndpointPlugin"), + ex.connection_library[RedfishSshProxyConnectionManager], + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/test/unit/plugin/test_afid_events_bmc_schema.py b/test/unit/plugin/test_afid_events_bmc_schema.py index 8529577c..cdcec83b 100644 --- a/test/unit/plugin/test_afid_events_bmc_schema.py +++ b/test/unit/plugin/test_afid_events_bmc_schema.py @@ -24,6 +24,7 @@ # ############################################################################### """AFID / serviceable unit extraction for OpenBMC-style LogEntry payloads.""" + from __future__ import annotations from serviceability_dummy_data import ( @@ -41,8 +42,12 @@ from nodescraper.plugins.serviceability.afid_events import ( _afid_event_from_rf_member, + amd_field_identifiers_with_units, build_afid_events_from_data, ) +from nodescraper.plugins.serviceability.se_runner import ( + drop_afids_without_serviceable_units, +) from nodescraper.plugins.serviceability.serviceability_data import ( ServiceabilityDataModel, ) @@ -80,3 +85,102 @@ def test_build_afid_events_from_data_includes_openbmc_entries(): by_afid_oam = {(e.afid, e.serviceable_unit) for e in events} assert (DUMMY_AFID_BELOW_RF, DUMMY_UNIT_A) in by_afid_oam assert (DUMMY_AFID_FATAL_HBM, DUMMY_UNIT_C) in by_afid_oam + + +def test_amd_field_identifiers_split_and_drop_members_without_a_location(): + located = { + "Id": "located", + "Oem": { + "AMD": { + "AMDFieldIdentifiers": [ + { + "AFID": DUMMY_AFID_A, + "ServiceableUnits": [{"@odata.id": f"/redfish/v1/Chassis/{DUMMY_UNIT_B}"}], + } + ] + } + }, + } + missing = { + "@odata.id": "/redfish/v1/Systems/dummy/LogServices/EventLog/Entries/9", + "Id": "missing", + "Oem": {"AMD": {"AMDFieldIdentifiers": [{"AFID": 11045, "Description": "update"}]}}, + } + mixed = { + "Id": "mixed", + "Oem": { + "AMD": { + "AMDFieldIdentifiers": [ + {"AFID": 11045}, + { + "AFID": DUMMY_AFID_A, + "ServiceableUnits": [{"@odata.id": f"/redfish/v1/Chassis/{DUMMY_UNIT_A}"}], + }, + ] + } + }, + } + plain = {"Id": "plain", "Message": "no oem"} + usable, unusable = amd_field_identifiers_with_units(missing["Oem"]) + assert usable == [] + assert unusable == [{"AFID": 11045, "Description": "update"}] + assert amd_field_identifiers_with_units(plain) is None + kept = drop_afids_without_serviceable_units([located, missing, mixed, plain]) + kept_ids = [member["Id"] for member in kept] + assert kept_ids == ["located", "mixed", "plain"] + mixed_kept = next(member for member in kept if member["Id"] == "mixed") + assert mixed_kept["Oem"]["AMD"]["AMDFieldIdentifiers"] == [ + { + "AFID": DUMMY_AFID_A, + "ServiceableUnits": [{"@odata.id": f"/redfish/v1/Chassis/{DUMMY_UNIT_A}"}], + } + ] + + +def test_ok_severity_without_serviceable_units_is_silent(caplog): + member = { + "Id": "ok-update", + "@odata.id": "/redfish/v1/Systems/dummy/LogServices/EventLog/Entries/1", + "Severity": "OK", + "Oem": {"AMD": {"AMDFieldIdentifiers": [{"AFID": 11045}]}}, + } + with caplog.at_level("WARNING"): + kept = drop_afids_without_serviceable_units([member]) + assert kept == [] + assert "11045" not in caplog.text + + +def test_non_ok_severity_without_serviceable_units_warns(caplog): + member = { + "Id": "fault", + "@odata.id": "/redfish/v1/Systems/dummy/LogServices/EventLog/Entries/2", + "Severity": "Critical", + "Oem": {"AMD": {"AMDFieldIdentifiers": [{"AFID": 11045}]}}, + } + with caplog.at_level("WARNING"): + kept = drop_afids_without_serviceable_units([member]) + assert kept == [] + assert "Waiving AFID 11045" in caplog.text + + +def test_serviceable_unit_from_oem_amd_field_identifiers(): + ev = _afid_event_from_rf_member( + { + "Created": DUMMY_TIMESTAMP, + "Oem": { + "AMD": { + "AMDFieldIdentifiers": [ + { + "AFID": DUMMY_AFID_A, + "ServiceableUnits": [ + {"@odata.id": f"/redfish/v1/Chassis/{DUMMY_UNIT_B}"} + ], + } + ] + } + }, + } + ) + assert ev is not None + assert ev.afid == DUMMY_AFID_A + assert ev.serviceable_unit == DUMMY_UNIT_B diff --git a/test/unit/plugin/test_mi4xx_plugin.py b/test/unit/plugin/test_mi4xx_plugin.py index 25964cd6..5d5edd81 100644 --- a/test/unit/plugin/test_mi4xx_plugin.py +++ b/test/unit/plugin/test_mi4xx_plugin.py @@ -23,6 +23,7 @@ # SOFTWARE. # ############################################################################### +import json from types import SimpleNamespace from unittest.mock import patch @@ -87,6 +88,7 @@ def test_mi4xx_analyzer_analyze_data_wrapped_once(): class _FakeHub: name = "ExampleHub" + version = "0.2.0" def analyze(self, events, afid_sag_path): assert afid_sag_path @@ -348,6 +350,38 @@ def test_mi4xx_analyzer_appends_afid_sag_metadata_artifact(system_info, tmp_path assert any(isinstance(artifact, AfidSagMetadataArtifact) for artifact in task.artifacts) +def test_mi4xx_analyzer_writes_top_action_summary_json(system_info, tmp_path): + sag = tmp_path / "afid_sag.json" + sag.write_text("{}", encoding="utf-8") + data = ServiceabilityDataModel( + rf_events=[ + { + "Afid": DUMMY_AFID_A, + "ServiceableUnit": DUMMY_UNIT_A, + "Created": DUMMY_TIMESTAMP, + } + ] + ) + fake_ep = SimpleNamespace(name="afse", load=lambda: _FakeHub) + analyzer = MI4XXAnalyzer(system_info=system_info) + with patch( + "nodescraper.plugins.serviceability.se_runner._entry_points_for_group", + return_value=[fake_ep], + ): + task = analyzer.analyze_data( + data, + Mi4xxServiceabilityAnalyzerArgs(afid_sag_path=str(sag)), + ) + log_dir = tmp_path / "mi4xx_analyzer" + task.log_result(str(log_dir)) + + summary = json.loads((log_dir / "summary.json").read_text(encoding="utf-8")) + assert summary["system_name"] == system_info.name + assert summary["afid_event_count"] == 1 + assert [action["afid"] for action in summary["top_service_actions"]] == [DUMMY_AFID_A] + assert summary["top_service_actions"][0]["rank"] == 1 + + def test_mi4xx_plugin_analyzes_offline_data_without_collection(system_info, tmp_path): sag = tmp_path / "afid_sag.json" sag.write_text("{}", encoding="utf-8") @@ -378,6 +412,56 @@ def test_mi4xx_plugin_analyzes_offline_data_without_collection(system_info, tmp_ assert result.result_data.analysis_result.status == ExecutionStatus.OK +def test_mi4xx_plugin_missing_sag_stops_before_collection(system_info, tmp_path): + fake_ep = SimpleNamespace(name="afse", load=lambda: _FakeHub) + plugin = Mi4xxServiceabilityPlugin(system_info=system_info) + with ( + patch( + "nodescraper.plugins.serviceability.se_runner._entry_points_for_group", + return_value=[fake_ep], + ), + patch.object(MI4XXCollector, "collect_data") as collect_data, + ): + result = plugin.run( + collection=True, + analysis=True, + analysis_args=Mi4xxServiceabilityAnalyzerArgs( + afid_sag_path=str(tmp_path / "missing_sag.json") + ), + ) + collect_data.assert_not_called() + assert result.status == ExecutionStatus.ERROR + assert "AFID_SAG preflight failed: AFID SAG file not found" in result.message + assert result.result_data.collection_result.status == ExecutionStatus.NOT_RAN + + +def test_mi4xx_plugin_sag_rejected_by_afse_stops_before_collection(system_info, tmp_path): + sag = tmp_path / "afid_sag.json" + sag.write_text("{}", encoding="utf-8") + + class _RejectingHub(_FakeHub): + def validate(self, afid_sag_path): + return {"status": "error", "error": {"message": "schema mismatch"}} + + fake_ep = SimpleNamespace(name="afse", load=lambda: _RejectingHub) + plugin = Mi4xxServiceabilityPlugin(system_info=system_info) + with ( + patch( + "nodescraper.plugins.serviceability.se_runner._entry_points_for_group", + return_value=[fake_ep], + ), + patch.object(MI4XXCollector, "collect_data") as collect_data, + ): + result = plugin.run( + collection=True, + analysis=True, + analysis_args=Mi4xxServiceabilityAnalyzerArgs(afid_sag_path=str(sag)), + ) + collect_data.assert_not_called() + assert result.status == ExecutionStatus.ERROR + assert "schema mismatch" in result.message + + def test_load_hub_from_entry_point_missing_raises(): with patch( "nodescraper.plugins.serviceability.se_runner._entry_points_for_group", diff --git a/test/unit/plugin/test_redfish_endpoint_analyzer.py b/test/unit/plugin/test_redfish_endpoint_analyzer.py index 55163824..1d389b05 100644 --- a/test/unit/plugin/test_redfish_endpoint_analyzer.py +++ b/test/unit/plugin/test_redfish_endpoint_analyzer.py @@ -129,6 +129,66 @@ def test_check_constraint_literal_match(): assert ok is False +def test_check_constraint_regex_pass(): + ok, _ = _check_constraint("123e4567-e89b-12d3-a456-426614174000", {"regex": r"^[0-9a-f-]{36}$"}) + assert ok is True + ok, _ = _check_constraint("SN1234", {"regex": r"\d"}) + assert ok is True + + +def test_check_constraint_regex_fail(): + ok, msg = _check_constraint("000000000000", {"regex": r"^(?!0+$).+$"}) + assert ok is False + assert "expected match" in msg + + +def test_check_constraint_regex_non_string_fails(): + for actual in (None, 42, {"a": 1}): + ok, msg = _check_constraint(actual, {"regex": ".*"}) + assert ok is False + assert "expected string" in msg + + +def test_check_constraint_regex_invalid_pattern_fails(): + ok, msg = _check_constraint("abc", {"regex": "("}) + assert ok is False + assert "invalid regex" in msg + + +def test_check_constraint_exists(): + for present in ("x", 0, False, {}, {"@odata.id": "/redfish/v1/Chassis/1/Sensors"}): + assert _check_constraint(present, {"exists": True})[0] is True + assert _check_constraint(present, {"exists": False})[0] is False + assert _check_constraint(None, {"exists": True})[0] is False + assert _check_constraint(None, {"exists": False})[0] is True + + +def test_redfish_endpoint_analyzer_regex_and_exists(redfish_endpoint_analyzer): + data = RedfishEndpointDataModel( + responses={ + "/redfish/v1/Chassis/1": { + "UUID": "123e4567-e89b-12d3-a456-426614174000", + "SerialNumber": "000000000000", + "Sensors": {"@odata.id": "/redfish/v1/Chassis/1/Sensors"}, + }, + } + ) + args = RedfishEndpointAnalyzerArgs( + checks={ + "/redfish/v1/Chassis/1": { + "UUID": {"regex": r"^[0-9a-fA-F-]{36}$"}, + "SerialNumber": {"regex": r"^(?!0+$).+$"}, + "Sensors": {"exists": True}, + "Assembly": {"exists": True}, + }, + } + ) + result = redfish_endpoint_analyzer.analyze_data(data, args=args) + assert result.status == ExecutionStatus.ERROR + failures = result.events[-1].data["failures"] + assert sorted(f["path"] for f in failures) == ["Assembly", "SerialNumber"] + + def test_redfish_endpoint_analyzer_no_checks(redfish_endpoint_analyzer): data = RedfishEndpointDataModel(responses={"/redfish/v1": {}}) result = redfish_endpoint_analyzer.analyze_data(data, args=None) diff --git a/test/unit/plugin/test_redfish_endpoint_collector.py b/test/unit/plugin/test_redfish_endpoint_collector.py index b68f57a2..7f6852ab 100644 --- a/test/unit/plugin/test_redfish_endpoint_collector.py +++ b/test/unit/plugin/test_redfish_endpoint_collector.py @@ -633,3 +633,89 @@ def test_clear_log_uris_multiple_uris_all_posted(redfish_endpoint_collector, red posted_paths = {call.args[0] for call in redfish_conn_mock.post.call_args_list} assert "/clear/a" in posted_paths assert "/clear/b" in posted_paths + + +def test_discover_tree_skips_oem_origin_and_fragments(): + conn = MagicMock() + root = { + "@odata.id": "/redfish/v1", + "Systems": {"@odata.id": "/redfish/v1/Systems"}, + "Oem": {"Secret": {"@odata.id": "/redfish/v1/Oem/Secret"}}, + "OriginOfCondition": {"@odata.id": "/redfish/v1/Chassis/other"}, + "Assembly": {"@odata.id": "/redfish/v1/Chassis/1/Assembly#/Assemblies/0"}, + } + systems = {"@odata.id": "/redfish/v1/Systems", "Members": []} + + def run_get(path): + body = root if path == "/redfish/v1" else systems + return RedfishGetResult(path=path, success=True, data=body, status_code=200) + + conn.run_get.side_effect = run_get + paths, responses, _results = ec._discover_tree( + conn, api_root="redfish/v1", max_depth=3, max_endpoints=0 + ) + assert "/redfish/v1/Systems" in paths + assert "/redfish/v1/Oem/Secret" not in responses + assert "/redfish/v1/Chassis/other" not in responses + assert "/redfish/v1/Chassis/1/Assembly" not in responses + + +def test_discover_tree_keeps_collection_pages_separate(): + conn = MagicMock() + first = "/redfish/v1/Managers/bmc/LogServices/EventLog/Entries" + second = first + "?$skip=2" + pages = { + "/redfish/v1": { + "@odata.id": "/redfish/v1", + "Managers": {"@odata.id": "/redfish/v1/Managers"}, + }, + "/redfish/v1/Managers": { + "@odata.id": "/redfish/v1/Managers", + "Members": [{"@odata.id": "/redfish/v1/Managers/bmc"}], + }, + "/redfish/v1/Managers/bmc": { + "@odata.id": "/redfish/v1/Managers/bmc", + "LogServices": {"@odata.id": "/redfish/v1/Managers/bmc/LogServices"}, + }, + "/redfish/v1/Managers/bmc/LogServices": { + "@odata.id": "/redfish/v1/Managers/bmc/LogServices", + "Members": [{"@odata.id": "/redfish/v1/Managers/bmc/LogServices/EventLog"}], + }, + "/redfish/v1/Managers/bmc/LogServices/EventLog": { + "@odata.id": "/redfish/v1/Managers/bmc/LogServices/EventLog", + "Entries": {"@odata.id": first}, + }, + first: { + "@odata.id": first, + "Members": [{"@odata.id": first + "/1"}, {"@odata.id": first + "/2"}], + "Members@odata.nextLink": second, + }, + second: { + "@odata.id": second, + "Members": [{"@odata.id": first + "/3"}], + }, + first + "/1": {"@odata.id": first + "/1", "Id": "1"}, + first + "/3": {"@odata.id": first + "/3", "Id": "3"}, + } + + def run_get(path): + body = pages.get(path) + if body is None: + return RedfishGetResult(path=path, success=False, error="missing", status_code=404) + return RedfishGetResult(path=path, success=True, data=body, status_code=200) + + conn.run_get.side_effect = run_get + _paths, responses, _results = ec._discover_tree( + conn, + api_root="redfish/v1", + max_depth=8, + max_endpoints=0, + max_collection_pages=2, + max_log_entry_members=1, + ) + assert responses[first]["Members@odata.nextLink"] == second + assert second in responses + assert responses[second]["Members"][0]["@odata.id"] == first + "/3" + assert first + "/1" in responses + assert first + "/2" not in responses + assert first + "/3" in responses diff --git a/test/unit/plugin/test_redfish_endpoint_plugin.py b/test/unit/plugin/test_redfish_endpoint_plugin.py index 5487137a..a0f9f5ab 100644 --- a/test/unit/plugin/test_redfish_endpoint_plugin.py +++ b/test/unit/plugin/test_redfish_endpoint_plugin.py @@ -23,7 +23,10 @@ # SOFTWARE. # ############################################################################### +from nodescraper.connection.redfish import RedfishSshProxyConnectionManager +from nodescraper.pluginregistry import PluginRegistry from nodescraper.plugins.ooband.redfish_endpoint import ( + AmcRedfishEndpointPlugin, RedfishEndpointAnalyzer, RedfishEndpointAnalyzerArgs, RedfishEndpointCollector, @@ -64,3 +67,13 @@ def test_redfish_endpoint_plugin_class_attributes(): assert RedfishEndpointPlugin.ANALYZER is RedfishEndpointAnalyzer assert RedfishEndpointPlugin.COLLECTOR_ARGS is RedfishEndpointCollectorArgs assert RedfishEndpointPlugin.ANALYZER_ARGS is RedfishEndpointAnalyzerArgs + + +def test_amc_endpoint_plugin_uses_the_ssh_proxy(): + assert AmcRedfishEndpointPlugin.CONNECTION_TYPE is RedfishSshProxyConnectionManager + assert AmcRedfishEndpointPlugin.COLLECTOR is RedfishEndpointCollector + assert AmcRedfishEndpointPlugin.ANALYZER is RedfishEndpointAnalyzer + assert AmcRedfishEndpointPlugin.DATA_MODEL is RedfishEndpointPlugin.DATA_MODEL + assert AmcRedfishEndpointPlugin.COLLECTOR_ARGS is RedfishEndpointCollectorArgs + assert AmcRedfishEndpointPlugin.ANALYZER_ARGS is RedfishEndpointPlugin.ANALYZER_ARGS + assert "AmcRedfishEndpointPlugin" in PluginRegistry().plugins diff --git a/test/unit/plugin/test_redfish_endpoint_ssh_proxy.py b/test/unit/plugin/test_redfish_endpoint_ssh_proxy.py new file mode 100644 index 00000000..8dbf4ec3 --- /dev/null +++ b/test/unit/plugin/test_redfish_endpoint_ssh_proxy.py @@ -0,0 +1,158 @@ +############################################################################### +# +# 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. +# +############################################################################### +"""RedfishEndpointPlugin run against a host behind a BMC via RedfishSshProxyConnectionManager. + +The plugin config names no connection: the executor falls back to the SSH proxy because it is +the only Redfish connection configured. Only the BMC SSH session and the curl call are faked. +""" +import json +from typing import Any, cast +from unittest.mock import MagicMock, patch + +import pytest + +from nodescraper.connection.redfish.ssh_proxy_connection import ( + CurlResponse, + SshProxyRedfishConnection, +) +from nodescraper.enums import ExecutionStatus, OSFamily, SystemLocation +from nodescraper.models import PluginConfig, SystemInfo +from nodescraper.pluginexecutor import PluginExecutor + +BASE = "http://192.0.2.10:80" + +CHASSIS = { + "/redfish/v1": {"@odata.id": "/redfish/v1"}, + "/redfish/v1/Chassis": {"Members@odata.count": 2}, + "/redfish/v1/Chassis/1": { + "Id": "1", + "UUID": "123e4567-e89b-12d3-a456-426614174000", + "SerialNumber": "SN0001", + "Sensors": {"@odata.id": "/redfish/v1/Chassis/1/Sensors"}, + "Status": {"Health": "OK", "State": "Enabled"}, + }, + "/redfish/v1/Chassis/2": { + "Id": "2", + "UUID": "not-a-uuid", + "SerialNumber": "000000000000", + "Status": {"Health": "OK", "State": "Enabled"}, + }, +} + +PLUGIN_ARGS: dict[str, Any] = { + "collection_args": {"uris": ["/redfish/v1/Chassis", "/redfish/v1/Chassis/1"]}, + "analysis_args": { + "checks": { + "/redfish/v1/Chassis": {"Members@odata.count": 2}, + "/redfish/v1/Chassis/1": { + "UUID": {"regex": r"^[0-9a-fA-F-]{36}$"}, + "SerialNumber": {"regex": r"^(?!0+$).+$"}, + "Sensors": {"exists": True}, + "Status/Health": "OK", + }, + } + }, +} + + +def _target(hostname: str) -> dict[str, Any]: + return { + "host": "192.0.2.10", + "ssh": {"hostname": hostname, "username": "root", "password": "x"}, + } + + +def _fake_curl(self: SshProxyRedfishConnection, url: str, extra_flags: str = "") -> CurlResponse: + cast(MagicMock, self._shell).urls.append(url) + path = url[len(BASE) :].rstrip("/") or "/" + body = CHASSIS.get(path) + if body is None: + return CurlResponse(404, b"") + return CurlResponse(200, json.dumps(body).encode()) + + +def _run(connection_args: dict[str, Any], plugin_args: dict[str, Any]): + shells: dict[str, MagicMock] = {} + + def make_shell(ssh_params): + shell = MagicMock(name=str(ssh_params.hostname)) + shell.urls = [] + shells[str(ssh_params.hostname)] = shell + return shell + + with ( + patch( + "nodescraper.connection.redfish.ssh_proxy_manager.RemoteShell", side_effect=make_shell + ), + patch.object(SshProxyRedfishConnection, "_curl", autospec=True, side_effect=_fake_curl), + ): + executor = PluginExecutor( + plugin_configs=[PluginConfig(plugins={"RedfishEndpointPlugin": plugin_args})], + connections={"RedfishSshProxyConnectionManager": connection_args}, + system_info=SystemInfo( + name="test", location=SystemLocation.REMOTE, os_family=OSFamily.LINUX + ), + ) + results = executor.run_queue() + return results, shells + + +@pytest.fixture(autouse=True) +def _no_log_dir(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + + +def test_single_target_collects_and_checks_through_proxy(): + results, shells = _run(_target("bmc-a"), PLUGIN_ARGS) + + assert len(results) == 1 + assert results[0].status == ExecutionStatus.OK, results[0].message + assert f"{BASE}/redfish/v1/Chassis/1" in shells["bmc-a"].urls + + +def test_single_target_reports_failed_checks(): + plugin_args = json.loads(json.dumps(PLUGIN_ARGS)) + plugin_args["collection_args"]["uris"] = ["/redfish/v1/Chassis/2"] + plugin_args["analysis_args"]["checks"] = { + "/redfish/v1/Chassis/2": { + "UUID": {"regex": r"^[0-9a-fA-F-]{36}$"}, + "SerialNumber": {"regex": r"^(?!0+$).+$"}, + "Sensors": {"exists": True}, + } + } + + results, _ = _run(_target("bmc-a"), plugin_args) + + assert results[0].status == ExecutionStatus.ERROR + assert "3 check(s) failed" in results[0].message + + +def test_multi_target_collects_from_each_bmc(): + results, shells = _run({"targets": [_target("bmc-a"), _target("bmc-b")]}, PLUGIN_ARGS) + + assert results[0].status == ExecutionStatus.OK, results[0].message + for host in ("bmc-a", "bmc-b"): + assert f"{BASE}/redfish/v1/Chassis/1" in shells[host].urls diff --git a/test/unit/plugin/test_redfish_inventory.py b/test/unit/plugin/test_redfish_inventory.py new file mode 100644 index 00000000..fd7f6648 --- /dev/null +++ b/test/unit/plugin/test_redfish_inventory.py @@ -0,0 +1,417 @@ +############################################################################### +# +# 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 json +from typing import Any, cast +from unittest.mock import MagicMock, patch + +import pytest +from pydantic import ValidationError + +from nodescraper.connection.redfish import ( + RedfishGetResult, + RedfishSshProxyConnectionManager, +) +from nodescraper.connection.redfish.ssh_proxy_connection import ( + CurlResponse, + SshProxyRedfishConnection, +) +from nodescraper.enums import ExecutionStatus, OSFamily, SystemLocation +from nodescraper.models import PluginConfig, SystemInfo +from nodescraper.pluginexecutor import PluginExecutor +from nodescraper.pluginregistry import PluginRegistry +from nodescraper.plugins.ooband.redfish_inventory import ( + AmcRedfishInventoryPlugin, + RedfishInventoryCollector, + RedfishInventoryCollectorArgs, + RedfishInventoryPlugin, +) + +ROOT = "/redfish/v1" +CHASSIS = "/redfish/v1/Chassis" +CHASSIS_MEMBER = "/redfish/v1/Chassis/1" +ASSEMBLY = "/redfish/v1/Chassis/1/Assembly" +POWER = "/redfish/v1/Chassis/1/PowerSubsystem" +SYSTEMS = "/redfish/v1/Systems" +SYSTEM = "/redfish/v1/Systems/1" +MEMORY = "/redfish/v1/Systems/1/Memory" +MEMORY_MEMBER = "/redfish/v1/Systems/1/Memory/1" +PROCESSORS = "/redfish/v1/Systems/1/Processors" +PROCESSOR = "/redfish/v1/Systems/1/Processors/1" + + +def _body(path: str, **extra: Any) -> dict[str, Any]: + payload: dict[str, Any] = {"@odata.id": path} + payload.update(extra) + return payload + + +def _service() -> dict[str, dict]: + return { + ROOT: _body( + ROOT, + Chassis={"@odata.id": CHASSIS}, + Systems={"@odata.id": SYSTEMS}, + Oem={"Secret": {"@odata.id": "/redfish/v1/Oem/Secret"}}, + ), + CHASSIS: _body(CHASSIS, Members=[{"@odata.id": CHASSIS_MEMBER}]), + CHASSIS_MEMBER: _body( + CHASSIS_MEMBER, + Assembly={"@odata.id": ASSEMBLY + "#/Assemblies/0"}, + PowerSubsystem={"@odata.id": POWER}, + ), + POWER: _body(POWER, Id="Power"), + SYSTEMS: _body( + SYSTEMS, + Members=[{"@odata.id": SYSTEM}], + **{"Members@odata.nextLink": SYSTEMS + "?$skip=1"}, + ), + SYSTEMS + "?$skip=1": _body(SYSTEMS + "?$skip=1", Members=[{"@odata.id": SYSTEM + "b"}]), + SYSTEM: _body( + SYSTEM, + Memory={"@odata.id": MEMORY}, + Processors={"@odata.id": PROCESSORS}, + ), + MEMORY: _body(MEMORY, Members=[{"@odata.id": MEMORY_MEMBER}]), + MEMORY_MEMBER: _body(MEMORY_MEMBER, Id="1"), + PROCESSORS: _body(PROCESSORS, Members=[{"@odata.id": PROCESSOR}]), + PROCESSOR: _body(PROCESSOR, Id="CPU1"), + } + + +def _run_get(service: dict[str, dict]): + def run_get(path: str) -> RedfishGetResult: + body = service.get(path) + if body is None: + return RedfishGetResult(path=path, success=False, error="missing", status_code=404) + return RedfishGetResult(path=path, success=True, data=body, status_code=200) + + return run_get + + +def test_bmc_profile_reads_chassis_power_and_system(system_info, redfish_conn_mock): + redfish_conn_mock.api_root = "redfish/v1" + redfish_conn_mock.run_get.side_effect = _run_get(_service()) + collector = RedfishInventoryCollector(system_info=system_info, connection=redfish_conn_mock) + result, data = collector.collect_data(args=RedfishInventoryCollectorArgs(profile="bmc")) + assert result.status == ExecutionStatus.OK + assert data is not None + assert data.responses[CHASSIS_MEMBER]["@odata.id"] == CHASSIS_MEMBER + assert data.responses[POWER]["Id"] == "Power" + assert data.responses[SYSTEM]["@odata.id"] == SYSTEM + assert ASSEMBLY not in data.responses + assert MEMORY not in data.responses + assert "/redfish/v1/Oem/Secret" not in data.responses + + +def test_smc_profile_reads_memory_and_processors(system_info, redfish_conn_mock): + redfish_conn_mock.api_root = "redfish/v1" + redfish_conn_mock.run_get.side_effect = _run_get(_service()) + collector = RedfishInventoryCollector(system_info=system_info, connection=redfish_conn_mock) + result, data = collector.collect_data( + args=RedfishInventoryCollectorArgs(profile="smc", max_pages=1) + ) + assert result.status == ExecutionStatus.OK + assert data is not None + assert data.responses[MEMORY_MEMBER]["Id"] == "1" + assert data.responses[PROCESSOR]["Id"] == "CPU1" + assert CHASSIS not in data.responses + assert SYSTEMS + "?$skip=1" not in data.responses + + +def test_max_pages_keeps_the_next_collection_page(system_info, redfish_conn_mock): + redfish_conn_mock.api_root = "redfish/v1" + redfish_conn_mock.run_get.side_effect = _run_get(_service()) + collector = RedfishInventoryCollector(system_info=system_info, connection=redfish_conn_mock) + result, data = collector.collect_data( + args=RedfishInventoryCollectorArgs(profile="smc", max_pages=2) + ) + assert result.status == ExecutionStatus.OK + assert data is not None + assert SYSTEMS + "?$skip=1" in data.responses + + +def test_missing_service_root_is_an_error(system_info, redfish_conn_mock): + redfish_conn_mock.api_root = "redfish/v1" + redfish_conn_mock.run_get.return_value = RedfishGetResult( + path=ROOT, success=False, error="refused", status_code=None + ) + collector = RedfishInventoryCollector(system_info=system_info, connection=redfish_conn_mock) + result, data = collector.collect_data(args=RedfishInventoryCollectorArgs(profile="bmc")) + assert result.status == ExecutionStatus.ERROR + assert data is None + + +def test_plugin_registers_collector(): + assert RedfishInventoryPlugin.COLLECTOR is RedfishInventoryCollector + assert RedfishInventoryPlugin.DATA_MODEL.__name__ == "RedfishInventoryDataModel" + assert RedfishInventoryPlugin.ANALYZER.__name__ == "RedfishInventoryAnalyzer" + + +def test_amc_inventory_plugin_uses_the_ssh_proxy(): + assert AmcRedfishInventoryPlugin.CONNECTION_TYPE is RedfishSshProxyConnectionManager + assert AmcRedfishInventoryPlugin.COLLECTOR is RedfishInventoryCollector + assert AmcRedfishInventoryPlugin.DATA_MODEL is RedfishInventoryPlugin.DATA_MODEL + assert AmcRedfishInventoryPlugin.COLLECTOR_ARGS is RedfishInventoryCollectorArgs + assert AmcRedfishInventoryPlugin.ANALYZER is RedfishInventoryPlugin.ANALYZER + assert "AmcRedfishInventoryPlugin" in PluginRegistry().plugins + + +BASE = "http://192.0.2.10:80" +PROXY_SERVICE = { + ROOT: _body(ROOT, Chassis={"@odata.id": CHASSIS}), + CHASSIS: _body(CHASSIS, Members=[{"@odata.id": CHASSIS_MEMBER}]), + CHASSIS_MEMBER: _body(CHASSIS_MEMBER, PowerSubsystem={"@odata.id": POWER}), + POWER: _body(POWER, Id="Power"), +} + + +def _fake_curl(self: SshProxyRedfishConnection, url: str, extra_flags: str = "") -> CurlResponse: + cast(MagicMock, self._shell).urls.append(url) + path = url[len(BASE) :].rstrip("/") or "/" + body = PROXY_SERVICE.get(path) + if body is None: + return CurlResponse(404, b"") + return CurlResponse(200, json.dumps(body).encode()) + + +def test_inventory_collects_through_ssh_proxy(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + shell = MagicMock(name="bmc-a") + shell.urls = [] + + def make_shell(ssh_params): + return shell + + with ( + patch( + "nodescraper.connection.redfish.ssh_proxy_manager.RemoteShell", side_effect=make_shell + ), + patch.object(SshProxyRedfishConnection, "_curl", autospec=True, side_effect=_fake_curl), + ): + executor = PluginExecutor( + plugin_configs=[ + PluginConfig( + plugins={"RedfishInventoryPlugin": {"collection_args": {"profile": "bmc"}}} + ) + ], + connections={ + "RedfishSshProxyConnectionManager": { + "host": "192.0.2.10", + "ssh": {"hostname": "bmc-a", "username": "root", "password": "x"}, + } + }, + system_info=SystemInfo( + name="test", location=SystemLocation.REMOTE, os_family=OSFamily.LINUX + ), + ) + results = executor.run_queue() + + assert results[0].status == ExecutionStatus.OK, results[0].message + assert f"{BASE}{POWER}" in shell.urls + + +UPDATE = "/redfish/v1/UpdateService" +FIRMWARE = "/redfish/v1/UpdateService/FirmwareInventory" +FIRMWARE_MEMBER = "/redfish/v1/UpdateService/FirmwareInventory/bmc" + + +def _service_with_firmware() -> dict[str, dict]: + service = _service() + service[ROOT]["UpdateService"] = {"@odata.id": UPDATE} + service[UPDATE] = _body(UPDATE, FirmwareInventory={"@odata.id": FIRMWARE}) + service[FIRMWARE] = _body(FIRMWARE, Members=[{"@odata.id": FIRMWARE_MEMBER}]) + service[FIRMWARE_MEMBER] = _body(FIRMWARE_MEMBER, Version="1.0") + return service + + +def test_profiles_read_firmware_inventory_members(system_info, redfish_conn_mock): + redfish_conn_mock.api_root = "redfish/v1" + redfish_conn_mock.run_get.side_effect = _run_get(_service_with_firmware()) + collector = RedfishInventoryCollector(system_info=system_info, connection=redfish_conn_mock) + for profile in ("bmc", "smc"): + result, data = collector.collect_data(args=RedfishInventoryCollectorArgs(profile=profile)) + assert result.status == ExecutionStatus.OK + assert data is not None + assert data.responses[FIRMWARE_MEMBER]["Version"] == "1.0" + + +def test_disabling_update_service_skips_firmware_inventory(system_info, redfish_conn_mock): + redfish_conn_mock.api_root = "redfish/v1" + redfish_conn_mock.run_get.side_effect = _run_get(_service_with_firmware()) + collector = RedfishInventoryCollector(system_info=system_info, connection=redfish_conn_mock) + result, data = collector.collect_data( + args=RedfishInventoryCollectorArgs(profile="bmc", enabled={"update_service": False}) + ) + assert result.status == ExecutionStatus.OK + assert data is not None + assert UPDATE not in data.responses + assert FIRMWARE_MEMBER not in data.responses + + +def test_extra_names_append_to_the_profile_set(): + args = RedfishInventoryCollectorArgs( + profile="bmc", + collections=["Sensors", "Chassis"], + member_links=["EnvironmentMetrics", "Assembly"], + ) + assert args.selected_collections()[:5] == [ + "Chassis", + "Managers", + "TelemetryService", + "UpdateService", + "Systems", + ] + assert args.selected_collections()[-1] == "Sensors" + assert args.selected_collections().count("Chassis") == 1 + assert args.selected_links()[-1] == "EnvironmentMetrics" + assert args.selected_links().count("Assembly") == 1 + assert "Memory" not in args.selected_links() + + +def test_enabled_false_still_removes_an_appended_known_area(): + args = RedfishInventoryCollectorArgs( + profile="smc", + collections=["Systems"], + enabled={"systems": False}, + ) + assert "Systems" not in args.selected_collections() + assert "UpdateService" in args.selected_collections() + + +def test_appended_collection_is_read(system_info, redfish_conn_mock): + sensors = "/redfish/v1/Sensors" + sensor = "/redfish/v1/Sensors/1" + service = _service() + service[ROOT]["Sensors"] = {"@odata.id": sensors} + service[sensors] = _body(sensors, Members=[{"@odata.id": sensor}]) + service[sensor] = _body(sensor, Id="1") + redfish_conn_mock.api_root = "redfish/v1" + redfish_conn_mock.run_get.side_effect = _run_get(service) + collector = RedfishInventoryCollector(system_info=system_info, connection=redfish_conn_mock) + result, data = collector.collect_data( + args=RedfishInventoryCollectorArgs(profile="bmc", collections=["Sensors"]) + ) + assert result.status == ExecutionStatus.OK + assert data is not None + assert data.responses[sensor]["Id"] == "1" + assert CHASSIS_MEMBER in data.responses + assert MEMORY not in data.responses + + +def test_default_args_enable_every_inventory_area(): + args = RedfishInventoryCollectorArgs() + assert args.selected_collections() == [ + "Chassis", + "Managers", + "TelemetryService", + "UpdateService", + "Systems", + ] + assert "Assembly" in args.selected_links() + assert "PowerSubsystem" in args.selected_links() + assert "FruDevices" in args.selected_links() + assert "Memory" in args.selected_links() + assert "Processors" in args.selected_links() + + +def test_enabled_can_disable_power_and_turn_systems_on(system_info, redfish_conn_mock): + redfish_conn_mock.api_root = "redfish/v1" + redfish_conn_mock.run_get.side_effect = _run_get(_service()) + collector = RedfishInventoryCollector(system_info=system_info, connection=redfish_conn_mock) + result, data = collector.collect_data( + args=RedfishInventoryCollectorArgs( + profile="bmc", + enabled={"power": False, "systems": True, "memory": True, "processors": False}, + ) + ) + assert result.status == ExecutionStatus.OK + assert data is not None + assert CHASSIS_MEMBER in data.responses + assert POWER not in data.responses + assert MEMORY_MEMBER in data.responses + assert PROCESSOR not in data.responses + + +def test_unknown_inventory_switch_is_rejected(): + with pytest.raises(ValidationError): + RedfishInventoryCollectorArgs(enabled={"sensors": False}) + + +MISSING_MEMBER = "/redfish/v1/Chassis/missing" +BROKEN_MEMBER = "/redfish/v1/Chassis/broken" + + +def _service_with_extra_members() -> dict[str, dict]: + service = _service() + service[CHASSIS] = _body( + CHASSIS, + Members=[ + {"@odata.id": CHASSIS_MEMBER}, + {"@odata.id": MISSING_MEMBER}, + {"@odata.id": BROKEN_MEMBER}, + ], + ) + return service + + +def _run_get_with_broken_member(service: dict[str, dict]): + base = _run_get(service) + + def run_get(path: str) -> RedfishGetResult: + if path == BROKEN_MEMBER: + return RedfishGetResult(path=path, success=False, error="broken", status_code=500) + return base(path) + + return run_get + + +def test_direct_connection_warns_when_a_listed_member_is_missing(system_info, redfish_conn_mock): + redfish_conn_mock.api_root = "redfish/v1" + redfish_conn_mock.run_get.side_effect = _run_get_with_broken_member( + _service_with_extra_members() + ) + collector = RedfishInventoryCollector(system_info=system_info, connection=redfish_conn_mock) + result, data = collector.collect_data(args=RedfishInventoryCollectorArgs(profile="bmc")) + descriptions = [event.description for event in result.events] + assert data is not None + assert CHASSIS_MEMBER in data.responses + assert any(MISSING_MEMBER in description for description in descriptions) + assert any(BROKEN_MEMBER in description for description in descriptions) + + +def test_ssh_proxy_treats_missing_collection_member_as_empty(system_info): + connection = SshProxyRedfishConnection(shell=MagicMock(), base_url="http://192.0.2.10:80") + connection.api_root = "redfish/v1" + connection.run_get = _run_get_with_broken_member(_service_with_extra_members()) + collector = RedfishInventoryCollector(system_info=system_info, connection=connection) + result, data = collector.collect_data(args=RedfishInventoryCollectorArgs(profile="bmc")) + descriptions = [event.description for event in result.events] + assert result.status == ExecutionStatus.OK + assert data is not None + assert CHASSIS_MEMBER in data.responses + assert MISSING_MEMBER not in data.responses + assert not any(MISSING_MEMBER in description for description in descriptions) + assert any(BROKEN_MEMBER in description for description in descriptions) diff --git a/test/unit/plugin/test_redfish_inventory_firmware.py b/test/unit/plugin/test_redfish_inventory_firmware.py new file mode 100644 index 00000000..8e9daf4d --- /dev/null +++ b/test/unit/plugin/test_redfish_inventory_firmware.py @@ -0,0 +1,198 @@ +############################################################################### +# +# 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.connection.redfish.redfish_manager import MultiTargetRedfishConnection +from nodescraper.enums import EventCategory, EventPriority, ExecutionStatus +from nodescraper.plugins.ooband.redfish_inventory.analyzer_args import ( + RedfishInventoryAnalyzerArgs, +) +from nodescraper.plugins.ooband.redfish_inventory.firmware_compare import ( + firmware_components, +) +from nodescraper.plugins.ooband.redfish_inventory.inventory_analyzer import ( + RedfishInventoryAnalyzer, +) +from nodescraper.plugins.ooband.redfish_inventory.inventory_data import ( + RedfishInventoryDataModel, +) +from nodescraper.plugins.ooband.redfish_inventory.inventory_plugin import ( + RedfishInventoryPlugin, +) + +FW_COLLECTION = "/redfish/v1/UpdateService/FirmwareInventory" +FW_ACTIVE = "/redfish/v1/UpdateService/FirmwareInventory/fw-active" +FW_SLOT_A = "/redfish/v1/UpdateService/FirmwareInventory/fw-slot-0001" +FW_SLOT_B = "/redfish/v1/UpdateService/FirmwareInventory/fw-slot-0002" +TARGET_A = "node-a" +TARGET_B = "node-b" + + +def _member(path: str, fw_id: str, version, **extra) -> dict: + body = { + "@odata.id": path, + "Id": fw_id, + "Version": version, + "Status": {"Health": "OK", "State": "Enabled"}, + "SoftwareId": "SW-1", + "Updateable": True, + "Oem": {"SVN": "4"}, + } + body.update(extra) + return body + + +def _model(*pairs: tuple[str, dict]) -> RedfishInventoryDataModel: + return RedfishInventoryDataModel(responses={path: body for path, body in pairs}) + + +def test_null_version_is_recorded_with_the_other_firmware_fields(): + model = _model( + ( + FW_ACTIVE, + _member(FW_ACTIVE, "fw-active", None, SoftwareId=None, Updateable=False, Oem={}), + ), + ( + FW_COLLECTION, + {"@odata.id": FW_COLLECTION, "Members": [{"@odata.id": FW_ACTIVE}]}, + ), + ) + components = firmware_components(model) + assert list(components) == ["fw-active"] + assert components["fw-active"] == { + "version": "N/A", + "health": "OK", + "state": "Enabled", + "software_id": "N/A", + "updateable": "False", + "svn": "", + } + + +def test_instance_suffixes_stay_separate_ids(): + model = _model( + (FW_SLOT_A, _member(FW_SLOT_A, "fw-slot-0001", "1.0")), + (FW_SLOT_B, _member(FW_SLOT_B, "fw-slot-0002", "1.0")), + ) + assert set(firmware_components(model)) == {"fw-slot-0001", "fw-slot-0002"} + + +def test_version_mismatch_warns_and_includes_firmware_fields(system_info): + analyzer = RedfishInventoryAnalyzer(system_info=system_info) + result = analyzer.compare_firmware( + { + TARGET_A: _model((FW_ACTIVE, _member(FW_ACTIVE, "fw-active", "1.0"))), + TARGET_B: _model((FW_ACTIVE, _member(FW_ACTIVE, "fw-active", "1.1", Oem={"SVN": "9"}))), + } + ) + assert result.status == ExecutionStatus.WARNING + assert result.message.startswith("Firmware differs for 1 component(s)") + assert len(result.events) == 1 + event = result.events[0] + assert event.category == EventCategory.FW.value + assert event.priority == EventPriority.WARNING + assert "node-a=version=1.0 health=OK state=Enabled software_id=SW-1 updateable=True svn=4" in ( + event.description + ) + assert "node-b=version=1.1 health=OK state=Enabled software_id=SW-1 updateable=True svn=9" in ( + event.description + ) + + +def test_missing_member_warns(system_info): + analyzer = RedfishInventoryAnalyzer(system_info=system_info) + result = analyzer.compare_firmware( + { + TARGET_A: _model((FW_SLOT_A, _member(FW_SLOT_A, "fw-slot-0001", "1.0"))), + TARGET_B: _model((FW_SLOT_B, _member(FW_SLOT_B, "fw-slot-0002", "1.0"))), + } + ) + assert result.status == ExecutionStatus.WARNING + ids = {event.data["firmware_id"] for event in result.events} + assert ids == {"fw-slot-0001", "fw-slot-0002"} + missing = next(event for event in result.events if event.data["firmware_id"] == "fw-slot-0001") + assert "node-b=missing" in missing.description + + +def test_matching_versions_are_ok(system_info): + member = _member(FW_ACTIVE, "fw-active", "1.0") + analyzer = RedfishInventoryAnalyzer(system_info=system_info) + result = analyzer.compare_firmware( + { + TARGET_A: _model((FW_ACTIVE, member)), + TARGET_B: _model((FW_ACTIVE, dict(member))), + } + ) + assert result.status == ExecutionStatus.OK + assert result.events == [] + assert result.message.startswith("Firmware versions match across 2 targets") + + +def test_compare_firmware_can_be_turned_off(system_info): + analyzer = RedfishInventoryAnalyzer(system_info=system_info) + result = analyzer.compare_firmware( + { + TARGET_A: _model((FW_ACTIVE, _member(FW_ACTIVE, "fw-active", "1.0"))), + TARGET_B: _model((FW_ACTIVE, _member(FW_ACTIVE, "fw-active", "9.0"))), + }, + RedfishInventoryAnalyzerArgs(compare_firmware=False), + ) + assert result.status == ExecutionStatus.OK + assert result.message == "Firmware comparison disabled" + assert result.events == [] + + +def test_no_firmware_members_warns_once(system_info): + analyzer = RedfishInventoryAnalyzer(system_info=system_info) + result = analyzer.compare_firmware( + { + TARGET_A: RedfishInventoryDataModel(responses={}), + TARGET_B: RedfishInventoryDataModel(responses={}), + } + ) + assert result.status == ExecutionStatus.WARNING + assert len(result.events) == 1 + assert result.events[0].description == ( + "Firmware comparison did not run: no FirmwareInventory members were collected" + ) + + +def test_plugin_compares_multi_target_inventory(system_info): + connection = MultiTargetRedfishConnection({}) + connection.multi_target_data = { + TARGET_A: _model((FW_ACTIVE, _member(FW_ACTIVE, "fw-active", None))), + TARGET_B: _model((FW_ACTIVE, _member(FW_ACTIVE, "fw-active", "2.0"))), + } + + class _Manager: + def __init__(self, open_connection): + self.connection = open_connection + + plugin = RedfishInventoryPlugin( + system_info=system_info, connection_manager=_Manager(connection) + ) + result = plugin.analyze() + assert result.status == ExecutionStatus.WARNING + assert "version=N/A" in result.events[0].description + assert "version=2.0" in result.events[0].description diff --git a/test/unit/plugin/test_se_runner.py b/test/unit/plugin/test_se_runner.py index d355d814..bb750bb5 100644 --- a/test/unit/plugin/test_se_runner.py +++ b/test/unit/plugin/test_se_runner.py @@ -24,6 +24,7 @@ # ############################################################################### import json +import logging from pathlib import Path from types import SimpleNamespace from typing import Any @@ -195,6 +196,63 @@ def test_resolve_hub_events_payload_honors_prefer_rf_events_false(): assert payload[0]["location"] == EXAMPLE_EVENTS[0].serviceable_unit +def _rf_member(odata_id, *identifiers): + return {"@odata.id": odata_id, "Oem": {"AMD": {"AMDFieldIdentifiers": list(identifiers)}}} + + +_UNIT = {"@odata.id": "/redfish/v1/Chassis/Instinct_EAM_0"} + + +def test_resolve_hub_events_payload_waives_afid_without_serviceable_units(caplog): + from nodescraper.plugins.serviceability.se_runner import ( + resolve_hub_events_payload, + ) + + good = _rf_member("/entries/1", {"AFID": 11101, "ServiceableUnits": [_UNIT]}) + bad = _rf_member("/entries/2", {"AFID": 11045, "ServiceableUnits": []}) + with caplog.at_level(logging.WARNING): + payload = resolve_hub_events_payload( + afid_events=EXAMPLE_EVENTS, rf_events=[good, bad], prefer_rf_events=True + ) + assert payload == [good] + assert "Waiving AFID 11045 from /entries/2" in caplog.text + + +def test_resolve_hub_events_payload_keeps_valid_entries_on_mixed_member(caplog): + from nodescraper.plugins.serviceability.se_runner import ( + resolve_hub_events_payload, + ) + + member = _rf_member( + "/entries/3", + {"AFID": 11101, "ServiceableUnits": [_UNIT]}, + {"AFID": 11045}, + {"AFID": 11046, "ServiceableUnits": [{"Name": "no odata id"}]}, + ) + with caplog.at_level(logging.WARNING): + payload = resolve_hub_events_payload( + afid_events=EXAMPLE_EVENTS, rf_events=[member], prefer_rf_events=True + ) + identifiers = payload[0]["Oem"]["AMD"]["AMDFieldIdentifiers"] + assert [entry["AFID"] for entry in identifiers] == [11101] + assert len(member["Oem"]["AMD"]["AMDFieldIdentifiers"]) == 3 + assert "Waiving AFID 11045" in caplog.text + assert "Waiving AFID 11046" in caplog.text + + +def test_resolve_hub_events_payload_falls_back_to_afid_events_when_all_waived(): + from nodescraper.plugins.serviceability.se_runner import ( + resolve_hub_events_payload, + ) + + payload = resolve_hub_events_payload( + afid_events=EXAMPLE_EVENTS[:1], + rf_events=[_rf_member("/entries/4", {"AFID": 11045})], + prefer_rf_events=True, + ) + assert payload[0]["afid"] == EXAMPLE_EVENTS[0].afid + + def test_resolved_hub_options_explicit_fields_override_options_bag(): args = ServiceabilityAnalyzerArgs( hub_python_module="dummy.test.module", @@ -599,13 +657,113 @@ def test_mi3xx_analyzer_runs_python_hub(system_info): def test_mi3xx_analyzer_writes_serviceability_json(tmp_path, system_info): + hub_response = { + "schema_version": "1.0", + "status": "ok", + "triage": {"top": [], "results": [], "multi_afid_summary": []}, + } data = ServiceabilityDataModel( afid_events=EXAMPLE_EVENTS[:1], serviceability=ServiceabilityBlock( afid_events=EXAMPLE_EVENTS[:1], solution=[], + solution_reasoning="node-scraper summary", + hub_analyze_response=hub_response, ), ) data.log_model(str(tmp_path)) payload = json.loads((tmp_path / "serviceability.json").read_text(encoding="utf-8")) - assert payload["afid_events"][0]["afid"] == DUMMY_AFID_A + assert payload == hub_response + assert "afid_events" not in payload + + +class _ValidatingHub: + name = "AFSE" + + def __init__(self, response=None, default_path=None): + self._response = response if response is not None else {"status": "ok"} + self._default_path = default_path + self.validated: list[str] = [] + + def validate(self, afid_sag_path): + self.validated.append(afid_sag_path) + return self._response + + def default_sag_path(self): + return self._default_path + + +def test_validate_afid_sag_with_hub_accepts_ok_status(tmp_path): + from nodescraper.plugins.serviceability.se_runner import validate_afid_sag_with_hub + + hub = _ValidatingHub() + assert validate_afid_sag_with_hub(hub, str(tmp_path / "sag.json")) is True + assert hub.validated == [str(tmp_path / "sag.json")] + + +def test_validate_afid_sag_with_hub_raises_on_error_status(): + from nodescraper.plugins.serviceability.se_runner import ( + HubRunError, + validate_afid_sag_with_hub, + ) + + hub = _ValidatingHub({"status": "error", "error": {"code": "E1", "message": "bad schema"}}) + with pytest.raises(HubRunError, match="AFSE SAG validation failed: bad schema"): + validate_afid_sag_with_hub(hub, "/tmp/sag.json") + + +def test_validate_afid_sag_with_hub_skips_hub_without_validate(): + from nodescraper.plugins.serviceability.se_runner import validate_afid_sag_with_hub + + assert validate_afid_sag_with_hub(SimpleNamespace(name="Other"), "/tmp/sag.json") is False + + +def test_resolve_runtime_afid_sag_path_prefers_configured_path(tmp_path): + from nodescraper.plugins.serviceability.se_runner import ( + resolve_runtime_afid_sag_path, + ) + + args = ServiceabilityAnalyzerArgs( + hub_entry_point="afse", afid_sag_path=str(tmp_path / "x.json") + ) + assert resolve_runtime_afid_sag_path(args) == str(tmp_path / "x.json") + + +def test_resolve_runtime_afid_sag_path_uses_hub_default(monkeypatch, tmp_path): + from nodescraper.plugins.serviceability import se_runner + + hub_default = str(tmp_path / "hub_default.json") + monkeypatch.setattr( + se_runner, + "load_hub_from_entry_point", + lambda name: _ValidatingHub(default_path=hub_default), + ) + args = ServiceabilityAnalyzerArgs(hub_entry_point="afse") + assert se_runner.resolve_runtime_afid_sag_path(args) == hub_default + + +def test_resolve_runtime_afid_sag_path_falls_back_to_builtin_default(monkeypatch): + from nodescraper.plugins.serviceability import se_runner + from nodescraper.plugins.serviceability.afid_sag_paths import DEFAULT_AFID_SAG_PATH + + monkeypatch.setattr( + se_runner, "load_hub_from_entry_point", lambda name: SimpleNamespace(name="AFSE") + ) + args = ServiceabilityAnalyzerArgs(hub_entry_point="afse") + assert se_runner.resolve_runtime_afid_sag_path(args) == DEFAULT_AFID_SAG_PATH + + +def test_run_entry_point_hub_stops_when_afse_rejects_sag(monkeypatch, tmp_path): + from nodescraper.plugins.serviceability import se_runner + + sag = tmp_path / "afid_sag.json" + sag.write_text("{}", encoding="utf-8") + hub = _ValidatingHub({"status": "error", "error": {"message": "schema mismatch"}}) + monkeypatch.setattr(se_runner, "load_hub_from_entry_point", lambda name: hub) + with pytest.raises(se_runner.HubRunError, match="schema mismatch"): + se_runner.run_entry_point_hub( + hub_entry_point="afse", + afid_events=EXAMPLE_EVENTS[:1], + afid_sag_path=str(sag), + ) + assert hub.validated == [str(sag.resolve())] diff --git a/test/unit/plugin/test_serviceability_api.py b/test/unit/plugin/test_serviceability_api.py index e11ebc5d..cf8e1d15 100644 --- a/test/unit/plugin/test_serviceability_api.py +++ b/test/unit/plugin/test_serviceability_api.py @@ -35,26 +35,27 @@ ) -def test_export_serviceability_json_keeps_raw_hub_and_drops_duplicate_rows(): +def test_export_serviceability_json_is_only_the_afse_response(): + hub_response = { + "schema_version": "1.0", + "status": "ok", + "triage": { + "top": [{"afid": DUMMY_AFID_A, "location": DUMMY_UNIT_A, "se_sort_priority": 1000}], + "results": [{"afid": DUMMY_AFID_A, "location": DUMMY_UNIT_A, "se_sort_priority": 1000}], + "multi_afid_summary": [], + }, + "pid": "SAG-00000", + "revision": "1.0.0", + } block = ServiceabilityBlock( afid_events=[ AfidEvent( afid=DUMMY_AFID_A, serviceable_unit="Instinct_EAM_0", time="2025-01-01T00:00:00Z" ) ], - hub_analyze_response={ - "schema_version": "1.0", - "status": "ok", - "triage": { - "top": [{"afid": DUMMY_AFID_A, "location": DUMMY_UNIT_A, "se_sort_priority": 1000}], - "results": [ - {"afid": DUMMY_AFID_A, "location": DUMMY_UNIT_A, "se_sort_priority": 1000} - ], - "multi_afid_summary": [], - }, - "pid": "SAG-00000", - "revision": "1.0.0", - }, + solution_reasoning="node-scraper summary", + hub_version="0.2.0", + hub_analyze_response=hub_response, hub_top_results=[ HubTriageResult( afid=DUMMY_AFID_A, @@ -75,6 +76,19 @@ def test_export_serviceability_json_keeps_raw_hub_and_drops_duplicate_rows(): payload = export_serviceability_json(block) - assert payload["hub_analyze_response"]["triage"]["top"][0]["se_sort_priority"] == 1000 - assert "hub_top_results" not in payload + assert payload == hub_response + assert "afid_events" not in payload + assert "solution" not in payload assert "hub_triage_results" not in payload + + +def test_export_serviceability_json_omits_file_without_afse_response(): + block = ServiceabilityBlock( + afid_events=[ + AfidEvent( + afid=DUMMY_AFID_A, serviceable_unit="Instinct_EAM_0", time="2025-01-01T00:00:00Z" + ) + ] + ) + + assert export_serviceability_json(block) is None diff --git a/test/unit/plugin/test_serviceability_recommendations_table.py b/test/unit/plugin/test_serviceability_recommendations_table.py index 12ad35b6..17971e82 100644 --- a/test/unit/plugin/test_serviceability_recommendations_table.py +++ b/test/unit/plugin/test_serviceability_recommendations_table.py @@ -23,6 +23,8 @@ # SOFTWARE. # ############################################################################### +import logging + from serviceability_dummy_data import ( DUMMY_AFID_A, DUMMY_AFID_B, @@ -110,7 +112,7 @@ def test_split_recommendation_rows_uses_hub_top(): assert additional[0].afid == DUMMY_AFID_B -def test_render_serviceability_recommendation_tables_splits_sections(): +def test_render_serviceability_recommendation_tables_prints_only_top_action(): block = ServiceabilityBlock( hub_top_results=[ _row( @@ -142,10 +144,123 @@ def test_render_serviceability_recommendation_tables_splits_sections(): output = render_serviceability_recommendation_tables(block) assert "Top Hub service action (rank 1)" in output - assert "Additional Hub service actions (rank 2)" in output assert f"{DUMMY_AFID_A}: Contact Support" in output - assert f"{DUMMY_AFID_B}: Update FW" in output - assert output.index("Top Hub service action") < output.index("Additional Hub service actions") + assert "Additional Hub service actions" not in output + assert f"{DUMMY_AFID_B}: Update FW" not in output + + +def test_table_prints_afse_triage_top_service_action(): + block = ServiceabilityBlock( + hub_analyze_response={ + "status": "ok", + "triage": { + "top": [ + { + "afid": DUMMY_AFID_A, + "location": DUMMY_UNIT_A, + "service_action_num": DUMMY_SERVICE_ACTION_NUM, + "priority": 20, + "sa_severity": 2, + "tier_label": "Secondary", + "service_action": { + "title": "Contact AMD", + "severity": 2, + "steps": [ + {"description": "Contact your AMD service provider."}, + ], + }, + } + ], + "results": [ + { + "afid": DUMMY_AFID_B, + "location": DUMMY_UNIT_B, + "service_action_num": 1, + } + ], + }, + }, + hub_top_results=[ + _row( + afid=DUMMY_AFID_B, + location=DUMMY_UNIT_B, + sort_priority=9000, + priority=1, + title="Update FW", + ) + ], + hub_triage_results=[ + _row( + afid=DUMMY_AFID_B, + location=DUMMY_UNIT_B, + sort_priority=9000, + priority=1, + title="Update FW", + ) + ], + ) + + output = render_serviceability_recommendation_tables(block) + + assert "Top Hub service action (rank 1)" in output + assert f"{DUMMY_AFID_A}: Contact AMD" in output + assert "Contact your AMD service provider." in output + assert f"{DUMMY_AFID_B}: Update FW" not in output + + +def test_missing_afse_top_skips_table_and_logs_warning(caplog): + block = ServiceabilityBlock( + hub_analyze_response={ + "status": "ok", + "triage": { + "top": [], + "results": [ + { + "afid": DUMMY_AFID_B, + "location": DUMMY_UNIT_B, + "service_action_num": 1, + } + ], + }, + }, + hub_top_results=[ + _row( + afid=DUMMY_AFID_B, + location=DUMMY_UNIT_B, + sort_priority=9000, + priority=1, + title="Update FW", + ) + ], + hub_triage_results=[ + _row( + afid=DUMMY_AFID_B, + location=DUMMY_UNIT_B, + sort_priority=9000, + priority=1, + title="Update FW", + ) + ], + ) + plugin_results = [ + PluginResult( + status=ExecutionStatus.OK, + source="Mi4xxServiceabilityPlugin", + message="ok", + result_data=DataPluginResult( + system_data=ServiceabilityDataModel(serviceability=block), + ), + ) + ] + + with caplog.at_level(logging.WARNING, logger="nodescraper"): + output = render_serviceability_recommendation_tables_for_plugin_results(plugin_results) + + assert output == "" + assert "Rank" not in output + assert "Update FW" not in output + assert "AFSE did not return a top action item" in caplog.text + assert render_serviceability_recommendation_tables(block) == "" def test_build_top_service_actions_includes_tied_entries():