Fix indexerror in delete_csnat_port

The code assumed every port returned from the csnat port query
would have a fixed_ip that it could compare the subnet it is
looking for to. This should be a valid assumption however there
is a path leading to a condition where it has no IPs. This makes
the cleanup code handle this case and dump a warning until we can
figure out what causes the interface to lose the IP.

Partial-Bug: #1609540
Change-Id: Ida024a231bb3fc09dad0e80498f57a5761ca3420

neutron/db/l3_dvr_db.py

@@ -898,7 +898,9 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin,
             filters={'id': ports}
         )
         for p in c_snat_ports:
-            if subnet_id is None:
+            if subnet_id is None or not p['fixed_ips']:
+                if not p['fixed_ips']:
+                    LOG.debug("CSNAT port has no IPs: %s", p)
                 self._core_plugin.delete_port(context,
                                               p['id'],
                                               l3_port_check=False)

neutron/tests/unit/db/test_l3_dvr_db.py

@@ -525,7 +525,7 @@ class L3DvrTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase):
                 self.ctx, filters=dvr_filters)
             self.assertEqual(1, len(dvr_ports))
 
-    def test_remove_router_interface_csnat_ports_removal_with_ipv6(self):
+    def _setup_router_with_v4_and_v6(self):
         router_dict = {'name': 'test_router', 'admin_state_up': True,
                        'distributed': True}
         router = self._create_router(router_dict)
@@ -548,36 +548,56 @@ class L3DvrTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase):
                     {'subnet_id': subnet_v4['subnet']['id']})
                 self.mixin.add_router_interface(self.ctx, router['id'],
                     {'subnet_id': subnet_v6['subnet']['id']})
-                csnat_filters = {'device_owner':
-                                 [l3_const.DEVICE_OWNER_ROUTER_SNAT]}
-                csnat_ports = self.core_plugin.get_ports(
-                    self.ctx, filters=csnat_filters)
-                self.assertEqual(2, len(csnat_ports))
-                dvr_filters = {'device_owner':
-                               [l3_const.DEVICE_OWNER_DVR_INTERFACE]}
-                dvr_ports = self.core_plugin.get_ports(
-                    self.ctx, filters=dvr_filters)
-                self.assertEqual(2, len(dvr_ports))
-                with mock.patch.object(
-                    manager.NeutronManager,
-                    'get_service_plugins') as get_svc_plugin:
-                    get_svc_plugin.return_value = {
-                        plugin_const.L3_ROUTER_NAT: plugin}
-                    self.mixin.manager = manager
-                    self.mixin.remove_router_interface(
-                        self.ctx, router['id'],
-                        {'subnet_id': subnet_v4['subnet']['id']})
+                get_svc_plugin = mock.patch.object(
+                    manager.NeutronManager, 'get_service_plugins').start()
+                get_svc_plugin.return_value = {
+                    plugin_const.L3_ROUTER_NAT: plugin}
+                self.mixin.manager = manager
+                return router, subnet_v4, subnet_v6
 
-                csnat_ports = self.core_plugin.get_ports(
-                    self.ctx, filters=csnat_filters)
-                self.assertEqual(1, len(csnat_ports))
-                self.assertEqual(
-                    subnet_v6['subnet']['id'],
-                    csnat_ports[0]['fixed_ips'][0]['subnet_id'])
+    def test_remove_router_interface_csnat_ports_removal_with_ipv6(self):
+        router, subnet_v4, subnet_v6 = self._setup_router_with_v4_and_v6()
+        csnat_filters = {'device_owner':
+                         [l3_const.DEVICE_OWNER_ROUTER_SNAT]}
+        csnat_ports = self.core_plugin.get_ports(
+            self.ctx, filters=csnat_filters)
+        self.assertEqual(2, len(csnat_ports))
+        dvr_filters = {'device_owner':
+                       [l3_const.DEVICE_OWNER_DVR_INTERFACE]}
+        dvr_ports = self.core_plugin.get_ports(
+            self.ctx, filters=dvr_filters)
+        self.assertEqual(2, len(dvr_ports))
+        self.mixin.remove_router_interface(
+            self.ctx, router['id'],
+            {'subnet_id': subnet_v4['subnet']['id']})
+        csnat_ports = self.core_plugin.get_ports(
+            self.ctx, filters=csnat_filters)
+        self.assertEqual(1, len(csnat_ports))
+        self.assertEqual(
+            subnet_v6['subnet']['id'],
+            csnat_ports[0]['fixed_ips'][0]['subnet_id'])
 
-                dvr_ports = self.core_plugin.get_ports(
-                    self.ctx, filters=dvr_filters)
-                self.assertEqual(1, len(dvr_ports))
+        dvr_ports = self.core_plugin.get_ports(
+            self.ctx, filters=dvr_filters)
+        self.assertEqual(1, len(dvr_ports))
+
+    def test_remove_router_interface_csnat_port_missing_ip(self):
+        # NOTE(kevinbenton): this is a contrived scenario to reproduce
+        # a condition observed in bug/1609540. Once we figure out why
+        # these ports lose their IP we can remove this test.
+        router, subnet_v4, subnet_v6 = self._setup_router_with_v4_and_v6()
+        self.mixin.remove_router_interface(
+            self.ctx, router['id'],
+            {'subnet_id': subnet_v4['subnet']['id']})
+        csnat_filters = {'device_owner':
+                         [l3_const.DEVICE_OWNER_ROUTER_SNAT]}
+        csnat_ports = self.core_plugin.get_ports(
+            self.ctx, filters=csnat_filters)
+        self.core_plugin.update_port(self.ctx, csnat_ports[0]['id'],
+                                     {'port': {'fixed_ips': []}})
+        self.mixin.remove_router_interface(
+            self.ctx, router['id'],
+            {'subnet_id': subnet_v6['subnet']['id']})
 
     def test__validate_router_migration_notify_advanced_services(self):
         router = {'name': 'foo_router', 'admin_state_up': False}