diff --git a/components/images-openstack.yaml b/components/images-openstack.yaml index d024a0d14..11beb989e 100644 --- a/components/images-openstack.yaml +++ b/components/images-openstack.yaml @@ -43,7 +43,7 @@ images: neutron_metadata: "ghcr.io/rackerlabs/understack/neutron:2026.1" neutron_ovn_metadata: "ghcr.io/rackerlabs/understack/neutron:2026.1" neutron_openvswitch_agent: "ghcr.io/rackerlabs/understack/neutron:2026.1" - neutron_server: "ghcr.io/rackerlabs/understack/neutron:2026.1" + neutron_server: "ghcr.io/rackerlabs/understack/neutron:pr-2256" neutron_rpc_server: "ghcr.io/rackerlabs/understack/neutron:2026.1" neutron_bagpipe_bgp: "ghcr.io/rackerlabs/understack/neutron:2026.1" neutron_netns_cleanup_cron: "ghcr.io/rackerlabs/understack/neutron:2026.1" diff --git a/python/neutron-understack/neutron_understack/ironic.py b/python/neutron-understack/neutron_understack/ironic.py index 096e8f6ce..485e31e57 100644 --- a/python/neutron-understack/neutron_understack/ironic.py +++ b/python/neutron-understack/neutron_understack/ironic.py @@ -121,6 +121,26 @@ def node_by_instance_uuid(self, instance_uuid: str) -> BaremetalNode | None: except StopIteration: return None + def attach_vif_to_node(self, node: str | BaremetalNode, vif_id: str) -> None: + """Attach a Neutron port (VIF) to the node.""" + node_id = node.id if isinstance(node, BaremetalNode) else node + LOG.info("Attaching VIF %s to Ironic node %s", vif_id, node_id) + self.irclient.attach_vif_to_node(node, vif_id) + + def detach_vif_from_node(self, node: str | BaremetalNode, vif_id: str) -> bool: + """Detach a VIF from the node. + + Returns whatever the SDK reports (False when the VIF was not attached); + ``ignore_missing=True`` so tearing down an already-detached VIF is a no-op. + """ + node_id = node.id if isinstance(node, BaremetalNode) else node + LOG.info("Detaching VIF %s from Ironic node %s", vif_id, node_id) + return self.irclient.detach_vif_from_node(node, vif_id, ignore_missing=True) + + def node_vif_ids(self, node: str | BaremetalNode) -> list[str]: + """Return the Neutron port (VIF) ids currently attached to the node.""" + return self.irclient.list_node_vifs(node) + def adopt_node_for_router( self, node: str | BaremetalNode, diff --git a/python/neutron-understack/neutron_understack/l3_router/palo_alto.py b/python/neutron-understack/neutron_understack/l3_router/palo_alto.py index 7f83650e8..41ccda720 100644 --- a/python/neutron-understack/neutron_understack/l3_router/palo_alto.py +++ b/python/neutron-understack/neutron_understack/l3_router/palo_alto.py @@ -5,12 +5,14 @@ from neutron_lib import constants as const from neutron_lib import context as n_context from neutron_lib import exceptions as n_exc +from neutron_lib.api.definitions import portbindings from neutron_lib.callbacks import events from neutron_lib.callbacks import registry from neutron_lib.callbacks import resources from neutron_lib.plugins import constants as plugin_constants from neutron_lib.plugins import directory +from neutron_understack import utils from neutron_understack.ironic import IronicClient LOG = logging.getLogger(__name__) @@ -18,6 +20,17 @@ # Single shared sentinel network owned by the router flavor code. ANCHOR_NETWORK_NAME = "palo_alto_router_anchor_network" +# Deterministic per-router names. Deriving the parent port and trunk names from +# the router id lets the gateway teardown find them by name without depending on +# catching a specific delete event with the gateway port still visible. +ANCHOR_PARENT_PORT_NAME_PREFIX = "palo-alto-router-anchor" +TRUNK_NAME_PREFIX = "palo-alto-router-trunk" + +# Temporary fixed trunk subport tag. This keeps the subport segmentation_id in +# the existing allowed/gap VLAN validation path. Replace with a configured or +# allocated model once the trunk-tag semantics are revisited. +GATEWAY_SUBPORT_VLAN = 200 + # Conflict -> HTTP 409: the request cannot be satisfied because the hardware # pool is exhausted. @@ -68,6 +81,25 @@ class PaloAlto(base.L3ServiceProvider): def __init__(self, l3_plugin): super().__init__(l3_plugin) self._palo_alto_provider = f"{__name__}.{self.__class__.__name__}" + # Gateway attach must run on AFTER_CREATE (the gateway port does not + # exist earlier) and must be cancellable so a wiring failure returns a + # real API error instead of a swallowed 200. @registry.receives cannot + # set cancellable, so subscribe explicitly. + registry.subscribe( + self._process_gateway_create, + resources.ROUTER_GATEWAY, + events.AFTER_CREATE, + cancellable=True, + ) + # Remove runs on BEFORE_DELETE: the gateway port still exists there (it + # is deleted only afterwards) so we can find it, and BEFORE_DELETE + # re-raises callback errors. Note it runs inside a DB write transaction. + registry.subscribe( + self._process_gateway_delete, + resources.ROUTER_GATEWAY, + events.BEFORE_DELETE, + cancellable=True, + ) LOG.info( "Palo Alto service provider initialized: driver=%r", self._palo_alto_provider, @@ -177,6 +209,360 @@ def _ensure_anchor_network(self): }, ) + # --- gateway attachment: names + lookups (read-only) --- + + @property + def _trunk_plugin(self): + return utils.fetch_trunk_plugin() + + def _parent_port_name(self, router_id: str) -> str: + """Deterministic name for the router's anchor-network parent port.""" + return f"{ANCHOR_PARENT_PORT_NAME_PREFIX}-{router_id}" + + def _trunk_name(self, router_id: str) -> str: + """Deterministic name for the router's trunk.""" + return f"{TRUNK_NAME_PREFIX}-{router_id}" + + 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 + ``device_owner == network:router_gateway``. + """ + core_plugin = directory.get_plugin() + admin_context = n_context.get_admin_context() + ports = core_plugin.get_ports( + admin_context, + filters={ + "device_id": [router_id], + "device_owner": [const.DEVICE_OWNER_ROUTER_GW], + }, + ) + if not ports: + LOG.debug("No gateway port found for Palo Alto router %s", router_id) + return None + if len(ports) > 1: + LOG.warning( + "Expected one gateway port for Palo Alto router %s, found %d; using %s", + router_id, + len(ports), + ports[0]["id"], + ) + return ports[0] + + 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() + ports = core_plugin.get_ports( + admin_context, + filters={ + "name": [self._parent_port_name(router_id)], + "network_id": [anchor_network["id"]], + }, + ) + return ports[0] if ports else None + + def _create_parent_port(self, router: dict) -> dict: + """Create the router's parent port on the anchor network. + + vnic_type=baremetal so Ironic can VIF-attach it to the adopted node. + Direct core-plugin call (server-side), so extension-default fields are + supplied explicitly, matching the codebase's other direct port creates. + """ + core_plugin = directory.get_plugin() + admin_context = n_context.get_admin_context() + anchor_network = self._ensure_anchor_network() + port_name = self._parent_port_name(router["id"]) + LOG.info( + "Creating Palo Alto anchor parent port %s for router %s", + port_name, + router["id"], + ) + return core_plugin.create_port( + admin_context, + { + "port": { + "name": port_name, + "network_id": anchor_network["id"], + "admin_state_up": True, + "device_owner": "", + "device_id": router["id"], + "mac_address": "", + "fixed_ips": [], + "project_id": admin_context.project_id or "", + portbindings.VNIC_TYPE: portbindings.VNIC_BAREMETAL, + } + }, + ) + + def _ensure_parent_port(self, router: dict) -> dict: + """Find-or-create the router's anchor-network parent port (idempotent).""" + 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"], + ) + return existing + return self._create_parent_port(router) + + def _fresh_port(self, port_id: str) -> dict: + """Re-read a port so callers see its current binding profile.""" + core_plugin = directory.get_plugin() + admin_context = n_context.get_admin_context() + return core_plugin.get_port(admin_context, port_id) + + def _ensure_parent_vif_attached(self, router: dict, 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 + node and annotates the Neutron port with local_link_information + + physical_network and host_id . + + 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 n_exc.BadRequest( + resource="router", + msg=( + f"Palo Alto router {router_id} has no adopted Ironic node to " + "attach the gateway uplink to." + ), + ) + + parent_port_id = parent_port["id"] + if parent_port_id in self._ironic.node_vif_ids(node): + LOG.debug( + "Parent port %s already VIF-attached to node %s", + parent_port_id, + node.id, + ) + else: + self._ironic.attach_vif_to_node(node, parent_port_id) + + fresh = self._fresh_port(parent_port_id) + self._verify_parent_annotated(router_id, fresh) + return fresh + + def _verify_parent_annotated(self, router_id: str, parent_port: dict) -> None: + """Fail fast if Ironic did not annotate the parent port. + + The trunk + undersync need host_id, physical_network and + local_link_information on the binding profile. If they are missing the + node's baremetal port most likely has no physical_network (enroll side); + surface a clear error here instead of a silent no-op at undersync. + """ + profile = parent_port.get(portbindings.PROFILE) or {} + missing = [ + name + for name, value in ( + ("binding:host_id", parent_port.get(portbindings.HOST_ID)), + ("physical_network", profile.get("physical_network")), + ("local_link_information", profile.get("local_link_information")), + ) + if not value + ] + if missing: + raise n_exc.BadRequest( + resource="router", + msg=( + f"Palo Alto router {router_id} parent port {parent_port['id']} " + f"was not annotated by Ironic (missing {', '.join(missing)}); " + "check the node's baremetal port has physical_network." + ), + ) + + def _trunk_for_router(self, router_id: str) -> dict | None: + """Return the router's existing trunk (by deterministic name), or None.""" + admin_context = n_context.get_admin_context() + trunks = self._trunk_plugin.get_trunks( + admin_context, filters={"name": [self._trunk_name(router_id)]} + ) + return trunks[0] if trunks else None + + def _create_trunk(self, router: dict, 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 = self._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"], + ) + return self._trunk_plugin.create_trunk( + admin_context, + { + "trunk": { + "name": trunk_name, + "port_id": parent_port["id"], + "admin_state_up": True, + "project_id": admin_context.project_id or "", + "sub_ports": [], + } + }, + ) + + def _ensure_trunk(self, router: dict, parent_port: dict) -> dict: + """Find-or-create the router's trunk (idempotent).""" + 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"], + ) + return existing + return self._create_trunk(router, parent_port) + + def _add_gateway_subport(self, router: dict, trunk: dict, gateway_port: dict): + """Add the gateway port to the trunk as a VLAN subport (idempotent). + + Adding the subport fires the understack trunk driver (SUBPORTS events), + which allocates the fabric segment, binds it, and calls undersync to + program the switch. No-ops if the gateway port is already a subport. + """ + gateway_port_id = gateway_port["id"] + existing = {sp["port_id"] for sp in trunk.get("sub_ports", [])} + if gateway_port_id in existing: + LOG.debug( + "Gateway port %s already a subport on trunk %s", + gateway_port_id, + trunk["id"], + ) + return trunk + + admin_context = n_context.get_admin_context() + LOG.info( + "Adding gateway port %s to trunk %s as VLAN %s subport for router %s", + gateway_port_id, + trunk["id"], + GATEWAY_SUBPORT_VLAN, + router["id"], + ) + # The trunk subport validator rejects a port that has device_id set + # (rules.py check_not_in_use). The gateway port has device_id=router_id, + # so clear it for the add and restore it afterwards so the router keeps + # its gateway-port association (our own lookups depend on it). + original_device_id = gateway_port["device_id"] + original_device_owner = gateway_port["device_owner"] + utils.clear_device_id_for_port(gateway_port_id) + try: + return self._trunk_plugin.add_subports( + admin_context, + trunk["id"], + { + "sub_ports": [ + { + "port_id": gateway_port_id, + "segmentation_type": "vlan", + "segmentation_id": GATEWAY_SUBPORT_VLAN, + } + ] + }, + ) + finally: + utils.set_device_id_and_owner_for_port( + gateway_port_id, original_device_id, original_device_owner + ) + + def _remove_gateway_subport(self, trunk: dict, gateway_port_id: str): + """Remove the gateway port from the trunk (idempotent). + + Fires the trunk driver's SUBPORTS delete events, which release the + fabric segment and update the switchport. No-ops if it is not a subport. + """ + existing = {sp["port_id"] for sp in trunk.get("sub_ports", [])} + if gateway_port_id not in existing: + LOG.debug( + "Gateway port %s is not a subport on trunk %s; skip removal", + gateway_port_id, + trunk["id"], + ) + return trunk + admin_context = n_context.get_admin_context() + LOG.info( + "Removing gateway subport %s from trunk %s", gateway_port_id, trunk["id"] + ) + return self._trunk_plugin.remove_subports( + admin_context, + trunk["id"], + {"sub_ports": [{"port_id": gateway_port_id}]}, + ) + + def _detach_and_delete_parent(self, router_id: str, parent_id: str) -> None: + """Detach the parent VIF from the node and delete the parent port.""" + node = self._ironic.node_by_instance_uuid(router_id) + if node is not None: + self._ironic.detach_vif_from_node(node, parent_id) + else: + LOG.warning( + "No node found for router %s while detaching parent %s", + router_id, + parent_id, + ) + core_plugin = directory.get_plugin() + admin_context = n_context.get_admin_context() + LOG.info("Deleting Palo Alto anchor parent port %s", parent_id) + core_plugin.delete_port(admin_context, parent_id) + + def _delete_parent_stack_if_unused(self, router_id: str, trunk: dict) -> None: + """Delete the trunk + parent port only if no subports remain. + + Re-reads the trunk so a subport removed just before this is reflected. If + other subports are still present (e.g. tenant subnets), leave the trunk + and parent for them to share. + """ + admin_context = n_context.get_admin_context() + fresh = self._trunk_plugin.get_trunk(admin_context, trunk["id"]) + if fresh.get("sub_ports"): + LOG.debug( + "Trunk %s still has subports; leaving parent stack for router %s", + trunk["id"], + router_id, + ) + return + parent_id = fresh["port_id"] + LOG.info( + "Deleting Palo Alto trunk %s (no subports left) for router %s", + trunk["id"], + router_id, + ) + self._trunk_plugin.delete_trunk(admin_context, trunk["id"]) + self._detach_and_delete_parent(router_id, parent_id) + + def _cleanup_gateway_attachment(self, router: dict, gateway_port: dict) -> None: + """Reverse of the add: remove subport, then tear down the parent stack. + + Handles a partially-built attach too: if a prior add failed after the + 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) + if parent is not None: + LOG.info( + "No trunk for Palo Alto router %s; deleting orphan parent %s", + router_id, + parent["id"], + ) + self._detach_and_delete_parent(router_id, parent["id"]) + else: + LOG.debug( + "No trunk or parent for Palo Alto router %s; nothing to clean up", + router_id, + ) + return + self._remove_gateway_subport(trunk, gateway_port["id"]) + self._delete_parent_stack_if_unused(router_id, trunk) + @registry.receives(resources.ROUTER, [events.BEFORE_CREATE]) def _process_router_create(self, resource, event, trigger, payload=None): """Realize the router on hardware, before the router row is created. @@ -243,3 +629,66 @@ def _process_router_delete(self, resource, event, trigger, payload=None): node.id, router["id"], ) + + def _process_gateway_create(self, resource, event, trigger, payload=None): + """ROUTER_GATEWAY / AFTER_CREATE (cancellable): wire the gateway. + + Orders the building blocks so the parent port is VIF-bound before the + subport is added -- the trunk driver only programs the switchport once + the parent is bound. + """ + context = payload.context + router_id = payload.resource_id + router = self.l3plugin.get_router(context, router_id) + if not self._is_palo_alto_provider(context, router): + return + + gateway_port = self._gateway_port_for_router(router_id) + if gateway_port is None: + raise n_exc.BadRequest( + resource="router", + msg=( + f"Palo Alto router {router_id} gateway was created but no " + "router gateway port was found." + ), + ) + + parent = self._ensure_parent_port(router) + parent = self._ensure_parent_vif_attached(router, parent) + trunk = self._ensure_trunk(router, parent) + self._add_gateway_subport(router, 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"], + ) + + def _process_gateway_delete(self, resource, event, trigger, payload=None): + """ROUTER_GATEWAY / BEFORE_DELETE (cancellable): tear down the wiring. + + The gateway port still exists at this point, so we can find it and + remove its subport before Neutron deletes it. + """ + context = payload.context + router_id = payload.resource_id + router = self.l3plugin.get_router(context, router_id) + if not self._is_palo_alto_provider(context, router): + return + + gateway_port = self._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._cleanup_gateway_attachment(router, gateway_port) + LOG.info( + "Cleaned Palo Alto router %s gateway attachment (port %s)", + router_id, + gateway_port["id"], + ) diff --git a/python/neutron-understack/neutron_understack/tests/test_ironic.py b/python/neutron-understack/neutron_understack/tests/test_ironic.py index 7adcff568..81756099c 100644 --- a/python/neutron-understack/neutron_understack/tests/test_ironic.py +++ b/python/neutron-understack/neutron_understack/tests/test_ironic.py @@ -41,6 +41,35 @@ def test_manage_failure_is_rolled_back_and_reraised(self, mocker): ) # manage + provide +class TestVifAttach: + def test_attach_calls_proxy(self, mocker): + client = _client(mocker) + node = mocker.Mock(id="n1") + + client.attach_vif_to_node(node, "port-1") + + client.irclient.attach_vif_to_node.assert_called_once_with(node, "port-1") + + def test_detach_uses_ignore_missing_and_returns_result(self, mocker): + client = _client(mocker) + node = mocker.Mock(id="n1") + client.irclient.detach_vif_from_node.return_value = True + + result = client.detach_vif_from_node(node, "port-1") + + assert result is True + client.irclient.detach_vif_from_node.assert_called_once_with( + node, "port-1", ignore_missing=True + ) + + def test_node_vif_ids(self, mocker): + client = _client(mocker) + node = mocker.Mock(id="n1") + client.irclient.list_node_vifs.return_value = ["p1", "p2"] + + assert client.node_vif_ids(node) == ["p1", "p2"] + + class TestReleaseClearsOwnership: def test_active_node_is_undeployed_then_ownership_cleared(self, mocker): client = _client(mocker) diff --git a/python/neutron-understack/neutron_understack/tests/test_palo_alto_provider.py b/python/neutron-understack/neutron_understack/tests/test_palo_alto_provider.py index 9d2466d71..b1f5c71f0 100644 --- a/python/neutron-understack/neutron_understack/tests/test_palo_alto_provider.py +++ b/python/neutron-understack/neutron_understack/tests/test_palo_alto_provider.py @@ -320,3 +320,431 @@ def test_skips_non_palo_alto_router(self, mocker): "router", "after_delete", "trigger", FakePayload(self._router()) ) ironic.release_node_for_router.assert_not_called() + + +class TestGatewayLookups: + def test_names_are_deterministic(self, mocker): + provider = _make_provider(mocker, FakeFlavorPlugin(_palo_alto_driver())) + assert provider._parent_port_name("r1") == "palo-alto-router-anchor-r1" + assert provider._trunk_name("r1") == "palo-alto-router-trunk-r1" + + def test_trunk_plugin_delegates_to_utils(self, mocker): + tp = mocker.Mock() + mocker.patch.object(palo_alto.utils, "fetch_trunk_plugin", return_value=tp) + provider = _make_provider(mocker, FakeFlavorPlugin(_palo_alto_driver())) + assert provider._trunk_plugin is tp + + def test_gateway_port_found_filters_by_owner_and_router(self, mocker): + core_plugin = mocker.Mock() + core_plugin.get_ports.return_value = [{"id": "gw-1"}] + provider = _make_provider( + mocker, FakeFlavorPlugin(_palo_alto_driver()), core_plugin=core_plugin + ) + + assert provider._gateway_port_for_router("r1") == {"id": "gw-1"} + _args, kwargs = core_plugin.get_ports.call_args + assert kwargs["filters"]["device_id"] == ["r1"] + assert kwargs["filters"]["device_owner"] == ["network:router_gateway"] + + def test_gateway_port_none_when_absent(self, mocker): + core_plugin = mocker.Mock() + core_plugin.get_ports.return_value = [] + provider = _make_provider( + mocker, FakeFlavorPlugin(_palo_alto_driver()), core_plugin=core_plugin + ) + assert provider._gateway_port_for_router("r1") is None + + +class TestParentPort: + def _core_plugin(self, mocker, ports): + core = mocker.Mock() + core.get_networks.return_value = [{"id": "anchor-net"}] # anchor exists + core.get_ports.return_value = ports + core.create_port.return_value = {"id": "parent-new"} + return core + + def test_creates_parent_when_absent(self, mocker): + core = self._core_plugin(mocker, ports=[]) + provider = _make_provider( + mocker, FakeFlavorPlugin(_palo_alto_driver()), core_plugin=core + ) + + port = provider._ensure_parent_port({"id": "r1"}) + + assert port == {"id": "parent-new"} + core.create_port.assert_called_once() + _ctx, body = core.create_port.call_args[0] + net = body["port"] + assert net["name"] == "palo-alto-router-anchor-r1" + assert net["network_id"] == "anchor-net" + assert net["binding:vnic_type"] == "baremetal" + assert net["device_id"] == "r1" + + def test_reuses_existing_parent(self, mocker): + core = self._core_plugin(mocker, ports=[{"id": "parent-existing"}]) + provider = _make_provider( + mocker, FakeFlavorPlugin(_palo_alto_driver()), core_plugin=core + ) + + port = provider._ensure_parent_port({"id": "r1"}) + + assert port == {"id": "parent-existing"} + core.create_port.assert_not_called() + + +_ANNOTATED_PARENT = { + "id": "parent-1", + "binding:host_id": "node-1", + "binding:profile": { + "physical_network": "n11-22-network", + "local_link_information": [{"switch_id": "aa", "port_id": "Eth1/1"}], + }, +} + + +class TestParentVifAttach: + def _provider(self, mocker, node, vif_ids, fresh_port=None): + ironic = mocker.Mock() + ironic.node_by_instance_uuid.return_value = node + ironic.node_vif_ids.return_value = vif_ids + core = mocker.Mock() + core.get_port.return_value = fresh_port or _ANNOTATED_PARENT + provider = _make_provider( + mocker, + FakeFlavorPlugin(_palo_alto_driver()), + ironic=ironic, + core_plugin=core, + ) + return provider, ironic + + def test_attaches_when_not_already(self, mocker): + node = mocker.Mock(id="node-1") + provider, ironic = self._provider(mocker, node, vif_ids=[]) + + result = provider._ensure_parent_vif_attached({"id": "r1"}, {"id": "parent-1"}) + + ironic.attach_vif_to_node.assert_called_once_with(node, "parent-1") + assert result == _ANNOTATED_PARENT # fresh, annotated copy returned + + def test_skips_attach_when_already_attached(self, mocker): + node = mocker.Mock(id="node-1") + provider, ironic = self._provider(mocker, node, vif_ids=["parent-1"]) + + provider._ensure_parent_vif_attached({"id": "r1"}, {"id": "parent-1"}) + + ironic.attach_vif_to_node.assert_not_called() + + def test_raises_when_no_adopted_node(self, mocker): + provider, ironic = self._provider(mocker, node=None, vif_ids=[]) + + with pytest.raises(n_exc.BadRequest): + provider._ensure_parent_vif_attached({"id": "r1"}, {"id": "parent-1"}) + ironic.attach_vif_to_node.assert_not_called() + + def test_raises_when_parent_not_annotated(self, mocker): + # e.g. the enrolled baremetal port had no physical_network + node = mocker.Mock(id="node-1") + unannotated = {"id": "parent-1", "binding:host_id": "", "binding:profile": {}} + provider, _ = self._provider(mocker, node, vif_ids=[], fresh_port=unannotated) + + with pytest.raises(n_exc.BadRequest): + provider._ensure_parent_vif_attached({"id": "r1"}, {"id": "parent-1"}) + + +class TestTrunk: + def _provider_with_trunk(self, mocker, existing_trunks): + tp = mocker.Mock() + tp.get_trunks.return_value = existing_trunks + tp.create_trunk.return_value = {"id": "trunk-new"} + mocker.patch.object(palo_alto.utils, "fetch_trunk_plugin", return_value=tp) + provider = _make_provider(mocker, FakeFlavorPlugin(_palo_alto_driver())) + return provider, tp + + def test_creates_trunk_when_absent(self, mocker): + provider, tp = self._provider_with_trunk(mocker, existing_trunks=[]) + + trunk = provider._ensure_trunk({"id": "r1"}, {"id": "parent-1"}) + + assert trunk == {"id": "trunk-new"} + tp.create_trunk.assert_called_once() + _ctx, body = tp.create_trunk.call_args[0] + assert body["trunk"]["name"] == "palo-alto-router-trunk-r1" + assert body["trunk"]["port_id"] == "parent-1" + assert body["trunk"]["sub_ports"] == [] + + def test_reuses_existing_trunk(self, mocker): + provider, tp = self._provider_with_trunk( + mocker, existing_trunks=[{"id": "trunk-existing"}] + ) + + trunk = provider._ensure_trunk({"id": "r1"}, {"id": "parent-1"}) + + assert trunk == {"id": "trunk-existing"} + tp.create_trunk.assert_not_called() + + +_GATEWAY_PORT = { + "id": "gw-1", + "device_id": "r1", + "device_owner": "network:router_gateway", +} + + +class TestGatewaySubport: + def _provider(self, mocker): + tp = mocker.Mock() + tp.add_subports.return_value = {"id": "trunk-1", "updated": True} + mocker.patch.object(palo_alto.utils, "fetch_trunk_plugin", return_value=tp) + self.clear = mocker.patch.object(palo_alto.utils, "clear_device_id_for_port") + self.restore = mocker.patch.object( + palo_alto.utils, "set_device_id_and_owner_for_port" + ) + provider = _make_provider(mocker, FakeFlavorPlugin(_palo_alto_driver())) + return provider, tp + + def test_adds_subport_with_fixed_vlan(self, mocker): + provider, tp = self._provider(mocker) + trunk = {"id": "trunk-1", "sub_ports": []} + + provider._add_gateway_subport({"id": "r1"}, trunk, dict(_GATEWAY_PORT)) + + tp.add_subports.assert_called_once() + _ctx, trunk_id, body = tp.add_subports.call_args[0] + assert trunk_id == "trunk-1" + sub = body["sub_ports"][0] + assert sub["port_id"] == "gw-1" + assert sub["segmentation_type"] == "vlan" + assert sub["segmentation_id"] == palo_alto.GATEWAY_SUBPORT_VLAN + # device_id cleared for the add (trunk validator rejects it) and restored + self.clear.assert_called_once_with("gw-1") + self.restore.assert_called_once_with("gw-1", "r1", "network:router_gateway") + + def test_add_subport_is_idempotent(self, mocker): + provider, tp = self._provider(mocker) + trunk = { + "id": "trunk-1", + "sub_ports": [ + { + "port_id": "gw-1", + "segmentation_id": palo_alto.GATEWAY_SUBPORT_VLAN, + } + ], + } + + provider._add_gateway_subport({"id": "r1"}, trunk, dict(_GATEWAY_PORT)) + + tp.add_subports.assert_not_called() + self.clear.assert_not_called() + + +class TestGatewayCreateHandler: + def _payload(self, mocker, router_id="r1"): + payload = mocker.Mock() + payload.context = "ctx" + payload.resource_id = router_id + return payload + + def test_orchestrates_in_order(self, mocker): + provider = _make_provider(mocker, FakeFlavorPlugin(_palo_alto_driver())) + router = {"id": "r1", "flavor_id": "f1"} + provider.l3plugin.get_router.return_value = router + mocker.patch.object(provider, "_is_palo_alto_provider", return_value=True) + mocker.patch.object( + provider, "_gateway_port_for_router", return_value={"id": "gw-1"} + ) + parent = {"id": "parent-1"} + bound = {"id": "parent-1", "bound": True} + trunk = {"id": "trunk-1"} + m_parent = mocker.patch.object( + provider, "_ensure_parent_port", return_value=parent + ) + m_vif = mocker.patch.object( + provider, "_ensure_parent_vif_attached", return_value=bound + ) + m_trunk = mocker.patch.object(provider, "_ensure_trunk", return_value=trunk) + m_sub = mocker.patch.object(provider, "_add_gateway_subport") + + provider._process_gateway_create("r", "e", "t", self._payload(mocker)) + + m_parent.assert_called_once_with(router) + # VIF-attach runs on the parent BEFORE the trunk/subport + m_vif.assert_called_once_with(router, parent) + # trunk + subport use the BOUND parent + m_trunk.assert_called_once_with(router, bound) + m_sub.assert_called_once_with(router, trunk, {"id": "gw-1"}) + + def test_skips_non_palo_alto_router(self, mocker): + provider = _make_provider( + mocker, FakeFlavorPlugin("neutron_understack.l3_router.vrf.Vrf") + ) + provider.l3plugin.get_router.return_value = {"id": "r1", "flavor_id": "f1"} + m_parent = mocker.patch.object(provider, "_ensure_parent_port") + + provider._process_gateway_create("r", "e", "t", self._payload(mocker)) + + m_parent.assert_not_called() + + def test_raises_when_gateway_port_missing(self, mocker): + provider = _make_provider(mocker, FakeFlavorPlugin(_palo_alto_driver())) + provider.l3plugin.get_router.return_value = {"id": "r1", "flavor_id": "f1"} + mocker.patch.object(provider, "_is_palo_alto_provider", return_value=True) + mocker.patch.object(provider, "_gateway_port_for_router", return_value=None) + + with pytest.raises(n_exc.BadRequest): + provider._process_gateway_create("r", "e", "t", self._payload(mocker)) + + +class TestGatewayTeardown: + def _provider(self, mocker, trunk_after_removal): + tp = mocker.Mock() + tp.get_trunk.return_value = trunk_after_removal + mocker.patch.object(palo_alto.utils, "fetch_trunk_plugin", return_value=tp) + ironic = mocker.Mock() + ironic.node_by_instance_uuid.return_value = mocker.Mock(id="node-1") + core = mocker.Mock() + provider = _make_provider( + mocker, + FakeFlavorPlugin(_palo_alto_driver()), + ironic=ironic, + core_plugin=core, + ) + return provider, tp, ironic, core + + def test_remove_subport_when_present(self, mocker): + provider, tp, _, _ = self._provider(mocker, trunk_after_removal={}) + trunk = {"id": "trunk-1", "sub_ports": [{"port_id": "gw-1"}]} + + provider._remove_gateway_subport(trunk, "gw-1") + + tp.remove_subports.assert_called_once() + _ctx, tid, body = tp.remove_subports.call_args[0] + assert tid == "trunk-1" + assert body["sub_ports"] == [{"port_id": "gw-1"}] + + def test_remove_subport_idempotent(self, mocker): + provider, tp, _, _ = self._provider(mocker, trunk_after_removal={}) + trunk = {"id": "trunk-1", "sub_ports": []} + + provider._remove_gateway_subport(trunk, "gw-1") + + tp.remove_subports.assert_not_called() + + def test_deletes_stack_when_no_subports_left(self, mocker): + # after removal the trunk has no subports -> delete trunk + parent + provider, tp, ironic, core = self._provider( + mocker, + trunk_after_removal={ + "id": "trunk-1", + "port_id": "parent-1", + "sub_ports": [], + }, + ) + + provider._delete_parent_stack_if_unused("r1", {"id": "trunk-1"}) + + tp.delete_trunk.assert_called_once() + ironic.detach_vif_from_node.assert_called_once() + core.delete_port.assert_called_once() + _ctx, parent_id = core.delete_port.call_args[0] + assert parent_id == "parent-1" + + def test_keeps_stack_when_subports_remain(self, mocker): + # a subnet subport still present -> leave trunk + parent alone + provider, tp, ironic, core = self._provider( + mocker, + trunk_after_removal={ + "id": "trunk-1", + "port_id": "parent-1", + "sub_ports": [{"port_id": "subnet-x"}], + }, + ) + + provider._delete_parent_stack_if_unused("r1", {"id": "trunk-1"}) + + tp.delete_trunk.assert_not_called() + ironic.detach_vif_from_node.assert_not_called() + core.delete_port.assert_not_called() + + +class TestGatewayDeleteHandler: + def _payload(self, mocker, router_id="r1"): + payload = mocker.Mock() + payload.context = "ctx" + payload.resource_id = router_id + return payload + + def test_cleans_up_when_palo_alto(self, mocker): + provider = _make_provider(mocker, FakeFlavorPlugin(_palo_alto_driver())) + router = {"id": "r1", "flavor_id": "f1"} + provider.l3plugin.get_router.return_value = router + mocker.patch.object(provider, "_is_palo_alto_provider", return_value=True) + mocker.patch.object( + provider, "_gateway_port_for_router", return_value={"id": "gw-1"} + ) + m_cleanup = mocker.patch.object(provider, "_cleanup_gateway_attachment") + + provider._process_gateway_delete("r", "e", "t", self._payload(mocker)) + + m_cleanup.assert_called_once_with(router, {"id": "gw-1"}) + + def test_skips_non_palo_alto(self, mocker): + provider = _make_provider( + mocker, FakeFlavorPlugin("neutron_understack.l3_router.vrf.Vrf") + ) + provider.l3plugin.get_router.return_value = {"id": "r1", "flavor_id": "f1"} + m_cleanup = mocker.patch.object(provider, "_cleanup_gateway_attachment") + + provider._process_gateway_delete("r", "e", "t", self._payload(mocker)) + + m_cleanup.assert_not_called() + + def test_skips_when_no_gateway_port(self, mocker): + provider = _make_provider(mocker, FakeFlavorPlugin(_palo_alto_driver())) + provider.l3plugin.get_router.return_value = {"id": "r1", "flavor_id": "f1"} + mocker.patch.object(provider, "_is_palo_alto_provider", return_value=True) + mocker.patch.object(provider, "_gateway_port_for_router", return_value=None) + m_cleanup = mocker.patch.object(provider, "_cleanup_gateway_attachment") + + provider._process_gateway_delete("r", "e", "t", self._payload(mocker)) + + m_cleanup.assert_not_called() + + +class TestGatewayCleanupPartialAdd: + def _provider(self, mocker, trunks, ports): + tp = mocker.Mock() + tp.get_trunks.return_value = trunks + mocker.patch.object(palo_alto.utils, "fetch_trunk_plugin", return_value=tp) + ironic = mocker.Mock() + ironic.node_by_instance_uuid.return_value = mocker.Mock(id="node-1") + core = mocker.Mock() + core.get_networks.return_value = [{"id": "anchor-net"}] + core.get_ports.return_value = ports + provider = _make_provider( + mocker, + FakeFlavorPlugin(_palo_alto_driver()), + ironic=ironic, + core_plugin=core, + ) + return provider, ironic, core + + def test_deletes_orphan_parent_when_no_trunk(self, mocker): + # partial add left a parent port but no trunk + provider, ironic, core = self._provider( + mocker, trunks=[], ports=[{"id": "parent-1"}] + ) + + provider._cleanup_gateway_attachment({"id": "r1"}, {"id": "gw-1"}) + + ironic.detach_vif_from_node.assert_called_once() + core.delete_port.assert_called_once() + _ctx, parent_id = core.delete_port.call_args[0] + assert parent_id == "parent-1" + + def test_noop_when_no_trunk_and_no_parent(self, mocker): + provider, ironic, core = self._provider(mocker, trunks=[], ports=[]) + + provider._cleanup_gateway_attachment({"id": "r1"}, {"id": "gw-1"}) + + core.delete_port.assert_not_called() + ironic.detach_vif_from_node.assert_not_called()