diff --git a/ovn_bgp_agent/constants.py b/ovn_bgp_agent/constants.py index d6f9e940..3bb70103 100644 --- a/ovn_bgp_agent/constants.py +++ b/ovn_bgp_agent/constants.py @@ -67,6 +67,9 @@ EXPOSE = "expose" WITHDRAW = "withdraw" OVN_REQUESTED_CHASSIS = "requested-chassis" +OVN_HOST_ID_EXT_ID_KEY = "neutron:host_id" +OVN_CHASSIS_AT_OPTIONS = "options" +OVN_CHASSIS_AT_EXT_IDS = "external-ids" # Exposing method names EXPOSE_METHOD_UNDERLAY = 'underlay' diff --git a/ovn_bgp_agent/drivers/openstack/utils/ovn.py b/ovn_bgp_agent/drivers/openstack/utils/ovn.py index 04dd5681..17b34d49 100644 --- a/ovn_bgp_agent/drivers/openstack/utils/ovn.py +++ b/ovn_bgp_agent/drivers/openstack/utils/ovn.py @@ -216,8 +216,13 @@ class OvsdbNbOvnIdl(nb_impl_idl.OvnNbApiIdlImpl, Backend): ports = [] cmd = self.db_find_rows('Logical_Switch_Port', ('up', '=', True)) for row in cmd.execute(check_error=True): - if (row.options and - row.options.get('requested-chassis') == chassis): + if (row.external_ids and + row.external_ids.get( + constants.OVN_HOST_ID_EXT_ID_KEY) == chassis): + ports.append(row) + elif (row.options and + row.options.get( + constants.OVN_REQUESTED_CHASSIS) == chassis): ports.append(row) return ports diff --git a/ovn_bgp_agent/drivers/openstack/watchers/base_watcher.py b/ovn_bgp_agent/drivers/openstack/watchers/base_watcher.py index 8548c66f..bd828546 100644 --- a/ovn_bgp_agent/drivers/openstack/watchers/base_watcher.py +++ b/ovn_bgp_agent/drivers/openstack/watchers/base_watcher.py @@ -13,9 +13,11 @@ # limitations under the License. from oslo_log import log as logging - from ovsdbapp.backend.ovs_idl import event as row_event +from ovn_bgp_agent import constants + + LOG = logging.getLogger(__name__) @@ -59,3 +61,14 @@ class LSPChassisEvent(Event): def _check_ip_associated(self, mac): return len(mac.strip().split(' ')) > 1 + + def _get_chassis(self, row): + if (hasattr(row, 'external_ids') and + row.external_ids.get(constants.OVN_HOST_ID_EXT_ID_KEY)): + return (row.external_ids[constants.OVN_HOST_ID_EXT_ID_KEY], + constants.OVN_CHASSIS_AT_EXT_IDS) + elif (hasattr(row, 'options') and + row.options.get(constants.OVN_REQUESTED_CHASSIS)): + return (row.options[constants.OVN_REQUESTED_CHASSIS], + constants.OVN_CHASSIS_AT_OPTIONS) + return None, None 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 bf83d47e..a4643cb3 100644 --- a/ovn_bgp_agent/drivers/openstack/watchers/nb_bgp_watcher.py +++ b/ovn_bgp_agent/drivers/openstack/watchers/nb_bgp_watcher.py @@ -35,19 +35,28 @@ class LogicalSwitchPortProviderCreateEvent(base_watcher.LSPChassisEvent): if not self._check_ip_associated(row.addresses[0]): return False - current_chassis = row.options.get(constants.OVN_REQUESTED_CHASSIS) + current_chassis, chassis_location = self._get_chassis(row) if current_chassis != self.agent.chassis: return False if not bool(row.up[0]): return False - if hasattr(old, 'options'): - old_chassis = old.options.get(constants.OVN_REQUESTED_CHASSIS) - if not old_chassis or current_chassis != old_chassis: - return True if hasattr(old, 'up'): if not bool(old.up[0]): return True + + # NOTE(ltomasbo): This can be updated/removed once neutron has + # chassis information on external ids + if chassis_location == constants.OVN_CHASSIS_AT_OPTIONS: + if hasattr(old, 'options'): + old_chassis, _ = self._get_chassis(old) + if not old_chassis or current_chassis != old_chassis: + return True + else: + if hasattr(old, 'external_ids'): + old_chassis, _ = self._get_chassis(old) + if not old_chassis or current_chassis != old_chassis: + return True except (IndexError, AttributeError): return False @@ -72,24 +81,40 @@ class LogicalSwitchPortProviderDeleteEvent(base_watcher.LSPChassisEvent): if not self._check_ip_associated(row.addresses[0]): return False - current_chassis = row.options.get(constants.OVN_REQUESTED_CHASSIS) + current_chassis, chassis_location = self._get_chassis(row) if event == self.ROW_DELETE: - return current_chassis == self.agent.chassis + return current_chassis == self.agent.chassis and row.up # ROW_UPDATE EVENT - if hasattr(old, 'options'): - # check chassis change - old_chassis = old.options.get(constants.OVN_REQUESTED_CHASSIS) - if old_chassis != self.agent.chassis: - return False - if not current_chassis or current_chassis != old_chassis: - return True - if hasattr(old, 'up'): if not bool(old.up[0]): return False if not bool(row.up[0]): return True + else: + # If there is no change on the status, and it was already down + # there is no need to remove it again + if not bool(row.up[0]): + return False + + # NOTE(ltomasbo): This can be updated/removed once neutron has + # chassis information on external ids + if chassis_location == constants.OVN_CHASSIS_AT_OPTIONS: + if hasattr(old, 'options'): + # check chassis change + old_chassis, _ = self._get_chassis(old) + if old_chassis != self.agent.chassis: + return False + if not current_chassis or current_chassis != old_chassis: + return True + else: + if hasattr(old, 'external_ids'): + # check chassis change + old_chassis, _ = self._get_chassis(old) + if old_chassis != self.agent.chassis: + return False + if not current_chassis or current_chassis != old_chassis: + return True except (IndexError, AttributeError): return False @@ -114,28 +139,41 @@ class LogicalSwitchPortFIPCreateEvent(base_watcher.LSPChassisEvent): if not self._check_ip_associated(row.addresses[0]): return False - current_chassis = row.options.get(constants.OVN_REQUESTED_CHASSIS) + current_chassis, chassis_location = self._get_chassis(row) current_port_fip = row.external_ids.get( constants.OVN_FIP_EXT_ID_KEY) if (current_chassis != self.agent.chassis or not bool(row.up[0]) or not current_port_fip): return False - if hasattr(old, 'options'): - # check chassis change - old_chassis = old.options.get(constants.OVN_REQUESTED_CHASSIS) - if not old_chassis or current_chassis != old_chassis: - return True - if hasattr(old, 'external_ids'): - # check fips addition - old_port_fip = old.external_ids.get( - constants.OVN_FIP_EXT_ID_KEY) - if not old_port_fip or current_port_fip != old_port_fip: - return True if hasattr(old, 'up'): # check port status change if not bool(old.up[0]): return True + + # NOTE(ltomasbo): This can be updated/removed once neutron has + # chassis information on external ids + if chassis_location == constants.OVN_CHASSIS_AT_OPTIONS: + if hasattr(old, 'options'): + old_chassis, _ = self._get_chassis(old) + if not old_chassis or current_chassis != old_chassis: + return True + if hasattr(old, 'external_ids'): + # check fips addition + old_port_fip = old.external_ids.get( + constants.OVN_FIP_EXT_ID_KEY) + if not old_port_fip or current_port_fip != old_port_fip: + return True + else: # by default expect the chassis information at external-ids + if hasattr(old, 'external_ids'): + # note the whole extenal-ids are included, even if only + # one field inside it is updated + old_chassis, _ = self._get_chassis(old) + old_port_fip = old.external_ids.get( + constants.OVN_FIP_EXT_ID_KEY) + if (current_chassis != old_chassis or + current_port_fip != old_port_fip): + return True except (IndexError, AttributeError): return False return False @@ -164,7 +202,7 @@ class LogicalSwitchPortFIPDeleteEvent(base_watcher.LSPChassisEvent): if not self._check_ip_associated(row.addresses[0]): return False - current_chassis = row.options.get(constants.OVN_REQUESTED_CHASSIS) + current_chassis, chassis_location = self._get_chassis(row) current_port_fip = row.external_ids.get( constants.OVN_FIP_EXT_ID_KEY) if event == self.ROW_DELETE: @@ -173,31 +211,50 @@ class LogicalSwitchPortFIPDeleteEvent(base_watcher.LSPChassisEvent): return True return False - if hasattr(old, 'options'): - # check chassis change - old_chassis = old.options.get(constants.OVN_REQUESTED_CHASSIS) - if (not old_chassis or old_chassis != self.agent.chassis): - return False - if current_chassis != old_chassis and current_port_fip: - return True - # There was no change in chassis, so only progress if the - # chassis matches - if current_chassis != self.agent.chassis: - return False - if hasattr(old, 'external_ids'): - # check fips deletion - old_port_fip = old.external_ids.get( - constants.OVN_FIP_EXT_ID_KEY) - if not old_port_fip: - return False - if old_port_fip != current_port_fip: - return True if hasattr(old, 'up'): # check port status change if not bool(old.up[0]): return False if not bool(row.up[0]) and current_port_fip: return True + + # NOTE(ltomasbo): This can be updated/removed once neutron has + # chassis information on external ids + if chassis_location == constants.OVN_CHASSIS_AT_OPTIONS: + if hasattr(old, 'options'): + # check chassis change + old_chassis, _ = self._get_chassis(old) + if (not old_chassis or old_chassis != self.agent.chassis): + return False + if current_chassis != old_chassis and current_port_fip: + return True + # There was no change in chassis, so only progress if the + # chassis matches + if current_chassis != self.agent.chassis: + return False + if hasattr(old, 'external_ids'): + # check fips deletion + old_port_fip = old.external_ids.get( + constants.OVN_FIP_EXT_ID_KEY) + if not old_port_fip: + return False + if old_port_fip != current_port_fip: + return True + else: # by default expect the chassis information at external-ids + if hasattr(old, 'external_ids'): + # check chassis change + old_chassis, _ = self._get_chassis(old) + if (not old_chassis or old_chassis != self.agent.chassis): + return False + if current_chassis != old_chassis and current_port_fip: + return True + # check fips deletion + old_port_fip = old.external_ids.get( + constants.OVN_FIP_EXT_ID_KEY) + if not old_port_fip: + return False + if old_port_fip != current_port_fip: + return True except (IndexError, AttributeError): return False return False diff --git a/ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_ovn.py b/ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_ovn.py index 76f54829..fa982c6e 100644 --- a/ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_ovn.py +++ b/ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_ovn.py @@ -99,12 +99,31 @@ class TestOvsdbNbOvnIdl(test_base.TestCase): 'NAT', ('logical_port', '=', logical_port)) - def test_get_active_ports_on_chassis(self): + def test_get_active_ports_on_chassis_options(self): chassis = 'local_chassis' row1 = fakes.create_object({ - 'options': {'requested-chassis': chassis}}) + 'options': {'requested-chassis': chassis}, + 'external_ids': {}}) row2 = fakes.create_object({ - 'options': {'requested-chassis': 'other_chassis'}}) + 'options': {'requested-chassis': 'other_chassis'}, + 'external_ids': {}}) + self.nb_idl.db_find_rows.return_value.execute.return_value = [ + row1, row2] + ret = self.nb_idl.get_active_ports_on_chassis(chassis) + + self.assertEqual([row1], ret) + self.nb_idl.db_find_rows.assert_called_once_with( + 'Logical_Switch_Port', + ('up', '=', True)) + + def test_get_active_ports_on_chassis_external_ids(self): + chassis = 'local_chassis' + row1 = fakes.create_object({ + 'options': {}, + 'external_ids': {'neutron:host_id': chassis}}) + row2 = fakes.create_object({ + 'options': {}, + 'external_ids': {'neutron:host_id': 'other_chassis'}}) self.nb_idl.db_find_rows.return_value.execute.return_value = [ row1, row2] ret = self.nb_idl.get_active_ports_on_chassis(chassis) 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 4051412b..eb4d7bdb 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 @@ -36,12 +36,30 @@ class TestLogicalSwitchPortProviderCreateEvent(test_base.TestCase): addresses=['mac 192.168.0.1'], options={'requested-chassis': self.chassis}, up=[True]) - old = utils.create_row(options={}, up=True) + old = utils.create_row(options={}, up=[True]) + self.assertTrue(self.event.match_fn(mock.Mock(), row, old)) + + def test_match_fn_port_up(self): + row = utils.create_row(type=constants.OVN_VM_VIF_PORT_TYPE, + addresses=['mac 192.168.0.1'], + options={'requested-chassis': self.chassis}, + up=[True]) + old = utils.create_row(type=constants.OVN_VM_VIF_PORT_TYPE, + addresses=['mac 192.168.0.1'], + options={'requested-chassis': self.chassis}, + up=[False]) + self.assertTrue(self.event.match_fn(mock.Mock(), row, old)) + + def test_match_fn_external_id(self): + row = utils.create_row(type=constants.OVN_VM_VIF_PORT_TYPE, + addresses=['mac 192.168.0.1'], + external_ids={'neutron:host_id': self.chassis}, + up=[True]) + old = utils.create_row(external_ids={}, up=[True]) self.assertTrue(self.event.match_fn(mock.Mock(), row, old)) def test_match_fn_exception(self): row = utils.create_row(type=constants.OVN_VM_VIF_PORT_TYPE, - addresses=['mac 192.168.0.1'], up=[False]) self.assertFalse(self.event.match_fn(mock.Mock(), row, mock.Mock())) @@ -112,9 +130,18 @@ class TestLogicalSwitchPortProviderDeleteEvent(test_base.TestCase): up=[True]) self.assertTrue(self.event.match_fn(event, row, old)) - def test_match_fn_exception(self): + def test_match_fn_update_chassis(self): + event = self.event.ROW_UPDATE row = utils.create_row(type=constants.OVN_VM_VIF_PORT_TYPE, addresses=['mac 192.168.0.1'], + external_ids={'neutron:host_id': 'chassis2'}, + up=[True]) + old = utils.create_row(external_ids={'neutron:host_id': self.chassis}, + up=[True]) + self.assertTrue(self.event.match_fn(event, row, old)) + + def test_match_fn_exception(self): + row = utils.create_row(type=constants.OVN_VM_VIF_PORT_TYPE, up=[False]) self.assertFalse(self.event.match_fn(mock.Mock(), row, mock.Mock())) @@ -179,6 +206,17 @@ class TestLogicalSwitchPortFIPCreateEvent(test_base.TestCase): old = utils.create_row(options={}, up=[True]) self.assertTrue(self.event.match_fn(mock.Mock(), row, old)) + def test_match_fn_chassis_change_external_ids(self): + row = utils.create_row( + type=constants.OVN_VM_VIF_PORT_TYPE, + addresses=['mac 192.168.0.1'], + external_ids={ + constants.OVN_HOST_ID_EXT_ID_KEY: self.chassis, + constants.OVN_FIP_EXT_ID_KEY: 'fip-ip'}, + up=[True]) + old = utils.create_row(external_ids={}, up=[True]) + self.assertTrue(self.event.match_fn(mock.Mock(), row, old)) + def test_match_fn_status_change(self): row = utils.create_row(type=constants.OVN_VM_VIF_PORT_TYPE, addresses=['mac 192.168.0.1'], @@ -304,6 +342,45 @@ class TestLogicalSwitchPortFIPDeleteEvent(test_base.TestCase): old = utils.create_row(up=[True]) self.assertTrue(self.event.match_fn(event, row, old)) + def test_match_fn_update_external_id(self): + event = self.event.ROW_UPDATE + row = utils.create_row( + type=constants.OVN_VM_VIF_PORT_TYPE, + addresses=['mac 192.168.0.1'], + external_ids={ + constants.OVN_HOST_ID_EXT_ID_KEY: 'other-chassis', + constants.OVN_FIP_EXT_ID_KEY: 'fip-ip'}, + up=[True]) + old = utils.create_row(external_ids={ + constants.OVN_HOST_ID_EXT_ID_KEY: self.chassis, + constants.OVN_FIP_EXT_ID_KEY: 'fip-ip'}) + self.assertTrue(self.event.match_fn(event, row, old)) + + def test_match_fn_update_external_id_remove_fip(self): + event = self.event.ROW_UPDATE + row = utils.create_row( + type=constants.OVN_VM_VIF_PORT_TYPE, + addresses=['mac 192.168.0.1'], + external_ids={ + constants.OVN_HOST_ID_EXT_ID_KEY: self.chassis}, + up=[True]) + old = utils.create_row(external_ids={ + constants.OVN_HOST_ID_EXT_ID_KEY: self.chassis, + constants.OVN_FIP_EXT_ID_KEY: 'fip-ip'}) + self.assertTrue(self.event.match_fn(event, row, old)) + + def test_match_fn_update_external_id_no_fip(self): + event = self.event.ROW_UPDATE + row = utils.create_row( + type=constants.OVN_VM_VIF_PORT_TYPE, + addresses=['mac 192.168.0.1'], + external_ids={ + constants.OVN_HOST_ID_EXT_ID_KEY: self.chassis}, + up=[True]) + old = utils.create_row(external_ids={ + constants.OVN_HOST_ID_EXT_ID_KEY: self.chassis}) + self.assertFalse(self.event.match_fn(event, row, old)) + def test_match_fn_exception(self): row = utils.create_row(type=constants.OVN_VM_VIF_PORT_TYPE, addresses=['mac 192.168.0.1'],