Merge "Remove csnat port when DVR migrated to non-DVR"

neutron/db/l3_dvr_db.py

@@ -143,6 +143,17 @@ class DVRResourceOperationHandler(object):
         for agent in cur_agents:
             self.l3plugin._unbind_router(context, router_db['id'], agent['id'])
 
+    @registry.receives(resources.ROUTER, [events.AFTER_UPDATE])
+    def _delete_snat_interfaces_after_change(self, resource, event, trigger,
+                                             context, router_id, router,
+                                             request_attrs, router_db,
+                                             **kwargs):
+        if router.get(l3.EXTERNAL_GW_INFO) and not router['distributed']:
+            old_router = kwargs['old_router']
+            if old_router and old_router['distributed']:
+                self.delete_csnat_router_interface_ports(
+                    context.elevated(), router_db)
+
     @registry.receives(resources.ROUTER,
                        [events.AFTER_CREATE, events.AFTER_UPDATE])
     def _create_snat_interfaces_after_change(self, resource, event, trigger,
@@ -528,19 +539,11 @@ class DVRResourceOperationHandler(object):
         # Each csnat router interface port is associated
         # with a subnet, so we need to pass the subnet id to
         # delete the right ports.
-
-        # TODO(markmcclain): This is suboptimal but was left to reduce
-        # changeset size since it is late in cycle
-        ports = [
-            rp.port.id for rp in
-            router.attached_ports.filter_by(
-                    port_type=const.DEVICE_OWNER_ROUTER_SNAT)
-            if rp.port
-        ]
-
+        filters = {'device_owner': [const.DEVICE_OWNER_ROUTER_SNAT],
+                   'device_id': [router.id]}
         c_snat_ports = self.l3plugin._core_plugin.get_ports(
             context,
-            filters={'id': ports}
+            filters=filters
         )
         for p in c_snat_ports:
             if subnet_id is None or not p['fixed_ips']:

neutron/tests/unit/db/test_l3_dvr_db.py

@@ -494,6 +494,40 @@ class L3DvrTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase):
                 self.ctx, filters=csnat_filters)
             self.assertEqual(1, len(csnat_ports))
 
+    def test_distributed_to_centralized_csnat_ports_removal(self):
+        router_dict = {'name': 'test_router', 'admin_state_up': True,
+                       'distributed': True}
+        router = self._create_router(router_dict)
+        with self.network() as net_ext,\
+                self.subnet() as subnet:
+            ext_net_id = net_ext['network']['id']
+            self.core_plugin.update_network(
+                self.ctx, ext_net_id,
+                {'network': {'router:external': True}})
+            self.mixin.update_router(
+                self.ctx, router['id'],
+                {'router': {'external_gateway_info':
+                            {'network_id': ext_net_id}}})
+            self.mixin.add_router_interface(self.ctx, router['id'],
+                {'subnet_id': subnet['subnet']['id']})
+
+            csnat_filters = {'device_owner':
+                             [const.DEVICE_OWNER_ROUTER_SNAT]}
+            csnat_ports = self.core_plugin.get_ports(
+                self.ctx, filters=csnat_filters)
+            self.assertEqual(1, len(csnat_ports))
+
+            self.mixin.update_router(
+                self.ctx, router['id'],
+                {'router': {'admin_state_up': False}})
+            self.mixin.update_router(
+                self.ctx, router['id'],
+                {'router': {'distributed': False}})
+
+            csnat_ports = self.core_plugin.get_ports(
+                self.ctx, filters=csnat_filters)
+            self.assertEqual(0, len(csnat_ports))
+
     def test_remove_router_interface_csnat_ports_removal(self):
         router_dict = {'name': 'test_router', 'admin_state_up': True,
                        'distributed': True}