From eda28dcfcb1e4e5e453f282d116cb92426073f47 Mon Sep 17 00:00:00 2001 From: Christian Berendt Date: Wed, 11 Nov 2026 22:13:09 +0100 Subject: [PATCH 1/8] Add run_remote helper for non-interactive SSH commands The report commands each build the same ssh command line and call it once per value they need. The hardware reports open one session per device, pipe a collector script to the remote python3, and run many sessions from worker threads, where a password prompt would block a thread for good. run_remote() runs the ssh binary with the options the report commands already use, adds BatchMode=yes, passes text on stdin and bounds the call with a timeout. Tests: the command line, stdin and timeout handling, and that the timeout exception reaches the caller. Part of #2876 and #2877. Assisted-by: Claude:claude-fable-5-2 Signed-off-by: Christian Berendt --- osism/utils/ssh.py | 57 ++++++++++++++++++++++++++++++++++ tests/unit/utils/test_ssh.py | 60 ++++++++++++++++++++++++++++++++++++ 2 files changed, 117 insertions(+) diff --git a/osism/utils/ssh.py b/osism/utils/ssh.py index e2d64ec00..2accaa1dd 100644 --- a/osism/utils/ssh.py +++ b/osism/utils/ssh.py @@ -10,6 +10,63 @@ # Default path for SSH known_hosts file KNOWN_HOSTS_PATH = "/share/known_hosts" +# Private key of the operator user, deployed to nodes and SONiC switches +OPERATOR_KEY_PATH = "/ansible/secrets/id_rsa.operator" + + +def run_remote( + host: str, + user: str, + command: str, + stdin: Optional[str] = None, + timeout: float = 60, + known_hosts_path: str = KNOWN_HOSTS_PATH, +) -> subprocess.CompletedProcess: + """Run a command on a remote host through the ssh binary. + + Authentication uses the operator key only. ``BatchMode=yes`` makes a + missing or rejected key fail right away instead of waiting for a + password on a terminal that is not there. + + Args: + host: Address or name of the remote host + user: Remote user name + command: Command line executed by the remote login shell + stdin: Text piped to the remote command, e.g. a script for + ``python3 -`` + timeout: Seconds after which the whole call is aborted + known_hosts_path: Path to the known_hosts file + + Returns: + The completed process with ``returncode``, ``stdout`` and ``stderr`` + + Raises: + subprocess.TimeoutExpired: The call did not finish within ``timeout`` + """ + return subprocess.run( + [ + "/usr/bin/ssh", + "-i", + OPERATOR_KEY_PATH, + "-o", + "BatchMode=yes", + "-o", + "StrictHostKeyChecking=no", + "-o", + "LogLevel=ERROR", + "-o", + f"UserKnownHostsFile={known_hosts_path}", + "-o", + "ConnectTimeout=10", + f"{user}@{host}", + command, + ], + input=stdin, + capture_output=True, + text=True, + timeout=timeout, + ) + def ensure_known_hosts_file(known_hosts_path: str = KNOWN_HOSTS_PATH) -> bool: """ diff --git a/tests/unit/utils/test_ssh.py b/tests/unit/utils/test_ssh.py index 497e662a3..95d3272e4 100644 --- a/tests/unit/utils/test_ssh.py +++ b/tests/unit/utils/test_ssh.py @@ -14,11 +14,13 @@ from osism.utils.ssh import ( KNOWN_HOSTS_PATH, + OPERATOR_KEY_PATH, backup_known_hosts, cleanup_ssh_known_hosts_for_node, ensure_known_hosts_file, get_host_identifiers, remove_known_hosts_entries, + run_remote, ) # --------------------------------------------------------------------------- @@ -619,3 +621,61 @@ def test_cleanup_remove_raises(mocker, loguru_logs): assert cleanup_ssh_known_hosts_for_node("node01") is False assert _has_log(loguru_logs, "ERROR", "Error during SSH known_hosts cleanup") + + +# --------------------------------------------------------------------------- +# run_remote +# --------------------------------------------------------------------------- + + +def test_run_remote_builds_a_batch_mode_ssh_call(mocker): + result = _run_result(stdout="ok") + run = mocker.patch("osism.utils.ssh.subprocess.run", return_value=result) + + assert run_remote("192.0.2.1", "dragon", "uname -a") is result + + argv = run.call_args.args[0] + assert argv[0] == "/usr/bin/ssh" + assert argv[argv.index("-i") + 1] == OPERATOR_KEY_PATH + options = [argv[i + 1] for i, item in enumerate(argv) if item == "-o"] + assert options == [ + "BatchMode=yes", + "StrictHostKeyChecking=no", + "LogLevel=ERROR", + f"UserKnownHostsFile={KNOWN_HOSTS_PATH}", + "ConnectTimeout=10", + ] + assert argv[-2:] == ["dragon@192.0.2.1", "uname -a"] + assert run.call_args.kwargs == { + "input": None, + "capture_output": True, + "text": True, + "timeout": 60, + } + + +def test_run_remote_pipes_stdin_and_passes_the_timeout(mocker): + run = mocker.patch("osism.utils.ssh.subprocess.run", return_value=_run_result()) + + run_remote( + "leaf-1a", + "admin", + "python3 -", + stdin="print(1)", + timeout=5, + known_hosts_path="/tmp/kh", + ) + + assert run.call_args.kwargs["input"] == "print(1)" + assert run.call_args.kwargs["timeout"] == 5 + assert "UserKnownHostsFile=/tmp/kh" in run.call_args.args[0] + + +def test_run_remote_lets_the_timeout_propagate(mocker): + mocker.patch( + "osism.utils.ssh.subprocess.run", + side_effect=subprocess.TimeoutExpired(cmd="ssh", timeout=5), + ) + + with pytest.raises(subprocess.TimeoutExpired): + run_remote("192.0.2.1", "dragon", "true", timeout=5) From cb316d17b89d68d6b08171c96477e093c0ef5fa4 Mon Sep 17 00:00:00 2001 From: Christian Berendt Date: Wed, 11 Nov 2026 22:13:09 +0100 Subject: [PATCH 2/8] Let the SONiC interface cache take interfaces fetched in bulk convert_netbox_interface_to_sonic() reads the interfaces of a device from the thread-local interface cache and fetches them from NetBox on a miss, one request per device. A caller that already loaded the interfaces of many devices with a single query had no way to hand them over. set_device_interfaces() stores such a list, so that later lookups for that device are served from the cache. Tests: a seeded device is served without a NetBox request, the list is copied, and a second call replaces the entry. Part of #2876 and #2877. Assisted-by: Claude:claude-fable-5-2 Signed-off-by: Christian Berendt --- osism/tasks/conductor/sonic/cache.py | 14 +++++++++ .../unit/tasks/conductor/sonic/test_cache.py | 29 +++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/osism/tasks/conductor/sonic/cache.py b/osism/tasks/conductor/sonic/cache.py index 985597f50..af69e7f19 100644 --- a/osism/tasks/conductor/sonic/cache.py +++ b/osism/tasks/conductor/sonic/cache.py @@ -46,6 +46,20 @@ def get_device_interfaces(self, device_id: int) -> List: return self._cache[device_id] + def set_device_interfaces(self, device_id: int, interfaces: List) -> None: + """Store interfaces of a device that were fetched elsewhere. + + A caller that already loaded the interfaces of many devices in bulk + can hand them over, so that later lookups do not fetch them again + device by device. + + Args: + device_id: NetBox device ID + interfaces: Interface objects of the device + """ + with self._lock: + self._cache[device_id] = list(interfaces) + def clear(self): """Clear the cache.""" with self._lock: diff --git a/tests/unit/tasks/conductor/sonic/test_cache.py b/tests/unit/tasks/conductor/sonic/test_cache.py index e69bd0a62..b34693ef6 100644 --- a/tests/unit/tasks/conductor/sonic/test_cache.py +++ b/tests/unit/tasks/conductor/sonic/test_cache.py @@ -182,6 +182,35 @@ def test_get_device_interfaces_handles_empty_filter_result(mock_nb): assert mock_nb.dcim.interfaces.filter.call_count == 1 +# --------------------------------------------------------------------------- +# InterfaceCache.set_device_interfaces +# --------------------------------------------------------------------------- + + +def test_set_device_interfaces_serves_later_lookups_without_fetching(mock_nb): + cache = InterfaceCache() + interfaces = [MagicMock(name="Eth1/1"), MagicMock(name="Eth1/2")] + + cache.set_device_interfaces(7, interfaces) + + assert cache.get_device_interfaces(7) == interfaces + mock_nb.dcim.interfaces.filter.assert_not_called() + assert cache.get_cache_stats() == {"cached_devices": 1, "total_interfaces": 2} + + +def test_set_device_interfaces_stores_a_copy_and_replaces_an_entry(mock_nb): + cache = InterfaceCache() + interfaces = [MagicMock()] + cache.set_device_interfaces(7, interfaces) + interfaces.append(MagicMock()) + + assert len(cache.get_device_interfaces(7)) == 1 + + cache.set_device_interfaces(7, []) + + assert cache.get_device_interfaces(7) == [] + + # --------------------------------------------------------------------------- # InterfaceCache.clear # --------------------------------------------------------------------------- From e5551083d39f478551b9befbf3bb7bde1aeec303 Mon Sep 17 00:00:00 2001 From: Christian Berendt Date: Wed, 11 Nov 2026 22:13:09 +0100 Subject: [PATCH 3/8] Add the hardware report package with MAC and sort helpers osism.utils.hardware will hold what the live hardware reports share: device selection, collection over SSH, parsing, interface mapping and rendering. It starts with two helpers used throughout. normalize_mac() brings the spellings found in sysfs, ethtool, LLDP and NetBox into one form and rejects the all-zero address. natural_key() sorts interface names by the value of their numbers, Ethernet2 before Ethernet10. Part of #2876 and #2877. Assisted-by: Claude:claude-fable-5-2 Signed-off-by: Christian Berendt --- osism/utils/hardware/__init__.py | 32 +++++++++++++++++++++ tests/unit/utils/hardware/__init__.py | 1 + tests/unit/utils/hardware/test_init.py | 40 ++++++++++++++++++++++++++ 3 files changed, 73 insertions(+) create mode 100644 osism/utils/hardware/__init__.py create mode 100644 tests/unit/utils/hardware/__init__.py create mode 100644 tests/unit/utils/hardware/test_init.py diff --git a/osism/utils/hardware/__init__.py b/osism/utils/hardware/__init__.py new file mode 100644 index 000000000..5984ee113 --- /dev/null +++ b/osism/utils/hardware/__init__.py @@ -0,0 +1,32 @@ +# SPDX-License-Identifier: Apache-2.0 + +"""Helpers shared by the live hardware reports (``osism report ...``).""" + +import re +from typing import Any, List, Optional + +_MAC_DIGITS = re.compile(r"^[0-9a-f]{12}$") + + +def normalize_mac(value: Any) -> Optional[str]: + """Return a MAC address as lower-case, colon-separated string. + + Accepts the usual spellings (``AA:BB:..``, ``aa-bb-..``, ``aabb.ccdd..``). + Returns ``None`` for anything that is not a MAC address and for the + all-zero address, which drivers report when they have no permanent one. + """ + if not value: + return None + digits = re.sub(r"[:.\-]", "", str(value).strip().lower()) + if not _MAC_DIGITS.match(digits) or digits == "0" * 12: + return None + return ":".join(digits[i : i + 2] for i in range(0, 12, 2)) + + +def natural_key(text: Any) -> List[Any]: + """Sort key ordering embedded numbers by value (Ethernet2 < Ethernet10).""" + return [ + (0, int(part), "") if part.isdigit() else (1, 0, part) + for part in re.split(r"(\d+)", str(text or "")) + if part != "" + ] diff --git a/tests/unit/utils/hardware/__init__.py b/tests/unit/utils/hardware/__init__.py new file mode 100644 index 000000000..988131360 --- /dev/null +++ b/tests/unit/utils/hardware/__init__.py @@ -0,0 +1 @@ +# SPDX-License-Identifier: Apache-2.0 diff --git a/tests/unit/utils/hardware/test_init.py b/tests/unit/utils/hardware/test_init.py new file mode 100644 index 000000000..e6284e075 --- /dev/null +++ b/tests/unit/utils/hardware/test_init.py @@ -0,0 +1,40 @@ +# SPDX-License-Identifier: Apache-2.0 + +"""Unit tests for the small helpers in ``osism.utils.hardware``.""" + +import pytest + +from osism.utils.hardware import natural_key, normalize_mac + + +@pytest.mark.parametrize( + "value", + ["AA:BB:CC:00:11:22", "aa-bb-cc-00-11-22", "aabb.cc00.1122", " aabbcc001122 "], +) +def test_normalize_mac_accepts_common_spellings(value): + assert normalize_mac(value) == "aa:bb:cc:00:11:22" + + +@pytest.mark.parametrize( + "value", + [None, "", "Ethernet0", "aa:bb:cc", "00:00:00:00:00:00", "zz:bb:cc:00:11:22"], +) +def test_normalize_mac_rejects_non_addresses(value): + assert normalize_mac(value) is None + + +def test_natural_key_orders_numbers_by_value(): + names = ["Ethernet10", "Ethernet2", "Ethernet1", "eth0", "ens1f1", "ens1f0"] + + assert sorted(names, key=natural_key) == [ + "Ethernet1", + "Ethernet2", + "Ethernet10", + "ens1f0", + "ens1f1", + "eth0", + ] + + +def test_natural_key_handles_none_and_mixed_values(): + assert sorted([None, "b", "1", "a"], key=natural_key) == [None, "1", "a", "b"] From d9b5a1963c6eafc1330146a557fe9700b74deee2 Mon Sep 17 00:00:00 2001 From: Christian Berendt Date: Wed, 11 Nov 2026 22:13:09 +0100 Subject: [PATCH 4/8] Add device selection for the hardware reports The existing report commands take their hosts from the Ansible inventory, which knows the nodes but not the SONiC switches, and can only be narrowed with an Ansible pattern. select_targets() queries NetBox with the filters given on the command line (site, role, rack, tag and so on, plus a generic KEY=VALUE filter), classifies every device as node or switch, and applies shell-style name patterns. A switch has to match NETBOX_FILTER_CONDUCTOR_SONIC, because those are the switches the operator key is deployed to; others are skipped with a warning unless --include-unmanaged is given. A node is a device whose hostname is in the inventory. Without NetBox the inventory is the only source and NetBox filters are rejected. resolve_addresses() then sets the SSH address and user. A switch is reached at the management IP of its config context, its OOB IP or its primary IP, so it does not need a synced config context. Targets carry the NetBox device as plain data. The collection can then run in threads without sharing the NetBox session. Tests: query building, selection with and without NetBox, the handling of unmanaged switches, address resolution, and the inventory and NetBox failure paths. Part of #2876 and #2877. Assisted-by: Claude:claude-fable-5-2 Signed-off-by: Christian Berendt --- osism/utils/hardware/selection.py | 456 +++++++++++++++ tests/unit/utils/hardware/test_selection.py | 614 ++++++++++++++++++++ 2 files changed, 1070 insertions(+) create mode 100644 osism/utils/hardware/selection.py create mode 100644 tests/unit/utils/hardware/test_selection.py diff --git a/osism/utils/hardware/selection.py b/osism/utils/hardware/selection.py new file mode 100644 index 000000000..3dde18775 --- /dev/null +++ b/osism/utils/hardware/selection.py @@ -0,0 +1,456 @@ +# SPDX-License-Identifier: Apache-2.0 + +"""Device selection for the live hardware reports. + +The reports log in to Linux nodes and SONiC switches. Which devices they +log in to is decided here: NetBox is queried with the filters given on the +command line, every device found is classified as node or switch, and name +patterns narrow the result. Without NetBox the nodes of the Ansible +inventory are the only possible targets. + +The result is a list of ``Target`` objects holding plain data, so that the +collection can run in worker threads without touching the NetBox session. +""" + +from dataclasses import dataclass, field +import fnmatch +import json +import subprocess +from types import SimpleNamespace +from typing import Any, Callable, Dict, Iterable, List, Optional, Set + +from loguru import logger + +from osism import settings +from osism.utils.inventory import get_hosts_from_inventory, get_inventory_path + +KIND_NODE = "node" +KIND_SWITCH = "switch" + +INVENTORY_PATH = "/ansible/inventory/hosts.yml" +DEFAULT_SWITCH_USER = "admin" +DEFAULT_STATUS = "active" + +# Options that take slugs and map 1:1 to a NetBox device filter. +SLUG_FILTERS = [ + ("--site", "site", "site"), + ("--role", "role", "device role"), + ("--region", "region", "region"), + ("--site-group", "site_group", "site group"), + ("--tenant", "tenant", "tenant"), + ("--tag", "tag", "tag"), + ("--manufacturer", "manufacturer", "manufacturer"), + ("--device-type", "device_type", "device type"), + ("--platform", "platform", "platform"), +] + +# Options that take names. NetBox only filters devices by rack_id and +# location_id, so the names are resolved to IDs first. +NAME_FILTERS = [ + ("--rack", "rack", "racks"), + ("--location", "location", "locations"), +] + + +class SelectionError(Exception): + """The selection could not be evaluated or matched no device.""" + + +@dataclass +class Target: + """One device a report logs in to. + + Attributes: + name: NetBox device name, or the inventory hostname without NetBox + hostname: Inventory hostname (custom field ``inventory_hostname``, + else the device name) + kind: ``node`` or ``switch`` + address: Address used for SSH, set by ``resolve_addresses`` + user: SSH user, set by ``resolve_addresses`` + device_id: NetBox device ID, ``None`` without NetBox + device: NetBox device as plain data, empty without NetBox + """ + + name: str + hostname: str + kind: str + address: str = "" + user: str = "" + device_id: Optional[int] = None + device: Dict[str, Any] = field(default_factory=dict, repr=False) + + @property + def hwsku(self) -> Optional[str]: + custom_fields = self.device.get("custom_fields") or {} + return (custom_fields.get("sonic_parameters") or {}).get("hwsku") + + def describe(self) -> Dict[str, Any]: + """Return the device attributes shown in a report.""" + device = self.device + device_type = device.get("device_type") or {} + return { + "name": self.name, + "inventory_hostname": self.hostname, + "kind": self.kind, + "address": self.address, + "site": _nested(device, "site", "slug"), + "rack": _nested(device, "rack", "name"), + "role": _role_slug(device), + "device_type": device_type.get("model"), + "manufacturer": _nested(device_type, "manufacturer", "name"), + "serial": device.get("serial") or None, + } + + +def add_selection_arguments(parser) -> None: + """Add the device selection options shared by the hardware reports.""" + parser.add_argument( + "names", + nargs="*", + metavar="NAME", + help="Shell-style pattern matched against the device name and the inventory hostname", + ) + parser.add_argument( + "--type", + choices=["all", KIND_NODE, KIND_SWITCH], + default="all", + help="Restrict the selection to nodes or switches (default: all)", + ) + parser.add_argument( + "--include-unmanaged", + action="store_true", + help="Also query SONiC switches that do not match NETBOX_FILTER_CONDUCTOR_SONIC", + ) + parser.add_argument( + "-l", + "--limit", + type=str, + help="Limit selected hosts to an additional pattern (Ansible pattern, selects nodes only)", + ) + for flag, dest, label in SLUG_FILTERS: + parser.add_argument( + flag, + dest=dest, + action="append", + metavar="SLUG", + help=f"Select devices by {label} slug (can be used multiple times)", + ) + for flag, dest, _ in NAME_FILTERS: + parser.add_argument( + flag, + dest=dest, + action="append", + metavar="NAME", + help=f"Select devices by {dest} name (can be used multiple times)", + ) + parser.add_argument( + "--status", + action="append", + metavar="STATUS", + help=f"Select devices by status (default: {DEFAULT_STATUS}, can be used multiple times)", + ) + parser.add_argument( + "--filter", + dest="netbox_filter", + action="append", + metavar="KEY=VALUE", + help="Pass any other NetBox device filter through unchanged (can be used multiple times)", + ) + + +def uses_netbox_filters(parsed_args) -> bool: + """Return True when an option was given that only NetBox can evaluate.""" + dests = [dest for _, dest, _ in SLUG_FILTERS + NAME_FILTERS] + dests.extend(["status", "netbox_filter"]) + return any(getattr(parsed_args, dest, None) for dest in dests) + + +def build_netbox_query(parsed_args, nb) -> Dict[str, List[Any]]: + """Translate the filter options into a NetBox device query. + + Values of one option are passed as a list, which NetBox combines the way + it combines a repeated query parameter. Different options are ANDed. + + Raises: + SelectionError: A rack or location name is unknown, or a ``--filter`` + value is not of the form ``KEY=VALUE`` + """ + query: Dict[str, List[Any]] = {} + + for _, dest, _ in SLUG_FILTERS: + values = getattr(parsed_args, dest, None) + if values: + query[dest] = list(values) + + for _, dest, endpoint_name in NAME_FILTERS: + names = getattr(parsed_args, dest, None) + if not names: + continue + endpoint = getattr(nb.dcim, endpoint_name) + ids: List[Any] = [] + for name in names: + try: + found = [record.id for record in endpoint.filter(name=name)] + except Exception as exc: + raise SelectionError(f"NetBox {dest} query failed: {exc}") + if not found: + raise SelectionError(f"No {dest} named {name} found in NetBox") + ids.extend(found) + query[f"{dest}_id"] = ids + + if getattr(parsed_args, "status", None): + query["status"] = list(parsed_args.status) + + for item in getattr(parsed_args, "netbox_filter", None) or []: + key, separator, value = item.partition("=") + if not separator or not key: + raise SelectionError(f"Invalid filter {item!r}, expected KEY=VALUE") + query.setdefault(key, []).append(value) + + query.setdefault("status", [DEFAULT_STATUS]) + return query + + +def load_inventory_hosts(limit: Optional[str] = None) -> List[str]: + """Return the hosts of the Ansible inventory, optionally limited. + + Raises: + SelectionError: The inventory could not be loaded + """ + command = [ + "ansible-inventory", + "-i", + get_inventory_path(INVENTORY_PATH), + "--list", + ] + if limit: + command.extend(["--limit", limit]) + + try: + result = subprocess.run(command, capture_output=True, text=True, timeout=30) + except subprocess.TimeoutExpired: + raise SelectionError("Timeout loading inventory.") + + if result.returncode != 0: + raise SelectionError("Error loading inventory.") + + try: + data = json.loads(result.stdout) + except ValueError: + raise SelectionError("Could not parse inventory.") + + return get_hosts_from_inventory(data) + + +def managed_switch_ids(nb) -> Set[int]: + """Return the IDs of the devices matching NETBOX_FILTER_CONDUCTOR_SONIC. + + Those are the switches ``sonic sync`` operates on, and therefore the ones + the operator key is deployed to. + """ + # Imported here: the conductor package is expensive to import and only + # needed when switches are part of the selection. + from osism.tasks.conductor.netbox import get_nb_device_query_list_sonic + + ids: Set[int] = set() + for query in get_nb_device_query_list_sonic(): + for device in nb.dcim.devices.filter(**query): + ids.add(device.id) + return ids + + +def select_targets(parsed_args, nb) -> List[Target]: + """Evaluate the selection options and return the devices to query. + + Args: + parsed_args: Namespace holding the options of + ``add_selection_arguments`` + nb: pynetbox API object, or ``None`` when NetBox is not configured + + Returns: + Targets sorted by name, without addresses + + Raises: + SelectionError: The selection could not be evaluated, or it matched + no device + """ + if nb is None: + targets = _select_from_inventory(parsed_args) + else: + targets = _select_from_netbox(parsed_args, nb) + + if not targets: + raise SelectionError("No devices matched the selection.") + + return sorted(targets, key=lambda target: target.name) + + +def resolve_addresses( + targets: Iterable[Target], resolve_node: Callable[[str], str] +) -> None: + """Set the SSH address and user of every target. + + Nodes are reached as the operator user at the address ``resolve_node`` + returns for the inventory hostname. Switches are reached at the + management IP of their config context, else the OOB IP, else the primary + IP. A switch without any of these keeps an empty address. + + This talks to NetBox for switches without an OOB IP and has to run in + the main thread, before the collection starts. + """ + for target in targets: + if target.kind == KIND_SWITCH: + target.address, target.user = _switch_access(target.device) + else: + target.address = resolve_node(target.hostname) + target.user = settings.OPERATOR_USER + + +def _select_from_inventory(parsed_args) -> List[Target]: + if uses_netbox_filters(parsed_args): + raise SelectionError( + "NetBox is not configured, NetBox filters cannot be used. " + "Select nodes by NAME or --limit." + ) + if parsed_args.type == KIND_SWITCH: + raise SelectionError( + "NetBox is not configured, SONiC switches cannot be discovered." + ) + + hosts = load_inventory_hosts(parsed_args.limit) + return [ + Target(name=host, hostname=host, kind=KIND_NODE) + for host in hosts + if _matches(parsed_args.names, host) + ] + + +def _select_from_netbox(parsed_args, nb) -> List[Target]: + from osism.tasks.conductor.sonic.constants import DEFAULT_SONIC_ROLES + + want_nodes = parsed_args.type in ("all", KIND_NODE) + # --limit is an Ansible pattern and can only ever select inventory hosts + want_switches = parsed_args.type in ("all", KIND_SWITCH) and not parsed_args.limit + + inventory_hosts: Set[str] = set() + if want_nodes: + inventory_hosts = set(load_inventory_hosts(parsed_args.limit)) + + query = build_netbox_query(parsed_args, nb) + try: + devices = [dict(record) for record in nb.dcim.devices.filter(**query)] + except Exception as exc: + raise SelectionError(f"NetBox device query failed: {exc}") + + managed: Optional[Set[int]] = None + targets: List[Target] = [] + unmanaged: List[str] = [] + selected_hosts: Set[str] = set() + + for device in devices: + name = device.get("name") or "" + hostname = _hostname(device) + if not name or not _matches(parsed_args.names, name, hostname): + continue + + if _role_slug(device) in DEFAULT_SONIC_ROLES: + if not want_switches: + continue + if not parsed_args.include_unmanaged: + if managed is None: + try: + managed = managed_switch_ids(nb) + except Exception as exc: + raise SelectionError( + f"Could not evaluate NETBOX_FILTER_CONDUCTOR_SONIC: {exc}" + ) + if device.get("id") not in managed: + unmanaged.append(name) + continue + targets.append(_target(device, KIND_SWITCH)) + elif want_nodes and hostname in inventory_hosts: + targets.append(_target(device, KIND_NODE)) + selected_hosts.add(hostname) + else: + logger.debug(f"Skipping {name}: neither a managed node nor a SONiC switch") + + if unmanaged: + logger.warning( + f"Skipping {len(unmanaged)} SONiC switch(es) not matching " + f"NETBOX_FILTER_CONDUCTOR_SONIC: {', '.join(sorted(unmanaged))}. " + "Use --include-unmanaged to query them." + ) + + # Without any narrowing option every inventory host is expected to show + # up. A host that does not is missing in NetBox or not active there. + if want_nodes and not parsed_args.names and not uses_netbox_filters(parsed_args): + unknown = sorted(inventory_hosts - selected_hosts) + if unknown: + logger.warning( + f"Skipping {len(unknown)} inventory host(s) without an active " + f"NetBox device: {', '.join(unknown)}" + ) + + return targets + + +def _target(device: Dict[str, Any], kind: str) -> Target: + return Target( + name=device["name"], + hostname=_hostname(device), + kind=kind, + device_id=device.get("id"), + device=device, + ) + + +def _matches(patterns: Optional[List[str]], *names: str) -> bool: + if not patterns: + return True + return any( + fnmatch.fnmatchcase(name, pattern) for pattern in patterns for name in names + ) + + +def _hostname(device: Dict[str, Any]) -> str: + custom_fields = device.get("custom_fields") or {} + return custom_fields.get("inventory_hostname") or device.get("name") or "" + + +def _role_slug(device: Dict[str, Any]) -> Optional[str]: + # NetBox before 3.6 calls the field device_role + role = device.get("role") or device.get("device_role") or {} + return role.get("slug") + + +def _nested(data: Dict[str, Any], key: str, attribute: str) -> Any: + return (data.get(key) or {}).get(attribute) + + +def _address(value: Any) -> str: + """Return the bare address of a NetBox IP (``10.0.0.1/24`` -> ``10.0.0.1``).""" + if isinstance(value, dict): + value = value.get("address") + return str(value).split("/")[0] if value else "" + + +def _switch_access(device: Dict[str, Any]) -> tuple: + management = (device.get("local_context_data") or {}).get("management") or {} + user = management.get("username") or DEFAULT_SWITCH_USER + + address = _address(management.get("ip")) or _address(device.get("oob_ip")) + if not address: + # Falls back to the IP of a management interface, which costs NetBox + # queries and is therefore only tried when there is no OOB IP field. + from osism.tasks.conductor.netbox import get_device_oob_ip + + oob = get_device_oob_ip( + SimpleNamespace(id=device.get("id"), name=device.get("name"), oob_ip=None) + ) + if oob: + address = oob[0] + if not address: + address = _address(device.get("primary_ip4")) or _address( + device.get("primary_ip6") + ) + return address, user diff --git a/tests/unit/utils/hardware/test_selection.py b/tests/unit/utils/hardware/test_selection.py new file mode 100644 index 000000000..7004d5be4 --- /dev/null +++ b/tests/unit/utils/hardware/test_selection.py @@ -0,0 +1,614 @@ +# SPDX-License-Identifier: Apache-2.0 + +"""Unit tests for ``osism.utils.hardware.selection``. + +NetBox is replaced by a ``MagicMock`` whose ``dcim.devices.filter`` returns +plain dicts. That is the shape ``dict(record)`` produces for a pynetbox +record, which is all the selection ever reads. +""" + +import argparse +import json +import subprocess +from types import SimpleNamespace +from unittest.mock import MagicMock, patch + +import pytest + +from osism.utils.hardware import selection +from osism.utils.hardware.selection import ( + KIND_NODE, + KIND_SWITCH, + SelectionError, + Target, + add_selection_arguments, + build_netbox_query, + load_inventory_hosts, + managed_switch_ids, + resolve_addresses, + select_targets, +) + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _args(*argv): + parser = argparse.ArgumentParser() + add_selection_arguments(parser) + return parser.parse_args(list(argv)) + + +def _device(device_id, name, role, **extra): + device = { + "id": device_id, + "name": name, + "role": {"slug": role}, + "custom_fields": {}, + } + device.update(extra) + return device + + +LEAF_A = _device(1, "leaf-1a", "leaf") +LEAF_B = _device(2, "leaf-1b", "leaf") +SPINE = _device(3, "spine-1", "spine") +COMPUTE = _device(10, "compute-041", "compute") +STORAGE = _device( + 11, "sto-1", "storage", custom_fields={"inventory_hostname": "storage-001"} +) +PDU = _device(20, "pdu-1", "pdu") + +ALL_DEVICES = [LEAF_A, LEAF_B, SPINE, COMPUTE, STORAGE, PDU] +INVENTORY = ["compute-041", "storage-001", "manager-1"] + + +def _nb(devices=None): + nb = MagicMock() + nb.dcim.devices.filter.return_value = list( + ALL_DEVICES if devices is None else devices + ) + return nb + + +@pytest.fixture +def inventory(): + with patch.object( + selection, "load_inventory_hosts", return_value=list(INVENTORY) + ) as load: + yield load + + +@pytest.fixture +def managed(): + """Treat leaf-1a and spine-1 as matching NETBOX_FILTER_CONDUCTOR_SONIC.""" + with patch.object(selection, "managed_switch_ids", return_value={1, 3}) as ids: + yield ids + + +def _has_log(records, level, substring): + return any(r["level"] == level and substring in r["message"] for r in records) + + +def _names(targets): + return [(target.name, target.kind) for target in targets] + + +# --------------------------------------------------------------------------- +# Arguments and NetBox query +# --------------------------------------------------------------------------- + + +def test_arguments_are_repeatable_and_default_to_everything(): + args = _args() + assert args.names == [] + assert args.type == "all" + assert args.include_unmanaged is False + assert args.limit is None + + args = _args( + "leaf-*", + "spine-*", + "--site", + "fra1", + "--site", + "fra2", + "--role", + "leaf", + "--rack", + "R12", + "--location", + "Hall 1", + "--region", + "eu", + "--site-group", + "prod", + "--tenant", + "t1", + "--tag", + "x", + "--manufacturer", + "dell", + "--device-type", + "s5248f-on", + "--platform", + "sonic", + "--status", + "planned", + "--filter", + "cf_maintenance=false", + "--type", + "switch", + "--include-unmanaged", + "-l", + "compute", + ) + assert args.names == ["leaf-*", "spine-*"] + assert args.site == ["fra1", "fra2"] + assert args.site_group == ["prod"] + assert args.device_type == ["s5248f-on"] + assert args.netbox_filter == ["cf_maintenance=false"] + assert args.limit == "compute" + + +def test_query_defaults_to_active_devices(): + assert build_netbox_query(_args(), MagicMock()) == {"status": ["active"]} + + +def test_query_combines_repeated_and_different_options(): + query = build_netbox_query( + _args("--site", "fra1", "--site", "fra2", "--role", "leaf", "--tag", "x"), + MagicMock(), + ) + + assert query == { + "site": ["fra1", "fra2"], + "role": ["leaf"], + "tag": ["x"], + "status": ["active"], + } + + +def test_query_status_option_replaces_the_default(): + query = build_netbox_query( + _args("--status", "planned", "--status", "staged"), MagicMock() + ) + + assert query["status"] == ["planned", "staged"] + + +def test_query_resolves_rack_and_location_names_to_ids(): + nb = MagicMock() + nb.dcim.racks.filter.side_effect = lambda name: { + "R12": [SimpleNamespace(id=7), SimpleNamespace(id=8)], + "R13": [SimpleNamespace(id=9)], + }[name] + nb.dcim.locations.filter.return_value = [SimpleNamespace(id=3)] + + query = build_netbox_query( + _args("--rack", "R12", "--rack", "R13", "--location", "Hall 1"), nb + ) + + assert query["rack_id"] == [7, 8, 9] + assert query["location_id"] == [3] + assert "rack" not in query and "location" not in query + nb.dcim.locations.filter.assert_called_once_with(name="Hall 1") + + +def test_query_rejects_unknown_rack_name(): + nb = MagicMock() + nb.dcim.racks.filter.return_value = [] + + with pytest.raises(SelectionError, match="No rack named R99"): + build_netbox_query(_args("--rack", "R99"), nb) + + +def test_query_reports_a_failing_rack_lookup(): + nb = MagicMock() + nb.dcim.racks.filter.side_effect = RuntimeError("connection refused") + + with pytest.raises(SelectionError, match="NetBox rack query failed: connection"): + build_netbox_query(_args("--rack", "R12"), nb) + + +def test_query_passes_generic_filters_through(): + query = build_netbox_query( + _args( + "--filter", + "cf_maintenance=false", + "--filter", + "has_primary_ip=true", + "--site", + "fra1", + "--filter", + "site=fra2", + "--filter", + "status=planned", + ), + MagicMock(), + ) + + assert query["cf_maintenance"] == ["false"] + assert query["has_primary_ip"] == ["true"] + assert query["site"] == ["fra1", "fra2"] + assert query["status"] == ["planned"] + + +@pytest.mark.parametrize("value", ["novalue", "=x"]) +def test_query_rejects_malformed_generic_filter(value): + with pytest.raises(SelectionError, match="expected KEY=VALUE"): + build_netbox_query(_args("--filter", value), MagicMock()) + + +# --------------------------------------------------------------------------- +# Selection without NetBox +# --------------------------------------------------------------------------- + + +def test_without_netbox_nodes_come_from_the_inventory(inventory): + targets = select_targets(_args(), None) + + assert _names(targets) == [ + ("compute-041", KIND_NODE), + ("manager-1", KIND_NODE), + ("storage-001", KIND_NODE), + ] + assert all(target.device_id is None for target in targets) + inventory.assert_called_once_with(None) + + +def test_without_netbox_names_and_limit_narrow_the_nodes(inventory): + targets = select_targets(_args("compute-*", "-l", "compute"), None) + + assert _names(targets) == [("compute-041", KIND_NODE)] + inventory.assert_called_once_with("compute") + + +@pytest.mark.parametrize( + "argv", + [ + ["--site", "fra1"], + ["--rack", "R12"], + ["--status", "active"], + ["--filter", "cf_x=1"], + ], +) +def test_without_netbox_filters_are_rejected(inventory, argv): + with pytest.raises(SelectionError, match="NetBox is not configured"): + select_targets(_args(*argv), None) + + inventory.assert_not_called() + + +def test_without_netbox_switches_cannot_be_selected(inventory): + with pytest.raises(SelectionError, match="SONiC switches cannot be discovered"): + select_targets(_args("--type", "switch"), None) + + +# --------------------------------------------------------------------------- +# Selection with NetBox +# --------------------------------------------------------------------------- + + +def test_classifies_nodes_and_switches_and_skips_the_rest(inventory, managed): + nb = _nb() + + targets = select_targets(_args("--include-unmanaged"), nb) + + assert _names(targets) == [ + ("compute-041", KIND_NODE), + ("leaf-1a", KIND_SWITCH), + ("leaf-1b", KIND_SWITCH), + ("spine-1", KIND_SWITCH), + ("sto-1", KIND_NODE), + ] + nb.dcim.devices.filter.assert_called_once_with(status=["active"]) + storage = targets[-1] + assert storage.hostname == "storage-001" + assert storage.device_id == 11 + assert storage.device == STORAGE + + +def test_filters_are_passed_to_netbox(inventory, managed): + nb = _nb([LEAF_A]) + + select_targets(_args("--site", "fra1", "--role", "leaf"), nb) + + nb.dcim.devices.filter.assert_called_once_with( + site=["fra1"], role=["leaf"], status=["active"] + ) + + +def test_role_is_read_from_device_role_on_old_netbox(inventory, managed): + old = { + "id": 1, + "name": "leaf-1a", + "device_role": {"slug": "leaf"}, + "custom_fields": {}, + } + + targets = select_targets(_args(), _nb([old])) + + assert _names(targets) == [("leaf-1a", KIND_SWITCH)] + + +def test_name_patterns_match_device_name_or_inventory_hostname(inventory, managed): + targets = select_targets(_args("storage-*", "leaf-1a"), _nb()) + + assert _names(targets) == [("leaf-1a", KIND_SWITCH), ("sto-1", KIND_NODE)] + + +def test_type_restricts_to_nodes(inventory, managed): + targets = select_targets(_args("--type", "node"), _nb()) + + assert _names(targets) == [("compute-041", KIND_NODE), ("sto-1", KIND_NODE)] + managed.assert_not_called() + + +def test_type_switch_does_not_need_the_inventory(inventory, managed): + targets = select_targets(_args("--type", "switch"), _nb()) + + assert _names(targets) == [("leaf-1a", KIND_SWITCH), ("spine-1", KIND_SWITCH)] + inventory.assert_not_called() + + +def test_limit_selects_nodes_only(managed): + with patch.object( + selection, "load_inventory_hosts", return_value=["compute-041"] + ) as load: + targets = select_targets(_args("-l", "compute"), _nb()) + + assert _names(targets) == [("compute-041", KIND_NODE)] + load.assert_called_once_with("compute") + managed.assert_not_called() + + +def test_unmanaged_switch_is_skipped_with_a_warning(inventory, managed, loguru_logs): + targets = select_targets(_args("--type", "switch"), _nb()) + + assert ("leaf-1b", KIND_SWITCH) not in _names(targets) + assert _has_log(loguru_logs, "WARNING", "Skipping 1 SONiC switch(es)") + assert _has_log(loguru_logs, "WARNING", "leaf-1b") + assert _has_log(loguru_logs, "WARNING", "--include-unmanaged") + + +def test_unmanaged_switch_is_skipped_even_when_named(inventory, managed, loguru_logs): + with pytest.raises(SelectionError, match="No devices matched"): + select_targets(_args("leaf-1b"), _nb()) + + assert _has_log(loguru_logs, "WARNING", "leaf-1b") + + +def test_include_unmanaged_queries_every_sonic_role_switch(inventory, managed): + targets = select_targets(_args("leaf-1b", "--include-unmanaged"), _nb()) + + assert _names(targets) == [("leaf-1b", KIND_SWITCH)] + managed.assert_not_called() + + +def test_managed_filter_failure_is_a_selection_error(inventory): + with patch.object( + selection, "managed_switch_ids", side_effect=ValueError("bad setting") + ): + with pytest.raises(SelectionError, match="NETBOX_FILTER_CONDUCTOR_SONIC"): + select_targets(_args(), _nb()) + + +def test_no_match_is_a_selection_error(inventory, managed): + with pytest.raises(SelectionError, match="No devices matched"): + select_targets(_args("does-not-exist"), _nb()) + + +def test_netbox_query_failure_is_a_selection_error(inventory, managed): + nb = MagicMock() + nb.dcim.devices.filter.side_effect = RuntimeError("400 Bad Request") + + with pytest.raises(SelectionError, match="NetBox device query failed: 400"): + select_targets(_args("--role", "nope"), nb) + + +def test_inventory_hosts_missing_in_netbox_are_reported( + inventory, managed, loguru_logs +): + select_targets(_args(), _nb()) + + assert _has_log( + loguru_logs, + "WARNING", + "Skipping 1 inventory host(s) without an active NetBox device: manager-1", + ) + + +def test_missing_hosts_are_not_reported_for_a_narrowed_selection( + inventory, managed, loguru_logs +): + select_targets(_args("compute-*"), _nb()) + select_targets(_args("--site", "fra1"), _nb()) + + assert not _has_log(loguru_logs, "WARNING", "inventory host(s)") + + +# --------------------------------------------------------------------------- +# Addresses +# --------------------------------------------------------------------------- + + +def _switch(**device): + data = {"id": 1, "name": "leaf-1a"} + data.update(device) + return Target( + name="leaf-1a", hostname="leaf-1a", kind=KIND_SWITCH, device_id=1, device=data + ) + + +def test_node_address_comes_from_the_resolver(): + node = Target(name="sto-1", hostname="storage-001", kind=KIND_NODE) + resolver = MagicMock(return_value="192.0.2.10") + + resolve_addresses([node], resolver) + + resolver.assert_called_once_with("storage-001") + assert node.address == "192.0.2.10" + assert node.user == selection.settings.OPERATOR_USER + + +def test_switch_prefers_the_management_ip_of_the_config_context(): + switch = _switch( + local_context_data={"management": {"ip": "10.0.0.1", "username": "ops"}}, + oob_ip={"address": "10.0.0.2/24"}, + ) + + resolve_addresses([switch], MagicMock()) + + assert (switch.address, switch.user) == ("10.0.0.1", "ops") + + +def test_switch_falls_back_to_the_oob_ip_and_the_admin_user(): + switch = _switch( + local_context_data=None, + oob_ip={"address": "10.0.0.2/24"}, + primary_ip4={"address": "10.0.0.3/24"}, + ) + + resolve_addresses([switch], MagicMock()) + + assert (switch.address, switch.user) == ("10.0.0.2", "admin") + + +def test_switch_uses_the_management_interface_ip_before_the_primary_ip(): + switch = _switch(primary_ip4={"address": "10.0.0.3/24"}) + + with patch( + "osism.tasks.conductor.netbox.get_device_oob_ip", + return_value=("10.0.0.4", 24), + ) as oob: + resolve_addresses([switch], MagicMock()) + + assert switch.address == "10.0.0.4" + assert oob.call_args.args[0].id == 1 + + +def test_switch_falls_back_to_the_primary_ip(): + switch = _switch(primary_ip4={"address": "10.0.0.3/24"}) + + with patch("osism.tasks.conductor.netbox.get_device_oob_ip", return_value=None): + resolve_addresses([switch], MagicMock()) + + assert switch.address == "10.0.0.3" + + +def test_switch_without_any_address_keeps_it_empty(): + switch = _switch() + + with patch("osism.tasks.conductor.netbox.get_device_oob_ip", return_value=None): + resolve_addresses([switch], MagicMock()) + + assert switch.address == "" + + +# --------------------------------------------------------------------------- +# Inventory, managed switches, target attributes +# --------------------------------------------------------------------------- + + +def _proc(returncode=0, stdout=""): + return MagicMock(returncode=returncode, stdout=stdout, stderr="") + + +def test_load_inventory_hosts_passes_the_limit(): + data = {"_meta": {"hostvars": {}}, "compute": {"hosts": ["c1", "c2"]}} + + with patch.object( + selection, "get_inventory_path", return_value="/inv/hosts.yml" + ), patch.object( + selection.subprocess, "run", return_value=_proc(stdout=json.dumps(data)) + ) as run: + hosts = load_inventory_hosts("compute") + + assert hosts == ["c1", "c2"] + assert run.call_args.args[0] == [ + "ansible-inventory", + "-i", + "/inv/hosts.yml", + "--list", + "--limit", + "compute", + ] + + +@pytest.mark.parametrize( + "result,message", + [ + (_proc(returncode=1), "Error loading inventory."), + (_proc(stdout="not json"), "Could not parse inventory."), + ( + subprocess.TimeoutExpired(cmd="ansible-inventory", timeout=30), + "Timeout loading inventory.", + ), + ], +) +def test_load_inventory_hosts_failures(result, message): + with patch.object( + selection, "get_inventory_path", return_value="/inv/hosts.yml" + ), patch.object(selection.subprocess, "run", side_effect=[result]): + with pytest.raises(SelectionError, match=message): + load_inventory_hosts() + + +def test_managed_switch_ids_unions_all_configured_queries(): + nb = MagicMock() + nb.dcim.devices.filter.side_effect = [ + [SimpleNamespace(id=1), SimpleNamespace(id=2)], + [SimpleNamespace(id=2), SimpleNamespace(id=5)], + ] + queries = [{"status": "active", "tag": ["a"]}, {"site": "fra1"}] + + with patch( + "osism.tasks.conductor.netbox.get_nb_device_query_list_sonic", + return_value=queries, + ): + ids = managed_switch_ids(nb) + + assert ids == {1, 2, 5} + assert [call.kwargs for call in nb.dcim.devices.filter.call_args_list] == queries + + +def test_target_describes_the_netbox_device(): + target = Target( + name="leaf-1a", + hostname="leaf-1a", + kind=KIND_SWITCH, + address="10.0.0.1", + device_id=1, + device={ + "site": {"slug": "fra1"}, + "rack": {"name": "R12"}, + "role": {"slug": "leaf"}, + "device_type": {"model": "S5248F-ON", "manufacturer": {"name": "Dell"}}, + "serial": "ABC123", + "custom_fields": {"sonic_parameters": {"hwsku": "DellEMC-S5248f-P-25G"}}, + }, + ) + + assert target.hwsku == "DellEMC-S5248f-P-25G" + assert target.describe() == { + "name": "leaf-1a", + "inventory_hostname": "leaf-1a", + "kind": "switch", + "address": "10.0.0.1", + "site": "fra1", + "rack": "R12", + "role": "leaf", + "device_type": "S5248F-ON", + "manufacturer": "Dell", + "serial": "ABC123", + } + + +def test_target_without_netbox_data_describes_itself(): + target = Target(name="compute-041", hostname="compute-041", kind=KIND_NODE) + + assert target.hwsku is None + assert target.describe()["site"] is None + assert target.describe()["serial"] is None From e137f6940849f672026d72d5c3b70ab991802380 Mon Sep 17 00:00:00 2001 From: Christian Berendt Date: Wed, 11 Nov 2026 22:13:09 +0100 Subject: [PATCH 5/8] Add parallel SSH collection with Linux and SONiC collectors The hardware reports need many values per device from many devices. One ssh call per value, device after device, as the existing report commands do it, does not scale to that. collect() opens one session per device from a thread pool. The session pipes a stdlib-only collector to python3 on the device, and the collector prints one JSON document of raw command output, sysfs values or Redis dumps. A device that times out, refuses the login or returns garbage is recorded as failed and does not hold up the rest. dump_raw() writes the documents to files, which is how parser fixtures are captured from real devices. The collectors gather and do not parse. This first version covers what both reports need: on Linux the physical ports from sysfs, link state, the permanent MAC and the LLDP neighbours; on SONiC the PORT tables of CONFIG_DB and APPL_DB, the device metadata and the management port. sonic-db-dump is tried as the login user and again with sudo, because the access to Redis differs between releases and was not verified on a switch. Tests: failure handling, parallelism and its limit, the collectors against a fake sysfs tree and a fake command runner. Part of #2876 and #2877. Assisted-by: Claude:claude-fable-5-2 Signed-off-by: Christian Berendt --- osism/utils/hardware/collect.py | 196 +++++++++++++++ osism/utils/hardware/remote_linux.py | 190 +++++++++++++++ osism/utils/hardware/remote_sonic.py | 163 +++++++++++++ tests/unit/utils/hardware/test_collect.py | 225 ++++++++++++++++++ .../unit/utils/hardware/test_remote_linux.py | 173 ++++++++++++++ .../unit/utils/hardware/test_remote_sonic.py | 139 +++++++++++ 6 files changed, 1086 insertions(+) create mode 100644 osism/utils/hardware/collect.py create mode 100644 osism/utils/hardware/remote_linux.py create mode 100644 osism/utils/hardware/remote_sonic.py create mode 100644 tests/unit/utils/hardware/test_collect.py create mode 100644 tests/unit/utils/hardware/test_remote_linux.py create mode 100644 tests/unit/utils/hardware/test_remote_sonic.py diff --git a/osism/utils/hardware/collect.py b/osism/utils/hardware/collect.py new file mode 100644 index 000000000..26e11c611 --- /dev/null +++ b/osism/utils/hardware/collect.py @@ -0,0 +1,196 @@ +# SPDX-License-Identifier: Apache-2.0 + +"""Parallel collection of raw hardware data over SSH. + +Every device gets one SSH session. The session pipes a small, stdlib-only +collector script to ``python3 -`` on the device, and the script prints one +JSON document with raw command output and sysfs or database values. Parsing +happens afterwards on the manager, see the ``parse_*`` modules. + +Nothing in here talks to NetBox, so the collection can fan out to worker +threads while the NetBox session stays in the main thread. +""" + +from concurrent.futures import ThreadPoolExecutor +from dataclasses import dataclass, field +import json +from pathlib import Path +import shlex +import subprocess +from typing import Any, Callable, Dict, Iterable, List + +from osism.utils.hardware.selection import KIND_NODE, KIND_SWITCH, Target +from osism.utils.ssh import run_remote + +COLLECTORS = { + KIND_NODE: "remote_linux.py", + KIND_SWITCH: "remote_sonic.py", +} + + +class CollectError(Exception): + """A device could not be queried.""" + + +@dataclass +class Request: + """What to collect on the devices of one kind. + + Attributes: + sections: Names of the collector sections to run + options: Further command line options of the collector + sudo: Run the collector as root (``sudo -n``) + """ + + sections: List[str] + options: List[str] = field(default_factory=list) + sudo: bool = False + + +@dataclass +class Collection: + """Result of a collection run. + + Attributes: + documents: Raw collector document per device name + failed: Devices that could not be queried, as ``{"name", "error"}`` + """ + + documents: Dict[str, Dict[str, Any]] = field(default_factory=dict) + failed: List[Dict[str, str]] = field(default_factory=list) + + +def add_collection_arguments(parser) -> None: + """Add the collection options shared by the hardware reports.""" + parser.add_argument( + "--parallel", + type=int, + default=10, + metavar="N", + help="Number of devices queried at the same time (default: 10)", + ) + parser.add_argument( + "--timeout", + type=float, + default=60, + metavar="SECONDS", + help="Seconds one device may take (default: 60)", + ) + parser.add_argument( + "--dump-raw", + metavar="DIR", + help="Write the raw data collected from each device to DIR/.json", + ) + + +def collector_script(kind: str) -> str: + """Return the source of the collector for a device kind.""" + return (Path(__file__).parent / COLLECTORS[kind]).read_text() + + +def remote_command(request: Request) -> str: + """Return the command line that runs a collector read from stdin.""" + arguments = ["python3", "-", "--sections", ",".join(request.sections)] + arguments.extend(request.options) + command = " ".join(shlex.quote(argument) for argument in arguments) + return f"sudo -n {command}" if request.sudo else command + + +def collect_one( + target: Target, + request: Request, + timeout: float, + run: Callable[..., subprocess.CompletedProcess] = run_remote, +) -> Dict[str, Any]: + """Run the collector on one device and return its document. + + Raises: + CollectError: The device has no address, did not answer in time, + refused the login, or returned something that is not a + collector document + """ + if not target.address: + raise CollectError("no management address") + + try: + result = run( + target.address, + target.user, + remote_command(request), + stdin=collector_script(target.kind), + timeout=timeout, + ) + except subprocess.TimeoutExpired: + raise CollectError("timeout") + + if result.returncode != 0: + lines = (result.stderr or "").strip().splitlines() + raise CollectError(lines[-1] if lines else f"exit code {result.returncode}") + + try: + document = json.loads(result.stdout) + except ValueError: + raise CollectError("unparsable collector output") + if not isinstance(document, dict): + raise CollectError("unparsable collector output") + + return document + + +def collect( + targets: Iterable[Target], + requests: Dict[str, Request], + parallel: int = 10, + timeout: float = 60, + run: Callable[..., subprocess.CompletedProcess] = run_remote, +) -> Collection: + """Query all targets in parallel. + + A device that cannot be queried is recorded in ``failed``. It neither + aborts the run nor delays the other devices beyond ``timeout``. + + Args: + targets: Devices to query, with addresses resolved + requests: What to collect, per device kind + parallel: Number of devices queried at the same time + timeout: Seconds one device may take + run: Function executing the remote command, replaceable in tests + """ + targets = list(targets) + collection = Collection() + + def job(target: Target) -> Dict[str, Any]: + return collect_one(target, requests[target.kind], timeout, run) + + with ThreadPoolExecutor(max_workers=max(1, parallel)) as executor: + futures = [(target, executor.submit(job, target)) for target in targets] + for target, future in futures: + try: + collection.documents[target.name] = future.result() + except CollectError as exc: + collection.failed.append({"name": target.name, "error": str(exc)}) + except Exception as exc: + collection.failed.append( + {"name": target.name, "error": f"{type(exc).__name__}: {exc}"} + ) + + return collection + + +def dump_raw(collection: Collection, directory: str) -> List[str]: + """Write every raw collector document to ``/.json``. + + The files are what the parsers consume, which makes them usable as test + fixtures and as attachments to a bug report. + + Returns: + Paths of the files written + """ + base = Path(directory) + base.mkdir(parents=True, exist_ok=True) + paths = [] + for name, document in sorted(collection.documents.items()): + path = base / f"{name.replace('/', '_')}.json" + path.write_text(json.dumps(document, indent=2, sort_keys=True) + "\n") + paths.append(str(path)) + return paths diff --git a/osism/utils/hardware/remote_linux.py b/osism/utils/hardware/remote_linux.py new file mode 100644 index 000000000..1fc915795 --- /dev/null +++ b/osism/utils/hardware/remote_linux.py @@ -0,0 +1,190 @@ +# SPDX-License-Identifier: Apache-2.0 + +"""Collector for Linux nodes, executed on the node itself. + +This file is piped to ``python3 -`` over SSH. It must stay self-contained: +standard library only, no imports from ``osism``, and no syntax newer than +Python 3.6, because it runs with whatever interpreter the node has. + +It only gathers raw command output and sysfs values and prints them as one +JSON document. All parsing happens on the manager (``parse_linux``, +``parse_lldp``), where it can be tested against captured documents. +""" + +import argparse +import json +import os +import re +import socket +import subprocess +import sys + +VERSION = 1 +SYS_NET = "/sys/class/net" + +ARPHRD_ETHER = "1" +ARPHRD_INFINIBAND = "32" + +SYSFS_ATTRIBUTES = [ + "address", + "carrier", + "dev_port", + "duplex", + "mtu", + "operstate", + "phys_port_name", + "speed", + "type", +] + +# phys_port_name of a switchdev representor, e.g. pf0vf3 or c1pf0sf2 +REPRESENTOR = re.compile(r"pf\d+(vf|sf)\d+$") + + +def run(argv, timeout=20): + """Run a command and return its exit code and output as a dict.""" + try: + process = subprocess.run( + argv, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + universal_newlines=True, + timeout=timeout, + ) + except OSError as exc: + return {"rc": 127, "out": "", "err": str(exc)} + except subprocess.TimeoutExpired: + return {"rc": 124, "out": "", "err": "timeout"} + return { + "rc": process.returncode, + "out": process.stdout, + "err": process.stderr.strip(), + } + + +def read(path): + """Return the stripped content of a file, or None when it is unreadable.""" + try: + with open(path) as handle: + return handle.read().strip() + except (IOError, OSError): + return None + + +def port_kind(name, sys_net=SYS_NET): + """Classify a network device. + + Returns ``physical``, ``vf`` or ``infiniband`` for a port backed by + hardware, and None for everything else: bonds, bridges, VLANs and other + virtual devices have no ``device`` link, and switchdev representors stand + for a function of the NIC, not for a port. + """ + base = os.path.join(sys_net, name) + if not os.path.exists(os.path.join(base, "device")): + return None + + link_type = read(os.path.join(base, "type")) + if link_type == ARPHRD_INFINIBAND: + return "infiniband" + if link_type != ARPHRD_ETHER: + return None + + if os.path.exists(os.path.join(base, "device", "physfn")): + return "vf" + if REPRESENTOR.search(read(os.path.join(base, "phys_port_name")) or ""): + return None + return "physical" + + +def list_ports(kinds, sys_net=SYS_NET): + """Return ``{name: kind}`` for the ports of the requested kinds.""" + try: + names = sorted(os.listdir(sys_net)) + except OSError: + return {} + ports = {} + for name in names: + kind = port_kind(name, sys_net) + if kind in kinds: + ports[name] = kind + return ports + + +def collect_port(name, kind, sys_net=SYS_NET, runner=run): + """Collect identity and link state of one port.""" + base = os.path.join(sys_net, name) + port = { + "kind": kind, + "sysfs": dict( + (attribute, read(os.path.join(base, attribute))) + for attribute in SYSFS_ATTRIBUTES + ), + "pci": os.path.basename(os.path.realpath(os.path.join(base, "device"))), + "ethtool_P": runner(["ethtool", "-P", name]), + } + if kind == "vf": + try: + port["physfn"] = sorted( + os.listdir(os.path.join(base, "device", "physfn", "net")) + ) + except OSError: + port["physfn"] = [] + return port + + +def section_ports(document, ports, sys_net, runner): + document["ip_link"] = runner(["ip", "-j", "-d", "link", "show"]) + for name, kind in ports.items(): + document["ports"][name].update(collect_port(name, kind, sys_net, runner)) + + +def section_lldp(document, ports, sys_net, runner): + document["lldpctl"] = runner(["lldpctl", "-f", "json"]) + + +SECTIONS = { + "ports": section_ports, + "lldp": section_lldp, +} + + +def collect(sections, kinds, sys_net=SYS_NET, runner=run): + """Run the requested sections and return the document.""" + ports = list_ports(kinds, sys_net) + document = { + "collector": "linux", + "version": VERSION, + "hostname": socket.gethostname(), + "sections": sections, + "ports": dict((name, {}) for name in ports), + } + for section in sections: + SECTIONS[section](document, ports, sys_net, runner) + return document + + +def main(argv=None, sys_net=SYS_NET, runner=run, stream=None): + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--sections", default="ports") + parser.add_argument("--include-vf", action="store_true") + parser.add_argument("--include-infiniband", action="store_true") + arguments = parser.parse_args(argv) + + sections = [name for name in arguments.sections.split(",") if name] + unknown = [name for name in sections if name not in SECTIONS] + if unknown: + sys.stderr.write("unknown section(s): %s\n" % ", ".join(unknown)) + return 2 + + kinds = ["physical"] + if arguments.include_vf: + kinds.append("vf") + if arguments.include_infiniband: + kinds.append("infiniband") + + json.dump(collect(sections, kinds, sys_net, runner), stream or sys.stdout) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/osism/utils/hardware/remote_sonic.py b/osism/utils/hardware/remote_sonic.py new file mode 100644 index 000000000..680d519d9 --- /dev/null +++ b/osism/utils/hardware/remote_sonic.py @@ -0,0 +1,163 @@ +# SPDX-License-Identifier: Apache-2.0 + +"""Collector for SONiC switches, executed on the switch itself. + +This file is piped to ``python3 -`` over SSH. It must stay self-contained: +standard library only, no imports from ``osism``, and no syntax newer than +Python 3.6, because it runs with the interpreter of the SONiC host system. + +It reads the SONiC Redis databases with ``sonic-db-dump`` instead of parsing +the human-readable ``show`` tables, and prints the raw dumps as one JSON +document. All parsing happens on the manager (``parse_sonic``, +``parse_lldp``), where it can be tested against captured documents. +""" + +import argparse +import json +import os +import socket +import subprocess +import sys + +VERSION = 1 +SYS_NET = "/sys/class/net" +MGMT_PORT = "eth0" + +# Tables dumped per section: (database, table, key pattern, mandatory) +TABLES = { + "ports": [ + ("CONFIG_DB", "PORT", "PORT|*", True), + ("CONFIG_DB", "DEVICE_METADATA", "DEVICE_METADATA|localhost", False), + ("APPL_DB", "PORT_TABLE", "PORT_TABLE:*", False), + ], +} + + +def run(argv, timeout=30): + """Run a command and return its exit code and output as a dict.""" + try: + process = subprocess.run( + argv, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + universal_newlines=True, + timeout=timeout, + ) + except OSError as exc: + return {"rc": 127, "out": "", "err": str(exc)} + except subprocess.TimeoutExpired: + return {"rc": 124, "out": "", "err": "timeout"} + return { + "rc": process.returncode, + "out": process.stdout, + "err": process.stderr.strip(), + } + + +def read(path): + """Return the stripped content of a file, or None when it is unreadable.""" + try: + with open(path) as handle: + return handle.read().strip() + except (IOError, OSError): + return None + + +def db_dump(database, pattern, runner=run): + """Dump the keys of a database matching a pattern. + + The dump is tried as the login user first and with ``sudo -n`` second, + because access to Redis is restricted on some SONiC releases. + + Returns: + ``(dump, access, error)``: the parsed dump with the access method + that worked, or None with the last error + """ + error = "no output" + for access, prefix in (("user", []), ("sudo", ["sudo", "-n"])): + result = runner(prefix + ["sonic-db-dump", "-n", database, "-y", "-k", pattern]) + if result["rc"] != 0: + error = result["err"] or "exit code %d" % result["rc"] + continue + try: + dump = json.loads(result["out"] or "{}") + except ValueError: + error = "unparsable sonic-db-dump output" + continue + if isinstance(dump, dict): + return dump, access, None + error = "unparsable sonic-db-dump output" + return None, None, error + + +def extra_ports(document, sys_net, runner): + base = os.path.join(sys_net, MGMT_PORT) + if os.path.isdir(base): + document["mgmt"] = { + MGMT_PORT: dict( + (attribute, read(os.path.join(base, attribute))) + for attribute in ("address", "carrier", "mtu", "operstate", "speed") + ) + } + + +EXTRAS = { + "ports": extra_ports, +} + + +def collect(sections, sys_net=SYS_NET, runner=run): + """Run the requested sections and return the document. + + Raises: + RuntimeError: A mandatory table could not be read + """ + document = { + "collector": "sonic", + "version": VERSION, + "hostname": socket.gethostname(), + "sections": sections, + "db": {}, + "errors": {}, + } + for section in sections: + for database, table, pattern, mandatory in TABLES.get(section, []): + dump, access, error = db_dump(database, pattern, runner) + if dump is None: + if mandatory: + raise RuntimeError( + "cannot read %s %s: %s" % (database, table, error) + ) + document["errors"]["%s %s" % (database, table)] = error + continue + document["db"].setdefault(database, {})[table] = dump + document["db_access"] = access + extra = EXTRAS.get(section) + if extra: + extra(document, sys_net, runner) + return document + + +def main(argv=None, sys_net=SYS_NET, runner=run, stream=None): + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--sections", default="ports") + arguments = parser.parse_args(argv) + + sections = [name for name in arguments.sections.split(",") if name] + unknown = [name for name in sections if name not in TABLES and name not in EXTRAS] + if unknown: + sys.stderr.write("unknown section(s): %s\n" % ", ".join(unknown)) + return 2 + + try: + document = collect(sections, sys_net, runner) + except RuntimeError as exc: + sys.stderr.write("%s\n" % exc) + return 1 + + json.dump(document, stream or sys.stdout) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/unit/utils/hardware/test_collect.py b/tests/unit/utils/hardware/test_collect.py new file mode 100644 index 000000000..ac71d9544 --- /dev/null +++ b/tests/unit/utils/hardware/test_collect.py @@ -0,0 +1,225 @@ +# SPDX-License-Identifier: Apache-2.0 + +"""Unit tests for ``osism.utils.hardware.collect``. + +The remote execution is replaced by a function with the signature of +``osism.utils.ssh.run_remote``, so no SSH process is started. +""" + +import argparse +import json +import subprocess +import threading +import time +from types import SimpleNamespace + +import pytest + +from osism.utils.hardware.collect import ( + CollectError, + Request, + add_collection_arguments, + collect, + collect_one, + collector_script, + dump_raw, + remote_command, +) +from osism.utils.hardware.selection import KIND_NODE, KIND_SWITCH, Target + +REQUESTS = { + KIND_NODE: Request(sections=["ports", "lldp"], sudo=True), + KIND_SWITCH: Request(sections=["ports"]), +} + + +def _target(name, kind=KIND_NODE, address="192.0.2.1", user="dragon"): + return Target(name=name, hostname=name, kind=kind, address=address, user=user) + + +def _result(stdout="{}", returncode=0, stderr=""): + return SimpleNamespace(returncode=returncode, stdout=stdout, stderr=stderr) + + +def test_collection_arguments(): + parser = argparse.ArgumentParser() + add_collection_arguments(parser) + + args = parser.parse_args([]) + assert (args.parallel, args.timeout, args.dump_raw) == (10, 60, None) + + args = parser.parse_args( + ["--parallel", "4", "--timeout", "2.5", "--dump-raw", "/tmp/capture"] + ) + assert (args.parallel, args.timeout, args.dump_raw) == (4, 2.5, "/tmp/capture") + + +def test_remote_command_quotes_arguments_and_adds_sudo(): + assert remote_command(Request(sections=["ports"])) == "python3 - --sections ports" + assert ( + remote_command( + Request(sections=["ports", "lldp"], options=["--include-vf"], sudo=True) + ) + == "sudo -n python3 - --sections ports,lldp --include-vf" + ) + assert "'a b'" in remote_command(Request(sections=["ports"], options=["a b"])) + + +@pytest.mark.parametrize("kind", [KIND_NODE, KIND_SWITCH]) +def test_collector_script_is_self_contained_python(kind): + source = collector_script(kind) + + compile(source, f"<{kind} collector>", "exec") + assert "import osism" not in source and "from osism" not in source + + +def test_collect_one_pipes_the_collector_and_returns_the_document(): + calls = [] + + def run(host, user, command, stdin=None, timeout=None): + calls.append((host, user, command, stdin, timeout)) + return _result(stdout=json.dumps({"collector": "linux"})) + + document = collect_one( + _target("node-a", address="192.0.2.7"), REQUESTS[KIND_NODE], 42, run + ) + + assert document == {"collector": "linux"} + host, user, command, stdin, timeout = calls[0] + assert (host, user, timeout) == ("192.0.2.7", "dragon", 42) + assert command == "sudo -n python3 - --sections ports,lldp" + assert stdin == collector_script(KIND_NODE) + + +def test_collect_one_uses_the_collector_of_the_device_kind(): + seen = {} + + def run(host, user, command, stdin=None, timeout=None): + seen["stdin"] = stdin + return _result() + + collect_one(_target("leaf", kind=KIND_SWITCH), REQUESTS[KIND_SWITCH], 5, run) + + assert seen["stdin"] == collector_script(KIND_SWITCH) + + +@pytest.mark.parametrize( + "outcome,message", + [ + ( + _result(returncode=255, stderr="Permission denied (publickey)."), + "Permission denied (publickey).", + ), + ( + _result( + returncode=1, stderr="warning\ncannot read CONFIG_DB PORT: denied\n" + ), + "cannot read CONFIG_DB PORT: denied", + ), + (_result(returncode=3, stderr=""), "exit code 3"), + (_result(stdout="not json"), "unparsable collector output"), + (_result(stdout="[1, 2]"), "unparsable collector output"), + (subprocess.TimeoutExpired(cmd="ssh", timeout=5), "timeout"), + ], +) +def test_collect_one_failures(outcome, message): + def run(host, user, command, stdin=None, timeout=None): + if isinstance(outcome, Exception): + raise outcome + return outcome + + with pytest.raises(CollectError) as excinfo: + collect_one(_target("node-a"), REQUESTS[KIND_NODE], 5, run) + + assert str(excinfo.value) == message + + +def test_collect_one_without_address_does_not_connect(): + def run(*args, **kwargs): + raise AssertionError("must not connect") + + with pytest.raises(CollectError, match="no management address"): + collect_one(_target("leaf", address=""), REQUESTS[KIND_NODE], 5, run) + + +def test_collect_reports_failed_devices_and_keeps_the_others(): + def run(host, user, command, stdin=None, timeout=None): + if host == "bad": + raise subprocess.TimeoutExpired(cmd="ssh", timeout=timeout) + if host == "broken": + raise OSError("ssh binary missing") + return _result(stdout=json.dumps({"host": host})) + + targets = [ + _target("node-a", address="a"), + _target("node-b", address="bad"), + _target("leaf-1", kind=KIND_SWITCH, address="c", user="admin"), + _target("node-c", address="broken"), + ] + + collection = collect(targets, REQUESTS, parallel=2, timeout=5, run=run) + + assert collection.documents == {"node-a": {"host": "a"}, "leaf-1": {"host": "c"}} + assert collection.failed == [ + {"name": "node-b", "error": "timeout"}, + {"name": "node-c", "error": "OSError: ssh binary missing"}, + ] + + +def test_collect_queries_devices_in_parallel(): + """A slow device must not hold up the others. + + All four calls have to be in flight at the same time before any of them + is released. With serial execution the barrier would never fill and the + calls would fail with ``BrokenBarrierError``. + """ + barrier = threading.Barrier(4, timeout=5) + + def run(host, user, command, stdin=None, timeout=None): + barrier.wait() + return _result(stdout=json.dumps({"host": host})) + + targets = [_target(f"node-{i}", address=str(i)) for i in range(4)] + + collection = collect(targets, REQUESTS, parallel=4, timeout=5, run=run) + + assert sorted(collection.documents) == ["node-0", "node-1", "node-2", "node-3"] + assert collection.failed == [] + + +def test_collect_honours_the_parallel_limit(): + lock = threading.Lock() + state = {"running": 0, "peak": 0} + + def run(host, user, command, stdin=None, timeout=None): + with lock: + state["running"] += 1 + state["peak"] = max(state["peak"], state["running"]) + time.sleep(0.02) + with lock: + state["running"] -= 1 + return _result() + + targets = [_target(f"node-{i}", address=str(i)) for i in range(6)] + + collect(targets, REQUESTS, parallel=2, timeout=5, run=run) + + assert state["peak"] <= 2 + + +def test_dump_raw_writes_one_file_per_device(tmp_path): + def run(host, user, command, stdin=None, timeout=None): + return _result(stdout=json.dumps({"host": host})) + + collection = collect( + [_target("node-a", address="a"), _target("rack/1", address="b")], + REQUESTS, + run=run, + ) + + paths = dump_raw(collection, str(tmp_path / "capture")) + + assert [p.rsplit("/", 1)[1] for p in paths] == ["node-a.json", "rack_1.json"] + assert json.loads((tmp_path / "capture" / "node-a.json").read_text()) == { + "host": "a" + } diff --git a/tests/unit/utils/hardware/test_remote_linux.py b/tests/unit/utils/hardware/test_remote_linux.py new file mode 100644 index 000000000..b84551348 --- /dev/null +++ b/tests/unit/utils/hardware/test_remote_linux.py @@ -0,0 +1,173 @@ +# SPDX-License-Identifier: Apache-2.0 + +"""Unit tests for the Linux collector ``osism.utils.hardware.remote_linux``. + +The collector runs on the node. Here it reads a fake ``/sys/class/net`` +below ``tmp_path`` and gets its command output from a fake runner. +""" + +import io +import json + +import pytest + +from osism.utils.hardware import remote_linux + + +def _netdev(sys_net, name, device=True, link_type="1", physfn=None, **attributes): + base = sys_net / name + base.mkdir(parents=True) + (base / "type").write_text(f"{link_type}\n") + for attribute, value in attributes.items(): + (base / attribute).write_text(f"{value}\n") + if device: + (base / "device").mkdir() + if physfn: + (base / "device" / "physfn" / "net" / physfn).mkdir(parents=True) + return base + + +@pytest.fixture +def sys_net(tmp_path): + root = tmp_path / "net" + root.mkdir() + _netdev(root, "lo", device=False, link_type="772") + _netdev(root, "bond0", device=False) + _netdev(root, "vlan100", device=False) + _netdev( + root, + "ens1f0", + address="aa:bb:cc:00:00:01", + operstate="up", + carrier="1", + speed="25000", + mtu="9000", + phys_port_name="p0", + ) + _netdev(root, "ens1f0v0", physfn="ens1f0", address="02:00:00:00:00:01") + _netdev(root, "ens1f0r0", phys_port_name="pf0vf0") + _netdev(root, "ib0", link_type="32") + _netdev(root, "wwan0", link_type="65534") + return str(root) + + +def _runner(calls=None): + def run(argv, timeout=20): + if calls is not None: + calls.append(argv) + return {"rc": 0, "out": "output of " + " ".join(argv), "err": ""} + + return run + + +def test_port_kind_classifies_network_devices(sys_net): + kinds = { + name: remote_linux.port_kind(name, sys_net) + for name in [ + "lo", + "bond0", + "vlan100", + "ens1f0", + "ens1f0v0", + "ens1f0r0", + "ib0", + "wwan0", + ] + } + + assert kinds == { + "lo": None, + "bond0": None, + "vlan100": None, + "ens1f0": "physical", + "ens1f0v0": "vf", + "ens1f0r0": None, + "ib0": "infiniband", + "wwan0": None, + } + + +def test_list_ports_returns_only_the_requested_kinds(sys_net): + assert remote_linux.list_ports(["physical"], sys_net) == {"ens1f0": "physical"} + assert remote_linux.list_ports(["physical", "vf", "infiniband"], sys_net) == { + "ens1f0": "physical", + "ens1f0v0": "vf", + "ib0": "infiniband", + } + + +def test_list_ports_without_sysfs_is_empty(tmp_path): + assert remote_linux.list_ports(["physical"], str(tmp_path / "missing")) == {} + + +def test_collect_gathers_identity_and_link_state(sys_net): + calls = [] + + document = remote_linux.collect( + ["ports", "lldp"], ["physical", "vf"], sys_net, _runner(calls) + ) + + assert document["collector"] == "linux" + assert document["sections"] == ["ports", "lldp"] + assert sorted(document["ports"]) == ["ens1f0", "ens1f0v0"] + + port = document["ports"]["ens1f0"] + assert port["kind"] == "physical" + assert port["sysfs"]["address"] == "aa:bb:cc:00:00:01" + assert port["sysfs"]["speed"] == "25000" + assert port["sysfs"]["duplex"] is None + assert port["ethtool_P"]["out"] == "output of ethtool -P ens1f0" + assert "physfn" not in port + assert document["ports"]["ens1f0v0"]["physfn"] == ["ens1f0"] + + assert document["ip_link"]["out"] == "output of ip -j -d link show" + assert document["lldpctl"]["out"] == "output of lldpctl -f json" + assert ["lldpctl", "-f", "json"] in calls + + +def test_collect_runs_only_the_requested_sections(sys_net): + calls = [] + + document = remote_linux.collect(["ports"], ["physical"], sys_net, _runner(calls)) + + assert "lldpctl" not in document + assert ["lldpctl", "-f", "json"] not in calls + + +def test_main_prints_one_json_document(sys_net): + stream = io.StringIO() + + code = remote_linux.main( + ["--sections", "ports,lldp", "--include-vf", "--include-infiniband"], + sys_net=sys_net, + runner=_runner(), + stream=stream, + ) + + assert code == 0 + document = json.loads(stream.getvalue()) + assert sorted(document["ports"]) == ["ens1f0", "ens1f0v0", "ib0"] + + +def test_main_defaults_to_physical_ports(sys_net): + stream = io.StringIO() + + remote_linux.main([], sys_net=sys_net, runner=_runner(), stream=stream) + + assert sorted(json.loads(stream.getvalue())["ports"]) == ["ens1f0"] + + +def test_main_rejects_unknown_sections(sys_net, capsys): + code = remote_linux.main(["--sections", "ports,bogus"], sys_net=sys_net) + + assert code == 2 + assert "unknown section(s): bogus" in capsys.readouterr().err + + +def test_run_reports_missing_commands_and_output(): + missing = remote_linux.run(["/nonexistent/osism-test-binary"]) + assert missing["rc"] == 127 + assert missing["out"] == "" + + ok = remote_linux.run(["python3", "-c", "print('hello')"]) + assert ok == {"rc": 0, "out": "hello\n", "err": ""} diff --git a/tests/unit/utils/hardware/test_remote_sonic.py b/tests/unit/utils/hardware/test_remote_sonic.py new file mode 100644 index 000000000..d6bde22a7 --- /dev/null +++ b/tests/unit/utils/hardware/test_remote_sonic.py @@ -0,0 +1,139 @@ +# SPDX-License-Identifier: Apache-2.0 + +"""Unit tests for the SONiC collector ``osism.utils.hardware.remote_sonic``.""" + +import io +import json + +from osism.utils.hardware import remote_sonic + +PORT_DUMP = {"PORT|Ethernet0": {"type": "hash", "value": {"alias": "Eth1/1"}}} + + +def _runner(responses, calls=None): + """Answer ``sonic-db-dump`` calls from ``{(sudo, db, pattern): result}``.""" + + def run(argv, timeout=30): + if calls is not None: + calls.append(argv) + sudo = argv[0] == "sudo" + # sudo has a -n of its own, so look behind the command name + dump = argv[argv.index("sonic-db-dump") :] + database = dump[dump.index("-n") + 1] + pattern = dump[dump.index("-k") + 1] + result = responses.get((sudo, database, pattern)) + if result is None: + return {"rc": 1, "out": "", "err": "no such table"} + if isinstance(result, dict) and "rc" in result: + return result + return {"rc": 0, "out": json.dumps(result), "err": ""} + + return run + + +def test_db_dump_reads_as_the_login_user_first(): + calls = [] + runner = _runner({(False, "CONFIG_DB", "PORT|*"): PORT_DUMP}, calls) + + dump, access, error = remote_sonic.db_dump("CONFIG_DB", "PORT|*", runner) + + assert (dump, access, error) == (PORT_DUMP, "user", None) + assert calls == [["sonic-db-dump", "-n", "CONFIG_DB", "-y", "-k", "PORT|*"]] + + +def test_db_dump_retries_with_sudo(): + calls = [] + runner = _runner( + { + (False, "CONFIG_DB", "PORT|*"): {"rc": 1, "out": "", "err": "denied"}, + (True, "CONFIG_DB", "PORT|*"): PORT_DUMP, + }, + calls, + ) + + dump, access, error = remote_sonic.db_dump("CONFIG_DB", "PORT|*", runner) + + assert (dump, access, error) == (PORT_DUMP, "sudo", None) + assert calls[1][:2] == ["sudo", "-n"] + + +def test_db_dump_returns_the_last_error(): + runner = _runner( + { + (False, "CONFIG_DB", "PORT|*"): {"rc": 0, "out": "garbage", "err": ""}, + (True, "CONFIG_DB", "PORT|*"): { + "rc": 1, + "out": "", + "err": "sudo: a password is required", + }, + } + ) + + dump, access, error = remote_sonic.db_dump("CONFIG_DB", "PORT|*", runner) + + assert dump is None and access is None + assert error == "sudo: a password is required" + + +def test_db_dump_treats_empty_output_as_an_empty_table(): + runner = _runner({(False, "STATE_DB", "X|*"): {"rc": 0, "out": "", "err": ""}}) + + assert remote_sonic.db_dump("STATE_DB", "X|*", runner) == ({}, "user", None) + + +def test_collect_dumps_the_tables_of_the_section(tmp_path): + eth0 = tmp_path / "eth0" + eth0.mkdir() + (eth0 / "address").write_text("0c:29:ef:d1:a0:01\n") + (eth0 / "operstate").write_text("up\n") + runner = _runner( + { + (False, "CONFIG_DB", "PORT|*"): PORT_DUMP, + (False, "APPL_DB", "PORT_TABLE:*"): {"PORT_TABLE:Ethernet0": {"value": {}}}, + } + ) + + document = remote_sonic.collect(["ports"], str(tmp_path), runner) + + assert document["collector"] == "sonic" + assert document["db"]["CONFIG_DB"]["PORT"] == PORT_DUMP + assert "PORT_TABLE" in document["db"]["APPL_DB"] + assert document["db_access"] == "user" + # Optional tables that cannot be read are recorded, not fatal + assert document["errors"] == {"CONFIG_DB DEVICE_METADATA": "no such table"} + assert document["mgmt"]["eth0"]["address"] == "0c:29:ef:d1:a0:01" + assert document["mgmt"]["eth0"]["carrier"] is None + + +def test_collect_without_management_port_omits_it(tmp_path): + runner = _runner({(False, "CONFIG_DB", "PORT|*"): PORT_DUMP}) + + assert "mgmt" not in remote_sonic.collect(["ports"], str(tmp_path), runner) + + +def test_main_fails_with_a_clear_message_when_the_port_table_is_unreadable( + tmp_path, capsys +): + code = remote_sonic.main([], sys_net=str(tmp_path), runner=_runner({})) + + assert code == 1 + assert "cannot read CONFIG_DB PORT: no such table" in capsys.readouterr().err + + +def test_main_prints_one_json_document(tmp_path): + stream = io.StringIO() + runner = _runner({(False, "CONFIG_DB", "PORT|*"): PORT_DUMP}) + + code = remote_sonic.main( + ["--sections", "ports"], sys_net=str(tmp_path), runner=runner, stream=stream + ) + + assert code == 0 + assert json.loads(stream.getvalue())["db"]["CONFIG_DB"]["PORT"] == PORT_DUMP + + +def test_main_rejects_unknown_sections(tmp_path, capsys): + code = remote_sonic.main(["--sections", "bogus"], sys_net=str(tmp_path)) + + assert code == 2 + assert "unknown section(s): bogus" in capsys.readouterr().err From 1970a11760b68ee817ea97511a548ff9003a82e5 Mon Sep 17 00:00:00 2001 From: Christian Berendt Date: Wed, 11 Nov 2026 22:13:09 +0100 Subject: [PATCH 6/8] Add parsers for collected port and LLDP data The collectors return raw output. These parsers turn it into the records the reports work with. parse_linux and parse_sonic produce one port record per physical port with name, alias, altnames, admin and oper state, speed, MTU and MAC addresses. A bond member keeps its permanent MAC, since its address is the MAC of the bond while NetBox documents the one burned into the NIC. A port on a USB device, such as the host interface of a BMC, has no PCI address. parse_lldp normalises lldpctl JSON into neighbour records and keeps several neighbours per port. The fixtures are collector documents in the format --dump-raw writes. linux-connectx4.json and sonic-enterprise.json were captured from a node and from a switch running Enterprise SONiC 4.5.2, anonymized and reduced to a selection of ports. linux-mellanox.json and sonic-synthetic.json are synthetic: a node with a bond, an optical module and an empty cage, which were not captured, and the switch it is cabled to. The README next to them says which is which. Tests: the parsers against the fixtures and against malformed input. Part of #2876 and #2877. Assisted-by: Claude:claude-fable-5-2 Signed-off-by: Christian Berendt --- osism/utils/hardware/parse_linux.py | 130 ++++ osism/utils/hardware/parse_lldp.py | 144 +++++ osism/utils/hardware/parse_sonic.py | 120 ++++ tests/unit/utils/hardware/conftest.py | 18 + tests/unit/utils/hardware/fixtures/README.md | 49 ++ .../hardware/fixtures/linux-connectx4.json | 101 +++ .../hardware/fixtures/linux-mellanox.json | 101 +++ .../hardware/fixtures/sonic-enterprise.json | 608 ++++++++++++++++++ .../hardware/fixtures/sonic-synthetic.json | 235 +++++++ tests/unit/utils/hardware/test_parse_linux.py | 210 ++++++ tests/unit/utils/hardware/test_parse_lldp.py | 174 +++++ tests/unit/utils/hardware/test_parse_sonic.py | 169 +++++ 12 files changed, 2059 insertions(+) create mode 100644 osism/utils/hardware/parse_linux.py create mode 100644 osism/utils/hardware/parse_lldp.py create mode 100644 osism/utils/hardware/parse_sonic.py create mode 100644 tests/unit/utils/hardware/conftest.py create mode 100644 tests/unit/utils/hardware/fixtures/README.md create mode 100644 tests/unit/utils/hardware/fixtures/linux-connectx4.json create mode 100644 tests/unit/utils/hardware/fixtures/linux-mellanox.json create mode 100644 tests/unit/utils/hardware/fixtures/sonic-enterprise.json create mode 100644 tests/unit/utils/hardware/fixtures/sonic-synthetic.json create mode 100644 tests/unit/utils/hardware/test_parse_linux.py create mode 100644 tests/unit/utils/hardware/test_parse_lldp.py create mode 100644 tests/unit/utils/hardware/test_parse_sonic.py diff --git a/osism/utils/hardware/parse_linux.py b/osism/utils/hardware/parse_linux.py new file mode 100644 index 000000000..cf21db7b5 --- /dev/null +++ b/osism/utils/hardware/parse_linux.py @@ -0,0 +1,130 @@ +# SPDX-License-Identifier: Apache-2.0 + +"""Parsers for the raw document of the Linux collector (``remote_linux``).""" + +import json +import re +from typing import Any, Dict, List, Optional + +from osism.utils.hardware import natural_key, normalize_mac + +_PERMANENT_ADDRESS = re.compile(r"Permanent address:\s*(\S+)") +# The domain has four digits, or more behind a bridge such as Intel VMD +_PCI_ADDRESS = re.compile(r"^[0-9a-f]{4,8}:[0-9a-f]{2}:[0-9a-f]{2}\.[0-7]$", re.I) + + +def command_output(result: Any) -> Optional[str]: + """Return the stdout of a collected command, or None when it failed.""" + if not isinstance(result, dict) or result.get("rc") != 0: + return None + return result.get("out") or "" + + +def parse_ip_link(result: Any) -> Dict[str, Dict[str, Any]]: + """Index the output of ``ip -j -d link show`` by interface name.""" + output = command_output(result) + if not output: + return {} + try: + links = json.loads(output) + except ValueError: + return {} + if not isinstance(links, list): + return {} + return { + link["ifname"]: link + for link in links + if isinstance(link, dict) and link.get("ifname") + } + + +def parse_permanent_address(result: Any) -> Optional[str]: + """Return the MAC address printed by ``ethtool -P``.""" + match = _PERMANENT_ADDRESS.search(command_output(result) or "") + return normalize_mac(match.group(1)) if match else None + + +def oper_status(operstate: Optional[str], carrier: Optional[str]) -> Optional[str]: + """Reduce operstate and carrier to ``up``, ``down`` or None (unknown).""" + state = (operstate or "").lower() + if state == "up": + return "up" + if state in ("down", "lowerlayerdown", "notpresent", "dormant"): + return "down" + if carrier == "1": + return "up" + if carrier == "0": + return "down" + return None + + +def pci_address(value: Any) -> Optional[str]: + """Return a PCI address, or None for the address of another bus. + + The collector reports the bus address of the device behind a port. For + a USB NIC, such as the host interface of a BMC, that is a USB path + (``1-7.2:2.0``), which says nothing to ``lspci`` or NetBox. + """ + text = str(value or "").strip() + return text if _PCI_ADDRESS.match(text) else None + + +def _positive_int(value: Any) -> Optional[int]: + try: + number = int(value) + except (TypeError, ValueError): + return None + return number if number > 0 else None + + +def parse_port(name: str, raw: Dict[str, Any], link: Dict[str, Any]) -> Dict[str, Any]: + """Normalise identity and link state of one port. + + Args: + name: Kernel name of the port + raw: Entry of the port in the collector document + link: Entry of the port in ``ip -j -d link show``, may be empty + """ + sysfs = raw.get("sysfs") or {} + mac = normalize_mac(link.get("address") or sysfs.get("address")) + # A bond member reports the MAC of the bond as its address. The address + # burned into the NIC is what NetBox documents. + permanent_mac = ( + parse_permanent_address(raw.get("ethtool_P")) + or normalize_mac(link.get("permaddr")) + or mac + ) + + admin_status = None + if "flags" in link: + admin_status = "up" if "UP" in (link.get("flags") or []) else "down" + + physfn = raw.get("physfn") or [] + return { + "name": name, + "kind": raw.get("kind") or "physical", + "parent": physfn[0] if physfn else None, + "alias": link.get("ifalias") or None, + "altnames": list(link.get("altnames") or []), + "admin_status": admin_status, + "oper_status": oper_status( + link.get("operstate") or sysfs.get("operstate"), sysfs.get("carrier") + ), + "speed": _positive_int(sysfs.get("speed")), + "duplex": sysfs.get("duplex") or None, + "mtu": _positive_int(link.get("mtu") or sysfs.get("mtu")), + "mac": mac, + "permanent_mac": permanent_mac, + "bond": link.get("master") or None, + "pci_address": pci_address(raw.get("pci")), + } + + +def parse_ports(document: Dict[str, Any]) -> List[Dict[str, Any]]: + """Return the ports of a node in natural name order.""" + links = parse_ip_link(document.get("ip_link")) + ports = [ + parse_port(name, raw or {}, links.get(name) or {}) + for name, raw in (document.get("ports") or {}).items() + ] + return sorted(ports, key=lambda port: natural_key(port["name"])) diff --git a/osism/utils/hardware/parse_lldp.py b/osism/utils/hardware/parse_lldp.py new file mode 100644 index 000000000..01564407b --- /dev/null +++ b/osism/utils/hardware/parse_lldp.py @@ -0,0 +1,144 @@ +# SPDX-License-Identifier: Apache-2.0 + +"""Normalisation of LLDP neighbour data into one record format. + +A neighbour record is a dict with the keys ``system_name``, ``chassis_id``, +``chassis_id_type``, ``port_id``, ``port_id_type``, ``port_description``, +``management_addresses`` and ``age``. A port can have several neighbours, +for example behind a hypervisor bridge, so neighbours are kept as a list per +local interface. +""" + +import json +from typing import Any, Dict, List, Optional + +Neighbours = Dict[str, List[Dict[str, Any]]] + + +class LldpError(Exception): + """LLDP data is not available on a device.""" + + +def neighbour( + system_name: Optional[str] = None, + chassis_id: Optional[str] = None, + chassis_id_type: Optional[str] = None, + port_id: Optional[str] = None, + port_id_type: Optional[str] = None, + port_description: Optional[str] = None, + management_addresses: Optional[List[str]] = None, + age: Optional[str] = None, +) -> Dict[str, Any]: + """Build a neighbour record.""" + return { + "system_name": system_name or None, + "chassis_id": chassis_id or None, + "chassis_id_type": chassis_id_type or None, + "port_id": port_id or None, + "port_id_type": port_id_type or None, + "port_description": port_description or None, + "management_addresses": list(management_addresses or []), + "age": age or None, + } + + +def _typed_value(data: Any) -> tuple: + """Return ``(value, type)`` of an lldpctl ``{"type": .., "value": ..}``.""" + if isinstance(data, dict): + return data.get("value"), data.get("type") + return (data or None), None + + +def _as_list(value: Any) -> List[Any]: + if value is None: + return [] + return value if isinstance(value, list) else [value] + + +def _parse_chassis(chassis: Any) -> tuple: + """Return ``(system name, chassis body)`` of an lldpctl chassis entry. + + lldpctl keys the chassis by its system name. A neighbour that announces + no name has the chassis fields directly under ``chassis``. + """ + if not isinstance(chassis, dict) or not chassis: + return None, {} + if isinstance(chassis.get("id"), dict) and "value" in chassis["id"]: + return None, chassis + name = next(iter(chassis)) + body = chassis[name] + return name, body if isinstance(body, dict) else {} + + +def parse_lldpctl(data: Any) -> Neighbours: + """Parse the output of ``lldpctl -f json``. + + Args: + data: The JSON text, or the already decoded document + + Returns: + Neighbour records per local interface + + Raises: + ValueError: The text is not valid JSON + """ + if isinstance(data, str): + data = json.loads(data) if data.strip() else {} + if not isinstance(data, dict): + raise ValueError("unexpected lldpctl document") + + interfaces = (data.get("lldp") or {}).get("interface") or {} + # lldpctl prints a dict for a single interface and a list of + # single-key dicts for several. Normalise to the list form. + if isinstance(interfaces, dict): + interfaces = [{name: entry} for name, entry in interfaces.items()] + + neighbours: Neighbours = {} + for item in interfaces: + if not isinstance(item, dict): + continue + for local_interface, entry in item.items(): + entry = entry if isinstance(entry, dict) else {} + system_name, chassis = _parse_chassis(entry.get("chassis")) + chassis_id, chassis_id_type = _typed_value(chassis.get("id")) + port = entry.get("port") + if not isinstance(port, dict): + port = {} + port_id, port_id_type = _typed_value(port.get("id")) + neighbours.setdefault(local_interface, []).append( + neighbour( + system_name=system_name, + chassis_id=chassis_id, + chassis_id_type=chassis_id_type, + port_id=port_id, + port_id_type=port_id_type, + port_description=port.get("descr"), + management_addresses=[ + str(address) for address in _as_list(chassis.get("mgmt-ip")) + ], + age=entry.get("age"), + ) + ) + return neighbours + + +def linux_neighbours(document: Dict[str, Any]) -> Neighbours: + """Return the neighbours found in a Linux collector document. + + Raises: + LldpError: ``lldpctl`` is missing, ``lldpd`` is not running, or the + output cannot be parsed + """ + result = document.get("lldpctl") + if not isinstance(result, dict): + raise LldpError("LLDP data was not collected") + if result.get("rc") == 127: + raise LldpError("lldpctl not found") + if result.get("rc") != 0: + detail = (result.get("err") or "").strip().splitlines() + reason = detail[-1] if detail else f"exit code {result.get('rc')}" + raise LldpError(f"lldpctl failed: {reason}") + try: + return parse_lldpctl(result.get("out") or "") + except ValueError: + raise LldpError("unparsable lldpctl output") diff --git a/osism/utils/hardware/parse_sonic.py b/osism/utils/hardware/parse_sonic.py new file mode 100644 index 000000000..b0a7c89c7 --- /dev/null +++ b/osism/utils/hardware/parse_sonic.py @@ -0,0 +1,120 @@ +# SPDX-License-Identifier: Apache-2.0 + +"""Parsers for the raw document of the SONiC collector (``remote_sonic``).""" + +import re +from typing import Any, Dict, List, Optional + +from osism.utils.hardware import natural_key, normalize_mac + +MGMT_PORT = "eth0" +_FRONT_PANEL = re.compile(r"^Ethernet\d+$") + + +def table( + document: Dict[str, Any], database: str, name: str +) -> Dict[str, Dict[str, Any]]: + """Return a dumped table as ``{key without table prefix: fields}``. + + ``sonic-db-dump`` prints ``{"PORT|Ethernet0": {"type": "hash", "value": + {...}}}``. The separator is ``|`` in CONFIG_DB and STATE_DB and ``:`` in + APPL_DB. + """ + raw = ((document.get("db") or {}).get(database) or {}).get(name) or {} + entries: Dict[str, Dict[str, Any]] = {} + for key, entry in raw.items(): + if not key.startswith(name) or len(key) <= len(name) + 1: + continue + fields = entry.get("value") if isinstance(entry, dict) else None + if not isinstance(fields, dict): + fields = entry if isinstance(entry, dict) else {} + entries[key[len(name) + 1 :]] = fields + return entries + + +def _int(value: Any) -> Optional[int]: + try: + return int(value) + except (TypeError, ValueError): + return None + + +def _status(value: Any) -> Optional[str]: + value = str(value or "").lower() + return value if value in ("up", "down") else None + + +def device_metadata(document: Dict[str, Any]) -> Dict[str, Any]: + """Return the fields of ``DEVICE_METADATA|localhost``.""" + return table(document, "CONFIG_DB", "DEVICE_METADATA").get("localhost") or {} + + +def parse_ports(document: Dict[str, Any]) -> List[Dict[str, Any]]: + """Return the front-panel ports of a switch in natural name order. + + The port list and the configuration come from ``CONFIG_DB``, the + operational state from ``APPL_DB``. + """ + config = table(document, "CONFIG_DB", "PORT") + state = table(document, "APPL_DB", "PORT_TABLE") + + ports: List[Dict[str, Any]] = [] + for name, fields in config.items(): + if not _FRONT_PANEL.match(name): + continue + live = state.get(name) or {} + ports.append( + { + "name": name, + "kind": "physical", + "parent": None, + "alias": fields.get("alias") or None, + "altnames": [], + "admin_status": _status( + fields.get("admin_status") or live.get("admin_status") + ) + or "down", + "oper_status": _status(live.get("oper_status")), + "speed": _int(live.get("speed")) or _int(fields.get("speed")), + "duplex": None, + "mtu": _int(fields.get("mtu") or live.get("mtu")), + "mac": None, + "permanent_mac": None, + "bond": None, + "fec": fields.get("fec") or live.get("fec") or None, + "description": fields.get("description") + or live.get("description") + or None, + "lanes": fields.get("lanes") or None, + "index": _int(fields.get("index")), + } + ) + return sorted(ports, key=lambda port: natural_key(port["name"])) + + +def parse_mgmt_port(document: Dict[str, Any]) -> Optional[Dict[str, Any]]: + """Return the management port ``eth0``, or None when it was not seen.""" + raw = (document.get("mgmt") or {}).get(MGMT_PORT) + if not isinstance(raw, dict): + return None + + state = (raw.get("operstate") or "").lower() + if state not in ("up", "down"): + state = {"1": "up", "0": "down"}.get(raw.get("carrier") or "", "") + mac = normalize_mac(raw.get("address")) + speed = _int(raw.get("speed")) + return { + "name": MGMT_PORT, + "kind": "mgmt", + "parent": None, + "alias": None, + "altnames": [], + "admin_status": None, + "oper_status": state or None, + "speed": speed if speed and speed > 0 else None, + "duplex": None, + "mtu": _int(raw.get("mtu")), + "mac": mac, + "permanent_mac": mac, + "bond": None, + } diff --git a/tests/unit/utils/hardware/conftest.py b/tests/unit/utils/hardware/conftest.py new file mode 100644 index 000000000..5c70ddb3f --- /dev/null +++ b/tests/unit/utils/hardware/conftest.py @@ -0,0 +1,18 @@ +# SPDX-License-Identifier: Apache-2.0 + +import json +from pathlib import Path + +import pytest + +FIXTURES = Path(__file__).parent / "fixtures" + + +@pytest.fixture +def load_fixture(): + """Return a loader for the collector documents in ``fixtures/``.""" + + def _load(name): + return json.loads((FIXTURES / name).read_text()) + + return _load diff --git a/tests/unit/utils/hardware/fixtures/README.md b/tests/unit/utils/hardware/fixtures/README.md new file mode 100644 index 000000000..5c376fe7c --- /dev/null +++ b/tests/unit/utils/hardware/fixtures/README.md @@ -0,0 +1,49 @@ +# Collector documents used as parser fixtures + +Each file is one raw document as printed by a collector +(`osism/utils/hardware/remote_linux.py`, `remote_sonic.py`) and consumed by +the `parse_*` modules. + +## Provenance + +| File | Origin | Device | +| --- | --- | --- | +| `linux-connectx4.json` | captured | Node with an Intel I350 (`igb`) and a Mellanox ConnectX-4 Lx (`mlx5_core`) on 25G passive copper cables, Ubuntu 24.04 | +| `sonic-enterprise.json` | captured | Accton AS7726-32X with Enterprise SONiC 4.5.2, QSFP28 cages broken out into 4x25G and 4x10G | +| `linux-mellanox.json` | synthetic | Node with a bond, an optical module and an empty cage | +| `sonic-synthetic.json` | synthetic | Switch `leaf-1a`, the cable peer of `linux-mellanox.json` in the tests of the report commands | + +### Captured files + +A captured file was collected from a device of a running deployment and +then changed in two ways: + +- It is anonymized. Host names, MAC addresses, IP addresses, serial numbers + and VLAN IDs are replaced. The same value is replaced by the same + replacement in all files, so the two ends of a cable still point at each + other: `leaf-1` sees `node-1` on the port that `node-1` sees `leaf-1` on. +- It is reduced. A switch document keeps a selection of its ports, and the + `ip link` output of a node keeps the physical ports and `lo`. + +Vendor names, part numbers, firmware versions, key names and the structure +of the output are as captured. + +### Synthetic files + +A synthetic file was written by hand from the documented output formats of +the tools involved (`ip -j link`, `ethtool`, `lldpctl -f json`, +`sonic-db-dump`). Host names, MAC addresses and serial numbers are made up. +It covers what no capture exists for yet, and is only as good as that +documentation. + +## Adding a capture + +The hardware reports write the raw documents they collected with +`--dump-raw DIR`, one `.json` per device: + + osism report transceiver --dump-raw /tmp/capture compute-042 leaf-1a + +A capture contains host names, MAC and IP addresses and serial numbers of +the deployment it came from. Replace them before committing it, +consistently with the files that are already here, and say in the table +above what the file was captured from. diff --git a/tests/unit/utils/hardware/fixtures/linux-connectx4.json b/tests/unit/utils/hardware/fixtures/linux-connectx4.json new file mode 100644 index 000000000..0b3cea67d --- /dev/null +++ b/tests/unit/utils/hardware/fixtures/linux-connectx4.json @@ -0,0 +1,101 @@ +{ + "collector": "linux", + "hostname": "node-1", + "ip_link": { + "err": "info: Using default fan map value (33)", + "out": "[{\"ifindex\":1,\"ifname\":\"lo\",\"flags\":[\"LOOPBACK\",\"UP\",\"LOWER_UP\"],\"mtu\":65536,\"qdisc\":\"noqueue\",\"operstate\":\"UNKNOWN\",\"linkmode\":\"DEFAULT\",\"group\":\"default\",\"txqlen\":1000,\"link_type\":\"loopback\",\"address\":\"02:00:00:00:00:18\",\"broadcast\":\"02:00:00:00:00:18\",\"promiscuity\":0,\"allmulti\":0,\"min_mtu\":0,\"max_mtu\":0,\"inet6_addr_gen_mode\":\"eui64\",\"num_tx_queues\":1,\"num_rx_queues\":1,\"gso_max_size\":65536,\"gso_max_segs\":65535,\"tso_max_size\":524280,\"tso_max_segs\":65535,\"gro_max_size\":65536},{\"ifindex\":2,\"ifname\":\"enp4s0f0\",\"flags\":[\"BROADCAST\",\"MULTICAST\",\"UP\",\"LOWER_UP\"],\"mtu\":1500,\"qdisc\":\"mq\",\"operstate\":\"UP\",\"linkmode\":\"DEFAULT\",\"group\":\"default\",\"txqlen\":1000,\"link_type\":\"ether\",\"address\":\"02:00:00:00:00:19\",\"broadcast\":\"02:00:00:00:00:1a\",\"promiscuity\":0,\"allmulti\":0,\"min_mtu\":68,\"max_mtu\":9216,\"inet6_addr_gen_mode\":\"eui64\",\"num_tx_queues\":8,\"num_rx_queues\":8,\"gso_max_size\":65536,\"gso_max_segs\":65535,\"tso_max_size\":65536,\"tso_max_segs\":65535,\"gro_max_size\":65536,\"parentbus\":\"pci\",\"parentdev\":\"0000:04:00.0\",\"vfinfo_list\":[]},{\"ifindex\":3,\"ifname\":\"enp4s0f1\",\"flags\":[\"BROADCAST\",\"MULTICAST\"],\"mtu\":1500,\"qdisc\":\"noop\",\"operstate\":\"DOWN\",\"linkmode\":\"DEFAULT\",\"group\":\"default\",\"txqlen\":1000,\"link_type\":\"ether\",\"address\":\"02:00:00:00:00:1b\",\"broadcast\":\"02:00:00:00:00:1a\",\"promiscuity\":0,\"allmulti\":0,\"min_mtu\":68,\"max_mtu\":9216,\"inet6_addr_gen_mode\":\"eui64\",\"num_tx_queues\":8,\"num_rx_queues\":8,\"gso_max_size\":65536,\"gso_max_segs\":65535,\"tso_max_size\":65536,\"tso_max_segs\":65535,\"gro_max_size\":65536,\"parentbus\":\"pci\",\"parentdev\":\"0000:04:00.1\",\"vfinfo_list\":[]},{\"ifindex\":4,\"ifname\":\"ens4f0np0\",\"flags\":[\"BROADCAST\",\"MULTICAST\",\"UP\",\"LOWER_UP\"],\"mtu\":9100,\"qdisc\":\"mq\",\"operstate\":\"UP\",\"linkmode\":\"DEFAULT\",\"group\":\"default\",\"txqlen\":1000,\"link_type\":\"ether\",\"address\":\"02:00:00:00:00:01\",\"broadcast\":\"02:00:00:00:00:1a\",\"promiscuity\":0,\"allmulti\":0,\"min_mtu\":68,\"max_mtu\":9978,\"inet6_addr_gen_mode\":\"eui64\",\"num_tx_queues\":256,\"num_rx_queues\":32,\"gso_max_size\":65536,\"gso_max_segs\":65535,\"tso_max_size\":524280,\"tso_max_segs\":65535,\"gro_max_size\":65536,\"phys_port_name\":\"p0\",\"phys_switch_id\":\"020000ffff000001\",\"parentbus\":\"pci\",\"parentdev\":\"0000:82:00.0\",\"vfinfo_list\":[],\"altnames\":[\"enp130s0f0np0\"]},{\"ifindex\":5,\"ifname\":\"ens4f1np1\",\"flags\":[\"BROADCAST\",\"MULTICAST\",\"UP\",\"LOWER_UP\"],\"mtu\":9100,\"qdisc\":\"mq\",\"operstate\":\"UP\",\"linkmode\":\"DEFAULT\",\"group\":\"default\",\"txqlen\":1000,\"link_type\":\"ether\",\"address\":\"02:00:00:00:00:02\",\"broadcast\":\"02:00:00:00:00:1a\",\"promiscuity\":0,\"allmulti\":0,\"min_mtu\":68,\"max_mtu\":9978,\"inet6_addr_gen_mode\":\"eui64\",\"num_tx_queues\":256,\"num_rx_queues\":32,\"gso_max_size\":65536,\"gso_max_segs\":65535,\"tso_max_size\":524280,\"tso_max_segs\":65535,\"gro_max_size\":65536,\"phys_port_name\":\"p1\",\"phys_switch_id\":\"020000ffff000002\",\"parentbus\":\"pci\",\"parentdev\":\"0000:82:00.1\",\"vfinfo_list\":[],\"altnames\":[\"enp130s0f1np1\"]}]\n", + "rc": 0 + }, + "lldpctl": { + "err": "", + "out": "{\n \"lldp\": {\n \"interface\": [\n {\n \"enp4s0f0\": {\n \"via\": \"LLDP\",\n \"rid\": \"3\",\n \"age\": \"5 days, 15:27:45\",\n \"chassis\": {\n \"oob-1\": {\n \"id\": {\n \"type\": \"mac\",\n \"value\": \"02:00:00:00:00:1c\"\n },\n \"descr\": \"MikroTik RouterOS 6.49.1 (stable) CRS354-48G-4S+2Q+\",\n \"mgmt-ip\": \"192.0.2.1\",\n \"mgmt-iface\": \"27\",\n \"capability\": [\n {\n \"type\": \"Bridge\",\n \"enabled\": true\n },\n {\n \"type\": \"Router\",\n \"enabled\": true\n }\n ]\n }\n },\n \"port\": {\n \"id\": {\n \"type\": \"ifname\",\n \"value\": \"bridge/ether14\"\n },\n \"ttl\": \"120\"\n }\n }\n },\n {\n \"ens4f0np0\": {\n \"via\": \"LLDP\",\n \"rid\": \"2\",\n \"age\": \"5 days, 15:28:25\",\n \"chassis\": {\n \"leaf-1\": {\n \"id\": {\n \"type\": \"mac\",\n \"value\": \"02:00:00:00:00:1d\"\n },\n \"descr\": \"SONiC Software Version: SONiC.4.5.2-Enterprise_Base - HwSku: Accton-AS7726-32X - Distribution: Debian 11.11 - Kernel: 5.10.0-39-amd64\",\n \"mgmt-ip\": \"192.0.2.2\",\n \"capability\": [\n {\n \"type\": \"Bridge\",\n \"enabled\": false\n },\n {\n \"type\": \"Router\",\n \"enabled\": true\n },\n {\n \"type\": \"Wlan\",\n \"enabled\": false\n },\n {\n \"type\": \"Station\",\n \"enabled\": false\n }\n ]\n }\n },\n \"port\": {\n \"id\": {\n \"type\": \"local\",\n \"value\": \"Eth1/1/1\"\n },\n \"descr\": \"Ethernet0\",\n \"ttl\": \"120\",\n \"mfs\": \"9100\"\n },\n \"vlan\": [\n {\n \"vlan-id\": \"101\",\n \"pvid\": false,\n \"value\": \"Vlan_101\"\n },\n {\n \"vlan-id\": \"102\",\n \"pvid\": false,\n \"value\": \"Vlan_102\"\n },\n {\n \"vlan-id\": \"103\",\n \"pvid\": false,\n \"value\": \"Vlan_103\"\n }\n ],\n \"lldp-med\": {\n \"device-type\": \"Network Connectivity Device\",\n \"capability\": [\n {\n \"type\": \"Capabilities\",\n \"available\": true\n },\n {\n \"type\": \"Policy\",\n \"available\": true\n },\n {\n \"type\": \"Location\",\n \"available\": true\n },\n {\n \"type\": \"MDI/PSE\",\n \"available\": true\n },\n {\n \"type\": \"MDI/PD\",\n \"available\": true\n },\n {\n \"type\": \"Inventory\",\n \"available\": true\n }\n ],\n \"inventory\": {\n \"hardware\": \"R01H\",\n \"software\": \"5.10.0-39-amd64\",\n \"firmware\": \"v40.01.01.01\",\n \"serial\": \"SN00000001\",\n \"manufacturer\": \"Accton\",\n \"model\": \"AS7726-32X\",\n \"asset\": \"None\"\n }\n },\n \"unknown-tlvs\": {\n \"unknown-tlv\": {\n \"oui\": \"00,80,C2\",\n \"subtype\": \"7\",\n \"len\": \"5\",\n \"value\": \"01,00,00,00,00\"\n }\n }\n }\n },\n {\n \"ens4f1np1\": {\n \"via\": \"LLDP\",\n \"rid\": \"1\",\n \"age\": \"5 days, 15:28:25\",\n \"chassis\": {\n \"leaf-2\": {\n \"id\": {\n \"type\": \"mac\",\n \"value\": \"02:00:00:00:00:1e\"\n },\n \"descr\": \"SONiC Software Version: SONiC.4.4.1-Enterprise_Base - HwSku: Accton-AS7726-32X - Distribution: Debian 11.11 - Kernel: 5.10.0-21-amd64\",\n \"mgmt-ip\": \"192.0.2.3\",\n \"capability\": [\n {\n \"type\": \"Bridge\",\n \"enabled\": false\n },\n {\n \"type\": \"Router\",\n \"enabled\": true\n },\n {\n \"type\": \"Wlan\",\n \"enabled\": false\n },\n {\n \"type\": \"Station\",\n \"enabled\": false\n }\n ]\n }\n },\n \"port\": {\n \"id\": {\n \"type\": \"local\",\n \"value\": \"Eth1/1/1\"\n },\n \"descr\": \"Ethernet0\",\n \"ttl\": \"120\",\n \"mfs\": \"9100\"\n },\n \"vlan\": [\n {\n \"vlan-id\": \"104\",\n \"pvid\": false,\n \"value\": \"Vlan_104\"\n },\n {\n \"vlan-id\": \"105\",\n \"pvid\": false,\n \"value\": \"Vlan_105\"\n }\n ],\n \"lldp-med\": {\n \"device-type\": \"Network Connectivity Device\",\n \"capability\": [\n {\n \"type\": \"Capabilities\",\n \"available\": true\n },\n {\n \"type\": \"Policy\",\n \"available\": true\n },\n {\n \"type\": \"Location\",\n \"available\": true\n },\n {\n \"type\": \"MDI/PSE\",\n \"available\": true\n },\n {\n \"type\": \"MDI/PD\",\n \"available\": true\n },\n {\n \"type\": \"Inventory\",\n \"available\": true\n }\n ],\n \"inventory\": {\n \"hardware\": \"R01H\",\n \"software\": \"5.10.0-21-amd64\",\n \"firmware\": \"v40.01.01.01\",\n \"serial\": \"SN00000002\",\n \"manufacturer\": \"Accton\",\n \"model\": \"AS7726-32X\",\n \"asset\": \"None\"\n }\n },\n \"unknown-tlvs\": {\n \"unknown-tlv\": {\n \"oui\": \"00,80,C2\",\n \"subtype\": \"7\",\n \"len\": \"5\",\n \"value\": \"01,00,00,00,00\"\n }\n }\n }\n }\n ]\n }\n}\n", + "rc": 0 + }, + "ports": { + "enp4s0f0": { + "ethtool_P": { + "err": "", + "out": "Permanent address: 02:00:00:00:00:19\n", + "rc": 0 + }, + "kind": "physical", + "pci": "0000:04:00.0", + "sysfs": { + "address": "02:00:00:00:00:19", + "carrier": "1", + "dev_port": "0", + "duplex": "full", + "mtu": "1500", + "operstate": "up", + "phys_port_name": null, + "speed": "1000", + "type": "1" + } + }, + "enp4s0f1": { + "ethtool_P": { + "err": "", + "out": "Permanent address: 02:00:00:00:00:1b\n", + "rc": 0 + }, + "kind": "physical", + "pci": "0000:04:00.1", + "sysfs": { + "address": "02:00:00:00:00:1b", + "carrier": null, + "dev_port": "0", + "duplex": null, + "mtu": "1500", + "operstate": "down", + "phys_port_name": null, + "speed": null, + "type": "1" + } + }, + "ens4f0np0": { + "ethtool_P": { + "err": "", + "out": "Permanent address: 02:00:00:00:00:01\n", + "rc": 0 + }, + "kind": "physical", + "pci": "0000:82:00.0", + "sysfs": { + "address": "02:00:00:00:00:01", + "carrier": "1", + "dev_port": "0", + "duplex": "full", + "mtu": "9100", + "operstate": "up", + "phys_port_name": "p0", + "speed": "25000", + "type": "1" + } + }, + "ens4f1np1": { + "ethtool_P": { + "err": "", + "out": "Permanent address: 02:00:00:00:00:02\n", + "rc": 0 + }, + "kind": "physical", + "pci": "0000:82:00.1", + "sysfs": { + "address": "02:00:00:00:00:02", + "carrier": "1", + "dev_port": "0", + "duplex": "full", + "mtu": "9100", + "operstate": "up", + "phys_port_name": "p1", + "speed": "25000", + "type": "1" + } + } + }, + "sections": [ + "ports", + "lldp" + ], + "version": 1 +} diff --git a/tests/unit/utils/hardware/fixtures/linux-mellanox.json b/tests/unit/utils/hardware/fixtures/linux-mellanox.json new file mode 100644 index 000000000..ef188c8b8 --- /dev/null +++ b/tests/unit/utils/hardware/fixtures/linux-mellanox.json @@ -0,0 +1,101 @@ +{ + "collector": "linux", + "hostname": "compute-042", + "ip_link": { + "err": "", + "out": "[{\"ifindex\": 1, \"ifname\": \"lo\", \"flags\": [\"LOOPBACK\", \"UP\", \"LOWER_UP\"], \"mtu\": 65536, \"operstate\": \"UNKNOWN\", \"link_type\": \"loopback\", \"address\": \"00:00:00:00:00:00\"}, {\"ifindex\": 2, \"ifname\": \"eno1\", \"flags\": [\"BROADCAST\", \"MULTICAST\", \"UP\", \"LOWER_UP\"], \"mtu\": 1500, \"qdisc\": \"mq\", \"operstate\": \"UP\", \"linkmode\": \"DEFAULT\", \"group\": \"default\", \"txqlen\": 1000, \"link_type\": \"ether\", \"address\": \"3c:ec:ef:10:20:30\", \"broadcast\": \"ff:ff:ff:ff:ff:ff\", \"altnames\": [\"enp3s0f0\"]}, {\"ifindex\": 3, \"ifname\": \"eno2\", \"flags\": [\"BROADCAST\", \"MULTICAST\"], \"mtu\": 1500, \"qdisc\": \"mq\", \"operstate\": \"DOWN\", \"linkmode\": \"DEFAULT\", \"group\": \"default\", \"txqlen\": 1000, \"link_type\": \"ether\", \"address\": \"3c:ec:ef:10:20:31\", \"broadcast\": \"ff:ff:ff:ff:ff:ff\", \"altnames\": [\"enp3s0f1\"]}, {\"ifindex\": 4, \"ifname\": \"ens1f0np0\", \"flags\": [\"BROADCAST\", \"MULTICAST\", \"SLAVE\", \"UP\", \"LOWER_UP\"], \"mtu\": 9000, \"qdisc\": \"mq\", \"operstate\": \"UP\", \"linkmode\": \"DEFAULT\", \"group\": \"default\", \"txqlen\": 1000, \"link_type\": \"ether\", \"address\": \"b8:ce:f6:aa:00:10\", \"broadcast\": \"ff:ff:ff:ff:ff:ff\", \"master\": \"bond0\", \"linkinfo\": {\"info_slave_kind\": \"bond\"}, \"altnames\": [\"enp59s0f0np0\"]}, {\"ifindex\": 5, \"ifname\": \"ens1f1np1\", \"flags\": [\"BROADCAST\", \"MULTICAST\", \"SLAVE\", \"UP\", \"LOWER_UP\"], \"mtu\": 9000, \"qdisc\": \"mq\", \"operstate\": \"UP\", \"linkmode\": \"DEFAULT\", \"group\": \"default\", \"txqlen\": 1000, \"link_type\": \"ether\", \"address\": \"b8:ce:f6:aa:00:10\", \"broadcast\": \"ff:ff:ff:ff:ff:ff\", \"master\": \"bond0\", \"permaddr\": \"b8:ce:f6:aa:00:11\", \"linkinfo\": {\"info_slave_kind\": \"bond\"}, \"altnames\": [\"enp59s0f1np1\"]}, {\"ifindex\": 6, \"ifname\": \"bond0\", \"flags\": [\"BROADCAST\", \"MULTICAST\", \"MASTER\", \"UP\", \"LOWER_UP\"], \"mtu\": 9000, \"qdisc\": \"mq\", \"operstate\": \"UP\", \"linkmode\": \"DEFAULT\", \"group\": \"default\", \"txqlen\": 1000, \"link_type\": \"ether\", \"address\": \"b8:ce:f6:aa:00:10\", \"broadcast\": \"ff:ff:ff:ff:ff:ff\", \"linkinfo\": {\"info_kind\": \"bond\"}}]", + "rc": 0 + }, + "lldpctl": { + "err": "", + "out": "{\"lldp\": {\"interface\": [{\"ens1f0np0\": {\"via\": \"LLDP\", \"rid\": \"1\", \"age\": \"3 days, 04:12:55\", \"chassis\": {\"leaf-1a\": {\"id\": {\"type\": \"mac\", \"value\": \"0c:29:ef:d1:a0:00\"}, \"descr\": \"SONiC Software Version: SONiC.4.5.0-Enterprise_Base\", \"mgmt-ip\": \"10.10.0.11\", \"capability\": [{\"type\": \"Bridge\", \"enabled\": true}, {\"type\": \"Router\", \"enabled\": true}]}}, \"port\": {\"id\": {\"type\": \"local\", \"value\": \"Eth1/12\"}, \"descr\": \"Ethernet11\", \"ttl\": \"120\"}}}, {\"ens1f1np1\": {\"via\": \"LLDP\", \"rid\": \"1\", \"age\": \"3 days, 04:12:51\", \"chassis\": {\"leaf-1b\": {\"id\": {\"type\": \"mac\", \"value\": \"0c:29:ef:d1:b0:00\"}, \"descr\": \"SONiC Software Version: SONiC.4.5.0-Enterprise_Base\", \"mgmt-ip\": \"10.10.0.12\", \"capability\": [{\"type\": \"Bridge\", \"enabled\": true}, {\"type\": \"Router\", \"enabled\": true}]}}, \"port\": {\"id\": {\"type\": \"local\", \"value\": \"Eth1/12\"}, \"descr\": \"Ethernet11\", \"ttl\": \"120\"}}}, {\"eno1\": {\"via\": \"LLDP\", \"rid\": \"1\", \"age\": \"12 days, 01:00:09\", \"chassis\": {\"mgmt-1\": {\"id\": {\"type\": \"mac\", \"value\": \"0c:29:ef:d1:c0:00\"}, \"descr\": \"SONiC Software Version: SONiC.4.5.0-Enterprise_Base\", \"mgmt-ip\": \"10.10.0.5\", \"capability\": [{\"type\": \"Bridge\", \"enabled\": true}, {\"type\": \"Router\", \"enabled\": true}]}}, \"port\": {\"id\": {\"type\": \"local\", \"value\": \"Eth1/42\"}, \"descr\": \"Ethernet41\", \"ttl\": \"120\"}}}]}}", + "rc": 0 + }, + "ports": { + "eno1": { + "ethtool_P": { + "err": "", + "out": "Permanent address: 3c:ec:ef:10:20:30\n", + "rc": 0 + }, + "kind": "physical", + "pci": "0000:03:00.0", + "sysfs": { + "address": "3c:ec:ef:10:20:30", + "carrier": "1", + "dev_port": "0", + "duplex": "full", + "mtu": "1500", + "operstate": "up", + "phys_port_name": null, + "speed": "1000", + "type": "1" + } + }, + "eno2": { + "ethtool_P": { + "err": "", + "out": "Permanent address: 3c:ec:ef:10:20:31\n", + "rc": 0 + }, + "kind": "physical", + "pci": "0000:03:00.1", + "sysfs": { + "address": "3c:ec:ef:10:20:31", + "carrier": null, + "dev_port": "0", + "duplex": null, + "mtu": "1500", + "operstate": "down", + "phys_port_name": null, + "speed": null, + "type": "1" + } + }, + "ens1f0np0": { + "ethtool_P": { + "err": "", + "out": "Permanent address: b8:ce:f6:aa:00:10\n", + "rc": 0 + }, + "kind": "physical", + "pci": "0000:3b:00.0", + "sysfs": { + "address": "b8:ce:f6:aa:00:10", + "carrier": "1", + "dev_port": "0", + "duplex": "full", + "mtu": "9000", + "operstate": "up", + "phys_port_name": "p0", + "speed": "25000", + "type": "1" + } + }, + "ens1f1np1": { + "ethtool_P": { + "err": "", + "out": "Permanent address: b8:ce:f6:aa:00:11\n", + "rc": 0 + }, + "kind": "physical", + "pci": "0000:3b:00.1", + "sysfs": { + "address": "b8:ce:f6:aa:00:10", + "carrier": "1", + "dev_port": "0", + "duplex": "full", + "mtu": "9000", + "operstate": "up", + "phys_port_name": "p1", + "speed": "25000", + "type": "1" + } + } + }, + "sections": [ + "ports", + "lldp" + ], + "version": 1 +} diff --git a/tests/unit/utils/hardware/fixtures/sonic-enterprise.json b/tests/unit/utils/hardware/fixtures/sonic-enterprise.json new file mode 100644 index 000000000..e28b51ba4 --- /dev/null +++ b/tests/unit/utils/hardware/fixtures/sonic-enterprise.json @@ -0,0 +1,608 @@ +{ + "collector": "sonic", + "db": { + "APPL_DB": { + "PORT_TABLE": { + "PORT_TABLE:Ethernet0": { + "expireat": 1794564779.5969937, + "ttl": -0.001, + "type": "hash", + "value": { + "admin_status": "up", + "adv_speeds": "all", + "alias": "Eth1/1/1", + "app_admin_status": "up", + "auto_fec": "rs", + "auto_unreliable_los": "off", + "autoneg": "off", + "description": "", + "index": "1", + "lanes": "1", + "last_change": "1794075220", + "link_training": "off", + "medium": "copper", + "mtu": "9100", + "oper_speed": "25000", + "oper_status": "up", + "oper_status_change_uptime": "2958561", + "port_load_interval": "10", + "preemphasis": "0x1a4a00", + "reason": "OPER_UP", + "speed": "25000", + "tagged_vlans": "103,102,101,106", + "unreliable_los": "auto" + } + }, + "PORT_TABLE:Ethernet1": { + "expireat": 1794564779.5964913, + "ttl": -0.001, + "type": "hash", + "value": { + "admin_status": "up", + "adv_speeds": "all", + "alias": "Eth1/1/2", + "app_admin_status": "up", + "auto_fec": "rs", + "auto_unreliable_los": "off", + "autoneg": "off", + "description": "", + "index": "1", + "lanes": "2", + "last_change": "1794078218", + "link_training": "off", + "medium": "copper", + "mtu": "9100", + "oper_speed": "25000", + "oper_status": "up", + "oper_status_change_uptime": "2961559", + "port_load_interval": "10", + "preemphasis": "0x1a4a00", + "reason": "OPER_UP", + "speed": "25000", + "tagged_vlans": "103,102,101,106", + "unreliable_los": "auto" + } + }, + "PORT_TABLE:Ethernet12": { + "expireat": 1794564779.5966353, + "ttl": -0.001, + "type": "hash", + "value": { + "admin_status": "up", + "adv_speeds": "all", + "alias": "Eth1/4/1", + "app_admin_status": "up", + "auto_fec": "none", + "auto_unreliable_los": "off", + "autoneg": "off", + "description": "", + "index": "4", + "lanes": "13", + "last_change": "1794057220", + "link_training": "off", + "medium": "copper", + "mtu": "9100", + "oper_speed": "10000", + "oper_status": "up", + "oper_status_change_uptime": "2940561", + "port_load_interval": "10", + "preemphasis": "0x144c04", + "reason": "OPER_UP", + "speed": "10000", + "unreliable_los": "auto" + } + }, + "PORT_TABLE:Ethernet120": { + "expireat": 1794564779.5966928, + "ttl": -0.001, + "type": "hash", + "value": { + "admin_status": "up", + "adv_speeds": "all", + "alias": "Eth1/31", + "app_admin_status": "up", + "auto_fec": "rs", + "auto_unreliable_los": "off", + "autoneg": "off", + "description": "", + "index": "31", + "lanes": "121,122,123,124", + "last_change": "1791116717", + "link_training": "off", + "medium": "backplane", + "mtu": "9100", + "oper_speed": "100000", + "oper_status": "up", + "oper_status_change_uptime": "58", + "port_load_interval": "10", + "preemphasis": "0x184804,0x184804,0x184804,0x184804", + "reason": "OPER_UP", + "speed": "100000", + "unreliable_los": "auto", + "valid_speeds": "100000, 40000" + } + }, + "PORT_TABLE:Ethernet124": { + "expireat": 1794564779.5967875, + "ttl": -0.001, + "type": "hash", + "value": { + "admin_status": "up", + "adv_speeds": "all", + "alias": "Eth1/32", + "app_admin_status": "up", + "auto_fec": "rs", + "auto_unreliable_los": "off", + "autoneg": "off", + "description": "", + "index": "32", + "lanes": "125,126,127,128", + "last_change": "1791116717", + "link_training": "off", + "medium": "backplane", + "mtu": "9100", + "oper_speed": "100000", + "oper_status": "up", + "oper_status_change_uptime": "58", + "port_load_interval": "10", + "preemphasis": "0x184804,0x184804,0x184804,0x184804", + "reason": "OPER_UP", + "speed": "100000", + "unreliable_los": "auto", + "valid_speeds": "100000, 40000" + } + }, + "PORT_TABLE:Ethernet125": { + "expireat": 1794564779.5969734, + "ttl": -0.001, + "type": "hash", + "value": { + "admin_status": "down", + "adv_speeds": "all", + "alias": "Eth1/33", + "autoneg": "off", + "description": "", + "index": "33", + "lanes": "129", + "link_training": "off", + "mtu": "9100", + "oper_status": "down", + "oper_status_change_uptime": "0", + "port_load_interval": "10", + "reason": "ADMIN_DOWN", + "speed": "10000", + "unreliable_los": "auto" + } + }, + "PORT_TABLE:Ethernet13": { + "expireat": 1794564779.5970259, + "ttl": -0.001, + "type": "hash", + "value": { + "admin_status": "up", + "adv_speeds": "all", + "alias": "Eth1/4/2", + "app_admin_status": "up", + "auto_fec": "none", + "auto_unreliable_los": "off", + "autoneg": "off", + "description": "", + "index": "4", + "lanes": "14", + "last_change": "1794079447", + "link_training": "off", + "medium": "copper", + "mtu": "9100", + "oper_speed": "10000", + "oper_status": "up", + "oper_status_change_uptime": "2962788", + "port_load_interval": "10", + "preemphasis": "0x164c02", + "reason": "OPER_UP", + "speed": "10000", + "unreliable_los": "auto" + } + }, + "PORT_TABLE:Ethernet22": { + "expireat": 1794564779.596839, + "ttl": -0.001, + "type": "hash", + "value": { + "admin_status": "up", + "adv_speeds": "all", + "alias": "Eth1/6/3", + "app_admin_status": "up", + "auto_fec": "none", + "auto_unreliable_los": "off", + "autoneg": "off", + "description": "", + "index": "6", + "lanes": "23", + "last_change": "1794146532", + "link_training": "off", + "medium": "copper", + "mtu": "9100", + "oper_speed": "10000", + "oper_status": "up", + "oper_status_change_uptime": "3029873", + "port_load_interval": "10", + "preemphasis": "0x0e4104", + "reason": "OPER_UP", + "speed": "10000", + "unreliable_los": "auto" + } + }, + "PORT_TABLE:Ethernet24": { + "expireat": 1794564779.5970433, + "ttl": -0.001, + "type": "hash", + "value": { + "admin_status": "down", + "adv_speeds": "all", + "alias": "Eth1/7", + "autoneg": "off", + "description": "", + "index": "7", + "lanes": "25,26,27,28", + "link_training": "off", + "mtu": "9100", + "oper_status": "down", + "oper_status_change_uptime": "0", + "port_load_interval": "10", + "reason": "ADMIN_DOWN", + "speed": "100000", + "unreliable_los": "auto", + "valid_speeds": "100000, 40000" + } + }, + "PORT_TABLE:Ethernet4": { + "expireat": 1794564779.5965965, + "ttl": -0.001, + "type": "hash", + "value": { + "admin_status": "down", + "adv_speeds": "all", + "alias": "Eth1/2/1", + "app_admin_status": "up", + "auto_fec": "rs", + "auto_unreliable_los": "off", + "autoneg": "off", + "description": "", + "index": "2", + "lanes": "5", + "link_training": "off", + "medium": "copper", + "mtu": "9100", + "oper_status": "down", + "oper_status_change_uptime": "0", + "port_load_interval": "10", + "preemphasis": "0x1a4a00", + "reason": "ADMIN_DOWN", + "speed": "25000", + "unreliable_los": "auto" + } + }, + "PORT_TABLE:Ethernet6": { + "expireat": 1794564779.5965073, + "ttl": -0.001, + "type": "hash", + "value": { + "admin_status": "up", + "adv_speeds": "all", + "alias": "Eth1/2/3", + "app_admin_status": "up", + "auto_fec": "rs", + "auto_unreliable_los": "off", + "autoneg": "off", + "description": "", + "index": "2", + "lanes": "7", + "last_change": "1791116719", + "link_training": "off", + "medium": "copper", + "mtu": "9100", + "oper_speed": "25000", + "oper_status": "up", + "oper_status_change_uptime": "60", + "port_load_interval": "10", + "preemphasis": "0x1a4a00", + "reason": "OPER_UP", + "speed": "25000", + "unreliable_los": "auto" + } + }, + "PORT_TABLE:Ethernet9": { + "expireat": 1794564779.5968866, + "ttl": -0.001, + "type": "hash", + "value": { + "admin_status": "up", + "adv_speeds": "all", + "alias": "Eth1/3/2", + "app_admin_status": "up", + "auto_fec": "rs", + "auto_unreliable_los": "off", + "autoneg": "off", + "description": "", + "index": "3", + "lanes": "10", + "last_change": "1791116719", + "link_training": "off", + "medium": "copper", + "mtu": "9100", + "oper_speed": "25000", + "oper_status": "up", + "oper_status_change_uptime": "60", + "port_load_interval": "10", + "preemphasis": "0x144c04", + "reason": "OPER_UP", + "speed": "25000", + "unreliable_los": "auto" + } + }, + "PORT_TABLE:PortConfigDone": { + "expireat": 1794564779.596601, + "ttl": -0.001, + "type": "hash", + "value": { + "count": "12" + } + }, + "PORT_TABLE:PortInitDone": { + "expireat": 1794564779.5968077, + "ttl": -0.001, + "type": "hash", + "value": { + "lanes": "0" + } + } + } + }, + "CONFIG_DB": { + "DEVICE_METADATA": { + "DEVICE_METADATA|localhost": { + "expireat": 1794564779.2995586, + "ttl": -0.001, + "type": "hash", + "value": { + "default_config_profile": "l3", + "frr_mgmt_framework_config": "true", + "hostname": "leaf-1", + "hwsku": "Accton-AS7726-32X", + "intf_naming_mode": "standard", + "mac": "02:00:00:00:00:20", + "platform": "x86_64-accton_as7726_32x-r0", + "type": "LeafRouter" + } + } + }, + "PORT": { + "PORT|Ethernet0": { + "expireat": 1794564779.0154903, + "ttl": -0.001, + "type": "hash", + "value": { + "admin_status": "up", + "adv_speeds": "all", + "alias": "Eth1/1/1", + "autoneg": "off", + "index": "1", + "lanes": "1", + "link_training": "off", + "mtu": "9100", + "speed": "25000", + "tagged_vlans@": "103,102,101,106", + "unreliable_los": "auto" + } + }, + "PORT|Ethernet1": { + "expireat": 1794564779.01553, + "ttl": -0.001, + "type": "hash", + "value": { + "admin_status": "up", + "adv_speeds": "all", + "alias": "Eth1/1/2", + "autoneg": "off", + "index": "1", + "lanes": "2", + "link_training": "off", + "mtu": "9100", + "speed": "25000", + "tagged_vlans@": "103,102,101,106", + "unreliable_los": "auto" + } + }, + "PORT|Ethernet12": { + "expireat": 1794564779.0156577, + "ttl": -0.001, + "type": "hash", + "value": { + "admin_status": "up", + "adv_speeds": "all", + "alias": "Eth1/4/1", + "autoneg": "off", + "index": "4", + "lanes": "13", + "link_training": "off", + "mtu": "9100", + "speed": "10000", + "unreliable_los": "auto" + } + }, + "PORT|Ethernet120": { + "expireat": 1794564779.0153978, + "ttl": -0.001, + "type": "hash", + "value": { + "admin_status": "up", + "adv_speeds": "all", + "alias": "Eth1/31", + "autoneg": "off", + "index": "31", + "lanes": "121,122,123,124", + "link_training": "off", + "mtu": "9100", + "speed": "100000", + "unreliable_los": "auto", + "valid_speeds": "100000, 40000" + } + }, + "PORT|Ethernet124": { + "expireat": 1794564779.0153232, + "ttl": -0.001, + "type": "hash", + "value": { + "admin_status": "up", + "adv_speeds": "all", + "alias": "Eth1/32", + "autoneg": "off", + "index": "32", + "lanes": "125,126,127,128", + "link_training": "off", + "mtu": "9100", + "speed": "100000", + "unreliable_los": "auto", + "valid_speeds": "100000, 40000" + } + }, + "PORT|Ethernet125": { + "expireat": 1794564779.015463, + "ttl": -0.001, + "type": "hash", + "value": { + "admin_status": "down", + "adv_speeds": "all", + "alias": "Eth1/33", + "autoneg": "off", + "index": "33", + "lanes": "129", + "link_training": "off", + "mtu": "9100", + "speed": "10000", + "unreliable_los": "auto" + } + }, + "PORT|Ethernet13": { + "expireat": 1794564779.0156224, + "ttl": -0.001, + "type": "hash", + "value": { + "admin_status": "up", + "adv_speeds": "all", + "alias": "Eth1/4/2", + "autoneg": "off", + "index": "4", + "lanes": "14", + "link_training": "off", + "mtu": "9100", + "speed": "10000", + "unreliable_los": "auto" + } + }, + "PORT|Ethernet22": { + "expireat": 1794564779.0154417, + "ttl": -0.001, + "type": "hash", + "value": { + "admin_status": "up", + "adv_speeds": "all", + "alias": "Eth1/6/3", + "autoneg": "off", + "index": "6", + "lanes": "23", + "link_training": "off", + "mtu": "9100", + "speed": "10000", + "unreliable_los": "auto" + } + }, + "PORT|Ethernet24": { + "expireat": 1794564779.0153353, + "ttl": -0.001, + "type": "hash", + "value": { + "admin_status": "down", + "adv_speeds": "all", + "alias": "Eth1/7", + "autoneg": "off", + "index": "7", + "lanes": "25,26,27,28", + "link_training": "off", + "mtu": "9100", + "speed": "100000", + "unreliable_los": "auto", + "valid_speeds": "100000, 40000" + } + }, + "PORT|Ethernet4": { + "expireat": 1794564779.0154836, + "ttl": -0.001, + "type": "hash", + "value": { + "admin_status": "down", + "adv_speeds": "all", + "alias": "Eth1/2/1", + "autoneg": "off", + "index": "2", + "lanes": "5", + "link_training": "off", + "mtu": "9100", + "speed": "25000", + "unreliable_los": "auto" + } + }, + "PORT|Ethernet6": { + "expireat": 1794564779.0154977, + "ttl": -0.001, + "type": "hash", + "value": { + "admin_status": "up", + "adv_speeds": "all", + "alias": "Eth1/2/3", + "autoneg": "off", + "index": "2", + "lanes": "7", + "link_training": "off", + "mtu": "9100", + "speed": "25000", + "unreliable_los": "auto" + } + }, + "PORT|Ethernet9": { + "expireat": 1794564779.0155737, + "ttl": -0.001, + "type": "hash", + "value": { + "admin_status": "up", + "adv_speeds": "all", + "alias": "Eth1/3/2", + "autoneg": "off", + "index": "3", + "lanes": "10", + "link_training": "off", + "mtu": "9100", + "speed": "25000", + "unreliable_los": "auto" + } + } + } + } + }, + "db_access": "user", + "errors": {}, + "hostname": "leaf-1", + "mgmt": { + "eth0": { + "address": "02:00:00:00:00:1d", + "carrier": "1", + "mtu": "1500", + "operstate": "up", + "speed": "1000" + } + }, + "sections": [ + "ports" + ], + "version": 1 +} diff --git a/tests/unit/utils/hardware/fixtures/sonic-synthetic.json b/tests/unit/utils/hardware/fixtures/sonic-synthetic.json new file mode 100644 index 000000000..e202318b5 --- /dev/null +++ b/tests/unit/utils/hardware/fixtures/sonic-synthetic.json @@ -0,0 +1,235 @@ +{ + "collector": "sonic", + "db": { + "APPL_DB": { + "PORT_TABLE": { + "PORT_TABLE:Ethernet0": { + "expireat": 1767225600.0, + "ttl": -0.001, + "type": "hash", + "value": { + "admin_status": "up", + "mtu": "9100", + "oper_status": "up", + "speed": "25000" + } + }, + "PORT_TABLE:Ethernet1": { + "expireat": 1767225600.0, + "ttl": -0.001, + "type": "hash", + "value": { + "admin_status": "up", + "mtu": "9100", + "oper_status": "up", + "speed": "10000" + } + }, + "PORT_TABLE:Ethernet10": { + "expireat": 1767225600.0, + "ttl": -0.001, + "type": "hash", + "value": { + "admin_status": "up", + "mtu": "9100", + "oper_status": "down", + "speed": "25000" + } + }, + "PORT_TABLE:Ethernet11": { + "expireat": 1767225600.0, + "ttl": -0.001, + "type": "hash", + "value": { + "admin_status": "up", + "mtu": "9100", + "oper_status": "up", + "speed": "25000" + } + }, + "PORT_TABLE:Ethernet2": { + "expireat": 1767225600.0, + "ttl": -0.001, + "type": "hash", + "value": { + "admin_status": "down", + "mtu": "9100", + "oper_status": "down", + "speed": "25000" + } + }, + "PORT_TABLE:Ethernet48": { + "expireat": 1767225600.0, + "ttl": -0.001, + "type": "hash", + "value": { + "admin_status": "up", + "mtu": "9100", + "oper_status": "up", + "speed": "100000" + } + }, + "PORT_TABLE:Ethernet52": { + "expireat": 1767225600.0, + "ttl": -0.001, + "type": "hash", + "value": { + "admin_status": "up", + "mtu": "9100", + "oper_status": "down", + "speed": "100000" + } + }, + "PORT_TABLE:PortConfigDone": { + "expireat": 1767225600.0, + "ttl": -0.001, + "type": "hash", + "value": { + "count": "58" + } + }, + "PORT_TABLE:PortInitDone": { + "expireat": 1767225600.0, + "ttl": -0.001, + "type": "hash", + "value": { + "lanes": "0" + } + } + } + }, + "CONFIG_DB": { + "DEVICE_METADATA": { + "DEVICE_METADATA|localhost": { + "expireat": 1767225600.0, + "ttl": -0.001, + "type": "hash", + "value": { + "hostname": "leaf-1a", + "hwsku": "Accton-AS7326-56X", + "intf_naming_mode": "standard", + "mac": "0c:29:ef:d1:a0:00", + "platform": "x86_64-accton_as7326_56x-r0", + "type": "LeafRouter" + } + } + }, + "PORT": { + "PORT|Ethernet0": { + "expireat": 1767225600.0, + "ttl": -0.001, + "type": "hash", + "value": { + "admin_status": "up", + "alias": "Eth1/1", + "fec": "rs", + "index": "0", + "lanes": "1", + "mtu": "9100", + "speed": "25000" + } + }, + "PORT|Ethernet1": { + "expireat": 1767225600.0, + "ttl": -0.001, + "type": "hash", + "value": { + "admin_status": "up", + "alias": "Eth1/2", + "fec": "rs", + "index": "1", + "lanes": "2", + "mtu": "9100", + "speed": "25000" + } + }, + "PORT|Ethernet10": { + "expireat": 1767225600.0, + "ttl": -0.001, + "type": "hash", + "value": { + "admin_status": "up", + "alias": "Eth1/11", + "fec": "rs", + "index": "10", + "lanes": "11", + "mtu": "9100", + "speed": "25000" + } + }, + "PORT|Ethernet11": { + "expireat": 1767225600.0, + "ttl": -0.001, + "type": "hash", + "value": { + "admin_status": "up", + "alias": "Eth1/12", + "description": "compute-042", + "fec": "rs", + "index": "11", + "lanes": "12", + "mtu": "9100", + "speed": "25000" + } + }, + "PORT|Ethernet2": { + "expireat": 1767225600.0, + "ttl": -0.001, + "type": "hash", + "value": { + "admin_status": "down", + "alias": "Eth1/3", + "index": "2", + "lanes": "3", + "mtu": "9100", + "speed": "25000" + } + }, + "PORT|Ethernet48": { + "expireat": 1767225600.0, + "ttl": -0.001, + "type": "hash", + "value": { + "admin_status": "up", + "alias": "Eth1/49", + "fec": "rs", + "index": "48", + "lanes": "49", + "mtu": "9100", + "speed": "100000" + } + }, + "PORT|Ethernet52": { + "expireat": 1767225600.0, + "ttl": -0.001, + "type": "hash", + "value": { + "admin_status": "up", + "alias": "Eth1/50", + "fec": "rs", + "index": "52", + "lanes": "53", + "mtu": "9100", + "speed": "100000" + } + } + } + } + }, + "db_access": "user", + "errors": {}, + "hostname": "leaf-1a", + "mgmt": { + "eth0": { + "address": "0c:29:ef:d1:a0:01", + "carrier": "1", + "mtu": "1500", + "operstate": "up", + "speed": "1000" + } + }, + "sections": [ + "ports" + ], + "version": 1 +} diff --git a/tests/unit/utils/hardware/test_parse_linux.py b/tests/unit/utils/hardware/test_parse_linux.py new file mode 100644 index 000000000..506712048 --- /dev/null +++ b/tests/unit/utils/hardware/test_parse_linux.py @@ -0,0 +1,210 @@ +# SPDX-License-Identifier: Apache-2.0 + +"""Unit tests for ``osism.utils.hardware.parse_linux``.""" + +import json + +import pytest + +from osism.utils.hardware.parse_linux import ( + command_output, + oper_status, + parse_ip_link, + parse_permanent_address, + parse_ports, + pci_address, +) + + +def _ok(text): + return {"rc": 0, "out": text, "err": ""} + + +def test_command_output_is_none_for_failed_or_missing_commands(): + assert command_output(_ok("x")) == "x" + assert command_output({"rc": 1, "out": "x", "err": "boom"}) is None + assert command_output(None) is None + + +def test_parse_ip_link_indexes_by_name_and_tolerates_garbage(): + links = parse_ip_link(_ok(json.dumps([{"ifname": "eth0", "mtu": 1500}, {"x": 1}]))) + + assert links == {"eth0": {"ifname": "eth0", "mtu": 1500}} + assert parse_ip_link(_ok("not json")) == {} + assert parse_ip_link(_ok("{}")) == {} + assert parse_ip_link({"rc": 127, "out": "", "err": "not found"}) == {} + + +def test_parse_permanent_address(): + assert ( + parse_permanent_address(_ok("Permanent address: B8:CE:F6:AA:00:11\n")) + == "b8:ce:f6:aa:00:11" + ) + assert parse_permanent_address(_ok("Permanent address: 00:00:00:00:00:00")) is None + assert parse_permanent_address({"rc": 1, "out": "", "err": "x"}) is None + + +@pytest.mark.parametrize( + "operstate,carrier,expected", + [ + ("UP", None, "up"), + ("up", "0", "up"), + ("DOWN", "1", "down"), + ("LOWERLAYERDOWN", None, "down"), + ("UNKNOWN", "1", "up"), + ("unknown", "0", "down"), + (None, None, None), + ], +) +def test_oper_status(operstate, carrier, expected): + assert oper_status(operstate, carrier) == expected + + +def test_parse_ports_of_a_node(load_fixture): + ports = parse_ports(load_fixture("linux-mellanox.json")) + + assert [port["name"] for port in ports] == [ + "eno1", + "eno2", + "ens1f0np0", + "ens1f1np1", + ] + by_name = {port["name"]: port for port in ports} + + eno1 = by_name["eno1"] + assert eno1["kind"] == "physical" + assert eno1["admin_status"] == "up" + assert eno1["oper_status"] == "up" + assert eno1["speed"] == 1000 + assert eno1["mtu"] == 1500 + assert eno1["altnames"] == ["enp3s0f0"] + assert eno1["bond"] is None + assert eno1["pci_address"] == "0000:03:00.0" + + eno2 = by_name["eno2"] + assert eno2["admin_status"] == "down" + assert eno2["oper_status"] == "down" + assert eno2["speed"] is None + + +def test_parse_ports_of_a_captured_node(load_fixture): + ports = parse_ports(load_fixture("linux-connectx4.json")) + + assert [port["name"] for port in ports] == [ + "enp4s0f0", + "enp4s0f1", + "ens4f0np0", + "ens4f1np1", + ] + by_name = {port["name"]: port for port in ports} + + first = by_name["ens4f0np0"] + assert first["kind"] == "physical" + assert first["admin_status"] == "up" + assert first["oper_status"] == "up" + assert first["speed"] == 25000 + assert first["duplex"] == "full" + assert first["mtu"] == 9100 + assert first["altnames"] == ["enp130s0f0np0"] + assert first["mac"] == "02:00:00:00:00:01" + assert first["permanent_mac"] == "02:00:00:00:00:01" + assert first["bond"] is None + assert first["pci_address"] == "0000:82:00.0" + + # A port that is administratively down has no readable carrier, speed + # or duplex in sysfs + unused = by_name["enp4s0f1"] + assert unused["admin_status"] == "down" + assert unused["oper_status"] == "down" + assert unused["speed"] is None + assert unused["duplex"] is None + + +@pytest.mark.parametrize( + "value,expected", + [ + ("0000:82:00.0", "0000:82:00.0"), + ("0000:65:00.1", "0000:65:00.1"), + ("10000:01:00.0", "10000:01:00.0"), + # USB path of the host interface of a BMC + ("1-7.2:2.0", None), + ("", None), + (None, None), + ], +) +def test_pci_address_is_none_for_other_buses(value, expected): + assert pci_address(value) == expected + + +def test_port_on_a_usb_device_has_no_pci_address(): + document = {"ports": {"enx0200000000ff": {"kind": "physical", "pci": "1-7.2:2.0"}}} + + (port,) = parse_ports(document) + + assert port["pci_address"] is None + + +def test_bond_member_keeps_its_permanent_mac(load_fixture): + ports = {p["name"]: p for p in parse_ports(load_fixture("linux-mellanox.json"))} + + second = ports["ens1f1np1"] + # Both members report the MAC of the bond as their address + assert second["mac"] == "b8:ce:f6:aa:00:10" + assert second["permanent_mac"] == "b8:ce:f6:aa:00:11" + assert second["bond"] == "bond0" + assert ports["ens1f0np0"]["permanent_mac"] == "b8:ce:f6:aa:00:10" + + +def test_permanent_mac_falls_back_to_permaddr_then_address(): + links = [ + { + "ifname": "a", + "address": "aa:aa:aa:00:00:01", + "permaddr": "aa:aa:aa:00:00:02", + }, + {"ifname": "b", "address": "aa:aa:aa:00:00:03"}, + ] + document = { + "ports": { + "a": {"kind": "physical", "ethtool_P": {"rc": 127, "out": "", "err": ""}}, + "b": {"kind": "physical"}, + }, + "ip_link": _ok(json.dumps(links)), + } + + ports = {port["name"]: port for port in parse_ports(document)} + + assert ports["a"]["permanent_mac"] == "aa:aa:aa:00:00:02" + assert ports["b"]["permanent_mac"] == "aa:aa:aa:00:00:03" + + +def test_parse_ports_without_ip_link_uses_sysfs(): + document = { + "ports": { + "eth1": { + "kind": "vf", + "physfn": ["eth0"], + "sysfs": { + "address": "02:00:00:00:00:01", + "operstate": "up", + "speed": "-1", + "mtu": "1500", + }, + } + }, + "ip_link": {"rc": 127, "out": "", "err": "not found"}, + } + + (port,) = parse_ports(document) + + assert port["kind"] == "vf" + assert port["parent"] == "eth0" + assert port["admin_status"] is None + assert port["oper_status"] == "up" + assert port["speed"] is None + assert port["mac"] == "02:00:00:00:00:01" + assert port["altnames"] == [] + + +def test_parse_ports_of_an_empty_document(): + assert parse_ports({}) == [] diff --git a/tests/unit/utils/hardware/test_parse_lldp.py b/tests/unit/utils/hardware/test_parse_lldp.py new file mode 100644 index 000000000..6fe7cbbc7 --- /dev/null +++ b/tests/unit/utils/hardware/test_parse_lldp.py @@ -0,0 +1,174 @@ +# SPDX-License-Identifier: Apache-2.0 + +"""Unit tests for ``osism.utils.hardware.parse_lldp``.""" + +import json + +import pytest + +from osism.utils.hardware.parse_lldp import ( + LldpError, + linux_neighbours, + neighbour, + parse_lldpctl, +) + +IFACE_ETH0 = { + "chassis": {"switch-1": {"id": {"type": "mac", "value": "aa:bb:cc:00:00:01"}}}, + "port": {"id": {"type": "ifname", "value": "Ethernet10"}, "descr": "downlink"}, + "age": "12 days", +} +IFACE_ETH1 = { + "chassis": {"switch-2": {}}, + "port": {"id": {"value": "Ethernet20"}, "descr": "uplink"}, + "age": "3 days", +} + + +def _lldp(interface): + return json.dumps({"lldp": {"interface": interface}}) + + +def test_single_interface_dict_is_normalised(): + neighbours = parse_lldpctl(_lldp({"eth0": IFACE_ETH0})) + + assert neighbours == { + "eth0": [ + neighbour( + system_name="switch-1", + chassis_id="aa:bb:cc:00:00:01", + chassis_id_type="mac", + port_id="Ethernet10", + port_id_type="ifname", + port_description="downlink", + age="12 days", + ) + ] + } + + +def test_interface_list_is_handled(): + neighbours = parse_lldpctl(_lldp([{"eth0": IFACE_ETH0}, {"eth1": IFACE_ETH1}])) + + assert sorted(neighbours) == ["eth0", "eth1"] + assert neighbours["eth1"][0]["system_name"] == "switch-2" + assert neighbours["eth1"][0]["port_id"] == "Ethernet20" + assert neighbours["eth1"][0]["chassis_id"] is None + + +def test_several_neighbours_on_one_port_are_all_kept(): + neighbours = parse_lldpctl(_lldp([{"eth0": IFACE_ETH0}, {"eth0": IFACE_ETH1}])) + + assert [n["system_name"] for n in neighbours["eth0"]] == ["switch-1", "switch-2"] + + +def test_missing_neighbour_details_become_none(): + neighbours = parse_lldpctl(_lldp({"eth0": {}})) + + assert neighbours == {"eth0": [neighbour()]} + assert neighbours["eth0"][0]["management_addresses"] == [] + + +def test_chassis_without_a_system_name(): + entry = { + "chassis": { + "id": {"type": "mac", "value": "aa:bb:cc:00:00:09"}, + "mgmt-ip": "10.0.0.9", + }, + "port": {"id": {"type": "mac", "value": "aa:bb:cc:00:00:0a"}}, + } + + (found,) = parse_lldpctl(_lldp({"eth0": entry}))["eth0"] + + assert found["system_name"] is None + assert found["chassis_id"] == "aa:bb:cc:00:00:09" + assert found["port_id_type"] == "mac" + assert found["management_addresses"] == ["10.0.0.9"] + + +def test_management_addresses_can_be_a_list(): + entry = {"chassis": {"sw": {"mgmt-ip": ["10.0.0.1", "fe80::1"]}}} + + (found,) = parse_lldpctl(_lldp({"eth0": entry}))["eth0"] + + assert found["management_addresses"] == ["10.0.0.1", "fe80::1"] + + +@pytest.mark.parametrize( + "text", ["", "{}", '{"lldp": {}}', '{"lldp": {"interface": []}}'] +) +def test_no_neighbours(text): + assert parse_lldpctl(text) == {} + + +def test_invalid_json_raises_value_error(): + with pytest.raises(ValueError): + parse_lldpctl("not json") + with pytest.raises(ValueError): + parse_lldpctl("[1, 2]") + + +def test_linux_neighbours_from_a_collector_document(load_fixture): + neighbours = linux_neighbours(load_fixture("linux-mellanox.json")) + + assert sorted(neighbours) == ["eno1", "ens1f0np0", "ens1f1np1"] + (found,) = neighbours["ens1f0np0"] + assert found["system_name"] == "leaf-1a" + assert found["port_id"] == "Eth1/12" + assert found["port_id_type"] == "local" + assert found["port_description"] == "Ethernet11" + assert found["chassis_id"] == "0c:29:ef:d1:a0:00" + assert found["management_addresses"] == ["10.10.0.11"] + assert found["age"] == "3 days, 04:12:55" + + +def test_linux_neighbours_of_a_captured_node(load_fixture): + neighbours = linux_neighbours(load_fixture("linux-connectx4.json")) + + assert sorted(neighbours) == ["enp4s0f0", "ens4f0np0", "ens4f1np1"] + + # A SONiC switch configured by OSISM advertises the alias of the port as + # a locally assigned port ID and the SONiC name as the description. The + # port is the first lane of a cage that is broken out. + (leaf,) = neighbours["ens4f0np0"] + assert leaf["system_name"] == "leaf-1" + assert leaf["chassis_id"] == "02:00:00:00:00:1d" + assert leaf["chassis_id_type"] == "mac" + assert leaf["port_id"] == "Eth1/1/1" + assert leaf["port_id_type"] == "local" + assert leaf["port_description"] == "Ethernet0" + assert leaf["management_addresses"] == ["192.0.2.2"] + assert neighbours["ens4f1np1"][0]["system_name"] == "leaf-2" + + # RouterOS names a bridge port after the bridge and sends no description + (oob,) = neighbours["enp4s0f0"] + assert oob["system_name"] == "oob-1" + assert oob["port_id"] == "bridge/ether14" + assert oob["port_id_type"] == "ifname" + assert oob["port_description"] is None + + +@pytest.mark.parametrize( + "result,message", + [ + (None, "LLDP data was not collected"), + ({"rc": 127, "out": "", "err": "No such file"}, "lldpctl not found"), + ( + { + "rc": 1, + "out": "", + "err": "warning\nunable to connect to socket /run/lldpd.socket", + }, + "lldpctl failed: unable to connect to socket /run/lldpd.socket", + ), + ({"rc": 1, "out": "", "err": ""}, "lldpctl failed: exit code 1"), + ({"rc": 0, "out": "not json", "err": ""}, "unparsable lldpctl output"), + ], +) +def test_linux_neighbours_failures_have_a_clear_message(result, message): + document = {} if result is None else {"lldpctl": result} + + with pytest.raises(LldpError) as excinfo: + linux_neighbours(document) + + assert str(excinfo.value) == message diff --git a/tests/unit/utils/hardware/test_parse_sonic.py b/tests/unit/utils/hardware/test_parse_sonic.py new file mode 100644 index 000000000..0bc4edd07 --- /dev/null +++ b/tests/unit/utils/hardware/test_parse_sonic.py @@ -0,0 +1,169 @@ +# SPDX-License-Identifier: Apache-2.0 + +"""Unit tests for ``osism.utils.hardware.parse_sonic``.""" + +from osism.utils.hardware.parse_sonic import ( + device_metadata, + parse_mgmt_port, + parse_ports, + table, +) + + +def test_table_strips_the_prefix_and_unwraps_the_value(load_fixture): + document = load_fixture("sonic-enterprise.json") + + config = table(document, "CONFIG_DB", "PORT") + assert config["Ethernet0"]["alias"] == "Eth1/1/1" + + state = table(document, "APPL_DB", "PORT_TABLE") + assert state["Ethernet0"]["oper_status"] == "up" + assert "PortInitDone" in state + + +def test_table_accepts_entries_without_a_value_wrapper(): + document = {"db": {"CONFIG_DB": {"PORT": {"PORT|Ethernet0": {"alias": "Eth1/1"}}}}} + + assert table(document, "CONFIG_DB", "PORT") == {"Ethernet0": {"alias": "Eth1/1"}} + + +def test_table_that_was_not_collected_is_empty(): + assert table({}, "STATE_DB", "TRANSCEIVER_INFO") == {} + assert table({"db": {"CONFIG_DB": {}}}, "CONFIG_DB", "PORT") == {} + + +def test_device_metadata(load_fixture): + metadata = device_metadata(load_fixture("sonic-enterprise.json")) + + assert metadata["hwsku"] == "Accton-AS7726-32X" + assert metadata["hostname"] == "leaf-1" + assert device_metadata({}) == {} + + +def test_parse_ports_of_a_switch(load_fixture): + ports = parse_ports(load_fixture("sonic-enterprise.json")) + + assert [port["name"] for port in ports] == [ + "Ethernet0", + "Ethernet1", + "Ethernet4", + "Ethernet6", + "Ethernet9", + "Ethernet12", + "Ethernet13", + "Ethernet22", + "Ethernet24", + "Ethernet120", + "Ethernet124", + "Ethernet125", + ] + by_name = {port["name"]: port for port in ports} + + # First lane of a QSFP28 cage that is broken out into 4x25G + first = by_name["Ethernet0"] + assert first["alias"] == "Eth1/1/1" + assert first["admin_status"] == "up" + assert first["oper_status"] == "up" + assert first["speed"] == 25000 + assert first["mtu"] == 9100 + assert first["lanes"] == "1" + assert first["index"] == 1 + # Enterprise SONiC keeps neither of them in PORT unless configured + assert first["fec"] is None + assert first["description"] is None + + assert by_name["Ethernet1"]["alias"] == "Eth1/1/2" + assert by_name["Ethernet1"]["index"] == 1 + assert by_name["Ethernet4"]["admin_status"] == "down" + assert by_name["Ethernet4"]["oper_status"] == "down" + assert by_name["Ethernet12"]["speed"] == 10000 + assert by_name["Ethernet120"]["alias"] == "Eth1/31" + assert by_name["Ethernet120"]["speed"] == 100000 + assert by_name["Ethernet120"]["lanes"] == "121,122,123,124" + + +def test_parse_ports_reads_speed_fec_and_description_from_either_table(): + document = { + "db": { + "CONFIG_DB": { + "PORT": { + "PORT|Ethernet0": { + "value": { + "speed": "25000", + "fec": "rs", + "description": "compute-042", + } + }, + "PORT|Ethernet4": {"value": {"speed": "25000"}}, + } + }, + "APPL_DB": { + "PORT_TABLE": { + "PORT_TABLE:Ethernet0": { + "value": {"speed": "10000", "description": "ignored"} + }, + "PORT_TABLE:Ethernet4": { + "value": {"fec": "fc", "description": "uplink"} + }, + } + }, + } + } + + first, second = parse_ports(document) + + # The operational speed wins over the configured one + assert first["speed"] == 10000 + assert first["fec"] == "rs" + assert first["description"] == "compute-042" + assert second["speed"] == 25000 + assert second["fec"] == "fc" + assert second["description"] == "uplink" + + +def test_parse_ports_without_operational_state(): + document = { + "db": { + "CONFIG_DB": { + "PORT": { + "PORT|Ethernet4": {"value": {"alias": "Eth1/2", "speed": "25000"}}, + "PORT|Loopback0": {"value": {}}, + } + } + } + } + + (port,) = parse_ports(document) + + assert port["name"] == "Ethernet4" + assert port["admin_status"] == "down" + assert port["oper_status"] is None + assert port["speed"] == 25000 + + +def test_parse_mgmt_port(load_fixture): + port = parse_mgmt_port(load_fixture("sonic-enterprise.json")) + + assert port["name"] == "eth0" + assert port["kind"] == "mgmt" + assert port["oper_status"] == "up" + assert port["mac"] == "02:00:00:00:00:1d" + assert port["permanent_mac"] == "02:00:00:00:00:1d" + assert port["speed"] == 1000 + + +def test_parse_mgmt_port_uses_carrier_for_unknown_operstate(): + document = { + "mgmt": {"eth0": {"operstate": "unknown", "carrier": "0", "speed": "-1"}} + } + + port = parse_mgmt_port(document) + + assert port["oper_status"] == "down" + assert port["speed"] is None + assert port["mac"] is None + + +def test_parse_mgmt_port_is_none_when_it_was_not_seen(): + assert parse_mgmt_port({}) is None + assert parse_mgmt_port({"mgmt": {}}) is None From 2b1f496162624ccac06168043b936378bb8a765b Mon Sep 17 00:00:00 2001 From: Christian Berendt Date: Wed, 11 Nov 2026 22:13:09 +0100 Subject: [PATCH 7/8] Add matching of live ports to NetBox interfaces A port is called ens1f0np0 or Ethernet0 on the device and something else in NetBox. Both reports have to bring the two together, and have to show how they did it, so that a wrong match is visible. fetch_interfaces() loads the interfaces of all devices with a few chunked queries and converts the records to plain data at once, including the far end of the cable path. Reading a missing attribute from a pynetbox record would fetch that record again, once per interface. Switch ports are matched through the HWSKU port config, then by alias, then by name. The translation is the one the config generator uses; the interface cache is seeded from the bulk fetch so that it does not query NetBox per switch. The management port is matched against the mgmt_only interfaces. Node ports are matched by permanent MAC, name, label and finally by their LLDP neighbour. A neighbour is compared by the names it gives its port: the port ID and the port description. RouterOS announces a port that is member of a bridge as bridge/ether9; the name of the port alone is one of its names as well. Tests: conversion of API data including real pynetbox records, the translation against the port config files of the repository, and every mapping method with its precedence. Part of #2876 and #2877. Assisted-by: Claude:claude-fable-5-2 Signed-off-by: Christian Berendt --- osism/utils/hardware/mapping.py | 392 ++++++++++++++ tests/unit/utils/hardware/test_mapping.py | 615 ++++++++++++++++++++++ 2 files changed, 1007 insertions(+) create mode 100644 osism/utils/hardware/mapping.py create mode 100644 tests/unit/utils/hardware/test_mapping.py diff --git a/osism/utils/hardware/mapping.py b/osism/utils/hardware/mapping.py new file mode 100644 index 000000000..0909dc193 --- /dev/null +++ b/osism/utils/hardware/mapping.py @@ -0,0 +1,392 @@ +# SPDX-License-Identifier: Apache-2.0 + +"""Matching of live ports to NetBox interfaces. + +The NetBox interfaces of all devices of a report are fetched in bulk and +turned into plain data right away. Each live port is then matched to at most +one NetBox interface, and the method that produced the match is recorded, so +a wrong or missing match is visible in the report. +""" + +from dataclasses import dataclass, field +import re +from types import SimpleNamespace +from typing import Any, Dict, Iterable, List, Optional, Sequence, Tuple + +from osism.utils.hardware import normalize_mac + +VIRTUAL_TYPES = {"virtual", "lag", "bridge"} +MAPPING_NONE = "none" + +_SONIC_NAME = re.compile(r"^Ethernet\d+$") + + +@dataclass +class Endpoint: + """Far end of a cable path: a device and one of its interfaces.""" + + device: str + interface: str + device_id: Optional[int] = None + interface_id: Optional[int] = None + + +@dataclass +class NetboxInterface: + """A NetBox interface reduced to what the reports need. + + ``endpoints`` is the far end of the complete cable path, through patch + panels. It is empty when there is no cable, and also when the path does + not end at a device interface; ``path_incomplete`` tells the two apart. + """ + + id: int + device_id: Optional[int] + device: str + name: str + label: str = "" + type: str = "" + mgmt_only: bool = False + macs: List[str] = field(default_factory=list) + has_cable: bool = False + endpoints: List[Endpoint] = field(default_factory=list) + + @property + def is_virtual(self) -> bool: + return self.type in VIRTUAL_TYPES + + @property + def path_incomplete(self) -> bool: + return self.has_cable and not self.endpoints + + +@dataclass +class Match: + """NetBox interface a live port was matched to, and how.""" + + interface: Optional[NetboxInterface] = None + method: str = MAPPING_NONE + + +def interface_from_netbox(data: Dict[str, Any]) -> NetboxInterface: + """Build a ``NetboxInterface`` from the plain data of an API record.""" + device = data.get("device") or {} + interface_type = data.get("type") + if isinstance(interface_type, dict): + interface_type = interface_type.get("value") + + # mac_address is the only field before NetBox 4.2. Later releases keep + # the addresses as objects of their own and list them in mac_addresses. + candidates = [data.get("mac_address")] + primary = data.get("primary_mac_address") + if isinstance(primary, dict): + candidates.append(primary.get("mac_address")) + for entry in data.get("mac_addresses") or []: + candidates.append( + entry.get("mac_address") if isinstance(entry, dict) else entry + ) + macs: List[str] = [] + for candidate in candidates: + mac = normalize_mac(candidate) + if mac and mac not in macs: + macs.append(mac) + + endpoints: List[Endpoint] = [] + endpoint_type = data.get("connected_endpoints_type") + if data.get("connected_endpoints_reachable") and endpoint_type in ( + None, + "dcim.interface", + ): + for endpoint in data.get("connected_endpoints") or []: + if not isinstance(endpoint, dict): + continue + peer = endpoint.get("device") + if not isinstance(peer, dict) or not peer.get("name"): + continue + if not endpoint.get("name"): + continue + endpoints.append( + Endpoint( + device=peer["name"], + interface=endpoint["name"], + device_id=peer.get("id"), + interface_id=endpoint.get("id"), + ) + ) + + return NetboxInterface( + id=data["id"], + device_id=device.get("id"), + device=device.get("name") or "", + name=data.get("name") or "", + label=data.get("label") or "", + type=interface_type or "", + mgmt_only=bool(data.get("mgmt_only")), + macs=macs, + has_cable=bool(data.get("cable")), + endpoints=endpoints, + ) + + +def fetch_interfaces( + nb, device_ids: Iterable[int], chunk_size: int = 50 +) -> Dict[int, List[NetboxInterface]]: + """Fetch the interfaces of many devices with few requests. + + Records are converted with ``dict()``, which only reads what the list + response contains. Reading a missing attribute from a pynetbox record + would fetch the record again, once per interface. + + Returns: + Interfaces per device ID; every requested ID is present + """ + ids = sorted(set(device_ids)) + interfaces: Dict[int, List[NetboxInterface]] = {device_id: [] for device_id in ids} + for start in range(0, len(ids), chunk_size): + chunk = ids[start : start + chunk_size] + for record in nb.dcim.interfaces.filter(device_id=chunk): + interface = interface_from_netbox(dict(record)) + if interface.device_id is not None: + interfaces.setdefault(interface.device_id, []).append(interface) + return interfaces + + +def physical_interfaces( + interfaces: Iterable[NetboxInterface], include_mgmt: bool = True +) -> List[NetboxInterface]: + """Drop virtual and LAG interfaces, and management ones on request.""" + return [ + interface + for interface in interfaces + if not interface.is_virtual and (include_mgmt or not interface.mgmt_only) + ] + + +def sonic_name_index( + device_id: Optional[int], + device_name: str, + hwsku: Optional[str], + interfaces: Sequence[NetboxInterface], +) -> Dict[str, NetboxInterface]: + """Index the NetBox interfaces of a switch by their SONiC port name. + + The translation is the one the SONiC config generator uses + (``convert_netbox_interface_to_sonic``), so it needs the HWSKU of the + device and the matching ``port_config`` file. Without them the index is + empty and the mapping falls back to alias and name. + + Args: + interfaces: All interfaces of the device, including virtual ones; + the translation looks at them to detect breakout ports + """ + if not hwsku or device_id is None: + return {} + + # Imported here: the conductor package is expensive to import. + from osism.tasks.conductor.sonic.cache import get_interface_cache + from osism.tasks.conductor.sonic.interface import ( + convert_netbox_interface_to_sonic, + get_port_config, + ) + + if not get_port_config(hwsku): + return {} + + # The translation reads the interfaces of the device from this cache and + # would otherwise fetch them again, with one request per switch. + get_interface_cache().set_device_interfaces(device_id, list(interfaces)) + device = SimpleNamespace( + id=device_id, + name=device_name, + custom_fields={"sonic_parameters": {"hwsku": hwsku}}, + ) + + index: Dict[str, NetboxInterface] = {} + for interface in interfaces: + if interface.is_virtual or interface.mgmt_only: + continue + if interface.name.startswith("Ethernet"): + continue + try: + sonic_name = convert_netbox_interface_to_sonic(interface.name, device) + except Exception: + continue + # The function returns its input when it cannot translate it. + if _SONIC_NAME.match(sonic_name or ""): + index.setdefault(sonic_name, interface) + return index + + +class _Matcher: + """Keeps a NetBox interface from being matched to two ports.""" + + def __init__(self, interfaces: Sequence[NetboxInterface]): + self.interfaces = list(interfaces) + self.matches: Dict[str, Match] = {} + self.used: set = set() + + def assign( + self, port: Dict[str, Any], interface: Optional[NetboxInterface], method: str + ): + if interface is None or port["name"] in self.matches: + return + if interface.id in self.used: + return + self.matches[port["name"]] = Match(interface, method) + self.used.add(interface.id) + + def free(self) -> List[NetboxInterface]: + return [i for i in self.interfaces if i.id not in self.used] + + def result( + self, ports: Sequence[Dict[str, Any]] + ) -> Tuple[Dict[str, Match], List[NetboxInterface]]: + for port in ports: + self.matches.setdefault(port["name"], Match()) + return self.matches, self.free() + + +def _unique(interfaces: Iterable[NetboxInterface], key) -> Dict[str, NetboxInterface]: + index: Dict[str, NetboxInterface] = {} + for interface in interfaces: + value = key(interface) + if value: + index.setdefault(value, interface) + return index + + +def map_switch_ports( + ports: Sequence[Dict[str, Any]], + interfaces: Sequence[NetboxInterface], + sonic_index: Optional[Dict[str, NetboxInterface]] = None, +) -> Tuple[Dict[str, Match], List[NetboxInterface]]: + """Match the ports of a SONiC switch to its NetBox interfaces. + + Front-panel ports are matched by ``port-config`` (translation through + the HWSKU port config), then ``alias`` (the live alias is the NetBox + name), then ``name`` (the NetBox interface is called ``EthernetN``). + + The management port (kind ``mgmt``) is matched against the ``mgmt_only`` + interfaces by ``mac``, then ``name``, and finally as the only + ``mgmt_only`` interface of the device (``mgmt-only``). + + Returns: + ``(matches, unmatched)``: a ``Match`` per port name, and the NetBox + interfaces no port was matched to + """ + matcher = _Matcher(interfaces) + allowed = {interface.id for interface in interfaces} + front = [i for i in interfaces if not i.mgmt_only] + mgmt = [i for i in interfaces if i.mgmt_only] + front_ports = [port for port in ports if port.get("kind") != "mgmt"] + mgmt_ports = [port for port in ports if port.get("kind") == "mgmt"] + + for port in front_ports: + candidate = (sonic_index or {}).get(port["name"]) + if candidate is not None and candidate.id in allowed: + matcher.assign(port, candidate, "port-config") + + by_name = _unique(front, lambda interface: interface.name) + for port in front_ports: + matcher.assign(port, by_name.get(port.get("alias") or ""), "alias") + for port in front_ports: + matcher.assign(port, by_name.get(port["name"]), "name") + + for port in mgmt_ports: + mac = port.get("permanent_mac") or port.get("mac") + for interface in mgmt: + if mac and mac in interface.macs: + matcher.assign(port, interface, "mac") + mgmt_by_name = _unique(mgmt, lambda interface: interface.name) + for port in mgmt_ports: + matcher.assign(port, mgmt_by_name.get(port["name"]), "name") + if len(mgmt) == 1 and len(mgmt_ports) == 1: + matcher.assign(mgmt_ports[0], mgmt[0], "mgmt-only") + + return matcher.result(ports) + + +def same_system(left: Optional[str], right: Optional[str]) -> bool: + """Compare two system names case-insensitively, with and without domain.""" + if not left or not right: + return False + left, right = left.lower(), right.lower() + return left == right or left.split(".")[0] == right.split(".")[0] + + +def neighbour_port_names(neighbour: Dict[str, Any]) -> List[str]: + """Return the names an LLDP neighbour gives its port, best first. + + These are the port ID and the port description. RouterOS announces a + port that is member of a bridge as ``/`` (``bridge/ether9``) + with the subtype ``ifname``; the name of the port alone is added for it. + A number after the slash is not a name: ``Ethernet1/12`` stays whole. + """ + port_id = neighbour.get("port_id") + names = [value for value in (port_id, neighbour.get("port_description")) if value] + if port_id and neighbour.get("port_id_type") == "ifname" and "/" in port_id: + member = port_id.rsplit("/", 1)[1] + if member and not member.isdigit() and member not in names: + names.append(member) + return names + + +def neighbour_is_endpoint(neighbour: Dict[str, Any], endpoint: Endpoint) -> bool: + """Return True when an LLDP neighbour announces itself as an endpoint.""" + if not same_system(neighbour.get("system_name"), endpoint.device): + return False + return endpoint.interface in neighbour_port_names(neighbour) + + +def map_node_ports( + ports: Sequence[Dict[str, Any]], + interfaces: Sequence[NetboxInterface], + neighbours: Optional[Dict[str, List[Dict[str, Any]]]] = None, +) -> Tuple[Dict[str, Match], List[NetboxInterface]]: + """Match the ports of a Linux node to its NetBox interfaces. + + The order is ``mac`` (permanent MAC of the port is a MAC of the NetBox + interface), ``name`` (kernel name or an altname is the NetBox name), + ``label`` (kernel name is the NetBox label), and ``lldp`` (the LLDP + neighbour of the port is the cable peer of exactly one NetBox interface + that is still unmatched). + + Returns: + ``(matches, unmatched)``: a ``Match`` per port name, and the NetBox + interfaces no port was matched to + """ + matcher = _Matcher(interfaces) + + by_mac: Dict[str, NetboxInterface] = {} + for interface in interfaces: + for mac in interface.macs: + by_mac.setdefault(mac, interface) + for port in ports: + matcher.assign(port, by_mac.get(port.get("permanent_mac") or ""), "mac") + + by_name = _unique(interfaces, lambda interface: interface.name) + for port in ports: + for name in [port["name"], *(port.get("altnames") or [])]: + matcher.assign(port, by_name.get(name), "name") + + by_label = _unique(interfaces, lambda interface: interface.label) + for port in ports: + matcher.assign(port, by_label.get(port["name"]), "label") + + for port in ports: + seen = (neighbours or {}).get(port["name"]) or [] + if not seen or port["name"] in matcher.matches: + continue + candidates = [ + interface + for interface in matcher.free() + if any( + neighbour_is_endpoint(neighbour, endpoint) + for neighbour in seen + for endpoint in interface.endpoints + ) + ] + if len(candidates) == 1: + matcher.assign(port, candidates[0], "lldp") + + return matcher.result(ports) diff --git a/tests/unit/utils/hardware/test_mapping.py b/tests/unit/utils/hardware/test_mapping.py new file mode 100644 index 000000000..bf4beb45b --- /dev/null +++ b/tests/unit/utils/hardware/test_mapping.py @@ -0,0 +1,615 @@ +# SPDX-License-Identifier: Apache-2.0 + +"""Unit tests for ``osism.utils.hardware.mapping``.""" + +from pathlib import Path +from unittest.mock import MagicMock + +import pynetbox +import pytest +from pynetbox.models.dcim import Interfaces + +from osism.tasks.conductor.sonic import interface as sonic_interface +from osism.tasks.conductor.sonic.cache import clear_interface_cache +from osism.utils.hardware.mapping import ( + Endpoint, + Match, + NetboxInterface, + fetch_interfaces, + interface_from_netbox, + map_node_ports, + map_switch_ports, + neighbour_is_endpoint, + neighbour_port_names, + physical_interfaces, + same_system, + sonic_name_index, +) +from osism.utils.hardware.parse_lldp import neighbour + +REPO_ROOT = Path(__file__).resolve().parents[4] + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _api_interface(interface_id, name, device_id=1, device="leaf-1a", **extra): + """Interface as the NetBox API returns it in a list response.""" + data = { + "id": interface_id, + "url": f"http://netbox.invalid/api/dcim/interfaces/{interface_id}/", + "name": name, + "label": "", + "type": {"value": "25gbase-x-sfp28", "label": "SFP28 (25GE)"}, + "device": { + "id": device_id, + "url": f"http://netbox.invalid/api/dcim/devices/{device_id}/", + "name": device, + }, + "mgmt_only": False, + "mac_address": None, + "cable": None, + "connected_endpoints": None, + "connected_endpoints_type": None, + "connected_endpoints_reachable": None, + } + data.update(extra) + return data + + +def _peer(interface_id, name, device_id, device): + return { + "id": interface_id, + "url": f"http://netbox.invalid/api/dcim/interfaces/{interface_id}/", + "name": name, + "device": { + "id": device_id, + "url": f"http://netbox.invalid/api/dcim/devices/{device_id}/", + "name": device, + }, + } + + +def _cabled(interface_id, name, peers, **extra): + return _api_interface( + interface_id, + name, + cable={ + "id": 900 + interface_id, + "url": "http://netbox.invalid/api/dcim/cables/1/", + }, + connected_endpoints=peers, + connected_endpoints_type="dcim.interface", + connected_endpoints_reachable=True, + **extra, + ) + + +def _iface(interface_id, name, **kwargs): + kwargs.setdefault("device_id", 1) + kwargs.setdefault("device", "dev") + return NetboxInterface(id=interface_id, name=name, **kwargs) + + +def _port(name, **kwargs): + port = {"name": name, "kind": "physical", "altnames": []} + port.update(kwargs) + return port + + +def _methods(matches): + return { + name: (match.interface.name if match.interface else None, match.method) + for name, match in matches.items() + } + + +# --------------------------------------------------------------------------- +# Conversion of NetBox data +# --------------------------------------------------------------------------- + + +def test_interface_without_cable(): + interface = interface_from_netbox(_api_interface(5, "Eth1/1", label="port 1")) + + assert interface == NetboxInterface( + id=5, + device_id=1, + device="leaf-1a", + name="Eth1/1", + label="port 1", + type="25gbase-x-sfp28", + ) + assert interface.has_cable is False + assert interface.path_incomplete is False + assert interface.is_virtual is False + + +def test_interface_with_a_reachable_peer(): + interface = interface_from_netbox( + _cabled(5, "Eth1/1", [_peer(77, "ens1f0", 10, "compute-041")]) + ) + + assert interface.has_cable is True + assert interface.endpoints == [ + Endpoint( + device="compute-041", interface="ens1f0", device_id=10, interface_id=77 + ) + ] + assert interface.path_incomplete is False + + +def test_breakout_cable_has_several_endpoints(): + interface = interface_from_netbox( + _cabled( + 5, + "Eth1/49", + [ + _peer(77, "ens1f0", 10, "compute-041"), + _peer(78, "ens1f0", 11, "compute-042"), + ], + ) + ) + + assert [(e.device, e.interface) for e in interface.endpoints] == [ + ("compute-041", "ens1f0"), + ("compute-042", "ens1f0"), + ] + + +@pytest.mark.parametrize( + "overrides", + [ + # The path ends at a patch panel port + {"connected_endpoints": None}, + # The path ends at a circuit termination + { + "connected_endpoints": [{"id": 3, "url": "http://netbox.invalid/x/3/"}], + "connected_endpoints_type": "circuits.circuittermination", + }, + # A cable of the path is marked as not connected + {"connected_endpoints_reachable": False}, + ], +) +def test_cable_path_that_does_not_end_at_an_interface_is_incomplete(overrides): + data = _cabled(5, "Eth1/1", [_peer(77, "ens1f0", 10, "compute-041")]) + data.update(overrides) + + interface = interface_from_netbox(data) + + assert interface.endpoints == [] + assert interface.path_incomplete is True + + +def test_mac_addresses_of_old_and_new_netbox_are_merged(): + interface = interface_from_netbox( + _api_interface( + 5, + "ens1f0", + mac_address="AA:BB:CC:00:00:01", + primary_mac_address={"id": 1, "mac_address": "AA:BB:CC:00:00:01"}, + mac_addresses=[ + {"id": 1, "mac_address": "AA:BB:CC:00:00:01"}, + {"id": 2, "mac_address": "AA:BB:CC:00:00:02"}, + ], + ) + ) + + assert interface.macs == ["aa:bb:cc:00:00:01", "aa:bb:cc:00:00:02"] + + +def test_virtual_and_management_interfaces_can_be_dropped(): + interfaces = [ + _iface(1, "Eth1/1"), + _iface(2, "PortChannel1", type="lag"), + _iface(3, "Vlan100", type="virtual"), + _iface(4, "br0", type="bridge"), + _iface(5, "eth0", mgmt_only=True), + ] + + assert [i.name for i in physical_interfaces(interfaces)] == ["Eth1/1", "eth0"] + assert [i.name for i in physical_interfaces(interfaces, include_mgmt=False)] == [ + "Eth1/1" + ] + + +def test_fetch_interfaces_queries_in_chunks(): + nb = MagicMock() + nb.dcim.interfaces.filter.side_effect = [ + [ + _api_interface(1, "Eth1/1", device_id=1), + _api_interface(2, "Eth1/1", device_id=2), + ], + [_api_interface(3, "Eth1/1", device_id=3)], + ] + + interfaces = fetch_interfaces(nb, [3, 1, 2, 1, 4], chunk_size=2) + + assert [c.kwargs for c in nb.dcim.interfaces.filter.call_args_list] == [ + {"device_id": [1, 2]}, + {"device_id": [3, 4]}, + ] + assert { + device_id: [i.id for i in found] for device_id, found in interfaces.items() + } == { + 1: [1], + 2: [2], + 3: [3], + 4: [], + } + + +def test_fetch_interfaces_reads_pynetbox_records_without_further_requests(): + """A list response on NetBox before 4.2 has no ``mac_addresses``. + + Reading a missing attribute from a pynetbox record fetches the record + again. The API object points at a host that does not exist, so any such + request would make this test fail. + """ + api = pynetbox.api("http://netbox.invalid", token="unused") + record = Interfaces( + _cabled( + 5, + "Eth1/1", + [_peer(77, "ens1f0", 10, "compute-041")], + mac_address="AA:BB:CC:00:00:01", + ), + api, + None, + ) + nb = MagicMock() + nb.dcim.interfaces.filter.return_value = [record] + + (interface,) = fetch_interfaces(nb, [1])[1] + + assert interface.name == "Eth1/1" + assert interface.macs == ["aa:bb:cc:00:00:01"] + assert interface.endpoints == [ + Endpoint( + device="compute-041", interface="ens1f0", device_id=10, interface_id=77 + ) + ] + assert record.has_details is False + + +# --------------------------------------------------------------------------- +# SONiC name index +# --------------------------------------------------------------------------- + + +@pytest.fixture +def port_config(monkeypatch, mocker): + """Use the port config files of the repository and forbid NetBox access.""" + monkeypatch.setattr( + sonic_interface, "PORT_CONFIG_PATH", str(REPO_ROOT / "files/sonic/port_config") + ) + sonic_interface.clear_port_config_cache() + nb = MagicMock() + mocker.patch("osism.utils.nb", new=nb, create=True) + yield nb + sonic_interface.clear_port_config_cache() + clear_interface_cache() + + +def test_sonic_name_index_translates_through_the_port_config(port_config): + interfaces = [ + _iface(1, "Eth1/1"), + _iface(2, "Eth1/12"), + _iface(3, "Ethernet52"), + _iface(4, "PortChannel1", type="lag"), + _iface(5, "eth0", mgmt_only=True), + ] + + index = sonic_name_index(1, "leaf-1a", "Accton-AS7326-56X", interfaces) + + assert {name: i.name for name, i in index.items()} == { + "Ethernet0": "Eth1/1", + "Ethernet11": "Eth1/12", + } + # The interfaces were handed to the cache instead of being fetched again + port_config.dcim.interfaces.filter.assert_not_called() + + +def test_sonic_name_index_needs_hwsku_and_port_config(port_config, loguru_logs): + interfaces = [_iface(1, "Eth1/1")] + + assert sonic_name_index(1, "leaf-1a", None, interfaces) == {} + assert sonic_name_index(None, "leaf-1a", "Accton-AS7326-56X", interfaces) == {} + assert sonic_name_index(1, "leaf-1a", "No-Such-HWSKU", interfaces) == {} + # No per-interface warning for a switch that cannot be translated + assert not any("No HWSKU found" in r["message"] for r in loguru_logs) + + +# --------------------------------------------------------------------------- +# Switch mapping +# --------------------------------------------------------------------------- + + +def test_switch_ports_match_by_port_config_alias_and_name(): + interfaces = [ + _iface(1, "Eth1/1"), + _iface(2, "Eth1/2"), + _iface(3, "Ethernet8"), + _iface(4, "Eth1/9"), + ] + ports = [ + _port("Ethernet0", alias="twentyFiveGigE1"), + _port("Ethernet4", alias="Eth1/2"), + _port("Ethernet8", alias="twentyFiveGigE3"), + _port("Ethernet12", alias="twentyFiveGigE4"), + ] + + matches, unmatched = map_switch_ports( + ports, interfaces, {"Ethernet0": interfaces[0]} + ) + + assert _methods(matches) == { + "Ethernet0": ("Eth1/1", "port-config"), + "Ethernet4": ("Eth1/2", "alias"), + "Ethernet8": ("Ethernet8", "name"), + "Ethernet12": (None, "none"), + } + assert [i.name for i in unmatched] == ["Eth1/9"] + + +def test_port_config_wins_over_a_conflicting_alias(): + interfaces = [_iface(1, "Eth1/1"), _iface(2, "Eth1/2")] + ports = [_port("Ethernet0", alias="Eth1/2"), _port("Ethernet4", alias="Eth1/2")] + + matches, unmatched = map_switch_ports( + ports, interfaces, {"Ethernet0": interfaces[0]} + ) + + assert _methods(matches) == { + "Ethernet0": ("Eth1/1", "port-config"), + "Ethernet4": ("Eth1/2", "alias"), + } + assert unmatched == [] + + +def test_an_interface_is_matched_to_one_port_only(): + interfaces = [_iface(1, "Eth1/1")] + ports = [_port("Ethernet0", alias="Eth1/1"), _port("Ethernet4", alias="Eth1/1")] + + matches, _ = map_switch_ports(ports, interfaces) + + assert _methods(matches) == { + "Ethernet0": ("Eth1/1", "alias"), + "Ethernet4": (None, "none"), + } + + +def test_port_config_index_only_counts_for_the_given_interfaces(): + hidden = _iface(9, "Eth1/1") + + matches, _ = map_switch_ports([_port("Ethernet0")], [], {"Ethernet0": hidden}) + + assert matches == {"Ethernet0": Match()} + + +@pytest.mark.parametrize( + "mgmt_interfaces,expected", + [ + ( + [ + _iface(7, "mgmt0", mgmt_only=True, macs=["aa:aa:aa:00:00:01"]), + _iface(8, "eth0", mgmt_only=True), + ], + ("mgmt0", "mac"), + ), + ( + [_iface(7, "mgmt0", mgmt_only=True), _iface(8, "eth0", mgmt_only=True)], + ("eth0", "name"), + ), + ([_iface(7, "Management1", mgmt_only=True)], ("Management1", "mgmt-only")), + ( + [_iface(7, "mgmt0", mgmt_only=True), _iface(8, "mgmt1", mgmt_only=True)], + (None, "none"), + ), + ([], (None, "none")), + ], +) +def test_management_port_matches_management_interfaces(mgmt_interfaces, expected): + interfaces = [_iface(1, "Eth1/1"), *mgmt_interfaces] + ports = [ + _port("Ethernet0", alias="Eth1/1"), + _port( + "eth0", + kind="mgmt", + mac="aa:aa:aa:00:00:01", + permanent_mac="aa:aa:aa:00:00:01", + ), + ] + + matches, _ = map_switch_ports(ports, interfaces) + + assert _methods(matches)["eth0"] == expected + assert _methods(matches)["Ethernet0"] == ("Eth1/1", "alias") + + +def test_front_panel_port_never_matches_a_management_interface(): + interfaces = [_iface(7, "Ethernet0", mgmt_only=True)] + + matches, unmatched = map_switch_ports([_port("Ethernet0")], interfaces) + + assert matches == {"Ethernet0": Match()} + assert unmatched == interfaces + + +# --------------------------------------------------------------------------- +# Node mapping +# --------------------------------------------------------------------------- + + +def test_node_ports_match_by_mac_name_and_label(): + interfaces = [ + _iface(1, "nic1-port1", macs=["b8:ce:f6:aa:00:10"]), + _iface(2, "enp59s0f1np1"), + _iface(3, "Port 3", label="eno1"), + _iface(4, "eno9"), + ] + ports = [ + _port("ens1f0np0", permanent_mac="b8:ce:f6:aa:00:10"), + _port( + "ens1f1np1", permanent_mac="b8:ce:f6:aa:00:11", altnames=["enp59s0f1np1"] + ), + _port("eno1", permanent_mac="3c:ec:ef:10:20:30"), + _port("eno2", permanent_mac="3c:ec:ef:10:20:31"), + ] + + matches, unmatched = map_node_ports(ports, interfaces) + + assert _methods(matches) == { + "ens1f0np0": ("nic1-port1", "mac"), + "ens1f1np1": ("enp59s0f1np1", "name"), + "eno1": ("Port 3", "label"), + "eno2": (None, "none"), + } + assert [i.name for i in unmatched] == ["eno9"] + + +def test_bond_members_match_by_permanent_mac_not_by_bond_mac(): + interfaces = [ + _iface(1, "p1", macs=["b8:ce:f6:aa:00:10"]), + _iface(2, "p2", macs=["b8:ce:f6:aa:00:11"]), + ] + ports = [ + _port("ens1f1np1", mac="b8:ce:f6:aa:00:10", permanent_mac="b8:ce:f6:aa:00:11"), + _port("ens1f0np0", mac="b8:ce:f6:aa:00:10", permanent_mac="b8:ce:f6:aa:00:10"), + ] + + matches, _ = map_node_ports(ports, interfaces) + + assert _methods(matches) == { + "ens1f1np1": ("p2", "mac"), + "ens1f0np0": ("p1", "mac"), + } + + +def test_mac_match_wins_over_a_name_match(): + interfaces = [_iface(1, "eth0"), _iface(2, "eth1", macs=["aa:aa:aa:00:00:01"])] + ports = [_port("eth0", permanent_mac="aa:aa:aa:00:00:01")] + + matches, _ = map_node_ports(ports, interfaces) + + assert _methods(matches) == {"eth0": ("eth1", "mac")} + + +def test_node_port_matches_by_lldp_neighbour_when_it_is_unambiguous(): + interfaces = [ + _iface(1, "nic-a", endpoints=[Endpoint("leaf-1a", "Eth1/12")]), + _iface(2, "nic-b", endpoints=[Endpoint("leaf-1b", "Eth1/12")]), + ] + ports = [_port("ens1f0"), _port("ens1f1"), _port("ens1f2")] + neighbours = { + "ens1f0": [neighbour(system_name="LEAF-1A.example.com", port_id="Eth1/12")], + "ens1f1": [ + neighbour( + system_name="leaf-1b", port_id="Ethernet11", port_description="Eth1/12" + ) + ], + "ens1f2": [neighbour(system_name="leaf-1c", port_id="Eth1/12")], + } + + matches, unmatched = map_node_ports(ports, interfaces, neighbours) + + assert _methods(matches) == { + "ens1f0": ("nic-a", "lldp"), + "ens1f1": ("nic-b", "lldp"), + "ens1f2": (None, "none"), + } + assert unmatched == [] + + +def test_ambiguous_lldp_neighbour_does_not_match(): + interfaces = [ + _iface(1, "nic-a", endpoints=[Endpoint("leaf-1a", "Eth1/12")]), + _iface(2, "nic-b", endpoints=[Endpoint("leaf-1a", "Eth1/12")]), + ] + neighbours = {"ens1f0": [neighbour(system_name="leaf-1a", port_id="Eth1/12")]} + + matches, unmatched = map_node_ports([_port("ens1f0")], interfaces, neighbours) + + assert matches == {"ens1f0": Match()} + assert len(unmatched) == 2 + + +@pytest.mark.parametrize( + "left,right,expected", + [ + ("leaf-1a", "LEAF-1A", True), + ("leaf-1a.example.com", "leaf-1a", True), + ("leaf-1a", "leaf-1a.other.net", True), + ("leaf-1a", "leaf-1b", False), + (None, "leaf-1a", False), + ("leaf-1a", "", False), + ], +) +def test_same_system(left, right, expected): + assert same_system(left, right) is expected + + +def test_neighbour_is_endpoint_compares_port_id_and_description(): + endpoint = Endpoint("leaf-1a", "Eth1/12") + + assert neighbour_is_endpoint( + neighbour(system_name="leaf-1a", port_id="Eth1/12"), endpoint + ) + assert neighbour_is_endpoint( + neighbour(system_name="leaf-1a", port_id="x", port_description="Eth1/12"), + endpoint, + ) + assert not neighbour_is_endpoint( + neighbour(system_name="leaf-1a", port_id="Eth1/13"), endpoint + ) + assert not neighbour_is_endpoint( + neighbour(system_name="leaf-1b", port_id="Eth1/12"), endpoint + ) + + +@pytest.mark.parametrize( + "record,expected", + [ + # A SONiC switch: alias as port ID, SONiC name as description + ( + neighbour( + port_id="Eth1/1/1", port_id_type="local", port_description="Ethernet0" + ), + ["Eth1/1/1", "Ethernet0"], + ), + # lldpd on a node: MAC address as port ID, port name as description + ( + neighbour( + port_id="02:00:00:00:00:01", + port_id_type="mac", + port_description="ens4f0np0", + ), + ["02:00:00:00:00:01", "ens4f0np0"], + ), + # RouterOS: a bridge port, without description + ( + neighbour(port_id="bridge/ether9", port_id_type="ifname"), + ["bridge/ether9", "ether9"], + ), + (neighbour(port_id="ether9", port_id_type="ifname"), ["ether9"]), + (neighbour(port_id="bridge/", port_id_type="ifname"), ["bridge/"]), + (neighbour(port_id="Ethernet1/12", port_id_type="ifname"), ["Ethernet1/12"]), + # Only an interface name is cut at the slash + (neighbour(port_id="Eth1/12", port_id_type="local"), ["Eth1/12"]), + (neighbour(port_id="Eth1/12"), ["Eth1/12"]), + (neighbour(port_description="uplink"), ["uplink"]), + (neighbour(), []), + ], +) +def test_neighbour_port_names(record, expected): + assert neighbour_port_names(record) == expected + + +def test_neighbour_is_endpoint_knows_the_bridge_ports_of_routeros(): + seen = neighbour( + system_name="oob-1", port_id="bridge/ether9", port_id_type="ifname" + ) + + assert neighbour_is_endpoint(seen, Endpoint("oob-1", "ether9")) + assert not neighbour_is_endpoint(seen, Endpoint("oob-1", "ether10")) From 2e9df2fe5636b9649aadc76919b86cfff7c3b501 Mon Sep 17 00:00:00 2001 From: Christian Berendt Date: Wed, 11 Nov 2026 22:13:09 +0100 Subject: [PATCH 8/8] Add table, CSV and JSON rendering for hardware reports The report commands print a psql table and nothing else. The hardware reports are also meant to be read by scripts and spreadsheets. render provides the --format, --column, --long and --output options and renders flat rows as table or CSV with a selectable set of columns. Table cells are not parsed as numbers, which would turn a serial number such as 1E5 into 100000.0. JSON is rendered from a document the command builds, because it is nested per device. Tests: column selection, flattening, natural sorting, the three formats, and that writing to a file leaves stdout empty. Part of #2876 and #2877. Assisted-by: Claude:claude-fable-5-2 Signed-off-by: Christian Berendt --- osism/utils/hardware/render.py | 185 +++++++++++++++++++ tests/unit/utils/hardware/test_render.py | 223 +++++++++++++++++++++++ 2 files changed, 408 insertions(+) create mode 100644 osism/utils/hardware/render.py create mode 100644 tests/unit/utils/hardware/test_render.py diff --git a/osism/utils/hardware/render.py b/osism/utils/hardware/render.py new file mode 100644 index 000000000..846cd8f2d --- /dev/null +++ b/osism/utils/hardware/render.py @@ -0,0 +1,185 @@ +# SPDX-License-Identifier: Apache-2.0 + +"""Rendering of hardware reports as table, CSV or JSON. + +A report is a list of flat rows (dicts). Columns describe which keys of a +row are shown and under which header. The table uses the headers, CSV uses +the keys, and JSON is built by the command itself, because it is nested per +device instead of flat. +""" + +import csv +from dataclasses import dataclass +import io +import json +from pathlib import Path +from typing import Any, Dict, Iterable, List, Optional, Sequence + +from loguru import logger +from tabulate import tabulate + +from osism.utils.hardware import natural_key + +FORMATS = ["table", "csv", "json"] + + +class RenderError(Exception): + """The requested output cannot be produced.""" + + +@dataclass(frozen=True) +class Column: + """One column of a report. + + Attributes: + key: Key of the value in a flat row, also the CSV header + header: Header in the table + default: Shown without ``--long`` or ``--column`` + """ + + key: str + header: str + default: bool = True + + +def add_output_arguments(parser) -> None: + """Add the output options shared by the hardware reports.""" + parser.add_argument( + "--format", + type=str, + choices=FORMATS, + default="table", + help="Output format (default: table)", + ) + parser.add_argument( + "--column", + action="append", + metavar="NAME", + help="Column to include in output (can be used multiple times)", + ) + parser.add_argument( + "--long", + action="store_true", + help="Show all columns", + ) + parser.add_argument( + "--output", + metavar="FILE", + help="Write the report to a file instead of stdout", + ) + + +def normalize_column_name(name: str) -> str: + return name.strip().lower().replace(" ", "_").replace("-", "_") + + +def select_columns( + columns: Sequence[Column], + selected: Optional[Sequence[str]] = None, + long: bool = False, +) -> List[Column]: + """Return the columns to show. + + Explicitly selected columns win over ``long`` and are returned in the + order they were requested. A name matches the key or the header of a + column, ignoring case and treating spaces and dashes as underscores. + + Raises: + RenderError: None of the selected names is a column + """ + if not selected: + return [column for column in columns if long or column.default] + + by_name: Dict[str, Column] = {} + for column in columns: + by_name.setdefault(normalize_column_name(column.key), column) + by_name.setdefault(normalize_column_name(column.header), column) + + result: List[Column] = [] + for name in selected: + found = by_name.get(normalize_column_name(name)) + if found is None: + logger.warning(f"Column '{name}' not found") + elif found not in result: + result.append(found) + + if not result: + names = ", ".join(column.key for column in columns) + raise RenderError(f"No valid columns specified. Available columns: {names}") + return result + + +def flatten(record: Dict[str, Any], prefix: str = "") -> Dict[str, Any]: + """Flatten nested dicts, joining keys with ``_``. + + ``{"transceiver": {"vendor": "X"}}`` becomes ``{"transceiver_vendor": + "X"}``. Lists are kept as values. + """ + flat: Dict[str, Any] = {} + for key, value in record.items(): + name = f"{prefix}{key}" + if isinstance(value, dict): + flat.update(flatten(value, f"{name}_")) + else: + flat[name] = value + return flat + + +def cell(value: Any) -> str: + """Return the text shown for a value in a table or CSV cell.""" + if value is None: + return "" + if isinstance(value, bool): + return "yes" if value else "no" + if isinstance(value, (list, tuple)): + return ", ".join(cell(item) for item in value) + if isinstance(value, dict): + return json.dumps(value, sort_keys=True) + return str(value) + + +def sort_rows(rows: Iterable[Dict[str, Any]], *keys: str) -> List[Dict[str, Any]]: + """Sort rows by the given keys in natural order (Ethernet2 < Ethernet10).""" + return sorted(rows, key=lambda row: [natural_key(row.get(key)) for key in keys]) + + +def render_table( + rows: Sequence[Dict[str, Any]], + columns: Sequence[Column], + footer: Optional[Sequence[str]] = None, +) -> str: + """Render rows as a ``psql`` table followed by footer lines.""" + table = tabulate( + [[cell(row.get(column.key)) for column in columns] for row in rows], + headers=[column.header for column in columns], + tablefmt="psql", + # Serial numbers such as 1E5 must not be turned into numbers + disable_numparse=True, + ) + if footer: + return "\n".join([table, "", *footer]) + return table + + +def render_csv(rows: Sequence[Dict[str, Any]], columns: Sequence[Column]) -> str: + """Render rows as CSV with a header row of column keys.""" + buffer = io.StringIO() + writer = csv.writer(buffer, lineterminator="\n") + writer.writerow([column.key for column in columns]) + for row in rows: + writer.writerow([cell(row.get(column.key)) for column in columns]) + return buffer.getvalue().rstrip("\n") + + +def render_json(document: Any) -> str: + """Render a document as indented JSON.""" + return json.dumps(document, indent=2) + + +def write_output(text: str, path: Optional[str] = None) -> None: + """Print the report, or write it to a file when a path is given.""" + if path: + Path(path).write_text(text + "\n") + logger.info(f"Report written to {path}") + else: + print(text) diff --git a/tests/unit/utils/hardware/test_render.py b/tests/unit/utils/hardware/test_render.py new file mode 100644 index 000000000..98c693696 --- /dev/null +++ b/tests/unit/utils/hardware/test_render.py @@ -0,0 +1,223 @@ +# SPDX-License-Identifier: Apache-2.0 + +"""Unit tests for ``osism.utils.hardware.render``.""" + +import argparse +import csv +import io +import json + +import pytest + +from osism.utils.hardware.render import ( + Column, + RenderError, + add_output_arguments, + cell, + flatten, + render_csv, + render_json, + render_table, + select_columns, + sort_rows, + write_output, +) + +COLUMNS = [ + Column("device_name", "Device"), + Column("interface_name", "Interface"), + Column("transceiver_vendor", "Vendor"), + Column("transceiver_part_number", "Part Number", default=False), +] +ROWS = [ + { + "device_name": "leaf-1a", + "interface_name": "Ethernet0", + "transceiver_vendor": "ACME", + "transceiver_part_number": "1E5", + }, + { + "device_name": "leaf-1a", + "interface_name": "Ethernet4", + "transceiver_vendor": None, + }, +] + + +def _has_log(records, level, substring): + return any(r["level"] == level and substring in r["message"] for r in records) + + +def test_output_arguments(): + parser = argparse.ArgumentParser() + add_output_arguments(parser) + + args = parser.parse_args([]) + assert (args.format, args.column, args.long, args.output) == ( + "table", + None, + False, + None, + ) + + args = parser.parse_args( + ["--format", "csv", "--column", "a", "--column", "b", "--long", "--output", "f"] + ) + assert (args.format, args.column, args.long, args.output) == ( + "csv", + ["a", "b"], + True, + "f", + ) + + with pytest.raises(SystemExit): + parser.parse_args(["--format", "yaml"]) + + +def test_default_columns_and_long(): + assert [c.key for c in select_columns(COLUMNS)] == [ + "device_name", + "interface_name", + "transceiver_vendor", + ] + assert len(select_columns(COLUMNS, long=True)) == 4 + + +def test_selected_columns_match_key_or_header_in_requested_order(): + columns = select_columns( + COLUMNS, ["Part Number", "device-name", "VENDOR", "device_name"], long=True + ) + + assert [c.key for c in columns] == [ + "transceiver_part_number", + "device_name", + "transceiver_vendor", + ] + + +def test_unknown_column_is_reported(loguru_logs): + columns = select_columns(COLUMNS, ["device_name", "bogus"]) + + assert [c.key for c in columns] == ["device_name"] + assert _has_log(loguru_logs, "WARNING", "Column 'bogus' not found") + + +def test_no_valid_column_is_an_error(): + with pytest.raises(RenderError, match="Available columns: device_name"): + select_columns(COLUMNS, ["bogus"]) + + +def test_flatten_joins_nested_keys(): + record = { + "name": "Ethernet0", + "transceiver": {"vendor": "ACME", "dom": {"temperature": 31.5}}, + "altnames": ["a", "b"], + } + + assert flatten(record) == { + "name": "Ethernet0", + "transceiver_vendor": "ACME", + "transceiver_dom_temperature": 31.5, + "altnames": ["a", "b"], + } + assert flatten({"vendor": "X"}, "nic_") == {"nic_vendor": "X"} + + +@pytest.mark.parametrize( + "value,expected", + [ + (None, ""), + (True, "yes"), + (False, "no"), + (25000, "25000"), + (["a", None, 2], "a, , 2"), + ({"b": 1, "a": 2}, '{"a": 2, "b": 1}'), + ("text", "text"), + ], +) +def test_cell(value, expected): + assert cell(value) == expected + + +def test_sort_rows_uses_natural_order(): + rows = [ + {"device": "leaf-2", "interface": "Ethernet2"}, + {"device": "leaf-10", "interface": "Ethernet0"}, + {"device": "leaf-2", "interface": "Ethernet10"}, + {"device": "leaf-2", "interface": None}, + ] + + ordered = sort_rows(rows, "device", "interface") + + assert [(r["device"], r["interface"]) for r in ordered] == [ + ("leaf-2", None), + ("leaf-2", "Ethernet2"), + ("leaf-2", "Ethernet10"), + ("leaf-10", "Ethernet0"), + ] + + +def test_render_table_with_footer(): + text = render_table( + ROWS, select_columns(COLUMNS, long=True), ["Devices: 1", "Ports: 2"] + ) + + lines = text.splitlines() + assert lines[0].startswith("+--") + assert "| Device" in lines[1] and "Part Number" in lines[1] + # The serial-like value stays a string instead of becoming 100000.0 + assert "| 1E5 " in text + assert lines[-3:] == ["", "Devices: 1", "Ports: 2"] + + +def test_render_table_without_footer_and_rows(): + text = render_table([], COLUMNS[:2]) + + assert "Device" in text + assert not text.endswith("\n") + + +def test_render_csv_has_a_key_header_and_no_footer(): + text = render_csv(ROWS, select_columns(COLUMNS, long=True)) + + parsed = list(csv.reader(io.StringIO(text))) + assert parsed == [ + [ + "device_name", + "interface_name", + "transceiver_vendor", + "transceiver_part_number", + ], + ["leaf-1a", "Ethernet0", "ACME", "1E5"], + ["leaf-1a", "Ethernet4", "", ""], + ] + + +def test_render_csv_quotes_separators(): + text = render_csv([{"device_name": "a,b"}], COLUMNS[:1]) + + assert list(csv.reader(io.StringIO(text)))[1] == ["a,b"] + + +def test_render_json_round_trips(): + document = {"devices": [{"name": "leaf-1a", "interfaces": []}], "failed": []} + + assert json.loads(render_json(document)) == document + + +def test_write_output_prints_to_stdout(capsys): + write_output("report") + + assert capsys.readouterr().out == "report\n" + + +def test_write_output_writes_a_file_and_keeps_stdout_clean( + tmp_path, capsys, loguru_logs +): + path = tmp_path / "report.csv" + + write_output("a,b\n1,2", str(path)) + + assert path.read_text() == "a,b\n1,2\n" + assert capsys.readouterr().out == "" + assert _has_log(loguru_logs, "INFO", f"Report written to {path}")