Fix check for a CR-LRP as a gateway port

As per [1], the gateway neutron-ovn-invalid-chassis previously
used for the CR-LRP gateway_chassis has been removed. However,
it is necessary to determine whether a Logical Router Port (LRP)
is a gateway port or not to appropriately assign the Load
Balancer (LB) to the Logical Switch (LS) associated with tenant
and provider networks.

This patch modifies the logic within the LogicalRouterPortEvent
to examine the external_ids and determine if the port is a
gateway port or not, regardless of whether gateway_chassis is
included or not.

[1] https://review.opendev.org/c/openstack/neutron/+/909305

Closes-Bug: #2056537

Change-Id: I05bc97362e45a0239cf9206ba8539fcfb10a1151

ovn_octavia_provider/common/constants.py

@@ -33,6 +33,7 @@ OVN_FIP_PORT_EXT_ID_KEY = 'neutron:fip_port_id'
 OVN_GW_PORT_EXT_ID_KEY = 'neutron:gw_port_id'
 OVN_PORT_CIDR_EXT_ID_KEY = 'neutron:cidrs'
 OVN_MEMBER_STATUS_KEY = 'neutron:member_status'
+OVN_ROUTER_IS_EXT_GW = 'neutron:is_ext_gw'
 
 # TODO(froyo): Use from neutron-lib once released.
 OVN_LB_HM_PORT_DISTRIBUTED = 'ovn-lb-hm:distributed'

ovn_octavia_provider/helper.py

@@ -27,6 +27,7 @@ import openstack
 from oslo_config import cfg
 from oslo_log import log as logging
 from oslo_serialization import jsonutils
+from oslo_utils import strutils
 from ovs.stream import Stream
 from ovsdbapp.backend.ovs_idl import idlutils
 from ovsdbapp.schema.ovn_northbound import commands as cmd
@@ -289,7 +290,9 @@ class OvnProviderHelper():
             return
         request_info = {'network': network,
                         'router': router,
-                        'gateway_chassis': row.gateway_chassis}
+                        'is_gw_port': strutils.bool_from_string(
+                            row.external_ids.get(
+                                ovn_const.OVN_ROUTER_IS_EXT_GW))}
         self.add_request({'type': ovn_const.REQ_TYPE_LB_CREATE_LRP_ASSOC,
                           'info': request_info})
 
@@ -306,13 +309,12 @@ class OvnProviderHelper():
                             lb.uuid, info['router'].uuid)
                 self._update_lb_to_lr_association_by_step(lb, info['router'])
 
-        # if gateway_chassis ports, there is no need to re-add the
+        # if lrp port is a gw port, there is no need to re-add the
         # loadbalancers from the router into the provider network.
         # This will be already done for loadbalancer created with VIPs on
-        # provider networks, regardless of the gateway_chassis lrp port
+        # provider networks. And it should never be True there when the VIPs
-        # existence. And it should never be there when the VIPs are on
+        # are on tenant networks.
-        # tenant networks.
+        if info['is_gw_port']:
-        if info['gateway_chassis']:
             return
 
         # Add those lb to the network which are unique to the router

ovn_octavia_provider/tests/functional/test_agent.py

@@ -18,7 +18,6 @@ import multiprocessing as mp
 
 from neutron.common import utils as n_utils
 from oslo_utils import uuidutils
-import testtools
 
 from ovn_octavia_provider import agent as ovn_agent
 from ovn_octavia_provider.common import config as ovn_config
@@ -153,11 +152,6 @@ class TestOvnOctaviaProviderAgent(ovn_base.TestOvnOctaviaBase):
     def test_lrp_event_handler_with_loadbalancer_cascade_delete(self):
         self._test_lrp_event_handler(cascade=True)
 
-    # NOTE(froyo): Test skipped by cross depedency among this patch with this
-    # https://review.opendev.org/c/openstack/ovn-octavia-provider/+/911701.
-    # The mentioned one will enable again this test.
-    @testtools.skip(
-        'https://review.opendev.org/c/openstack/ovn-octavia-provider/+/911701')
     def test_lrp_event_handler_lrp_with_external_gateway(self):
         # Create Network N1 on router R1 and LBA on N1
         network_N1 = self._create_net('N' + uuidutils.generate_uuid()[:4])
@@ -191,11 +185,6 @@ class TestOvnOctaviaProviderAgent(ovn_base.TestOvnOctaviaBase):
                 ovn_const.LR_REF_KEY_HEADER + provider_net['network']['id']),
             timeout=10)
 
-    # NOTE(froyo): Test skipped by cross depedency among this patch with this
-    # https://review.opendev.org/c/openstack/ovn-octavia-provider/+/911701.
-    # The mentioned one will enable again this test.
-    @testtools.skip(
-        'https://review.opendev.org/c/openstack/ovn-octavia-provider/+/911701')
     def test_fip_on_lb_vip(self):
         """This test checks if FIP on LB VIP is configured.
 
@@ -272,11 +261,6 @@ class TestOvnOctaviaProviderAgent(ovn_base.TestOvnOctaviaBase):
                 # Make sure that LB1 is not added to provider net - e1 LS
                 self.assertListEqual([], ls.load_balancer)
 
-    # NOTE(froyo): Test skipped by cross depedency among this patch with this
-    # https://review.opendev.org/c/openstack/ovn-octavia-provider/+/911701.
-    # The mentioned one will enable again this test.
-    @testtools.skip(
-        'https://review.opendev.org/c/openstack/ovn-octavia-provider/+/911701')
     def test_fip_on_lb_additional_vip(self):
         """This test checks if FIP on LB additional VIP is configured.
 

ovn_octavia_provider/tests/unit/test_helper.py

@@ -2196,13 +2196,13 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
         self.router_port_event = ovn_event.LogicalRouterPortEvent(
             self.helper)
         row = fakes.FakeOvsdbRow.create_one_ovsdb_row(
-            attrs={'gateway_chassis': []})
+            attrs={'external_ids': {ovn_const.OVN_ROUTER_IS_EXT_GW: 'False'}})
         self.router_port_event.run('create', row, mock.ANY)
         expected = {
             'info':
                 {'router': self.router,
                  'network': self.network,
-                 'gateway_chassis': []},
+                 'is_gw_port': False},
             'type': 'lb_create_lrp_assoc'}
         self.mock_add_request.assert_called_once_with(expected)
 
@@ -2211,7 +2211,7 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
         self.router_port_event = ovn_event.LogicalRouterPortEvent(
             self.helper)
         row = fakes.FakeOvsdbRow.create_one_ovsdb_row(
-            attrs={'gateway_chassis': []})
+            attrs={})
         self.router_port_event.run('delete', row, mock.ANY)
         expected = {
             'info':
@@ -2225,13 +2225,13 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
         self.router_port_event = ovn_event.LogicalRouterPortEvent(
             self.helper)
         row = fakes.FakeOvsdbRow.create_one_ovsdb_row(
-            attrs={'gateway_chassis': ['temp-gateway-chassis']})
+            attrs={'external_ids': {ovn_const.OVN_ROUTER_IS_EXT_GW: 'True'}})
         self.router_port_event.run(mock.ANY, row, mock.ANY)
         expected = {
             'info':
                 {'router': self.router,
                  'network': self.network,
-                 'gateway_chassis': ['temp-gateway-chassis']},
+                 'is_gw_port': True},
             'type': 'lb_create_lrp_assoc'}
         self.mock_add_request.assert_called_once_with(expected)
 
@@ -2404,13 +2404,13 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
 
     def test_lb_create_lrp_assoc_handler(self):
         lrp = fakes.FakeOvsdbRow.create_one_ovsdb_row(
-            attrs={'gateway_chassis': []})
+            attrs={'external_ids': {ovn_const.OVN_ROUTER_IS_EXT_GW: 'False'}})
         self.helper.lb_create_lrp_assoc_handler(lrp)
         expected = {
             'info':
                 {'router': self.router,
                  'network': self.network,
-                 'gateway_chassis': []},
+                 'is_gw_port': False},
             'type': 'lb_create_lrp_assoc'}
         self.mock_add_request.assert_called_once_with(expected)
 
@@ -2429,7 +2429,7 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
         info = {
             'network': self.network,
             'router': self.router,
-            'gateway_chassis': [],
+            'is_gw_port': False,
         }
         self.helper.lb_create_lrp_assoc(info)
         self.helper._update_lb_to_lr_association.assert_called_once_with(
@@ -2440,7 +2440,7 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
         info = {
             'network': self.network,
             'router': self.router,
-            'gateway_chassis': [],
+            'is_gw_port': False,
         }
         self.helper._update_lb_to_ls_association.side_effect = [
             idlutils.RowNotFound]
@@ -2458,7 +2458,7 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
         info = {
             'network': self.network,
             'router': self.router,
-            'gateway_chassis': 'fake-chassis',
+            'is_gw_port': True,
         }
         self.helper._update_lb_to_lr_association.side_effect = [
             idlutils.RowNotFound]
@@ -2475,7 +2475,7 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
         info = {
             'network': self.network,
             'router': self.router,
-            'gateway_chassis': 'fake-chassis',
+            'is_gw_port': True,
         }
         # Make it already uniq.
         self.network.load_balancer = self.router.load_balancer