From ec35abbd9540e2f46df17d6f8ae04cade03da9fe Mon Sep 17 00:00:00 2001 From: Steve Keay Date: Tue, 26 Nov 2024 10:23:21 +0000 Subject: [PATCH 1/4] Improve diagnostics around Nautobot duplicate IP address assignments --- .../understack_workflows/nautobot_device.py | 52 +++++++++++++------ 1 file changed, 37 insertions(+), 15 deletions(-) diff --git a/python/understack-workflows/understack_workflows/nautobot_device.py b/python/understack-workflows/understack_workflows/nautobot_device.py index 4d8aa331f..b27090afb 100644 --- a/python/understack-workflows/understack_workflows/nautobot_device.py +++ b/python/understack-workflows/understack_workflows/nautobot_device.py @@ -54,7 +54,7 @@ def find_or_create(chassis_info: ChassisInfo, nautobot) -> NautobotDevice: # TODO: performance: our single graphql query here fetches the device from # nautobot with all existing interfaces, macs, cable and connected switches. # We then query some of those items again, which adds unnecessary - # round-trips to the DRAC. + # round-trips to Nautobot and/or DRAC. # # TODO: delete any extra items from nautobot (however we don't want to # delete cables that temporarily went down). @@ -396,13 +396,17 @@ def connect_interface_to_switch( def assign_ip_address(nautobot, nautobot_interface, ipv4_address: IPv4Interface, mac): + """Find or create IP Address in Nautobot IPAM. + + If the existing IP address is a "dhcp" type then upgrade it to a "host" type. + """ try: ip = nautobot.ipam.ip_addresses.get(address=str(ipv4_address.ip)) if ip and ip.type == "dhcp" and ip.custom_fields.get("pydhcp_mac") == mac: - # Make our DHCP assignment permanent: + logger.info(f"Making DHCP lease permanent in Nautobot {dict(ip)}") ip.update(type="host", cf_pydhcp_expire=None) elif ip: - logger.info(f"Nautobot IP already exists! {dict(ip)}") + logger.info(f"{ipv4_address} exists as {ip.id} in Nautobot IPAM") else: ip = nautobot.ipam.ip_addresses.create( address=str(ipv4_address.ip), @@ -419,17 +423,35 @@ def assign_ip_address(nautobot, nautobot_interface, ipv4_address: IPv4Interface, def associate_ip_address(nautobot, nautobot_interface, ip_id): - identity = { - "ip_address": ip_id, - "interface": nautobot_interface.id, - } - try: - if nautobot.ipam.ip_address_to_interface.get(**identity): - logger.info(f"IP address {ip_id} is already on {nautobot_interface.name}") - else: - nautobot.ipam.ip_address_to_interface.create(**identity, is_primary=True) - logger.info(f"Associated IP address {ip_id} with {nautobot_interface.name}") - except pynautobot.core.query.RequestError as e: # type: ignore + """Associate a given IP Address with a given Interface in Nautobot IPAM. + + If the IP Address is already associated with some other Interface then an + Exception is raised. + + If the Interface is already associated to some other IP addres then an + Exception is raised. + """ + existing_association = nautobot.ipam.ip_address_to_interface.get(ip_address=ip_id) + + if existing_association is None: + try: + nautobot.ipam.ip_address_to_interface.create( + ip_address=ip_id, + interface=nautobot_interface.id, + is_primary=True + ) + except pynautobot.core.query.RequestError as e: # type: ignore + raise Exception( + f"Failed to associate IPAddress {ip_id} in Nautobot: {e}" + ) from None + logger.info(f"Associated IP address {ip_id} with {nautobot_interface.name}") + elif existing_association.interface == nautobot_interface.id: + logger.info( + f"IP address {ip_id} already correct on {nautobot_interface.name}" + ) + else: raise Exception( - f"Failed to associate IPAddress {ip_id} in Nautobot: {e}" + f"Failed to document discovered server in Nautobot - IP Address " + f"{ip_id} is already associated with some other interface: " + f"{existing_association.interface.id=} {existing_association.display}" ) from None From ac672d009bca939d36b4682cfe2c17174b93ea39 Mon Sep 17 00:00:00 2001 From: Steve Keay Date: Tue, 26 Nov 2024 13:52:30 +0000 Subject: [PATCH 2/4] Refactor: improve type signatures --- .../understack_workflows/nautobot_device.py | 28 +++++++++++++------ 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/python/understack-workflows/understack_workflows/nautobot_device.py b/python/understack-workflows/understack_workflows/nautobot_device.py index b27090afb..b12bcd69f 100644 --- a/python/understack-workflows/understack_workflows/nautobot_device.py +++ b/python/understack-workflows/understack_workflows/nautobot_device.py @@ -1,6 +1,7 @@ import re from dataclasses import dataclass from ipaddress import IPv4Interface +from typing import Any import pynautobot @@ -110,7 +111,7 @@ def location_from(switches): return next(iter(locations)) -def switches_for(nautobot, chassis_info: ChassisInfo) -> dict: +def switches_for(nautobot, chassis_info: ChassisInfo) -> dict[str, dict]: """Get all possible switches from the discovered LLDP neighbor information. We search for two possible mac addresses for each neighbor because some @@ -167,16 +168,19 @@ def nautobot_switches(nautobot, mac_addresses: set[str]) -> dict[str, dict]: return {switch["mac"]: switch for switch in switches} -def nautobot_switch(all_switches, interface): +def nautobot_switch(all_switches: dict[str, Any], interface: InterfaceInfo): + if not interface.remote_switch_mac_address or not interface.remote_switch_port_name: + raise ValueError(f"missing LDLDP info in {interface}") + mac_address = interface.remote_switch_mac_address base_mac_address = base_mac(mac_address, interface.remote_switch_port_name) switch = all_switches.get(mac_address, all_switches.get(base_mac_address)) if not switch: raise Exception( - f"Looking for a switch in nautobot that matches the LLDP " - f"info reported by server BMC - " - f"No device in nautobot with chassis_mac_address {mac_address}, " - f"nor the calculated base mac address {base_mac_address}." + f"There is no switch Device in nautobot that matches the LLDP info " + f"reported by server BMC for {interface} - I was looking for " + f"chassis_mac_address= {mac_address}, or the calculated base mac " + f"chassis_mac_address= {base_mac_address}." ) return switch @@ -197,7 +201,9 @@ def base_mac(mac: str, port_name: str) -> str: return ":".join(hexadecimal[i : i + 2] for i in range(0, 12, 2)) -def server_device_payload(location_id, rack_id, chassis_info): +def server_device_payload( + location_id: str, rack_id: str, chassis_info: ChassisInfo +) -> dict: manufacturer = _parse_manufacturer(chassis_info.manufacturer) name = f"{manufacturer}-{chassis_info.serial_number}" @@ -298,14 +304,18 @@ def parse_interface(data: dict) -> NautobotInterface: ) -def find_or_create_interfaces(nautobot, chassis_info: ChassisInfo, device_id, switches): +def find_or_create_interfaces( + nautobot, chassis_info: ChassisInfo, device_id, switches: dict[str, dict] +): """Update Nautobot Device Interfaces using the Nautobot API.""" for interface in chassis_info.interfaces: if interface.mac_address: setup_nautobot_interface(nautobot, interface, device_id, switches) -def setup_nautobot_interface(nautobot, interface: InterfaceInfo, device_id, switches): +def setup_nautobot_interface( + nautobot, interface: InterfaceInfo, device_id, switches: dict[str, dict] +): nautobot_int = find_or_create_interface(nautobot, interface, device_id) if interface.ipv4_address: From eda522cf00559e696729f2a71dd0d464bcd54133 Mon Sep 17 00:00:00 2001 From: Steve Keay Date: Tue, 26 Nov 2024 21:31:48 +0000 Subject: [PATCH 3/4] Log a note if the LLDP neighbor data is reported stale by the BMC --- .../understack_workflows/bmc_chassis_info.py | 4 ++++ .../understack_workflows/nautobot_device.py | 3 +++ 2 files changed, 7 insertions(+) diff --git a/python/understack-workflows/understack_workflows/bmc_chassis_info.py b/python/understack-workflows/understack_workflows/bmc_chassis_info.py index ce425abdc..a78a5d21f 100644 --- a/python/understack-workflows/understack_workflows/bmc_chassis_info.py +++ b/python/understack-workflows/understack_workflows/bmc_chassis_info.py @@ -21,6 +21,7 @@ class InterfaceInfo: dhcp: bool = False remote_switch_mac_address: str | None = None remote_switch_port_name: str | None = None + remote_switch_data_stale: bool = False @dataclass(frozen=True) @@ -232,16 +233,19 @@ def parse_lldp_port(port_data: dict[str, str]) -> dict: """ mac = str(port_data["SwitchConnectionID"]).upper() port_name = normalize_interface_name(port_data["SwitchPortConnectionID"]) + stale = str(port_data["StaleData"]) != "NotStale" if mac in ["NOT AVAILABLE", "NO LINK", "NOT SUPPORTED"]: return { "remote_switch_mac_address": None, "remote_switch_port_name": None, + "remote_switch_data_stale": stale, } else: return { "remote_switch_mac_address": mac, "remote_switch_port_name": port_name, + "remote_switch_data_stale": stale, } diff --git a/python/understack-workflows/understack_workflows/nautobot_device.py b/python/understack-workflows/understack_workflows/nautobot_device.py index b12bcd69f..86946cc6a 100644 --- a/python/understack-workflows/understack_workflows/nautobot_device.py +++ b/python/understack-workflows/understack_workflows/nautobot_device.py @@ -169,6 +169,9 @@ def nautobot_switches(nautobot, mac_addresses: set[str]) -> dict[str, dict]: def nautobot_switch(all_switches: dict[str, Any], interface: InterfaceInfo): + if interface.remote_switch_data_stale: + logger.info(f"Warning: BMC marked LLDP data stale for {interface.name}") + if not interface.remote_switch_mac_address or not interface.remote_switch_port_name: raise ValueError(f"missing LDLDP info in {interface}") From b8e7b64ef264bcc298865de0d8aa7c4923699250 Mon Sep 17 00:00:00 2001 From: Steve Keay Date: Tue, 26 Nov 2024 21:33:19 +0000 Subject: [PATCH 4/4] Improve diagnostic messages on enroll failures --- .../understack_workflows/nautobot_device.py | 68 ++++++++++++------- 1 file changed, 43 insertions(+), 25 deletions(-) diff --git a/python/understack-workflows/understack_workflows/nautobot_device.py b/python/understack-workflows/understack_workflows/nautobot_device.py index 86946cc6a..aed543955 100644 --- a/python/understack-workflows/understack_workflows/nautobot_device.py +++ b/python/understack-workflows/understack_workflows/nautobot_device.py @@ -345,7 +345,7 @@ def find_or_create_interface(nautobot, interface: InterfaceInfo, device_id: str) server_nautobot_interface = nautobot.dcim.interfaces.get(**id) if server_nautobot_interface: logger.info( - f"Updating existing interface {interface.name} " + f"Found existing interface {interface.name} " f"{server_nautobot_interface.id} in Nautobot" ) server_nautobot_interface.update(attrs) @@ -401,11 +401,16 @@ def connect_interface_to_switch( cable = nautobot.dcim.cables.create(**identity, **attrs) except pynautobot.core.query.RequestError as e: # type: ignore raise Exception( - f"Failed to create nautobot cable {identity}: {e}" + f"Failed to document discovered server in Nautobot - Server " + f"Interface {server_nautobot_interface.id} {interface.name} " + f"is detected as connected to Switch Interface " + f"{switch_interface.id} {connected_switch['name']} " + f"{switch_port_name}, but in Nautobot, when we try to create " + f"that cable {identity}, Nautobot gave error {e}" ) from None logger.info(f"Created cable {cable.id} in Nautobot") else: - logger.info(f"Cable {cable.id} already exists in Nautobot") + logger.info(f"Cable {cable.id} already correctly exists in Nautobot") def assign_ip_address(nautobot, nautobot_interface, ipv4_address: IPv4Interface, mac): @@ -419,7 +424,7 @@ def assign_ip_address(nautobot, nautobot_interface, ipv4_address: IPv4Interface, logger.info(f"Making DHCP lease permanent in Nautobot {dict(ip)}") ip.update(type="host", cf_pydhcp_expire=None) elif ip: - logger.info(f"{ipv4_address} exists as {ip.id} in Nautobot IPAM") + logger.info(f"IP Address {ipv4_address} found, {ip.id} in Nautobot") else: ip = nautobot.ipam.ip_addresses.create( address=str(ipv4_address.ip), @@ -441,30 +446,43 @@ def associate_ip_address(nautobot, nautobot_interface, ip_id): If the IP Address is already associated with some other Interface then an Exception is raised. - If the Interface is already associated to some other IP addres then an + If the Interface is already associated to some other IP address then an Exception is raised. """ - existing_association = nautobot.ipam.ip_address_to_interface.get(ip_address=ip_id) + existing_record = nautobot.ipam.ip_address_to_interface.get(ip_address=ip_id) - if existing_association is None: - try: - nautobot.ipam.ip_address_to_interface.create( - ip_address=ip_id, - interface=nautobot_interface.id, - is_primary=True - ) - except pynautobot.core.query.RequestError as e: # type: ignore - raise Exception( - f"Failed to associate IPAddress {ip_id} in Nautobot: {e}" - ) from None - logger.info(f"Associated IP address {ip_id} with {nautobot_interface.name}") - elif existing_association.interface == nautobot_interface.id: - logger.info( - f"IP address {ip_id} already correct on {nautobot_interface.name}" + if existing_record and existing_record.interface.id == nautobot_interface.id: + logger.info(f"IP Address {ip_id} already on {nautobot_interface.name}") + return + elif existing_record: + raise Exception( + f"Failed to document discovered server IP Address in Nautobot - " + f"We need to associate IP address {ip_id} with the server " + f"interface {nautobot_interface.id}, but the IP address is already " + f"associated with another interface {existing_record.interface.id} " + f"({existing_record.display}) Please resolve IP address clash and " + f"then re-try enrollment." ) - else: + + existing_record = nautobot.ipam.ip_address_to_interface.get( + interface=nautobot_interface.id + ) + if existing_record: + raise Exception( + f"Failed to document discovered server IP Address in Nautobot - " + f"We need to associate IP address {ip_id} with the server " + f"interface {nautobot_interface.id}, but that interface is already " + f"associated with a different IP address {existing_record.id} " + f"({existing_record.display}) Please resolve IP address clash and " + f"then re-try enrollment." + ) + + try: + nautobot.ipam.ip_address_to_interface.create( + ip_address=ip_id, interface=nautobot_interface.id, is_primary=True + ) + except pynautobot.core.query.RequestError as e: # type: ignore raise Exception( - f"Failed to document discovered server in Nautobot - IP Address " - f"{ip_id} is already associated with some other interface: " - f"{existing_association.interface.id=} {existing_association.display}" + f"Failed to associate IPAddress {ip_id} in Nautobot: {e}" ) from None + logger.info(f"Associated IP address {ip_id} with {nautobot_interface.name}")