diff --git a/components/images-openstack.yaml b/components/images-openstack.yaml index 490161535..afad7a645 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-2310" neutron_rpc_server: "ghcr.io/rackerlabs/understack/neutron:2026.1" neutron_ovn_maintenance_worker: "ghcr.io/rackerlabs/understack/neutron:2026.1" neutron_bagpipe_bgp: "ghcr.io/rackerlabs/understack/neutron:2026.1" 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 41ccda720..bf5f6cea2 100644 --- a/python/neutron-understack/neutron_understack/l3_router/palo_alto.py +++ b/python/neutron-understack/neutron_understack/l3_router/palo_alto.py @@ -7,6 +7,7 @@ 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 priority_group from neutron_lib.callbacks import registry from neutron_lib.callbacks import resources from neutron_lib.plugins import constants as plugin_constants @@ -30,6 +31,7 @@ # 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 +INTERFACE_SUBPORT_VLAN_START = GATEWAY_SUBPORT_VLAN + 1 # Conflict -> HTTP 409: the request cannot be satisfied because the hardware @@ -49,6 +51,13 @@ class PaloAltoFlavorMisconfigured(n_exc.BadRequest): ) +class NoPaloAltoSubportVlanAvailable(n_exc.Conflict): + message = ( + "No Palo Alto trunk subport VLAN is available for router %(router_id)s " + "on trunk %(trunk_id)s. Allowed ranges: %(network_segment_ranges)s." + ) + + def _parse_metainfo(raw) -> dict: """Service-profile metainfo is stored as a JSON string.""" if not raw: @@ -100,6 +109,22 @@ def __init__(self, l3_plugin): events.BEFORE_DELETE, cancellable=True, ) + registry.subscribe( + self._process_router_interface_create, + resources.ROUTER_INTERFACE, + events.AFTER_CREATE, + cancellable=True, + ) + # Run before neutron.services.trunk.rules.enforce_port_deletion_rules + # (PRIORITY_DEFAULT) so a Palo Alto router-interface port can be removed + # from its trunk before Neutron checks whether the port is in use. + registry.subscribe( + self._process_router_interface_port_delete, + resources.PORT, + events.BEFORE_DELETE, + priority=priority_group.PRIORITY_DEFAULT - 1000, + cancellable=True, + ) LOG.info( "Palo Alto service provider initialized: driver=%r", self._palo_alto_provider, @@ -420,38 +445,72 @@ def _ensure_trunk(self, router: dict, parent_port: dict) -> dict: 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). + def _used_subport_vlans(self, trunk: dict) -> set[int]: + """Return VLAN segmentation IDs already used on a router trunk.""" + return { + sp["segmentation_id"] + for sp in trunk.get("sub_ports", []) + if sp.get("segmentation_type") == "vlan" + and sp.get("segmentation_id") is not None + } + + def _next_available_subport_vlan( + self, router_id: str, trunk: dict, start_vlan: int + ) -> int: + """Pick the first allowed, unused Palo Alto subport VLAN from start.""" + used = self._used_subport_vlans(trunk) + ranges = sorted(utils.allowed_tenant_vlan_id_ranges()) + for start, end in ranges: + for vlan in range(max(start, start_vlan), end + 1): + if vlan not in used: + return vlan + raise NoPaloAltoSubportVlanAvailable( + router_id=router_id, + trunk_id=trunk["id"], + network_segment_ranges=utils.printable_ranges(ranges), + ) + + def _add_router_port_subport( + self, + router: dict, + trunk: dict, + port: dict, + segmentation_id: int, + label: str, + ): + """Add a router-owned port to the trunk as a VLAN subport. 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. + program the switch. No-ops if the port is already a subport. """ - gateway_port_id = gateway_port["id"] + port_id = port["id"] existing = {sp["port_id"] for sp in trunk.get("sub_ports", [])} - if gateway_port_id in existing: + if port_id in existing: LOG.debug( - "Gateway port %s already a subport on trunk %s", - gateway_port_id, + "Palo Alto %s port %s already a subport on trunk %s", + label, + 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, + "Adding Palo Alto %s port %s to trunk %s as VLAN %s subport for router %s", + label, + port_id, trunk["id"], - GATEWAY_SUBPORT_VLAN, + segmentation_id, 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) + # (rules.py check_not_in_use). Router-owned ports have + # device_id=router_id, so clear it for the add and restore it + # afterwards so the router keeps its port association. + original_device_id = port["device_id"] + original_device_owner = port["device_owner"] + utils.clear_device_id_for_port(port_id) try: return self._trunk_plugin.add_subports( admin_context, @@ -459,42 +518,92 @@ def _add_gateway_subport(self, router: dict, trunk: dict, gateway_port: dict): { "sub_ports": [ { - "port_id": gateway_port_id, + "port_id": port_id, "segmentation_type": "vlan", - "segmentation_id": GATEWAY_SUBPORT_VLAN, + "segmentation_id": segmentation_id, } ] }, ) finally: utils.set_device_id_and_owner_for_port( - gateway_port_id, original_device_id, original_device_owner + 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). + def _add_gateway_subport(self, router: dict, trunk: dict, gateway_port: dict): + """Add the gateway port to the trunk as a VLAN subport (idempotent).""" + port_id = gateway_port["id"] + if port_id in {sp["port_id"] for sp in trunk.get("sub_ports", [])}: + LOG.debug( + "Palo Alto gateway port %s already a subport on trunk %s", + port_id, + trunk["id"], + ) + return trunk + return self._add_router_port_subport( + router, + trunk, + gateway_port, + GATEWAY_SUBPORT_VLAN, + "gateway", + ) + + def _add_interface_subport(self, router: dict, trunk: dict, interface_port: dict): + """Add a router-interface port to the trunk as a VLAN subport.""" + port_id = interface_port["id"] + if port_id in {sp["port_id"] for sp in trunk.get("sub_ports", [])}: + LOG.debug( + "Palo Alto interface port %s already a subport on trunk %s", + port_id, + trunk["id"], + ) + return trunk + return self._add_router_port_subport( + router, + trunk, + interface_port, + self._next_available_subport_vlan( + router["id"], trunk, INTERFACE_SUBPORT_VLAN_START + ), + "interface", + ) + + def _remove_router_port_subport(self, trunk: dict, port_id: str, label: str): + """Remove a router-owned 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: + if port_id not in existing: LOG.debug( - "Gateway port %s is not a subport on trunk %s; skip removal", - gateway_port_id, + "Palo Alto %s port %s is not a subport on trunk %s; skip removal", + label, + 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"] + "Removing Palo Alto %s subport %s from trunk %s", + label, + port_id, + trunk["id"], ) return self._trunk_plugin.remove_subports( admin_context, trunk["id"], - {"sub_ports": [{"port_id": gateway_port_id}]}, + {"sub_ports": [{"port_id": port_id}]}, ) + def _remove_gateway_subport(self, trunk: dict, gateway_port_id: str): + """Remove the gateway port from the trunk (idempotent).""" + return self._remove_router_port_subport(trunk, gateway_port_id, "gateway") + + def _remove_interface_subport(self, trunk: dict, interface_port_id: str): + """Remove a router-interface port from the trunk (idempotent).""" + return self._remove_router_port_subport(trunk, interface_port_id, "interface") + 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) @@ -536,7 +645,12 @@ 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_gateway_attachment(self, router: dict, gateway_port: dict) -> None: + def _cleanup_router_port_attachment( + self, + router: dict, + port_id: str, + label: str, + ) -> 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 @@ -560,9 +674,17 @@ def _cleanup_gateway_attachment(self, router: dict, gateway_port: dict) -> None: router_id, ) return - self._remove_gateway_subport(trunk, gateway_port["id"]) + 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") + @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. @@ -692,3 +814,77 @@ def _process_gateway_delete(self, resource, event, trigger, payload=None): router_id, gateway_port["id"], ) + + def _process_router_interface_create(self, resource, event, trigger, payload=None): + """ROUTER_INTERFACE / AFTER_CREATE: wire a subnet interface port.""" + 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 + + interface_port = payload.metadata.get("port") + if not interface_port: + raise n_exc.BadRequest( + resource="router", + msg=( + f"Palo Alto router {router_id} interface was created but no " + "router interface port was supplied." + ), + ) + if interface_port.get("device_owner") not in const.ROUTER_INTERFACE_OWNERS: + return + + parent = self._ensure_parent_port(router) + parent = self._ensure_parent_vif_attached(router, parent) + trunk = self._ensure_trunk(router, parent) + self._add_interface_subport(router, trunk, interface_port) + + LOG.info( + "Attached Palo Alto router %s interface port %s via parent %s trunk %s", + router_id, + interface_port["id"], + parent["id"], + trunk["id"], + ) + + def _process_router_interface_port_delete( + self, resource, event, trigger, payload=None + ): + """PORT / BEFORE_DELETE: remove Palo Alto interface trunk wiring. + + This runs before the trunk plugin's own port-in-use check. That allows + Neutron to delete a router-interface port that we previously attached as + a trunk subport. + """ + port = payload.metadata.get("port") if payload else None + if not port or port.get("device_owner") not in const.ROUTER_INTERFACE_OWNERS: + return + + context = payload.context + router_id = port.get("device_id") + if not router_id: + return + + try: + router = self.l3plugin.get_router(context, router_id) + is_palo_alto = self._is_palo_alto_provider(context, router) + except Exception: + LOG.debug( + "Skipping Palo Alto interface cleanup for port %s; router %s " + "could not be confirmed as Palo Alto", + port["id"], + router_id, + exc_info=True, + ) + return + + if not is_palo_alto: + return + + self._cleanup_interface_attachment(router, port) + LOG.info( + "Cleaned Palo Alto router %s interface attachment (port %s)", + router_id, + port["id"], + ) 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 b1f5c71f0..75a16e26b 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 @@ -131,6 +131,9 @@ def test_no_node_available_is_conflict(self): def test_flavor_misconfigured_is_bad_request(self): assert issubclass(palo_alto.PaloAltoFlavorMisconfigured, n_exc.BadRequest) + def test_no_subport_vlan_available_is_conflict(self): + assert issubclass(palo_alto.NoPaloAltoSubportVlanAvailable, n_exc.Conflict) + class TestResourceClassLookup: def test_reads_resource_class_from_profile_metainfo(self, mocker): @@ -489,6 +492,12 @@ def test_reuses_existing_trunk(self, mocker): "device_owner": "network:router_gateway", } +_INTERFACE_PORT = { + "id": "intf-1", + "device_id": "r1", + "device_owner": "network:router_interface", +} + class TestGatewaySubport: def _provider(self, mocker): @@ -537,6 +546,139 @@ def test_add_subport_is_idempotent(self, mocker): self.clear.assert_not_called() +class TestSubportVlanAllocation: + def _provider(self, mocker, ranges): + mocker.patch.object( + palo_alto.utils, + "allowed_tenant_vlan_id_ranges", + return_value=ranges, + ) + return _make_provider(mocker, FakeFlavorPlugin(_palo_alto_driver())) + + def test_starts_at_requested_vlan(self, mocker): + provider = self._provider(mocker, ranges=[(1, 199), (200, 202)]) + trunk = {"id": "trunk-1", "sub_ports": []} + + assert ( + provider._next_available_subport_vlan( + "r1", trunk, palo_alto.INTERFACE_SUBPORT_VLAN_START + ) + == 201 + ) + + def test_skips_used_vlans(self, mocker): + provider = self._provider(mocker, ranges=[(200, 202)]) + trunk = { + "id": "trunk-1", + "sub_ports": [ + { + "port_id": "gw-1", + "segmentation_type": "vlan", + "segmentation_id": 200, + }, + { + "port_id": "intf-1", + "segmentation_type": "vlan", + "segmentation_id": 201, + }, + ], + } + + assert ( + provider._next_available_subport_vlan( + "r1", trunk, palo_alto.INTERFACE_SUBPORT_VLAN_START + ) + == 202 + ) + + def test_raises_when_no_vlan_available(self, mocker): + provider = self._provider(mocker, ranges=[(200, 201)]) + trunk = { + "id": "trunk-1", + "sub_ports": [ + { + "port_id": "gw-1", + "segmentation_type": "vlan", + "segmentation_id": 200, + }, + { + "port_id": "intf-1", + "segmentation_type": "vlan", + "segmentation_id": 201, + }, + ], + } + + with pytest.raises(palo_alto.NoPaloAltoSubportVlanAvailable): + provider._next_available_subport_vlan( + "r1", trunk, palo_alto.INTERFACE_SUBPORT_VLAN_START + ) + + +class TestInterfaceSubport: + 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_next_available_vlan(self, mocker): + provider, tp = self._provider(mocker) + next_vlan = mocker.patch.object( + provider, "_next_available_subport_vlan", return_value=201 + ) + trunk = { + "id": "trunk-1", + "sub_ports": [ + { + "port_id": "gw-1", + "segmentation_type": "vlan", + "segmentation_id": palo_alto.GATEWAY_SUBPORT_VLAN, + } + ], + } + + provider._add_interface_subport({"id": "r1"}, trunk, dict(_INTERFACE_PORT)) + + next_vlan.assert_called_once_with( + "r1", trunk, palo_alto.INTERFACE_SUBPORT_VLAN_START + ) + 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"] == "intf-1" + assert sub["segmentation_type"] == "vlan" + assert sub["segmentation_id"] == 201 + self.clear.assert_called_once_with("intf-1") + self.restore.assert_called_once_with("intf-1", "r1", "network:router_interface") + + def test_add_subport_is_idempotent(self, mocker): + provider, tp = self._provider(mocker) + next_vlan = mocker.patch.object(provider, "_next_available_subport_vlan") + trunk = { + "id": "trunk-1", + "sub_ports": [ + { + "port_id": "intf-1", + "segmentation_type": "vlan", + "segmentation_id": 201, + } + ], + } + + provider._add_interface_subport({"id": "r1"}, trunk, dict(_INTERFACE_PORT)) + + next_vlan.assert_not_called() + tp.add_subports.assert_not_called() + self.clear.assert_not_called() + + class TestGatewayCreateHandler: def _payload(self, mocker, router_id="r1"): payload = mocker.Mock() @@ -594,6 +736,73 @@ def test_raises_when_gateway_port_missing(self, mocker): provider._process_gateway_create("r", "e", "t", self._payload(mocker)) +class TestRouterInterfaceCreateHandler: + def _payload(self, mocker, router_id="r1", port=None): + payload = mocker.Mock() + payload.context = "ctx" + payload.resource_id = router_id + payload.metadata = {"port": port if port is not None else dict(_INTERFACE_PORT)} + 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) + 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_interface_subport") + + provider._process_router_interface_create("r", "e", "t", self._payload(mocker)) + + m_parent.assert_called_once_with(router) + m_vif.assert_called_once_with(router, parent) + m_trunk.assert_called_once_with(router, bound) + m_sub.assert_called_once_with(router, trunk, dict(_INTERFACE_PORT)) + + 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_router_interface_create("r", "e", "t", self._payload(mocker)) + + m_parent.assert_not_called() + + def test_skips_non_router_interface_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) + m_parent = mocker.patch.object(provider, "_ensure_parent_port") + port = {**_INTERFACE_PORT, "device_owner": "network:dhcp"} + + provider._process_router_interface_create( + "r", "e", "t", self._payload(mocker, port=port) + ) + + m_parent.assert_not_called() + + def test_raises_when_interface_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) + payload = self._payload(mocker, port=None) + payload.metadata = {} + + with pytest.raises(n_exc.BadRequest): + provider._process_router_interface_create("r", "e", "t", payload) + + class TestGatewayTeardown: def _provider(self, mocker, trunk_after_removal): tp = mocker.Mock() @@ -629,6 +838,17 @@ def test_remove_subport_idempotent(self, mocker): tp.remove_subports.assert_not_called() + def test_remove_interface_subport_when_present(self, mocker): + provider, tp, _, _ = self._provider(mocker, trunk_after_removal={}) + trunk = {"id": "trunk-1", "sub_ports": [{"port_id": "intf-1"}]} + + provider._remove_interface_subport(trunk, "intf-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": "intf-1"}] + 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( @@ -710,6 +930,96 @@ def test_skips_when_no_gateway_port(self, mocker): m_cleanup.assert_not_called() +class TestRouterInterfacePortDeleteHandler: + def _payload(self, mocker, port=None): + payload = mocker.Mock() + payload.context = "ctx" + payload.resource_id = "intf-1" + payload.metadata = {"port": port if port is not None else dict(_INTERFACE_PORT)} + return payload + + def test_cleans_up_before_port_delete_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) + m_cleanup = mocker.patch.object(provider, "_cleanup_interface_attachment") + + provider._process_router_interface_port_delete( + "r", "e", "t", self._payload(mocker) + ) + + m_cleanup.assert_called_once_with(router, dict(_INTERFACE_PORT)) + + def test_skips_non_router_interface_port(self, mocker): + provider = _make_provider(mocker, FakeFlavorPlugin(_palo_alto_driver())) + m_cleanup = mocker.patch.object(provider, "_cleanup_interface_attachment") + port = {**_INTERFACE_PORT, "device_owner": "network:dhcp"} + + provider._process_router_interface_port_delete( + "r", "e", "t", self._payload(mocker, port=port) + ) + + m_cleanup.assert_not_called() + provider.l3plugin.get_router.assert_not_called() + + 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_cleanup = mocker.patch.object(provider, "_cleanup_interface_attachment") + + provider._process_router_interface_port_delete( + "r", "e", "t", self._payload(mocker) + ) + + m_cleanup.assert_not_called() + + def test_skips_when_router_lookup_fails(self, mocker): + provider = _make_provider(mocker, FakeFlavorPlugin(_palo_alto_driver())) + provider.l3plugin.get_router.side_effect = RuntimeError("db hiccup") + m_cleanup = mocker.patch.object(provider, "_cleanup_interface_attachment") + + provider._process_router_interface_port_delete( + "r", "e", "t", self._payload(mocker) + ) + + m_cleanup.assert_not_called() + + def test_skips_when_palo_alto_check_fails(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", + side_effect=RuntimeError("flavor lookup failed"), + ) + m_cleanup = mocker.patch.object(provider, "_cleanup_interface_attachment") + + provider._process_router_interface_port_delete( + "r", "e", "t", self._payload(mocker) + ) + + m_cleanup.assert_not_called() + + def test_cleanup_error_still_raises_for_confirmed_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, + "_cleanup_interface_attachment", + side_effect=RuntimeError("cleanup failed"), + ) + + with pytest.raises(RuntimeError, match="cleanup failed"): + provider._process_router_interface_port_delete( + "r", "e", "t", self._payload(mocker) + ) + + class TestGatewayCleanupPartialAdd: def _provider(self, mocker, trunks, ports): tp = mocker.Mock()