From 32214e08b49f1852087a94afa343bf0e6b838e5e Mon Sep 17 00:00:00 2001 From: Michel Nederlof Date: Mon, 29 Jan 2024 16:14:16 +0100 Subject: [PATCH] Check for networks on router port in match_fn Ideally the _run method should not do a check to see if row is valid. Also, add the _get_ips_info method in base to get in line with LSP events Change-Id: I7320ba37d622cd6b7de55e994caf657fb8b70b8d (cherry picked from commit 2402fdcc00686b77ce280d65f2d511fb65617905) --- .../openstack/watchers/base_watcher.py | 9 +++++ .../openstack/watchers/nb_bgp_watcher.py | 34 +++++++------------ .../openstack/watchers/test_nb_bgp_watcher.py | 34 +++++++++---------- 3 files changed, 37 insertions(+), 40 deletions(-) diff --git a/ovn_bgp_agent/drivers/openstack/watchers/base_watcher.py b/ovn_bgp_agent/drivers/openstack/watchers/base_watcher.py index 29d1ecb0..628956e6 100644 --- a/ovn_bgp_agent/drivers/openstack/watchers/base_watcher.py +++ b/ovn_bgp_agent/drivers/openstack/watchers/base_watcher.py @@ -106,3 +106,12 @@ class LRPChassisEvent(Event): 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), + 'router': row.external_ids.get(constants.OVN_LR_NAME_EXT_ID_KEY), + } 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 4b3f9dec..aea87b9d 100644 --- a/ovn_bgp_agent/drivers/openstack/watchers/nb_bgp_watcher.py +++ b/ovn_bgp_agent/drivers/openstack/watchers/nb_bgp_watcher.py @@ -322,6 +322,9 @@ class ChassisRedirectCreateEvent(base_watcher.LRPChassisEvent): def match_fn(self, event, row, old): try: + if not row.networks: + return False + # check if hosting-chassis is being added hosting_chassis = row.status.get(constants.OVN_STATUS_CHASSIS) if hosting_chassis != self.agent.chassis_id: @@ -340,17 +343,9 @@ class ChassisRedirectCreateEvent(base_watcher.LRPChassisEvent): def _run(self, event, row, old): with _SYNC_STATE_LOCK.read_lock(): - if row.networks: - ips_info = { - 'mac': row.mac, - 'cidrs': row.networks, - 'type': constants.OVN_CR_LRP_PORT_TYPE, - 'logical_switch': self._get_network(row), - 'router': row.external_ids.get( - constants.OVN_LR_NAME_EXT_ID_KEY) - } - ips = [net.split("/")[0] for net in row.networks] - self.agent.expose_ip(ips, ips_info) + ips_info = self._get_ips_info(row) + ips = [net.split("/")[0] for net in row.networks] + self.agent.expose_ip(ips, ips_info) class ChassisRedirectDeleteEvent(base_watcher.LRPChassisEvent): @@ -361,6 +356,9 @@ class ChassisRedirectDeleteEvent(base_watcher.LRPChassisEvent): def match_fn(self, event, row, old): try: + if not row.networks: + return + if event == self.ROW_DELETE: return (row.status.get(constants.OVN_STATUS_CHASSIS) == self.agent.chassis_id) @@ -379,17 +377,9 @@ class ChassisRedirectDeleteEvent(base_watcher.LRPChassisEvent): def _run(self, event, row, old): with _SYNC_STATE_LOCK.read_lock(): - if row.networks: - ips_info = { - 'mac': row.mac, - 'cidrs': row.networks, - 'type': constants.OVN_CR_LRP_PORT_TYPE, - 'logical_switch': self._get_network(row), - 'router': row.external_ids.get( - constants.OVN_LR_NAME_EXT_ID_KEY) - } - ips = [net.split("/")[0] for net in row.networks] - self.agent.withdraw_ip(ips, ips_info) + ips_info = self._get_ips_info(row) + ips = [net.split("/")[0] for net in row.networks] + self.agent.withdraw_ip(ips, ips_info) class LogicalSwitchPortSubnetAttachEvent(base_watcher.LSPChassisEvent): diff --git a/ovn_bgp_agent/tests/unit/drivers/openstack/watchers/test_nb_bgp_watcher.py b/ovn_bgp_agent/tests/unit/drivers/openstack/watchers/test_nb_bgp_watcher.py index 88df8759..1c948f68 100644 --- a/ovn_bgp_agent/tests/unit/drivers/openstack/watchers/test_nb_bgp_watcher.py +++ b/ovn_bgp_agent/tests/unit/drivers/openstack/watchers/test_nb_bgp_watcher.py @@ -562,6 +562,14 @@ class TestChassisRedirectCreateEvent(test_base.TestCase): status={'hosting-chassis': 'other_chassis'}) self.assertFalse(self.event.match_fn(mock.Mock(), row, mock.Mock())) + def test_match_fn_no_networks(self): + row = utils.create_row( + mac='fake-mac', + networks=[], + status={'hosting-chassis': self.chassis_id}, + external_ids={constants.OVN_LS_NAME_EXT_ID_KEY: 'test-ls'}) + self.assertFalse(self.event.match_fn(None, row, None)) + def test_run(self): row = utils.create_row( mac='fake-mac', @@ -576,15 +584,6 @@ class TestChassisRedirectCreateEvent(test_base.TestCase): self.event.run(None, row, None) self.agent.expose_ip.assert_called_once_with(['192.168.0.2'], ips_info) - def test_run_no_networks(self): - row = utils.create_row( - mac='fake-mac', - networks=[], - status={'hosting-chassis': self.chassis_id}, - external_ids={constants.OVN_LS_NAME_EXT_ID_KEY: 'test-ls'}) - self.event.run(None, row, None) - self.agent.expose_ip.assert_not_called() - class TestChassisRedirectDeleteEvent(test_base.TestCase): def setUp(self): @@ -635,6 +634,14 @@ class TestChassisRedirectDeleteEvent(test_base.TestCase): status={'hosting-chassis': 'different_chassis'}) self.assertFalse(self.event.match_fn(mock.Mock(), row, old)) + def test_match_fn_no_networks(self): + row = utils.create_row( + mac='fake-mac', + networks=[], + status={'hosting-chassis': self.chassis_id}, + external_ids={constants.OVN_LS_NAME_EXT_ID_KEY: 'test-ls'}) + self.assertFalse(self.event.match_fn(None, row, None)) + def test_run(self): row = utils.create_row( mac='fake-mac', @@ -650,15 +657,6 @@ class TestChassisRedirectDeleteEvent(test_base.TestCase): self.agent.withdraw_ip.assert_called_once_with(['192.168.0.2'], ips_info) - def test_run_no_networks(self): - row = utils.create_row( - mac='fake-mac', - networks=[], - status={'hosting-chassis': self.chassis_id}, - external_ids={constants.OVN_LS_NAME_EXT_ID_KEY: 'test-ls'}) - self.event.run(None, row, None) - self.agent.withdraw_ip.assert_not_called() - class TestLogicalSwitchPortSubnetAttachEvent(test_base.TestCase): def setUp(self):