Merge pull request #1041 from rackerlabs/stop-stuff-deletion

fix: router iface removal: switch to PRECOMMIT_DELETE

Author: skrobul

python/neutron-understack/neutron_understack/neutron_understack_mech.py

@@ -64,7 +64,7 @@ def subscribe(self):
         registry.subscribe(
             routers.handle_router_interface_removal,
             resources.PORT,
-            events.BEFORE_DELETE,
+            events.PRECOMMIT_DELETE,
             cancellable=True,
         )
 

python/neutron-understack/neutron_understack/routers.py

@@ -219,10 +219,13 @@ def handle_router_interface_removal(_resource, _event, trigger, payload) -> None
     In situation 2, we don't have to do anything. Router-specific port gets
     deleted by Neutron and segment-shared port stays around.
     """
-    port = payload.metadata["port"]
-    network_id = port["network_id"]
+    LOG.debug(
+        "handle_router_interface_removal received %(payload)s", {"payload": payload}
+    )
+    port = payload.metadata["port_db"]
+    network_id = payload.metadata["network"]["id"]
 
-    if port["device_owner"] not in ROUTER_INTERFACE_AND_GW:
+    if port.device_owner not in ROUTER_INTERFACE_AND_GW:
         return
 
     if not is_only_router_port_on_network(network_id):

python/neutron-understack/neutron_understack/tests/conftest.py

@@ -338,6 +338,15 @@ def port_payload(network_id) -> DBEventPayload:
     return DBEventPayload("context", metadata=metadata)
 
 
+@pytest.fixture
+def port_db_payload(network) -> DBEventPayload:
+    metadata = {
+        "port_db": PortModel(device_owner=p_const.DEVICE_OWNER_ROUTER_GW),
+        "network": network,
+    }
+    return DBEventPayload("context", metadata=metadata)
+
+
 @pytest.fixture
 def ml2_understack_conf(mocker, ucvni_group_id) -> None:
     mocker.patch(

python/neutron-understack/neutron_understack/tests/test_routers.py

@@ -79,21 +79,22 @@ def test_when_successful(self, mocker, port_id, trunk_id):
 
 
 class TestHandleRouterInterfaceRemoval:
-    def test_case_for_non_router(self, mocker, port_payload):
+    def test_case_for_non_router(self, mocker, port_db_payload):
         mock_sb_removal = mocker.patch(
             "neutron_understack.routers.handle_subport_removal"
         )
         mock_localnet_removal = mocker.patch(
             "neutron_understack.routers.delete_uplink_port"
         )
 
-        port_payload.metadata["port"]["device_owner"] = "not_a_router"
-        handle_router_interface_removal(None, None, None, port_payload)
+        port_db_payload.metadata["port_db"].device_owner = "not_a_router"
+
+        handle_router_interface_removal(None, None, None, port_db_payload)
 
         mock_sb_removal.assert_not_called()
         mock_localnet_removal.assert_not_called()
 
-    def test_when_network_in_use(self, mocker, port_payload):
+    def test_when_network_in_use(self, mocker, port_db_payload):
         mock_sb_removal = mocker.patch(
             "neutron_understack.routers.handle_subport_removal"
         )
@@ -105,12 +106,14 @@ def test_when_network_in_use(self, mocker, port_payload):
             return_value=False,
         )
 
-        handle_router_interface_removal(None, None, None, port_payload)
+        handle_router_interface_removal(None, None, None, port_db_payload)
 
         mock_sb_removal.assert_not_called()
         mock_localnet_removal.assert_not_called()
 
-    def test_last_port_on_network(self, mocker, port_object, port_payload, network_id):
+    def test_last_port_on_network(
+        self, mocker, port_object, port_db_payload, network_id
+    ):
         mock_sb_removal = mocker.patch(
             "neutron_understack.routers.handle_subport_removal"
         )
@@ -133,7 +136,7 @@ def test_last_port_on_network(self, mocker, port_object, port_payload, network_i
         )
         delete_shared_port = mocker.patch.object(port_object, "delete")
 
-        handle_router_interface_removal(None, None, None, port_payload)
+        handle_router_interface_removal(None, None, None, port_db_payload)
 
         mock_sb_removal.assert_called_once_with(port_object)
         mock_localnet_removal.assert_called_once_with(fake_segment, str(network_id))