Merge "Fix event handling for LSP and prefer the options.requested-chassis info" into stable/2023.2

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)

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):

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():

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'})

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):