From 5c9336cac35277e18c5dd39f8de4a352d2b7080a Mon Sep 17 00:00:00 2001
From: Kenny Hackworth <khackwor@rackspace.com>
Date: Fri, 23 Oct 2026 11:32:43 -0400
Subject: [PATCH 01/12] feat: Add LLDP port discovery to PAN-OS inspect
 interface

Adds automatic network port discovery to PAN-OS firewalls during
inspection using LLDP neighbor discovery.

Implementation:
1. Removes factory default zone and virtual-wire configurations that
   would block interface configuration
2. Enables LLDP globally and configures it on all physical interfaces
3. Waits 30 seconds for LLDP neighbor discovery
4. Creates Ironic ports with local_link_connection metadata including:
   - switch_id (chassis ID)
   - port_id (remote port identifier)
   - switch_info (normalized FQDN switch name)
5. Derives VLAN groups from switch names using existing conventions
6. Removes temporary LLDP configuration to clean up

Port creation:
- Handles re-inspection gracefully by checking for existing ports
  by both MAC address and port name
- Normalizes switch names to FQDN format (adds .rackspace.net suffix)
  to match baremetal server port format
- Uses correct config group path for switch_name_vlan_group_mapping

IMPORTANT: This modifies device configuration and deletes factory
default settings. It should ONLY run during initial enrollment when
the device is in 'manageable' state, before any production
configuration exists. The inspect interface is only called during
enrollment, never on already-configured devices.
---
 components/ironic/values.yaml                 |   7 +
 .../drivers/panos_inspect.py                  | 333 +++++++++++++++++-
 .../ironic_understack/utils/panos_api.py      | 284 +++++++++++++++
 3 files changed, 615 insertions(+), 9 deletions(-)

diff --git a/components/ironic/values.yaml b/components/ironic/values.yaml
index c28e48562..053be9221 100644
--- a/components/ironic/values.yaml
+++ b/components/ironic/values.yaml
@@ -1,4 +1,11 @@
 ---
+# Override image tag for PR testing
+images:
+  tags:
+    ironic_conductor: ghcr.io/rackerlabs/understack/ironic:pr-2564
+    ironic_api: ghcr.io/rackerlabs/understack/ironic:pr-2564
+    ironic_pxe: ghcr.io/rackerlabs/understack/ironic:pr-2564
+
 bootstrap:
   image:
     enabled: false
diff --git a/python/ironic-understack/ironic_understack/drivers/panos_inspect.py b/python/ironic-understack/ironic_understack/drivers/panos_inspect.py
index 972cbbf82..e3b9fe6ad 100644
--- a/python/ironic-understack/ironic_understack/drivers/panos_inspect.py
+++ b/python/ironic-understack/ironic_understack/drivers/panos_inspect.py
@@ -1,14 +1,17 @@
 """Inspect interface for PAN-OS (Palo Alto) appliances."""
 
+import time
 from typing import ClassVar
 
 import requests
 from ironic.common import exception
 from ironic.common import states
 from ironic.drivers import base
+from ironic.objects import port
 from oslo_log import log
 
 from ironic_understack.utils import panos_api
+from ironic_understack.utils import vlan_group_name_convention
 
 LOG = log.getLogger(__name__)
 
@@ -50,15 +53,18 @@ def inspect_hardware(self, task):
 
         _apply_system_info(node, system)
 
-        # TODO: discover data plane connectivity and create ports from it:
-        # - show interface all, for the data plane interfaces and their MACs
-        # - LLDP neighbors. Interfaces without config don't transmit LLDP, so
-        #   this needs to enable LLDP and a transmit-receive profile on them,
-        #   commit, wait for neighbors, collect them, then restore the
-        #   original config and commit again.
-        # - an Ironic port per neighbor named "<node name>:<interface>", with
-        #   local_link_connection from the neighbor and the remote interface
-        #   as extra["bios_name"]
+        # Discover data plane connectivity via LLDP
+        try:
+            _discover_and_update_ports(task, client)
+        except Exception as e:
+            LOG.warning(
+                "[node:%s] Could not discover ports via LLDP: %s",
+                node.uuid,
+                e,
+                exc_info=True,
+            )
+            # Don't fail inspect if port discovery fails
+
         # TODO: show high-availability all, recording the HA peer serial in
         # extra["mate_serial"] (which enroll-fw also sets) when HA is enabled.
 
@@ -95,3 +101,312 @@ def _apply_system_info(node, system: dict[str, str]) -> None:
         properties["firmware_version"] = system["sw-version"]
     node.properties = properties
     node.extra = extra
+
+
+def _discover_and_update_ports(task, client: panos_api.PanosClient) -> None:
+    """Discover interfaces and LLDP neighbors, then update/create Ironic ports.
+
+    This temporarily enables LLDP on unconfigured interfaces, collects neighbors,
+    then restores the original configuration.
+    """
+    node = task.node
+
+    # Step 1: Get current interface configuration to determine what needs LLDP
+    configured_interfaces = _get_configured_interfaces(client)
+    LOG.debug(
+        "[node:%s] Found %d configured interfaces",
+        node.uuid,
+        len(configured_interfaces),
+    )
+
+    # Step 2: Get all physical interfaces based on model
+    model = node.properties.get("model", "PA-1410")
+    all_interfaces = _get_all_physical_interfaces(model)
+    interfaces_to_configure = [
+        iface for iface in all_interfaces if iface not in configured_interfaces
+    ]
+
+    if not interfaces_to_configure:
+        LOG.info(
+            "[node:%s] All interfaces configured, skipping temporary LLDP setup",
+            node.uuid,
+        )
+        # Still collect whatever LLDP is already there
+        interface_macs = client.get_interface_macs()
+        lldp_neighbors = client.get_lldp_neighbors()
+        _update_ports_from_neighbors(task, lldp_neighbors, interface_macs)
+        return
+
+    LOG.debug(
+        "[node:%s] Will configure LLDP on %d unconfigured interfaces",
+        node.uuid,
+        len(interfaces_to_configure),
+    )
+
+    # Step 3: Enable LLDP and configure unconfigured interfaces
+    _enable_lldp_discovery(client, interfaces_to_configure)
+
+    # Step 4: Wait for LLDP neighbors
+    LOG.debug("[node:%s] Waiting 30s for LLDP neighbor discovery", node.uuid)
+    time.sleep(30)
+
+    # Step 5: Collect interface MACs and LLDP neighbors
+    interface_macs = client.get_interface_macs()
+    lldp_neighbors = client.get_lldp_neighbors()
+    LOG.info("[node:%s] Discovered %d LLDP neighbors", node.uuid, len(lldp_neighbors))
+
+    # Step 6: Restore original configuration
+    _restore_interface_config(client, interfaces_to_configure)
+
+    # Step 7: Update Ironic ports
+    _update_ports_from_neighbors(task, lldp_neighbors, interface_macs)
+
+
+def _get_configured_interfaces(client: panos_api.PanosClient) -> set[str]:
+    """Get set of interfaces that have any configuration."""
+    configured = set()
+
+    # Check ethernet interface config
+    try:
+        xpath = (
+            "/config/devices/entry[@name='localhost.localdomain']"
+            "/network/interface/ethernet"
+        )
+        result = client.config("get", xpath)
+
+        # Parse XML result
+        for entry in result.findall(".//entry"):
+            name = entry.get("name")
+            if name and any(
+                entry.find(tag) is not None
+                for tag in [
+                    "layer3",
+                    "layer2",
+                    "tap",
+                    "ha",
+                    "aggregate-ethernet",
+                    "virtual-wire",
+                ]
+            ):
+                configured.add(name)
+    except panos_api.PanosApiError as e:
+        LOG.warning("Could not check interface configuration: %s", e)
+
+    # Check virtual-wire configurations
+    try:
+        xpath = (
+            "/config/devices/entry[@name='localhost.localdomain']/network/virtual-wire"
+        )
+        result = client.config("get", xpath)
+
+        for entry in result.findall(".//entry"):
+            for tag in ["interface1", "interface2"]:
+                iface = entry.findtext(tag)
+                if iface:
+                    configured.add(iface)
+    except panos_api.PanosApiError as e:
+        LOG.debug("Could not check virtual-wire configuration: %s", e)
+
+    return configured
+
+
+def _get_all_physical_interfaces(model: str) -> list[str]:
+    """Get list of all physical ethernet interfaces based on model."""
+    if "1410" in model:
+        return [f"ethernet1/{i}" for i in range(1, 23)]  # PA-1410 has 22 ports
+    # Default to 22 ports for unknown models
+    return [f"ethernet1/{i}" for i in range(1, 23)]
+
+
+def _enable_lldp_discovery(
+    client: panos_api.PanosClient, interfaces: list[str]
+) -> None:
+    """Enable LLDP globally and configure it on specified interfaces."""
+    # Remove factory default config that blocks interface configuration
+    # Must delete zones first (they reference virtual-wires)
+    try:
+        xpath = (
+            "/config/devices/entry[@name='localhost.localdomain']"
+            "/vsys/entry[@name='vsys1']/zone"
+        )
+        client.config("delete", xpath, None)
+    except Exception as e:
+        LOG.debug("No zone config to remove: %s", e)
+
+    try:
+        xpath = (
+            "/config/devices/entry[@name='localhost.localdomain']"
+            "/network/virtual-wire"
+        )
+        client.config("delete", xpath, None)
+    except Exception as e:
+        LOG.debug("No virtual-wire config to remove: %s", e)
+
+    # Commit factory default cleanup
+    job_id = client.commit("Inspect: Remove factory default config")
+    if job_id:
+        client.wait_for_commit(job_id)
+
+    # Enable LLDP globally
+    xpath = "/config/devices/entry[@name='localhost.localdomain']/network/lldp"
+    element = "<enable>yes</enable>"
+    client.config("set", xpath, element)
+
+    # Create LLDP profile
+    xpath = (
+        "/config/devices/entry[@name='localhost.localdomain']"
+        "/network/profiles/lldp-profile/entry[@name='lldp-inspect']"
+    )
+    element = "<mode>transmit-receive</mode>"
+    client.config("set", xpath, element)
+
+    # Commit LLDP profile
+    job_id = client.commit("Inspect: Enable LLDP profile")
+    if job_id:
+        client.wait_for_commit(job_id)
+
+    # Configure interfaces with LLDP
+    for iface in interfaces:
+        xpath = (
+            f"/config/devices/entry[@name='localhost.localdomain']"
+            f"/network/interface/ethernet/entry[@name='{iface}']"
+        )
+        element = (
+            "<layer3>"
+            "<lldp><enable>yes</enable><profile>lldp-inspect</profile></lldp>"
+            "</layer3>"
+            "<link-speed>auto</link-speed>"
+            "<link-state>up</link-state>"
+        )
+        client.config("set", xpath, element)
+
+    # Commit interface changes
+    job_id = client.commit("Inspect: Enable interfaces for LLDP discovery")
+    if job_id:
+        client.wait_for_commit(job_id)
+
+
+def _restore_interface_config(
+    client: panos_api.PanosClient, interfaces: list[str]
+) -> None:
+    """Remove temporary LLDP configuration from interfaces."""
+    for iface in interfaces:
+        xpath = (
+            f"/config/devices/entry[@name='localhost.localdomain']"
+            f"/network/interface/ethernet/entry[@name='{iface}']"
+        )
+        try:
+            client.config("delete", xpath)
+        except panos_api.PanosApiError as e:
+            LOG.warning("Could not delete interface %s config: %s", iface, e)
+
+    # Commit restore
+    job_id = client.commit("Inspect: Restore original interface configuration")
+    if job_id:
+        client.wait_for_commit(job_id)
+
+
+def _update_ports_from_neighbors(
+    task, neighbors: list[dict[str, str]], interface_macs: dict[str, str]
+) -> None:
+    """Create or update Ironic ports based on LLDP neighbor data."""
+    from ironic_understack import conf
+    from ironic_understack.utils.inspected_port import InspectedPort
+
+    node = task.node
+    CONF = conf.CONF
+
+    def _normalize_switch_name(name: str) -> str:
+        """Normalize switch name to FQDN format matching baremetal servers."""
+        suffix = ".rackspace.net"
+        name = str(name).lower()
+        return name if name.endswith(suffix) else name + suffix
+
+    # First, create InspectedPort objects for all neighbors with MACs
+    inspected_ports = []
+    neighbor_by_mac = {}
+
+    for neighbor in neighbors:
+        local_interface = neighbor["local_interface"]
+        mac_address = interface_macs.get(local_interface)
+
+        if not mac_address:
+            LOG.warning(
+                "[node:%s] No MAC address for %s, skipping port creation",
+                node.uuid,
+                local_interface,
+            )
+            continue
+
+        normalized_switch_name = _normalize_switch_name(neighbor["system_name"])
+
+        inspected_port = InspectedPort(
+            mac_address=mac_address,
+            name=local_interface,
+            switch_system_name=normalized_switch_name,
+            switch_port_id=neighbor["port_id"],
+            switch_chassis_id=neighbor["chassis_id"],
+        )
+        inspected_ports.append(inspected_port)
+        neighbor_by_mac[mac_address.lower()] = neighbor
+
+    if not inspected_ports:
+        LOG.info("[node:%s] No ports with MAC addresses to create", node.uuid)
+        return
+
+    # Get VLAN group names for all ports
+    try:
+        vlan_group_mapping = vlan_group_name_convention.vlan_group_names(
+            inspected_ports, CONF.ironic_understack.switch_name_vlan_group_mapping
+        )
+    except Exception as e:
+        LOG.warning(
+            "[node:%s] Could not derive VLAN groups: %s. "
+            "Creating ports without physical_network.",
+            node.uuid,
+            e,
+        )
+        vlan_group_mapping = {}
+
+    # Now create or update ports
+    for inspected_port in inspected_ports:
+        mac_address = inspected_port.mac_address
+        neighbor = neighbor_by_mac[mac_address.lower()]
+        local_interface = inspected_port.name
+
+        physical_network = vlan_group_mapping.get(neighbor["system_name"])
+        port_name = f"{node.name}:{local_interface}"
+
+        # Check if port already exists (by MAC or name)
+        existing_port = None
+        for p in task.ports:
+            if p.address.lower() == mac_address.lower():
+                existing_port = p
+                break
+            if p.name == port_name:
+                existing_port = p
+                break
+
+        if existing_port:
+            # Update existing port
+            existing_port.address = mac_address
+            existing_port.local_link_connection = inspected_port.local_link_connection
+            existing_port.extra = {"bios_name": local_interface}
+            existing_port.physical_network = physical_network
+            existing_port.name = port_name
+            existing_port.save()
+            LOG.debug("[node:%s] Updated port %s", node.uuid, port_name)
+        else:
+            # Create new port
+            port_obj = port.Port(task.context)
+            port_obj.node_id = node.id
+            port_obj.address = mac_address
+            port_obj.name = port_name
+            port_obj.local_link_connection = inspected_port.local_link_connection
+            port_obj.extra = {"bios_name": local_interface}
+            port_obj.physical_network = physical_network
+            port_obj.pxe_enabled = True
+            port_obj.create()
+            LOG.info(
+                "[node:%s] Created port %s (%s)", node.uuid, port_name, mac_address
+            )
diff --git a/python/ironic-understack/ironic_understack/utils/panos_api.py b/python/ironic-understack/ironic_understack/utils/panos_api.py
index 5b6bde78f..cfa652e78 100644
--- a/python/ironic-understack/ironic_understack/utils/panos_api.py
+++ b/python/ironic-understack/ironic_understack/utils/panos_api.py
@@ -28,6 +28,33 @@
 )
 
 
+def _xml_to_dict(element: ET.Element) -> dict | str | list:
+    """Convert an XML element tree to nested dicts/lists/strings.
+
+    Handles the PAN-OS XML response format where repeated elements
+    become lists and single elements become dicts.
+    """
+    children = list(element)
+    if not children:
+        # Leaf node - return the text content
+        return (element.text or "").strip()
+
+    # Group children by tag name
+    grouped = {}
+    for child in children:
+        tag = child.tag
+        if tag not in grouped:
+            grouped[tag] = []
+        grouped[tag].append(_xml_to_dict(child))
+
+    # Convert single-element lists to scalars
+    result = {}
+    for tag, values in grouped.items():
+        result[tag] = values[0] if len(values) == 1 else values
+
+    return result
+
+
 class PanosApiError(Exception):
     """The PAN-OS API rejected a request or returned an unusable response."""
 
@@ -99,6 +126,263 @@ def system_info(self) -> dict[str, str]:
             raise PanosApiError("show system info returned no system element")
         return {child.tag: (child.text or "").strip() for child in system}
 
+    def config(self, action: str, xpath: str, element: str | None = None) -> ET.Element:
+        """Execute a config API command (set, edit, delete, get, show).
+
+        :param action: Config action (set, edit, delete, get, show)
+        :param xpath: XPath to the config element
+        :param element: XML element string for set/edit actions
+        :returns: The response result element
+        """
+        if self.api_key is None:
+            raise PanosApiError("not logged in")
+
+        params = {
+            "type": "config",
+            "key": self.api_key,
+            "action": action,
+            "xpath": xpath,
+        }
+        if element and action in ("set", "edit"):
+            params["element"] = element
+
+        LOG.debug(
+            "PAN-OS %s config %s: xpath=%s element=%s",
+            self.address,
+            action,
+            xpath,
+            element[:100] if element else None,
+        )
+
+        resp = self.session.get(
+            self.url,
+            params=params,
+            timeout=TIMEOUT,
+        )
+        root = _parse(resp)
+        result = root.find("result")
+
+        LOG.debug(
+            "PAN-OS %s config %s: status=%s, full response=%s",
+            self.address,
+            action,
+            root.get("status"),
+            ET.tostring(root, encoding="unicode")[:500],
+        )
+
+        if result is None:
+            # For delete/set operations, success response may not have a result
+            return root
+        return result
+
+    def commit(self, description: str | None = None) -> str | None:
+        """Commit configuration changes.
+
+        :param description: Optional commit description
+        :returns: Job ID or None if no changes to commit
+        :raises PanosApiError: If commit fails
+        """
+        if self.api_key is None:
+            raise PanosApiError("not logged in")
+
+        cmd = "<commit></commit>"
+        if description:
+            cmd = f"<commit><description>{description}</description></commit>"
+
+        LOG.debug(
+            "PAN-OS %s committing: %s", self.address, description or "no description"
+        )
+
+        resp = self.session.get(
+            self.url,
+            params={"type": "commit", "key": self.api_key, "cmd": cmd},
+            timeout=120,
+        )
+        root = _parse(resp)
+
+        LOG.debug(
+            "PAN-OS %s commit response: status=%s, code=%s, msg=%s, full_xml=%s",
+            self.address,
+            root.get("status"),
+            root.get("code"),
+            root.findtext("msg"),
+            ET.tostring(root, encoding="unicode")[:800],
+        )
+
+        # Find job ID first
+        job = root.findtext(".//job") or root.findtext("result/job")
+
+        # Check for "no changes to commit" (code 19 with specific message)
+        if root.get("code") == "19":
+            msg = root.findtext("msg") or ""
+            if "no changes" in msg.lower():
+                LOG.debug(
+                    "PAN-OS %s commit returned code 19 (no changes): %s",
+                    self.address,
+                    msg,
+                )
+                return None
+            else:
+                LOG.debug(
+                    "PAN-OS %s commit returned code 19 but message doesn't "
+                    "indicate 'no changes': %s",
+                    self.address,
+                    msg,
+                )
+        if not job:
+            LOG.error(
+                "PAN-OS %s commit response missing job ID. Root: %s",
+                self.address,
+                ET.tostring(root, encoding="unicode"),
+            )
+            raise PanosApiError("Could not find job ID in commit response")
+
+        LOG.debug("PAN-OS %s commit started, job ID: %s", self.address, job)
+        return job
+
+    def check_job(self, job_id: str) -> dict[str, str]:
+        """Check the status of a commit job.
+
+        :param job_id: Job ID from commit()
+        :returns: Dict with status, result, progress, details
+        """
+        result = self.op(f"<show><jobs><id>{job_id}</id></jobs></show>")
+        job = result.find("job")
+        if job is None:
+            return {
+                "status": "unknown",
+                "result": "unknown",
+                "progress": "0",
+                "details": "",
+            }
+
+        return {
+            "status": job.findtext("status") or "unknown",
+            "result": job.findtext("result") or "unknown",
+            "progress": job.findtext("progress") or "0",
+            "details": job.findtext("details") or "",
+        }
+
+    def wait_for_commit(self, job_id: str, timeout: int = 60) -> bool:
+        """Wait for a commit job to complete.
+
+        :param job_id: Job ID from commit()
+        :param timeout: Max seconds to wait
+        :returns: True if commit succeeded, False otherwise
+        """
+        import time
+
+        for _ in range(timeout // 2):
+            time.sleep(2)
+            status = self.check_job(job_id)
+
+            if status["status"] == "FIN":
+                if status["result"] == "OK":
+                    LOG.debug("PAN-OS %s commit job %s succeeded", self.address, job_id)
+                    return True
+                else:
+                    LOG.error(
+                        "PAN-OS %s commit failed: %s", self.address, status["result"]
+                    )
+                    return False
+
+        LOG.error("PAN-OS %s commit timed out", self.address)
+        return False
+
+    def get_interface_macs(self) -> dict[str, str]:
+        """Get MAC addresses for all interfaces.
+
+        :returns: Dict mapping interface name to MAC address
+        """
+        result = self.op("<show><interface>all</interface></show>")
+        macs = {}
+
+        hw = result.find("hw")
+        if hw is not None:
+            for entry in hw.findall("entry"):
+                name = entry.findtext("name")
+                mac = entry.findtext("mac")
+                if name and mac:
+                    macs[name] = mac
+
+        return macs
+
+    def get_lldp_neighbors(self) -> list[dict[str, str]]:
+        """Get LLDP neighbor information for all interfaces.
+
+        :returns: List of dicts with keys: local_interface, chassis_id,
+                  port_id, system_name
+        """
+        # Try both command formats
+        result = None
+        for cmd in [
+            "<show><lldp><neighbors>all</neighbors></lldp></show>",
+            "<show><lldp><neighbors/></lldp></show>",
+        ]:
+            try:
+                result = self.op(cmd)
+                break
+            except PanosApiError:
+                continue
+
+        if result is None:
+            return []
+
+        # Convert XML to dict for easier parsing (same as panos.py script)
+        result_dict = _xml_to_dict(result)
+
+        # Extract entries - handle both single entry (dict) and multiple (list)
+        entries = []
+        if isinstance(result_dict, dict) and "entry" in result_dict:
+            entry_data = result_dict["entry"]
+            entries = entry_data if isinstance(entry_data, list) else [entry_data]
+
+        neighbors = []
+        for entry in entries:
+            if not isinstance(entry, dict):
+                continue
+
+            # Get local interface info
+            local_info = entry.get("local", {})
+            if not isinstance(local_info, dict):
+                continue
+
+            port_id = local_info.get("port-id", "")
+            if not port_id:
+                continue
+
+            local_interface = f"ethernet1/{port_id}"
+
+            # Get neighbor info
+            neighbor_info = entry.get("neighbors", {})
+            if not neighbor_info or neighbor_info == "":
+                # No neighbor on this interface - skip it
+                continue
+
+            # neighbors contains an entry element
+            if not isinstance(neighbor_info, dict):
+                continue
+
+            neighbor_entry = neighbor_info.get("entry", {})
+            if not neighbor_entry or not isinstance(neighbor_entry, dict):
+                LOG.debug(
+                    "PAN-OS %s LLDP skipping entry - neighbor_entry invalid: %s",
+                    self.address,
+                    neighbor_entry,
+                )
+                continue
+
+            neighbors.append(
+                {
+                    "local_interface": local_interface,
+                    "chassis_id": neighbor_entry.get("chassis-id", ""),
+                    "port_id": neighbor_entry.get("port-id", ""),
+                    "system_name": neighbor_entry.get("system-name", ""),
+                }
+            )
+
+        return neighbors
+
 
 def _parse(resp: requests.Response) -> ET.Element:
     try:

From 6fabfae8087bcdccbca194e286328c7b1bbcf264 Mon Sep 17 00:00:00 2001
From: Kenny Hackworth <khackwor@rackspace.com>
Date: Fri, 23 Oct 2026 13:42:40 -0400
Subject: [PATCH 02/12] fix: handle virtual-wire interfaces and improve commit
 error handling

- Remove virtual-wire config from each interface before configuring as layer3
- Only commit factory cleanup if we actually deleted something
- Check commit return values and fail loudly with proper errors
- Include job details in commit failure error messages

This fixes inspection failures on devices with virtual-wire interfaces
configured, where commits were silently failing and code continued anyway.
---
 .../drivers/panos_inspect.py                  | 43 ++++++++++++++-----
 1 file changed, 33 insertions(+), 10 deletions(-)

diff --git a/python/ironic-understack/ironic_understack/drivers/panos_inspect.py b/python/ironic-understack/ironic_understack/drivers/panos_inspect.py
index e3b9fe6ad..f85023ee6 100644
--- a/python/ironic-understack/ironic_understack/drivers/panos_inspect.py
+++ b/python/ironic-understack/ironic_understack/drivers/panos_inspect.py
@@ -224,12 +224,16 @@ def _enable_lldp_discovery(
     """Enable LLDP globally and configure it on specified interfaces."""
     # Remove factory default config that blocks interface configuration
     # Must delete zones first (they reference virtual-wires)
+    zones_deleted = False
+    vwires_deleted = False
+
     try:
         xpath = (
             "/config/devices/entry[@name='localhost.localdomain']"
             "/vsys/entry[@name='vsys1']/zone"
         )
         client.config("delete", xpath, None)
+        zones_deleted = True
     except Exception as e:
         LOG.debug("No zone config to remove: %s", e)
 
@@ -239,13 +243,28 @@ def _enable_lldp_discovery(
             "/network/virtual-wire"
         )
         client.config("delete", xpath, None)
+        vwires_deleted = True
     except Exception as e:
         LOG.debug("No virtual-wire config to remove: %s", e)
 
-    # Commit factory default cleanup
-    job_id = client.commit("Inspect: Remove factory default config")
-    if job_id:
-        client.wait_for_commit(job_id)
+    # Remove virtual-wire config from interfaces we want to configure
+    for iface in interfaces:
+        try:
+            xpath = (
+                f"/config/devices/entry[@name='localhost.localdomain']"
+                f"/network/interface/ethernet/entry[@name='{iface}']/virtual-wire"
+            )
+            client.config("delete", xpath, None)
+        except Exception as e:
+            LOG.debug("No virtual-wire config on %s: %s", iface, e)
+
+    # Only commit if we actually deleted something
+    if zones_deleted or vwires_deleted:
+        job_id = client.commit("Inspect: Remove factory default config")
+        if job_id and not client.wait_for_commit(job_id):
+            raise panos_api.PanosApiError(
+                "Failed to commit factory default cleanup configuration"
+            )
 
     # Enable LLDP globally
     xpath = "/config/devices/entry[@name='localhost.localdomain']/network/lldp"
@@ -262,8 +281,8 @@ def _enable_lldp_discovery(
 
     # Commit LLDP profile
     job_id = client.commit("Inspect: Enable LLDP profile")
-    if job_id:
-        client.wait_for_commit(job_id)
+    if job_id and not client.wait_for_commit(job_id):
+        raise panos_api.PanosApiError("Failed to commit LLDP profile configuration")
 
     # Configure interfaces with LLDP
     for iface in interfaces:
@@ -282,8 +301,10 @@ def _enable_lldp_discovery(
 
     # Commit interface changes
     job_id = client.commit("Inspect: Enable interfaces for LLDP discovery")
-    if job_id:
-        client.wait_for_commit(job_id)
+    if job_id and not client.wait_for_commit(job_id):
+        raise panos_api.PanosApiError(
+            "Failed to commit interface configuration for LLDP"
+        )
 
 
 def _restore_interface_config(
@@ -302,8 +323,10 @@ def _restore_interface_config(
 
     # Commit restore
     job_id = client.commit("Inspect: Restore original interface configuration")
-    if job_id:
-        client.wait_for_commit(job_id)
+    if job_id and not client.wait_for_commit(job_id):
+        raise panos_api.PanosApiError(
+            "Failed to commit interface restoration configuration"
+        )
 
 
 def _update_ports_from_neighbors(

From c61c6d5360497f7884580d02d8b2b805bcc52fe0 Mon Sep 17 00:00:00 2001
From: Kenny Hackworth <khackwor@rackspace.com>
Date: Fri, 23 Oct 2026 13:57:07 -0400
Subject: [PATCH 03/12] fix: delete security rules before zones during LLDP
 setup

Security rules reference zones, so they must be deleted first to avoid
validation errors when cleaning up factory/staging config for LLDP discovery.

This allows inspect to work on devices that have been through a staging
station that adds generic security rules.
---
 .../ironic_understack/drivers/panos_inspect.py  | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/python/ironic-understack/ironic_understack/drivers/panos_inspect.py b/python/ironic-understack/ironic_understack/drivers/panos_inspect.py
index f85023ee6..01ee19fdd 100644
--- a/python/ironic-understack/ironic_understack/drivers/panos_inspect.py
+++ b/python/ironic-understack/ironic_understack/drivers/panos_inspect.py
@@ -223,10 +223,23 @@ def _enable_lldp_discovery(
 ) -> None:
     """Enable LLDP globally and configure it on specified interfaces."""
     # Remove factory default config that blocks interface configuration
-    # Must delete zones first (they reference virtual-wires)
+    # Must delete security rules first (they reference zones),
+    # then zones (they reference virtual-wires)
+    rules_deleted = False
     zones_deleted = False
     vwires_deleted = False
 
+    try:
+        xpath = (
+            "/config/devices/entry[@name='localhost.localdomain']"
+            "/vsys/entry[@name='vsys1']/rulebase/security/rules"
+        )
+        client.config("delete", xpath, None)
+        rules_deleted = True
+        LOG.debug("Deleted security rules")
+    except Exception as e:
+        LOG.debug("No security rules to remove: %s", e)
+
     try:
         xpath = (
             "/config/devices/entry[@name='localhost.localdomain']"
@@ -259,7 +272,7 @@ def _enable_lldp_discovery(
             LOG.debug("No virtual-wire config on %s: %s", iface, e)
 
     # Only commit if we actually deleted something
-    if zones_deleted or vwires_deleted:
+    if rules_deleted or zones_deleted or vwires_deleted:
         job_id = client.commit("Inspect: Remove factory default config")
         if job_id and not client.wait_for_commit(job_id):
             raise panos_api.PanosApiError(

From dc9eeaae1ed23f913580fadce65337df939b9a0a Mon Sep 17 00:00:00 2001
From: Kenny Hackworth <khackwor@rackspace.com>
Date: Fri, 23 Oct 2026 14:12:23 -0400
Subject: [PATCH 04/12] refactor: configure LLDP on all interfaces
 unconditionally

Simplified LLDP discovery to configure all interfaces instead of trying
to detect which ones are already configured. Since the device must be in
manageable (not production) state, we can be destructive and just blast
through all interfaces.

This fixes the issue where ethernet1/1 and ethernet1/2 had virtual-wire
config that was detected as 'configured', causing them to be skipped,
which meant they never got LLDP enabled.
---
 .../drivers/panos_inspect.py                  | 103 +++---------------
 1 file changed, 17 insertions(+), 86 deletions(-)

diff --git a/python/ironic-understack/ironic_understack/drivers/panos_inspect.py b/python/ironic-understack/ironic_understack/drivers/panos_inspect.py
index 01ee19fdd..3b88586db 100644
--- a/python/ironic-understack/ironic_understack/drivers/panos_inspect.py
+++ b/python/ironic-understack/ironic_understack/drivers/panos_inspect.py
@@ -106,110 +106,40 @@ def _apply_system_info(node, system: dict[str, str]) -> None:
 def _discover_and_update_ports(task, client: panos_api.PanosClient) -> None:
     """Discover interfaces and LLDP neighbors, then update/create Ironic ports.
 
-    This temporarily enables LLDP on unconfigured interfaces, collects neighbors,
+    This temporarily enables LLDP on all interfaces, collects neighbors,
     then restores the original configuration.
     """
     node = task.node
 
-    # Step 1: Get current interface configuration to determine what needs LLDP
-    configured_interfaces = _get_configured_interfaces(client)
-    LOG.debug(
-        "[node:%s] Found %d configured interfaces",
-        node.uuid,
-        len(configured_interfaces),
-    )
-
-    # Step 2: Get all physical interfaces based on model
+    # Get all physical interfaces based on model
     model = node.properties.get("model", "PA-1410")
     all_interfaces = _get_all_physical_interfaces(model)
-    interfaces_to_configure = [
-        iface for iface in all_interfaces if iface not in configured_interfaces
-    ]
-
-    if not interfaces_to_configure:
-        LOG.info(
-            "[node:%s] All interfaces configured, skipping temporary LLDP setup",
-            node.uuid,
-        )
-        # Still collect whatever LLDP is already there
-        interface_macs = client.get_interface_macs()
-        lldp_neighbors = client.get_lldp_neighbors()
-        _update_ports_from_neighbors(task, lldp_neighbors, interface_macs)
-        return
 
     LOG.debug(
-        "[node:%s] Will configure LLDP on %d unconfigured interfaces",
+        "[node:%s] Configuring LLDP on all %d interfaces",
         node.uuid,
-        len(interfaces_to_configure),
+        len(all_interfaces),
     )
 
-    # Step 3: Enable LLDP and configure unconfigured interfaces
-    _enable_lldp_discovery(client, interfaces_to_configure)
+    # Enable LLDP on all interfaces
+    _enable_lldp_discovery(client, all_interfaces)
 
-    # Step 4: Wait for LLDP neighbors
+    # Wait for LLDP neighbors
     LOG.debug("[node:%s] Waiting 30s for LLDP neighbor discovery", node.uuid)
     time.sleep(30)
 
-    # Step 5: Collect interface MACs and LLDP neighbors
+    # Collect interface MACs and LLDP neighbors
     interface_macs = client.get_interface_macs()
     lldp_neighbors = client.get_lldp_neighbors()
     LOG.info("[node:%s] Discovered %d LLDP neighbors", node.uuid, len(lldp_neighbors))
 
-    # Step 6: Restore original configuration
-    _restore_interface_config(client, interfaces_to_configure)
+    # Restore original configuration (delete all interface config we added)
+    _restore_interface_config(client, all_interfaces)
 
-    # Step 7: Update Ironic ports
+    # Update Ironic ports
     _update_ports_from_neighbors(task, lldp_neighbors, interface_macs)
 
 
-def _get_configured_interfaces(client: panos_api.PanosClient) -> set[str]:
-    """Get set of interfaces that have any configuration."""
-    configured = set()
-
-    # Check ethernet interface config
-    try:
-        xpath = (
-            "/config/devices/entry[@name='localhost.localdomain']"
-            "/network/interface/ethernet"
-        )
-        result = client.config("get", xpath)
-
-        # Parse XML result
-        for entry in result.findall(".//entry"):
-            name = entry.get("name")
-            if name and any(
-                entry.find(tag) is not None
-                for tag in [
-                    "layer3",
-                    "layer2",
-                    "tap",
-                    "ha",
-                    "aggregate-ethernet",
-                    "virtual-wire",
-                ]
-            ):
-                configured.add(name)
-    except panos_api.PanosApiError as e:
-        LOG.warning("Could not check interface configuration: %s", e)
-
-    # Check virtual-wire configurations
-    try:
-        xpath = (
-            "/config/devices/entry[@name='localhost.localdomain']/network/virtual-wire"
-        )
-        result = client.config("get", xpath)
-
-        for entry in result.findall(".//entry"):
-            for tag in ["interface1", "interface2"]:
-                iface = entry.findtext(tag)
-                if iface:
-                    configured.add(iface)
-    except panos_api.PanosApiError as e:
-        LOG.debug("Could not check virtual-wire configuration: %s", e)
-
-    return configured
-
-
 def _get_all_physical_interfaces(model: str) -> list[str]:
     """Get list of all physical ethernet interfaces based on model."""
     if "1410" in model:
@@ -221,10 +151,11 @@ def _get_all_physical_interfaces(model: str) -> list[str]:
 def _enable_lldp_discovery(
     client: panos_api.PanosClient, interfaces: list[str]
 ) -> None:
-    """Enable LLDP globally and configure it on specified interfaces."""
-    # Remove factory default config that blocks interface configuration
-    # Must delete security rules first (they reference zones),
-    # then zones (they reference virtual-wires)
+    """Enable LLDP globally and configure it on specified interfaces.
+
+    Destructively removes any existing config that would block LLDP setup.
+    """
+    # Remove all blocking config: security rules -> zones -> virtual-wires
     rules_deleted = False
     zones_deleted = False
     vwires_deleted = False
@@ -260,7 +191,7 @@ def _enable_lldp_discovery(
     except Exception as e:
         LOG.debug("No virtual-wire config to remove: %s", e)
 
-    # Remove virtual-wire config from interfaces we want to configure
+    # Remove virtual-wire config from all specified interfaces
     for iface in interfaces:
         try:
             xpath = (

From 6956f5979772687157471cf13927f22ba8ffc7a1 Mon Sep 17 00:00:00 2001
From: Kenny Hackworth <khackwor@rackspace.com>
Date: Fri, 23 Oct 2026 14:33:45 -0400
Subject: [PATCH 05/12] refactor: reduce excessive logging in LLDP discovery

- Remove verbose DEBUG logs showing full XML responses and config operations from panos_api.py
- Remove per-interface deletion logs (22 lines per run)
- Add single summary log showing which factory configs were deleted
- Use contextlib.suppress for cleaner exception handling
- Keep essential INFO logs for key events (neighbor discovery, port creation)
---
 .../drivers/panos_inspect.py                  | 25 ++++------
 .../ironic_understack/utils/panos_api.py      | 48 -------------------
 2 files changed, 8 insertions(+), 65 deletions(-)

diff --git a/python/ironic-understack/ironic_understack/drivers/panos_inspect.py b/python/ironic-understack/ironic_understack/drivers/panos_inspect.py
index 3b88586db..f0c4eedb0 100644
--- a/python/ironic-understack/ironic_understack/drivers/panos_inspect.py
+++ b/python/ironic-understack/ironic_understack/drivers/panos_inspect.py
@@ -44,7 +44,6 @@ def inspect_hardware(self, task):
         client = panos_api.PanosClient(address)
         try:
             client.login()
-            LOG.debug("[node:%s] Logged in to %s", node.uuid, address)
             system = client.system_info()
         except (panos_api.PanosApiError, requests.RequestException) as e:
             raise exception.HardwareInspectionFailure(
@@ -115,17 +114,10 @@ def _discover_and_update_ports(task, client: panos_api.PanosClient) -> None:
     model = node.properties.get("model", "PA-1410")
     all_interfaces = _get_all_physical_interfaces(model)
 
-    LOG.debug(
-        "[node:%s] Configuring LLDP on all %d interfaces",
-        node.uuid,
-        len(all_interfaces),
-    )
-
     # Enable LLDP on all interfaces
     _enable_lldp_discovery(client, all_interfaces)
 
     # Wait for LLDP neighbors
-    LOG.debug("[node:%s] Waiting 30s for LLDP neighbor discovery", node.uuid)
     time.sleep(30)
 
     # Collect interface MACs and LLDP neighbors
@@ -167,9 +159,8 @@ def _enable_lldp_discovery(
         )
         client.config("delete", xpath, None)
         rules_deleted = True
-        LOG.debug("Deleted security rules")
-    except Exception as e:
-        LOG.debug("No security rules to remove: %s", e)
+    except Exception:
+        pass
 
     try:
         xpath = (
@@ -178,8 +169,8 @@ def _enable_lldp_discovery(
         )
         client.config("delete", xpath, None)
         zones_deleted = True
-    except Exception as e:
-        LOG.debug("No zone config to remove: %s", e)
+    except Exception:
+        pass
 
     try:
         xpath = (
@@ -188,8 +179,8 @@ def _enable_lldp_discovery(
         )
         client.config("delete", xpath, None)
         vwires_deleted = True
-    except Exception as e:
-        LOG.debug("No virtual-wire config to remove: %s", e)
+    except Exception:
+        pass
 
     # Remove virtual-wire config from all specified interfaces
     for iface in interfaces:
@@ -199,8 +190,8 @@ def _enable_lldp_discovery(
                 f"/network/interface/ethernet/entry[@name='{iface}']/virtual-wire"
             )
             client.config("delete", xpath, None)
-        except Exception as e:
-            LOG.debug("No virtual-wire config on %s: %s", iface, e)
+        except Exception:
+            pass
 
     # Only commit if we actually deleted something
     if rules_deleted or zones_deleted or vwires_deleted:
diff --git a/python/ironic-understack/ironic_understack/utils/panos_api.py b/python/ironic-understack/ironic_understack/utils/panos_api.py
index cfa652e78..613351920 100644
--- a/python/ironic-understack/ironic_understack/utils/panos_api.py
+++ b/python/ironic-understack/ironic_understack/utils/panos_api.py
@@ -146,14 +146,6 @@ def config(self, action: str, xpath: str, element: str | None = None) -> ET.Elem
         if element and action in ("set", "edit"):
             params["element"] = element
 
-        LOG.debug(
-            "PAN-OS %s config %s: xpath=%s element=%s",
-            self.address,
-            action,
-            xpath,
-            element[:100] if element else None,
-        )
-
         resp = self.session.get(
             self.url,
             params=params,
@@ -162,14 +154,6 @@ def config(self, action: str, xpath: str, element: str | None = None) -> ET.Elem
         root = _parse(resp)
         result = root.find("result")
 
-        LOG.debug(
-            "PAN-OS %s config %s: status=%s, full response=%s",
-            self.address,
-            action,
-            root.get("status"),
-            ET.tostring(root, encoding="unicode")[:500],
-        )
-
         if result is None:
             # For delete/set operations, success response may not have a result
             return root
@@ -189,10 +173,6 @@ def commit(self, description: str | None = None) -> str | None:
         if description:
             cmd = f"<commit><description>{description}</description></commit>"
 
-        LOG.debug(
-            "PAN-OS %s committing: %s", self.address, description or "no description"
-        )
-
         resp = self.session.get(
             self.url,
             params={"type": "commit", "key": self.api_key, "cmd": cmd},
@@ -200,15 +180,6 @@ def commit(self, description: str | None = None) -> str | None:
         )
         root = _parse(resp)
 
-        LOG.debug(
-            "PAN-OS %s commit response: status=%s, code=%s, msg=%s, full_xml=%s",
-            self.address,
-            root.get("status"),
-            root.get("code"),
-            root.findtext("msg"),
-            ET.tostring(root, encoding="unicode")[:800],
-        )
-
         # Find job ID first
         job = root.findtext(".//job") or root.findtext("result/job")
 
@@ -216,19 +187,7 @@ def commit(self, description: str | None = None) -> str | None:
         if root.get("code") == "19":
             msg = root.findtext("msg") or ""
             if "no changes" in msg.lower():
-                LOG.debug(
-                    "PAN-OS %s commit returned code 19 (no changes): %s",
-                    self.address,
-                    msg,
-                )
                 return None
-            else:
-                LOG.debug(
-                    "PAN-OS %s commit returned code 19 but message doesn't "
-                    "indicate 'no changes': %s",
-                    self.address,
-                    msg,
-                )
         if not job:
             LOG.error(
                 "PAN-OS %s commit response missing job ID. Root: %s",
@@ -237,7 +196,6 @@ def commit(self, description: str | None = None) -> str | None:
             )
             raise PanosApiError("Could not find job ID in commit response")
 
-        LOG.debug("PAN-OS %s commit started, job ID: %s", self.address, job)
         return job
 
     def check_job(self, job_id: str) -> dict[str, str]:
@@ -278,7 +236,6 @@ def wait_for_commit(self, job_id: str, timeout: int = 60) -> bool:
 
             if status["status"] == "FIN":
                 if status["result"] == "OK":
-                    LOG.debug("PAN-OS %s commit job %s succeeded", self.address, job_id)
                     return True
                 else:
                     LOG.error(
@@ -365,11 +322,6 @@ def get_lldp_neighbors(self) -> list[dict[str, str]]:
 
             neighbor_entry = neighbor_info.get("entry", {})
             if not neighbor_entry or not isinstance(neighbor_entry, dict):
-                LOG.debug(
-                    "PAN-OS %s LLDP skipping entry - neighbor_entry invalid: %s",
-                    self.address,
-                    neighbor_entry,
-                )
                 continue
 
             neighbors.append(

From 9266a0a203f43bbe6a508bf6b454bae68486b074 Mon Sep 17 00:00:00 2001
From: Kenny Hackworth <khackwor@rackspace.com>
Date: Fri, 23 Oct 2026 14:46:49 -0400
Subject: [PATCH 06/12] chore: remove test image override from values.yaml

---
 components/ironic/values.yaml | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/components/ironic/values.yaml b/components/ironic/values.yaml
index 053be9221..c28e48562 100644
--- a/components/ironic/values.yaml
+++ b/components/ironic/values.yaml
@@ -1,11 +1,4 @@
 ---
-# Override image tag for PR testing
-images:
-  tags:
-    ironic_conductor: ghcr.io/rackerlabs/understack/ironic:pr-2564
-    ironic_api: ghcr.io/rackerlabs/understack/ironic:pr-2564
-    ironic_pxe: ghcr.io/rackerlabs/understack/ironic:pr-2564
-
 bootstrap:
   image:
     enabled: false

From 924db13848a99eefacdae4892d10c503d4138015 Mon Sep 17 00:00:00 2001
From: Kenny Hackworth <khackwor@rackspace.com>
Date: Fri, 23 Oct 2026 14:52:32 -0400
Subject: [PATCH 07/12] fix: use specific PanosApiError exception instead of
 bare Exception

Ruff S110 requires specific exception types instead of bare except Exception.
This also improves error handling by only catching the expected exception type.
---
 .../drivers/panos_inspect.py                  | 27 ++++++++++++-------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/python/ironic-understack/ironic_understack/drivers/panos_inspect.py b/python/ironic-understack/ironic_understack/drivers/panos_inspect.py
index f0c4eedb0..c20939aba 100644
--- a/python/ironic-understack/ironic_understack/drivers/panos_inspect.py
+++ b/python/ironic-understack/ironic_understack/drivers/panos_inspect.py
@@ -1,6 +1,7 @@
 """Inspect interface for PAN-OS (Palo Alto) appliances."""
 
 import time
+from contextlib import suppress
 from typing import ClassVar
 
 import requests
@@ -159,8 +160,8 @@ def _enable_lldp_discovery(
         )
         client.config("delete", xpath, None)
         rules_deleted = True
-    except Exception:
-        pass
+    except panos_api.PanosApiError:
+        pass  # No security rules to delete
 
     try:
         xpath = (
@@ -169,8 +170,8 @@ def _enable_lldp_discovery(
         )
         client.config("delete", xpath, None)
         zones_deleted = True
-    except Exception:
-        pass
+    except panos_api.PanosApiError:
+        pass  # No zone config to delete
 
     try:
         xpath = (
@@ -179,19 +180,25 @@ def _enable_lldp_discovery(
         )
         client.config("delete", xpath, None)
         vwires_deleted = True
-    except Exception:
-        pass
+    except panos_api.PanosApiError:
+        pass  # No virtual-wire config to delete
 
-    # Remove virtual-wire config from all specified interfaces
+    if rules_deleted or zones_deleted or vwires_deleted:
+        LOG.debug(
+            "Deleted factory config: rules=%s zones=%s vwires=%s",
+            rules_deleted,
+            zones_deleted,
+            vwires_deleted,
+        )
+
+    # Remove virtual-wire config from all interfaces (no per-interface logs)
     for iface in interfaces:
-        try:
+        with suppress(panos_api.PanosApiError):
             xpath = (
                 f"/config/devices/entry[@name='localhost.localdomain']"
                 f"/network/interface/ethernet/entry[@name='{iface}']/virtual-wire"
             )
             client.config("delete", xpath, None)
-        except Exception:
-            pass
 
     # Only commit if we actually deleted something
     if rules_deleted or zones_deleted or vwires_deleted:

From ebc2a0fc0012e5ee5801f0ab7954f034ff6aabeb Mon Sep 17 00:00:00 2001
From: Kenny Hackworth <khackwor@rackspace.com>
Date: Mon, 26 Oct 2026 10:16:18 -0400
Subject: [PATCH 08/12] feat: add port category assignment and make --ports
 optional in enroll-fw

Port Category Assignment (LLDP Discovery):
- Assign 'outside'/'inside' categories to discovered firewall ports
- Sort ports by interface number (e.g., ethernet1/1 < ethernet1/2)
- For exactly 2 ports: lowest=outside, highest=inside
- For 3+ ports: first two get outside/inside, rest get no category
- For 1 port: no category assigned

Make --ports Optional (enroll-fw):
- Change --ports from required to optional with default []
- Update help text to clarify LLDP discovery is the primary method
- --ports now serves as override for cases where LLDP doesn't work
- Backward compatible: existing calls with --ports still work

This enables automatic firewall provisioning without manual port specification.
---
 .../drivers/panos_inspect.py                  | 35 ++++++++++++++++++-
 .../understack_workflows/main/enroll_fw.py    |  8 +++--
 2 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/python/ironic-understack/ironic_understack/drivers/panos_inspect.py b/python/ironic-understack/ironic_understack/drivers/panos_inspect.py
index c20939aba..ea1da273f 100644
--- a/python/ironic-understack/ironic_understack/drivers/panos_inspect.py
+++ b/python/ironic-understack/ironic_understack/drivers/panos_inspect.py
@@ -333,6 +333,32 @@ def _normalize_switch_name(name: str) -> str:
         )
         vlan_group_mapping = {}
 
+    # Determine port categories based on interface numbers
+    # Sort ports by interface number
+    # - If exactly 2 ports: lowest is "outside", highest is "inside"
+    # - If 3+ ports: first two in sorted order get "outside"/"inside", rest get None
+    def _extract_interface_number(iface_name: str) -> int:
+        """Extract numeric part from interface name like 'ethernet1/1' -> 1."""
+        try:
+            return int(iface_name.split("/")[-1])
+        except (ValueError, IndexError):
+            return 999  # Put unparsable interfaces at the end
+
+    sorted_ports = sorted(
+        inspected_ports, key=lambda p: _extract_interface_number(p.name)
+    )
+    port_categories = {}
+
+    if len(sorted_ports) == 2:
+        # Exactly 2 ports: lowest is outside, highest is inside
+        port_categories[sorted_ports[0].mac_address.lower()] = "outside"
+        port_categories[sorted_ports[1].mac_address.lower()] = "inside"
+    elif len(sorted_ports) >= 3:
+        # 3+ ports: only first two consecutive get categories
+        port_categories[sorted_ports[0].mac_address.lower()] = "outside"
+        port_categories[sorted_ports[1].mac_address.lower()] = "inside"
+    # If only 1 port, no category assigned
+
     # Now create or update ports
     for inspected_port in inspected_ports:
         mac_address = inspected_port.mac_address
@@ -341,6 +367,7 @@ def _normalize_switch_name(name: str) -> str:
 
         physical_network = vlan_group_mapping.get(neighbor["system_name"])
         port_name = f"{node.name}:{local_interface}"
+        category = port_categories.get(mac_address.lower())
 
         # Check if port already exists (by MAC or name)
         existing_port = None
@@ -358,6 +385,7 @@ def _normalize_switch_name(name: str) -> str:
             existing_port.local_link_connection = inspected_port.local_link_connection
             existing_port.extra = {"bios_name": local_interface}
             existing_port.physical_network = physical_network
+            existing_port.category = category
             existing_port.name = port_name
             existing_port.save()
             LOG.debug("[node:%s] Updated port %s", node.uuid, port_name)
@@ -370,8 +398,13 @@ def _normalize_switch_name(name: str) -> str:
             port_obj.local_link_connection = inspected_port.local_link_connection
             port_obj.extra = {"bios_name": local_interface}
             port_obj.physical_network = physical_network
+            port_obj.category = category
             port_obj.pxe_enabled = True
             port_obj.create()
             LOG.info(
-                "[node:%s] Created port %s (%s)", node.uuid, port_name, mac_address
+                "[node:%s] Created port %s (%s) category=%s",
+                node.uuid,
+                port_name,
+                mac_address,
+                category or "none",
             )
diff --git a/python/understack-workflows/understack_workflows/main/enroll_fw.py b/python/understack-workflows/understack_workflows/main/enroll_fw.py
index 88c4428d3..88517622b 100644
--- a/python/understack-workflows/understack_workflows/main/enroll_fw.py
+++ b/python/understack-workflows/understack_workflows/main/enroll_fw.py
@@ -263,8 +263,12 @@ def argument_parser():
     )
     parser.add_argument(
         "--ports",
-        required=True,
-        help="JSON array of ports (same format as enroll-netdev). 'switch' must "
+        required=False,
+        default="[]",
+        help="Optional JSON array of ports to override LLDP discovery (same format "
+        "as enroll-netdev). By default, ports are discovered automatically via LLDP "
+        "during inspection. Use this option to manually specify ports that may not "
+        "be discovered properly or to override auto-discovered values. 'switch' must "
         "be the switch FQDN (e.g. n11-22-1.dfw3.rackspace.net) so the Nautobot "
         "sync can resolve the cable. Optional per-port 'bios_name' (the device "
         "interface name); defaults to the label.",

From 5adf8114668c5c6c469226db5f387e7ed4a8d4d0 Mon Sep 17 00:00:00 2001
From: Kenny Hackworth <khackwor@rackspace.com>
Date: Tue, 27 Oct 2026 08:28:30 -0400
Subject: [PATCH 09/12] feat: manage CUSTOM_NETDEV_ROLE_ATTACH trait when port
 categories are assigned

- Add CUSTOM_NETDEV_ROLE_ATTACH trait when port categories (outside/inside) are assigned
- Remove trait when no categories are assigned
- Uses proper Ironic objects API: objects.Trait.create() and objects.Trait.destroy()
- Addresses feedback from PR #2578 (netdev role attachment integration)
---
 .../drivers/panos_inspect.py                  | 40 ++++++++++++++++++-
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/python/ironic-understack/ironic_understack/drivers/panos_inspect.py b/python/ironic-understack/ironic_understack/drivers/panos_inspect.py
index ea1da273f..645317576 100644
--- a/python/ironic-understack/ironic_understack/drivers/panos_inspect.py
+++ b/python/ironic-understack/ironic_understack/drivers/panos_inspect.py
@@ -5,10 +5,10 @@
 from typing import ClassVar
 
 import requests
+from ironic import objects
 from ironic.common import exception
 from ironic.common import states
 from ironic.drivers import base
-from ironic.objects import port
 from oslo_log import log
 
 from ironic_understack.utils import panos_api
@@ -391,7 +391,7 @@ def _extract_interface_number(iface_name: str) -> int:
             LOG.debug("[node:%s] Updated port %s", node.uuid, port_name)
         else:
             # Create new port
-            port_obj = port.Port(task.context)
+            port_obj = objects.Port(task.context)
             port_obj.node_id = node.id
             port_obj.address = mac_address
             port_obj.name = port_name
@@ -408,3 +408,39 @@ def _extract_interface_number(iface_name: str) -> int:
                 mac_address,
                 category or "none",
             )
+
+    # Manage CUSTOM_NETDEV_ROLE_ATTACH trait based on whether roles were assigned
+    # Add trait if we assigned categories (outside/inside), remove if we didn't
+    role_attach_trait = "CUSTOM_NETDEV_ROLE_ATTACH"
+    categories_assigned = bool(port_categories)
+
+    try:
+        current_traits = set(task.node.traits or [])
+        has_trait = role_attach_trait in current_traits
+
+        if categories_assigned and not has_trait:
+            LOG.info(
+                "[node:%s] Adding trait %s (port roles assigned)",
+                node.uuid,
+                role_attach_trait,
+            )
+            new_trait = objects.Trait(
+                task.context, node_id=task.node.id, trait=role_attach_trait
+            )
+            new_trait.create()
+        elif not categories_assigned and has_trait:
+            LOG.info(
+                "[node:%s] Removing trait %s (no port roles assigned)",
+                node.uuid,
+                role_attach_trait,
+            )
+            objects.Trait.destroy(
+                task.context, node_id=task.node.id, trait=role_attach_trait
+            )
+    except Exception as e:
+        LOG.warning(
+            "[node:%s] Could not update trait %s: %s",
+            node.uuid,
+            role_attach_trait,
+            e,
+        )

From 1e7870c3cf366a586fb4f3cddb31e766d8838883 Mon Sep 17 00:00:00 2001
From: Kenny Hackworth <khackwor@rackspace.com>
Date: Tue, 27 Oct 2026 09:09:08 -0400
Subject: [PATCH 10/12] Revert "feat: manage CUSTOM_NETDEV_ROLE_ATTACH trait
 when port categories are assigned"

This reverts commit 5adf8114668c5c6c469226db5f387e7ed4a8d4d0.
---
 .../drivers/panos_inspect.py                  | 40 +------------------
 1 file changed, 2 insertions(+), 38 deletions(-)

diff --git a/python/ironic-understack/ironic_understack/drivers/panos_inspect.py b/python/ironic-understack/ironic_understack/drivers/panos_inspect.py
index 645317576..ea1da273f 100644
--- a/python/ironic-understack/ironic_understack/drivers/panos_inspect.py
+++ b/python/ironic-understack/ironic_understack/drivers/panos_inspect.py
@@ -5,10 +5,10 @@
 from typing import ClassVar
 
 import requests
-from ironic import objects
 from ironic.common import exception
 from ironic.common import states
 from ironic.drivers import base
+from ironic.objects import port
 from oslo_log import log
 
 from ironic_understack.utils import panos_api
@@ -391,7 +391,7 @@ def _extract_interface_number(iface_name: str) -> int:
             LOG.debug("[node:%s] Updated port %s", node.uuid, port_name)
         else:
             # Create new port
-            port_obj = objects.Port(task.context)
+            port_obj = port.Port(task.context)
             port_obj.node_id = node.id
             port_obj.address = mac_address
             port_obj.name = port_name
@@ -408,39 +408,3 @@ def _extract_interface_number(iface_name: str) -> int:
                 mac_address,
                 category or "none",
             )
-
-    # Manage CUSTOM_NETDEV_ROLE_ATTACH trait based on whether roles were assigned
-    # Add trait if we assigned categories (outside/inside), remove if we didn't
-    role_attach_trait = "CUSTOM_NETDEV_ROLE_ATTACH"
-    categories_assigned = bool(port_categories)
-
-    try:
-        current_traits = set(task.node.traits or [])
-        has_trait = role_attach_trait in current_traits
-
-        if categories_assigned and not has_trait:
-            LOG.info(
-                "[node:%s] Adding trait %s (port roles assigned)",
-                node.uuid,
-                role_attach_trait,
-            )
-            new_trait = objects.Trait(
-                task.context, node_id=task.node.id, trait=role_attach_trait
-            )
-            new_trait.create()
-        elif not categories_assigned and has_trait:
-            LOG.info(
-                "[node:%s] Removing trait %s (no port roles assigned)",
-                node.uuid,
-                role_attach_trait,
-            )
-            objects.Trait.destroy(
-                task.context, node_id=task.node.id, trait=role_attach_trait
-            )
-    except Exception as e:
-        LOG.warning(
-            "[node:%s] Could not update trait %s: %s",
-            node.uuid,
-            role_attach_trait,
-            e,
-        )

From 8dcb56605b23c660ee2db34aa9cae5e37d63b5ae Mon Sep 17 00:00:00 2001
From: Kenny Hackworth <khackwor@rackspace.com>
Date: Tue, 27 Oct 2026 09:20:45 -0400
Subject: [PATCH 11/12] fix: use natural sort for interface names to match
 netdev_reconciler

Changes interface sorting to match Nidhi's reconciler implementation:
- Uses re.split(r'(\d+)', name) to split on ALL digits, not just the last one
- Creates tuples for natural sorting (0, digit) or (1, string)
- Correctly sorts ethernet1/20 before ethernet2/19 (slot takes precedence)
- Previously only looked at port number, incorrectly sorting ethernet2/19 first

This ensures port category assignment in inspect matches enroll-fw exactly.
---
 .../drivers/panos_inspect.py                  | 23 +++++++++++--------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/python/ironic-understack/ironic_understack/drivers/panos_inspect.py b/python/ironic-understack/ironic_understack/drivers/panos_inspect.py
index ea1da273f..8eb7e0cd2 100644
--- a/python/ironic-understack/ironic_understack/drivers/panos_inspect.py
+++ b/python/ironic-understack/ironic_understack/drivers/panos_inspect.py
@@ -1,5 +1,6 @@
 """Inspect interface for PAN-OS (Palo Alto) appliances."""
 
+import re
 import time
 from contextlib import suppress
 from typing import ClassVar
@@ -334,19 +335,21 @@ def _normalize_switch_name(name: str) -> str:
         vlan_group_mapping = {}
 
     # Determine port categories based on interface numbers
-    # Sort ports by interface number
+    # Sort ports by interface number (natural sort, e.g., ethernet1/20 < ethernet2/19)
     # - If exactly 2 ports: lowest is "outside", highest is "inside"
     # - If 3+ ports: first two in sorted order get "outside"/"inside", rest get None
-    def _extract_interface_number(iface_name: str) -> int:
-        """Extract numeric part from interface name like 'ethernet1/1' -> 1."""
-        try:
-            return int(iface_name.split("/")[-1])
-        except (ValueError, IndexError):
-            return 999  # Put unparsable interfaces at the end
+    def _interface_sort_key(iface_name: str) -> tuple:
+        """Natural sort key for interface names like 'ethernet1/20'.
+
+        Splits on digits to create tuples for natural sorting:
+        ethernet1/20 -> ('ethernet', 1, '/', 20) < ('ethernet', 2, '/', 19)
+        """
+        pieces = re.split(r"(\d+)", iface_name)
+        return tuple(
+            (0, int(piece)) if piece.isdigit() else (1, piece) for piece in pieces
+        )
 
-    sorted_ports = sorted(
-        inspected_ports, key=lambda p: _extract_interface_number(p.name)
-    )
+    sorted_ports = sorted(inspected_ports, key=lambda p: _interface_sort_key(p.name))
     port_categories = {}
 
     if len(sorted_ports) == 2:

From 69e49ba745a09ca04584efdc35fad603da64d911 Mon Sep 17 00:00:00 2001
From: Kenny Hackworth <khackwor@rackspace.com>
Date: Tue, 27 Oct 2026 11:13:30 -0400
Subject: [PATCH 12/12] feat: add firewall readiness validation before
 manageable state

Validates minimum requirements before marking firewall as manageable:
- Requires at least 2 ports (outside/inside) - no 1-armed firewalls
- Warns if PA-1410 uses slow 1G ports (eth1/1-18) instead of 10G (eth1/19-22)
- Validates port categories were assigned correctly

The < 2 ports check fails fast before port creation and is outside the
exception handler so it properly raises HardwareInspectionFailure.

Validation only runs if port discovery succeeded - if LLDP fails, we
still return manageable with a warning (existing behavior preserved).
---
 .../drivers/panos_inspect.py                  | 80 +++++++++++++++++++
 1 file changed, 80 insertions(+)

diff --git a/python/ironic-understack/ironic_understack/drivers/panos_inspect.py b/python/ironic-understack/ironic_understack/drivers/panos_inspect.py
index 8eb7e0cd2..d01e107e1 100644
--- a/python/ironic-understack/ironic_understack/drivers/panos_inspect.py
+++ b/python/ironic-understack/ironic_understack/drivers/panos_inspect.py
@@ -55,8 +55,10 @@ def inspect_hardware(self, task):
         _apply_system_info(node, system)
 
         # Discover data plane connectivity via LLDP
+        ports_discovered = False
         try:
             _discover_and_update_ports(task, client)
+            ports_discovered = True
         except Exception as e:
             LOG.warning(
                 "[node:%s] Could not discover ports via LLDP: %s",
@@ -66,6 +68,11 @@ def inspect_hardware(self, task):
             )
             # Don't fail inspect if port discovery fails
 
+        # Validate that the device is usable before marking as manageable
+        # Only validate if port discovery succeeded
+        if ports_discovered:
+            _validate_firewall_readiness(task)
+
         # TODO: show high-availability all, recording the HA peer serial in
         # extra["mate_serial"] (which enroll-fw also sets) when HA is enabled.
 
@@ -320,6 +327,14 @@ def _normalize_switch_name(name: str) -> str:
         LOG.info("[node:%s] No ports with MAC addresses to create", node.uuid)
         return
 
+    # Validate minimum 2 interfaces before creating ports
+    # A firewall needs at least outside/inside ports to function properly
+    if len(inspected_ports) < 2:
+        raise exception.HardwareInspectionFailure(
+            error=f"Firewall {node.uuid} requires at least 2 ports (outside/inside), "
+            f"but only discovered {len(inspected_ports)} port(s) via LLDP. "
+        )
+
     # Get VLAN group names for all ports
     try:
         vlan_group_mapping = vlan_group_name_convention.vlan_group_names(
@@ -411,3 +426,68 @@ def _interface_sort_key(iface_name: str) -> tuple:
                 mac_address,
                 category or "none",
             )
+
+
+def _validate_firewall_readiness(task) -> None:
+    """Validate that the firewall has minimum requirements to become manageable.
+
+    Checks:
+    - Port categories assigned (outside/inside)
+    - For PA-1410: warn if using slow 1G ports (eth1/1-18 instead of 10G eth1/19-22)
+
+    Note: The minimum 2 ports check is done earlier in _update_ports_from_neighbors
+    before ports are created.
+
+    :raises HardwareInspectionFailure: if requirements not met
+    """
+    node = task.node
+    ports = list(task.ports())
+
+    # Check port categories (should have been assigned during port creation)
+    categories = {getattr(p, "category", None) for p in ports}
+    if "outside" not in categories or "inside" not in categories:
+        LOG.warning(
+            "[node:%s] Port categories incomplete: found %s, expected outside/inside",
+            node.uuid,
+            categories - {None},
+        )
+
+    # For PA-1410: warn if using slow 1G ports
+    model = node.properties.get("model", "")
+    if "1410" in model:
+        slow_ports = []
+        for port in ports:
+            bios_name = port.extra.get("bios_name", "") if port.extra else ""
+            if _is_slow_port_1410(bios_name):
+                slow_ports.append(bios_name)
+
+        if slow_ports:
+            LOG.warning(
+                "[node:%s] PA-1410 using slow 1G ports %s; "
+                "should use 10G ports ethernet1/19-22 for production traffic",
+                node.uuid,
+                slow_ports,
+            )
+
+    LOG.info(
+        "[node:%s] Firewall validation passed: %d ports discovered, " "categories=%s",
+        node.uuid,
+        len(ports),
+        sorted(categories - {None}),
+    )
+
+
+def _is_slow_port_1410(interface_name: str) -> bool:
+    """Check if interface is a slow 1G port on PA-1410.
+
+    PA-1410 has:
+    - ethernet1/1-18: 1G ports (slow)
+    - ethernet1/19-22: 10G ports (fast)
+    """
+    if not interface_name:
+        return False
+    match = re.match(r"ethernet1/(\d+)", interface_name)
+    if not match:
+        return False
+    port_num = int(match.group(1))
+    return 1 <= port_num <= 18