diff --git a/python/understack-workflows/tests/test_nautobot_device_sync.py b/python/understack-workflows/tests/test_nautobot_device_sync.py index 806041a01..d524be1e2 100644 --- a/python/understack-workflows/tests/test_nautobot_device_sync.py +++ b/python/understack-workflows/tests/test_nautobot_device_sync.py @@ -509,7 +509,7 @@ def test_extra_overrides_switch_lookup(self, mock_nautobot, mock_rack, location) MagicMock(local_link_connection={"switch_info": "switch1.example.com"}) ] - device_info, _, _ = fetch_node_details( + device_info, _, _, _ = fetch_node_details( "test-uuid", ironic_client, mock_nautobot, location ) @@ -838,7 +838,7 @@ def test_sync_creates_new_device( location_id="location-uuid", status="Active", ) - mock_fetch.return_value = (device_info, {}, []) + mock_fetch.return_value = (device_info, {}, [], MagicMock()) mock_nautobot.dcim.devices.get.return_value = None mock_nautobot.dcim.devices.create.return_value = MagicMock() mock_sync_interfaces.return_value = EXIT_STATUS_SUCCESS @@ -867,7 +867,7 @@ def test_sync_updates_existing_device( name="Dell-ABC123", status="Active", ) - mock_fetch.return_value = (device_info, {}, []) + mock_fetch.return_value = (device_info, {}, [], MagicMock()) existing_device = MagicMock() existing_device.status = MagicMock(name="Planned") @@ -898,7 +898,7 @@ def test_sync_without_location_skips_for_uninspected_node( """Test that sync skips gracefully for uninspected nodes without location.""" node_uuid = str(uuid.uuid4()) device_info = DeviceInfo(uuid=node_uuid) # No location - mock_fetch.return_value = (device_info, {}, []) + mock_fetch.return_value = (device_info, {}, [], MagicMock()) mock_nautobot.dcim.devices.get.return_value = None result = sync_device_to_nautobot(node_uuid, mock_nautobot, location) @@ -932,7 +932,7 @@ def test_sync_recreates_device_with_mismatched_uuid( location_id="location-uuid", status="Active", ) - mock_fetch.return_value = (device_info, {}, []) + mock_fetch.return_value = (device_info, {}, [], MagicMock()) # First get by ID returns None # Second get by name returns device with different UUID @@ -976,7 +976,7 @@ def test_sync_device_not_found_by_name_creates_new( location_id="location-uuid", status="Active", ) - mock_fetch.return_value = (device_info, {}, []) + mock_fetch.return_value = (device_info, {}, [], MagicMock()) # Both lookups return None mock_nautobot.dcim.devices.get.side_effect = [None, None] @@ -1016,7 +1016,7 @@ def test_sync_uuid_mismatch_uses_old_device_location( # No location_id from switch lookup status="Active", ) - mock_fetch.return_value = (device_info, {}, []) + mock_fetch.return_value = (device_info, {}, [], MagicMock()) # Old device has location existing_device = MagicMock() diff --git a/python/understack-workflows/understack_workflows/oslo_event/nautobot_device_interface_sync.py b/python/understack-workflows/understack_workflows/oslo_event/nautobot_device_interface_sync.py index 1cb13c82c..fbd3fe09f 100644 --- a/python/understack-workflows/understack_workflows/oslo_event/nautobot_device_interface_sync.py +++ b/python/understack-workflows/understack_workflows/oslo_event/nautobot_device_interface_sync.py @@ -206,39 +206,48 @@ def _assign_ip_to_interface( logger.warning("Failed to associate IP %s with interface: %s", ip_address, e) -def sync_idrac_interface( +def sync_management_interface( device_uuid: str, - bmc_mac: str, + interface_name: str, + mac: str, nautobot_client: Nautobot, - bmc_ip: str | None = None, + ip: str | None = None, + description: str | None = None, ) -> None: - """Sync iDRAC interface to Nautobot. + """Sync management interface (iDRAC, management, etc.) to Nautobot. - Creates or updates the iDRAC management interface for a device. + Creates or updates a management interface for a device. Looks up existing interface by name + device_id. - Optionally assigns the BMC IP address to the interface. + Optionally assigns an IP address to the interface. - TODO: Add cable management for iDRAC. Currently not implemented because - LLDP data for iDRAC switch connection is not available in Ironic inventory. - Would require querying the BMC directly via Redfish (see bmc_chassis_info.py). + TODO: Add cable management for management interfaces. Currently not implemented + because LLDP data for management switch connections is not always available in + Ironic. Would require additional discovery or manual specification. Args: device_uuid: Nautobot device UUID - bmc_mac: BMC MAC address from inventory + interface_name: Interface name (e.g., "iDRAC", "management") + mac: MAC address nautobot_client: Nautobot API client - bmc_ip: Optional BMC IP address from inventory (bmc_address) + ip: Optional IP address to assign + description: Optional interface description """ - if not bmc_mac: - logger.debug("No bmc_mac provided for device %s", device_uuid) + if not mac: + logger.debug( + "No MAC provided for %s interface on device %s", interface_name, device_uuid + ) return - mac_address = bmc_mac.upper() - idrac_interface = None + mac_address = mac.upper() + mgmt_interface = None + + if not description: + description = f"Management interface ({interface_name})" - # Check if iDRAC interface already exists + # Check if interface already exists existing = nautobot_client.dcim.interfaces.get( device_id=device_uuid, - name="iDRAC", + name=interface_name, ) # pynautobot.get() can return Record, list, or None - we expect a single Record @@ -251,33 +260,67 @@ def sync_idrac_interface( if current_mac != mac_address: existing.mac_address = mac_address # type: ignore[attr-defined] existing.save() # type: ignore[union-attr] - logger.info("Updated iDRAC MAC for device %s: %s", device_uuid, mac_address) + logger.info( + "Updated %s MAC for device %s: %s", + interface_name, + device_uuid, + mac_address, + ) else: logger.debug( - "iDRAC interface already up to date for device %s", device_uuid + "%s interface already up to date for device %s", + interface_name, + device_uuid, ) - idrac_interface = existing + mgmt_interface = existing else: - # Create new iDRAC interface - idrac_interface = nautobot_client.dcim.interfaces.create( + # Create new management interface + mgmt_interface = nautobot_client.dcim.interfaces.create( device=device_uuid, - name="iDRAC", + name=interface_name, type="1000base-t", mac_address=mac_address, - description="Dedicated iDRAC interface", + description=description, mgmt_only=True, enabled=True, status="Active", ) logger.info( - "Created iDRAC interface for device %s: %s", device_uuid, mac_address + "Created %s interface for device %s: %s", + interface_name, + device_uuid, + mac_address, ) - # Assign BMC IP address to iDRAC interface - if idrac_interface and bmc_ip: - idrac_id = getattr(idrac_interface, "id", None) - if idrac_id: - _assign_ip_to_interface(nautobot_client, idrac_id, bmc_ip) + # Assign IP address to interface + if mgmt_interface and ip: + mgmt_id = getattr(mgmt_interface, "id", None) + if mgmt_id: + _assign_ip_to_interface(nautobot_client, mgmt_id, ip) + + +def sync_idrac_interface( + device_uuid: str, + bmc_mac: str, + nautobot_client: Nautobot, + bmc_ip: str | None = None, +) -> None: + """Sync iDRAC interface to Nautobot (wrapper for sync_management_interface). + + Args: + device_uuid: Nautobot device UUID + bmc_mac: BMC MAC address from inventory + nautobot_client: Nautobot API client + bmc_ip: Optional BMC IP address from inventory (bmc_address) + """ + sync_management_interface( + device_uuid=device_uuid, + interface_name="iDRAC", + mac=bmc_mac, + nautobot_client=nautobot_client, + ip=bmc_ip, + description="Dedicated iDRAC interface", + ) def _build_interfaces_from_ports( @@ -396,8 +439,8 @@ def _cleanup_stale_interfaces( intf_name = getattr(intf, "name", None) intf_id = getattr(intf, "id", None) - # Skip iDRAC - it's managed separately and not in Ironic ports - if intf_name == "iDRAC": + # Skip management interfaces - managed separately, not in Ironic ports + if intf_name in ("iDRAC", "management"): continue if intf_id not in valid_interface_ids: @@ -562,6 +605,7 @@ def sync_interfaces_from_data( inventory: dict, ports: list, nautobot_client: Nautobot, + node=None, ) -> int: """Sync interfaces to Nautobot using pre-fetched inventory and ports. @@ -573,6 +617,7 @@ def sync_interfaces_from_data( inventory: Ironic node inventory dict (from get_node_inventory) ports: List of Ironic port objects (from list_ports) nautobot_client: Nautobot API client + node: Optional Ironic node object (for accessing extra fields) Returns: EXIT_STATUS_SUCCESS on success, EXIT_STATUS_FAILURE on failure @@ -608,6 +653,26 @@ def sync_interfaces_from_data( if bmc_mac: sync_idrac_interface(node_uuid, bmc_mac, nautobot_client, bmc_ip) + # Sync management interface for netdev appliances (firewalls, etc.) + # Management data comes from node.driver_info for netdev devices + if node: + driver_info = getattr(node, "driver_info", {}) or {} + mgmt_ip = driver_info.get("management_ip") + + # MAC might be in driver_info or extra - check both + extra = getattr(node, "extra", {}) or {} + mgmt_mac = driver_info.get("management_mac") or extra.get("mgmt_mac") + + if mgmt_mac: + sync_management_interface( + device_uuid=node_uuid, + interface_name="management", + mac=mgmt_mac, + nautobot_client=nautobot_client, + ip=mgmt_ip, + description="Management interface", + ) + # Cleanup stale interfaces no longer in Ironic valid_ids = {intf.uuid for intf in interfaces} _cleanup_stale_interfaces(node_uuid, valid_ids, nautobot_client) diff --git a/python/understack-workflows/understack_workflows/oslo_event/nautobot_device_sync.py b/python/understack-workflows/understack_workflows/oslo_event/nautobot_device_sync.py index e77b676f7..8efcd5269 100644 --- a/python/understack-workflows/understack_workflows/oslo_event/nautobot_device_sync.py +++ b/python/understack-workflows/understack_workflows/oslo_event/nautobot_device_sync.py @@ -45,6 +45,62 @@ class InvalidRackPositionError(ValueError): """Raised when an explicit rack position cannot be applied safely.""" +def _is_netdev_appliance(node) -> bool: + """Check if node is a network appliance (netdev driver). + + Network appliances (firewalls, load balancers, network security devices) + use the netdev driver and require different handling than servers: + - No hardware inventory (no inspection process yet for most) + - Manufacturer/model come from node properties, not inventory + - Device role determined by device type (firewall, load balancer, etc.) + """ + return getattr(node, "driver", None) == "netdev" + + +def _determine_device_role(node) -> str: + """Determine Nautobot device role from node properties. + + For servers: role is 'server' + For netdev appliances: infer from model or resource_class + - PA-* models → 'firewall' + - F5-* models → 'load balancer' (future) + - rubrik-* resource_class → 'backup appliance' (future) + Default to 'firewall' for netdev if can't determine + """ + if not _is_netdev_appliance(node): + return "server" + + # Try to determine from properties.model + props = node.properties or {} + model = props.get("model", "") + + if model.startswith("PA-"): + return "firewall" + elif model.startswith("F5-"): + return "load balancer" + + # Try from resource_class as fallback + rc = getattr(node, "resource_class", "") + if rc.startswith("pa"): + return "firewall" + elif rc.startswith("f5"): + return "load balancer" + elif "rubrik" in rc.lower(): + return "backup appliance" + + # Default for netdev + logger.warning( + ( + "Could not determine role for netdev node %s (model=%s, rc=%s), " + "defaulting to 'firewall'" + ), + getattr(node, "uuid", "unknown"), + model, + rc, + ) + return "firewall" + + def _is_retryable_error(exc: BaseException) -> bool: """Determine if an exception is retryable. @@ -155,14 +211,16 @@ def _normalise_manufacturer(name: str) -> str: return "Dell" elif "HP" in name.upper(): return "HPE" - raise ValueError(f"Server manufacturer {name} not supported") + elif "PALO ALTO" in name.upper(): + return "Palo Alto" + raise ValueError(f"Manufacturer {name} not supported") def _populate_from_node(device_info: DeviceInfo, node) -> None: """Populate device info from Ironic node object.""" props = node.properties or {} - # Hardware specs + # Hardware specs (servers only - netdev appliances have empty properties) if props.get("memory_mb"): device_info.memory_mb = int(props["memory_mb"]) if props.get("cpus"): @@ -175,6 +233,9 @@ def _populate_from_node(device_info: DeviceInfo, node) -> None: if hasattr(node, "traits") and node.traits: device_info.traits = list(node.traits) + # Device role - determine from driver and properties + device_info.role = _determine_device_role(node) + # Provision state -> Nautobot status device_info.status = ProvisionStateMapper.translate_to_nautobot( node.provision_state @@ -220,6 +281,58 @@ def _populate_from_inventory(device_info: DeviceInfo, inventory: dict | None) -> device_info.serial_number = system_vendor.get("serial_number") +def _populate_from_netdev_properties(device_info: DeviceInfo, node) -> None: + """Populate device info from node properties for netdev appliances. + + Network appliances (firewalls, load balancers, etc.) are enrolled with + manufacturer, model, and serial set in node properties and extra fields + during enrollment. They do not currently have an inspection process. + + Future work: Build an inspect process for netdev devices that would fetch + device info directly (e.g., via API or SSH) and populate these same fields, + similar to how servers use Redfish inspection. + """ + props = node.properties or {} + extra = node.extra or {} + + # Manufacturer from properties.vendor + vendor = props.get("vendor") + if vendor: + device_info.manufacturer = vendor + else: + # Infer from model if it starts with known prefix + model = props.get("model", "") + if model.startswith("PA-"): + device_info.manufacturer = "Palo Alto" + logger.debug( + "[node:%s] Inferred manufacturer 'Palo Alto' from model '%s'", + device_info.uuid, + model, + ) + else: + logger.warning( + ( + "[node:%s] Could not determine manufacturer from properties " + "(vendor=%s, model=%s)" + ), + device_info.uuid, + vendor, + model, + ) + + # Model from properties + if props.get("model"): + device_info.model = props.get("model") + + # Serial from extra (netdev devices store it there during enrollment) + if extra.get("serial"): + device_info.serial_number = extra.get("serial") + + # HA mate serial for firewalls (informational only until proper HA design exists) + if extra.get("mate_serial"): + device_info.custom_fields["mate_serial"] = str(extra["mate_serial"]) + + def _generate_device_name(device_info: DeviceInfo) -> None: """Generate device name from manufacturer and serial number.""" if device_info.manufacturer and device_info.serial_number: @@ -386,7 +499,7 @@ def fetch_node_details( ironic_client: IronicClient, nautobot_client: Nautobot, location: Record, -) -> tuple[DeviceInfo, dict, list]: +) -> tuple[DeviceInfo, dict, list, object]: """Fetch complete device info from Ironic. Args: @@ -397,31 +510,46 @@ def fetch_node_details( location-scoped Nautobot lookups Returns: - Tuple of (DeviceInfo, inventory dict, ports list) + Tuple of (DeviceInfo, inventory dict, ports list, node object) """ device_info = DeviceInfo(uuid=node_uuid) node = ironic_client.get_node(node_uuid) - # Inventory may not exist yet for newly created nodes (pre-inspection) + # Inventory only exists for servers that have been inspected + # Netdev appliances (firewalls, etc.) don't have inventory try: inventory = ironic_client.get_node_inventory(node_ident=node_uuid) except ironic_exceptions.NotFound: - logger.info("No inventory yet for node %s (not inspected)", node_uuid) + if _is_netdev_appliance(node): + logger.debug( + "[node:%s] No inventory for netdev appliance (expected)", node_uuid + ) + else: + logger.info( + "[node:%s] No inventory yet for server (not inspected)", node_uuid + ) inventory = {} ports = ironic_client.list_ports(node_id=node_uuid) # Populate in order _populate_from_node(device_info, node) - _populate_from_inventory(device_info, inventory) + + # Use different population method based on device type + if _is_netdev_appliance(node): + _populate_from_netdev_properties(device_info, node) + else: + _populate_from_inventory(device_info, inventory) + _generate_device_name(device_info) + # Prefer an explicit rack/position from the node's extra field; fall back # to deriving location from the connected switches when it isn't set. if not _set_location_from_extra(device_info, node, nautobot_client, location): _set_location_from_switches(device_info, ports, nautobot_client) - return device_info, inventory, ports + return device_info, inventory, ports, node def _create_nautobot_device(device_info: DeviceInfo, nautobot_client: Nautobot): @@ -726,7 +854,7 @@ def sync_device_to_nautobot( try: ironic_client = IronicClient() - ironic_node_info, inventory, ports = fetch_node_details( + ironic_node_info, inventory, ports, node = fetch_node_details( node_uuid, ironic_client, nautobot_client, location ) @@ -738,7 +866,7 @@ def sync_device_to_nautobot( if sync_interfaces: interface_result = sync_interfaces_from_data( - node_uuid, inventory, ports, nautobot_client + node_uuid, inventory, ports, nautobot_client, node ) if interface_result != EXIT_STATUS_SUCCESS: logger.warning(