[ovn] Do not create empty default route when empty gateway_ip

Before this patch, if a OVN router was associated with an
external network whose subnet had an empty `gateway_ip`, a
default route with an empty dst-ip is created in the OVN
database.

As documented in the Neutron API [0], it is allowed to create
a subnet without a gateway_ip.

0: https://docs.openstack.org/api-ref/network/v2/index.html?expanded=create-subnet-detail#create-subnet

Closes-Bug: #2002993
Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com>
Change-Id: Ica76e0821d753af883444d2a449283e9e69ba03f

neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py

@@ -891,6 +891,33 @@ class DBInconsistenciesPeriodics(SchemaAwarePeriodicsBase):
 
         raise periodics.NeverAgain()
 
+    @periodics.periodic(spacing=600, run_immediately=True)
+    def check_router_default_route_empty_dst_ip(self):
+        """Check routers with default route with empty dst-ip (LP: #2002993).
+        """
+        if not self.has_lock:
+            return
+
+        cmds = []
+        for router in self._nb_idl.lr_list().execute(check_error=True):
+            if not router.external_ids.get(ovn_const.OVN_REV_NUM_EXT_ID_KEY):
+                continue
+            for route in self._nb_idl.lr_route_list(router.uuid).execute(
+                    check_error=True):
+                if (route.nexthop == '' and
+                        (route.ip_prefix == n_const.IPv4_ANY or
+                         route.ip_prefix == n_const.IPv6_ANY)):
+                    cmds.append(
+                        self._nb_idl.delete_static_route(
+                            router.name, route.ip_prefix, ''))
+
+        if cmds:
+            with self._nb_idl.transaction(check_error=True) as txn:
+                for cmd in cmds:
+                    txn.add(cmd)
+
+        raise periodics.NeverAgain()
+
 
 class HashRingHealthCheckPeriodics(object):
 

neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py

@@ -1264,6 +1264,8 @@ class OVNClient(object):
         # 2. Add default route with nexthop as gateway ip
         lrouter_name = utils.ovn_name(router['id'])
         for gw_info in gateways:
+            if gw_info.gateway_ip is None:
+                continue
             columns = {'external_ids': {
                 ovn_const.OVN_ROUTER_IS_EXT_GW: 'true',
                 ovn_const.OVN_SUBNET_EXT_ID_KEY: gw_info.subnet_id}}

neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py

@@ -16,6 +16,7 @@
 from unittest import mock
 
 from futurist import periodics
+from neutron_lib import constants as n_const
 from neutron_lib import context
 from neutron_lib.db import api as db_api
 from oslo_config import cfg
@@ -797,3 +798,36 @@ class TestDBInconsistenciesPeriodics(testlib_api.SqlTestCaseLight,
         expected_calls = [mock.call('Logical_Switch_Port', lsp0.uuid,
                                     ('type', constants.LSP_TYPE_VIRTUAL))]
         nb_idl.db_set.assert_has_calls(expected_calls)
+
+    def test_check_router_default_route_empty_dst_ip(self):
+        nb_idl = self.fake_ovn_client._nb_idl
+        route0 = fakes.FakeOvsdbRow.create_one_ovsdb_row(
+            attrs={'ip_prefix': n_const.IPv4_ANY,
+                   'nexthop': '10.42.0.1'})
+        route1 = fakes.FakeOvsdbRow.create_one_ovsdb_row(
+            attrs={'ip_prefix': n_const.IPv4_ANY,
+                   'nexthop': ''})
+        route2 = fakes.FakeOvsdbRow.create_one_ovsdb_row(
+            attrs={'ip_prefix': n_const.IPv6_ANY,
+                   'nexthop': '2001:db8:42::1'})
+        route3 = fakes.FakeOvsdbRow.create_one_ovsdb_row(
+            attrs={'ip_prefix': n_const.IPv6_ANY,
+                   'nexthop': ''})
+        router0 = fakes.FakeOvsdbRow.create_one_ovsdb_row()
+        router1 = fakes.FakeOvsdbRow.create_one_ovsdb_row(
+                attrs={
+                    'external_ids': {constants.OVN_REV_NUM_EXT_ID_KEY: 1}
+                })
+        nb_idl.lr_list.return_value.execute.return_value = (router0, router1)
+        nb_idl.lr_route_list.return_value.execute.return_value = (
+            route0, route1, route2, route3)
+        self.assertRaises(
+            periodics.NeverAgain,
+            self.periodic.check_router_default_route_empty_dst_ip)
+        nb_idl.delete_static_route.assert_has_calls([
+            mock.call(router1.name, route1.ip_prefix, route1.nexthop),
+            mock.call(router1.name, route3.ip_prefix, route3.nexthop),
+        ])
+        self.assertEqual(
+            2,
+            nb_idl.delete_static_route.call_count)

neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_client.py

@@ -20,7 +20,9 @@ from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf
 from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import ovn_client
 from neutron.tests import base
 from neutron.tests.unit import fake_resources as fakes
+from neutron_lib.api.definitions import l3
 from neutron_lib.api.definitions import portbindings
+from neutron_lib import constants as const
 
 
 class TestOVNClientBase(base.BaseTestCase):
@@ -33,6 +35,70 @@ class TestOVNClientBase(base.BaseTestCase):
         self.ovn_client = ovn_client.OVNClient(self.nb_idl, self.sb_idl)
 
 
+class TestOVNClient(TestOVNClientBase):
+
+    def setUp(self):
+        super(TestOVNClient, self).setUp()
+        self.get_plugin = mock.patch(
+            'neutron_lib.plugins.directory.get_plugin').start()
+
+    def test__add_router_ext_gw_default_route(self):
+        plugin = mock.MagicMock()
+        self.get_plugin.return_value = plugin
+        subnet = {
+            'subnet_id': 'fake-subnet-id',
+            'gateway_ip': '10.42.0.1',
+            'ip_version': const.IP_VERSION_4,
+        }
+        plugin.get_subnet.return_value = subnet
+        router = {
+            'id': 'fake-router-id',
+            l3.EXTERNAL_GW_INFO: {
+                'external_fixed_ips': [{
+                        'subnet_id': subnet.get('subnet_id'),
+                        'ip_address': '10.42.0.42'}],
+            },
+            'gw_port_id': 'fake-port-id',
+        }
+        networks = mock.MagicMock()
+        txn = mock.MagicMock()
+        self.assertEqual(
+            self.get_plugin().get_port(),
+            self.ovn_client._add_router_ext_gw(router, networks, txn))
+        self.nb_idl.add_static_route.assert_called_once_with(
+            'neutron-' + router['id'],
+            ip_prefix='0.0.0.0/0',
+            nexthop='10.42.0.1',
+            external_ids={
+                'neutron:is_ext_gw': 'true',
+                'neutron:subnet_id': subnet['subnet_id']})
+
+    def test__add_router_ext_gw_no_default_route(self):
+        plugin = mock.MagicMock()
+        self.get_plugin.return_value = plugin
+        subnet = {
+            'subnet_id': 'fake-subnet-id',
+            'gateway_ip': None,
+            'ip_version': const.IP_VERSION_4
+        }
+        plugin.get_subnet.return_value = subnet
+        router = {
+            'id': 'fake-router-id',
+            l3.EXTERNAL_GW_INFO: {
+                'external_fixed_ips': [{
+                        'subnet_id': subnet.get('subnet_id'),
+                        'ip_address': '10.42.0.42'}],
+            },
+            'gw_port_id': 'fake-port-id',
+        }
+        networks = mock.MagicMock()
+        txn = mock.MagicMock()
+        self.assertEqual(
+            self.get_plugin().get_port(),
+            self.ovn_client._add_router_ext_gw(router, networks, txn))
+        self.nb_idl.add_static_route.assert_not_called()
+
+
 class TestOVNClientDetermineBindHost(TestOVNClientBase):
 
     def setUp(self):