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.

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

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

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:

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

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,22 +20,22 @@ spec:
             arguments:
               parameters:
                 - name: hostname
-                  value: "{{workflow.parameters.device_hostname}}"
+                  value: "{{workflow.parameters.hostname}}"
         - - name: get-obm-creds-secret
             templateRef:
               name: get-obm-creds
               template: main
             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}}"

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

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"]:

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,29 +11,51 @@
 
 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)
 
     interfaces = [
         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__":

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)