Remove LB from LS belonging to provider networks

In core OVN, LBs on switches with localnet ports (i.e., neutron
provider networks) don't work if traffic comes from localnet [1]

In order to force NAT to happen at the virtual router instead
of the LS level, when the VIP of the LoadBalancer is associated
to a provider network we should avoid adding the LB to the
LS associated to the provider network

[1] https://bugzilla.redhat.com/show_bug.cgi?id=2164652

Closes-Bug: #2003997

Change-Id: I009ddd2604d208bbf793e2d19d4195b77726f7b2
(cherry picked from commit d10feb642f)

ovn_octavia_provider/helper.py

@@ -18,6 +18,7 @@ import re
 import threading
 
 import netaddr
+from neutron_lib.api.definitions import provider_net
 from neutron_lib import constants as n_const
 from neutronclient.common import exceptions as n_exc
 from octavia_lib.api.drivers import data_models as o_datamodels
@@ -990,11 +991,17 @@ class OvnProviderHelper():
                 loadbalancer[constants.ID],
                 protocol=protocol)
             ovn_lb = ovn_lb if protocol else ovn_lb[0]
-            # NOTE(froyo): This is the association of the lb to the VIP ls
+
-            # so this is executed right away
+            # NOTE(ltomasbo): If the VIP is on a provider network, it does
-            self._update_lb_to_ls_association(
+            # not need to be associated to its LS
-                ovn_lb, network_id=port['network_id'],
+            network = neutron_client.show_network(
-                associate=True, update_ls_ref=True)
+                port['network_id'])['network']
+            if not network.get(provider_net.PHYSICAL_NETWORK, False):
+                # NOTE(froyo): This is the association of the lb to the VIP ls
+                # so this is executed right away
+                self._update_lb_to_ls_association(
+                    ovn_lb, network_id=port['network_id'],
+                    associate=True, update_ls_ref=True)
             ls_name = utils.ovn_name(port['network_id'])
             ovn_ls = self.ovn_nbdb_api.ls_get(ls_name).execute(
                 check_error=True)

ovn_octavia_provider/tests/functional/base.py

@@ -57,6 +57,7 @@ class TestOvnOctaviaBase(base.TestOVNFunctionalBase,
         self.fake_neutron_client = mock.MagicMock()
         clients.get_neutron_client = mock.MagicMock()
         clients.get_neutron_client.return_value = self.fake_neutron_client
+        self.fake_neutron_client.show_network = self._mock_show_network
         self.fake_neutron_client.show_subnet = self._mock_show_subnet
         self.fake_neutron_client.list_ports = self._mock_list_ports
         self.fake_neutron_client.show_port = self._mock_show_port
@@ -66,6 +67,12 @@ class TestOvnOctaviaBase(base.TestOVNFunctionalBase,
         self._local_port_cache = {'ports': []}
         self.core_plugin = directory.get_plugin()
 
+    def _mock_show_network(self, network_id):
+        network = {}
+        network['id'] = network_id
+        network['provider:physical_network'] = None
+        return {'network': network}
+
     def _mock_show_subnet(self, subnet_id):
         subnet = {}
         subnet['network_id'] = self._local_net_cache[subnet_id]

ovn_octavia_provider/tests/unit/test_helper.py

@@ -14,6 +14,7 @@
 import copy
 from unittest import mock
 
+from neutron_lib.api.definitions import provider_net
 from neutron_lib import constants as n_const
 from neutronclient.common import exceptions as n_exc
 from octavia_lib.api.drivers import data_models
@@ -641,7 +642,7 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
             selection_fields=['ip_src', 'ip_dst', 'tp_src', 'tp_dst'])
 
     @mock.patch('ovn_octavia_provider.common.clients.get_neutron_client')
-    def _test_lb_create_on_multi_protocol(self, protocol, net_cli):
+    def _test_lb_create_on_multi_protocol(self, protocol, provider, net_cli):
         """This test situation when new protocol is added
 
            to the same loadbalancer and we need to add
@@ -652,6 +653,11 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
         self.lb[ovn_const.LB_EXT_IDS_LR_REF_KEY] = 'foo'
         self.lb[ovn_const.LB_EXT_IDS_LS_REFS_KEY] = '{\"neutron-foo\": 1}'
         net_cli.return_value.list_ports.return_value = self.ports
+        fake_network = {'id': self.lb['vip_network_id'],
+                        provider_net.PHYSICAL_NETWORK: provider}
+        net_cli.return_value.show_network.return_value = {
+            'network': fake_network}
+
         status = self.helper.lb_create(self.lb, protocol=protocol)
         self.assertEqual(status['loadbalancers'][0]['provisioning_status'],
                          constants.ACTIVE)
@@ -666,18 +672,26 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
             name=mock.ANY,
             protocol=protocol.lower(),
             selection_fields=['ip_src', 'ip_dst', 'tp_src', 'tp_dst'])
-        self.helper._update_lb_to_ls_association.assert_has_calls([
+        if provider:
-            mock.call(self.ovn_lb, associate=True,
+            self.helper._update_lb_to_ls_association.assert_not_called()
-                      network_id=self.lb['vip_network_id'],
+        else:
-                      update_ls_ref=True),
+            self.helper._update_lb_to_ls_association.assert_has_calls([
-            mock.call(self.ovn_lb, associate=True, network_id='foo',
+                mock.call(self.ovn_lb, associate=True,
-                      update_ls_ref=True)])
+                          network_id=self.lb['vip_network_id'],
+                          update_ls_ref=True),
+                mock.call(self.ovn_lb, associate=True, network_id='foo',
+                          update_ls_ref=True)])
 
     def test_lb_create_on_multi_protocol_UDP(self):
-        self._test_lb_create_on_multi_protocol('UDP')
+        self._test_lb_create_on_multi_protocol('UDP', None)
 
     def test_lb_create_on_multi_protocol_SCTP(self):
-        self._test_lb_create_on_multi_protocol('SCTP')
+        self._test_lb_create_on_multi_protocol('SCTP', None)
+
+    def _test_lb_create_on_provider_network(self):
+        # Test case for LB created on provider network.
+        # Ensure LB is not associated to the LS in that case
+        self._test_lb_create_on_multi_protocol('TCP', "provider")
 
     @mock.patch('ovn_octavia_provider.common.clients.get_neutron_client')
     def test_lb_create_neutron_client_exception(self, net_cli):