[ovn] Ensure OVN DB update on change of number of GW ports

If a router is created and a GW port is subsequently added by
updating the router, the change is not always propagated to the
OVN DB.

This patch fixes this and also adds a functional test case.

Trivial-Fix
Partial-Bug: #2002687
Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com>
Change-Id: I9455678d73fb35b77eac7416917200a419abfa84

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

@@ -38,6 +38,7 @@ from neutron_lib.utils import net as n_net
 from oslo_config import cfg
 from oslo_log import log
 from oslo_utils import excutils
+from oslo_utils import strutils
 from oslo_utils import timeutils
 from oslo_utils import versionutils
 from ovsdbapp.backend.ovs_idl import idlutils
@@ -1467,8 +1468,16 @@ class OVNClient(object):
                 elif gateway_new and gateway_old:
                     # Check if external gateway has changed, if yes, delete
                     # the old gateway and add the new gateway
-                    if self._check_external_ips_changed(
-                            ovn_snats, gateway_old, new_router):
+                    ovn_router_ext_gw_lrps = [
+                        port
+                        for port in getattr(ovn_router, 'ports', [])
+                        if strutils.bool_from_string(
+                            getattr(port, 'external_ids', {}).get(
+                                ovn_const.OVN_ROUTER_IS_EXT_GW, False))
+                    ]
+                    if (len(gateway_new) != len(ovn_router_ext_gw_lrps) or
+                        self._check_external_ips_changed(
+                            ovn_snats, gateway_old, new_router)):
                         txn.add(self._nb_idl.delete_lrouter_ext_gw(
                             router_name))
                         if router_object:

neutron/tests/unit/fake_resources.py

@@ -800,6 +800,42 @@ FakeStaticRoute = collections.namedtuple(
     'Static_Routes', ['ip_prefix', 'nexthop', 'external_ids'])
 
 
+class FakeOVNRouterPort(object):
+    """Fake one or more Logical Router Ports."""
+
+    @staticmethod
+    def create_one_port(attrs=None):
+        """Create a fake ovn Logical Router Port.
+
+        :param Dictionary attrs:
+            A dictionary with all attributes
+        :return:
+            A FakeResource object faking the port
+        """
+        attrs = attrs or {}
+
+        # Set default attributes.
+        fake_uuid = uuidutils.generate_uuid()
+        port_attrs = {
+            'enabled': True,
+            'external_ids': {},
+            'gateway_chassis': [],
+            'ha_chassis_group': [],
+            'ipv6_prefix': [],
+            'ipv6_ra_configs': {},
+            'mac': '',
+            'name': fake_uuid,
+            'networks': [],
+            'options': {},
+            'peer': '',
+            'status': '',
+        }
+
+        # Overwrite default attributes.
+        port_attrs.update(attrs)
+        return type('Logical_Router_Port', (object, ), port_attrs)()
+
+
 class FakeOVNRouter(object):
 
     @staticmethod
@@ -835,6 +871,7 @@ class FakeOVNRouter(object):
                 router.get('name', 'no_router_name')}
 
         # Get the routes
+        ports = []
         routes = []
         for r in router.get('routes', []):
             routes.append(FakeStaticRoute(ip_prefix=r['destination'],
@@ -849,11 +886,16 @@ class FakeOVNRouter(object):
             routes.append(FakeStaticRoute(
                 ip_prefix='0.0.0.0/0', nexthop='',
                 external_ids=external_ids))
+            ports.append(FakeOVNRouterPort.create_one_port(
+                attrs={
+                    'external_ids': {ovn_const.OVN_ROUTER_IS_EXT_GW: 'True'},
+                }))
 
         return FakeOVNRouter.create_one_router(
             {'external_ids': external_ids,
              'enabled': router.get('admin_state_up') or False,
              'name': ovn_utils.ovn_name(router['id']),
+             'ports': ports,
              'static_routes': routes})
 
 

neutron/tests/unit/services/ovn_l3/test_plugin.py

@@ -844,6 +844,9 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase):
             [self.fake_ext_gw_port],
             [self.fake_ext_gw_port],
         ]
+        self.l3_inst._nb_ovn.get_lrouter.return_value = (
+            fake_resources.FakeOVNRouter.from_neutron_router(
+                self.fake_router_with_ext_gw))
 
         payload = self._create_payload_for_router_update(
             self.get_router.return_value, self.fake_router_with_ext_gw)
@@ -1023,6 +1026,9 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase):
             'ext-subnet-id': self.fake_ext_subnet}.get(sid, self.fake_subnet)
         self.get_port.return_value = self.fake_ext_gw_port
         grps.return_value = self.fake_router_ports
+        self.l3_inst._nb_ovn.get_lrouter.return_value = (
+            fake_resources.FakeOVNRouter.from_neutron_router(
+                self.fake_router_with_ext_gw))
 
         payload = self._create_payload_for_router_update(
             self.fake_router_with_ext_gw, ur.return_value)