Merge pull request #47 from stackhpc/upstream/yoga-2023-05-29

Synchronise yoga with upstream

Author: markgoddard

neutron/agent/linux/iptables_manager.py

@@ -304,7 +304,7 @@ class IptablesManager(object):
     # run iptables-restore without it.
     use_table_lock = False
 
-    # Flag to denote iptables supports --random-fully argument
+    # Flag to denote iptables --random-fully option enabled
     _random_fully = None
 
     def __init__(self, state_less=False, use_ipv6=False, nat=True,
@@ -495,10 +495,11 @@ def random_fully(self):
             return self._random_fully
 
         version = self._get_version()
-        self.__class__._random_fully = utils.is_version_greater_equal(
+
+        random_fully_support = utils.is_version_greater_equal(
             version, n_const.IPTABLES_RANDOM_FULLY_VERSION)
 
-        self._random_fully = self._random_fully and \
+        self.__class__._random_fully = random_fully_support and \
             cfg.CONF.AGENT.use_random_fully
 
         return self._random_fully

neutron/db/db_base_plugin_v2.py

@@ -110,11 +110,18 @@ def _update_subnetpool_dict(orig_pool, new_pool):
     return updated
 
 
+def _port_query_hook(context, original_model, query):
+    # Apply the port query only in non-admin and non-advsvc context
+    if ndb_utils.model_query_scope_is_project(context, original_model):
+        query = query.join(models_v2.Network,
+                           models_v2.Network.id == models_v2.Port.network_id)
+    return query
+
+
 def _port_filter_hook(context, original_model, conditions):
     # Apply the port filter only in non-admin and non-advsvc context
     if ndb_utils.model_query_scope_is_project(context, original_model):
         conditions |= and_(
-            models_v2.Port.network_id == models_v2.Network.id,
             models_v2.Network.project_id == context.project_id)
     return conditions
 
@@ -150,7 +157,7 @@ def __new__(cls, *args, **kwargs):
         model_query.register_hook(
             models_v2.Port,
             "port",
-            query_hook=None,
+            query_hook=_port_query_hook,
             filter_hook=_port_filter_hook,
             result_filters=None)
         return super(NeutronDbPluginV2, cls).__new__(cls, *args, **kwargs)

neutron/db/securitygroups_db.py

@@ -253,12 +253,6 @@ def delete_security_group(self, context, id):
             if sg['name'] == 'default' and not context.is_admin:
                 raise ext_sg.SecurityGroupCannotRemoveDefault()
 
-            # Check if there are rules with remote_group_id ponting to
-            # the security_group to be deleted
-            rules_ids_as_remote = self._get_security_group_rules_by_remote(
-                context=context, remote_id=id,
-            )
-
         self._registry_publish(resources.SECURITY_GROUP,
                                events.BEFORE_DELETE,
                                exc_cls=ext_sg.SecurityGroupInUse, id=id,
@@ -291,20 +285,6 @@ def delete_security_group(self, context, id):
                              context, resource_id=id, states=(sec_group,),
                              metadata={'security_group_rule_ids': sgr_ids,
                                        'name': sg['name']}))
-        for rule in rules_ids_as_remote:
-            registry.publish(
-                resources.SECURITY_GROUP_RULE,
-                events.AFTER_DELETE,
-                self,
-                payload=events.DBEventPayload(
-                    context,
-                    resource_id=rule['id'],
-                    metadata={'security_group_id': rule['security_group_id'],
-                              'remote_group_id': rule['remote_group_id'],
-                              'rule': rule
-                              }
-                )
-            )
 
     @db_api.retry_if_session_inactive()
     def update_security_group(self, context, id, security_group):
@@ -391,23 +371,6 @@ def _get_port_security_group_bindings(self, context,
             self._make_security_group_binding_dict,
             filters=filters, fields=fields)
 
-    def _get_security_group_rules_by_remote(self, context, remote_id):
-        return model_query.get_collection(
-            context, sg_models.SecurityGroupRule,
-            self._make_security_group_rule_dict,
-            filters={'remote_group_id': [remote_id]},
-            fields=['id',
-                    'remote_group_id',
-                    'security_group_id',
-                    'direction',
-                    'ethertype',
-                    'protocol',
-                    'port_range_min',
-                    'port_range_max',
-                    'normalized_cidr'
-                    ]
-        )
-
     @db_api.retry_if_session_inactive()
     def _delete_port_security_group_bindings(self, context, port_id):
         with db_api.CONTEXT_WRITER.using(context):

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

@@ -253,6 +253,9 @@ def subscribe(self):
             registry.subscribe(self._create_security_group,
                                resources.SECURITY_GROUP,
                                events.AFTER_CREATE)
+            registry.subscribe(self._delete_security_group_precommit,
+                               resources.SECURITY_GROUP,
+                               events.PRECOMMIT_DELETE)
             registry.subscribe(self._delete_security_group,
                                resources.SECURITY_GROUP,
                                events.AFTER_DELETE)
@@ -265,9 +268,6 @@ def subscribe(self):
             registry.subscribe(self._process_sg_rule_notification,
                                resources.SECURITY_GROUP_RULE,
                                events.BEFORE_DELETE)
-            registry.subscribe(self._process_sg_rule_after_del_notification,
-                               resources.SECURITY_GROUP_RULE,
-                               events.AFTER_DELETE)
 
     def _clean_hash_ring(self, *args, **kwargs):
         admin_context = n_context.get_admin_context()
@@ -384,6 +384,14 @@ def _create_security_group(self, resource, event, trigger, payload):
         self._ovn_client.create_security_group(context,
                                                security_group)
 
+    def _delete_security_group_precommit(self, resource, event, trigger,
+                                         payload):
+        context = n_context.get_admin_context()
+        security_group_id = payload.resource_id
+        for sg_rule in self._plugin.get_security_group_rules(
+                context, filters={'remote_group_id': [security_group_id]}):
+            self._ovn_client.delete_security_group_rule(context, sg_rule)
+
     def _delete_security_group(self, resource, event, trigger, payload):
         context = payload.context
         security_group_id = payload.resource_id
@@ -441,12 +449,6 @@ def _process_sg_rule_notification(
                 context,
                 sg_rule)
 
-    def _process_sg_rule_after_del_notification(
-            self, resource, event, trigger, payload):
-        context = payload.context
-        sg_rule = payload.metadata['rule']
-        self._ovn_client.delete_security_group_rule(context, sg_rule)
-
     def _sg_has_rules_with_same_normalized_cidr(self, sg_rule):
         compare_keys = [
             'ethertype', 'direction', 'protocol',

neutron/services/logapi/drivers/ovn/driver.py

@@ -142,7 +142,8 @@ def _remove_acls_log(self, pgs, ovn_txn, log_name=None):
                 if hasattr(acl, 'label'):
                     columns['label'] = 0
                     ovn_txn.add(self.ovn_nb.db_remove(
-                        "ACL", acl_uuid, 'options', 'log-related'))
+                        "ACL", acl_uuid, 'options', 'log-related',
+                        if_exists=True))
                 ovn_txn.add(self.ovn_nb.db_set(
                     "ACL", acl_uuid, *columns.items()))
                 acl_changes += 1

neutron/tests/unit/agent/linux/test_iptables_manager.py

@@ -1395,3 +1395,37 @@ def test_initialize_nat_table(self):
         iptables.initialize_nat_table()
         self.assertIn('nat', iptables.ipv4)
         self.assertIn('mangle', iptables.ipv4)
+
+
+class IptablesRandomFullyFixture(fixtures.Fixture):
+    def _setUp(self):
+        # We MUST save and restore _random_fully because it is a class
+        # attribute and could change state in some tests, which can cause
+        # the other router test cases to randomly fail due to race conditions.
+        self._random_fully = iptables_manager.IptablesManager._random_fully
+        iptables_manager.IptablesManager._random_fully = None
+        self.addCleanup(self._reset)
+
+    def _reset(self):
+        iptables_manager.IptablesManager._random_fully = self._random_fully
+
+
+class IptablesManagerDisableRandomFullyTestCase(base.BaseTestCase):
+
+    def setUp(self):
+        super(IptablesManagerDisableRandomFullyTestCase, self).setUp()
+        self.useFixture(IptablesRandomFullyFixture())
+        self.execute = mock.patch.object(linux_utils, "execute").start()
+        cfg.CONF.set_override('use_random_fully', False, "AGENT")
+
+    def test_verify_disable_random_fully(self):
+        expected_calls_and_values = [
+            (mock.call(['iptables', '--version'],
+                       run_as_root=True, privsep_exec=True),
+             "iptables v1.6.2")]
+        tools.setup_mock_calls(self.execute, expected_calls_and_values)
+        iptables_mgrs = [iptables_manager.IptablesManager() for _ in range(3)]
+        # The random_full properties of all
+        # IptablesManager instances must return False
+        for ipt_mgr in iptables_mgrs:
+            self.assertFalse(ipt_mgr.random_fully)

neutron/tests/unit/db/test_securitygroups_db.py

@@ -404,38 +404,6 @@ def test_security_group_precommit_and_after_delete_event(self):
             self.assertEqual([mock.ANY, mock.ANY],
                              payload.metadata.get('security_group_rule_ids'))
 
-    def test_security_group_rule_after_delete_event_for_remot_group(self):
-        sg1_dict = self.mixin.create_security_group(self.ctx, FAKE_SECGROUP)
-        sg2_dict = self.mixin.create_security_group(self.ctx, FAKE_SECGROUP)
-
-        fake_rule = copy.deepcopy(FAKE_SECGROUP_RULE)
-        fake_rule['security_group_rule']['security_group_id'] = sg1_dict['id']
-        fake_rule['security_group_rule']['remote_group_id'] = sg2_dict['id']
-        fake_rule['security_group_rule']['remote_ip_prefix'] = None
-        remote_rule = self.mixin.create_security_group_rule(
-            self.ctx, fake_rule)
-
-        with mock.patch.object(registry, "publish") as mock_publish:
-            self.mixin.delete_security_group(self.ctx, sg2_dict['id'])
-            mock_publish.assert_has_calls(
-                [mock.call('security_group', 'before_delete',
-                           mock.ANY, payload=mock.ANY),
-                 mock.call('security_group', 'precommit_delete',
-                           mock.ANY,
-                           payload=mock.ANY),
-                 mock.call('security_group', 'after_delete',
-                           mock.ANY,
-                           payload=mock.ANY),
-                 mock.call('security_group_rule', 'after_delete',
-                           mock.ANY,
-                           payload=mock.ANY)])
-            rule_payload = mock_publish.mock_calls[3][2]['payload']
-            self.assertEqual(remote_rule['id'], rule_payload.resource_id)
-            self.assertEqual(sg1_dict['id'],
-                             rule_payload.metadata['security_group_id'])
-            self.assertEqual(sg2_dict['id'],
-                             rule_payload.metadata['remote_group_id'])
-
     def test_security_group_rule_precommit_create_event_fail(self):
         registry.subscribe(fake_callback, resources.SECURITY_GROUP_RULE,
                            events.PRECOMMIT_CREATE)

neutron/tests/unit/services/logapi/drivers/ovn/test_driver.py

@@ -27,6 +27,7 @@
 
 FAKE_CFG_RATE = 123
 FAKE_CFG_BURST = 321
+FAKE_LABEL = 1
 
 
 class TestOVNDriverBase(base.BaseTestCase):
@@ -113,6 +114,7 @@ def __init__(self, name=None, **acl_dict):
             acl_defaults_dict = {
                 "name": [name] if name else [],
                 "action": ovn_const.ACL_ACTION_ALLOW_RELATED,
+                "label": FAKE_LABEL
             }
             self.__dict__ = {**acl_defaults_dict, **acl_dict}
 
@@ -247,6 +249,19 @@ def _mock_lookup(_pg_table, acl_uuid, default):
         self.assertEqual(len(pg_dict["acls"]) - 1,
                          self._nb_ovn.db_set.call_count)
 
+    # This test is enforcing the use of if_exists so that we don't get
+    # unexpected errors while doing parallel operations like erasing log
+    # objects and security groups
+    @mock.patch.object(ovn_driver.LOG, 'info')
+    def test__remove_acls_log_only_if_exists(self, m_info):
+        pg_dict = self._fake_pg_dict(acls=['acl1', 'acl2', 'acl3'])
+
+        def _only_if_exists(_pg_table, acl_uuid, col, val, if_exists):
+            self.assertTrue(if_exists)
+
+        self._nb_ovn.db_remove.side_effect = _only_if_exists
+        self._log_driver._remove_acls_log([pg_dict], self._nb_ovn.transaction)
+
     @mock.patch.object(ovn_driver.LOG, 'info')
     def test__remove_acls_log_with_log_name(self, m_info):
         pg_dict = self._fake_pg_dict(acls=['acl1', 'acl2', 'acl3', 'acl4'])