From e5e268c2f67fb9c3572b5f4643eac8854a83016b Mon Sep 17 00:00:00 2001 From: Doug Goldstein Date: Mon, 5 Aug 2024 14:01:46 -0500 Subject: [PATCH] fix: stop using device name for interface sync Use the device ID / UUID which is a better identifier for us to use to find the device to sync the interfaces for. This saves us a trip to the Nautobot DB as well. In Nautobot the device name does not have to be unique so this avoids possible issues there. Added tests of the parser to make sure we get the correct data. Added types so we can have better tests in the future. Cleaned up the duplicate hostname usage in the workflow as well. --- .../nb-oob-interface-update-sensor.yaml | 4 +- .../sync-nb-server-to-ironic.yaml | 2 +- .../synchronize-obm-creds.yaml | 2 +- .../sync-interfaces-to-nautobot.yaml | 4 +- .../sync-srv-redfish-intfs-to-nb.yaml | 10 ++-- .../tests/test_sync_nautobot_interfaces.py | 23 ++++++++ .../understack_workflows/helpers.py | 13 ----- .../main/sync_nautobot_interfaces.py | 57 +++++++++++++------ .../understack_workflows/nautobot.py | 36 +++++------- 9 files changed, 90 insertions(+), 61 deletions(-) create mode 100644 python/understack-workflows/tests/test_sync_nautobot_interfaces.py diff --git a/argo-workflows/shared-sensors/nb-oob-interface-update-sensor.yaml b/argo-workflows/shared-sensors/nb-oob-interface-update-sensor.yaml index 501a9623d..d5f2787fd 100644 --- a/argo-workflows/shared-sensors/nb-oob-interface-update-sensor.yaml +++ b/argo-workflows/shared-sensors/nb-oob-interface-update-sensor.yaml @@ -63,8 +63,8 @@ spec: parameters: - name: interface_update_event value: Some nautobot interface has changed - - name: device_hostname - value: hostname of the device that event is for + - name: device_id + value: device id that event is for - name: hostname value: to support other workflows that references hostname entrypoint: start diff --git a/argo-workflows/sync-nb-server-to-ironic/workflowtemplates/sync-nb-server-to-ironic.yaml b/argo-workflows/sync-nb-server-to-ironic/workflowtemplates/sync-nb-server-to-ironic.yaml index 8d19b5057..a967dfe2f 100644 --- a/argo-workflows/sync-nb-server-to-ironic/workflowtemplates/sync-nb-server-to-ironic.yaml +++ b/argo-workflows/sync-nb-server-to-ironic/workflowtemplates/sync-nb-server-to-ironic.yaml @@ -8,7 +8,7 @@ spec: parameters: - name: interface_update_event value: Some nautobot interface has changed - - name: device_hostname + - name: hostname value: hostname of the device that event is for templates: - name: main diff --git a/argo-workflows/sync-nb-server-to-ironic/workflowtemplates/synchronize-obm-creds.yaml b/argo-workflows/sync-nb-server-to-ironic/workflowtemplates/synchronize-obm-creds.yaml index f36cf0dd3..8c0240991 100644 --- a/argo-workflows/sync-nb-server-to-ironic/workflowtemplates/synchronize-obm-creds.yaml +++ b/argo-workflows/sync-nb-server-to-ironic/workflowtemplates/synchronize-obm-creds.yaml @@ -18,7 +18,7 @@ spec: arguments: parameters: - name: hostname - value: '{{workflow.parameters.device_hostname}}' + value: '{{workflow.parameters.hostname}}' - - name: synchronize-obm-creds template: synchronize-obm-creds arguments: diff --git a/argo-workflows/sync-srv-redfish-intfs-to-nb/workflowtemplates/sync-interfaces-to-nautobot.yaml b/argo-workflows/sync-srv-redfish-intfs-to-nb/workflowtemplates/sync-interfaces-to-nautobot.yaml index 2b1b931d1..2f704c568 100644 --- a/argo-workflows/sync-srv-redfish-intfs-to-nb/workflowtemplates/sync-interfaces-to-nautobot.yaml +++ b/argo-workflows/sync-srv-redfish-intfs-to-nb/workflowtemplates/sync-interfaces-to-nautobot.yaml @@ -5,7 +5,7 @@ kind: WorkflowTemplate spec: arguments: parameters: - - name: hostname + - name: device_id value: "{}" - name: oob_secret value: "{}" @@ -15,7 +15,7 @@ spec: image: ghcr.io/rackerlabs/understack/ironic-nautobot-client:latest command: - sync-nautobot-interfaces - args: ["--hostname", "{{workflow.parameters.device_hostname}}"] + args: ["--device-id", "{{workflow.parameters.device_id}}"] volumeMounts: - mountPath: /etc/nb-token/ name: nb-token diff --git a/argo-workflows/sync-srv-redfish-intfs-to-nb/workflowtemplates/sync-srv-redfish-intfs-to-nb.yaml b/argo-workflows/sync-srv-redfish-intfs-to-nb/workflowtemplates/sync-srv-redfish-intfs-to-nb.yaml index af5a56a1d..898fe2513 100644 --- a/argo-workflows/sync-srv-redfish-intfs-to-nb/workflowtemplates/sync-srv-redfish-intfs-to-nb.yaml +++ b/argo-workflows/sync-srv-redfish-intfs-to-nb/workflowtemplates/sync-srv-redfish-intfs-to-nb.yaml @@ -6,6 +6,8 @@ spec: entrypoint: main arguments: parameters: + - name: device_id + value: "{}" - name: hostname value: "{}" templates: @@ -18,7 +20,7 @@ spec: arguments: parameters: - name: hostname - value: "{{workflow.parameters.device_hostname}}" + value: "{{workflow.parameters.hostname}}" - - name: get-obm-creds-secret templateRef: name: get-obm-creds @@ -26,14 +28,14 @@ spec: arguments: parameters: - name: hostname - value: "{{workflow.parameters.device_hostname}}" + value: "{{workflow.parameters.hostname}}" - - name: synchronize-interfaces-to-nautobot templateRef: name: sync-interfaces-to-nautobot template: synchronize-interfaces arguments: parameters: - - name: hostname - value: "{{workflow.parameters.device_hostname}}" + - name: device_id + value: "{{workflow.parameters.device_id}}" - name: oob_secret value: "{{steps.get-obm-creds-secret.outputs.parameters.secret}}" diff --git a/python/understack-workflows/tests/test_sync_nautobot_interfaces.py b/python/understack-workflows/tests/test_sync_nautobot_interfaces.py new file mode 100644 index 000000000..486917de9 --- /dev/null +++ b/python/understack-workflows/tests/test_sync_nautobot_interfaces.py @@ -0,0 +1,23 @@ +import uuid + +import pytest + +from understack_workflows.main.sync_nautobot_interfaces import argument_parser + + +@pytest.fixture +def device_id() -> uuid.UUID: + return uuid.uuid4() + + +def test_parse_device_name(): + parser = argument_parser(__name__) + with pytest.raises(SystemExit): + parser.parse_args(["--device-id", "FOO"]) + + +def test_parse_device_id(device_id): + parser = argument_parser(__name__) + args = parser.parse_args(["--device-id", str(device_id)]) + + assert args.device_id == device_id diff --git a/python/understack-workflows/understack_workflows/helpers.py b/python/understack-workflows/understack_workflows/helpers.py index 2d5f78b59..14c453e89 100644 --- a/python/understack-workflows/understack_workflows/helpers.py +++ b/python/understack-workflows/understack_workflows/helpers.py @@ -1,6 +1,5 @@ import argparse import logging -import os import pathlib from functools import partial @@ -24,18 +23,6 @@ def setup_logger(name: str | None = None, level: int = logging.DEBUG): return logging.getLogger(name) -def arg_parser(name): - parser = argparse.ArgumentParser( - prog=os.path.basename(name), description="Nautobot Interface sync" - ) - parser.add_argument("--hostname", required=True, help="Nautobot device name") - parser.add_argument("--oob_username", required=False, help="OOB username") - parser.add_argument("--oob_password", required=False, help="OOB password") - parser.add_argument("--nautobot_url", required=False) - parser.add_argument("--nautobot_token", required=False) - return parser - - def boolean_args(val): normalised = str(val).upper() if normalised in ["YES", "TRUE", "T", "1"]: diff --git a/python/understack-workflows/understack_workflows/main/sync_nautobot_interfaces.py b/python/understack-workflows/understack_workflows/main/sync_nautobot_interfaces.py index 0bfe8c9ef..9a0d4712e 100644 --- a/python/understack-workflows/understack_workflows/main/sync_nautobot_interfaces.py +++ b/python/understack-workflows/understack_workflows/main/sync_nautobot_interfaces.py @@ -1,4 +1,7 @@ -from understack_workflows.helpers import arg_parser +import argparse +import os +from uuid import UUID + from understack_workflows.helpers import credential from understack_workflows.helpers import is_off_board from understack_workflows.helpers import oob_sushy_session @@ -8,21 +11,32 @@ logger = setup_logger(__name__) - -def main(): - parser = arg_parser(__file__) - args = parser.parse_args() - - default_nb_url = "http://nautobot-default.nautobot.svc.cluster.local" - device_name = args.hostname - nb_url = args.nautobot_url or default_nb_url - nb_token = args.nautobot_token or credential("nb-token", "token") - oob_username = args.oob_username or credential("oob-secrets", "username") - oob_password = args.oob_password or credential("oob-secrets", "password") - +DEFAULT_NB_URL = "http://nautobot-default.nautobot.svc.cluster.local" + + +def argument_parser(name): + parser = argparse.ArgumentParser( + prog=os.path.basename(name), description="Nautobot Interface sync" + ) + parser.add_argument( + "--device-id", type=UUID, required=True, help="Nautobot device ID" + ) + parser.add_argument("--oob_username", required=False, help="OOB username") + parser.add_argument("--oob_password", required=False, help="OOB password") + parser.add_argument( + "--nautobot_url", + required=False, + help="Nautobot API %(default)s", + default=DEFAULT_NB_URL, + ) + parser.add_argument("--nautobot_token", required=False) + return parser + + +def do_sync(device_id: UUID, nb_url: str, nb_token: str, bmc_user: str, bmc_pass: str): nautobot = Nautobot(nb_url, nb_token, logger=logger) - oob_ip = nautobot.device_oob_ip(device_name) - oob = oob_sushy_session(oob_ip, oob_username, oob_password) + oob_ip = nautobot.device_oob_ip(device_id) + oob = oob_sushy_session(oob_ip, bmc_user, bmc_pass) chassis = Chassis.from_redfish(oob) @@ -30,7 +44,18 @@ def main(): interface for interface in chassis.network_interfaces if is_off_board(interface) ] - nautobot.bulk_create_interfaces(device_name, interfaces) + nautobot.bulk_create_interfaces(device_id, interfaces) + + +def main(): + parser = argument_parser(__file__) + args = parser.parse_args() + + nb_token = args.nautobot_token or credential("nb-token", "token") + bmc_user = args.oob_username or credential("oob-secrets", "username") + bmc_pass = args.oob_password or credential("oob-secrets", "password") + + do_sync(args.device_id, args.nautobot_url, nb_token, bmc_user, bmc_pass) if __name__ == "__main__": diff --git a/python/understack-workflows/understack_workflows/nautobot.py b/python/understack-workflows/understack_workflows/nautobot.py index ee4866b74..d8ada3539 100644 --- a/python/understack-workflows/understack_workflows/nautobot.py +++ b/python/understack-workflows/understack_workflows/nautobot.py @@ -1,10 +1,10 @@ import logging import sys from typing import Protocol +from uuid import UUID 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 @@ -29,22 +29,16 @@ def exit_with_error(self, error): def api_session(self, url: str, token: str) -> NautobotApi: return pynautobot.api(url, token=token) - def device(self, device_name: str) -> NautobotDevice: - device = self.session.dcim.devices.get(name=device_name) - if not device: - self.exit_with_error(f"Device {device_name} not found in Nautobot") - return device - def device_oob_interface( self, - device: NautobotDevice, + device_id: UUID, ) -> NautobotInterface: oob_intf = self.session.dcim.interfaces.get( - device_id=device.id, name=["iDRAC", "iLO"] + device_id=device_id, name=["iDRAC", "iLO"] ) if not oob_intf: self.exit_with_error( - f"No OOB interfaces found for {device.name} in Nautobot" + f"No OOB interfaces found for device {device_id!s} in Nautobot" ) return oob_intf @@ -56,17 +50,15 @@ def ip_from_interface(self, interface: NautobotInterface) -> str: ) return ips[0].host - def device_oob_ip(self, device_name: str) -> str: - device = self.device(device_name) - oob_intf = self.device_oob_interface(device) + def device_oob_ip(self, device_id: UUID) -> str: + oob_intf = self.device_oob_interface(device_id) oob_ip = self.ip_from_interface(oob_intf) return oob_ip def construct_interfaces_payload( self, interfaces: list[Interface], - device_id: str, - device_name: str, + device_id: UUID, ) -> list[dict]: payload = [] for interface in interfaces: @@ -75,19 +67,20 @@ def construct_interfaces_payload( ) if nautobot_intf is None: self.logger.info( - f"{interface.name} was NOT found for {device_name}, creating..." + f"{interface.name} was NOT found for device {device_id!s}, " + f"creating..." ) payload.append(self.interface_payload_data(device_id, interface)) else: self.logger.info( f"{nautobot_intf.name} found in Nautobot for " - f"{device_name}, no action will be taken." + f"device {device_id!s}, no action will be taken." ) return payload - def interface_payload_data(self, device_id: str, interface: Interface) -> dict: + def interface_payload_data(self, device_id: UUID, interface: Interface) -> dict: return { - "device": device_id, + "device": str(device_id), "name": interface.name, "mac_address": interface.mac_addr, "type": "other", @@ -96,10 +89,9 @@ def interface_payload_data(self, device_id: str, interface: Interface) -> dict: } def bulk_create_interfaces( - self, device_name: str, interfaces: list[Interface] + self, device_id: UUID, interfaces: list[Interface] ) -> list[NautobotInterface] | None: - device = self.device(device_name) - payload = self.construct_interfaces_payload(interfaces, device.id, device.name) + payload = self.construct_interfaces_payload(interfaces, device_id) if payload: try: req = self.session.dcim.interfaces.create(payload)