Maintenance: Fix router ports

Prior to this patch the maintenance thread would fail to fix a router
port resource upon a failure in OVN to create it.

The maintence thread was calling the add_router_interface() method from
the l3_ovn.py module but, if the router port already existed in Neutron
(but didn't exist in OVN) that method would fail with a PortInUse
exception.

This patch accounts for that problem and if the port router already
exist in Neutron it will only fetch it from the database and continue
the work until it's created in the OVN database as well.

A new functional test has been added which to test this scenario.

Closes-Bug: #1746979
Change-Id: Iae500d17d0efe17f3460dc2f09356675d406abed

networking_ovn/common/maintenance.py

@@ -268,4 +268,4 @@ class DBInconsistenciesPeriodics(object):
         admin_context = n_context.get_admin_context()
         router_id = port['device_id']
         self._ovn_client._l3_plugin.add_router_interface(
-            admin_context, router_id, {'port_id': port['id']})
+            admin_context, router_id, {'port_id': port['id']}, may_exist=True)

networking_ovn/l3/l3_ovn.py

@@ -160,10 +160,39 @@ class OVNL3RouterPlugin(service_base.ServicePluginBase,
                 super(OVNL3RouterPlugin, self).create_router(
                     context, {'router': original_router})
 
-    def add_router_interface(self, context, router_id, interface_info):
-        router_interface_info = \
-            super(OVNL3RouterPlugin, self).add_router_interface(
-                context, router_id, interface_info)
+    def _add_neutron_router_interface(self, context, router_id,
+                                      interface_info, may_exist=False):
+        try:
+            router_interface_info = (
+                super(OVNL3RouterPlugin, self).add_router_interface(
+                    context, router_id, interface_info))
+        except n_exc.PortInUse:
+            if not may_exist:
+                raise
+            # NOTE(lucasagomes): If the port is already being used it means
+            # the interface has been created already, let's just fetch it from
+            # the database. Perhaps the code below should live in Neutron
+            # itself, a get_router_interface() method in the main class
+            # would be handy
+            port = self._plugin.get_port(context, interface_info['port_id'])
+            fixed_ips = [ip for ip in port['fixed_ips']]
+            subnets = []
+            for fixed_ip in fixed_ips:
+                subnets.append(self._ovn_client._plugin.get_subnet(
+                    context, fixed_ip['subnet_id']))
+
+            router_interface_info = (
+                self._make_router_interface_info(
+                    router_id, port['tenant_id'], port['id'],
+                    port['network_id'], subnets[0]['id'],
+                    [subnet['id'] for subnet in subnets]))
+
+        return router_interface_info
+
+    def add_router_interface(self, context, router_id, interface_info,
+                             may_exist=False):
+        router_interface_info = self._add_neutron_router_interface(
+            context, router_id, interface_info, may_exist=may_exist)
         port = self._plugin.get_port(context, router_interface_info['port_id'])
 
         multi_prefix = False

networking_ovn/tests/functional/test_maintenance.py

@@ -171,12 +171,19 @@ class _TestMaintenanceHelper(base.TestOVNFunctionalBase):
                     ovn_const.OVN_SG_RULE_EXT_ID_KEY) == sgr_id):
                 return row
 
-    def _add_router_interface(self, router_id, port_id):
-        req = self.new_action_request('routers', {'port_id': port_id},
-                                      router_id, 'add_router_interface')
+    def _process_router_interface(self, action, router_id, subnet_id):
+        req = self.new_action_request(
+            'routers', {'subnet_id': subnet_id}, router_id,
+            '%s_router_interface' % action)
         res = req.get_response(self.api)
         return self.deserialize(self.fmt, res)
 
+    def _add_router_interface(self, router_id, subnet_id):
+        return self._process_router_interface('add', router_id, subnet_id)
+
+    def _remove_router_interface(self, router_id, subnet_id):
+        return self._process_router_interface('remove', router_id, subnet_id)
+
     def _find_router_port_row_by_port_id(self, port_id):
         for row in self.nb_api._tables['Logical_Router_Port'].rows.values():
             if row.name == utils.ovn_lrouter_port_name(port_id):
@@ -496,3 +503,41 @@ class TestMaintenance(_TestMaintenanceHelper):
         # FIXME(lucasagomes): Maintenance thread fixing deleted
         # security group rules is currently broken due to:
         # https://bugs.launchpad.net/networking-ovn/+bug/1756123
+
+    def test_router_port(self):
+        neutron_net = self._create_network('networktest', external=True)
+        neutron_subnet = self._create_subnet('subnettest', neutron_net['id'])
+        neutron_router = self._create_router('routertest')
+
+        with mock.patch.object(self._l3_ovn_client, 'create_router_port'):
+            with mock.patch('networking_ovn.db.revision.bump_revision'):
+                neutron_obj = self._add_router_interface(neutron_router['id'],
+                                                         neutron_subnet['id'])
+
+        # Assert the router port doesn't exist in OVN
+        self.assertIsNone(
+            self._find_router_port_row_by_port_id(neutron_obj['port_id']))
+
+        # Call the maintenance thread to fix the problem
+        self.maint.check_for_inconsistencies()
+
+        # Assert the router port was now created
+        self.assertIsNotNone(
+            self._find_router_port_row_by_port_id(neutron_obj['port_id']))
+
+        # > Delete
+
+        with mock.patch.object(self._l3_ovn_client, 'delete_router_port'):
+            self._remove_router_interface(neutron_router['id'],
+                                          neutron_subnet['id'])
+
+        # Assert the router port still exists in OVNDB
+        self.assertIsNotNone(
+            self._find_router_port_row_by_port_id(neutron_obj['port_id']))
+
+        # Call the maintenance thread to fix the problem
+        self.maint.check_for_inconsistencies()
+
+        # Assert the router port is now deleted from OVNDB
+        self.assertIsNone(
+            self._find_router_port_row_by_port_id(neutron_obj['port_id']))

networking_ovn/tests/unit/common/test_maintenance.py

@@ -174,4 +174,5 @@ class TestDBInconsistenciesPeriodics(db_base.DBTestCase,
         self.periodic._create_lrouter_port(port)
         l3_mock = self.periodic._ovn_client._l3_plugin
         l3_mock.add_router_interface.assert_called_once_with(
-            mock.ANY, port['device_id'], {'port_id': port['id']})
+            mock.ANY, port['device_id'], {'port_id': port['id']},
+            may_exist=True)