diff --git a/ovn_bgp_agent/drivers/openstack/utils/port.py b/ovn_bgp_agent/drivers/openstack/utils/port.py index fca9e17f..b65a8748 100644 --- a/ovn_bgp_agent/drivers/openstack/utils/port.py +++ b/ovn_bgp_agent/drivers/openstack/utils/port.py @@ -12,6 +12,32 @@ # 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 + def has_ip_address_defined(address): return ' ' in address.strip() + + +def get_fip(lsp): + return common.get_from_external_ids(lsp, key=constants.OVN_FIP_EXT_ID_KEY) + + +def has_additional_binding(row): + # requested-chassis can be a comma separated list, so if there + # is a comma in the string, there is an additional binding. + return ',' in getattr(row, 'options', {}).get( + constants.OVN_REQUESTED_CHASSIS, '') + + +def make_lsp_dict(row): + # TODO(jlibosva): Stop passing around dynamic maps + 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': common.get_from_external_ids( + row, constants.OVN_LS_NAME_EXT_ID_KEY), + } diff --git a/ovn_bgp_agent/drivers/openstack/watchers/base_watcher.py b/ovn_bgp_agent/drivers/openstack/watchers/base_watcher.py index c9873fc5..a9038e2c 100644 --- a/ovn_bgp_agent/drivers/openstack/watchers/base_watcher.py +++ b/ovn_bgp_agent/drivers/openstack/watchers/base_watcher.py @@ -67,30 +67,6 @@ class LSPChassisEvent(Event): return driver_utils.get_port_chassis(row, self.agent.chassis, default_port_type=default_type) - def _has_additional_binding(self, row): - if (hasattr(row, 'options') and - row.options.get(constants.OVN_REQUESTED_CHASSIS)): - - # requested-chassis can be a comma separated list, so if there - # is a comma in the string, there is an additional binding. - return ',' in row.options[constants.OVN_REQUESTED_CHASSIS] - - return False - - 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': common_utils.get_from_external_ids( - row, constants.OVN_LS_NAME_EXT_ID_KEY), - } - - def _get_port_fip(self, row): - return getattr(row, 'external_ids', {}).get( - constants.OVN_FIP_EXT_ID_KEY) - class LRPChassisEvent(Event): def __init__(self, bgp_agent, events): 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 2cdacb0c..d1452528 100644 --- a/ovn_bgp_agent/drivers/openstack/watchers/nb_bgp_watcher.py +++ b/ovn_bgp_agent/drivers/openstack/watchers/nb_bgp_watcher.py @@ -83,7 +83,7 @@ class LogicalSwitchPortProviderCreateEvent(base_watcher.LSPChassisEvent): def _run(self, event, row, old): with _SYNC_STATE_LOCK.read_lock(): ips = row.addresses[0].split(' ')[1:] - ips_info = self._get_ips_info(row) + ips_info = port_utils.make_lsp_dict(row) self.agent.expose_ip(ips, ips_info) @@ -130,7 +130,7 @@ class LogicalSwitchPortProviderDeleteEvent(base_watcher.LSPChassisEvent): # 1. port went down (while only attached here) if (hasattr(old, 'up') and bool(old.up[0]) and # port was up not bool(row.up[0]) and # is now down - not self._has_additional_binding(row)): # and bound here + not port_utils.has_additional_binding(row)): # and bound return True # 2. port no longer bound here @@ -142,7 +142,7 @@ class LogicalSwitchPortProviderDeleteEvent(base_watcher.LSPChassisEvent): def _run(self, event, row, old): with _SYNC_STATE_LOCK.read_lock(): ips = row.addresses[0].split(' ')[1:] - ips_info = self._get_ips_info(row) + ips_info = port_utils.make_lsp_dict(row) self.agent.withdraw_ip(ips, ips_info) @@ -281,8 +281,8 @@ class LogicalSwitchPortFIPDeleteEvent(base_watcher.LSPChassisEvent): if not port_utils.has_ip_address_defined(row.addresses[0]): return False - current_port_fip = self._get_port_fip(row) - old_port_fip = self._get_port_fip(old) + current_port_fip = port_utils.get_fip(row) + old_port_fip = port_utils.get_fip(old) if not current_port_fip and not old_port_fip: # This port is not a floating ip update return False @@ -321,11 +321,11 @@ class LogicalSwitchPortFIPDeleteEvent(base_watcher.LSPChassisEvent): def _run(self, event, row, old): # First check to remove the fip provided in old (since this might # have been updated) - fip = self._get_port_fip(old) + fip = port_utils.get_fip(old) if not fip: # Remove the fip provided in the current row, probably a # disassociate of the fip (or a down or a move) - fip = self._get_port_fip(row) + fip = port_utils.get_fip(row) if not fip: return with _SYNC_STATE_LOCK.read_lock(): diff --git a/ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_port.py b/ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_port.py index 7f216b06..f9e3a347 100644 --- a/ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_port.py +++ b/ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_port.py @@ -13,8 +13,10 @@ # 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 port from ovn_bgp_agent.tests import base as test_base +from ovn_bgp_agent.tests import utils as test_utils class TestHasIpAddressDefined(test_base.TestCase): @@ -35,3 +37,48 @@ class TestHasIpAddressDefined(test_base.TestCase): self.assertTrue( port.has_ip_address_defined( 'aa:bb:cc:dd:ee:ff 10.10.1.16 10.10.1.17 10.10.1.18')) + + +class TestGetFip(test_base.TestCase): + def test_get_fip(self): + value = 'foo' + lsp = test_utils.create_row( + external_ids={constants.OVN_FIP_EXT_ID_KEY: value}) + observed = port.get_fip(lsp) + + self.assertEqual(value, observed) + + def test_get_fip_not_present(self): + lsp = test_utils.create_row() + self.assertIsNone(port.get_fip(lsp)) + + +class TestHasAdditionalBinding(test_base.TestCase): + def test_has_multiple_chassis(self): + lsp = test_utils.create_row( + options={constants.OVN_REQUESTED_CHASSIS: "chassis1,chassis2"}) + + self.assertTrue(port.has_additional_binding(lsp)) + + def test_has_one_chassis(self): + lsp = test_utils.create_row( + options={constants.OVN_REQUESTED_CHASSIS: "chassis1"}) + + self.assertFalse(port.has_additional_binding(lsp)) + + def test_no_options(self): + lsp = test_utils.create_row() + + self.assertFalse(port.has_additional_binding(lsp)) + + def test_no_requested_chassis(self): + lsp = test_utils.create_row( + options={}) + + self.assertFalse(port.has_additional_binding(lsp)) + + def test_empty_requested_chassis(self): + lsp = test_utils.create_row( + options={constants.OVN_REQUESTED_CHASSIS: ""}) + + self.assertFalse(port.has_additional_binding(lsp)) 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 912f1017..f575578a 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 @@ -15,37 +15,8 @@ from unittest import mock -from ovn_bgp_agent import constants from ovn_bgp_agent.drivers.openstack.watchers import base_watcher from ovn_bgp_agent.tests import base as test_base -from ovn_bgp_agent.tests import utils - - -class FakeLSPChassisEvent(base_watcher.LSPChassisEvent): - def run(self): - pass - - -class TestLSPChassisEvent(test_base.TestCase): - - def setUp(self): - super(TestLSPChassisEvent, self).setUp() - self.lsp_event = FakeLSPChassisEvent( - mock.Mock(), [mock.Mock()]) - - def test__has_additional_binding(self): - row = utils.create_row( - options={constants.OVN_REQUESTED_CHASSIS: 'host1,host2'}) - self.assertTrue(self.lsp_event._has_additional_binding(row)) - - def test__has_additional_binding_no_options(self): - row = utils.create_row() - self.assertFalse(self.lsp_event._has_additional_binding(row)) - - def test__has_additional_binding_single_host(self): - row = utils.create_row( - options={constants.OVN_REQUESTED_CHASSIS: 'host1'}) - self.assertFalse(self.lsp_event._has_additional_binding(row)) class TestChassisCreateEvent(test_base.TestCase):