diff --git a/ovn_bgp_agent/drivers/openstack/nb_ovn_bgp_driver.py b/ovn_bgp_agent/drivers/openstack/nb_ovn_bgp_driver.py index b5401134..d366bb9f 100644 --- a/ovn_bgp_agent/drivers/openstack/nb_ovn_bgp_driver.py +++ b/ovn_bgp_agent/drivers/openstack/nb_ovn_bgp_driver.py @@ -356,6 +356,48 @@ class NBOVNBGPDriver(driver_api.AgentDriverBase): if lb.external_ids.get(constants.OVN_LB_VIP_FIP_EXT_ID_KEY): self._withdraw_ovn_lb_fip(lb) + def is_ls_provider(self, logical_switch): + '''Check if given logical switch is a provider network on this host + + It will also validate that the provider network actually has been + exposed by the driver. + ''' + + if logical_switch is None: + self.ovn_tenant_ls[logical_switch] = True # just for caching + return False + + # Check if the ls has already been identified as a tenant network + if self.ovn_tenant_ls.get(logical_switch, None) is True: + return False + + # Check the bridge device from provider network + _, bridge_device, _ = self._get_provider_ls_info(logical_switch) + + # Check if the bridge device has been exposed by the wiring methods + if bridge_device not in self.ovn_bridge_mappings.values(): + return False + + return bridge_device is not None + + def is_ip_exposed(self, logical_switch, ips): + '''Check if the ip(s) from given logical_switch is exported. + + So basically, check if the ips are listed in self._exposed_ips. + If it is in there, it should be exposed by ovn-bgp-agent. + + This helps a lot in evaluating events. + ''' + # Ip may be a list + if not isinstance(ips, (list, tuple, set)): + ips = [ips] + + for ip in ips: + if ip in self._exposed_ips.get(logical_switch, {}): + return True + + return False + @lockutils.synchronized('nbbgp') def expose_ip(self, ips, ips_info): '''Advertice BGP route by adding IP to device. @@ -497,6 +539,32 @@ class NBOVNBGPDriver(driver_api.AgentDriverBase): ips_info['router']) LOG.debug("Deleted BGP route for logical port with ip %s", ips) + def _get_provider_ls_info(self, logical_switch): + '''Helper method for _get_ls_localnet_info + + It returns the information from cached self.ovn_provider_ls dictionary + or calls the method and populates the dictionary for given + logical_switch. + + It returns a tuple of localnet, bridge_device and bridge_vlan for + compatibilty with _get_ls_localnet_info + ''' + if logical_switch is None: + return None, None, None + + if logical_switch not in self.ovn_provider_ls: + localnet, bridge_dev, bridge_vlan = self._get_ls_localnet_info( + logical_switch) + + self.ovn_provider_ls[logical_switch] = { + 'bridge_device': bridge_dev, + 'bridge_vlan': bridge_vlan, + 'localnet': localnet + } + + ls = self.ovn_provider_ls[logical_switch] + return ls['localnet'], ls['bridge_device'], ls['bridge_vlan'] + def _get_ls_localnet_info(self, logical_switch): localnet_ports = self.nb_idl.ls_get_localnet_ports( logical_switch, if_exists=True).execute(check_error=True) diff --git a/ovn_bgp_agent/drivers/openstack/watchers/base_watcher.py b/ovn_bgp_agent/drivers/openstack/watchers/base_watcher.py index 084cc1f1..f1864e74 100644 --- a/ovn_bgp_agent/drivers/openstack/watchers/base_watcher.py +++ b/ovn_bgp_agent/drivers/openstack/watchers/base_watcher.py @@ -74,23 +74,61 @@ 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 + def _get_chassis(self, row, default_type=constants.OVN_VM_VIF_PORT_TYPE): + # row.options['requested-chassis'] superseeds the id in external_ids. + # Since it is not used for virtual ports by ovn, this option will be + # ignored for virtual ports. + + # since 'old' rows could be used, it will not hold the type information + # if that is the case, please supply a default in the arguments. + row_type = getattr(row, 'type', default_type) + if (row_type not in [constants.OVN_VIRTUAL_VIF_PORT_TYPE] and + hasattr(row, 'options') and + row.options.get(constants.OVN_REQUESTED_CHASSIS)): + + # requested-chassis can be a comma separated list, + # so lets only return our chassis if it is a list, to be able to + # do a == equal comparison + req_chassis = row.options[constants.OVN_REQUESTED_CHASSIS] + if self.agent.chassis in req_chassis.split(','): + req_chassis = self.agent.chassis + + return (req_chassis, constants.OVN_CHASSIS_AT_OPTIONS) + elif (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 + 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_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), + } + + 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 8093da86..2f1c3851 100644 --- a/ovn_bgp_agent/drivers/openstack/watchers/nb_bgp_watcher.py +++ b/ovn_bgp_agent/drivers/openstack/watchers/nb_bgp_watcher.py @@ -31,51 +31,49 @@ class LogicalSwitchPortProviderCreateEvent(base_watcher.LSPChassisEvent): bgp_agent, events) def match_fn(self, event, row, old): + '''Match port updates to see if we should expose this lsp + + If the event matches the following criteria, we should + totally ignore this event, since it is not meant for this host. + + 1. this host does not own this lsp + 2. the lsp is not up + 3. the logical switch is not exposed with agent, which means it + is not a provider network + + When the event still has not been rejected, then the only thing to + do is to check if the ips for this lsp have not been exported yet. + ''' + if row.type not in [constants.OVN_VM_VIF_PORT_TYPE, + constants.OVN_VIRTUAL_VIF_PORT_TYPE]: + return try: # single and dual-stack format if not self._check_ip_associated(row.addresses[0]): return False - current_chassis, chassis_location = self._get_chassis(row) - if current_chassis != self.agent.chassis: - return False - if not bool(row.up[0]): + current_chassis, _ = self._get_chassis(row) + logical_switch = self._get_network(row) + + # Check for rejection criteria + if (current_chassis != self.agent.chassis or + not bool(row.up[0]) or + not self.agent.is_ls_provider(logical_switch)): return False - if hasattr(old, 'up'): - if not bool(old.up[0]): - return True + # At this point, the port is bound on this host, it is up and + # the logical switch is exposable by the agent. + # Only create the ips if not already exposed. + ips = row.addresses[0].split(' ')[1:] + return not self.agent.is_ip_exposed(logical_switch, ips) - # 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 - return False def _run(self, event, row, old): - if row.type not in [constants.OVN_VM_VIF_PORT_TYPE, - constants.OVN_VIRTUAL_VIF_PORT_TYPE]: - return with _SYNC_STATE_LOCK.read_lock(): ips = row.addresses[0].split(' ')[1:] - mac = row.addresses[0].strip().split(' ')[0] - ips_info = { - 'mac': mac, - 'cidrs': row.external_ids.get(constants.OVN_CIDRS_EXT_ID_KEY, - "").split(), - 'type': row.type, - 'logical_switch': self._get_network(row) - } + ips_info = self._get_ips_info(row) self.agent.expose_ip(ips, ips_info) @@ -86,125 +84,127 @@ class LogicalSwitchPortProviderDeleteEvent(base_watcher.LSPChassisEvent): bgp_agent, events) def match_fn(self, event, row, old): + '''Match port deletes or port downs or migrations + + 1. [DELETE] Port has been deleted, and we're hosting it + 2. [UPDATE] Port went down, withdraw if we announced it + 3. [UPDATE] Port has been migrated away and we're hosting it + ''' + if row.type not in [constants.OVN_VM_VIF_PORT_TYPE, + constants.OVN_VIRTUAL_VIF_PORT_TYPE]: + return try: # single and dual-stack format if not self._check_ip_associated(row.addresses[0]): return False - current_chassis, chassis_location = self._get_chassis(row) + ips = row.addresses[0].split(' ')[1:] + logical_switch = self._get_network(row) + + # Do nothing if we do not expose the current port + if not self.agent.is_ip_exposed(logical_switch, ips): + return False + + # Delete event, always execute (since we expose it) if event == self.ROW_DELETE: - return (current_chassis == self.agent.chassis and - bool(row.up[0])) + return True - # ROW_UPDATE EVENT - if hasattr(old, 'up'): - if not bool(old.up[0]): - return False - # Assumes chassis and status are not changed at the same time - if (not bool(row.up[0]) and - current_chassis == self.agent.chassis): - 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 + current_chassis, _ = self._get_chassis(row) + # Delete the port from current chassis, if + # 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 + return True + + # 2. port no longer bound here + return current_chassis != self.agent.chassis - # 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 - return False def _run(self, event, row, old): - if row.type not in [constants.OVN_VM_VIF_PORT_TYPE, - constants.OVN_VIRTUAL_VIF_PORT_TYPE]: - return with _SYNC_STATE_LOCK.read_lock(): ips = row.addresses[0].split(' ')[1:] - mac = row.addresses[0].strip().split(' ')[0] - ips_info = { - 'mac': mac, - 'cidrs': row.external_ids.get(constants.OVN_CIDRS_EXT_ID_KEY, - "").split(), - 'type': row.type, - 'logical_switch': row.external_ids.get( - constants.OVN_LS_NAME_EXT_ID_KEY) - } + ips_info = self._get_ips_info(row) self.agent.withdraw_ip(ips, ips_info) class LogicalSwitchPortFIPCreateEvent(base_watcher.LSPChassisEvent): + '''Floating IP create events based on the LogicalSwitchPort + + The LSP has information about the host is should be exposed to, which + adds a bit of complexity in the event match, but saves a lot of queries + to the OVN NB DB. + + Should trigger on: + - floating ip was attached to a lsp (external_ids.neutron:port_fip + appeared with information) + - port with floating ip attached was set to up (old.up = false and + row.up = true) + + During a migration of a lsp, the following events happen (chronologically): + 1. options.requested_chassis is updated (now a comma separated list) + we also get external_ids, but only revision_number is updated. + 2. update with only external_ids update (with only a revnum update) + 3. port is set down (by ovn-controller on source host) + 4. update with only external_ids update (with only a revnum update) + 5. external_ids update (neutron:host_id is removed) + 6. options.requested_chassis is updated (with only dest host) + and external_ids update which now includes neutron:host_id again + 7. port is set up (by ovn-controller on dest host) + 8 and 9 are only a revnum update in the external_ids + + So for migration flow we are only interested in event 7. + Otherwise the floating ip would be added upon event 2, deleted with + event 3 and re-added with event 7. + ''' def __init__(self, bgp_agent): events = (self.ROW_UPDATE,) super(LogicalSwitchPortFIPCreateEvent, self).__init__( bgp_agent, events) def match_fn(self, event, row, old): + if row.type not in [constants.OVN_VM_VIF_PORT_TYPE, + constants.OVN_VIRTUAL_VIF_PORT_TYPE]: + return try: # single and dual-stack format if not self._check_ip_associated(row.addresses[0]): return False - current_chassis, chassis_location = self._get_chassis(row) + current_chassis, _ = 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): + if (current_chassis != self.agent.chassis or + not bool(row.up[0]) or not current_port_fip): + # Port is not bound on this host, is down or does not have a + # floating ip attached. return False - if hasattr(old, 'up'): - # check port status change - if not bool(old.up[0]): - return True + if hasattr(old, 'up') and not bool(old.up[0]): + # Port changed up, which happens when the port is picked up + # on this host by the ovn-controller during migrations + return True + + old_port_fip = getattr(old, 'external_ids', {}).get( + constants.OVN_FIP_EXT_ID_KEY) + if old_port_fip == current_port_fip: + # Only if the floating ip has changed (for example from empty + # to something else) we need to process this update. + # If nothing else changed in the external_ids, we do not care + # as it would just cause unnecessary events during migrations. + # (see the docstring of this class) + return False + + # Check if the current port_fip has not been exposed yet + return not self.agent.is_ip_exposed(self._get_network(row), + current_port_fip) - # 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 def _run(self, event, row, old): - if row.type not in [constants.OVN_VM_VIF_PORT_TYPE, - constants.OVN_VIRTUAL_VIF_PORT_TYPE]: - return external_ip, external_mac, ls_name = ( self.agent.get_port_external_ip_and_ls(row.name)) if not external_ip or not ls_name: @@ -215,83 +215,86 @@ class LogicalSwitchPortFIPCreateEvent(base_watcher.LSPChassisEvent): class LogicalSwitchPortFIPDeleteEvent(base_watcher.LSPChassisEvent): + '''Floating IP delete events based on the LogicalSwitchPort + + The LSP has information about the host is should be exposed to, which + adds a bit of complexity in the event match, but saves a lot of queries + to the OVN NB DB. + + Should trigger on: + - lsp deleted and bound on this host + - floating ip removed from a lsp (external_ids.neutron:port_fip + disappeared with information) + - port with floating ip attached was set to down (old.up = true and + row.up = false) + - current floating ip is not the same as old floating ip + ''' def __init__(self, bgp_agent): events = (self.ROW_UPDATE, self.ROW_DELETE,) super(LogicalSwitchPortFIPDeleteEvent, self).__init__( bgp_agent, events) def match_fn(self, event, row, old): + '''Match port deletes or port downs or migrations or fip changes + + 1. [DELETE] Port has been deleted, and we're hosting it + 2. [UPDATE] Port went down, withdraw if we announced it + 3. [UPDATE] Floating IP has been disassociated (or re-associated + with another floating ip) + 4. [UPDATE] Port has been migrated away and we're hosting it + ''' + if row.type not in [constants.OVN_VM_VIF_PORT_TYPE, + constants.OVN_VIRTUAL_VIF_PORT_TYPE]: + return try: # single and dual-stack format if not self._check_ip_associated(row.addresses[0]): return False - 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: - if (current_chassis == self.agent.chassis and - bool(row.up[0]) and current_port_fip): - return True + current_port_fip = self._get_port_fip(row) + old_port_fip = self._get_port_fip(old) + if not current_port_fip and not old_port_fip: + # This port is not a floating ip update return False - if hasattr(old, 'up'): - # check port status change - if not bool(old.up[0]): - return False - # Assumes chassis and status are not changed at the same time - if (not bool(row.up[0]) and current_port_fip and - current_chassis == self.agent.chassis): - return True + logical_switch = self._get_network(row) + is_exposed = self.agent.is_ip_exposed(logical_switch, + old_port_fip or + current_port_fip) + if not is_exposed: + # already deleted or not exposed. + return False + + # From here on we know we are exposing a FIP (either old or + # current) + + if event == self.ROW_DELETE: + # Port is deleting + return True + + if (hasattr(old, 'up') and bool(old.up[0]) and # port was up + not bool(row.up[0])): # is now down + return True + + if old_port_fip is not None and current_port_fip != old_port_fip: + # fip has changed, we should remove the old one. + return True + + # If we reach here, just check if host changed + current_chassis, _ = self._get_chassis(row) + return current_chassis != self.agent.chassis - # 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 def _run(self, event, row, old): - if row.type not in [constants.OVN_VM_VIF_PORT_TYPE, - constants.OVN_VIRTUAL_VIF_PORT_TYPE]: - return - fip = row.external_ids.get(constants.OVN_FIP_EXT_ID_KEY) + # First check to remove the fip provided in old (since this might + # have been updated) + fip = self._get_port_fip(old) if not fip: - fip = old.external_ids.get(constants.OVN_FIP_EXT_ID_KEY) + # 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) if not fip: return with _SYNC_STATE_LOCK.read_lock(): 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 e444a9bc..429941a9 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 @@ -99,6 +99,65 @@ class TestLSPChassisEvent(test_base.TestCase): self.assertTrue(self.lsp_event._check_ip_associated( 'aa:bb:cc:dd:ee:ff 10.10.1.16 10.10.1.17 10.10.1.18')) + def test__check_chassis_from_options(self): + my_host = 'foo-host' + + # it is a VM port type, should use options field. + row = utils.create_row( + external_ids={constants.OVN_HOST_ID_EXT_ID_KEY: 'bar-host'}, + options={constants.OVN_REQUESTED_CHASSIS: my_host}) + + self.assertEqual(self.lsp_event._get_chassis(row), + (my_host, constants.OVN_CHASSIS_AT_OPTIONS)) + + def test__check_chassis_from_external_ids(self): + my_host = 'foo-host' + + # it is a VM port type, should use options field. + row = utils.create_row( + external_ids={constants.OVN_HOST_ID_EXT_ID_KEY: my_host}) + + self.assertEqual(self.lsp_event._get_chassis(row), + (my_host, constants.OVN_CHASSIS_AT_EXT_IDS)) + + def test__check_chassis_from_external_ids_virtual_port(self): + my_host = 'foo-host' + + # it is a VM port type, should use options field. + row = utils.create_row( + external_ids={constants.OVN_HOST_ID_EXT_ID_KEY: my_host}, + options={constants.OVN_REQUESTED_CHASSIS: 'bar-host'}, + type=constants.OVN_VIRTUAL_VIF_PORT_TYPE) + + self.assertEqual(self.lsp_event._get_chassis(row), + (my_host, constants.OVN_CHASSIS_AT_EXT_IDS)) + + def test__check_chassis_no_information(self): + row = utils.create_row() + self.assertEqual(self.lsp_event._get_chassis(row), + (None, None)) + + def test__check_chassis_no_information_virtual_port(self): + row = utils.create_row( + options={constants.OVN_REQUESTED_CHASSIS: 'bar-host'}, + type=constants.OVN_VIRTUAL_VIF_PORT_TYPE) + self.assertEqual(self.lsp_event._get_chassis(row), + (None, None)) + + 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)) + def test__get_network(self): row = utils.create_row( external_ids={constants.OVN_LS_NAME_EXT_ID_KEY: 'test-net'}) 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 1c948f68..3bc5deae 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 @@ -28,6 +28,13 @@ class TestLogicalSwitchPortProviderCreateEvent(test_base.TestCase): super(TestLogicalSwitchPortProviderCreateEvent, self).setUp() self.chassis = 'fake-chassis' self.agent = mock.Mock(chassis=self.chassis) + + # Assume the logical switch has been setup properly. + self.agent.is_ls_provider.return_value = True + + # Assume the ip is not exposed yet + self.agent.is_ip_exposed.return_value = False + self.event = nb_bgp_watcher.LogicalSwitchPortProviderCreateEvent( self.agent) @@ -85,6 +92,24 @@ class TestLogicalSwitchPortProviderCreateEvent(test_base.TestCase): old = utils.create_row(options={}, up=[True]) self.assertFalse(self.event.match_fn(mock.Mock(), row, old)) + def test_match_fn_additional_bindings(self): + event = self.event.ROW_UPDATE + bindings = ','.join([self.chassis, 'other-chassis']) + row = utils.create_row(type=constants.OVN_VM_VIF_PORT_TYPE, + addresses=['mac 192.168.0.1'], + options={'requested-chassis': bindings}, + up=[True]) + old = utils.create_row(options={'requested-chassis': self.chassis}) + self.assertTrue(self.event.match_fn(event, row, old)) + + def test_match_fn_wrong_type(self): + row = utils.create_row( + type=constants.OVN_CHASSISREDIRECT_VIF_PORT_TYPE, + addresses=['mac 192.168.0.1'], + options={'requested-chassis': self.chassis}, + up=[True]) + self.assertIsNone(self.event.match_fn(mock.Mock(), row, mock.Mock())) + def test_run(self): row = utils.create_row( type=constants.OVN_VM_VIF_PORT_TYPE, @@ -102,15 +127,6 @@ class TestLogicalSwitchPortProviderCreateEvent(test_base.TestCase): self.event.run(mock.Mock(), row, mock.Mock()) self.agent.expose_ip.assert_called_once_with(['192.168.0.1'], ips_info) - def test_run_wrong_type(self): - row = utils.create_row( - type=constants.OVN_CHASSISREDIRECT_VIF_PORT_TYPE, - addresses=['mac 192.168.0.1'], - options={'requested-chassis': self.chassis}, - up=[True]) - self.event.run(mock.Mock(), row, mock.Mock()) - self.agent.expose_ip.assert_not_called() - class TestLogicalSwitchPortProviderDeleteEvent(test_base.TestCase): @@ -118,6 +134,13 @@ class TestLogicalSwitchPortProviderDeleteEvent(test_base.TestCase): super(TestLogicalSwitchPortProviderDeleteEvent, self).setUp() self.chassis = 'fake-chassis' self.agent = mock.Mock(chassis=self.chassis) + + # Assume the logical switch has been setup properly. + self.agent.is_ls_provider.return_value = True + + # Assume the ip is exposed + self.agent.is_ip_exposed.return_value = True + self.event = nb_bgp_watcher.LogicalSwitchPortProviderDeleteEvent( self.agent) @@ -150,6 +173,9 @@ class TestLogicalSwitchPortProviderDeleteEvent(test_base.TestCase): self.assertTrue(self.event.match_fn(event, row, old)) def test_match_fn_update_status_different_chassis(self): + # Update test assumption, since the ip should not be exposed + self.agent.is_ip_exposed.return_value = False + event = self.event.ROW_UPDATE row = utils.create_row(type=constants.OVN_VM_VIF_PORT_TYPE, addresses=['mac 192.168.0.1'], @@ -173,6 +199,17 @@ class TestLogicalSwitchPortProviderDeleteEvent(test_base.TestCase): up=[False]) self.assertFalse(self.event.match_fn(mock.Mock(), row, old)) + def test_match_fn_ignore_not_up_with_additional_bindings(self): + event = self.event.ROW_UPDATE + bindings = ','.join([self.chassis, 'other-chassis']) + row = utils.create_row(type=constants.OVN_VM_VIF_PORT_TYPE, + addresses=['mac 192.168.0.1'], + options={'requested-chassis': bindings}, + up=[False]) + old = utils.create_row(options={'requested-chassis': self.chassis}, + up=[True]) + self.assertFalse(self.event.match_fn(event, row, old)) + def test_match_fn_invalid_address(self): row = utils.create_row(type=constants.OVN_VM_VIF_PORT_TYPE, addresses=['mac '], @@ -188,6 +225,14 @@ class TestLogicalSwitchPortProviderDeleteEvent(test_base.TestCase): old = utils.create_row(options={'requested-chassis': 'other_chassis'}) self.assertFalse(self.event.match_fn(mock.Mock(), row, old)) + def test_match_fn_wrong_type(self): + row = utils.create_row( + type=constants.OVN_CHASSISREDIRECT_VIF_PORT_TYPE, + addresses=['mac 192.168.0.1'], + options={'requested-chassis': self.chassis}, + up=[True]) + self.assertIsNone(self.event.match_fn(mock.Mock(), row, mock.Mock())) + def test_run(self): row = utils.create_row( type=constants.OVN_VM_VIF_PORT_TYPE, @@ -207,15 +252,6 @@ class TestLogicalSwitchPortProviderDeleteEvent(test_base.TestCase): self.agent.withdraw_ip.assert_called_once_with(['192.168.0.1'], ips_info) - def test_run_wrong_type(self): - row = utils.create_row( - type=constants.OVN_CHASSISREDIRECT_VIF_PORT_TYPE, - addresses=['mac 192.168.0.1'], - options={'requested-chassis': self.chassis}, - up=[True]) - self.event.run(mock.Mock(), row, mock.Mock()) - self.agent.withdraw_ip.assert_not_called() - class TestLogicalSwitchPortFIPCreateEvent(test_base.TestCase): @@ -223,6 +259,13 @@ class TestLogicalSwitchPortFIPCreateEvent(test_base.TestCase): super(TestLogicalSwitchPortFIPCreateEvent, self).setUp() self.chassis = 'fake-chassis' self.agent = mock.Mock(chassis=self.chassis) + + # Assume the logical switch has been setup properly. + self.agent.is_ls_provider.return_value = True + + # Assume the ip is not exposed yet + self.agent.is_ip_exposed.return_value = False + self.event = nb_bgp_watcher.LogicalSwitchPortFIPCreateEvent( self.agent) @@ -314,6 +357,11 @@ class TestLogicalSwitchPortFIPCreateEvent(test_base.TestCase): up=[False]) self.assertFalse(self.event.match_fn(mock.Mock(), row, mock.Mock())) + def test_match_fn_wrong_type(self): + row = utils.create_row( + type=constants.OVN_CHASSISREDIRECT_VIF_PORT_TYPE) + self.assertIsNone(self.event.match_fn(mock.Mock(), row, mock.Mock())) + def test_run(self): external_ip = '10.0.0.10' ls_name = 'neutron-net-id' @@ -339,12 +387,6 @@ class TestLogicalSwitchPortFIPCreateEvent(test_base.TestCase): self.event.run(mock.Mock(), row, mock.Mock()) self.agent.expose_fip.assert_not_called() - def test_run_wrong_type(self): - row = utils.create_row( - type=constants.OVN_CHASSISREDIRECT_VIF_PORT_TYPE) - self.event.run(mock.Mock(), row, mock.Mock()) - self.agent.expose_fip.assert_not_called() - class TestLogicalSwitchPortFIPDeleteEvent(test_base.TestCase): @@ -352,6 +394,13 @@ class TestLogicalSwitchPortFIPDeleteEvent(test_base.TestCase): super(TestLogicalSwitchPortFIPDeleteEvent, self).setUp() self.chassis = 'fake-chassis' self.agent = mock.Mock(chassis=self.chassis) + + # Assume the logical switch has been setup properly. + self.agent.is_ls_provider.return_value = True + + # Assume the ip is exposed + self.agent.is_ip_exposed.return_value = True + self.event = nb_bgp_watcher.LogicalSwitchPortFIPDeleteEvent( self.agent) @@ -363,7 +412,7 @@ class TestLogicalSwitchPortFIPDeleteEvent(test_base.TestCase): external_ids={ constants.OVN_FIP_EXT_ID_KEY: 'fip-ip'}, up=[True]) - self.assertTrue(self.event.match_fn(event, row, mock.Mock())) + self.assertTrue(self.event.match_fn(event, row, utils.create_row())) def test_match_fn_update(self): event = self.event.ROW_UPDATE @@ -377,6 +426,9 @@ class TestLogicalSwitchPortFIPDeleteEvent(test_base.TestCase): self.assertTrue(self.event.match_fn(event, row, old)) def test_match_fn_update_different_chassis(self): + # Update test assumption, since the ip should not be exposed + self.agent.is_ip_exposed.return_value = False + event = self.event.ROW_UPDATE row = utils.create_row(type=constants.OVN_VM_VIF_PORT_TYPE, addresses=['mac 192.168.0.1'], @@ -430,7 +482,8 @@ class TestLogicalSwitchPortFIPDeleteEvent(test_base.TestCase): 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())) + old = utils.create_row() + self.assertFalse(self.event.match_fn(mock.Mock(), row, old)) def test_match_fn_not_up(self): row = utils.create_row(type=constants.OVN_VM_VIF_PORT_TYPE, @@ -481,12 +534,17 @@ class TestLogicalSwitchPortFIPDeleteEvent(test_base.TestCase): external_ids={constants.OVN_FIP_EXT_ID_KEY: 'fip-ip'}) self.assertTrue(self.event.match_fn(mock.Mock(), row, old)) + def test_match_fn_wrong_type(self): + row = utils.create_row( + type=constants.OVN_CHASSISREDIRECT_VIF_PORT_TYPE) + self.assertIsNone(self.event.match_fn(mock.Mock(), row, mock.Mock())) + def test_run(self): row = utils.create_row(type=constants.OVN_VM_VIF_PORT_TYPE, external_ids={ constants.OVN_FIP_EXT_ID_KEY: 'fip-ip'}, up=[True]) - self.event.run(mock.Mock(), row, mock.Mock()) + self.event.run(mock.Mock(), row, utils.create_row()) self.agent.withdraw_fip.assert_called_once_with('fip-ip', row) def test_run_no_fip(self): @@ -497,12 +555,6 @@ class TestLogicalSwitchPortFIPDeleteEvent(test_base.TestCase): self.event.run(mock.Mock(), row, old) self.agent.withdraw_fip.assert_not_called() - def test_run_wrong_type(self): - row = utils.create_row( - type=constants.OVN_CHASSISREDIRECT_VIF_PORT_TYPE) - self.event.run(mock.Mock(), row, mock.Mock()) - self.agent.withdraw_fip.assert_not_called() - class TestLocalnetCreateDeleteEvent(test_base.TestCase):