From: Guillaume Abrioux Date: Wed, 10 Jun 2026 10:51:47 +0000 (+0200) Subject: node-proxy: override with atollon specific X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=9b3bfd171afe6714cb23dc0fe55bacad671a94dc;p=ceph.git node-proxy: override with atollon specific This adds AtollonSystem for AMI/Atollon BMCs: - memory Id mapping, - storage description fixes, - StorageControllers based drive enrichment It also wires vendor selection through cephadm (hw_monitoring_vendor) and show slot/firmware in hardware status storage output. Fixes: https://tracker.ceph.com/issues/77408 Signed-off-by: Guillaume Abrioux --- diff --git a/src/ceph-node-proxy/ceph_node_proxy/atollon.py b/src/ceph-node-proxy/ceph_node_proxy/atollon.py index f870d455d18..32aa249b4be 100644 --- a/src/ceph-node-proxy/ceph_node_proxy/atollon.py +++ b/src/ceph-node-proxy/ceph_node_proxy/atollon.py @@ -1,10 +1,126 @@ -from typing import Any +import re +from typing import Any, Dict, Optional from ceph_node_proxy.baseredfishsystem import BaseRedfishSystem from ceph_node_proxy.util import get_logger +INVALID_SERIALS = ("unknown", "not available", "n/a", "") + class AtollonSystem(BaseRedfishSystem): def __init__(self, **kw: Any) -> None: super().__init__(**kw) self.log = get_logger(__name__) + + def get_component_spec_overrides(self) -> Dict[str, Dict[str, Any]]: + return { + "memory": { + "fields": [ + "Id" if field == "Description" else field + for field in BaseRedfishSystem.MEMORY_FIELDS + ], + }, + } + + def _update_memory(self) -> None: + super()._update_memory() + for members in self._sys.get("memory", {}).values(): + for member_data in members.values(): + member_id = member_data.get("id") + if member_id is not None and member_data.get("description") is None: + member_data["description"] = member_id + + def _update_storage(self) -> None: + super()._update_storage() + self.fix_storage_descriptions() + self.enrich_storage_from_controllers() + + def fix_storage_descriptions(self) -> None: + for members in self._sys.get("storage", {}).values(): + for drive_id, drive_data in members.items(): + description = drive_data.get("description") + if description is not None and description != "unknown": + continue + endpoint = drive_data.get("redfish_endpoint", "") + if isinstance(endpoint, str) and endpoint: + drive_data["description"] = endpoint.rstrip("/").split("/")[-1] + else: + drive_data["description"] = drive_id + + def enrich_storage_from_controllers(self) -> None: + for member in self.endpoints["systems"].get_members_names(): + drives = self._sys.get("storage", {}).get(member) + if not drives: + continue + storage_units = self.endpoints["systems"][member]["Storage"].get_members_data() + for entity, storage_unit in storage_units.items(): + controllers = storage_unit.get("StorageControllers") or [] + by_serial: Dict[str, Dict[str, Any]] = {} + by_member_id: Dict[str, Dict[str, Any]] = {} + for controller in controllers: + member_id = controller.get("MemberId") + if member_id is not None: + by_member_id[str(member_id)] = controller + serial = controller.get("SerialNumber") + if isinstance(serial, str) and serial.lower() not in INVALID_SERIALS: + by_serial[serial] = controller + for drive_key, drive_data in drives.items(): + if drive_data.get("entity") != entity: + continue + controller = self.match_storage_controller( + drive_key, drive_data, by_serial, by_member_id + ) + if controller is not None: + self.apply_storage_controller(drive_data, controller) + + def match_storage_controller( + self, + drive_key: str, + drive_data: Dict[str, Any], + by_serial: Dict[str, Dict[str, Any]], + by_member_id: Dict[str, Dict[str, Any]], + ) -> Optional[Dict[str, Any]]: + serial = drive_data.get("serial_number") + if isinstance(serial, str) and serial.lower() not in INVALID_SERIALS: + controller = by_serial.get(serial) + if controller is not None: + return controller + description = drive_data.get("description") + device_index = self.parse_drive_device_index(drive_key, description) + if device_index is not None: + return by_member_id.get(device_index) + return None + + def parse_drive_device_index( + self, drive_key: str, description: Any + ) -> Optional[str]: + for text in (description, drive_key): + if not isinstance(text, str): + continue + match = re.search(r"Device(\d+)", text, re.IGNORECASE) + if match: + return match.group(1) + return None + + def apply_storage_controller( + self, drive_data: Dict[str, Any], controller: Dict[str, Any] + ) -> None: + firmware_version = controller.get("FirmwareVersion") + if firmware_version is not None: + drive_data["firmware_version"] = firmware_version + member_id = controller.get("MemberId") + if member_id is not None: + drive_data["slot"] = member_id + if drive_data.get("physical_location") in (None, "unknown"): + slot_value: Any = ( + int(member_id) if str(member_id).isdigit() else member_id + ) + drive_data["physical_location"] = { + "partlocation": { + "locationordinalvalue": slot_value, + "locationtype": "Slot", + } + } + speed_gbps = controller.get("SpeedGbps") + if speed_gbps is not None: + drive_data["speed_gbps"] = speed_gbps diff --git a/src/ceph-node-proxy/ceph_node_proxy/bootstrap.py b/src/ceph-node-proxy/ceph_node_proxy/bootstrap.py index 352d9c562b0..0e85718c5c0 100644 --- a/src/ceph-node-proxy/ceph_node_proxy/bootstrap.py +++ b/src/ceph-node-proxy/ceph_node_proxy/bootstrap.py @@ -1,7 +1,7 @@ from typing import TYPE_CHECKING from ceph_node_proxy.config import CephadmConfig, get_node_proxy_config -from ceph_node_proxy.util import DEFAULTS, write_tmp_file +from ceph_node_proxy.util import DEFAULTS, _deep_merge, write_tmp_file if TYPE_CHECKING: from ceph_node_proxy.main import NodeProxyManager @@ -18,9 +18,12 @@ def create_node_proxy_manager(cephadm_config: CephadmConfig) -> "NodeProxyManage cephadm_config.root_cert_pem, prefix_name="cephadm-endpoint-root-cert-", ) + defaults = DEFAULTS + if cephadm_config.system: + defaults = _deep_merge(DEFAULTS, {"system": cephadm_config.system}) config = get_node_proxy_config( path=cephadm_config.node_proxy_config_path, - defaults=DEFAULTS, + defaults=defaults, ) manager = NodeProxyManager( diff --git a/src/ceph-node-proxy/ceph_node_proxy/config.py b/src/ceph-node-proxy/ceph_node_proxy/config.py index 50bb3a89996..c331e70e3cc 100644 --- a/src/ceph-node-proxy/ceph_node_proxy/config.py +++ b/src/ceph-node-proxy/ceph_node_proxy/config.py @@ -30,6 +30,7 @@ class CephadmConfig: listener_key: str name: str node_proxy_config_path: str + system: Dict[str, Any] @classmethod def from_dict(cls, data: Dict[str, Any]) -> CephadmConfig: @@ -41,6 +42,7 @@ class CephadmConfig: "NODE_PROXY_CONFIG", "/etc/ceph/node-proxy.yml" ) assert node_proxy_config_path is not None + system = data.get("system") return cls( target_ip=data["target_ip"], target_port=data["target_port"], @@ -50,6 +52,7 @@ class CephadmConfig: listener_key=data["listener.key"], name=data["name"], node_proxy_config_path=node_proxy_config_path, + system=system if isinstance(system, dict) else {}, ) diff --git a/src/ceph-node-proxy/ceph_node_proxy/main.py b/src/ceph-node-proxy/ceph_node_proxy/main.py index a7869a4119c..6f0ab0b593d 100644 --- a/src/ceph-node-proxy/ceph_node_proxy/main.py +++ b/src/ceph-node-proxy/ceph_node_proxy/main.py @@ -104,6 +104,7 @@ class NodeProxyManager: raise SystemExit(1) try: vendor = self.config.get("system", {}).get("vendor", "generic") + self.log.info("Using Redfish vendor: %s", vendor) system_cls = get_system_class(vendor) self.system = system_cls( host=oob_details["host"], diff --git a/src/ceph-node-proxy/tests/test_atollon.py b/src/ceph-node-proxy/tests/test_atollon.py new file mode 100644 index 00000000000..7c4b9a179e1 --- /dev/null +++ b/src/ceph-node-proxy/tests/test_atollon.py @@ -0,0 +1,165 @@ +from unittest.mock import MagicMock, patch + +import pytest + +from ceph_node_proxy.atollon import AtollonSystem +from ceph_node_proxy.baseredfishsystem import BaseRedfishSystem + + +@pytest.fixture +def atollon_system(): + with ( + patch("ceph_node_proxy.baseredfishsystem.RedFishClient", return_value=MagicMock()), + patch("ceph_node_proxy.baseredfishsystem.EndpointMgr", return_value=MagicMock()), + ): + return AtollonSystem( + host="testhost", + port="443", + username="user", + password="secret", + config={}, + ) + + +class TestAtollonSystemMemoryOverrides: + def test_get_specs_memory_uses_id_instead_of_description(self, atollon_system): + specs = atollon_system.get_specs("memory") + assert len(specs) == 1 + assert "Id" in specs[0].fields + assert "Description" not in specs[0].fields + assert "MemoryDeviceType" in specs[0].fields + + def test_update_memory_maps_id_to_description(self, atollon_system): + atollon_system._sys["memory"] = { + "1": { + "dimm0": { + "id": "dimm0", + "memory_device_type": "DDR4", + "capacity_mib": 16384, + "status": {"health": "OK", "state": "Enabled"}, + }, + }, + } + + with patch.object(BaseRedfishSystem, "_update_memory") as mock_super: + mock_super.side_effect = lambda: None + atollon_system._update_memory() + + assert atollon_system._sys["memory"]["1"]["dimm0"]["description"] == "dimm0" + + +class TestAtollonSystemStorageOverrides: + def test_update_storage_replaces_unknown_description(self, atollon_system): + atollon_system._sys["storage"] = { + "Self": { + "nvme_device0_nsid1": { + "description": "unknown", + "model": "Micron_2550_MTFDKBK512TGE", + "capacity_bytes": 512110190592, + "protocol": "NVMe", + "serial_number": "24424BAA3C40", + "status": {"health": "OK", "state": "Enabled"}, + "redfish_endpoint": ( + "/redfish/v1/Systems/Self/Storage/StorageUnit_0/" + "Drives/NVMe_Device0_NSID1" + ), + "entity": "StorageUnit_0", + }, + }, + } + + with patch.object(BaseRedfishSystem, "_update_storage") as mock_super: + mock_super.side_effect = lambda: None + atollon_system.fix_storage_descriptions() + + drive = atollon_system._sys["storage"]["Self"]["nvme_device0_nsid1"] + assert drive["description"] == "NVMe_Device0_NSID1" + + def test_enrich_storage_from_controllers_by_serial(self, atollon_system): + atollon_system._sys["storage"] = { + "Self": { + "nvme_device0_nsid1": { + "description": "NVMe_Device0_NSID1", + "model": "Micron_2550_MTFDKBK512TGE", + "serial_number": "24424BAA3C40", + "entity": "StorageUnit_0", + "physical_location": "unknown", + }, + }, + } + mock_storage = MagicMock() + mock_storage.get_members_data.return_value = { + "StorageUnit_0": { + "StorageControllers": [ + { + "MemberId": "0", + "SerialNumber": "24424BAA3C40", + "FirmwareVersion": "V6MA001", + "SpeedGbps": 63.02, + }, + ], + }, + } + member_endpoint = MagicMock() + member_endpoint.__getitem__ = MagicMock( + side_effect=lambda key: mock_storage if key == "Storage" else MagicMock() + ) + systems_endpoint = MagicMock() + systems_endpoint.get_members_names.return_value = ["Self"] + systems_endpoint.__getitem__ = MagicMock( + side_effect=lambda key: member_endpoint if key == "Self" else MagicMock() + ) + atollon_system.endpoints = MagicMock() + atollon_system.endpoints.__getitem__ = MagicMock( + side_effect=lambda key: systems_endpoint if key == "systems" else MagicMock() + ) + + atollon_system.enrich_storage_from_controllers() + + drive = atollon_system._sys["storage"]["Self"]["nvme_device0_nsid1"] + assert drive["firmware_version"] == "V6MA001" + assert drive["slot"] == "0" + assert drive["speed_gbps"] == 63.02 + assert drive["physical_location"]["partlocation"]["locationordinalvalue"] == 0 + + def test_enrich_storage_from_controllers_by_device_index(self, atollon_system): + atollon_system._sys["storage"] = { + "Self": { + "nvme_device2_nsid1": { + "description": "NVMe_Device2_NSID1", + "serial_number": "unknown", + "entity": "StorageUnit_0", + }, + }, + } + mock_storage = MagicMock() + mock_storage.get_members_data.return_value = { + "StorageUnit_0": { + "StorageControllers": [ + { + "MemberId": "2", + "SerialNumber": "03NK797YS344D57S056", + "FirmwareVersion": "000A5305", + }, + ], + }, + } + member_endpoint = MagicMock() + member_endpoint.__getitem__ = MagicMock( + side_effect=lambda key: mock_storage if key == "Storage" else MagicMock() + ) + systems_endpoint = MagicMock() + systems_endpoint.get_members_names.return_value = ["Self"] + systems_endpoint.__getitem__ = MagicMock( + side_effect=lambda key: member_endpoint if key == "Self" else MagicMock() + ) + atollon_system.endpoints = MagicMock() + atollon_system.endpoints.__getitem__ = MagicMock( + side_effect=lambda key: systems_endpoint if key == "systems" else MagicMock() + ) + + atollon_system.enrich_storage_from_controllers() + + drive = atollon_system._sys["storage"]["Self"]["nvme_device2_nsid1"] + assert drive["firmware_version"] == "000A5305" + assert drive["slot"] == "2" diff --git a/src/ceph-node-proxy/tests/test_config.py b/src/ceph-node-proxy/tests/test_config.py new file mode 100644 index 00000000000..3fa307f08c8 --- /dev/null +++ b/src/ceph-node-proxy/tests/test_config.py @@ -0,0 +1,61 @@ +import json +import tempfile +from pathlib import Path + +from ceph_node_proxy.config import CephadmConfig, get_node_proxy_config, load_cephadm_config +from ceph_node_proxy.util import DEFAULTS, _deep_merge + + +def test_cephadm_config_parses_system_vendor() -> None: + data = { + "target_ip": "10.0.0.1", + "target_port": "8443", + "keyring": "secret", + "root_cert.pem": "ca", + "listener.crt": "crt", + "listener.key": "key", + "name": "node-proxy.host01", + "system": {"vendor": "atollon"}, + } + config = CephadmConfig.from_dict(data) + assert config.system == {"vendor": "atollon"} + + +def test_load_cephadm_config_from_file() -> None: + payload = { + "target_ip": "10.0.0.1", + "target_port": "8443", + "keyring": "secret", + "root_cert.pem": "ca", + "listener.crt": "crt", + "listener.key": "key", + "name": "node-proxy.host01", + "system": {"vendor": "atollon"}, + } + with tempfile.TemporaryDirectory() as tmpdir: + path = Path(tmpdir) / "node-proxy.json" + path.write_text(json.dumps(payload)) + config = load_cephadm_config(str(path)) + assert config.system["vendor"] == "atollon" + + +def test_cephadm_system_vendor_merged_into_node_proxy_config() -> None: + cephadm_config = CephadmConfig.from_dict( + { + "target_ip": "10.0.0.1", + "target_port": "8443", + "keyring": "secret", + "root_cert.pem": "ca", + "listener.crt": "crt", + "listener.key": "key", + "name": "node-proxy.host01", + "system": {"vendor": "atollon"}, + } + ) + defaults = _deep_merge(DEFAULTS, {"system": cephadm_config.system}) + with tempfile.TemporaryDirectory() as tmpdir: + config = get_node_proxy_config( + path=str(Path(tmpdir) / "missing.yml"), + defaults=defaults, + ) + assert config.get("system", {}).get("vendor") == "atollon" diff --git a/src/ceph-node-proxy/tests/test_redfish.py b/src/ceph-node-proxy/tests/test_redfish.py new file mode 100644 index 00000000000..bb1a2d39d86 --- /dev/null +++ b/src/ceph-node-proxy/tests/test_redfish.py @@ -0,0 +1,63 @@ +from unittest.mock import MagicMock + +from ceph_node_proxy.redfish import build_data + + +def test_build_data_thermal_fans_and_temperatures() -> None: + thermal = { + "Fans": [ + { + "MemberId": "0", + "Name": "FAN1_TACH_IN", + "PhysicalContext": "Fan", + "Reading": 23835, + "ReadingUnits": "RPM", + "Status": {"Health": "OK", "State": "Enabled"}, + } + ], + "Temperatures": [ + { + "MemberId": "0", + "Name": "C1_DCSCM_TEMP", + "PhysicalContext": "Intake", + "ReadingCelsius": 31, + "Status": {"Health": "OK", "State": "Enabled"}, + } + ], + } + log = MagicMock() + fields = ["Name", "PhysicalContext", "Reading", "ReadingUnits", "Status"] + + fans = build_data(thermal, fields, log, attribute="Fans") + assert fans["0"]["reading"] == 23835 + assert fans["0"]["reading_units"] == "RPM" + + temps = build_data(thermal, fields, log, attribute="Temperatures") + assert temps["0"]["reading"] == 31 + assert temps["0"]["reading_units"] == "Cel" + + +def test_build_data_skips_absent_members() -> None: + thermal = { + "Temperatures": [ + { + "MemberId": "0", + "Name": "C1_DCSCM_TEMP", + "PhysicalContext": "Intake", + "ReadingCelsius": 31, + "Status": {"Health": "OK", "State": "Enabled"}, + }, + { + "MemberId": "1", + "Name": "C1_DIMMB_TEMP", + "PhysicalContext": "Intake", + "Status": {"State": "Absent"}, + }, + ], + } + log = MagicMock() + fields = ["Name", "PhysicalContext", "Reading", "ReadingUnits", "Status"] + + temps = build_data(thermal, fields, log, attribute="Temperatures") + assert list(temps.keys()) == ["0"] + assert temps["0"]["name"] == "C1_DCSCM_TEMP" diff --git a/src/pybind/mgr/cephadm/module.py b/src/pybind/mgr/cephadm/module.py index 96cb2aee2d0..3bd1bbfb73d 100644 --- a/src/pybind/mgr/cephadm/module.py +++ b/src/pybind/mgr/cephadm/module.py @@ -400,6 +400,12 @@ class CephadmOrchestrator(orchestrator.Orchestrator, MgrModule): default=False, desc='Whether the hardware monitoring daemon be deployed on every host?' ), + Option( + 'hw_monitoring_vendor', + type='str', + default='generic', + desc='Redfish vendor implementation for node-proxy (generic, dell, atollon).' + ), Option( 'max_osd_draining_count', type='int', @@ -621,6 +627,7 @@ class CephadmOrchestrator(orchestrator.Orchestrator, MgrModule): self.agent_down_multiplier = 0.0 self.agent_starting_port = 0 self.hw_monitoring = False + self.hw_monitoring_vendor = 'generic' self.service_discovery_port = 0 self.secure_monitoring_stack = False self.apply_spec_fails: List[Tuple[str, str]] = [] diff --git a/src/pybind/mgr/cephadm/services/node_proxy.py b/src/pybind/mgr/cephadm/services/node_proxy.py index 9b9de4e6e13..908d8fad3c6 100644 --- a/src/pybind/mgr/cephadm/services/node_proxy.py +++ b/src/pybind/mgr/cephadm/services/node_proxy.py @@ -68,6 +68,7 @@ class NodeProxy(CephService): 'root_cert.pem': self.mgr.cert_mgr.get_root_ca(), 'listener.crt': tls_pair.cert, 'listener.key': tls_pair.key, + 'system': {'vendor': self.mgr.hw_monitoring_vendor}, } config = {'node-proxy.json': json.dumps(cfg)} diff --git a/src/pybind/mgr/orchestrator/module.py b/src/pybind/mgr/orchestrator/module.py index abb5f205435..0930d3f267b 100644 --- a/src/pybind/mgr/orchestrator/module.py +++ b/src/pybind/mgr/orchestrator/module.py @@ -530,7 +530,7 @@ class OrchestratorCli(OrchestratorClientMixin, MgrModule): 'firmwares': ['HOST', 'COMPONENT', 'NAME', 'DATE', 'VERSION', 'STATUS'], 'criticals': ['HOST', 'SYS_ID', 'COMPONENT', 'NAME', 'STATUS', 'STATE'], 'memory': ['HOST', 'SYS_ID', 'NAME', 'STATUS', 'STATE'], - 'storage': ['HOST', 'SYS_ID', 'NAME', 'MODEL', 'SIZE', 'PROTOCOL', 'SN', 'STATUS', 'STATE'], + 'storage': ['HOST', 'SYS_ID', 'NAME', 'MODEL', 'SIZE', 'PROTOCOL', 'SN', 'SLOT', 'FW', 'STATUS', 'STATE'], 'processors': ['HOST', 'SYS_ID', 'NAME', 'MODEL', 'CORES', 'THREADS', 'STATUS', 'STATE'], 'network': ['HOST', 'SYS_ID', 'NAME', 'SPEED', 'STATUS', 'STATE'], 'power': ['HOST', 'CHASSIS_ID', 'ID', 'NAME', 'MODEL', 'MANUFACTURER', 'STATUS', 'STATE'], @@ -616,7 +616,7 @@ class OrchestratorCli(OrchestratorClientMixin, MgrModule): return json.dumps(data) mapping = { 'memory': ('description', 'health', 'state'), - 'storage': ('description', 'model', 'capacity_bytes', 'protocol', 'serial_number', 'health', 'state'), + 'storage': ('description', 'model', 'capacity_bytes', 'protocol', 'serial_number', 'slot', 'firmware_version', 'health', 'state'), 'processors': ('model', 'total_cores', 'total_threads', 'health', 'state'), 'network': ('name', 'speed_mbps', 'health', 'state'), 'power': ('name', 'model', 'manufacturer', 'health', 'state'),