give PaloAltoWiring a public, router_id-based API

Author: nidzrai

python/neutron-understack/neutron_understack/l3_router/palo_alto.py

@@ -224,7 +224,7 @@ def _process_router_create(self, resource, event, trigger, payload=None):
 
         # Ensure the shared anchor network first: it is idempotent and meant to
         # persist, so creating it before adoption never strands an adopted node.
-        self._wiring._ensure_anchor_network()
+        self._wiring.ensure_anchor_network()
         self._ironic.adopt_node_for_router(
             node,
             project_id=router.get("project_id"),
@@ -282,25 +282,22 @@ def _process_gateway_create(self, resource, event, trigger, payload=None):
         the parent is bound.
         """
         router_id = payload.resource_id
-        router = self._palo_alto_router(payload.context, router_id)
-        if router is None:
+        if self._palo_alto_router(payload.context, router_id) is None:
             return
 
-        gateway_port = self._wiring._gateway_port_for_router(router_id)
+        gateway_port = self._wiring.gateway_port_for_router(router_id)
         if gateway_port is None:
             raise PaloAltoGatewayPortNotFound(router_id=router_id)
 
-        parent = self._wiring._ensure_parent_port(router)
-        parent = self._wiring._ensure_parent_vif_attached(router, parent)
-        trunk = self._wiring._ensure_trunk(router, parent)
-        self._wiring._add_gateway_subport(router, trunk, gateway_port)
+        stack = self._wiring.ensure_stack(router_id)
+        self._wiring.add_gateway_subport(router_id, stack.trunk, gateway_port)
 
         LOG.info(
             "Attached Palo Alto router %s gateway port %s via parent %s trunk %s",
             router_id,
             gateway_port["id"],
-            parent["id"],
-            trunk["id"],
+            stack.parent["id"],
+            stack.trunk["id"],
         )
 
     def _process_gateway_delete(self, resource, event, trigger, payload=None):
@@ -310,19 +307,18 @@ def _process_gateway_delete(self, resource, event, trigger, payload=None):
         remove its subport before Neutron deletes it.
         """
         router_id = payload.resource_id
-        router = self._palo_alto_router(payload.context, router_id)
-        if router is None:
+        if self._palo_alto_router(payload.context, router_id) is None:
             return
 
-        gateway_port = self._wiring._gateway_port_for_router(router_id)
+        gateway_port = self._wiring.gateway_port_for_router(router_id)
         if gateway_port is None:
             LOG.debug(
                 "Palo Alto router %s gateway cleanup skipped; gateway port not found",
                 router_id,
             )
             return
 
-        self._wiring._cleanup_gateway_attachment(router, gateway_port)
+        self._wiring.cleanup_attachment(router_id, gateway_port["id"], "gateway")
         LOG.info(
             "Cleaned Palo Alto router %s gateway attachment (port %s)",
             router_id,
@@ -332,8 +328,7 @@ def _process_gateway_delete(self, resource, event, trigger, payload=None):
     def _process_router_interface_create(self, resource, event, trigger, payload=None):
         """Wire the interface before Neutron commits its RouterPort association."""
         router_id = payload.resource_id
-        router = self._palo_alto_router(payload.context, router_id)
-        if router is None:
+        if self._palo_alto_router(payload.context, router_id) is None:
             return
 
         interface_port = payload.metadata.get("port")
@@ -342,14 +337,12 @@ def _process_router_interface_create(self, resource, event, trigger, payload=Non
         if interface_port.get("device_owner") not in const.ROUTER_INTERFACE_OWNERS:
             return
 
-        self._interface_snapshots[payload] = (
-            self._wiring._snapshot_interface_attachment(router_id, interface_port["id"])
+        self._interface_snapshots[payload] = self._wiring.snapshot_interface_attachment(
+            router_id, interface_port["id"]
         )
         try:
-            parent = self._wiring._ensure_parent_port(router)
-            parent = self._wiring._ensure_parent_vif_attached(router, parent)
-            trunk = self._wiring._ensure_trunk(router, parent)
-            self._wiring._add_interface_subport(router, trunk, interface_port)
+            stack = self._wiring.ensure_stack(router_id)
+            self._wiring.add_interface_subport(router_id, stack.trunk, interface_port)
         except Exception:
             # Attach-by-port is reverted with a port update by Neutron, so it
             # cannot rely on PORT/BEFORE_DELETE to undo partial realization.
@@ -360,8 +353,8 @@ def _process_router_interface_create(self, resource, event, trigger, payload=Non
             "Attached Palo Alto router %s interface port %s via parent %s trunk %s",
             router_id,
             interface_port["id"],
-            parent["id"],
-            trunk["id"],
+            stack.parent["id"],
+            stack.trunk["id"],
         )
 
     def _rollback_interface_attachment(self, payload) -> None:
@@ -370,7 +363,7 @@ def _rollback_interface_attachment(self, payload) -> None:
         if snapshot is None:
             return
         try:
-            self._wiring._undo_interface_attachment(snapshot)
+            self._wiring.undo_interface_attachment(snapshot)
         except Exception:
             # Preserve the original attach error. Keep the snapshot so the
             # subsequent ABORT_CREATE can retry compensation if it failed here.
@@ -432,7 +425,7 @@ def _process_router_interface_port_delete(
         ):
             return
 
-        self._wiring._cleanup_interface_attachment(router, port)
+        self._wiring.cleanup_attachment(router_id, port["id"], "interface")
         LOG.info(
             "Cleaned Palo Alto router %s interface attachment (port %s)",
             router_id,

python/neutron-understack/neutron_understack/l3_router/palo_alto_wiring.py

@@ -60,6 +60,14 @@ class AttachmentSnapshot:
     parent_vif_attached: bool
 
 
+@dataclass(frozen=True)
+class AttachmentStack:
+    """A router's VIF-bound anchor parent port and the trunk built on it."""
+
+    parent: dict
+    trunk: dict
+
+
 # Conflict -> HTTP 409: every allowed VLAN on the router's trunk is in use.
 class NoPaloAltoSubportVlanAvailable(n_exc.Conflict):
     message = (
@@ -173,7 +181,7 @@ def _ironic(self) -> IronicClient:
     def _trunk_plugin(self):
         return utils.fetch_trunk_plugin()
 
-    def _ensure_anchor_network(self) -> dict:
+    def ensure_anchor_network(self) -> dict:
         """Create the shared sentinel anchor network if it does not exist."""
         core_plugin = directory.get_plugin()
         admin_context = n_context.get_admin_context()
@@ -206,7 +214,7 @@ def _ensure_anchor_network(self) -> dict:
             },
         )
 
-    def _gateway_port_for_router(self, router_id: str) -> dict | None:
+    def gateway_port_for_router(self, router_id: str) -> dict | None:
         """Return the router's Neutron external-gateway port, or None.
 
         The gateway port is owned by the router (``device_id == router_id``) with
@@ -237,7 +245,7 @@ def _parent_port_for_router(self, router_id: str) -> dict | None:
         """Return the router's existing anchor-network parent port, or None."""
         core_plugin = directory.get_plugin()
         admin_context = n_context.get_admin_context()
-        anchor_network = self._ensure_anchor_network()
+        anchor_network = self.ensure_anchor_network()
         ports = core_plugin.get_ports(
             admin_context,
             filters={
@@ -247,7 +255,7 @@ def _parent_port_for_router(self, router_id: str) -> dict | None:
         )
         return ports[0] if ports else None
 
-    def _create_parent_port(self, router: dict) -> dict:
+    def _create_parent_port(self, router_id: str) -> dict:
         """Create the router's parent port on the anchor network.
 
         vnic_type=baremetal so Ironic can VIF-attach it to the adopted node.
@@ -256,12 +264,12 @@ def _create_parent_port(self, router: dict) -> dict:
         """
         core_plugin = directory.get_plugin()
         admin_context = n_context.get_admin_context()
-        anchor_network = self._ensure_anchor_network()
-        port_name = _parent_port_name(router["id"])
+        anchor_network = self.ensure_anchor_network()
+        port_name = _parent_port_name(router_id)
         LOG.info(
             "Creating Palo Alto anchor parent port %s for router %s",
             port_name,
-            router["id"],
+            router_id,
         )
         return core_plugin.create_port(
             admin_context,
@@ -271,7 +279,7 @@ def _create_parent_port(self, router: dict) -> dict:
                     "network_id": anchor_network["id"],
                     "admin_state_up": True,
                     "device_owner": "",
-                    "device_id": router["id"],
+                    "device_id": router_id,
                     "mac_address": "",
                     "fixed_ips": [],
                     "project_id": admin_context.project_id or "",
@@ -280,17 +288,17 @@ def _create_parent_port(self, router: dict) -> dict:
             },
         )
 
-    def _ensure_parent_port(self, router: dict) -> dict:
+    def _ensure_parent_port(self, router_id: str) -> dict:
         """Find-or-create the router's anchor-network parent port (idempotent)."""
-        existing = self._parent_port_for_router(router["id"])
+        existing = self._parent_port_for_router(router_id)
         if existing is not None:
             LOG.debug(
                 "Reusing Palo Alto anchor parent port %s for router %s",
                 existing["id"],
-                router["id"],
+                router_id,
             )
             return existing
-        return self._create_parent_port(router)
+        return self._create_parent_port(router_id)
 
     def _fresh_port(self, port_id: str) -> dict:
         """Re-read a port so callers see its current binding profile."""
@@ -318,7 +326,7 @@ def _vif_landed_after_error(
             return False
         return parent_port_id in vif_ids
 
-    def _ensure_parent_vif_attached(self, router: dict, parent_port: dict) -> dict:
+    def _ensure_parent_vif_attached(self, router_id: str, parent_port: dict) -> dict:
         """VIF-attach the parent port to the router's node (idempotent).
 
         Attaching a single VIF; Ironic binds it to a free baremetal port on the
@@ -327,7 +335,6 @@ def _ensure_parent_vif_attached(self, router: dict, parent_port: dict) -> dict:
 
         Returns a fresh copy of the parent port reflecting the new binding.
         """
-        router_id = router["id"]
         node = self._ironic().node_by_instance_uuid(router_id)
         if node is None:
             raise PaloAltoNodeNotAdopted(router_id=router_id)
@@ -382,15 +389,15 @@ def _trunk_for_router(self, router_id: str) -> dict | None:
         )
         return trunks[0] if trunks else None
 
-    def _create_trunk(self, router: dict, parent_port: dict) -> dict:
+    def _create_trunk(self, router_id: str, parent_port: dict) -> dict:
         """Create the router's trunk with the parent port as its trunk parent."""
         admin_context = n_context.get_admin_context()
-        trunk_name = _trunk_name(router["id"])
+        trunk_name = _trunk_name(router_id)
         LOG.info(
             "Creating Palo Alto trunk %s on parent port %s for router %s",
             trunk_name,
             parent_port["id"],
-            router["id"],
+            router_id,
         )
         return self._trunk_plugin().create_trunk(
             admin_context,
@@ -405,17 +412,29 @@ def _create_trunk(self, router: dict, parent_port: dict) -> dict:
             },
         )
 
-    def _ensure_trunk(self, router: dict, parent_port: dict) -> dict:
+    def _ensure_trunk(self, router_id: str, parent_port: dict) -> dict:
         """Find-or-create the router's trunk (idempotent)."""
-        existing = self._trunk_for_router(router["id"])
+        existing = self._trunk_for_router(router_id)
         if existing is not None:
             LOG.debug(
                 "Reusing Palo Alto trunk %s for router %s",
                 existing["id"],
-                router["id"],
+                router_id,
             )
             return existing
-        return self._create_trunk(router, parent_port)
+        return self._create_trunk(router_id, parent_port)
+
+    def ensure_stack(self, router_id: str) -> AttachmentStack:
+        """Find or create the router's parent port, VIF attachment and trunk.
+
+        Builds them in this order so the parent is VIF-bound before any subport
+        is added: the trunk driver only programs the switchport once the parent
+        is bound.
+        """
+        parent = self._ensure_parent_port(router_id)
+        parent = self._ensure_parent_vif_attached(router_id, parent)
+        trunk = self._ensure_trunk(router_id, parent)
+        return AttachmentStack(parent=parent, trunk=trunk)
 
     def _next_available_subport_vlan(
         self, router_id: str, trunk: dict, start_vlan: int
@@ -433,7 +452,7 @@ def _next_available_subport_vlan(
 
     def _add_router_port_subport(
         self,
-        router: dict,
+        router_id: str,
         trunk: dict,
         port: dict,
         label: PortLabel,
@@ -466,7 +485,7 @@ def _add_router_port_subport(
             port_id,
             trunk["id"],
             segmentation_id,
-            router["id"],
+            router_id,
         )
         with _device_id_cleared(port):
             return self._trunk_plugin().add_subports(
@@ -483,25 +502,25 @@ def _add_router_port_subport(
                 },
             )
 
-    def _add_gateway_subport(
-        self, router: dict, trunk: dict, gateway_port: dict
+    def add_gateway_subport(
+        self, router_id: str, trunk: dict, gateway_port: dict
     ) -> dict:
         """Add the gateway port to the trunk as a VLAN subport (idempotent)."""
         return self._add_router_port_subport(
-            router, trunk, gateway_port, "gateway", lambda: GATEWAY_SUBPORT_VLAN
+            router_id, trunk, gateway_port, "gateway", lambda: GATEWAY_SUBPORT_VLAN
         )
 
-    def _add_interface_subport(
-        self, router: dict, trunk: dict, interface_port: dict
+    def add_interface_subport(
+        self, router_id: str, trunk: dict, interface_port: dict
     ) -> dict:
         """Add a router-interface port to the trunk as a VLAN subport."""
         return self._add_router_port_subport(
-            router,
+            router_id,
             trunk,
             interface_port,
             "interface",
             lambda: self._next_available_subport_vlan(
-                router["id"], trunk, INTERFACE_SUBPORT_VLAN_START
+                router_id, trunk, INTERFACE_SUBPORT_VLAN_START
             ),
         )
 
@@ -575,9 +594,9 @@ def _delete_parent_stack_if_unused(self, router_id: str, trunk: dict) -> None:
         self._trunk_plugin().delete_trunk(admin_context, trunk["id"])
         self._detach_and_delete_parent(router_id, parent_id)
 
-    def _cleanup_router_port_attachment(
+    def cleanup_attachment(
         self,
-        router: dict,
+        router_id: str,
         port_id: str,
         label: PortLabel,
     ) -> None:
@@ -587,7 +606,6 @@ def _cleanup_router_port_attachment(
         parent port was created/VIF-attached but before the trunk existed, there
         is no trunk to key off, so find and tear down the orphan parent directly.
         """
-        router_id = router["id"]
         trunk = self._trunk_for_router(router_id)
         if trunk is None:
             parent = self._parent_port_for_router(router_id)
@@ -607,15 +625,7 @@ def _cleanup_router_port_attachment(
         self._remove_router_port_subport(trunk, port_id, label)
         self._delete_parent_stack_if_unused(router_id, trunk)
 
-    def _cleanup_gateway_attachment(self, router: dict, gateway_port: dict) -> None:
-        """Clean up a gateway port's Palo Alto trunk attachment."""
-        self._cleanup_router_port_attachment(router, gateway_port["id"], "gateway")
-
-    def _cleanup_interface_attachment(self, router: dict, interface_port: dict) -> None:
-        """Clean up a router-interface port's Palo Alto trunk attachment."""
-        self._cleanup_router_port_attachment(router, interface_port["id"], "interface")
-
-    def _snapshot_interface_attachment(
+    def snapshot_interface_attachment(
         self, router_id: str, port_id: str
     ) -> AttachmentSnapshot:
         """Record which wiring already exists before an interface attach."""
@@ -635,7 +645,7 @@ def _snapshot_interface_attachment(
             parent_vif_attached=parent_vif_attached,
         )
 
-    def _undo_interface_attachment(self, snapshot: AttachmentSnapshot) -> None:
+    def undo_interface_attachment(self, snapshot: AttachmentSnapshot) -> None:
         """Return the router's wiring to the state recorded in the snapshot.
 
         Re-reads current state rather than replaying the calls that succeeded,