From 3abce4c6d88995083cf0ac4217cd1adb680526c0 Mon Sep 17 00:00:00 2001 From: Ihar Hrachyshka Date: Fri, 30 Aug 2024 13:52:47 -0400 Subject: [PATCH] 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 | 16 ++- .../conf/plugins/ml2/drivers/ovn/ovn_conf.py | 9 -- .../drivers/ovn/mech_driver/mech_driver.py | 15 +-- .../ovn/mech_driver/ovsdb/ovn_client.py | 11 +-- .../ovn/mech_driver/ovsdb/ovn_db_sync.py | 29 +++--- .../ovn/mech_driver/test_mech_driver.py | 97 +++++++------------ ...ported-config-option-8d85a4bed0f04e76.yaml | 6 ++ 7 files changed, 61 insertions(+), 122 deletions(-) create mode 100644 releasenotes/notes/remove-allow_stateless_action_supported-config-option-8d85a4bed0f04e76.yaml diff --git a/neutron/common/ovn/acl.py b/neutron/common/ovn/acl.py index d1309c61144..07a7aeb9c49 100644 --- a/neutron/common/ovn/acl.py +++ b/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): - if stateless_supported: - return sg.get("stateful", True) - return True +def is_sg_stateful(sg): + return sg.get("stateful", True) -def add_acls_for_sg_port_group(ovn, security_group, txn, - stateless_supported=True): - stateful = is_sg_stateful(security_group, stateless_supported) +def add_acls_for_sg_port_group(ovn, security_group, txn): + stateful = is_sg_stateful(security_group) 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, - stateless_supported=True): + is_add_acl=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), diff --git a/neutron/conf/plugins/ml2/drivers/ovn/ovn_conf.py b/neutron/conf/plugins/ml2/drivers/ovn/ovn_conf.py index 3e4f622d91f..d55a494a709 100644 --- a/neutron/conf/plugins/ml2/drivers/ovn/ovn_conf.py +++ b/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 ' diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py index d7d6319a193..87ac21454c4 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py +++ b/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) - if not cfg.CONF.ovn.allow_stateless_action_supported: - supported_extensions.discard(stateful_security_group.ALIAS) - return set(supported_extensions) & extensions + return set(ovn_extensions.ML2_SUPPORTED_API_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 = ( - self._ovn_client.is_allow_stateless_supported() - ) - 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) + old_stateful = ovn_acl.is_sg_stateful(old_state) + new_stateful = ovn_acl.is_sg_stateful(new_state) if old_stateful != new_stateful: for rule in self._plugin.get_security_group_rules( context, {'security_group_id': [security_group['id']]}): diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py index c6c3c3a2c36..f10ea71c634 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py +++ b/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.is_allow_stateless_supported()) + self._nb_idl, security_group, txn) 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, - stateless_supported=self.is_allow_stateless_supported()) + is_add_acl=is_add_acl) def create_security_group_rule(self, context, rule): self._process_security_group_rule(rule) diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_db_sync.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_db_sync.py index 5d8ae1d3f21..9f1f02f8349 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_db_sync.py +++ b/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 - # stateful is set - if self._ovn_client.is_allow_stateless_supported(): - for sg in self.core_plugin.get_security_groups(ctx): - stateful = sg.get("stateful", True) - pg_name = utils.ovn_port_group_name(sg['id']) - for sgr in self.core_plugin.get_security_group_rules( - ctx, {'security_group_id': sg['id']}): - neutron_acls.append( - 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)) + # we have to fetch groups to determine if stateful is set + for sg in self.core_plugin.get_security_groups(ctx): + stateful = sg.get("stateful", True) + pg_name = utils.ovn_port_group_name(sg['id']) + for sgr in self.core_plugin.get_security_group_rules( + ctx, {'security_group_id': sg['id']}): + neutron_acls.append( + acl_utils._add_sg_rule_acl_for_port_group( + pg_name, stateful, 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]) diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py index ab5ebc65277..99daeff1095 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py +++ b/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( - self, stateful, stateless_supported, mock_bump): + def _test__create_security_group(self, stateful, mock_bump): self.fake_sg["stateful"] = stateful - with mock.patch.object(self.mech_driver._ovn_client, - 'is_allow_stateless_supported', - return_value=stateless_supported): - self.mech_driver._create_security_group( - resources.SECURITY_GROUP, events.AFTER_CREATE, {}, - payload=events.DBEventPayload( - self.context, states=(self.fake_sg,))) + self.mech_driver._create_security_group( + 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): - self._test__create_security_group(True, True) + def test__create_security_group_stateful(self): + self._test__create_security_group(True) - def test__create_security_group_stateful_not_supported(self): - self._test__create_security_group(True, 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) + def test__create_security_group_stateless(self): + self._test__create_security_group(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( - self, stateful, stateless_supported, mock_del, mock_bump): + def _test__update_security_group(self, stateful, 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, - 'is_allow_stateless_supported', - return_value=stateless_supported), \ - mock.patch.object(securitygroups_db.SecurityGroupDbMixin, + with 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, - # otherwise will just bump the revision on the SG. - bump_calls = [] - if stateless_supported: - acl_calls = [ - mock.call(mock.ANY, mock.ANY, mock.ANY, - 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) + # we will update the SG rules + ovn_acl_up.assert_has_calls([ + mock.call(mock.ANY, mock.ANY, mock.ANY, + rule['security_group_id'], rule, is_add_acl=False), + mock.call(mock.ANY, mock.ANY, mock.ANY, + rule['security_group_id'], rule, is_add_acl=True) + ]) - mock_del.assert_called_once_with( - mock.ANY, rule['id'], ovn_const.TYPE_SECURITY_GROUP_RULES) + mock_del.assert_called_once_with( + mock.ANY, rule['id'], ovn_const.TYPE_SECURITY_GROUP_RULES) - bump_calls.extend([ - mock.call(mock.ANY, rule, - ovn_const.TYPE_SECURITY_GROUP_RULES)]) + mock_bump.assert_has_calls([ + mock.call(mock.ANY, rule, ovn_const.TYPE_SECURITY_GROUP_RULES), + mock.call(mock.ANY, fake_sg_update, + ovn_const.TYPE_SECURITY_GROUPS), + ]) - bump_calls.extend([ - mock.call(mock.ANY, fake_sg_update, - ovn_const.TYPE_SECURITY_GROUPS)]) - mock_bump.assert_has_calls(bump_calls) + def test__update_security_group_stateful(self): + self._test__update_security_group(True) - def test__update_security_group_stateful_supported(self): - self._test__update_security_group(True, True) - - 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) + def test__update_security_group_stateless(self): + self._test__update_security_group(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, - 'sg_id', rule, is_add_acl=True, stateless_supported=False) + mock.ANY, mock.ANY, mock.ANY, 'sg_id', rule, is_add_acl=True) 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, - 'sg_id', rule, is_add_acl=True, stateless_supported=False) + mock.ANY, mock.ANY, mock.ANY, 'sg_id', rule, is_add_acl=True) 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, - 'sg_id', rule, is_add_acl=False, stateless_supported=False) + mock.ANY, mock.ANY, mock.ANY, 'sg_id', rule, is_add_acl=False) mock_delrev.assert_called_once_with( mock.ANY, rule['id'], ovn_const.TYPE_SECURITY_GROUP_RULES) diff --git a/releasenotes/notes/remove-allow_stateless_action_supported-config-option-8d85a4bed0f04e76.yaml b/releasenotes/notes/remove-allow_stateless_action_supported-config-option-8d85a4bed0f04e76.yaml new file mode 100644 index 00000000000..77863444da5 --- /dev/null +++ b/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.