From 7fedfbb438efaa6f865040c52296ac273a57036c Mon Sep 17 00:00:00 2001 From: Jaspal Singh Date: Mon, 19 Oct 2026 15:20:59 +0000 Subject: [PATCH 1/7] vendor info enhancement --- .../plugins/inband/amdsmi/amdsmi_analyzer.py | 99 ++++++++++++++- .../plugins/inband/amdsmi/amdsmi_collector.py | 31 +++-- .../plugins/inband/amdsmi/analyzer_args.py | 15 +++ .../plugins/inband/bios/analyzer_args.py | 41 +++++-- .../plugins/inband/bios/bios_analyzer.py | 114 +++++++++++++----- .../plugins/inband/bios/bios_collector.py | 88 +++++++++++++- nodescraper/plugins/inband/bios/biosdata.py | 4 + .../device_enumeration/analyzer_args.py | 28 ++++- .../device_enumeration_analyzer.py | 40 +++++- .../device_enumeration_collector.py | 99 +++++++++++++-- .../device_enumeration/deviceenumdata.py | 2 + .../plugins/inband/memory/analyzer_args.py | 9 ++ .../plugins/inband/memory/memory_analyzer.py | 25 ++++ nodescraper/utils.py | 34 ++++++ test/unit/framework/test_utils.py | 18 ++- test/unit/plugin/test_amdsmi_analyzer.py | 46 ++++++- test/unit/plugin/test_amdsmi_collector.py | 11 ++ .../test_analyzer_args_build_from_model.py | 14 ++- test/unit/plugin/test_bios_analyzer.py | 46 +++++++ test/unit/plugin/test_bios_collector.py | 58 ++++++--- .../test_device_enumeration_analyzer.py | 29 +++++ .../test_device_enumeration_collector.py | 29 ++++- test/unit/plugin/test_memory_analyzer.py | 20 +++ 23 files changed, 814 insertions(+), 86 deletions(-) diff --git a/nodescraper/plugins/inband/amdsmi/amdsmi_analyzer.py b/nodescraper/plugins/inband/amdsmi/amdsmi_analyzer.py index 0a74c710..5e451c03 100644 --- a/nodescraper/plugins/inband/amdsmi/amdsmi_analyzer.py +++ b/nodescraper/plugins/inband/amdsmi/amdsmi_analyzer.py @@ -31,17 +31,20 @@ from nodescraper.enums import EventCategory, EventPriority from nodescraper.interfaces import DataAnalyzer from nodescraper.models import TaskResult +from nodescraper.utils import unit_size_to_bytes from .amdsmidata import ( AmdSmiDataModel, AmdSmiMetric, AmdSmiStatic, EccData, + EccState, Fabric, Fw, LinkStatusTable, Partition, Processes, + ValueUnit, XgmiLinks, XgmiMetrics, ) @@ -83,6 +86,24 @@ def _gpu_unavailable_description( return description, description +def _umc_ecc_state(ecc_block_state: Union[dict, str]) -> Optional[EccState]: + """Return the UMC block state from amd-smi static RAS data.""" + if not isinstance(ecc_block_state, dict): + return None + for name, state in ecc_block_state.items(): + if str(name).strip().upper() == "UMC": + return state if isinstance(state, EccState) else None + return None + + +def _vram_size_gib(size: ValueUnit) -> Optional[float]: + """Convert an amd-smi VRAM size to GiB.""" + nbytes = unit_size_to_bytes(size.value, size.unit) + if nbytes is None: + return None + return nbytes / (1024**3) + + def _is_all_zeros(value: Union[str, list[str]]) -> bool: """Return True if every comma or space separated entry is zero.""" values = value if isinstance(value, list) else [value] @@ -605,6 +626,71 @@ def check_gpu_memory( console_log=True, ) + def check_min_hbm_gib_per_gpu( + self, amdsmi_static_data: list[AmdSmiStatic], minimum_gib: float + ) -> None: + """Require each GPU's static VRAM size to be at least minimum_gib.""" + short: dict[int, float] = {} + unavailable: list[int] = [] + for gpu_data in amdsmi_static_data: + size = gpu_data.vram.size if gpu_data.vram is not None else None + gib = _vram_size_gib(size) if size is not None else None + if gib is None: + unavailable.append(gpu_data.gpu) + elif gib < minimum_gib: + short[gpu_data.gpu] = round(gib, 2) + + if unavailable: + desc, details = _gpu_unavailable_description( + "HBM size", + unavailable, + expected=f"{minimum_gib} GiB", + ) + self._log_event( + category=EventCategory.PLATFORM, + description=desc, + priority=EventPriority.ERROR, + data={"gpus": unavailable, "min_hbm_gib_per_gpu": minimum_gib, "details": details}, + console_log=True, + ) + if short: + desc, details = _gpu_mismatch_description( + "HBM size", + minimum_gib, + short, + unit=" GiB", + ) + self._log_event( + category=EventCategory.PLATFORM, + description=desc, + priority=EventPriority.ERROR, + data={ + "hbm_gib_by_gpu": short, + "min_hbm_gib_per_gpu": minimum_gib, + "details": details, + }, + console_log=True, + ) + + def check_umc_ras_enabled(self, amdsmi_static_data: list[AmdSmiStatic]) -> None: + """Require UMC RAS ENABLED. That is the in-band proxy for HBM ECC being on.""" + actual_by_gpu: dict[int, str] = {} + for gpu_data in amdsmi_static_data: + umc_state = _umc_ecc_state(gpu_data.ras.ecc_block_state) + if umc_state != EccState.ENABLED: + shown = umc_state.value if isinstance(umc_state, EccState) else "missing" + actual_by_gpu[gpu_data.gpu] = shown + if not actual_by_gpu: + return + desc, details = _gpu_mismatch_description("UMC RAS (HBM ECC)", "ENABLED", actual_by_gpu) + self._log_event( + category=EventCategory.RAS, + description=desc, + priority=EventPriority.ERROR, + data={"umc_ras_by_gpu": actual_by_gpu, "details": details}, + console_log=True, + ) + def check_amdsmi_metric_ecc(self, amdsmi_metric_data: list[AmdSmiMetric]): """Check ECC counts in all blocks for all GPUs @@ -1162,7 +1248,7 @@ def _add(gpu: int, field: str, expected: object, actual: object) -> None: gpus_affected = len({i["gpu"] for i in issues}) self._log_event( category=EventCategory.NETWORK, - description=f"Fabric data mismatch on {gpus_affected} GPU(s): {details}", + description=f"Fabric data mismatch on {gpus_affected} GPU(s)", priority=EventPriority.ERROR, data={ "expected_accel_state": expected_state, @@ -1283,6 +1369,13 @@ def analyze_data( priority=EventPriority.WARNING, data={"amdsmi_static_data": data.static}, ) + if args.min_hbm_gib_per_gpu is not None or args.ecc_required: + self._log_event( + category=EventCategory.PLATFORM, + description="AMD SMI static data is required for HBM size and ECC gates", + priority=EventPriority.ERROR, + console_log=True, + ) else: if args.expected_max_power: self.check_expected_max_power(data.static, args.expected_max_power) @@ -1307,6 +1400,10 @@ def analyze_data( (args.devid_ep, args.devid_ep), sku_name=args.sku_name, ) + if args.min_hbm_gib_per_gpu is not None: + self.check_min_hbm_gib_per_gpu(data.static, args.min_hbm_gib_per_gpu) + if args.ecc_required: + self.check_umc_ras_enabled(data.static) if args.expected_memory_partition_mode or args.expected_compute_partition_mode: self.check_expected_memory_partition_mode( diff --git a/nodescraper/plugins/inband/amdsmi/amdsmi_collector.py b/nodescraper/plugins/inband/amdsmi/amdsmi_collector.py index d04b3fd7..649b51aa 100644 --- a/nodescraper/plugins/inband/amdsmi/amdsmi_collector.py +++ b/nodescraper/plugins/inband/amdsmi/amdsmi_collector.py @@ -335,6 +335,28 @@ def _valueunit_req(self, v: object, unit: str) -> ValueUnit: assert vu is not None return vu + def _static_vram_size(self, vram: dict) -> Optional[ValueUnit]: + """Read VRAM capacity from current or older amd-smi static JSON. + + Current amd-smi reports ``size: {value, unit}``. Older output reports + ``vram_size_mb`` as a number of megabytes. + """ + raw_size = vram.get("size") + if isinstance(raw_size, dict): + value = raw_size.get("value") + if value in (None, "", "N/A"): + return None + unit = str(raw_size.get("unit") or "B") + return self._valueunit(value, unit) + + if vram.get("vram_size_mb") is not None: + try: + nbytes = int(vram["vram_size_mb"]) * 1024 * 1024 + except (TypeError, ValueError): + return None + return self._valueunit(nbytes, "B") + return None + def _normalize(self, val: object, default: str = "unknown", slot_type: bool = False) -> str: """Normalize strings @@ -1052,17 +1074,10 @@ def get_static(self) -> Optional[list[AmdSmiStatic]]: vram_type = str(vram.get("vram_type", "") or "unknown") vram_vendor = vram.get("vram_vendor") vram_bits = vram.get("vram_bit_width") - vram_size_b: Optional[int] = None - if vram.get("vram_size_mb") is not None: - try: - vram_size_b = int(vram["vram_size_mb"]) * 1024 * 1024 - except Exception: - vram_size_b = None - vram_model = StaticVram( type=vram_type, vendor=None if vram_vendor in (None, "", "N/A") else str(vram_vendor), - size=self._valueunit(vram_size_b, "B"), + size=self._static_vram_size(vram), bit_width=self._valueunit(vram_bits, "bit"), max_bandwidth=None, ) diff --git a/nodescraper/plugins/inband/amdsmi/analyzer_args.py b/nodescraper/plugins/inband/amdsmi/analyzer_args.py index d53bd2fd..0beed0e0 100644 --- a/nodescraper/plugins/inband/amdsmi/analyzer_args.py +++ b/nodescraper/plugins/inband/amdsmi/analyzer_args.py @@ -90,6 +90,21 @@ class AmdSmiAnalyzerArgs(AnalyzerArgs): default=None, description="Minimum free VRAM threshold to validate for each GPU.", ) + min_hbm_gib_per_gpu: Optional[float] = Field( + default=None, + gt=0, + description=( + "Minimum HBM capacity in GiB required on each GPU. " + "Compared to amd-smi static vram size." + ), + ) + ecc_required: bool = Field( + default=False, + description=( + "When true, require UMC RAS ENABLED on each GPU. " + "That block state is the in-band proxy for HBM ECC being on." + ), + ) check_xgmi_or_peer_links_status: bool = Field( default=False, description="Check XGMI or peer-link status for each GPU.", diff --git a/nodescraper/plugins/inband/bios/analyzer_args.py b/nodescraper/plugins/inband/bios/analyzer_args.py index 6b8f2213..3f5250e8 100644 --- a/nodescraper/plugins/inband/bios/analyzer_args.py +++ b/nodescraper/plugins/inband/bios/analyzer_args.py @@ -28,7 +28,7 @@ from pydantic import Field, field_validator from nodescraper.models import AnalyzerArgs -from nodescraper.plugins.inband.os.osdata import OsDataModel +from nodescraper.plugins.inband.bios.biosdata import BiosDataModel class BiosAnalyzerArgs(AnalyzerArgs): @@ -36,29 +36,38 @@ class BiosAnalyzerArgs(AnalyzerArgs): default_factory=list, description="Expected BIOS version(s) to match against collected value (str or list).", ) + exp_server_vendor: list[str] = Field( + default_factory=list, + description="Expected server vendor(s) from DMI system manufacturer (str or list).", + ) + exp_server_model: list[str] = Field( + default_factory=list, + description="Expected server model(s) from DMI system product name (str or list).", + ) regex_match: bool = Field( default=False, - description="If True, match exp_bios_version as regex; otherwise exact match.", + description="If True, match expected BIOS, vendor, and model values as regex; otherwise exact match.", ) - @field_validator("exp_bios_version", mode="before") + @field_validator("exp_bios_version", "exp_server_vendor", "exp_server_model", mode="before") @classmethod - def validate_exp_bios_version(cls, exp_bios_version: Union[str, list]) -> list: - """support str or list input for exp_bios_version + def validate_expected_str_list(cls, value: Union[str, list, None]) -> list: + """support str or list input for expected string fields Args: - exp_bios_version (Union[str, list]): expected BIOS version(s) to match against + value (Union[str, list, None]): expected value(s) to match against Returns: - list: a list of expected BIOS versions + list: a list of expected values """ - if isinstance(exp_bios_version, str): - exp_bios_version = [exp_bios_version] - - return exp_bios_version + if value is None: + return [] + if isinstance(value, str): + return [value] + return value @classmethod - def build_from_model(cls, datamodel: OsDataModel) -> "BiosAnalyzerArgs": + def build_from_model(cls, datamodel: BiosDataModel) -> "BiosAnalyzerArgs": """build analyzer args from data model Args: @@ -67,4 +76,10 @@ def build_from_model(cls, datamodel: OsDataModel) -> "BiosAnalyzerArgs": Returns: BiosAnalyzerArgs: instance of analyzer args class """ - return cls(exp_bios_version=datamodel.bios_version) + return cls( + exp_bios_version=datamodel.bios_version, + exp_server_vendor=( + [datamodel.server_vendor] if datamodel.server_vendor is not None else [] + ), + exp_server_model=[datamodel.server_model] if datamodel.server_model is not None else [], + ) diff --git a/nodescraper/plugins/inband/bios/bios_analyzer.py b/nodescraper/plugins/inband/bios/bios_analyzer.py index 961d9e36..c9cb9a35 100644 --- a/nodescraper/plugins/inband/bios/bios_analyzer.py +++ b/nodescraper/plugins/inband/bios/bios_analyzer.py @@ -54,42 +54,100 @@ def analyze_data( the expected versions or not. """ - if not args or not args.exp_bios_version: + if not args or not ( + args.exp_bios_version or args.exp_server_vendor or args.exp_server_model + ): self.result.message = "Expected bios not provided" self.result.status = ExecutionStatus.NOT_RAN return self.result - for bios_version in args.exp_bios_version: - self.logger.info(bios_version) - if args.regex_match: + failures: list[str] = [] + if args.exp_bios_version and not self._matches_expected( + data.bios_version, args.exp_bios_version, args.regex_match + ): + failures.append("Bios data mismatch!") + self._log_event( + category=EventCategory.BIOS, + description=( + f"Bios data mismatch! Expected {args.exp_bios_version}, " + f"actual: {data.bios_version}" + ), + data={ + "actual_bios_version": data.bios_version, + "expected_bios_version": args.exp_bios_version, + }, + priority=EventPriority.ERROR, + console_log=True, + ) + + if args.exp_server_vendor and not self._matches_expected( + data.server_vendor, args.exp_server_vendor, args.regex_match + ): + failures.append("Server vendor mismatch!") + self._log_event( + category=EventCategory.PLATFORM, + description=( + f"Server vendor mismatch! Expected {args.exp_server_vendor}, " + f"actual: {data.server_vendor}" + ), + data={ + "actual_server_vendor": data.server_vendor, + "expected_server_vendor": args.exp_server_vendor, + }, + priority=EventPriority.ERROR, + console_log=True, + ) + + if args.exp_server_model and not self._matches_expected( + data.server_model, args.exp_server_model, args.regex_match + ): + failures.append("Server model mismatch!") + self._log_event( + category=EventCategory.PLATFORM, + description=( + f"Server model mismatch! Expected {args.exp_server_model}, " + f"actual: {data.server_model}" + ), + data={ + "actual_server_model": data.server_model, + "expected_server_model": args.exp_server_model, + }, + priority=EventPriority.ERROR, + console_log=True, + ) + + if not failures: + self.result.message = "Bios data matches expected" + self.result.status = ExecutionStatus.OK + else: + self.result.message = " ".join(failures) + self.result.status = ExecutionStatus.ERROR + return self.result + + def _matches_expected( + self, actual: Optional[str], expected: list[str], regex_match: bool + ) -> bool: + """Return True when actual matches one expected value. + + Invalid regex patterns are logged and skipped, matching the previous BIOS + version behavior of recording that error and then failing the check. + """ + if actual is None: + return False + for item in expected: + self.logger.info(item) + if regex_match: try: - bios_regex = re.compile(bios_version) + pattern = re.compile(item) except re.error: self._log_event( category=EventCategory.BIOS, - description=f"Invalid regex pattern: {bios_version}", + description=f"Invalid regex pattern: {item}", priority=EventPriority.ERROR, ) continue - if bios_regex.match(data.bios_version): - self.result.message = "Bios data matches expected" - self.result.status = ExecutionStatus.OK - return self.result - elif data.bios_version == bios_version: - self.result.message = "Bios data matches expected" - self.result.status = ExecutionStatus.OK - return self.result - - self.result.message = "Bios data mismatch!" - self.result.status = ExecutionStatus.ERROR - self._log_event( - category=EventCategory.BIOS, - description=f"Bios data mismatch! Expected {args.exp_bios_version}, actual: {data.bios_version}", - data={ - "actual_bios_version": data.bios_version, - "expected_bios_version": args.exp_bios_version, - }, - priority=EventPriority.ERROR, - console_log=True, - ) - return self.result + if pattern.match(actual): + return True + elif actual == item: + return True + return False diff --git a/nodescraper/plugins/inband/bios/bios_collector.py b/nodescraper/plugins/inband/bios/bios_collector.py index e94242ef..ebd0e644 100644 --- a/nodescraper/plugins/inband/bios/bios_collector.py +++ b/nodescraper/plugins/inband/bios/bios_collector.py @@ -39,8 +39,12 @@ class BiosCollector(InBandDataCollector[BiosDataModel, None]): SUPPORTED_OS_FAMILY: set[OSFamily] = {OSFamily.WINDOWS, OSFamily.LINUX, OSFamily.ESXI} DATA_MODEL = BiosDataModel CMD_WINDOWS = "wmic bios get SMBIOSBIOSVersion /Value" + CMD_SYSTEM_WINDOWS = "wmic computersystem get Manufacturer,Model /Value" CMD = "sh -c 'cat /sys/devices/virtual/dmi/id/bios_version'" + CMD_SYS_VENDOR = "sh -c 'cat /sys/devices/virtual/dmi/id/sys_vendor'" + CMD_PRODUCT_NAME = "sh -c 'cat /sys/devices/virtual/dmi/id/product_name'" CMD_ESXI = "smbiosDump | grep -A5 'BIOS Info (Type 0)' | grep 'Version:' | head -1" + CMD_ESXI_SYSTEM = "smbiosDump | grep -A20 'System Info (Type 1)'" def collect_data( self, @@ -53,6 +57,8 @@ def collect_data( or None if the BIOS version could not be determined. """ bios = None + server_vendor = None + server_model = None if self.system_info.os_family == OSFamily.WINDOWS: res = self._run_sut_cmd(self.CMD_WINDOWS) @@ -80,7 +86,12 @@ def collect_data( ) if bios: - bios_data = BiosDataModel(bios_version=bios) + server_vendor, server_model = self._collect_server_identity() + bios_data = BiosDataModel( + bios_version=bios, + server_vendor=server_vendor, + server_model=server_model, + ) self._log_event( category="BIOS_READ", description="BIOS version read", @@ -99,3 +110,78 @@ def collect_data( self.result.status = ExecutionStatus.ERROR return self.result, bios_data + + def _collect_server_identity(self) -> tuple[Optional[str], Optional[str]]: + """Read the server vendor and model. These are the platform, not the GPU.""" + if self.system_info.os_family == OSFamily.WINDOWS: + res = self._run_sut_cmd(self.CMD_SYSTEM_WINDOWS) + if res.exit_code != 0: + self._log_identity_warning(res, "server vendor and model") + return None, None + vendor = self._labeled_value(res.stdout, "Manufacturer") + model = self._labeled_value(res.stdout, "Model") + return vendor, model + + if self.system_info.os_family == OSFamily.ESXI: + res = self._run_sut_cmd(self.CMD_ESXI_SYSTEM) + if res.exit_code != 0: + self._log_identity_warning(res, "server vendor and model") + return None, None + return self._parse_esxi_system(res.stdout) + + vendor = self._read_sysfs_value(self.CMD_SYS_VENDOR, "server vendor") + model = self._read_sysfs_value(self.CMD_PRODUCT_NAME, "server model") + return vendor, model + + def _read_sysfs_value(self, command: str, label: str) -> Optional[str]: + res = self._run_sut_cmd(command) + if res.exit_code != 0: + self._log_identity_warning(res, label) + return None + text = (res.stdout or "").strip() + return text or None + + def _log_identity_warning(self, res, label: str) -> None: + self._log_event( + category=EventCategory.OS, + description=f"Error checking {label}", + data={"command": res.command, "exit_code": res.exit_code}, + priority=EventPriority.WARNING, + console_log=True, + ) + + @staticmethod + def _labeled_value(stdout: str, key: str) -> Optional[str]: + prefix = f"{key}=" + for line in stdout.splitlines(): + if line.startswith(prefix): + value = line.split("=", 1)[1].strip() + return value or None + return None + + @staticmethod + def _parse_esxi_system(stdout: str) -> tuple[Optional[str], Optional[str]]: + vendor = None + model = None + in_system = False + for line in stdout.splitlines(): + if "System Info" in line and "Type 1" in line: + in_system = True + continue + if not in_system: + continue + stripped = line.strip() + if stripped.startswith("System Info") or re.match(r".*Info \(Type \d+\)", stripped): + break + if stripped.lower().startswith("manufacturer:"): + vendor = BiosCollector._unquote_smbios(stripped.split(":", 1)[1]) + elif stripped.lower().startswith("product:"): + model = BiosCollector._unquote_smbios(stripped.split(":", 1)[1]) + return vendor, model + + @staticmethod + def _unquote_smbios(value: str) -> Optional[str]: + text = value.strip() + if len(text) >= 2 and text[0] == '"' and text[-1] == '"': + text = text[1:-1].strip() + return text or None diff --git a/nodescraper/plugins/inband/bios/biosdata.py b/nodescraper/plugins/inband/bios/biosdata.py index 63b36275..344060e6 100644 --- a/nodescraper/plugins/inband/bios/biosdata.py +++ b/nodescraper/plugins/inband/bios/biosdata.py @@ -23,8 +23,12 @@ # SOFTWARE. # ############################################################################### +from typing import Optional + from nodescraper.models import DataModel class BiosDataModel(DataModel): bios_version: str + server_vendor: Optional[str] = None + server_model: Optional[str] = None diff --git a/nodescraper/plugins/inband/device_enumeration/analyzer_args.py b/nodescraper/plugins/inband/device_enumeration/analyzer_args.py index 5dc8b38c..7311e0c5 100644 --- a/nodescraper/plugins/inband/device_enumeration/analyzer_args.py +++ b/nodescraper/plugins/inband/device_enumeration/analyzer_args.py @@ -35,7 +35,19 @@ class DeviceEnumerationAnalyzerArgs(AnalyzerArgs): cpu_count: Optional[list[int]] = Field( default=None, - description="Expected CPU count(s); pass as int or list of ints. Analysis passes if actual is in list.", + description="Expected CPU socket count(s) from lscpu Socket(s); pass as int or list of ints.", + ) + cpu_architecture: Optional[list[str]] = Field( + default=None, + description="Expected CPU architecture(s) from lscpu Architecture (e.g. x86_64).", + ) + min_cpu_cores: Optional[int] = Field( + default=None, + ge=1, + description=( + "Minimum physical CPU cores, Core(s) per socket times Socket(s). " + "This is not the logical CPU count from nproc." + ), ) gpu_count: Optional[list[int]] = Field( default=None, @@ -46,6 +58,16 @@ class DeviceEnumerationAnalyzerArgs(AnalyzerArgs): description="Expected virtual function count(s); pass as int or list of ints. Analysis passes if actual is in list.", ) + @field_validator("cpu_architecture", mode="before") + @classmethod + def normalize_architecture(cls, v: Any) -> Optional[list[str]]: + """Convert a single architecture string to a list.""" + if v is None: + return None + if isinstance(v, str): + return [v] + return v + @field_validator("cpu_count", "gpu_count", "vf_count", mode="before") @classmethod def normalize_to_list(cls, v: Any) -> Optional[list[int]]: @@ -77,6 +99,10 @@ def build_from_model( """ return cls( cpu_count=[datamodel.cpu_count] if datamodel.cpu_count is not None else None, + cpu_architecture=( + [datamodel.cpu_architecture] if datamodel.cpu_architecture is not None else None + ), + min_cpu_cores=datamodel.cpu_cores, gpu_count=[datamodel.gpu_count] if datamodel.gpu_count is not None else None, vf_count=[datamodel.vf_count] if datamodel.vf_count is not None else None, ) diff --git a/nodescraper/plugins/inband/device_enumeration/device_enumeration_analyzer.py b/nodescraper/plugins/inband/device_enumeration/device_enumeration_analyzer.py index cdc58c0f..ea6ea818 100644 --- a/nodescraper/plugins/inband/device_enumeration/device_enumeration_analyzer.py +++ b/nodescraper/plugins/inband/device_enumeration/device_enumeration_analyzer.py @@ -73,8 +73,46 @@ def analyze_data( priority=EventPriority.CRITICAL, console_log=True, ) + + if args.cpu_architecture: + accepted = [arch.lower() for arch in args.cpu_architecture] + actual_arch = (data.cpu_architecture or "").strip().lower() + if actual_arch not in accepted: + self.result.message = "Device Enumeration mismatch" + self.result.status = ExecutionStatus.ERROR + self._log_event( + category=EventCategory.PLATFORM, + description=( + f"Expected cpu_architecture in {args.cpu_architecture}, " + f"but got {data.cpu_architecture}" + ), + data={"cpu_architecture": data.cpu_architecture}, + priority=EventPriority.CRITICAL, + console_log=True, + ) + + if args.min_cpu_cores is not None and ( + data.cpu_cores is None or data.cpu_cores < args.min_cpu_cores + ): + self.result.message = "Device Enumeration mismatch" + self.result.status = ExecutionStatus.ERROR + self._log_event( + category=EventCategory.PLATFORM, + description=( + f"Expected cpu_cores >= {args.min_cpu_cores}, but got {data.cpu_cores}" + ), + data={"cpu_cores": data.cpu_cores, "min_cpu_cores": args.min_cpu_cores}, + priority=EventPriority.CRITICAL, + console_log=True, + ) + if self.result.message == "": + validated = list(checks) + if args.cpu_architecture: + validated.append("cpu_architecture") + if args.min_cpu_cores is not None: + validated.append("cpu_cores") self.result.status = ExecutionStatus.OK - self.result.message = f"Device Enumeration validated on {checks.keys()}." + self.result.message = f"Device Enumeration validated on {validated}." return self.result diff --git a/nodescraper/plugins/inband/device_enumeration/device_enumeration_collector.py b/nodescraper/plugins/inband/device_enumeration/device_enumeration_collector.py index 78c54ad0..36865fe9 100644 --- a/nodescraper/plugins/inband/device_enumeration/device_enumeration_collector.py +++ b/nodescraper/plugins/inband/device_enumeration/device_enumeration_collector.py @@ -54,6 +54,17 @@ class DeviceEnumerationCollector( CMD_CPU_COUNT_WINDOWS = ( 'powershell -Command "(Get-WmiObject -Class Win32_Processor | Measure-Object).Count"' ) + # Architecture codes are Win32_Processor.Architecture. Names match lscpu + # (9 -> x86_64) so the same analyzer value works on Linux and Windows. + CMD_CPU_INFO_WINDOWS = ( + "powershell -NoProfile -Command " + '"$p = @(Get-CimInstance -ClassName Win32_Processor); ' + "$cores = ($p | Measure-Object -Property NumberOfCores -Sum).Sum; " + "$map = @{0='i686'; 5='arm'; 6='ia64'; 9='x86_64'; 12='aarch64'}; " + "$arch = $map[[int]$p[0].Architecture]; " + "Write-Output ('Architecture=' + $arch); " + "Write-Output ('Cores=' + $cores)\"" + ) CMD_GPU_COUNT_WINDOWS = 'powershell -Command "(wmic path win32_VideoController get name | findstr AMD | Measure-Object).Count"' CMD_VF_COUNT_WINDOWS = ( 'powershell -Command "(Get-VMHostPartitionableGpu | Measure-Object).Count"' @@ -108,6 +119,78 @@ def _parse_count( return None return int(text) + def _apply_lscpu(self, device_enum: DeviceEnumerationDataModel, lscpu_res: CommandArtifact): + """Fill socket count, architecture, and physical cores from lscpu. + + Physical cores are Core(s) per socket times Socket(s). nproc's logical + CPU count is not used for the core gate. + """ + fields: dict[str, str] = {} + for line in lscpu_res.stdout.splitlines(): + if ":" not in line: + continue + key, value = line.split(":", 1) + fields[key.strip().lower()] = value.strip() + + socket_text = fields.get("socket(s)") + if socket_text: + try: + device_enum.cpu_count = int(socket_text) + except ValueError: + self._warning( + description="Cannot parse CPU count from lscpu output", + command=lscpu_res, + ) + + architecture = fields.get("architecture") + if architecture: + device_enum.cpu_architecture = architecture + + cores_text = fields.get("core(s) per socket") + if cores_text and device_enum.cpu_count: + try: + device_enum.cpu_cores = int(cores_text) * device_enum.cpu_count + except ValueError: + self._warning( + description="Cannot parse physical CPU cores from lscpu output", + command=lscpu_res, + ) + + def _apply_windows_cpu_info( + self, device_enum: DeviceEnumerationDataModel, cpu_info_res: CommandArtifact + ): + """Fill architecture and total physical cores from Win32_Processor.""" + if cpu_info_res.exit_code != 0 or not cpu_info_res.stdout: + self._warning( + description="Cannot collect Windows CPU architecture and cores", + command=cpu_info_res, + ) + return + fields: dict[str, str] = {} + for line in cpu_info_res.stdout.splitlines(): + if "=" not in line: + continue + key, value = line.split("=", 1) + fields[key.strip().lower()] = value.strip() + + architecture = fields.get("architecture") + if architecture: + device_enum.cpu_architecture = architecture + else: + self._warning( + description="Cannot parse CPU architecture from Windows processor info", + command=cpu_info_res, + ) + + cores_text = fields.get("cores") + if cores_text and cores_text.isdigit(): + device_enum.cpu_cores = int(cores_text) + else: + self._warning( + description="Cannot parse physical CPU cores from Windows processor info", + command=cpu_info_res, + ) + def _esxi_device_count(self, device_id: Optional[int]) -> CommandArtifact: """Count PCI devices on ESXi whose Device ID matches ``device_id`` (as hex). @@ -128,6 +211,7 @@ def collect_data( """ if args is None: args = DeviceEnumerationCollectorArgs() + cpu_info_res = None if self.system_info.os_family == OSFamily.LINUX: lscpu_res = self._run_sut_cmd(self.CMD_LSCPU_LINUX, log_artifact=False) @@ -157,22 +241,13 @@ def collect_data( cpu_count_res = self._run_sut_cmd(self.CMD_CPU_COUNT_WINDOWS) gpu_count_res = self._run_sut_cmd(self.CMD_GPU_COUNT_WINDOWS) vf_count_res = self._run_sut_cmd(self.CMD_VF_COUNT_WINDOWS) + cpu_info_res = self._run_sut_cmd(self.CMD_CPU_INFO_WINDOWS) device_enum = DeviceEnumerationDataModel() if self.system_info.os_family == OSFamily.LINUX: if lscpu_res.exit_code == 0 and lscpu_res.stdout: - # Extract socket count from lscpu output - for line in lscpu_res.stdout.splitlines(): - if line.startswith("Socket(s):"): - try: - device_enum.cpu_count = int(line.split(":")[1].strip()) - break - except (ValueError, IndexError): - self._warning( - description="Cannot parse CPU count from lscpu output", - command=lscpu_res, - ) + self._apply_lscpu(device_enum, lscpu_res) device_enum.lscpu_output = lscpu_res.stdout self._log_event( category=EventCategory.PLATFORM, @@ -185,6 +260,8 @@ def collect_data( cpu_count = self._parse_count(cpu_count_res, "Cannot determine CPU count") if cpu_count is not None: device_enum.cpu_count = cpu_count + if cpu_info_res is not None: + self._apply_windows_cpu_info(device_enum, cpu_info_res) gpu_count = self._parse_count(gpu_count_res, "Cannot determine GPU count") if gpu_count is not None: diff --git a/nodescraper/plugins/inband/device_enumeration/deviceenumdata.py b/nodescraper/plugins/inband/device_enumeration/deviceenumdata.py index bef13492..bed35015 100644 --- a/nodescraper/plugins/inband/device_enumeration/deviceenumdata.py +++ b/nodescraper/plugins/inband/device_enumeration/deviceenumdata.py @@ -30,6 +30,8 @@ class DeviceEnumerationDataModel(DataModel): cpu_count: Optional[int] = None + cpu_architecture: Optional[str] = None + cpu_cores: Optional[int] = None gpu_count: Optional[int] = None vf_count: Optional[int] = None lscpu_output: Optional[str] = None diff --git a/nodescraper/plugins/inband/memory/analyzer_args.py b/nodescraper/plugins/inband/memory/analyzer_args.py index 9a4000a2..90cd3f98 100644 --- a/nodescraper/plugins/inband/memory/analyzer_args.py +++ b/nodescraper/plugins/inband/memory/analyzer_args.py @@ -33,6 +33,15 @@ class MemoryAnalyzerArgs(AnalyzerArgs): + minimum_system_memory_gib: Optional[float] = Field( + default=None, + ge=0, + description=( + "Minimum installed system memory in GiB. Compared to MemTotal. " + "Passes when reported memory is at least 98% of this value, because " + "the OS reports slightly less than nominal DIMM capacity." + ), + ) minimum_free_memory_percent: Optional[float] = Field( default=None, ge=0, diff --git a/nodescraper/plugins/inband/memory/memory_analyzer.py b/nodescraper/plugins/inband/memory/memory_analyzer.py index 98b124f6..34642337 100644 --- a/nodescraper/plugins/inband/memory/memory_analyzer.py +++ b/nodescraper/plugins/inband/memory/memory_analyzer.py @@ -38,6 +38,8 @@ class MemoryAnalyzer(DataAnalyzer[MemoryDataModel, MemoryAnalyzerArgs]): """Check memory usage is within the maximum allowed used memory""" DATA_MODEL = MemoryDataModel + # free(1) MemTotal is a bit under the nominal DIMM capacity. + _INSTALLED_MEMORY_TOLERANCE = 0.98 def analyze_data( self, data: MemoryDataModel, args: Optional[MemoryAnalyzerArgs] = None @@ -61,6 +63,29 @@ def _bytes_to_gb(n: float) -> float: total_memory = convert_to_bytes(data.mem_total) used_memory = total_memory - available_memory + if args.minimum_system_memory_gib is not None: + minimum_bytes = args.minimum_system_memory_gib * (1024**3) + required_bytes = minimum_bytes * self._INSTALLED_MEMORY_TOLERANCE + if total_memory < required_bytes: + total_gib = _bytes_to_gb(total_memory) + self.result.status = ExecutionStatus.ERROR + self.result.message = "Installed system memory is below the minimum" + self._log_event( + category=EventCategory.OS, + description=( + f"Installed memory is {total_gib:.2f} GiB " + f"(minimum {args.minimum_system_memory_gib:.2f} GiB)" + ), + priority=EventPriority.CRITICAL, + data={ + "total_memory": total_memory, + "total_memory_gib": total_gib, + "minimum_system_memory_gib": args.minimum_system_memory_gib, + }, + console_log=True, + ) + return self.result + available_percent = None if args.minimum_free_memory_percent is not None: if total_memory <= 0: diff --git a/nodescraper/utils.py b/nodescraper/utils.py index f862e83c..4f63ccc9 100644 --- a/nodescraper/utils.py +++ b/nodescraper/utils.py @@ -24,6 +24,7 @@ # ############################################################################### import inspect +import math import os import re import traceback @@ -225,6 +226,39 @@ def convert_to_bytes(value: str, si=False) -> int: return int(value) +_UNIT_BYTES = { + "B": 1, + "BYTE": 1, + "BYTES": 1, + "KB": 1024, + "KIB": 1024, + "MB": 1024**2, + "MIB": 1024**2, + "GB": 1024**3, + "GIB": 1024**3, + "TB": 1024**4, + "TIB": 1024**4, +} + + +def unit_size_to_bytes(value: Union[int, float, str], unit: str) -> Optional[float]: + """Convert a numeric size and a unit label to bytes. + + KB, MB, GB, and TB use powers of 1024, the same scale as convert_to_bytes. + Unknown units and non-numeric values return None. + """ + try: + number = float(value) + except (TypeError, ValueError): + return None + if not math.isfinite(number): + return None + factor = _UNIT_BYTES.get((unit or "").strip().upper()) + if factor is None: + return None + return number * factor + + def get_unique_filename(directory, filename) -> str: """Checks if the file exists in the directory and returns a new filename if it does. Parameters diff --git a/test/unit/framework/test_utils.py b/test/unit/framework/test_utils.py index 399ae438..b2d153f1 100644 --- a/test/unit/framework/test_utils.py +++ b/test/unit/framework/test_utils.py @@ -4,7 +4,11 @@ from typing import Literal, Optional, Union -from nodescraper.utils import bytes_to_human_readable, find_annotation_in_container +from nodescraper.utils import ( + bytes_to_human_readable, + find_annotation_in_container, + unit_size_to_bytes, +) class Target: @@ -39,3 +43,15 @@ def test_bytes_to_human_readable_terabytes(): def test_bytes_to_human_readable_petabytes(): """PB is a documented unit, so petabyte scale values must not be reported in TB.""" assert bytes_to_human_readable(2 * 10**15) == "2.0PB" + + +def test_unit_size_to_bytes_treats_gb_as_binary(): + """GB and GiB labels both use 1024**3, matching convert_to_bytes.""" + assert unit_size_to_bytes(256, "GB") == 256 * 1024**3 + assert unit_size_to_bytes("256", "GiB") == 256 * 1024**3 + + +def test_unit_size_to_bytes_rejects_unknown_unit(): + """An unrecognized unit is not treated as raw bytes.""" + assert unit_size_to_bytes(256, "FOO") is None + assert unit_size_to_bytes("n/a", "GB") is None diff --git a/test/unit/plugin/test_amdsmi_analyzer.py b/test/unit/plugin/test_amdsmi_analyzer.py index 1b2329e8..39672b96 100644 --- a/test/unit/plugin/test_amdsmi_analyzer.py +++ b/test/unit/plugin/test_amdsmi_analyzer.py @@ -1516,7 +1516,8 @@ def test_check_fabric_inactive_output(mock_analyzer): "vpod_size", } assert {m["gpu"] for m in event.data["mismatches"]} == {0, 1, 2, 3} - assert "Fabric data mismatch on 4 GPU(s)" in event.description + assert event.description == "Fabric data mismatch on 4 GPU(s)" + assert "GPU 0 accel_state" in event.data["details"] def test_check_fabric_unexpected_fabric_type(mock_analyzer): @@ -1565,3 +1566,46 @@ def test_check_fabric_zero_ppod_id(mock_analyzer): mismatches = analyzer.result.events[0].data["mismatches"] assert [m["field"] for m in mismatches] == ["ppod_id"] assert mismatches[0]["expected"] == "non-zero" + + +def test_check_min_hbm_gib_per_gpu_passes(mock_analyzer): + analyzer = mock_analyzer + gpu = create_static_gpu(0) + gpu.vram.size = ValueUnit(value=256, unit="GB") + + analyzer.check_min_hbm_gib_per_gpu([gpu], 256) + + assert len(analyzer.result.events) == 0 + + +def test_check_min_hbm_gib_per_gpu_fails(mock_analyzer): + analyzer = mock_analyzer + gpu = create_static_gpu(0) + gpu.vram.size = ValueUnit(value=192, unit="GB") + + analyzer.check_min_hbm_gib_per_gpu([gpu], 256) + + assert len(analyzer.result.events) == 1 + assert analyzer.result.events[0].priority == EventPriority.ERROR + assert "HBM size" in analyzer.result.events[0].description + + +def test_check_umc_ras_enabled_passes(mock_analyzer): + analyzer = mock_analyzer + gpu = create_static_gpu(0) + gpu.ras.ecc_block_state = {"UMC": EccState.ENABLED, "GFX": EccState.DISABLED} + + analyzer.check_umc_ras_enabled([gpu]) + + assert len(analyzer.result.events) == 0 + + +def test_check_umc_ras_enabled_fails_when_umc_missing(mock_analyzer): + analyzer = mock_analyzer + gpu = create_static_gpu(0) + + analyzer.check_umc_ras_enabled([gpu]) + + assert len(analyzer.result.events) == 1 + assert analyzer.result.events[0].priority == EventPriority.ERROR + assert "UMC RAS" in analyzer.result.events[0].description diff --git a/test/unit/plugin/test_amdsmi_collector.py b/test/unit/plugin/test_amdsmi_collector.py index 85988d3e..626bc6c8 100644 --- a/test/unit/plugin/test_amdsmi_collector.py +++ b/test/unit/plugin/test_amdsmi_collector.py @@ -422,6 +422,9 @@ def test_get_static(collector): # VRAM assert s.vram.type == "HBM3" assert s.vram.vendor == "Micron" + assert s.vram.size is not None + assert s.vram.size.value == 65536 * 1024 * 1024 + assert s.vram.size.unit == "B" # Cache assert s.cache_info is not None and len(s.cache_info) == 1 @@ -435,6 +438,14 @@ def test_get_static(collector): assert s.clock["clk"].frequency_levels is not None +def test_static_vram_size_reads_current_amd_smi_size(collector): + """amd-smi 27 reports VRAM as size.value plus a unit, not vram_size_mb.""" + size = collector._static_vram_size({"size": {"value": 294896, "unit": "MB"}}) + assert size is not None + assert size.value == 294896 + assert size.unit == "MB" + + def test_cache_properties_parsing(collector): """Test cache properties string parsing""" stat = collector.get_static() diff --git a/test/unit/plugin/test_analyzer_args_build_from_model.py b/test/unit/plugin/test_analyzer_args_build_from_model.py index 5702a481..06874a8a 100644 --- a/test/unit/plugin/test_analyzer_args_build_from_model.py +++ b/test/unit/plugin/test_analyzer_args_build_from_model.py @@ -73,11 +73,15 @@ def test_package_analyzer_args_build_from_model(): def test_device_enumeration_analyzer_args_build_from_model(): """Test DeviceEnumerationAnalyzerArgs.build_from_model includes all fields""" - datamodel = DeviceEnumerationDataModel(cpu_count=2, gpu_count=8, vf_count=0) + datamodel = DeviceEnumerationDataModel( + cpu_count=2, cpu_architecture="x86_64", cpu_cores=128, gpu_count=8, vf_count=0 + ) args = DeviceEnumerationAnalyzerArgs.build_from_model(datamodel) assert isinstance(args, DeviceEnumerationAnalyzerArgs) assert args.cpu_count == [2] + assert args.cpu_architecture == ["x86_64"] + assert args.min_cpu_cores == 128 assert args.gpu_count == [8] assert args.vf_count == [0] @@ -126,11 +130,17 @@ def test_os_analyzer_args_build_from_model(): def test_bios_analyzer_args_build_from_model(): """Test BiosAnalyzerArgs.build_from_model includes all fields""" - datamodel = BiosDataModel(bios_version="1.2.3") + datamodel = BiosDataModel( + bios_version="1.2.3", + server_vendor="SuperXYZ", + server_model="AS -1234GS-ABCD", + ) args = BiosAnalyzerArgs.build_from_model(datamodel) assert isinstance(args, BiosAnalyzerArgs) assert args.exp_bios_version == ["1.2.3"] + assert args.exp_server_vendor == ["SuperXYZ"] + assert args.exp_server_model == ["AS -1234GS-ABCD"] def test_cmdline_analyzer_args_build_from_model(): diff --git a/test/unit/plugin/test_bios_analyzer.py b/test/unit/plugin/test_bios_analyzer.py index e7abdd86..1a81f08b 100644 --- a/test/unit/plugin/test_bios_analyzer.py +++ b/test/unit/plugin/test_bios_analyzer.py @@ -78,6 +78,7 @@ def test_unexpected_bios(system_info): args = BiosAnalyzerArgs(exp_bios_version=["some_other_bios"]) res = analyzer.analyze_data(model, args) assert res.status == ExecutionStatus.ERROR + assert res.message.startswith("Bios data mismatch!") assert len(res.events) == 1 assert res.events[0].category == EventCategory.BIOS.value assert res.events[0].priority == EventPriority.ERROR @@ -103,6 +104,51 @@ def test_bios_regex_no_match(system_info): assert res.events[0].priority == EventPriority.ERROR +def test_server_vendor_and_model_match(system_info): + model = BiosDataModel( + bios_version="TESTBIOS", + server_vendor="Supermicro", + server_model="AS -4125GS-TNHR", + ) + analyzer = BiosAnalyzer(system_info=system_info) + args = BiosAnalyzerArgs( + exp_server_vendor="Supermicro", + exp_server_model=["AS -4125GS-TNHR"], + ) + res = analyzer.analyze_data(model, args) + assert res.status == ExecutionStatus.OK + assert len(res.events) == 0 + + +def test_server_vendor_mismatch(system_info): + model = BiosDataModel( + bios_version="TESTBIOS", + server_vendor="Dell", + server_model="AS -4125GS-TNHR", + ) + analyzer = BiosAnalyzer(system_info=system_info) + args = BiosAnalyzerArgs(exp_bios_version=["TESTBIOS"], exp_server_vendor=["Supermicro"]) + res = analyzer.analyze_data(model, args) + assert res.status == ExecutionStatus.ERROR + assert res.message.startswith("Server vendor mismatch!") + assert "Bios data mismatch!" not in res.message + assert any("Server vendor mismatch" in event.description for event in res.events) + + +def test_server_model_mismatch(system_info): + model = BiosDataModel( + bios_version="TESTBIOS", + server_vendor="Supermicro", + server_model="Other", + ) + analyzer = BiosAnalyzer(system_info=system_info) + args = BiosAnalyzerArgs(exp_server_model=["AS -4125GS-TNHR"]) + res = analyzer.analyze_data(model, args) + assert res.status == ExecutionStatus.ERROR + assert res.message.startswith("Server model mismatch!") + assert "Bios data mismatch!" not in res.message + + def test_invalid_regex(system_info): model = BiosDataModel(bios_version="TEST1234BIOS") analyzer = BiosAnalyzer(system_info=system_info) diff --git a/test/unit/plugin/test_bios_collector.py b/test/unit/plugin/test_bios_collector.py index a2455e1c..196dc120 100644 --- a/test/unit/plugin/test_bios_collector.py +++ b/test/unit/plugin/test_bios_collector.py @@ -49,13 +49,24 @@ def test_task_body_windows(system_info, bios_collector): system_info.os_family = OSFamily.WINDOWS bios_collector._run_sut_cmd = MagicMock( - return_value=MagicMock( - exit_code=0, - stdout="\n\nSMBIOSBIOSVersion=TESTBIOS (1.40 )\n\n\n\n", - ) + side_effect=[ + MagicMock( + exit_code=0, + stdout="\n\nSMBIOSBIOSVersion=TESTBIOS (1.40 )\n\n\n\n", + ), + MagicMock( + exit_code=0, + stdout="Manufacturer=Supermicro\nModel=AS -4125GS-TNHR\n", + command="wmic computersystem get Manufacturer,Model /Value", + ), + ] ) - exp_data = BiosDataModel(bios_version="TESTBIOS (1.40 )") + exp_data = BiosDataModel( + bios_version="TESTBIOS (1.40 )", + server_vendor="Supermicro", + server_model="AS -4125GS-TNHR", + ) res, data = bios_collector.collect_data() assert data == exp_data @@ -66,13 +77,18 @@ def test_task_body_linux(system_info, bios_collector): system_info.os_family = OSFamily.LINUX bios_collector._run_sut_cmd = MagicMock( - return_value=MagicMock( - exit_code=0, - stdout="2.0.1", - ) + side_effect=[ + MagicMock(exit_code=0, stdout="2.0.1", command="bios_version"), + MagicMock(exit_code=0, stdout="Supermicro\n", command="sys_vendor"), + MagicMock(exit_code=0, stdout="AS -4125GS-TNHR\n", command="product_name"), + ] ) - exp_data = BiosDataModel(bios_version="2.0.1") + exp_data = BiosDataModel( + bios_version="2.0.1", + server_vendor="Supermicro", + server_model="AS -4125GS-TNHR", + ) res, data = bios_collector.collect_data() assert data == exp_data @@ -83,15 +99,27 @@ def test_task_body_esxi(system_info, bios_collector): system_info.os_family = OSFamily.ESXI bios_collector._run_sut_cmd = MagicMock( - return_value=MagicMock( - exit_code=0, - stdout=' Version: "1.8"', - ) + side_effect=[ + MagicMock(exit_code=0, stdout=' Version: "1.8"', command="bios"), + MagicMock( + exit_code=0, + stdout=( + " System Info (Type 1):\n" + ' Manufacturer: "Supermicro"\n' + ' Product: "AS -4125GS-TNHR"\n' + ), + command="system", + ), + ] ) res, data = bios_collector.collect_data() assert res.status == ExecutionStatus.OK - assert data == BiosDataModel(bios_version="1.8") + assert data == BiosDataModel( + bios_version="1.8", + server_vendor="Supermicro", + server_model="AS -4125GS-TNHR", + ) def test_task_body_error(system_info, bios_collector): diff --git a/test/unit/plugin/test_device_enumeration_analyzer.py b/test/unit/plugin/test_device_enumeration_analyzer.py index c58c9ea0..07b0382e 100644 --- a/test/unit/plugin/test_device_enumeration_analyzer.py +++ b/test/unit/plugin/test_device_enumeration_analyzer.py @@ -115,6 +115,35 @@ def test_analyze_mismatched_cpu_count(device_enumeration_analyzer): assert event.category == EventCategory.PLATFORM.value +def test_analyze_architecture_and_physical_cores(device_enumeration_analyzer): + data = DeviceEnumerationDataModel( + cpu_count=2, cpu_architecture="x86_64", cpu_cores=128, gpu_count=8, vf_count=0 + ) + args = DeviceEnumerationAnalyzerArgs( + cpu_architecture="x86_64", + min_cpu_cores=96, + gpu_count=8, + ) + + result = device_enumeration_analyzer.analyze_data(data=data, args=args) + + assert result.status == ExecutionStatus.OK + assert "cpu_architecture" in result.message + assert "cpu_cores" in result.message + assert "gpu_count" in result.message + assert len(result.events) == 0 + + +def test_analyze_physical_cores_below_minimum(device_enumeration_analyzer): + data = DeviceEnumerationDataModel(cpu_count=2, cpu_architecture="x86_64", cpu_cores=64) + args = DeviceEnumerationAnalyzerArgs(cpu_architecture=["x86_64"], min_cpu_cores=96) + + result = device_enumeration_analyzer.analyze_data(data=data, args=args) + + assert result.status == ExecutionStatus.ERROR + assert any("cpu_cores" in event.description for event in result.events) + + def test_analyze_list_of_accepted_counts(device_enumeration_analyzer): """Test with a list of acceptable counts""" diff --git a/test/unit/plugin/test_device_enumeration_collector.py b/test/unit/plugin/test_device_enumeration_collector.py index 3577e402..975f7504 100644 --- a/test/unit/plugin/test_device_enumeration_collector.py +++ b/test/unit/plugin/test_device_enumeration_collector.py @@ -55,7 +55,12 @@ def test_collect_linux(system_info, device_enumeration_collector): """Test linux typical output""" system_info.os_family = OSFamily.LINUX - lscpu_output = "Architecture: x86_64\nCPU(s): 64\nSocket(s): 2" + lscpu_output = ( + "Architecture: x86_64\n" + "CPU(s): 256\n" + "Core(s) per socket: 64\n" + "Socket(s): 2" + ) lshw_output = "*-cpu\n product: AMD EPYC 1234 64-Core Processor" device_enumeration_collector._run_sut_cmd = MagicMock( @@ -94,7 +99,13 @@ def test_collect_linux(system_info, device_enumeration_collector): result, data = device_enumeration_collector.collect_data() assert result.status == ExecutionStatus.OK assert data == DeviceEnumerationDataModel( - cpu_count=2, gpu_count=8, vf_count=0, lscpu_output=lscpu_output, lshw_output=lshw_output + cpu_count=2, + cpu_architecture="x86_64", + cpu_cores=128, + gpu_count=8, + vf_count=0, + lscpu_output=lscpu_output, + lshw_output=lshw_output, ) assert ( len([a for a in result.artifacts if hasattr(a, "filename") and a.filename == "lshw.txt"]) @@ -126,12 +137,24 @@ def test_collect_windows(system_info, device_enumeration_collector): stderr="", command='powershell -Command "(Get-VMHostPartitionableGpu | Measure-Object).Count"', ), + MagicMock( + exit_code=0, + stdout="Architecture=x86_64\nCores=128\n", + stderr="", + command="Win32_Processor", + ), ] ) result, data = device_enumeration_collector.collect_data() assert result.status == ExecutionStatus.OK - assert data == DeviceEnumerationDataModel(cpu_count=2, gpu_count=8, vf_count=8) + assert data == DeviceEnumerationDataModel( + cpu_count=2, + cpu_architecture="x86_64", + cpu_cores=128, + gpu_count=8, + vf_count=8, + ) def test_collect_error(system_info, device_enumeration_collector): diff --git a/test/unit/plugin/test_memory_analyzer.py b/test/unit/plugin/test_memory_analyzer.py index 38297320..382be3b9 100644 --- a/test/unit/plugin/test_memory_analyzer.py +++ b/test/unit/plugin/test_memory_analyzer.py @@ -54,6 +54,26 @@ def test_too_much_memory_usage(analyzer, model_obj): assert result.status == ExecutionStatus.ERROR +def test_minimum_system_memory_passes_nominal_2tib(analyzer): + """2048 GiB nominal still passes when the OS reports slightly less.""" + model = MemoryDataModel(mem_free="1900Gi", mem_total="2016Gi", mem_available="1900Gi") + result = analyzer.analyze_data( + model, + MemoryAnalyzerArgs(minimum_system_memory_gib=2048, ratio=0.9, memory_threshold="4Ti"), + ) + assert result.status == ExecutionStatus.OK + + +def test_minimum_system_memory_fails(analyzer): + model = MemoryDataModel(mem_free="100Gi", mem_total="1024Gi", mem_available="100Gi") + result = analyzer.analyze_data( + model, + MemoryAnalyzerArgs(minimum_system_memory_gib=2048), + ) + assert result.status == ExecutionStatus.ERROR + assert "Installed system memory is below the minimum" in result.message + + def test_config_provided(analyzer, model_obj): args = MemoryAnalyzerArgs(ratio=0.66, memory_threshold="30Gi") result = analyzer.analyze_data(model_obj, args) From fad927bb2f52a51c778c23ec26bd4b641a717510 Mon Sep 17 00:00:00 2001 From: Jaspal Singh Date: Mon, 19 Oct 2026 16:02:17 +0000 Subject: [PATCH 2/7] test failure fix --- nodescraper/plugins/inband/memory/analyzer_args.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nodescraper/plugins/inband/memory/analyzer_args.py b/nodescraper/plugins/inband/memory/analyzer_args.py index 90cd3f98..46763c00 100644 --- a/nodescraper/plugins/inband/memory/analyzer_args.py +++ b/nodescraper/plugins/inband/memory/analyzer_args.py @@ -38,7 +38,7 @@ class MemoryAnalyzerArgs(AnalyzerArgs): ge=0, description=( "Minimum installed system memory in GiB. Compared to MemTotal. " - "Passes when reported memory is at least 98% of this value, because " + "Passes when reported memory is at least 98 percent of this value, because " "the OS reports slightly less than nominal DIMM capacity." ), ) From 884c3587a3ac91bec18c84673782a4704cf00550 Mon Sep 17 00:00:00 2001 From: jaspals3123 Date: Fri, 30 Oct 2026 15:30:30 +0000 Subject: [PATCH 3/7] Keep in-band checks from failing closed or clearing a real error. Quote the Windows CPU command so it survives OpenSSH and PowerShell, treat missing UMC RAS data as unavailable rather than an ECC mismatch, and finish the other memory checks after an installed-memory failure without letting a later pass set OK. Co-authored-by: Cursor --- .../plugins/inband/amdsmi/amdsmi_analyzer.py | 57 ++++++++++---- .../device_enumeration_collector.py | 11 ++- .../plugins/inband/memory/memory_analyzer.py | 78 +++++++++++-------- test/unit/plugin/test_amdsmi_analyzer.py | 34 +++++++- test/unit/plugin/test_memory_analyzer.py | 42 ++++++++++ 5 files changed, 167 insertions(+), 55 deletions(-) diff --git a/nodescraper/plugins/inband/amdsmi/amdsmi_analyzer.py b/nodescraper/plugins/inband/amdsmi/amdsmi_analyzer.py index 5e451c03..09970ed8 100644 --- a/nodescraper/plugins/inband/amdsmi/amdsmi_analyzer.py +++ b/nodescraper/plugins/inband/amdsmi/amdsmi_analyzer.py @@ -673,23 +673,48 @@ def check_min_hbm_gib_per_gpu( ) def check_umc_ras_enabled(self, amdsmi_static_data: list[AmdSmiStatic]) -> None: - """Require UMC RAS ENABLED. That is the in-band proxy for HBM ECC being on.""" - actual_by_gpu: dict[int, str] = {} + """Require UMC RAS to be enabled when per-block data is available. + + Args: + amdsmi_static_data: Static amd-smi data for each GPU. + + Returns: + None. + """ + unavailable: list[int] = [] + not_enabled: dict[int, str] = {} for gpu_data in amdsmi_static_data: - umc_state = _umc_ecc_state(gpu_data.ras.ecc_block_state) - if umc_state != EccState.ENABLED: - shown = umc_state.value if isinstance(umc_state, EccState) else "missing" - actual_by_gpu[gpu_data.gpu] = shown - if not actual_by_gpu: - return - desc, details = _gpu_mismatch_description("UMC RAS (HBM ECC)", "ENABLED", actual_by_gpu) - self._log_event( - category=EventCategory.RAS, - description=desc, - priority=EventPriority.ERROR, - data={"umc_ras_by_gpu": actual_by_gpu, "details": details}, - console_log=True, - ) + block_states = gpu_data.ras.ecc_block_state + if not isinstance(block_states, dict): + unavailable.append(gpu_data.gpu) + continue + umc_state = _umc_ecc_state(block_states) + if umc_state is None: + unavailable.append(gpu_data.gpu) + elif umc_state != EccState.ENABLED: + not_enabled[gpu_data.gpu] = umc_state.value + + if unavailable: + self._log_event( + category=EventCategory.RAS, + description="UMC RAS state is unavailable for one or more GPUs", + priority=EventPriority.ERROR, + data={"gpus": unavailable}, + console_log=True, + ) + if not_enabled: + desc, details = _gpu_mismatch_description( + "UMC RAS (HBM ECC)", + "ENABLED", + not_enabled, + ) + self._log_event( + category=EventCategory.RAS, + description=desc, + priority=EventPriority.ERROR, + data={"umc_ras_by_gpu": not_enabled, "details": details}, + console_log=True, + ) def check_amdsmi_metric_ecc(self, amdsmi_metric_data: list[AmdSmiMetric]): """Check ECC counts in all blocks for all GPUs diff --git a/nodescraper/plugins/inband/device_enumeration/device_enumeration_collector.py b/nodescraper/plugins/inband/device_enumeration/device_enumeration_collector.py index 36865fe9..2f952e0b 100644 --- a/nodescraper/plugins/inband/device_enumeration/device_enumeration_collector.py +++ b/nodescraper/plugins/inband/device_enumeration/device_enumeration_collector.py @@ -56,14 +56,17 @@ class DeviceEnumerationCollector( ) # Architecture codes are Win32_Processor.Architecture. Names match lscpu # (9 -> x86_64) so the same analyzer value works on Linux and Windows. + # -Command is single-quoted so $vars are not expanded by a PowerShell parent + # and so OpenSSH's cmd /c "..." wrapper does not split the script. Inner + # quotes are doubled ('') because they sit inside that single-quoted script. CMD_CPU_INFO_WINDOWS = ( "powershell -NoProfile -Command " - '"$p = @(Get-CimInstance -ClassName Win32_Processor); ' + "'$p = @(Get-CimInstance -ClassName Win32_Processor); " "$cores = ($p | Measure-Object -Property NumberOfCores -Sum).Sum; " - "$map = @{0='i686'; 5='arm'; 6='ia64'; 9='x86_64'; 12='aarch64'}; " + "$map = @{0=''i686''; 5=''arm''; 6=''ia64''; 9=''x86_64''; 12=''aarch64''}; " "$arch = $map[[int]$p[0].Architecture]; " - "Write-Output ('Architecture=' + $arch); " - "Write-Output ('Cores=' + $cores)\"" + "Write-Output (''Architecture='' + $arch); " + "Write-Output (''Cores='' + $cores)'" ) CMD_GPU_COUNT_WINDOWS = 'powershell -Command "(wmic path win32_VideoController get name | findstr AMD | Measure-Object).Count"' CMD_VF_COUNT_WINDOWS = ( diff --git a/nodescraper/plugins/inband/memory/memory_analyzer.py b/nodescraper/plugins/inband/memory/memory_analyzer.py index 34642337..83fe8ae4 100644 --- a/nodescraper/plugins/inband/memory/memory_analyzer.py +++ b/nodescraper/plugins/inband/memory/memory_analyzer.py @@ -63,13 +63,15 @@ def _bytes_to_gb(n: float) -> float: total_memory = convert_to_bytes(data.mem_total) used_memory = total_memory - available_memory + # Recorded here and applied once at the end so a later passing check + # cannot replace this error with ExecutionStatus.OK. + installed_memory_error = False if args.minimum_system_memory_gib is not None: minimum_bytes = args.minimum_system_memory_gib * (1024**3) required_bytes = minimum_bytes * self._INSTALLED_MEMORY_TOLERANCE if total_memory < required_bytes: total_gib = _bytes_to_gb(total_memory) - self.result.status = ExecutionStatus.ERROR - self.result.message = "Installed system memory is below the minimum" + installed_memory_error = True self._log_event( category=EventCategory.OS, description=( @@ -84,13 +86,10 @@ def _bytes_to_gb(n: float) -> float: }, console_log=True, ) - return self.result available_percent = None if args.minimum_free_memory_percent is not None: if total_memory <= 0: - self.result.status = ExecutionStatus.WARNING - self.result.message = "Total memory is unavailable" self._log_event( category=EventCategory.OS, description="Cannot validate minimum free memory percentage", @@ -98,28 +97,32 @@ def _bytes_to_gb(n: float) -> float: data={"total_memory": total_memory, "available_memory": available_memory}, console_log=True, ) - return self.result - - available_percent = available_memory / total_memory * 100 - if available_percent < args.minimum_free_memory_percent: - self.result.status = ExecutionStatus.ERROR - self.result.message = "Minimum free memory percentage not met" - self._log_event( - category=EventCategory.OS, - description=( - f"Available memory is {available_percent:.2f}% " - f"(minimum {args.minimum_free_memory_percent:.2f}%)" - ), - priority=EventPriority.CRITICAL, - data={ - "available_memory": available_memory, - "total_memory": total_memory, - "available_percent": available_percent, - "minimum_free_memory_percent": args.minimum_free_memory_percent, - }, - console_log=True, - ) - return self.result + if not installed_memory_error: + self.result.status = ExecutionStatus.WARNING + self.result.message = "Total memory is unavailable" + return self.result + else: + available_percent = available_memory / total_memory * 100 + if available_percent < args.minimum_free_memory_percent: + self._log_event( + category=EventCategory.OS, + description=( + f"Available memory is {available_percent:.2f}% " + f"(minimum {args.minimum_free_memory_percent:.2f}%)" + ), + priority=EventPriority.CRITICAL, + data={ + "available_memory": available_memory, + "total_memory": total_memory, + "available_percent": available_percent, + "minimum_free_memory_percent": args.minimum_free_memory_percent, + }, + console_log=True, + ) + if not installed_memory_error: + self.result.status = ExecutionStatus.ERROR + self.result.message = "Minimum free memory percentage not met" + return self.result threshold_bytes = convert_to_bytes(args.memory_threshold) @@ -136,20 +139,19 @@ def _bytes_to_gb(n: float) -> float: allowed_gb = _bytes_to_gb(max_allowed_used_mem) base_gb = _bytes_to_gb(base_bytes) - if used_memory < max_allowed_used_mem: - self.result.message = ( + usage_within_limit = used_memory < max_allowed_used_mem + if usage_within_limit: + usage_message = ( f"Memory usage is within limit: Used {used_gb:.2f} GB " f"(allowed {allowed_gb:.2f} GB; base={base_source} {base_gb:.2f} GB × ratio={args.ratio:.2f})" ) if available_percent is not None: - self.result.message += ( + usage_message += ( f"; available memory is {available_percent:.2f}% " f"(minimum {args.minimum_free_memory_percent:.2f}%)" ) - self.result.status = ExecutionStatus.OK else: - self.result.message = "Memory usage exceeded max allowed!" - self.result.status = ExecutionStatus.ERROR + usage_message = "Memory usage exceeded max allowed!" self._log_event( category=EventCategory.OS, description=f"Memory usage exceeded max allowed! Used {used_gb:.2f} GB, max allowed {allowed_gb:.2f} GB (base={base_source} {base_gb:.2f} GB × ratio={args.ratio:.2f})", @@ -163,4 +165,14 @@ def _bytes_to_gb(n: float) -> float: }, ) + if installed_memory_error: + self.result.status = ExecutionStatus.ERROR + self.result.message = "Installed system memory is below the minimum" + elif usage_within_limit: + self.result.status = ExecutionStatus.OK + self.result.message = usage_message + else: + self.result.status = ExecutionStatus.ERROR + self.result.message = usage_message + return self.result diff --git a/test/unit/plugin/test_amdsmi_analyzer.py b/test/unit/plugin/test_amdsmi_analyzer.py index 39672b96..bc96a8b1 100644 --- a/test/unit/plugin/test_amdsmi_analyzer.py +++ b/test/unit/plugin/test_amdsmi_analyzer.py @@ -1600,7 +1600,8 @@ def test_check_umc_ras_enabled_passes(mock_analyzer): assert len(analyzer.result.events) == 0 -def test_check_umc_ras_enabled_fails_when_umc_missing(mock_analyzer): +def test_check_umc_ras_enabled_unavailable_when_umc_missing(mock_analyzer): + """Absent per-block data cannot be validated, so it is not an ECC mismatch.""" analyzer = mock_analyzer gpu = create_static_gpu(0) @@ -1608,4 +1609,33 @@ def test_check_umc_ras_enabled_fails_when_umc_missing(mock_analyzer): assert len(analyzer.result.events) == 1 assert analyzer.result.events[0].priority == EventPriority.ERROR - assert "UMC RAS" in analyzer.result.events[0].description + assert analyzer.result.events[0].description == ( + "UMC RAS state is unavailable for one or more GPUs" + ) + assert analyzer.result.events[0].data["gpus"] == [0] + + +def test_check_umc_ras_enabled_unavailable_when_block_state_is_string(mock_analyzer): + """Older amd-smi output reports ecc_block_state as a string, not a UMC map.""" + analyzer = mock_analyzer + gpu = create_static_gpu(0) + gpu.ras.ecc_block_state = "N/A" + + analyzer.check_umc_ras_enabled([gpu]) + + assert len(analyzer.result.events) == 1 + assert analyzer.result.events[0].data["gpus"] == [0] + assert "umc_ras_by_gpu" not in analyzer.result.events[0].data + + +def test_check_umc_ras_enabled_fails_when_disabled(mock_analyzer): + analyzer = mock_analyzer + gpu = create_static_gpu(0) + gpu.ras.ecc_block_state = {"UMC": EccState.DISABLED} + + analyzer.check_umc_ras_enabled([gpu]) + + assert len(analyzer.result.events) == 1 + assert analyzer.result.events[0].priority == EventPriority.ERROR + assert "UMC RAS (HBM ECC)" in analyzer.result.events[0].description + assert analyzer.result.events[0].data["umc_ras_by_gpu"] == {0: "DISABLED"} diff --git a/test/unit/plugin/test_memory_analyzer.py b/test/unit/plugin/test_memory_analyzer.py index 382be3b9..ade8e33e 100644 --- a/test/unit/plugin/test_memory_analyzer.py +++ b/test/unit/plugin/test_memory_analyzer.py @@ -65,6 +65,7 @@ def test_minimum_system_memory_passes_nominal_2tib(analyzer): def test_minimum_system_memory_fails(analyzer): + """Installed-memory failure still runs the max-usage check and stays an error.""" model = MemoryDataModel(mem_free="100Gi", mem_total="1024Gi", mem_available="100Gi") result = analyzer.analyze_data( model, @@ -72,6 +73,47 @@ def test_minimum_system_memory_fails(analyzer): ) assert result.status == ExecutionStatus.ERROR assert "Installed system memory is below the minimum" in result.message + descriptions = [event.description for event in result.events] + assert any(description.startswith("Installed memory is") for description in descriptions) + assert any(description.startswith("Memory usage exceeded") for description in descriptions) + + +def test_minimum_system_memory_failure_keeps_error_when_later_checks_pass(analyzer): + """A passing free-memory and max-usage check must not clear the installed-memory error.""" + model = MemoryDataModel(mem_free="1010Gi", mem_total="1024Gi", mem_available="1010Gi") + result = analyzer.analyze_data( + model, + MemoryAnalyzerArgs( + minimum_system_memory_gib=2048, + minimum_free_memory_percent=10, + ratio=0.9, + memory_threshold="4Ti", + ), + ) + assert result.status == ExecutionStatus.ERROR + assert "Installed system memory is below the minimum" in result.message + assert "within limit" not in result.message + assert len(result.events) == 1 + assert result.events[0].description.startswith("Installed memory is") + + +def test_minimum_system_memory_failure_still_checks_free_percent(analyzer): + model = MemoryDataModel(mem_free="10Gi", mem_total="1024Gi", mem_available="10Gi") + result = analyzer.analyze_data( + model, + MemoryAnalyzerArgs( + minimum_system_memory_gib=2048, + minimum_free_memory_percent=50, + ratio=0.999, + memory_threshold="4Ti", + ), + ) + assert result.status == ExecutionStatus.ERROR + assert "Installed system memory is below the minimum" in result.message + descriptions = [event.description for event in result.events] + assert any(description.startswith("Installed memory is") for description in descriptions) + assert any(description.startswith("Available memory is") for description in descriptions) + assert not any(description.startswith("Memory usage exceeded") for description in descriptions) def test_config_provided(analyzer, model_obj): From 3b8c3e28d86e9d9dd88ba4807a3a8b5dd5da98b1 Mon Sep 17 00:00:00 2001 From: jaspals3123 Date: Fri, 30 Oct 2026 15:59:41 +0000 Subject: [PATCH 4/7] Add the blank lines Black requires between top-level test helpers. Co-authored-by: Cursor --- test/unit/plugin/test_amdsmi_analyzer.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/unit/plugin/test_amdsmi_analyzer.py b/test/unit/plugin/test_amdsmi_analyzer.py index da464bfb..73711b30 100644 --- a/test/unit/plugin/test_amdsmi_analyzer.py +++ b/test/unit/plugin/test_amdsmi_analyzer.py @@ -1639,6 +1639,8 @@ def test_check_umc_ras_enabled_fails_when_disabled(mock_analyzer): assert analyzer.result.events[0].priority == EventPriority.ERROR assert "UMC RAS (HBM ECC)" in analyzer.result.events[0].description assert analyzer.result.events[0].data["umc_ras_by_gpu"] == {0: "DISABLED"} + + def create_unconfigured_fabric(gpu: int) -> Fabric: """Helper building amd-smi fabric output for a GPU whose fabric is not configured.""" return Fabric.model_validate( From 5283134ca0e858f7319b48afe80ae406124f0b37 Mon Sep 17 00:00:00 2001 From: jaspals3123 Date: Fri, 30 Oct 2026 16:01:18 +0000 Subject: [PATCH 5/7] Expect binary petabytes from bytes_to_human_readable. Co-authored-by: Cursor --- test/unit/framework/test_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/framework/test_utils.py b/test/unit/framework/test_utils.py index 7dc4f52c..3b279a89 100644 --- a/test/unit/framework/test_utils.py +++ b/test/unit/framework/test_utils.py @@ -42,7 +42,7 @@ def test_bytes_to_human_readable_terabytes(): def test_bytes_to_human_readable_petabytes(): """PB is a documented unit, so petabyte scale values must not be reported in TB.""" - assert bytes_to_human_readable(2 * 10**15) == "2.0PB" + assert bytes_to_human_readable(2 << 50) == "2.0 PB" def test_unit_size_to_bytes_treats_gb_as_binary(): From a307d45f3d7d15055376edd93733a4a79c2b5c13 Mon Sep 17 00:00:00 2001 From: jaspals3123 Date: Fri, 30 Oct 2026 16:23:24 +0000 Subject: [PATCH 6/7] Include fabric mismatch details on the analyzer event. Co-authored-by: Cursor --- nodescraper/plugins/inband/amdsmi/amdsmi_analyzer.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/nodescraper/plugins/inband/amdsmi/amdsmi_analyzer.py b/nodescraper/plugins/inband/amdsmi/amdsmi_analyzer.py index 8a99cdef..e767cc11 100644 --- a/nodescraper/plugins/inband/amdsmi/amdsmi_analyzer.py +++ b/nodescraper/plugins/inband/amdsmi/amdsmi_analyzer.py @@ -1289,6 +1289,11 @@ def _add(gpu: int, field: str, expected: object, actual: object) -> None: if issues: gpus_affected = len({i["gpu"] for i in issues}) + details = "; ".join( + f"GPU {item['gpu']} {item['field']}: expected {item['expected']}, " + f"actual {item['actual']}" + for item in issues + ) self._log_event( category=EventCategory.NETWORK, description=f"Fabric data mismatch on {gpus_affected} GPU(s)", @@ -1297,6 +1302,7 @@ def _add(gpu: int, field: str, expected: object, actual: object) -> None: "expected_accel_state": expected_state, "expected_fabric_type": expected_type, "mismatches": issues, + "details": details, }, console_log=True, ) From e66349a34a33fc0f7ecd66ece3661c0f1b52deed Mon Sep 17 00:00:00 2001 From: jaspals3123 Date: Fri, 30 Oct 2026 16:33:38 +0000 Subject: [PATCH 7/7] Treat IEC memory suffixes as powers of 1024. Co-authored-by: Cursor --- nodescraper/utils.py | 34 +++++++++++++++---------------- test/unit/framework/test_utils.py | 9 ++++++++ 2 files changed, 26 insertions(+), 17 deletions(-) diff --git a/nodescraper/utils.py b/nodescraper/utils.py index 034414ad..2e17ba3f 100644 --- a/nodescraper/utils.py +++ b/nodescraper/utils.py @@ -190,41 +190,41 @@ def command_result_event_data( def convert_to_bytes(value: str, si=True) -> int: - """ - Convert human-readable memory sizes (like GB, MB) to bytes. - If si=False (default), IEC units are used (1KiB=1024 bytes). - If si=True, SI units are used (1KB=1000 bytes). - Factor of powers of 2, not 10. (e.g. 1KB=1000 bytes and 1KiB=1024 bytes) - This can be changed with si=True (1KB=1000 bytes) + """Convert a memory size string to bytes. + + An IEC suffix (Ki, Mi, Gi, Ti, and the rest) always uses powers of 1024. + Without that suffix, ``si=True`` uses powers of 1000 and ``si=False`` uses + powers of 1024. A bare number is already a byte count. """ value = value.strip().upper() unit_names = ["K", "M", "G", "T", "P", "E", "Z", "Y"] - if si: - exponent_base = 10 - exponent_power = 3 - else: - exponent_base = 2 - exponent_power = 10 - # Extract the numeric part and the unit - pattern = re.compile(r"(\d+\.?\d*)([YZEPTGMK]?)") + # Extract the numeric part and the unit. The optional I marks an IEC suffix. + pattern = re.compile(r"(\d+\.?\d*)([YZEPTGMK]?)(I?)") match = pattern.match(value) if not match: raise ValueError(f"Invalid memory value: {value}") # Handle the numeric value and ensure it's a valid number try: - value = float(match.group(1)) + number = float(match.group(1)) except ValueError as err: raise ValueError(f"Invalid numeric value in: {value}") from err unit = match.group(2) + iec = bool(match.group(3)) + if iec or not si: + exponent_base = 2 + exponent_power = 10 + else: + exponent_base = 10 + exponent_power = 3 # Convert the value to bytes for unit_index, unit_name in enumerate(unit_names): if unit == unit_name: - return int(float(value) * (exponent_base ** ((unit_index + 1) * exponent_power))) + return int(number * (exponent_base ** ((unit_index + 1) * exponent_power))) # If the unit is not found, it is bytes - return int(value) + return int(number) _UNIT_BYTES = { diff --git a/test/unit/framework/test_utils.py b/test/unit/framework/test_utils.py index 3b279a89..748ff971 100644 --- a/test/unit/framework/test_utils.py +++ b/test/unit/framework/test_utils.py @@ -6,6 +6,7 @@ from nodescraper.utils import ( bytes_to_human_readable, + convert_to_bytes, find_annotation_in_container, unit_size_to_bytes, ) @@ -40,6 +41,14 @@ def test_bytes_to_human_readable_terabytes(): assert bytes_to_human_readable(2 << 40) == "2.0 TB" +def test_convert_to_bytes_iec_suffix_is_binary(): + """Gi and Ti are gibibytes and tebibytes, not decimal G and T.""" + assert convert_to_bytes("2016Gi") == 2016 * 1024**3 + assert convert_to_bytes("4Ti") == 4 * 1024**4 + assert convert_to_bytes("2G") == 2 * 10**9 + assert convert_to_bytes("2G", si=False) == 2 * 1024**3 + + def test_bytes_to_human_readable_petabytes(): """PB is a documented unit, so petabyte scale values must not be reported in TB.""" assert bytes_to_human_readable(2 << 50) == "2.0 PB"