Merge pull request #17 from stackhpc/upstream/yoga-2022-12-12

Synchronise yoga with upstream

Author: markgoddard

neutron/agent/linux/keepalived.py

@@ -152,6 +152,11 @@ def build_config(self):
                 LOG.warning("keepalived_use_no_track cfg option is True but "
                             "keepalived on host seems to not support this "
                             "option")
+        # NOTE(mstinsky): neutron and keepalived are adding the same routes on
+        # primary routers. With this we ensure that both are adding the routes
+        # with the same procotol and prevent duplicated routes which result in
+        # neutron exception for ip route commands.
+        output += ' protocol static'
         return output
 
 

neutron/common/ovn/utils.py

@@ -633,8 +633,8 @@ def compute_address_pairs_diff(ovn_port, neutron_port):
 
 def get_ovn_cms_options(chassis):
     """Return the list of CMS options in a Chassis."""
-    return [opt.strip() for opt in chassis.external_ids.get(
-            constants.OVN_CMS_OPTIONS, '').split(',')]
+    return [opt.strip() for opt in get_ovn_chassis_other_config(chassis).get(
+        constants.OVN_CMS_OPTIONS, '').split(',')]
 
 
 def is_gateway_chassis(chassis):
@@ -823,3 +823,11 @@ def create_neutron_pg_drop():
         }]
 
     OvsdbClientTransactCommand.run(command)
+
+
+def get_ovn_chassis_other_config(chassis):
+    # NOTE(ralonsoh): LP#1990229 to be removed when min OVN version is 22.09
+    try:
+        return chassis.other_config
+    except AttributeError:
+        return chassis.external_ids

neutron/db/l3_db.py

@@ -1025,10 +1025,15 @@ def _add_router_port(self, context, port_id, router, device_owner):
                     subnets_id.extend([fixed_ip['subnet_id']
                                        for fixed_ip in port['fixed_ips']])
                 else:
-                    raise l3_exc.RouterInterfaceNotFound(
-                        router_id=router.id, port_id=rp.port_id)
-
-            if subnets_id:
+                    # due to race conditions maybe the port under analysis is
+                    # deleted, so instead returning a RouterInterfaceNotFound
+                    # we continue the analysis avoiding that port
+                    LOG.debug("Port %s could not be found, it might have been "
+                              "deleted concurrently. Will not be checked for "
+                              "an overlapping router interface.",
+                              rp.port_id)
+
+            if len(subnets_id) > 1:
                 id_filter = {'id': subnets_id}
                 subnets = self._core_plugin.get_subnets(context.elevated(),
                                                         filters=id_filter)

neutron/plugins/ml2/drivers/ovn/agent/neutron_agent.py

@@ -89,7 +89,8 @@ def as_dict(self):
             'configurations': {
                 'chassis_name': self.chassis.name,
                 'bridge-mappings':
-                    self.chassis.external_ids.get('ovn-bridge-mappings', '')},
+                    ovn_utils.get_ovn_chassis_other_config(self.chassis).get(
+                        'ovn-bridge-mappings', '')},
             'start_flag': True,
             'agent_type': self.agent_type,
             'id': self.agent_id,
@@ -141,9 +142,9 @@ class ControllerAgent(NeutronAgent):
 
     @staticmethod  # it is by default, but this makes pep8 happy
     def __new__(cls, chassis_private, driver):
-        external_ids = cls.chassis_from_private(chassis_private).external_ids
-        if ('enable-chassis-as-gw' in
-                external_ids.get('ovn-cms-options', [])):
+        _chassis = cls.chassis_from_private(chassis_private)
+        other_config = ovn_utils.get_ovn_chassis_other_config(_chassis)
+        if 'enable-chassis-as-gw' in other_config.get('ovn-cms-options', []):
             cls = ControllerGatewayAgent
         return super().__new__(cls)
 
@@ -166,8 +167,9 @@ def description(self):
 
     def update(self, chassis_private, clear_down=False):
         super().update(chassis_private, clear_down)
-        external_ids = self.chassis_from_private(chassis_private).external_ids
-        if 'enable-chassis-as-gw' in external_ids.get('ovn-cms-options', []):
+        _chassis = self.chassis_from_private(chassis_private)
+        other_config = ovn_utils.get_ovn_chassis_other_config(_chassis)
+        if 'enable-chassis-as-gw' in other_config.get('ovn-cms-options', []):
             self.__class__ = ControllerGatewayAgent
 
 
@@ -176,9 +178,10 @@ class ControllerGatewayAgent(ControllerAgent):
 
     def update(self, chassis_private, clear_down=False):
         super().update(chassis_private, clear_down)
-        external_ids = self.chassis_from_private(chassis_private).external_ids
+        _chassis = self.chassis_from_private(chassis_private)
+        other_config = ovn_utils.get_ovn_chassis_other_config(_chassis)
         if ('enable-chassis-as-gw' not in
-                external_ids.get('ovn-cms-options', [])):
+                other_config.get('ovn-cms-options', [])):
             self.__class__ = ControllerAgent
 
 

neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py

@@ -179,7 +179,8 @@ def sb_ovn(self, val):
     def get_supported_vif_types(self):
         vif_types = set()
         for ch in self.sb_ovn.chassis_list().execute(check_error=True):
-            dp_type = ch.external_ids.get('datapath-type', '')
+            other_config = ovn_utils.get_ovn_chassis_other_config(ch)
+            dp_type = other_config.get('datapath-type', '')
             if dp_type == ovn_const.CHASSIS_DATAPATH_NETDEV:
                 vif_types.add(portbindings.VIF_TYPE_VHOST_USER)
             else:
@@ -961,8 +962,9 @@ def bind_port(self, context):
                                       'agent': agent})
             return
         chassis = agent.chassis
-        datapath_type = chassis.external_ids.get('datapath-type', '')
-        iface_types = chassis.external_ids.get('iface-types', '')
+        other_config = ovn_utils.get_ovn_chassis_other_config(chassis)
+        datapath_type = other_config.get('datapath-type', '')
+        iface_types = other_config.get('iface-types', '')
         iface_types = iface_types.split(',') if iface_types else []
         chassis_physnets = self.sb_ovn._get_chassis_physnets(chassis)
         for segment_to_bind in context.segments_to_bind:

neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/placement.py

@@ -42,7 +42,8 @@ def _parse_ovn_cms_options(chassis):
 
 
 def _parse_bridge_mappings(chassis):
-    bridge_mappings = chassis.external_ids.get('ovn-bridge-mappings', '')
+    other_config = ovn_utils.get_ovn_chassis_other_config(chassis)
+    bridge_mappings = other_config.get('ovn-bridge-mappings', '')
     bridge_mappings = helpers.parse_mappings(bridge_mappings.split(','),
                                              unique_values=False)
     return {k: [v] for k, v in bridge_mappings.items()}

neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py

@@ -830,7 +830,8 @@ def from_worker(cls, worker_class, driver=None):
         return cls(conn)
 
     def _get_chassis_physnets(self, chassis):
-        bridge_mappings = chassis.external_ids.get('ovn-bridge-mappings', '')
+        other_config = utils.get_ovn_chassis_other_config(chassis)
+        bridge_mappings = other_config.get('ovn-bridge-mappings', '')
         mapping_dict = helpers.parse_mappings(bridge_mappings.split(','))
         return list(mapping_dict.keys())
 
@@ -848,7 +849,8 @@ def get_gateway_chassis_from_cms_options(self, name_only=True):
         return [ch.name if name_only else ch
                 for ch in self.chassis_list().execute(check_error=True)
                 if ovn_const.CMS_OPT_CHASSIS_AS_GW in
-                ch.external_ids.get(ovn_const.OVN_CMS_OPTIONS, '').split(',')]
+                utils.get_ovn_chassis_other_config(ch).get(
+                    ovn_const.OVN_CMS_OPTIONS, '').split(',')]
 
     def get_chassis_and_physnets(self):
         chassis_info_dict = {}
@@ -867,7 +869,7 @@ def get_chassis_by_card_serial_from_cms_options(self,
             if ('{}={}'
                 .format(ovn_const.CMS_OPT_CARD_SERIAL_NUMBER,
                         card_serial_number)
-                    in ch.external_ids.get(
+                    in utils.get_ovn_chassis_other_config(ch).get(
                         ovn_const.OVN_CMS_OPTIONS, '').split(',')):
                 return ch
         msg = _('Chassis with %s %s %s does not exist'

neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py

@@ -172,12 +172,21 @@ def handle_ha_chassis_group_changes(self, event, row, old):
     def match_fn(self, event, row, old):
         if event != self.ROW_UPDATE:
             return True
-        # NOTE(lucasgomes): If the external_ids column wasn't updated
-        # (meaning, Chassis "gateway" status didn't change) just returns
-        if not hasattr(old, 'external_ids') and event == self.ROW_UPDATE:
+
+        # NOTE(ralonsoh): LP#1990229 to be removed when min OVN version is
+        # 22.09
+        other_config = ('other_config' if hasattr(row, 'other_config') else
+                        'external_ids')
+        # NOTE(lucasgomes): If the other_config/external_ids column wasn't
+        # updated (meaning, Chassis "gateway" status didn't change) just
+        # returns
+        if not hasattr(old, other_config) and event == self.ROW_UPDATE:
             return False
-        if (old.external_ids.get('ovn-bridge-mappings') !=
-                row.external_ids.get('ovn-bridge-mappings')):
+        old_br_mappings = utils.get_ovn_chassis_other_config(old).get(
+            'ovn-bridge-mappings')
+        new_br_mappings = utils.get_ovn_chassis_other_config(row).get(
+            'ovn-bridge-mappings')
+        if old_br_mappings != new_br_mappings:
             return True
         # Check if either the Gateway status or Availability Zones has
         # changed in the Chassis
@@ -192,8 +201,9 @@ def match_fn(self, event, row, old):
     def run(self, event, row, old):
         host = row.hostname
         phy_nets = []
+        new_other_config = utils.get_ovn_chassis_other_config(row)
         if event != self.ROW_DELETE:
-            bridge_mappings = row.external_ids.get('ovn-bridge-mappings', '')
+            bridge_mappings = new_other_config.get('ovn-bridge-mappings', '')
             mapping_dict = helpers.parse_mappings(bridge_mappings.split(','))
             phy_nets = list(mapping_dict)
 
@@ -208,9 +218,10 @@ def run(self, event, row, old):
             if event == self.ROW_DELETE:
                 kwargs['event_from_chassis'] = row.name
             elif event == self.ROW_UPDATE:
-                old_mappings = old.external_ids.get('ovn-bridge-mappings',
+                old_other_config = utils.get_ovn_chassis_other_config(old)
+                old_mappings = old_other_config.get('ovn-bridge-mappings',
                                                     set()) or set()
-                new_mappings = row.external_ids.get('ovn-bridge-mappings',
+                new_mappings = new_other_config.get('ovn-bridge-mappings',
                                                     set()) or set()
                 if old_mappings:
                     old_mappings = set(old_mappings.split(','))
@@ -339,11 +350,17 @@ class ChassisAgentTypeChangeEvent(ChassisEvent):
     events = (BaseEvent.ROW_UPDATE,)
 
     def match_fn(self, event, row, old=None):
-        if not getattr(old, 'external_ids', False):
+        # NOTE(ralonsoh): LP#1990229 to be removed when min OVN version is
+        # 22.09
+        other_config = ('other_config' if hasattr(row, 'other_config') else
+                        'external_ids')
+        if not getattr(old, other_config, False):
             return False
-        agent_type_change = n_agent.NeutronAgent.chassis_from_private(
-                row).external_ids.get('ovn-cms-options', []) != (
-                        old.external_ids.get('ovn-cms-options', []))
+        chassis = n_agent.NeutronAgent.chassis_from_private(row)
+        new_other_config = utils.get_ovn_chassis_other_config(chassis)
+        old_other_config = utils.get_ovn_chassis_other_config(old)
+        agent_type_change = new_other_config.get('ovn-cms-options', []) != (
+            old_other_config.get('ovn-cms-options', []))
         return agent_type_change
 
     def run(self, event, row, old):

neutron/tests/base.py

@@ -492,7 +492,11 @@ def setup_rootwrap(self):
                     root_helper_daemon=get_rootwrap_daemon_cmd())
 
     def _simulate_concurrent_requests_process_and_raise(self, calls, args):
+        self._simulate_concurrent_requests_process(calls, args,
+                                                   raise_on_exception=True)
 
+    def _simulate_concurrent_requests_process(self, calls, args,
+                                              raise_on_exception=False):
         class SimpleThread(threading.Thread):
             def __init__(self, q):
                 super(SimpleThread, self).__init__()
@@ -529,10 +533,16 @@ def get_exception(self):
             t.start()
         q.join()
 
+        threads_exceptions = []
         for t in threads:
             e = t.get_exception()
             if e:
-                raise e
+                if raise_on_exception:
+                    raise e
+                else:
+                    threads_exceptions.append(e)
+
+        return threads_exceptions
 
 
 class PluginFixture(fixtures.Fixture):

neutron/tests/fullstack/test_l3_agent.py

@@ -270,25 +270,37 @@ def _is_filter_set(direction):
     def _test_concurrent_router_subnet_attachment_overlapping_cidr(self,
                                                                    ha=False):
         tenant_id = uuidutils.generate_uuid()
-        subnet_cidr = '10.100.0.0/24'
-        network1 = self.safe_client.create_network(
-            tenant_id, name='foo-network1')
-        subnet1 = self.safe_client.create_subnet(
-            tenant_id, network1['id'], subnet_cidr)
-        network2 = self.safe_client.create_network(
-            tenant_id, name='foo-network2')
-        subnet2 = self.safe_client.create_subnet(
-            tenant_id, network2['id'], subnet_cidr)
+        subnet_cidr = '10.200.0.0/24'
+        # to have many port interactions where race conditions would happen
+        # deleting ports meanwhile find operations to evaluate the overlapping
+        subnets = 10
+
+        funcs = []
+        args = []
         router = self.safe_client.create_router(tenant_id, ha=ha)
 
-        funcs = [self.safe_client.add_router_interface,
-                 self.safe_client.add_router_interface]
-        args = [(router['id'], subnet1['id']), (router['id'], subnet2['id'])]
-        self.assertRaises(
-            exceptions.BadRequest,
-            self._simulate_concurrent_requests_process_and_raise,
-            funcs,
-            args)
+        for i in range(subnets):
+            network_tmp = self.safe_client.create_network(
+                tenant_id, name='foo-network' + str(i))
+            subnet_tmp = self.safe_client.create_subnet(
+                tenant_id, network_tmp['id'], subnet_cidr)
+            funcs.append(self.safe_client.add_router_interface)
+            args.append((router['id'], subnet_tmp['id']))
+
+        exception_requests = self._simulate_concurrent_requests_process(
+            funcs, args)
+
+        if not all(type(e) == exceptions.BadRequest
+                   for e in exception_requests):
+            self.fail('Unexpected exception adding interfaces to router from '
+                      'different subnets overlapping')
+
+        if not len(exception_requests) >= (subnets - 1):
+            self.fail('If we have tried to associate %s subnets overlapping '
+                      'cidr to the router, we should have received at least '
+                      '%s or %s rejected requests, but we have only received '
+                      '%s', (str(subnets), str(subnets - 1), str(subnets),
+                             str(len(exception_requests))))
 
 
 class TestLegacyL3Agent(TestL3Agent):
@@ -441,7 +453,7 @@ def test_external_subnet_changed(self):
     def test_router_fip_qos_after_admin_state_down_up(self):
         self._router_fip_qos_after_admin_state_down_up()
 
-    def test_concurrent_router_subnet_attachment_overlapping_cidr_(self):
+    def test_concurrent_router_subnet_attachment_overlapping_cidr(self):
         self._test_concurrent_router_subnet_attachment_overlapping_cidr()
 
 
@@ -601,6 +613,6 @@ def test_external_subnet_changed(self):
     def test_router_fip_qos_after_admin_state_down_up(self):
         self._router_fip_qos_after_admin_state_down_up(ha=True)
 
-    def test_concurrent_router_subnet_attachment_overlapping_cidr_(self):
+    def test_concurrent_router_subnet_attachment_overlapping_cidr(self):
         self._test_concurrent_router_subnet_attachment_overlapping_cidr(
             ha=True)