ovn: Clean up code to handle old versions with no stateless acl support

It's reasonable to expect ovn 21.06+ used with master neutron.

Change-Id: I421d702f38d483e498bc65e968d40ed5fae4640e

neutron/common/ovn/acl.py

@@ -261,15 +261,12 @@ def _acl_columns_name_severity_supported(nb_idl):
     return ('name' in columns) and ('severity' in columns)
 
 
-def is_sg_stateful(sg, stateless_supported):
+def is_sg_stateful(sg):
-    if stateless_supported:
+    return sg.get("stateful", True)
-        return sg.get("stateful", True)
-    return True
 
 
-def add_acls_for_sg_port_group(ovn, security_group, txn,
+def add_acls_for_sg_port_group(ovn, security_group, txn):
-                               stateless_supported=True):
+    stateful = is_sg_stateful(security_group)
-    stateful = is_sg_stateful(security_group, stateless_supported)
     for r in security_group['security_group_rules']:
         acl = _add_sg_rule_acl_for_port_group(
             utils.ovn_port_group_name(security_group['id']), stateful, r)
@@ -281,8 +278,7 @@ def update_acls_for_security_group(plugin,
                                    ovn,
                                    security_group_id,
                                    security_group_rule,
-                                   is_add_acl=True,
+                                   is_add_acl=True):
-                                   stateless_supported=True):
 
     # Skip ACLs if security groups aren't enabled
     if not is_sg_enabled():
@@ -292,7 +288,7 @@ def update_acls_for_security_group(plugin,
     keep_name_severity = _acl_columns_name_severity_supported(ovn)
 
     sg = plugin.get_security_group(admin_context, security_group_id)
-    stateful = is_sg_stateful(sg, stateless_supported)
+    stateful = is_sg_stateful(sg)
 
     acl = _add_sg_rule_acl_for_port_group(
         utils.ovn_port_group_name(security_group_id),

neutron/conf/plugins/ml2/drivers/ovn/ovn_conf.py

@@ -194,15 +194,6 @@ ovn_opts = [
                        'provisioning over IPv6 this option should be set '
                        'to "True" and neutron-dhcp-agent should be used '
                        'instead. Defaults to "False".')),
-    cfg.BoolOpt('allow_stateless_action_supported',
-                default=True,
-                deprecated_for_removal=True,
-                deprecated_since="2023.1",
-                help=_('If OVN older than 21.06 is used together with '
-                       'Neutron, this option should be set to ``False`` in '
-                       'order to disable the ``stateful-security-group`` API '
-                       'extension as ``allow-stateless`` keyword is only '
-                       'supported by OVN >= 21.06.')),
     cfg.BoolOpt('localnet_learn_fdb',
                 default=False,
                 help=_('If enabled it will allow localnet ports to learn MAC '

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

@@ -26,7 +26,6 @@ import uuid
 from neutron_lib.api.definitions import portbindings
 from neutron_lib.api.definitions import provider_net
 from neutron_lib.api.definitions import segment as segment_def
-from neutron_lib.api.definitions import stateful_security_group
 from neutron_lib.callbacks import events
 from neutron_lib.callbacks import registry
 from neutron_lib.callbacks import resources
@@ -230,10 +229,7 @@ class OVNMechanismDriver(api.MechanismDriver):
         return portbindings.CONNECTIVITY_L2
 
     def supported_extensions(self, extensions):
-        supported_extensions = set(ovn_extensions.ML2_SUPPORTED_API_EXTENSIONS)
+        return set(ovn_extensions.ML2_SUPPORTED_API_EXTENSIONS) & extensions
-        if not cfg.CONF.ovn.allow_stateless_action_supported:
-            supported_extensions.discard(stateful_security_group.ALIAS)
-        return set(supported_extensions) & extensions
 
     @staticmethod
     def provider_network_attribute_updates_supported():
@@ -430,13 +426,8 @@ class OVNMechanismDriver(api.MechanismDriver):
         security_group = payload.latest_state
 
         old_state, new_state = payload.states
-        is_allow_stateless_supported = (
+        old_stateful = ovn_acl.is_sg_stateful(old_state)
-            self._ovn_client.is_allow_stateless_supported()
+        new_stateful = ovn_acl.is_sg_stateful(new_state)
-        )
-        old_stateful = ovn_acl.is_sg_stateful(
-            old_state, is_allow_stateless_supported)
-        new_stateful = ovn_acl.is_sg_stateful(
-            new_state, is_allow_stateless_supported)
         if old_stateful != new_stateful:
             for rule in self._plugin.get_security_group_rules(
                     context, {'security_group_id': [security_group['id']]}):

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

@@ -142,11 +142,6 @@ class OVNClient(object):
         return self._nb_idl.is_col_present(
             'Logical_Switch_Port', 'ha_chassis_group')
 
-    # TODO(ihrachys) remove when min OVN version >= 21.06
-    def is_allow_stateless_supported(self):
-        return self._nb_idl.is_col_supports_value('ACL', 'action',
-                                                  'allow-stateless')
-
     def _get_allowed_addresses_from_port(self, port):
         if not port.get(psec.PORTSECURITY):
             return [], []
@@ -2539,8 +2534,7 @@ class OVNClient(object):
             # When a SG is created, it comes with some default rules,
             # so we'll apply them to the Port Group.
             ovn_acl.add_acls_for_sg_port_group(
-                self._nb_idl, security_group, txn,
+                self._nb_idl, security_group, txn)
-                self.is_allow_stateless_supported())
         db_rev.bump_revision(
             context, security_group, ovn_const.TYPE_SECURITY_GROUPS)
 
@@ -2565,8 +2559,7 @@ class OVNClient(object):
         ovn_acl.update_acls_for_security_group(
             self._plugin, admin_context, self._nb_idl,
             rule['security_group_id'], rule,
-            is_add_acl=is_add_acl,
+            is_add_acl=is_add_acl)
-            stateless_supported=self.is_allow_stateless_supported())
 
     def create_security_group_rule(self, context, rule):
         self._process_security_group_rule(rule)

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

@@ -260,24 +260,17 @@ class OvnNbSynchronizer(OvnDbSynchronizer):
         LOG.debug('OVN-NB Sync ACLs started @ %s', str(datetime.now()))
 
         neutron_acls = []
-        # if allow-stateless supported, we have to fetch groups to determine if
+        # we have to fetch groups to determine if stateful is set
-        # stateful is set
+        for sg in self.core_plugin.get_security_groups(ctx):
-        if self._ovn_client.is_allow_stateless_supported():
+            stateful = sg.get("stateful", True)
-            for sg in self.core_plugin.get_security_groups(ctx):
+            pg_name = utils.ovn_port_group_name(sg['id'])
-                stateful = sg.get("stateful", True)
+            for sgr in self.core_plugin.get_security_group_rules(
-                pg_name = utils.ovn_port_group_name(sg['id'])
+                    ctx, {'security_group_id': sg['id']}):
-                for sgr in self.core_plugin.get_security_group_rules(
+                neutron_acls.append(
-                        ctx, {'security_group_id': sg['id']}):
+                    acl_utils._add_sg_rule_acl_for_port_group(
-                    neutron_acls.append(
+                        pg_name, stateful, sgr)
-                        acl_utils._add_sg_rule_acl_for_port_group(
+                )
-                            pg_name, stateful, sgr)
+
-                    )
-        else:
-            # TODO(ihrachys) remove when min OVN version >= 21.06
-            for sgr in self.core_plugin.get_security_group_rules(ctx):
-                pg_name = utils.ovn_port_group_name(sgr['security_group_id'])
-                neutron_acls.append(acl_utils._add_sg_rule_acl_for_port_group(
-                    pg_name, True, sgr))
         # Sort the acls in the Neutron database according to the security
         # group rule ID for easy comparison in the future.
         neutron_acls.sort(key=lambda x: x[ovn_const.OVN_SG_RULE_EXT_ID_KEY])

neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py

@@ -270,23 +270,19 @@ class TestOVNMechanismDriver(TestOVNMechanismDriverBase):
             self._test__validate_network_segments_id_succeed, 300)
 
     @mock.patch.object(ovn_revision_numbers_db, 'bump_revision')
-    def _test__create_security_group(
+    def _test__create_security_group(self, stateful, mock_bump):
-            self, stateful, stateless_supported, mock_bump):
         self.fake_sg["stateful"] = stateful
-        with mock.patch.object(self.mech_driver._ovn_client,
+        self.mech_driver._create_security_group(
-                               'is_allow_stateless_supported',
+            resources.SECURITY_GROUP, events.AFTER_CREATE, {},
-                               return_value=stateless_supported):
+            payload=events.DBEventPayload(
-            self.mech_driver._create_security_group(
+                self.context, states=(self.fake_sg,)))
-                resources.SECURITY_GROUP, events.AFTER_CREATE, {},
-                payload=events.DBEventPayload(
-                    self.context, states=(self.fake_sg,)))
         external_ids = {ovn_const.OVN_SG_EXT_ID_KEY: self.fake_sg['id']}
         pg_name = ovn_utils.ovn_port_group_name(self.fake_sg['id'])
 
         self.nb_ovn.pg_add.assert_called_once_with(
             name=pg_name, acls=[], external_ids=external_ids)
 
-        if stateful or not stateless_supported:
+        if stateful:
             expected = ovn_const.ACL_ACTION_ALLOW_RELATED
         else:
             expected = ovn_const.ACL_ACTION_ALLOW_STATELESS
@@ -296,17 +292,11 @@ class TestOVNMechanismDriver(TestOVNMechanismDriverBase):
         mock_bump.assert_called_once_with(
             mock.ANY, self.fake_sg, ovn_const.TYPE_SECURITY_GROUPS)
 
-    def test__create_security_group_stateful_supported(self):
+    def test__create_security_group_stateful(self):
-        self._test__create_security_group(True, True)
+        self._test__create_security_group(True)
 
-    def test__create_security_group_stateful_not_supported(self):
+    def test__create_security_group_stateless(self):
-        self._test__create_security_group(True, False)
+        self._test__create_security_group(False)
-
-    def test__create_security_group_stateless_supported(self):
-        self._test__create_security_group(False, True)
-
-    def test__create_security_group_stateless_not_supported(self):
-        self._test__create_security_group(False, False)
 
     @mock.patch.object(ovn_revision_numbers_db, 'delete_revision')
     def test__delete_security_group(self, mock_del_rev):
@@ -326,16 +316,12 @@ class TestOVNMechanismDriver(TestOVNMechanismDriverBase):
 
     @mock.patch.object(ovn_revision_numbers_db, 'bump_revision')
     @mock.patch.object(ovn_revision_numbers_db, 'delete_revision')
-    def _test__update_security_group(
+    def _test__update_security_group(self, stateful, mock_del, mock_bump):
-            self, stateful, stateless_supported, mock_del, mock_bump):
         self.fake_sg['stateful'] = stateful
         fake_sg_update = copy.deepcopy(self.fake_sg)
         fake_sg_update['stateful'] = not stateful
         rule = fake_sg_update['security_group_rules'][0]
-        with mock.patch.object(self.mech_driver._ovn_client,
+        with mock.patch.object(securitygroups_db.SecurityGroupDbMixin,
-                               'is_allow_stateless_supported',
-                               return_value=stateless_supported), \
-                mock.patch.object(securitygroups_db.SecurityGroupDbMixin,
                                'get_security_group_rules',
                                return_value=[rule]), \
                 mock.patch.object(
@@ -347,42 +333,28 @@ class TestOVNMechanismDriver(TestOVNMechanismDriverBase):
                     states=(self.fake_sg, fake_sg_update),
                     resource_id=self.fake_sg['id']))
 
-            # When stateless is supported we will update the SG rules,
+            # we will update the SG rules
-            # otherwise will just bump the revision on the SG.
+            ovn_acl_up.assert_has_calls([
-            bump_calls = []
+                mock.call(mock.ANY, mock.ANY, mock.ANY,
-            if stateless_supported:
+                    rule['security_group_id'], rule, is_add_acl=False),
-                acl_calls = [
+                mock.call(mock.ANY, mock.ANY, mock.ANY,
-                    mock.call(mock.ANY, mock.ANY, mock.ANY,
+                    rule['security_group_id'], rule, is_add_acl=True)
-                        rule['security_group_id'], rule, is_add_acl=False,
+            ])
-                        stateless_supported=stateless_supported),
-                    mock.call(mock.ANY, mock.ANY, mock.ANY,
-                        rule['security_group_id'], rule, is_add_acl=True,
-                        stateless_supported=stateless_supported)]
-                ovn_acl_up.assert_has_calls(acl_calls)
 
-                mock_del.assert_called_once_with(
+            mock_del.assert_called_once_with(
-                    mock.ANY, rule['id'], ovn_const.TYPE_SECURITY_GROUP_RULES)
+                mock.ANY, rule['id'], ovn_const.TYPE_SECURITY_GROUP_RULES)
 
-                bump_calls.extend([
+            mock_bump.assert_has_calls([
-                    mock.call(mock.ANY, rule,
+                mock.call(mock.ANY, rule, ovn_const.TYPE_SECURITY_GROUP_RULES),
-                        ovn_const.TYPE_SECURITY_GROUP_RULES)])
+                mock.call(mock.ANY, fake_sg_update,
+                          ovn_const.TYPE_SECURITY_GROUPS),
+            ])
 
-            bump_calls.extend([
+    def test__update_security_group_stateful(self):
-                 mock.call(mock.ANY, fake_sg_update,
+        self._test__update_security_group(True)
-                          ovn_const.TYPE_SECURITY_GROUPS)])
-            mock_bump.assert_has_calls(bump_calls)
 
-    def test__update_security_group_stateful_supported(self):
+    def test__update_security_group_stateless(self):
-        self._test__update_security_group(True, True)
+        self._test__update_security_group(False)
-
-    def test__update_security_group_stateful_not_supported(self):
-        self._test__update_security_group(True, False)
-
-    def test__update_security_group_stateless_supported(self):
-        self._test__update_security_group(False, True)
-
-    def test__update_security_group_stateless_not_supported(self):
-        self._test__update_security_group(False, False)
 
     @mock.patch.object(ovn_revision_numbers_db, 'bump_revision')
     def test__process_sg_rule_notifications_sgr_create(self, mock_bump):
@@ -398,8 +370,7 @@ class TestOVNMechanismDriver(TestOVNMechanismDriverBase):
                     self.context, states=(rule,)))
             has_same_rules.assert_not_called()
             ovn_acl_up.assert_called_once_with(
-                mock.ANY, mock.ANY, mock.ANY,
+                mock.ANY, mock.ANY, mock.ANY, 'sg_id', rule, is_add_acl=True)
-                'sg_id', rule, is_add_acl=True, stateless_supported=False)
             mock_bump.assert_called_once_with(
                 mock.ANY, rule, ovn_const.TYPE_SECURITY_GROUP_RULES)
 
@@ -419,8 +390,7 @@ class TestOVNMechanismDriver(TestOVNMechanismDriverBase):
                     self.context, states=(rule,)))
             has_same_rules.assert_not_called()
             ovn_acl_up.assert_called_once_with(
-                mock.ANY, mock.ANY, mock.ANY,
+                mock.ANY, mock.ANY, mock.ANY, 'sg_id', rule, is_add_acl=True)
-                'sg_id', rule, is_add_acl=True, stateless_supported=False)
             mock_bump.assert_called_once_with(
                 mock.ANY, rule, ovn_const.TYPE_SECURITY_GROUP_RULES)
 
@@ -437,8 +407,7 @@ class TestOVNMechanismDriver(TestOVNMechanismDriverBase):
                 payload=events.DBEventPayload(
                     self.context, states=(rule,)))
             ovn_acl_up.assert_called_once_with(
-                mock.ANY, mock.ANY, mock.ANY,
+                mock.ANY, mock.ANY, mock.ANY, 'sg_id', rule, is_add_acl=False)
-                'sg_id', rule, is_add_acl=False, stateless_supported=False)
             mock_delrev.assert_called_once_with(
                 mock.ANY, rule['id'], ovn_const.TYPE_SECURITY_GROUP_RULES)
 

releasenotes/notes/remove-allow_stateless_action_supported-config-option-8d85a4bed0f04e76.yaml

@@ -0,0 +1,6 @@
+---
+upgrade:
+  - |
+    The ``allow_stateless_action_supported`` configuration option for OVN is
+    removed. The ``stateful-security-group`` API is now unconditionally
+    enabled. Please upgrade OVN to 21.06.0 or a later version.