nb driver: Don't expose FIP if the external_mac is not set

If the external_mac in the NAT entry is not set then it means two
things:

1) The DVR is disabled (see [2])

2) The external_mac has not been set yet because Neutron doesn't
populate the external_mac column of the FIP NAT entry until
the associated port with the FIP is bound to a chassis. This is
because of LB VIPs that don't get bound [1].

For case 1) we need to expose the FIP on the gateway node where the LRP
is hosted. For case 2) we can ignore if the external_mac is not set but
then we need another event that would re-act on setting the
external_mac. This will be done in a followup patch.

[1] https://bugs.launchpad.net/networking-ovn/+bug/1789686
[2] https://bugs.launchpad.net/ovn-bgp-agent/+bug/2056477

Related-Bug: #2073403

Change-Id: Ie24f1f370c44f95840af2d4d2010c20655738ebc
Signed-off-by: Jakub Libosvar <libosvar@redhat.com>

ovn_bgp_agent/drivers/openstack/nb_exceptions.py

@@ -0,0 +1,17 @@
+# Copyright 2024 Red Hat, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+
+class NATNotFound(Exception):
+    pass

ovn_bgp_agent/drivers/openstack/nb_ovn_bgp_driver.py

@@ -22,6 +22,7 @@ from oslo_log import log as logging
 
 from ovn_bgp_agent import constants
 from ovn_bgp_agent.drivers import driver_api
+from ovn_bgp_agent.drivers.openstack import nb_exceptions
 from ovn_bgp_agent.drivers.openstack.utils import bgp as bgp_utils
 from ovn_bgp_agent.drivers.openstack.utils import driver_utils
 from ovn_bgp_agent.drivers.openstack.utils import ovn
@@ -231,9 +232,12 @@ class NBOVNBGPDriver(driver_api.AgentDriverBase):
     def _ensure_lsp_exposed(self, port):
         port_fip = port.external_ids.get(constants.OVN_FIP_EXT_ID_KEY)
         if port_fip:
-            external_ip, external_mac, ls_name = (
-                self.get_port_external_ip_and_ls(port.name))
-            if not external_ip or not ls_name:
+            try:
+                external_ip, external_mac, ls_name = (
+                    self.get_port_external_ip_and_ls(port.name))
+            except nb_exceptions.NATNotFound as e:
+                LOG.debug("Logical Switch Port %s does not have all data "
+                          "required in its NAT entry: %s", port.name, e)
                 return
             return self._expose_fip(external_ip, external_mac, ls_name, port)
 
@@ -551,14 +555,26 @@ class NBOVNBGPDriver(driver_api.AgentDriverBase):
 
     def get_port_external_ip_and_ls(self, port):
         nat_entry = self.nb_idl.get_nat_by_logical_port(port)
-        if not nat_entry:
-            return None, None, None
-        net_id = nat_entry.external_ids.get(constants.OVN_FIP_NET_EXT_ID_KEY)
-        if not net_id:
-            return nat_entry.external_ip, nat_entry.external_mac[0], None
-        else:
-            ls_name = "neutron-{}".format(net_id)
-            return nat_entry.external_ip, nat_entry.external_mac[0], ls_name
+        try:
+            net_id = nat_entry.external_ids[constants.OVN_FIP_NET_EXT_ID_KEY]
+        except AttributeError:
+            # nat_entry is None and has no external_ids attribute
+            raise nb_exceptions.NATNotFound(
+                "NAT entry for port %s not found" % port)
+        except KeyError:
+            # network ID was not found in the NAT entry
+            raise nb_exceptions.NATNotFound(
+                "NAT entry for port %s does not contain network ID in its "
+                "external_ids" % port)
+        try:
+            external_mac = nat_entry.external_mac[0]
+        except IndexError:
+            raise nb_exceptions.NATNotFound(
+                "NAT entry for port %s does not have external_mac set" % (
+                    port))
+
+        ls_name = "neutron-{}".format(net_id)
+        return nat_entry.external_ip, external_mac, ls_name
 
     @lockutils.synchronized('nbbgp')
     def expose_fip(self, ip, mac, logical_switch, row):
@@ -1052,11 +1068,12 @@ class NBOVNBGPDriver(driver_api.AgentDriverBase):
             LOG.debug("Something went wrong, VIP port %s not found", vip_port)
             return
 
-        external_ip, external_mac, ls_name = (
-            self.get_port_external_ip_and_ls(vip_lsp.name))
-        if not external_ip or not ls_name:
-            LOG.debug("Something went wrong, no NAT entry for the VIP %s",
-                      vip_port)
+        try:
+            external_ip, external_mac, ls_name = (
+                self.get_port_external_ip_and_ls(vip_lsp.name))
+        except nb_exceptions.NATNotFound as e:
+            LOG.debug("Something went wrong with the VIP %s: %s",
+                      vip_port, e)
             return
         self._expose_fip(external_ip, external_mac, ls_name, vip_lsp)
 

ovn_bgp_agent/drivers/openstack/watchers/nb_bgp_watcher.py

@@ -16,6 +16,7 @@ from oslo_concurrency import lockutils
 from oslo_log import log as logging
 
 from ovn_bgp_agent import constants
+from ovn_bgp_agent.drivers.openstack import nb_exceptions
 from ovn_bgp_agent.drivers.openstack.utils import common as common_utils
 from ovn_bgp_agent.drivers.openstack.utils import driver_utils
 from ovn_bgp_agent.drivers.openstack.utils import loadbalancer as lb_utils
@@ -235,9 +236,12 @@ class LogicalSwitchPortFIPCreateEvent(base_watcher.LSPChassisEvent):
             return False
 
     def _run(self, event, row, old):
-        external_ip, external_mac, ls_name = (
-            self.agent.get_port_external_ip_and_ls(row.name))
-        if not external_ip or not ls_name:
+        try:
+            external_ip, external_mac, ls_name = (
+                self.agent.get_port_external_ip_and_ls(row.name))
+        except nb_exceptions.NATNotFound as e:
+            LOG.debug("Logical Switch Port %s does not have all data required"
+                      " in its NAT entry: %s", row.name, e)
             return
 
         with _SYNC_STATE_LOCK.read_lock():

ovn_bgp_agent/tests/unit/drivers/openstack/test_nb_ovn_bgp_driver.py

@@ -18,6 +18,7 @@ from unittest import mock
 from oslo_config import cfg
 
 from ovn_bgp_agent import constants
+from ovn_bgp_agent.drivers.openstack import nb_exceptions
 from ovn_bgp_agent.drivers.openstack import nb_ovn_bgp_driver
 from ovn_bgp_agent.drivers.openstack.utils import bgp as bgp_utils
 from ovn_bgp_agent.drivers.openstack.utils import driver_utils
@@ -740,9 +741,9 @@ class TestNBOVNBGPDriver(test_base.TestCase):
     def test_get_port_external_ip_and_ls_no_nat_entry(self):
         self.nb_idl.get_nat_by_logical_port.return_value = None
 
-        ret = self.nb_bgp_driver.get_port_external_ip_and_ls('fake-port')
-
-        self.assertEqual(ret, (None, None, None))
+        self.assertRaises(
+            nb_exceptions.NATNotFound,
+            self.nb_bgp_driver.get_port_external_ip_and_ls, 'fake-port')
 
     def test_get_port_external_ip_and_ls_no_external_id(self):
         nat_entry = fakes.create_object({
@@ -751,10 +752,9 @@ class TestNBOVNBGPDriver(test_base.TestCase):
             'external_mac': ['fake-mac']})
         self.nb_idl.get_nat_by_logical_port.return_value = nat_entry
 
-        ret = self.nb_bgp_driver.get_port_external_ip_and_ls('fake-port')
-
-        self.assertEqual(
-            ret, (nat_entry.external_ip, nat_entry.external_mac[0], None))
+        self.assertRaises(
+            nb_exceptions.NATNotFound,
+            self.nb_bgp_driver.get_port_external_ip_and_ls, 'fake-port')
 
     def test_expose_fip(self):
         ip = '10.0.0.1'
@@ -1706,9 +1706,9 @@ class TestNBOVNBGPDriver(test_base.TestCase):
                 constants.OVN_LS_NAME_EXT_ID_KEY: 'net2'
             })
         self.nb_idl.lsp_get.return_value.execute.return_value = vip_lsp
-        mock_get_port_external_ip_and_ls = mock.patch.object(
-            self.nb_bgp_driver, 'get_port_external_ip_and_ls').start()
-        mock_get_port_external_ip_and_ls.return_value = (None, None, None)
+        mock.patch.object(
+            self.nb_bgp_driver, 'get_port_external_ip_and_ls',
+            side_effect=nb_exceptions.NATNotFound).start()
         mock_expose_fip = mock.patch.object(
             self.nb_bgp_driver, '_expose_fip').start()
 

ovn_bgp_agent/tests/unit/drivers/openstack/watchers/test_nb_bgp_watcher.py

@@ -16,6 +16,7 @@
 from unittest import mock
 
 from ovn_bgp_agent import constants
+from ovn_bgp_agent.drivers.openstack import nb_exceptions
 from ovn_bgp_agent.drivers.openstack.watchers import nb_bgp_watcher
 from ovn_bgp_agent.tests import base as test_base
 from ovn_bgp_agent.tests import utils
@@ -420,15 +421,13 @@ class TestLogicalSwitchPortFIPCreateEvent(test_base.TestCase):
                                                       ls_name, row)
 
     def test_run_no_external_ip(self):
-        external_ip = None
-        ls_name = 'logical_switch'
-        self.agent.get_port_external_ip_and_ls.return_value = (external_ip,
-                                                               'mac',
-                                                               ls_name)
+        self.agent.get_port_external_ip_and_ls.side_effect = (
+            nb_exceptions.NATNotFound)
+
         row = utils.create_row(type=constants.OVN_VM_VIF_PORT_TYPE,
                                addresses=['mac 192.168.0.1'],
                                name='net-id')
-        self.event.run(mock.Mock(), row, mock.Mock())
+        self.event.run(mock.ANY, row, mock.ANY)
         self.agent.expose_fip.assert_not_called()