diff --git a/ovn_bgp_agent/drivers/openstack/utils/router.py b/ovn_bgp_agent/drivers/openstack/utils/router.py new file mode 100644 index 00000000..8ba9965c --- /dev/null +++ b/ovn_bgp_agent/drivers/openstack/utils/router.py @@ -0,0 +1,25 @@ +# 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. + +from ovn_bgp_agent import constants +from ovn_bgp_agent.drivers.openstack.utils import common as common_utils + + +def get_name_from_external_ids(row, key=constants.OVN_LB_LR_REF_EXT_ID_KEY): + router_name = common_utils.get_from_external_ids(row, key) + + try: + return router_name.replace('neutron-', "", 1) + except AttributeError: + pass diff --git a/ovn_bgp_agent/drivers/openstack/watchers/base_watcher.py b/ovn_bgp_agent/drivers/openstack/watchers/base_watcher.py index 5c209caf..b8ab5874 100644 --- a/ovn_bgp_agent/drivers/openstack/watchers/base_watcher.py +++ b/ovn_bgp_agent/drivers/openstack/watchers/base_watcher.py @@ -49,12 +49,6 @@ class OVNLBEvent(Event): super().__init__(bgp_agent, events, table) self.event_name = self.__class__.__name__ - def _get_router(self, row, key=constants.OVN_LB_LR_REF_EXT_ID_KEY): - try: - return row.external_ids[key].replace('neutron-', "", 1) - except (AttributeError, KeyError): - return - def _get_ip_from_vips(self, row): return [driver_utils.remove_port_from_ip(ipport) for ipport in getattr(row, 'vips', {}).keys()] diff --git a/ovn_bgp_agent/drivers/openstack/watchers/nb_bgp_watcher.py b/ovn_bgp_agent/drivers/openstack/watchers/nb_bgp_watcher.py index 3d057edd..7460390e 100644 --- a/ovn_bgp_agent/drivers/openstack/watchers/nb_bgp_watcher.py +++ b/ovn_bgp_agent/drivers/openstack/watchers/nb_bgp_watcher.py @@ -19,6 +19,7 @@ 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.utils import router as router_utils from ovn_bgp_agent.drivers.openstack.watchers import base_watcher @@ -690,7 +691,7 @@ class OVNLBCreateEvent(base_watcher.OVNLBEvent): try: if not row.vips: return False - lb_router = self._get_router(row) + lb_router = router_utils.get_name_from_external_ids(row) if lb_router not in self.agent.ovn_local_cr_lrps.keys(): return False @@ -706,7 +707,7 @@ class OVNLBCreateEvent(base_watcher.OVNLBEvent): if hasattr(old, 'external_ids'): # Check if the lb_router was added - old_lb_router = self._get_router(old) + old_lb_router = router_utils.get_name_from_external_ids(old) if lb_router != old_lb_router: return True except AttributeError: @@ -729,7 +730,8 @@ class OVNLBCreateEvent(base_watcher.OVNLBEvent): # since the subnet did not have access to the public network if hasattr(old, 'external_ids'): with _SYNC_STATE_LOCK.read_lock(): - if self._get_router(old) != self._get_router(row): + if (router_utils.get_name_from_external_ids(old) != + router_utils.get_name_from_external_ids(row)): self.agent.expose_ovn_lb_vip(row) @@ -747,15 +749,15 @@ class OVNLBDeleteEvent(base_watcher.OVNLBEvent): if event == self.ROW_DELETE: if not row.vips: return False - lb_router = self._get_router(row) + lb_router = router_utils.get_name_from_external_ids(row) if lb_router in self.agent.ovn_local_cr_lrps.keys(): return True return False # ROW UPDATE EVENT - lb_router = self._get_router(row) + lb_router = router_utils.get_name_from_external_ids(row) if hasattr(old, 'external_ids'): - old_lb_router = self._get_router(old) + old_lb_router = router_utils.get_name_from_external_ids(old) if not old_lb_router: return False if old_lb_router not in self.agent.ovn_local_cr_lrps.keys(): @@ -800,7 +802,8 @@ class OVNLBDeleteEvent(base_watcher.OVNLBEvent): # router unset ext-gw if hasattr(old, 'external_ids'): with _SYNC_STATE_LOCK.read_lock(): - if self._get_router(old) != self._get_router(row): + if (router_utils.get_name_from_external_ids(old) != + router_utils.get_name_from_external_ids(row)): self.agent.withdraw_ovn_lb_vip(old) @@ -822,7 +825,8 @@ class OVNPFBaseEvent(base_watcher.OVNLBEvent): if not row.vips: return False - lb_router = self._get_router(row, constants.OVN_LR_NAME_EXT_ID_KEY) + lb_router = router_utils.get_name_from_external_ids( + row, constants.OVN_LR_NAME_EXT_ID_KEY) return lb_router in self.agent.ovn_local_cr_lrps.keys() diff --git a/ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_router.py b/ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_router.py new file mode 100644 index 00000000..52dc85b0 --- /dev/null +++ b/ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_router.py @@ -0,0 +1,62 @@ +# 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 import constants +from ovn_bgp_agent.drivers.openstack.utils import router as r_utils +from ovn_bgp_agent.tests import base as test_base +from ovn_bgp_agent.tests import utils as test_utils + + +class TestGetNameFromExternalIds(test_base.TestCase): + def test_router_present(self): + expected_router = 'foo' + r_ext_id = 'neutron-{:s}'.format(expected_router) + row = test_utils.create_row( + external_ids={ + constants.OVN_LB_LR_REF_EXT_ID_KEY: r_ext_id}) + router = r_utils.get_name_from_external_ids(row) + + self.assertEqual(expected_router, router) + + def test_router_present_custom_field(self): + expected_router = 'foo' + custom_field = 'bar' + r_ext_id = 'neutron-{:s}'.format(expected_router) + row = test_utils.create_row( + external_ids={custom_field: r_ext_id}) + router = r_utils.get_name_from_external_ids(row, key=custom_field) + + self.assertEqual(expected_router, router) + + def test_router_missing(self): + row = test_utils.create_row(external_ids={}) + router = r_utils.get_name_from_external_ids(row) + + self.assertIsNone(router) + + def test_router_missing_custom_field(self): + row = test_utils.create_row(external_ids={}) + router = r_utils.get_name_from_external_ids(row, key='foo') + + self.assertIsNone(router) + + def test_router_bad_name(self): + expected_router = 'foo' + row = test_utils.create_row( + external_ids={ + constants.OVN_LB_LR_REF_EXT_ID_KEY: expected_router}) + router = r_utils.get_name_from_external_ids(row) + + self.assertEqual(expected_router, router) diff --git a/ovn_bgp_agent/tests/unit/drivers/openstack/watchers/test_base_watcher.py b/ovn_bgp_agent/tests/unit/drivers/openstack/watchers/test_base_watcher.py index 6d8b7e56..d6ccc038 100644 --- a/ovn_bgp_agent/tests/unit/drivers/openstack/watchers/test_base_watcher.py +++ b/ovn_bgp_agent/tests/unit/drivers/openstack/watchers/test_base_watcher.py @@ -33,19 +33,6 @@ class TestOVNLBEvent(test_base.TestCase): self.ovnlb_event = FakeOVNLBEvent( mock.Mock(), [mock.Mock()]) - def test__get_router(self): - row = utils.create_row( - external_ids={constants.OVN_LB_LR_REF_EXT_ID_KEY: 'neutron-net'}) - self.assertEqual('net', self.ovnlb_event._get_router( - row, constants.OVN_LB_LR_REF_EXT_ID_KEY)) - self.assertEqual('net', self.ovnlb_event._get_router(row)) - row = utils.create_row( - external_ids={constants.OVN_LR_NAME_EXT_ID_KEY: 'neutron-router1'}) - self.assertEqual('router1', self.ovnlb_event._get_router( - row, constants.OVN_LR_NAME_EXT_ID_KEY)) - row = utils.create_row(external_ids={}) - self.assertEqual(None, self.ovnlb_event._get_router(row)) - def test__is_vip(self): row = utils.create_row( external_ids={constants.OVN_LB_VIP_IP_EXT_ID_KEY: '192.168.1.50',