Add update_baremetal_port ironic inspection hook

Author: stevekeay

python/ironic-understack/ironic_understack/tests/test_update_baremetal_port.py

@@ -0,0 +1,52 @@
+from ironic.objects import port as ironic_port
+from oslo_utils import uuidutils
+
+from ironic_understack.update_baremetal_port import UpdateBaremetalPortsHook
+
+_INTERFACE_1 = {
+    "name": "example1",
+    "mac_address": "11:11:11:11:11:11",
+    "ipv4_address": "1.1.1.1",
+    "lldp": [
+        (0, ""),
+        (1, "04885a92ec5459"),
+        (2, "0545746865726e6574312f3138"),
+        (3, "0078"),
+        (5, "6632302d332d32662e69616433"),
+    ],
+}
+
+_PLUGIN_DATA = {"all_interfaces": {"example1": _INTERFACE_1}}
+
+_INVENTORY = {"interfaces": [_INTERFACE_1]}
+
+
+def test_with_valid_data(mocker):
+    node_uuid = uuidutils.generate_uuid()
+    mock_node = mocker.Mock()
+    mock_task = mocker.Mock(node=mock_node)
+    mock_port = mocker.Mock(
+        uuid=uuidutils.generate_uuid(),
+        node_id=node_uuid,
+        address="11:11:11:11:11:11",
+        local_link_connection={},
+        physical_network="original_value",
+    )
+    mocker.patch(
+        "ironic_understack.update_baremetal_port.objects.port.Port.get_by_address",
+        return_value=mock_port,
+    )
+
+    UpdateBaremetalPortsHook().__call__(mock_task, _INVENTORY, _PLUGIN_DATA)
+
+    assert mock_port.local_link_connection == {
+        "port_id": "Ethernet1/18",
+        "switch_id": "88:5a:92:ec:54:59",
+        "switch_info": "f20-3-2f.iad3",
+    }
+    assert mock_port.physical_network == "f20-3-storage"
+
+    mock_port.save.assert_called()
+    mock_node.remove_trait.assert_not_called()
+    mock_node.add_trait.assert_called_once_with("CUSTOM_STORAGE_SWITCH")
+    mock_node.save.assert_called_once()

python/ironic-understack/ironic_understack/tests/test_vlan_group_name_convention.py

@@ -0,0 +1,45 @@
+import pytest
+
+from ironic_understack.vlan_group_name_convention import vlan_group_name
+
+
+def test_vlan_group_name_valid_switches():
+    assert vlan_group_name("a1-1-1") == "a1-1-network"
+    assert vlan_group_name("a1-2-1") == "a1-2-network"
+    assert vlan_group_name("b12-1") == "b12-network"
+    assert vlan_group_name("a2-12-1") == "a2-12-network"
+    assert vlan_group_name("a2-12-2") == "a2-12-network"
+    assert vlan_group_name("a2-12-1f") == "a2-12-storage"
+    assert vlan_group_name("a2-12-2f") == "a2-12-storage"
+    assert vlan_group_name("a2-12-3f") == "a2-12-storage-appliance"
+    assert vlan_group_name("a2-12-4f") == "a2-12-storage-appliance"
+    assert vlan_group_name("a2-12-1d") == "a2-12-bmc"
+
+
+def test_vlan_group_name_with_domain():
+    assert vlan_group_name("a2-12-1.iad3.rackspace.net") == "a2-12-network"
+    assert vlan_group_name("a2-12-1f.lon3.rackspace.net") == "a2-12-storage"
+
+
+def test_vlan_group_name_case_insensitive():
+    assert vlan_group_name("A2-12-1F") == "a2-12-storage"
+    assert vlan_group_name("A2-12-1") == "a2-12-network"
+
+
+def test_vlan_group_name_invalid_format():
+    with pytest.raises(ValueError, match="Unknown switch name format"):
+        vlan_group_name("invalid")
+
+    with pytest.raises(ValueError, match="Unknown switch name format"):
+        vlan_group_name("")
+
+
+def test_vlan_group_name_unknown_suffix():
+    with pytest.raises(ValueError, match="Unknown switch suffix"):
+        vlan_group_name("a2-12-99")
+
+    with pytest.raises(ValueError, match="Unknown switch suffix"):
+        vlan_group_name("a2-12-5f")
+
+    with pytest.raises(ValueError, match="Unknown switch suffix"):
+        vlan_group_name("a2-12-xyz")

python/ironic-understack/ironic_understack/update_baremetal_port.py

@@ -0,0 +1,178 @@
+import binascii
+from typing import Any
+
+import netaddr
+import openstack
+from construct import core
+from ironic import objects
+from ironic.common import exception
+from ironic.drivers.modules.inspector import lldp_tlvs
+from ironic.drivers.modules.inspector.hooks import base
+from oslo_log import log as logging
+
+import ironic_understack.vlan_group_name_convention
+
+LOG = logging.getLogger(__name__)
+
+
+class UpdateBaremetalPortsHook(base.InspectionHook):
+    """Hook to update ports according to LLDP data."""
+
+    dependencies = ["validate-interfaces"]
+
+    def __call__(self, task, inventory, plugin_data):
+        """Update Ports' local_link_info and physnet based on LLDP data.
+
+        Process the LLDP packet fields for each NIC in the inventory.
+
+        Updates attributes of the baremetal port:
+
+        - local_link_info.port_id (e.g. "Ethernet1/1")
+        - local_link_info.switch_id (e.g. "aa:bb:cc:dd:ee:ff")
+        - local_link_info.switch_info (e.g. "a1-1-1.ord1")
+        - physical_network (e.g. "a1-1-network")
+
+        Also adds or removes node "traits" based on the inventory data.  We
+        control the trait "CUSTOM_STORAGE_SWITCH".
+        """
+        lldp_raw = plugin_data.get("lldp_raw") or {}
+        node_uuid = task.node.id
+        interfaces = inventory["interfaces"]
+        # The all_interfaces field in plugin_data is provided by the
+        # validate-interfaces hook, so it is a dependency for this hook
+        all_interfaces = plugin_data["all_interfaces"]
+        context = task.context
+        vlan_groups = set()
+
+        for iface in interfaces:
+            if iface["name"] not in all_interfaces:
+                # This interface was not "validated" so don't bother with it
+                continue
+
+            mac_address = iface["mac_address"]
+            port = objects.port.Port.get_by_address(context, mac_address)
+            if not port:
+                LOG.debug(
+                    "Skipping LLDP processing for interface %s of node "
+                    "%s: matching port not found in Ironic.",
+                    mac_address,
+                    node_uuid,
+                )
+                continue
+
+            lldp_data = lldp_raw.get(iface["name"]) or iface.get("lldp")
+            if lldp_data is None:
+                LOG.warning(
+                    "No LLDP data found for interface %s of node %s",
+                    mac_address,
+                    node_uuid,
+                )
+                continue
+
+            local_link_connection = _parse_lldp(lldp_data, node_uuid)
+            vlan_group = vlan_group_name(local_link_connection)
+
+            _set_local_link_connection(port, node_uuid, local_link_connection)
+            _update_port_physical_network(port, vlan_group)
+            vlan_groups.add(vlan_group)
+        _update_node_traits(task, vlan_groups)
+
+
+def _set_local_link_connection(port: Any, node_uuid: str, local_link_connection: dict):
+    try:
+        LOG.debug("Updating port %s for node %s", port.address, node_uuid)
+        port.local_link_connection = local_link_connection
+        port.save()
+    except exception.IronicException as e:
+        LOG.warning(
+            "Failed to update port %(uuid)s for node %(node)s. Error: %(error)s",
+            {"uuid": port.id, "node": node_uuid, "error": e},
+        )
+
+
+def _parse_lldp(lldp_data: list, node_id: str) -> dict[str, str]:
+    """Convert Ironic's "lldp_raw" format to local_link dict."""
+    try:
+        decoded = {}
+        for tlv_type, tlv_value in lldp_data:
+            if tlv_type not in decoded:
+                decoded[tlv_type] = []
+            decoded[tlv_type].append(bytearray(binascii.unhexlify(tlv_value)))
+
+        return {
+            "port_id": _extract_port_id(decoded),
+            "switch_id": _extract_switch_id(decoded),
+            "switch_info": _extract_hostname(decoded),
+        }
+    except (binascii.Error, core.MappingError, netaddr.AddrFormatError) as e:
+        LOG.warning("Failed to parse lldp_raw data for Node %s: %s", node_id, e)
+    return {}
+
+
+def _extract_port_id(data: dict) -> str | None:
+    for value in data.get(lldp_tlvs.LLDP_TLV_PORT_ID, []):
+        parsed = lldp_tlvs.PortId.parse(value)
+        if parsed.value:  # pyright: ignore reportAttributeAccessIssue
+            return parsed.value.value  # pyright: ignore reportAttributeAccessIssue
+
+
+def _extract_switch_id(data: dict) -> str | None:
+    for value in data.get(lldp_tlvs.LLDP_TLV_CHASSIS_ID, []):
+        parsed = lldp_tlvs.ChassisId.parse(value)
+        if "mac_address" in parsed.subtype:  # pyright: ignore reportAttributeAccessIssue
+            return str(parsed.value.value)  # pyright: ignore reportAttributeAccessIssue
+
+
+def _extract_hostname(data: dict) -> str | None:
+    for value in data.get(lldp_tlvs.LLDP_TLV_SYS_NAME, []):
+        parsed = lldp_tlvs.SysName.parse(value)
+        if parsed.value:  # pyright: ignore reportAttributeAccessIssue
+            return parsed.value  # pyright: ignore reportAttributeAccessIssue
+
+
+def vlan_group_name(local_link_connection) -> str | None:
+    switch_name = local_link_connection.get("switch_info")
+    if not switch_name:
+        return
+
+    return ironic_understack.vlan_group_name_convention.vlan_group_name(switch_name)
+
+
+def _update_port_physical_network(port, new_physical_network: str|None):
+    old_physical_network = port.physical_network
+
+    if new_physical_network == old_physical_network:
+        return
+
+    LOG.debug(
+        "Updating port %s physical_network from %s to %s",
+        port.id,
+        old_physical_network,
+        new_physical_network,
+    )
+    port.physical_network = new_physical_network
+    port.save()
+
+
+def _update_node_traits(task, vlan_groups: set[str]):
+    """Add or remove traits to the node.
+
+    We manage one trait: "CUSTOM_STORAGE_SWITCH" which is added if the node has
+    any ports connected to a storage fabric, othwise it is removed from the
+    node.
+    """
+    TRAIT_STORAGE_SWITCH = "CUSTOM_STORAGE_SWITCH"
+
+    storage_vlan_groups = {
+        x for x in vlan_groups if x.endswith("-storage")
+    }
+
+    if storage_vlan_groups:
+        task.node.add_trait(TRAIT_STORAGE_SWITCH)
+    else:
+        try:
+            task.node.remove_trait(TRAIT_STORAGE_SWITCH)
+        except openstack.exceptions.NotFoundException:
+            pass
+
+    task.node.save()

python/ironic-understack/ironic_understack/vlan_group_name_convention.py

@@ -0,0 +1,54 @@
+def vlan_group_name(switch_name: str) -> str:
+    """The VLAN GROUP name is a function of the switch name.
+
+    Switch hostname convention is site-dependent, but in Rackspace all
+    top-of-rack switch names follow the convention: <cabinet-name>-<suffix>
+    Example switch names include a2-12-1f and a2-12-1.  (These are normally
+    qualified with a site-specific domain name like a2-12-1.iad3.rackspace.net,
+    but we are only considering the unqualified name, ignoring everything after
+    the first dot).
+
+    It easy to parse the switch name into cabinet and suffix.  Convert the
+    switch-name-suffix to vlan-group-suffix using the following mapping:
+
+        1 → network
+        2 → network
+        1f → storage
+        2f → storage
+        3f → storage-appliance
+        4f → storage-appliance
+        1d → bmc
+
+    The VLAN GROUP name results from joining the cabinet name to the new suffix
+    with a hyphen. The result is in lower case: <cabinet-name>-<vlan-group-suffix>
+
+    So for example, switch a2-12-1 is in VLAN GROUP a2-12-network.
+    """
+    # Remove domain suffix if present (everything after first dot)
+    switch_name = switch_name.split(".")[0].lower()
+
+    # Split into cabinet and suffix (last component after last hyphen)
+    parts = switch_name.rsplit("-", 1)
+    if len(parts) != 2:
+        raise ValueError(f"Unknown switch name format: {switch_name}")
+
+    cabinet_name, suffix = parts
+
+    # Map suffix to VLAN group suffix
+    suffix_mapping = {
+        "1": "network",
+        "2": "network",
+        "3": "network",
+        "4": "network",
+        "1f": "storage",
+        "2f": "storage",
+        "3f": "storage-appliance",
+        "4f": "storage-appliance",
+        "1d": "bmc",
+    }
+
+    vlan_suffix = suffix_mapping.get(suffix)
+    if vlan_suffix is None:
+        raise ValueError(f"Unknown switch suffix: {suffix}")
+
+    return f"{cabinet_name}-{vlan_suffix}"

python/ironic-understack/pyproject.toml

@@ -12,6 +12,7 @@ readme = "README.md"
 license = "MIT"
 dependencies = [
     "ironic>=24.1",
+    "pytest-mock>=3.15.1",
     "pyyaml~=6.0",
     "understack-flavor-matcher",
 ]
@@ -20,6 +21,7 @@ dependencies = [
 resource-class = "ironic_understack.resource_class:UndercloudResourceClassHook"
 
 [project.entry-points."ironic.hardware.interfaces.inspect"]
+update-baremetal-port = "ironic_understack.update_baremetal_port:UpdateBaremetalPortsHook"
 redfish-understack = "ironic_understack.redfish_inspect_understack:UnderstackRedfishInspect"
 idrac-redfish-understack = "ironic_understack.redfish_inspect_understack:UnderstackDracRedfishInspect"