From 3981b2085b50b5bd8f7f29bfef4442c215d2e24f Mon Sep 17 00:00:00 2001 From: Steve Keay Date: Fri, 1 Nov 2024 14:42:37 +0000 Subject: [PATCH 01/22] Remove dead code --- .../understack-workflows/tests/test_bmc_chassis_info.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/python/understack-workflows/tests/test_bmc_chassis_info.py b/python/understack-workflows/tests/test_bmc_chassis_info.py index 7160b545..eeff9cb8 100644 --- a/python/understack-workflows/tests/test_bmc_chassis_info.py +++ b/python/understack-workflows/tests/test_bmc_chassis_info.py @@ -6,7 +6,6 @@ from understack_workflows import bmc_chassis_info -FIXTURE_PATH = "json_samples/bmc_chassis_info" class FakeBmc: @@ -19,13 +18,6 @@ def redfish_request(self, path: str) -> dict: return self.fixtures[path] -def redfish_fixtures_by_platform() -> dict: - return { - platform: read_fixtures(FIXTURE_PATH.joinpath(platform)) - for platform in sorted(os.listdir(FIXTURE_PATH)) - } - - def read_fixtures(path) -> dict: path = pathlib.Path(__file__).parent.joinpath(path) return { From bf0c74b6197d757cdfd5ee4f1ff6c511ee36344d Mon Sep 17 00:00:00 2001 From: Steve Keay Date: Fri, 1 Nov 2024 14:42:55 +0000 Subject: [PATCH 02/22] Base Mock on original class to satisfy type checker --- python/understack-workflows/tests/test_bmc_chassis_info.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/python/understack-workflows/tests/test_bmc_chassis_info.py b/python/understack-workflows/tests/test_bmc_chassis_info.py index eeff9cb8..9cb7796e 100644 --- a/python/understack-workflows/tests/test_bmc_chassis_info.py +++ b/python/understack-workflows/tests/test_bmc_chassis_info.py @@ -5,15 +5,15 @@ from ipaddress import IPv4Interface from understack_workflows import bmc_chassis_info +from understack_workflows.bmc import Bmc - -class FakeBmc: +class FakeBmc(Bmc): def __init__(self, fixtures): self.fixtures = fixtures self.ip_address = "1.2.3.4" - def redfish_request(self, path: str) -> dict: + def redfish_request(self, path: str, *_args, **_kw) -> dict: path = path.replace("/", "_") + ".json" return self.fixtures[path] From 043e44ac9617d1e7ea0a81b376d2c80ea6305aa2 Mon Sep 17 00:00:00 2001 From: Steve Keay Date: Fri, 1 Nov 2024 14:43:29 +0000 Subject: [PATCH 03/22] Add explicit return to give a consistent return type --- python/understack-workflows/understack_workflows/bmc.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/python/understack-workflows/understack_workflows/bmc.py b/python/understack-workflows/understack_workflows/bmc.py index a12bb8a5..5ca1419e 100644 --- a/python/understack-workflows/understack_workflows/bmc.py +++ b/python/understack-workflows/understack_workflows/bmc.py @@ -58,6 +58,8 @@ def redfish_request( ) if r.text: return r.json() + else: + return {} def sushy(self): return Sushy( From 073edce93635f900361fd835ba8713f6d3144e12 Mon Sep 17 00:00:00 2001 From: Steve Keay Date: Fri, 1 Nov 2024 14:46:17 +0000 Subject: [PATCH 04/22] Add null value checks to avoid pyright warning and runtime errors --- .../understack_workflows/bmc_network_config.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/python/understack-workflows/understack_workflows/bmc_network_config.py b/python/understack-workflows/understack_workflows/bmc_network_config.py index 778702e7..d3a013d2 100644 --- a/python/understack-workflows/understack_workflows/bmc_network_config.py +++ b/python/understack-workflows/understack_workflows/bmc_network_config.py @@ -16,6 +16,9 @@ def bmc_set_permanent_ip_addr(bmc: Bmc, interface_info: InterfaceInfo): logger.info("BMC interface was not set to DHCP") return + if not (interface_info.ipv4_address and interface_info.ipv4_gateway): + raise ValueError("BMC InterfaceInfo has missing IP information") + payload = { "Attributes": { "IPv4.1.DHCPEnable": "Disabled", From 4ce12366847adca8400a1f2d5db9f2fcab15e93e Mon Sep 17 00:00:00 2001 From: Steve Keay Date: Fri, 1 Nov 2024 14:47:30 +0000 Subject: [PATCH 05/22] Fix types in method signature to match implementation --- .../understack-workflows/understack_workflows/ironic/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/understack-workflows/understack_workflows/ironic/client.py b/python/understack-workflows/understack_workflows/ironic/client.py index ddcc4abf..22758737 100644 --- a/python/understack-workflows/understack_workflows/ironic/client.py +++ b/python/understack-workflows/understack_workflows/ironic/client.py @@ -58,7 +58,7 @@ def delete_port(self, port_id: str): port_id, ) - def list_ports(self, node_id: dict): + def list_ports(self, node_id: str): self._ensure_logged_in() return self.client.port.list(node=node_id, detail=True) From 0ae695723bcbf6d3c1e945f5796248c37c4d742e Mon Sep 17 00:00:00 2001 From: Steve Keay Date: Fri, 1 Nov 2024 14:51:34 +0000 Subject: [PATCH 06/22] Cast to string to avoid pyright false positive --- .../understack_workflows/main/print_bmc_password.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/python/understack-workflows/understack_workflows/main/print_bmc_password.py b/python/understack-workflows/understack_workflows/main/print_bmc_password.py index c74f3edb..3f189fd6 100755 --- a/python/understack-workflows/understack_workflows/main/print_bmc_password.py +++ b/python/understack-workflows/understack_workflows/main/print_bmc_password.py @@ -4,7 +4,7 @@ from understack_workflows.bmc_password_standard import standard_password -def main(program_name, bmc_ip_address=None): +def main(program_name: str, bmc_ip_address: str | None = None): """CLI script to obtain standard BMC Password. Requires the master secret to be available in BMC_MASTER environment @@ -13,12 +13,11 @@ def main(program_name, bmc_ip_address=None): if bmc_ip_address is None: print(f"Usage: {program_name} ", file=sys.stderr) exit(1) - if os.getenv("BMC_MASTER") is None: print("Please set the BMC_MASTER environment variable", file=sys.stderr) exit(1) - password = standard_password(bmc_ip_address, os.getenv("BMC_MASTER")) + password = standard_password(str(bmc_ip_address), str(os.getenv("BMC_MASTER"))) print(password) From 7d6cab88d8199e5150a52b8bb763f9293874918e Mon Sep 17 00:00:00 2001 From: Steve Keay Date: Fri, 1 Nov 2024 14:52:05 +0000 Subject: [PATCH 07/22] Catch null value to avoid pyright error and possible runtime error --- .../understack_workflows/main/undersync_device.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/python/understack-workflows/understack_workflows/main/undersync_device.py b/python/understack-workflows/understack_workflows/main/undersync_device.py index e01f94cd..6c0758da 100644 --- a/python/understack-workflows/understack_workflows/main/undersync_device.py +++ b/python/understack-workflows/understack_workflows/main/undersync_device.py @@ -50,6 +50,8 @@ def update_nautobot_for_provisioning( interface = nautobot.update_switch_interface_status( device_id, interface_mac, new_status ) + if not interface.device: + raise Exception("Interface has no associated device") vlan_group_id = vlan_group_id_for(interface.device.id, nautobot) logger.debug( f"Switch interface {interface.device} {interface} found in {vlan_group_id=}" From 7ba03a4a025fa544e56aede671a049350cf57e10 Mon Sep 17 00:00:00 2001 From: Steve Keay Date: Fri, 1 Nov 2024 14:52:47 +0000 Subject: [PATCH 08/22] Refactor code to avoid typing issues --- .../understack_workflows/models.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/python/understack-workflows/understack_workflows/models.py b/python/understack-workflows/understack_workflows/models.py index 298c2a58..0c85e7d7 100644 --- a/python/understack-workflows/understack_workflows/models.py +++ b/python/understack-workflows/understack_workflows/models.py @@ -29,19 +29,20 @@ def from_redfish(cls, data: NetworkAdapter) -> NIC: @classmethod def from_hp_json(cls, data: dict) -> NIC: - nic = cls(data.get("name"), data.get("location"), [], data.get("name")) - ports = data.get("network_ports") or data.get("unknown_ports") + nic = cls(data["name"], data["location"], [], data["name"]) + ports = data.get("network_ports") or data.get("unknown_ports", []) nic.interfaces = [Interface.from_hp_json(i, nic, ports) for i in ports] return nic @classmethod def nic_location(cls, nic: NetworkAdapter) -> str: - try: - return nic.json["Controllers"][0]["Location"]["PartLocation"][ - "ServiceLabel" - ] - except KeyError: - return nic.identity + json = nic.json or {} + controller = json.get("Controllers", [])[0] or {} + return ( + controller.get("Location", {}) + .get("PartLocation", {}) + .get("ServiceLabel", nic.identity) + ) @classmethod def nic_ports(cls, nic: NetworkAdapter) -> list[NetworkPort]: @@ -58,7 +59,7 @@ class Interface: @classmethod def from_redfish(cls, data: NetworkPort, nic: NIC) -> Interface: - if data.root.json["Vendor"] == "HPE": + if data.root and data.root.json["Vendor"] == "HPE": name = f"{nic.name}_{data.physical_port_number}" macaddr = data.associated_network_addresses[0] else: From cdca9a53a2df151fc9f689a180702be90cd2fe47 Mon Sep 17 00:00:00 2001 From: Steve Keay Date: Fri, 1 Nov 2024 14:56:03 +0000 Subject: [PATCH 09/22] Update type signature to match implementation --- python/understack-workflows/understack_workflows/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/understack-workflows/understack_workflows/models.py b/python/understack-workflows/understack_workflows/models.py index 0c85e7d7..344f1a93 100644 --- a/python/understack-workflows/understack_workflows/models.py +++ b/python/understack-workflows/understack_workflows/models.py @@ -117,7 +117,7 @@ class Chassis: name: str nics: list[NIC] network_interfaces: list[Interface] - system_info: Systeminfo + system_info: Systeminfo | None @classmethod def check_manufacturer(cls, manufacturer: str) -> None: From cecdd9285f0c0595afb91a1e0fd653716e446cc4 Mon Sep 17 00:00:00 2001 From: Steve Keay Date: Fri, 1 Nov 2024 14:56:39 +0000 Subject: [PATCH 10/22] Supply missing argument to avoid linter warning --- python/understack-workflows/understack_workflows/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/understack-workflows/understack_workflows/models.py b/python/understack-workflows/understack_workflows/models.py index 344f1a93..ebc35292 100644 --- a/python/understack-workflows/understack_workflows/models.py +++ b/python/understack-workflows/understack_workflows/models.py @@ -159,7 +159,7 @@ def from_hp_json(cls, oob_obj: Sushy, chassis_name: str) -> Chassis: data = cls.chassis_hp_json_data(oob_obj) nics = [NIC.from_hp_json(i) for i in data] network_interfaces = cls.interfaces_from_nics(nics) - return cls(chassis_name, nics, network_interfaces) + return cls(chassis_name, nics, network_interfaces, None) @classmethod def interfaces_from_nics(cls, nics: list[NIC]) -> list[Interface]: From b8ae303d50683a05fbf05e026341dfa1ff06d5e9 Mon Sep 17 00:00:00 2001 From: Steve Keay Date: Fri, 1 Nov 2024 14:58:00 +0000 Subject: [PATCH 11/22] Refactor code for readability and to avoid type issues --- .../understack_workflows/models.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/python/understack-workflows/understack_workflows/models.py b/python/understack-workflows/understack_workflows/models.py index ebc35292..16286950 100644 --- a/python/understack-workflows/understack_workflows/models.py +++ b/python/understack-workflows/understack_workflows/models.py @@ -146,13 +146,16 @@ def from_redfish(cls, oob_obj: Sushy) -> Chassis: if cls.bmc_is_ilo4(chassis_data): return cls.from_hp_json(oob_obj, chassis_data.name) - chassis = cls(chassis_data.name, [], [], []) - chassis.nics = [ + nics = [ NIC.from_redfish(i) for i in chassis_data.network_adapters.get_members() ] - chassis.network_interfaces = cls.interfaces_from_nics(chassis.nics) - chassis.system_info = Systeminfo.from_redfish(chassis_data) - return chassis + + return cls( + name=chassis_data.name, # type: ignore + nics=nics, + network_interfaces=cls.interfaces_from_nics(nics), + system_info=Systeminfo.from_redfish(chassis_data), + ) @classmethod def from_hp_json(cls, oob_obj: Sushy, chassis_name: str) -> Chassis: From 8197d4d42057355e96005d366beb3753d7907066 Mon Sep 17 00:00:00 2001 From: Steve Keay Date: Fri, 1 Nov 2024 15:01:32 +0000 Subject: [PATCH 12/22] Add null checks to satisfy type checker and avoid runtime errors --- .../understack-workflows/understack_workflows/nautobot.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/python/understack-workflows/understack_workflows/nautobot.py b/python/understack-workflows/understack_workflows/nautobot.py index 3ab7174c..5526600e 100644 --- a/python/understack-workflows/understack_workflows/nautobot.py +++ b/python/understack-workflows/understack_workflows/nautobot.py @@ -4,8 +4,7 @@ import pynautobot from pynautobot.core.api import Api as NautobotApi -from pynautobot.models.dcim import Devices as NautobotDevice -from pynautobot.models.dcim import Interfaces as NautobotInterface +import pynautobot.models.dcim class Nautobot: @@ -51,6 +50,10 @@ def non_lag_interface_by_mac( def update_cf(self, device_id: UUID, field_name: str, field_value: str): device = self.device_by_id(device_id) + if not device: + raise Exception(f"No such device {device_id}") + if not device.custom_fields: + raise Exception(f"Device {device_id} has no custom fields") device.custom_fields[field_name] = field_value response = device.save() self.logger.info(f"save result: {response}") From fcecd19ef94a1b85656649a193c8299ebf635a0d Mon Sep 17 00:00:00 2001 From: Steve Keay Date: Fri, 1 Nov 2024 15:03:02 +0000 Subject: [PATCH 13/22] Refactor code to avoid type errors and avoid naming clash --- .../understack_workflows/nautobot.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/python/understack-workflows/understack_workflows/nautobot.py b/python/understack-workflows/understack_workflows/nautobot.py index 5526600e..60f9228b 100644 --- a/python/understack-workflows/understack_workflows/nautobot.py +++ b/python/understack-workflows/understack_workflows/nautobot.py @@ -4,7 +4,7 @@ import pynautobot from pynautobot.core.api import Api as NautobotApi -import pynautobot.models.dcim +from pynautobot.models import dcim class Nautobot: @@ -22,13 +22,13 @@ def exit_with_error(self, error): def api_session(self, url: str, token: str) -> NautobotApi: return pynautobot.api(url, token=token) - def device_by_id(self, device_id: UUID) -> NautobotDevice: + def device_by_id(self, device_id: UUID) -> dcim.Devices: device = self.session.dcim.devices.get(device_id) if not device: self.exit_with_error(f"Device {device_id!s} not found in Nautobot") return device - def interface_by_id(self, interface_id: UUID) -> NautobotInterface: + def interface_by_id(self, interface_id: UUID) -> dcim.Interfaces: interface = self.session.dcim.interfaces.get(interface_id) if not interface: self.exit_with_error(f"Interface {interface_id!s} not found in Nautobot") @@ -36,17 +36,17 @@ def interface_by_id(self, interface_id: UUID) -> NautobotInterface: def non_lag_interface_by_mac( self, device_id: UUID, mac_address: str - ) -> list[NautobotInterface]: - interfaces = self.session.dcim.interfaces.filter( + ) -> dcim.Interfaces: + interface = self.session.dcim.interfaces.get( device_id=device_id, mac_address=mac_address, type__n="lag", ) - if not interfaces: + if not interface: self.exit_with_error( f"Interface with {device_id=} and {mac_address=} not found in Nautobot" ) - return interfaces[0] + return interface # type: ignore def update_cf(self, device_id: UUID, field_name: str, field_value: str): device = self.device_by_id(device_id) @@ -61,7 +61,7 @@ def update_cf(self, device_id: UUID, field_name: str, field_value: str): def update_switch_interface_status( self, device_id: UUID, server_interface_mac: str, new_status: str - ) -> NautobotInterface: + ) -> dcim.Interfaces: """Change the Interface Status in Nautobot for interfaces. The device_id and interface MAC address parameters identify one or more From 98948472fa01227dde09f2b896e0d36cecefca23 Mon Sep 17 00:00:00 2001 From: Steve Keay Date: Fri, 1 Nov 2024 15:04:13 +0000 Subject: [PATCH 14/22] Cast to string to avoid typeing issue --- .../understack_workflows/openstack/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/understack-workflows/understack_workflows/openstack/client.py b/python/understack-workflows/understack_workflows/openstack/client.py index b755ab06..a686cfa2 100644 --- a/python/understack-workflows/understack_workflows/openstack/client.py +++ b/python/understack-workflows/understack_workflows/openstack/client.py @@ -11,7 +11,7 @@ from openstack.connection import Connection try: - _pkg_ver = _meta.version(__package__.split(".")[0]) + _pkg_ver = _meta.version(str(__package__).split(".")[0]) except Exception: _pkg_ver = "dev" From 587fcfd4f72085ecc1b7b8ed154be1537cc38e1c Mon Sep 17 00:00:00 2001 From: Steve Keay Date: Fri, 1 Nov 2024 15:04:39 +0000 Subject: [PATCH 15/22] Suppress pyright checking for imports that don't play nicely --- .../understack_workflows/bmc.py | 2 +- .../understack_workflows/bmc_credentials.py | 2 +- .../understack_workflows/ironic_node.py | 1 + .../main/sync_keystone.py | 16 +++++++------- .../understack_workflows/models.py | 21 ++++++++++--------- .../understack_workflows/nautobot.py | 10 ++++----- .../understack_workflows/nautobot_device.py | 6 +++--- .../understack_workflows/openstack/client.py | 2 +- 8 files changed, 31 insertions(+), 29 deletions(-) diff --git a/python/understack-workflows/understack_workflows/bmc.py b/python/understack-workflows/understack_workflows/bmc.py index 5ca1419e..9cf703d7 100644 --- a/python/understack-workflows/understack_workflows/bmc.py +++ b/python/understack-workflows/understack_workflows/bmc.py @@ -9,7 +9,7 @@ from understack_workflows.bmc_password_standard import standard_password from understack_workflows.helpers import credential -urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning) +urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning) # type: ignore logging.getLogger("urllib3").setLevel(logging.WARNING) HEADERS = { diff --git a/python/understack-workflows/understack_workflows/bmc_credentials.py b/python/understack-workflows/understack_workflows/bmc_credentials.py index 31de58ce..03ec0b6d 100644 --- a/python/understack-workflows/understack_workflows/bmc_credentials.py +++ b/python/understack-workflows/understack_workflows/bmc_credentials.py @@ -3,7 +3,7 @@ from understack_workflows.helpers import setup_logger -urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning) +urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning) # type: ignore FACTORY_PASSWORD = "calvin" diff --git a/python/understack-workflows/understack_workflows/ironic_node.py b/python/understack-workflows/understack_workflows/ironic_node.py index 78e48d96..47e6c852 100644 --- a/python/understack-workflows/understack_workflows/ironic_node.py +++ b/python/understack-workflows/understack_workflows/ironic_node.py @@ -29,6 +29,7 @@ def create_or_update( ironic_node = create_ironic_node( client, node_uuid, device_hostname, driver, bmc ) + return ironic_node.provision_state # type: ignore if ironic_node.provision_state in STATES_ALLOWING_UPDATES: update_ironic_node(client, node_uuid, device_hostname, driver, bmc) diff --git a/python/understack-workflows/understack_workflows/main/sync_keystone.py b/python/understack-workflows/understack_workflows/main/sync_keystone.py index 8f91d455..a18865a2 100644 --- a/python/understack-workflows/understack_workflows/main/sync_keystone.py +++ b/python/understack-workflows/understack_workflows/main/sync_keystone.py @@ -53,7 +53,7 @@ def is_valid_domain( ) -> bool: if only_domain is None: return True - project = conn.identity.get_project(project_id.hex) + project = conn.identity.get_project(project_id.hex) # type: ignore ret = project.domain_id == only_domain.hex if not ret: logger.info( @@ -65,22 +65,22 @@ def is_valid_domain( def handle_project_create(conn: Connection, nautobot: Nautobot, project_id: uuid.UUID): logger.info(f"got request to create tenant {project_id!s}") - project = conn.identity.get_project(project_id.hex) + project = conn.identity.get_project(project_id.hex) # type: ignore ten_api = nautobot.session.tenancy.tenants ten_api.url = f"{ten_api.base_url}/plugins/uuid-api-endpoints/tenant" ten = ten_api.create( id=str(project_id), name=project.name, description=project.description ) - logger.info(f"tenant '{project_id!s}' created {ten.created}") + logger.info(f"tenant '{project_id!s}' created {ten.created}") # type: ignore def handle_project_update(conn: Connection, nautobot: Nautobot, project_id: uuid.UUID): logger.info(f"got request to update tenant {project_id!s}") - project = conn.identity.get_project(project_id.hex) + project = conn.identity.get_project(project_id.hex) # type: ignore ten = nautobot.session.tenancy.tenants.get(project_id) - ten.description = project.description - ten.save() - logger.info(f"tenant '{project_id!s}' last updated {ten.last_updated}") + ten.description = project.description # type: ignore + ten.save() # type: ignore + logger.info(f"tenant '{project_id!s}' last updated {ten.last_updated}") # type: ignore def handle_project_delete(conn: Connection, nautobot: Nautobot, project_id: uuid.UUID): @@ -89,7 +89,7 @@ def handle_project_delete(conn: Connection, nautobot: Nautobot, project_id: uuid if not ten: logger.warn(f"tenant '{project_id!s}' does not exist already") return - ten.delete() + ten.delete() # type: ignore logger.info(f"deleted tenant {project_id!s}") diff --git a/python/understack-workflows/understack_workflows/models.py b/python/understack-workflows/understack_workflows/models.py index 16286950..9cf0c1cd 100644 --- a/python/understack-workflows/understack_workflows/models.py +++ b/python/understack-workflows/understack_workflows/models.py @@ -23,7 +23,7 @@ class NIC: @classmethod def from_redfish(cls, data: NetworkAdapter) -> NIC: location = cls.nic_location(data) - nic = cls(data.identity, location, [], data.model) + nic = cls(data.identity, location, [], data.model) # type: ignore nic.interfaces = [Interface.from_redfish(i, nic) for i in cls.nic_ports(data)] return nic @@ -61,15 +61,15 @@ class Interface: def from_redfish(cls, data: NetworkPort, nic: NIC) -> Interface: if data.root and data.root.json["Vendor"] == "HPE": name = f"{nic.name}_{data.physical_port_number}" - macaddr = data.associated_network_addresses[0] + macaddr = data.associated_network_addresses[0] # type: ignore else: name = data.identity macaddr = cls.fetch_macaddr_from_sys_resource(data) return cls( - name, + name, # type: ignore macaddr, nic.location, - data.current_link_speed_mbps, + data.current_link_speed_mbps, # type: ignore nic.model, ) @@ -88,9 +88,9 @@ def from_hp_json(cls, data: dict, nic: NIC, ports: list) -> Interface: @classmethod def fetch_macaddr_from_sys_resource(cls, data: NetworkPort) -> str: try: - path = f"{data.root.get_system().ethernet_interfaces.path}/{data.identity}" + path = f"{data.root.get_system().ethernet_interfaces.path}/{data.identity}" # type: ignore macaddr = ( - data.root.get_system().ethernet_interfaces.get_member(path).mac_address + data.root.get_system().ethernet_interfaces.get_member(path).mac_address # type: ignore ) except ResourceNotFoundError: macaddr = "" @@ -138,13 +138,13 @@ def bmc_is_ilo4(cls, chassis_data: SushyChassis) -> bool: @classmethod def from_redfish(cls, oob_obj: Sushy) -> Chassis: chassis_data = oob_obj.get_chassis( - oob_obj.get_chassis_collection().members_identities[0] + oob_obj.get_chassis_collection().members_identities[0] # type: ignore ) - cls.check_manufacturer(chassis_data.manufacturer) + cls.check_manufacturer(chassis_data.manufacturer) # type: ignore if cls.bmc_is_ilo4(chassis_data): - return cls.from_hp_json(oob_obj, chassis_data.name) + return cls.from_hp_json(oob_obj, chassis_data.name) # type: ignore nics = [ NIC.from_redfish(i) for i in chassis_data.network_adapters.get_members() @@ -171,7 +171,8 @@ def interfaces_from_nics(cls, nics: list[NIC]) -> list[Interface]: @classmethod def chassis_hp_json_data(cls, oob_obj: Sushy) -> dict: oob_obj._conn.set_http_basic_auth( - username=oob_obj._auth._username, password=oob_obj._auth._password + username=oob_obj._auth._username, # type: ignore + password=oob_obj._auth._password, # type: ignore ) resp = oob_obj._conn.get(path="/json/comm_controller_info") resp.raise_for_status() diff --git a/python/understack-workflows/understack_workflows/nautobot.py b/python/understack-workflows/understack_workflows/nautobot.py index 60f9228b..ae48a307 100644 --- a/python/understack-workflows/understack_workflows/nautobot.py +++ b/python/understack-workflows/understack_workflows/nautobot.py @@ -26,13 +26,13 @@ def device_by_id(self, device_id: UUID) -> dcim.Devices: device = self.session.dcim.devices.get(device_id) if not device: self.exit_with_error(f"Device {device_id!s} not found in Nautobot") - return device + return device # type: ignore def interface_by_id(self, interface_id: UUID) -> dcim.Interfaces: interface = self.session.dcim.interfaces.get(interface_id) if not interface: self.exit_with_error(f"Interface {interface_id!s} not found in Nautobot") - return interface + return interface # type: ignore def non_lag_interface_by_mac( self, device_id: UUID, mac_address: str @@ -85,13 +85,13 @@ def update_switch_interface_status( f"Interface {server_interface_mac=} {server_interface.type} " "is not connected in Nautobot" ) - switch_interface_id = connected_endpoint.id + switch_interface_id = connected_endpoint.id # type: ignore self.logger.debug( f"Interface {server_interface_mac=} connects to {switch_interface_id=}" ) switch_interface = self.interface_by_id(switch_interface_id) - switch_interface.status = new_status + switch_interface.status = new_status # type: ignore result = switch_interface.save() self.logger.debug( @@ -101,7 +101,7 @@ def update_switch_interface_status( def update_device_status(self, device_id: UUID, device_status: str): device = self.device_by_id(device_id) - device.status = device_status + device.status = device_status # type: ignore result = device.save() self.logger.info( f"device {device_id} updated to Status {device_status} {result=}" diff --git a/python/understack-workflows/understack_workflows/nautobot_device.py b/python/understack-workflows/understack_workflows/nautobot_device.py index f75ac73e..1615f70a 100644 --- a/python/understack-workflows/understack_workflows/nautobot_device.py +++ b/python/understack-workflows/understack_workflows/nautobot_device.py @@ -380,7 +380,7 @@ def connect_interface_to_switch( if cable is None: try: cable = nautobot.dcim.cables.create(**identity, **attrs) - except pynautobot.core.query.RequestError as e: + except pynautobot.core.query.RequestError as e: # type: ignore raise Exception( f"Failed to create nautobot cable {identity}: {e}" ) from None @@ -407,7 +407,7 @@ def assign_ip_address(nautobot, nautobot_interface, ipv4_address: IPv4Interface, }, ) logger.info(f"Created Nautobot IP {ip.id} for {ipv4_address}") - except pynautobot.core.query.RequestError as e: + except pynautobot.core.query.RequestError as e: # type: ignore raise Exception(f"Failed to assign {ipv4_address=} in Nautobot: {e}") from None return ip @@ -423,7 +423,7 @@ def associate_ip_address(nautobot, nautobot_interface, ip_id): 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: + except pynautobot.core.query.RequestError as e: # type: ignore raise Exception( f"Failed to associate IPAddress {ip_id} in Nautobot: {e}" ) from None diff --git a/python/understack-workflows/understack_workflows/openstack/client.py b/python/understack-workflows/understack_workflows/openstack/client.py index a686cfa2..3cf7bb81 100644 --- a/python/understack-workflows/understack_workflows/openstack/client.py +++ b/python/understack-workflows/understack_workflows/openstack/client.py @@ -40,7 +40,7 @@ def get_openstack_client(cloud=None, region_name="") -> Connection: return Connection(config=cloud_region) -def get_ironic_client(cloud=None, region_name="") -> IronicClient: +def get_ironic_client(cloud=None, region_name="") -> IronicClient: # type: ignore """Returns our Ironic Client wrapper configured from our clouds.yaml.""" cloud_region = _get_os_cloud_region(cloud, region_name) client = _get_ironic_client( From d82c2e548426418ef846596e0615af7594daddbe Mon Sep 17 00:00:00 2001 From: Steve Keay Date: Fri, 1 Nov 2024 15:14:59 +0000 Subject: [PATCH 16/22] Add checks, casts and update signatures to avoid typing errors --- .../understack_workflows/nautobot_device.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/python/understack-workflows/understack_workflows/nautobot_device.py b/python/understack-workflows/understack_workflows/nautobot_device.py index 1615f70a..2e1225ed 100644 --- a/python/understack-workflows/understack_workflows/nautobot_device.py +++ b/python/understack-workflows/understack_workflows/nautobot_device.py @@ -86,14 +86,18 @@ def find_or_create(chassis_info: ChassisInfo, nautobot) -> NautobotDevice: # Re-run the graphql query to fetch any auto-created defaults from # nautobot (e.g. it automatically creates a BMC interface): device = nautobot_server(nautobot, serial=chassis_info.serial_number) + if not device: + raise Exception("Failed to create device in Nautobot") find_or_create_interfaces(nautobot, chassis_info, device.id, switches) # Run the graphql query yet again, to include all the data we just populated # in nautobot. Fairly innefficient for the case where we didn't change # anything, but we need the accurate data. - return nautobot_server(nautobot, serial=chassis_info.serial_number) - + device = nautobot_server(nautobot, serial=chassis_info.serial_number) + if not device: + raise Exception("Failed to create device in Nautobot") + return device def location_from(switches): locations = { @@ -119,7 +123,7 @@ def switches_for(nautobot, chassis_info: ChassisInfo) -> dict: if interface.remote_switch_mac_address } base_switch_macs = { - base_mac(interface.remote_switch_mac_address, interface.remote_switch_port_name) + base_mac(interface.remote_switch_mac_address, str(interface.remote_switch_port_name)) for interface in chassis_info.interfaces if interface.remote_switch_mac_address } @@ -129,7 +133,7 @@ def switches_for(nautobot, chassis_info: ChassisInfo) -> dict: return switches -def nautobot_switches(nautobot, mac_addresses: list[str]) -> dict[str, dict]: +def nautobot_switches(nautobot, mac_addresses: set[str]) -> dict[str, dict]: """Get switches by MAC address. Assumes that MAC addresses in Nautobot are normalized to upcase From b6d7cd532630926e95f9de14dcc56cb31fc698be Mon Sep 17 00:00:00 2001 From: Steve Keay Date: Fri, 1 Nov 2024 15:17:09 +0000 Subject: [PATCH 17/22] Allow for null Interface attributes --- .../understack_workflows/nautobot_device.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/python/understack-workflows/understack_workflows/nautobot_device.py b/python/understack-workflows/understack_workflows/nautobot_device.py index 2e1225ed..798ec8d1 100644 --- a/python/understack-workflows/understack_workflows/nautobot_device.py +++ b/python/understack-workflows/understack_workflows/nautobot_device.py @@ -27,14 +27,14 @@ class NautobotInterface: description: str mac_address: str status: str - ip_address: str - neighbor_device_id: str - neighbor_device_name: str - neighbor_interface_id: str - neighbor_interface_name: str - neighbor_chassis_mac: str - neighbor_location_name: str - neighbor_rack_name: str + ip_address: str | None + neighbor_device_id: str | None + neighbor_device_name: str | None + neighbor_interface_id: str | None + neighbor_interface_name: str | None + neighbor_chassis_mac: str | None + neighbor_location_name: str | None + neighbor_rack_name: str | None @dataclass From 1e1c0b456984994b97577f9eec95f02844d8a231 Mon Sep 17 00:00:00 2001 From: Steve Keay Date: Fri, 1 Nov 2024 15:18:06 +0000 Subject: [PATCH 18/22] Fail hard if there is a missing mac_addr in HP payload --- python/understack-workflows/understack_workflows/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/understack-workflows/understack_workflows/models.py b/python/understack-workflows/understack_workflows/models.py index 9cf0c1cd..5b91982e 100644 --- a/python/understack-workflows/understack_workflows/models.py +++ b/python/understack-workflows/understack_workflows/models.py @@ -79,7 +79,7 @@ def from_hp_json(cls, data: dict, nic: NIC, ports: list) -> Interface: interface_name = f"NIC.{nic.location.replace(' ', '.')}_{p_num}" return cls( interface_name, - data.get("mac_addr"), + data["mac_addr"], nic.location, data.get("speed", 0), nic.model, From a8f32524610320acd836691cc94197de424423a8 Mon Sep 17 00:00:00 2001 From: Steve Keay Date: Fri, 1 Nov 2024 15:25:59 +0000 Subject: [PATCH 19/22] Fix code formatting --- .../understack_workflows/nautobot_device.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/python/understack-workflows/understack_workflows/nautobot_device.py b/python/understack-workflows/understack_workflows/nautobot_device.py index 798ec8d1..3ee80cc8 100644 --- a/python/understack-workflows/understack_workflows/nautobot_device.py +++ b/python/understack-workflows/understack_workflows/nautobot_device.py @@ -99,6 +99,7 @@ def find_or_create(chassis_info: ChassisInfo, nautobot) -> NautobotDevice: raise Exception("Failed to create device in Nautobot") return device + def location_from(switches): locations = { (switch["location"]["id"], switch["rack"]["id"]) for switch in switches @@ -123,7 +124,9 @@ def switches_for(nautobot, chassis_info: ChassisInfo) -> dict: if interface.remote_switch_mac_address } base_switch_macs = { - base_mac(interface.remote_switch_mac_address, str(interface.remote_switch_port_name)) + base_mac( + interface.remote_switch_mac_address, str(interface.remote_switch_port_name) + ) for interface in chassis_info.interfaces if interface.remote_switch_mac_address } @@ -384,7 +387,7 @@ def connect_interface_to_switch( if cable is None: try: cable = nautobot.dcim.cables.create(**identity, **attrs) - except pynautobot.core.query.RequestError as e: # type: ignore + except pynautobot.core.query.RequestError as e: # type: ignore raise Exception( f"Failed to create nautobot cable {identity}: {e}" ) from None From ca4b15bf0e96b2d98689920f69ea3e718e5f1f41 Mon Sep 17 00:00:00 2001 From: Steve Keay Date: Fri, 1 Nov 2024 15:26:15 +0000 Subject: [PATCH 20/22] Get a None rather than empty list when there are no ip addresses --- .../understack_workflows/nautobot_device.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/understack-workflows/understack_workflows/nautobot_device.py b/python/understack-workflows/understack_workflows/nautobot_device.py index 3ee80cc8..1694bc85 100644 --- a/python/understack-workflows/understack_workflows/nautobot_device.py +++ b/python/understack-workflows/understack_workflows/nautobot_device.py @@ -280,7 +280,7 @@ def parse_device(data: dict) -> NautobotDevice: def parse_interface(data: dict) -> NautobotInterface: connected = data["connected_interface"] - ip_address = data["ip_addresses"] and data["ip_addresses"][0] + ip_address = data["ip_addresses"][0] if data["ip_addresses"] else None return NautobotInterface( id=data["id"], name=data["name"], From eecc3ca72c6b8ff6c79673ea8ffc315117a88722 Mon Sep 17 00:00:00 2001 From: Steve Keay Date: Fri, 8 Nov 2024 11:08:23 +0000 Subject: [PATCH 21/22] Fix types of newly-added fixtures --- .../understack-workflows/tests/fixture_nautobot_device.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/python/understack-workflows/tests/fixture_nautobot_device.py b/python/understack-workflows/tests/fixture_nautobot_device.py index 86addd25..38c38cc9 100644 --- a/python/understack-workflows/tests/fixture_nautobot_device.py +++ b/python/understack-workflows/tests/fixture_nautobot_device.py @@ -16,7 +16,7 @@ description="Integrated NIC 1 Port 1", mac_address="D4:04:E6:4F:8D:B4", status="Active", - ip_address=[], + ip_address=None, neighbor_device_id="275ef491-2b27-4d1b-bd45-330bd6b7e0cf", neighbor_device_name="f20-2-1.iad3.rackspace.net", neighbor_interface_id="f9a5cc87-d10a-4827-99e8-48961fd1d773", @@ -32,7 +32,7 @@ description="Integrated NIC 1 Port 2", mac_address="D4:04:E6:4F:8D:B5", status="Active", - ip_address=[], + ip_address=None, neighbor_device_id="05f6715a-4dbe-4fd6-af20-1e73adb285c2", neighbor_device_name="f20-2-2.iad3.rackspace.net", neighbor_interface_id="2148cf50-f70e-42c9-9f68-8ce98d61498c", @@ -48,7 +48,7 @@ description="NIC in Slot 1 Port 1", mac_address="14:23:F3:F5:25:F0", status="Active", - ip_address=[], + ip_address=None, neighbor_device_id="05f6715a-4dbe-4fd6-af20-1e73adb285c2", neighbor_device_name="f20-2-2.iad3.rackspace.net", neighbor_interface_id="f72bb830-3f3c-4aba-b7d5-9680ea4d358e", @@ -64,7 +64,7 @@ description="NIC in Slot 1 Port 2", mac_address="14:23:F3:F5:25:F1", status="Active", - ip_address=[], + ip_address=None, neighbor_device_id="275ef491-2b27-4d1b-bd45-330bd6b7e0cf", neighbor_device_name="f20-2-1.iad3.rackspace.net", neighbor_interface_id="c210be75-1038-4ba3-9923-60050e1c5362", From 1719ad5407acdf3c8444efb70207ddabd4496965 Mon Sep 17 00:00:00 2001 From: Steve Keay Date: Thu, 31 Oct 2024 11:39:08 +0000 Subject: [PATCH 22/22] Add pyright type checking for understack-workflows python code Note that we hard-code python deps because the pre-commit checker operates in its own independent venv and cannot access the deps that poetry installs. This seems broken, as it allows the versions to drift. However it is better than nothing. --- .pre-commit-config.yaml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index ca26115c..866259c1 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -65,3 +65,19 @@ repos: rev: 38.114.0 hooks: - id: renovate-config-validator + - repo: https://github.com/RobertCraigie/pyright-python + rev: v1.1.387 + hooks: + - id: pyright + files: '^python/understack-workflows/' + args: ["--threads"] + additional_dependencies: + - "kubernetes" + - "pydantic" + - "pynautobot" + - "pytest" + - "pytest_lazy_fixtures" + - "python-ironicclient" + - "requests" + - "sushy" + - "types-requests"