Remove duplicated _get_network method

The method was tied to an instance of some events but didn't actually
modify state of the instance. It was replaced with a generic helper
function.

Change-Id: I0d24ead8e44cde1df62df77408801cbbc758c4a6
Signed-off-by: Jakub Libosvar <libosvar@redhat.com>
(cherry picked from commit acb53b40099b3a1c3d730ebbd898b0febe52070d)

ovn_bgp_agent/drivers/openstack/utils/common.py

@@ -0,0 +1,20 @@
+# 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.
+
+
+def get_from_external_ids(row, key):
+    try:
+        return row.external_ids[key]
+    except (AttributeError, KeyError):
+        pass

ovn_bgp_agent/drivers/openstack/watchers/base_watcher.py

@@ -16,6 +16,7 @@ from oslo_log import log as logging
 from ovsdbapp.backend.ovs_idl import event as row_event
 
 from ovn_bgp_agent import constants
+from ovn_bgp_agent.drivers.openstack.utils import common as common_utils
 from ovn_bgp_agent.drivers.openstack.utils import driver_utils
 
 
@@ -109,19 +110,14 @@ class LSPChassisEvent(Event):
 
         return False
 
-    def _get_network(self, row):
-        try:
-            return row.external_ids[constants.OVN_LS_NAME_EXT_ID_KEY]
-        except (AttributeError, KeyError):
-            return
-
     def _get_ips_info(self, row):
         return {
             'mac': row.addresses[0].strip().split(' ')[0],
             'cidrs': row.external_ids.get(constants.OVN_CIDRS_EXT_ID_KEY,
                                           "").split(),
             'type': row.type,
-            'logical_switch': self._get_network(row),
+            'logical_switch': common_utils.get_from_external_ids(
+                row, constants.OVN_LS_NAME_EXT_ID_KEY),
         }
 
     def _get_port_fip(self, row):
@@ -135,18 +131,13 @@ class LRPChassisEvent(Event):
         super().__init__(bgp_agent, events, table)
         self.event_name = self.__class__.__name__
 
-    def _get_network(self, row):
-        try:
-            return row.external_ids[constants.OVN_LS_NAME_EXT_ID_KEY]
-        except (AttributeError, KeyError):
-            return
-
     def _get_ips_info(self, row):
         return {
             'mac': row.mac,
             'cidrs': row.networks,
             'type': constants.OVN_CR_LRP_PORT_TYPE,
-            'logical_switch': self._get_network(row),
+            'logical_switch': common_utils.get_from_external_ids(
+                row, constants.OVN_LS_NAME_EXT_ID_KEY),
             'router': row.external_ids.get(constants.OVN_LR_NAME_EXT_ID_KEY),
         }
 

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.utils import common as common_utils
 from ovn_bgp_agent.drivers.openstack.utils import driver_utils
 from ovn_bgp_agent.drivers.openstack.utils import port as port_utils
 from ovn_bgp_agent.drivers.openstack.watchers import base_watcher
@@ -54,7 +55,8 @@ class LogicalSwitchPortProviderCreateEvent(base_watcher.LSPChassisEvent):
                 return False
 
             current_chassis = self._get_chassis(row)
-            logical_switch = self._get_network(row)
+            logical_switch = common_utils.get_from_external_ids(
+                row, constants.OVN_LS_NAME_EXT_ID_KEY)
 
             if logical_switch in self.agent.ovn_local_lrps:
                 # This is a tenant network, routed through lrp, handled by
@@ -105,7 +107,8 @@ class LogicalSwitchPortProviderDeleteEvent(base_watcher.LSPChassisEvent):
                 return False
 
             ips = row.addresses[0].split(' ')[1:]
-            logical_switch = self._get_network(row)
+            logical_switch = common_utils.get_from_external_ids(
+                row, constants.OVN_LS_NAME_EXT_ID_KEY)
 
             if logical_switch in self.agent.ovn_local_lrps:
                 # This is a tenant network, routed through lrp, handled by
@@ -209,8 +212,10 @@ class LogicalSwitchPortFIPCreateEvent(base_watcher.LSPChassisEvent):
                 return False
 
             # Check if the current port_fip has not been exposed yet
-            return not self.agent.is_ip_exposed(self._get_network(row),
+            return not self.agent.is_ip_exposed(
-                                                current_port_fip)
+                common_utils.get_from_external_ids(
+                    row, constants.OVN_LS_NAME_EXT_ID_KEY),
+                current_port_fip)
 
         except (IndexError, AttributeError):
             return False
@@ -268,7 +273,8 @@ class LogicalSwitchPortFIPDeleteEvent(base_watcher.LSPChassisEvent):
                 # This port is not a floating ip update
                 return False
 
-            logical_switch = self._get_network(row)
+            logical_switch = common_utils.get_from_external_ids(
+                row, constants.OVN_LS_NAME_EXT_ID_KEY)
             is_exposed = self.agent.is_ip_exposed(logical_switch,
                                                   old_port_fip or
                                                   current_port_fip)
@@ -479,7 +485,8 @@ class LogicalSwitchPortSubnetAttachEvent(base_watcher.LSPChassisEvent):
             subnet_info = {
                 'associated_router': row.external_ids.get(
                     constants.OVN_DEVICE_ID_EXT_ID_KEY),
-                'network': self._get_network(row),
+                'network': common_utils.get_from_external_ids(
+                    row, constants.OVN_LS_NAME_EXT_ID_KEY),
                 'address_scopes': driver_utils.get_addr_scopes(row)}
             self.agent.expose_subnet(ips, subnet_info)
 
@@ -554,7 +561,8 @@ class LogicalSwitchPortSubnetDetachEvent(base_watcher.LSPChassisEvent):
                 subnet_info = {
                     'associated_router': row.external_ids.get(
                         constants.OVN_DEVICE_ID_EXT_ID_KEY),
-                    'network': self._get_network(row),
+                    'network': common_utils.get_from_external_ids(
+                        row, constants.OVN_LS_NAME_EXT_ID_KEY),
                     'address_scopes': driver_utils.get_addr_scopes(row)}
             else:
                 associated_router = row.external_ids.get(
@@ -566,7 +574,8 @@ class LogicalSwitchPortSubnetDetachEvent(base_watcher.LSPChassisEvent):
                         associated_router = previous_associated_router
                 subnet_info = {
                     'associated_router': associated_router,
-                    'network': self._get_network(row),
+                    'network': common_utils.get_from_external_ids(
+                        row, constants.OVN_LS_NAME_EXT_ID_KEY),
                     'address_scopes': driver_utils.get_addr_scopes(row)}
             self.agent.withdraw_subnet(ips, subnet_info)
 
@@ -586,7 +595,8 @@ class LogicalSwitchPortTenantCreateEvent(base_watcher.LSPChassisEvent):
             if not bool(row.up[0]):
                 return False
 
-            current_network = self._get_network(row)
+            current_network = common_utils.get_from_external_ids(
+                row, constants.OVN_LS_NAME_EXT_ID_KEY)
             if current_network not in self.agent.ovn_local_lrps:
                 return False
 
@@ -595,7 +605,8 @@ class LogicalSwitchPortTenantCreateEvent(base_watcher.LSPChassisEvent):
                     return True
 
             if hasattr(old, 'external_ids'):
-                old_network = self._get_network(old)
+                old_network = common_utils.get_from_external_ids(
+                    old, constants.OVN_LS_NAME_EXT_ID_KEY)
                 if old_network != current_network:
                     return True
         except (IndexError, AttributeError):
@@ -614,7 +625,8 @@ class LogicalSwitchPortTenantCreateEvent(base_watcher.LSPChassisEvent):
                 'cidrs': row.external_ids.get(constants.OVN_CIDRS_EXT_ID_KEY,
                                               "").split(),
                 'type': row.type,
-                'logical_switch': self._get_network(row)
+                'logical_switch': common_utils.get_from_external_ids(
+                    row, constants.OVN_LS_NAME_EXT_ID_KEY),
             }
             self.agent.expose_remote_ip(ips, ips_info)
 
@@ -631,7 +643,8 @@ class LogicalSwitchPortTenantDeleteEvent(base_watcher.LSPChassisEvent):
             if not port_utils.has_ip_address_defined(row.addresses[0]):
                 return False
 
-            current_network = self._get_network(row)
+            current_network = common_utils.get_from_external_ids(
+                row, constants.OVN_LS_NAME_EXT_ID_KEY)
             # Assuming the current_network cannot be changed at once
             if current_network not in self.agent.ovn_local_lrps:
                 return False
@@ -658,7 +671,8 @@ class LogicalSwitchPortTenantDeleteEvent(base_watcher.LSPChassisEvent):
                 'cidrs': row.external_ids.get(constants.OVN_CIDRS_EXT_ID_KEY,
                                               "").split(),
                 'type': row.type,
-                'logical_switch': self._get_network(row)
+                'logical_switch': common_utils.get_from_external_ids(
+                    row, constants.OVN_LS_NAME_EXT_ID_KEY),
             }
             self.agent.withdraw_remote_ip(ips, ips_info)
 

ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_common.py

@@ -0,0 +1,38 @@
+# Copyright 2024 Red Hat, Inc.
+# All Rights Reserved.
+#
+#    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.
+
+from ovn_bgp_agent.drivers.openstack.utils import common
+from ovn_bgp_agent.tests import base as test_base
+from ovn_bgp_agent.tests import utils as test_utils
+
+
+class TestGetFromExternalIds(test_base.TestCase):
+    def test_all_present(self):
+        key = 'foo'
+        value = 'bar'
+        row = test_utils.create_row(external_ids={key: value})
+
+        observed_value = common.get_from_external_ids(row, key)
+        self.assertEqual(value, observed_value)
+
+    def test_external_ids_missing(self):
+        row = test_utils.create_row()
+
+        self.assertIsNone(common.get_from_external_ids(row, 'key'))
+
+    def test_key_missing(self):
+        row = test_utils.create_row(external_ids={})
+
+        self.assertIsNone(common.get_from_external_ids(row, 'key'))

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

@@ -106,33 +106,6 @@ class TestLSPChassisEvent(test_base.TestCase):
             options={constants.OVN_REQUESTED_CHASSIS: 'host1'})
         self.assertFalse(self.lsp_event._has_additional_binding(row))
 
-    def test__get_network(self):
-        row = utils.create_row(
-            external_ids={constants.OVN_LS_NAME_EXT_ID_KEY: 'test-net'})
-        self.assertEqual('test-net', self.lsp_event._get_network(row))
-        row = utils.create_row(external_ids={})
-        self.assertEqual(None, self.lsp_event._get_network(row))
-
-
-class FakeLRPChassisEvent(base_watcher.LRPChassisEvent):
-    def run(self):
-        pass
-
-
-class TestLRPChassisEvent(test_base.TestCase):
-
-    def setUp(self):
-        super(TestLRPChassisEvent, self).setUp()
-        self.lrp_event = FakeLRPChassisEvent(
-            mock.Mock(), [mock.Mock()])
-
-    def test__get_network(self):
-        row = utils.create_row(
-            external_ids={constants.OVN_LS_NAME_EXT_ID_KEY: 'test-net'})
-        self.assertEqual('test-net', self.lrp_event._get_network(row))
-        row = utils.create_row(external_ids={})
-        self.assertEqual(None, self.lrp_event._get_network(row))
-
 
 class TestChassisCreateEvent(test_base.TestCase):
     _event = base_watcher.ChassisCreateEvent