From b4c8cc600a21469e247a0012585969a7897a0929 Mon Sep 17 00:00:00 2001 From: Terry Wilson Date: Tue, 16 Jul 2024 18:35:40 -0500 Subject: [PATCH] Actualy set global "removal limit" options Neither fdb_removal_limit nor mac_binding_removal_limit config options currently get set in the OVN DB. This patch corrects that and adds missing testing for the MAC_Binding aging maintenance task. Fixes: 0a554b4f29 ("Add support for OVN MAC_Binding aging") Fixes: 1e9f50c736 ("Add support for FDB aging") Closes-Bug: #2073309 Change-Id: I80d79faeb9f1057d398ee750ae6e246598fd13d2 --- .../conf/plugins/ml2/drivers/ovn/ovn_conf.py | 4 ++ .../ovn/mech_driver/ovsdb/maintenance.py | 9 +++- .../ovn/mech_driver/ovsdb/test_maintenance.py | 49 +++++++++++++++++++ .../ovn/mech_driver/ovsdb/test_maintenance.py | 17 +++++-- 4 files changed, 74 insertions(+), 5 deletions(-) diff --git a/neutron/conf/plugins/ml2/drivers/ovn/ovn_conf.py b/neutron/conf/plugins/ml2/drivers/ovn/ovn_conf.py index c85ce89281e..4cb13117dec 100644 --- a/neutron/conf/plugins/ml2/drivers/ovn/ovn_conf.py +++ b/neutron/conf/plugins/ml2/drivers/ovn/ovn_conf.py @@ -386,5 +386,9 @@ def get_ovn_mac_binding_age_threshold(): return str(cfg.CONF.ovn.mac_binding_age_threshold) +def get_ovn_mac_binding_removal_limit(): + return str(cfg.CONF.ovn_nb_global.mac_binding_removal_limit) + + def is_broadcast_arps_to_all_routers_enabled(): return cfg.CONF.ovn.broadcast_arps_to_all_routers diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py index 7b25e793b5d..5eda426535e 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py @@ -761,7 +761,10 @@ class DBInconsistenciesPeriodics(SchemaAwarePeriodicsBase): Ensure FDB aging settings are enforced. """ context = n_context.get_admin_context() - cmds = [] + cmds = [self._nb_idl.db_set( + "NB_Global", '.', + options={"fdb_removal_limit": + ovn_conf.get_fdb_removal_limit()})] config_fdb_age_threshold = ovn_conf.get_fdb_age_threshold() # Get provider networks @@ -792,6 +795,10 @@ class DBInconsistenciesPeriodics(SchemaAwarePeriodicsBase): def update_mac_aging_settings(self): """Ensure that MAC_Binding aging options are set""" with self._nb_idl.transaction(check_error=True) as txn: + txn.add(self._nb_idl.db_set( + "NB_Global", ".", + options={"mac_binding_removal_limit": + ovn_conf.get_ovn_mac_binding_removal_limit()})) txn.add(self._nb_idl.set_router_mac_age_limit()) raise periodics.NeverAgain() diff --git a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py index 01f919f927a..36006c33ac7 100644 --- a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py +++ b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py @@ -775,8 +775,12 @@ class TestMaintenance(_TestMaintenanceHelper): self.assertEqual( '0', ls.other_config.get(ovn_const.LS_OPTIONS_FDB_AGE_THRESHOLD)) + self.assertEqual( + '0', self.nb_api.nb_global.options.get("fdb_removal_limit", '0')) + # Change the value of the configuration cfg.CONF.set_override('fdb_age_threshold', 5, group='ovn') + cfg.CONF.set_override('fdb_removal_limit', 100, group='ovn_nb_global') # Call the maintenance task and check that the value has been # updated in the Logical Switch @@ -787,6 +791,51 @@ class TestMaintenance(_TestMaintenanceHelper): self.assertEqual( '5', ls.other_config.get(ovn_const.LS_OPTIONS_FDB_AGE_THRESHOLD)) + self.assertEqual( + '100', self.nb_api.nb_global.options.get("fdb_removal_limit")) + + def test_update_mac_aging_settings(self): + ext_net = self._create_network('ext_networktest', external=True) + ext_subnet = self._create_subnet( + 'ext_subnettest', + ext_net['id'], + **{'cidr': '100.0.0.0/24', + 'gateway_ip': '100.0.0.254', + 'allocation_pools': [ + {'start': '100.0.0.2', 'end': '100.0.0.253'}], + 'enable_dhcp': False}) + self._create_network('network1test', external=False) + external_gateway_info = { + 'enable_snat': True, + 'network_id': ext_net['id'], + 'external_fixed_ips': [ + {'ip_address': '100.0.0.2', 'subnet_id': ext_subnet['id']}]} + router = self._create_router( + 'routertest', external_gateway_info=external_gateway_info) + + options = self.nb_api.nb_global.options + lr = self.nb_api.get_lrouter(router["id"]) + + self.assertEqual( + '0', lr.options.get(ovn_const.LR_OPTIONS_MAC_AGE_LIMIT)) + + self.assertEqual('0', options.get('mac_binding_removal_limit', '0')) + + cfg.CONF.set_override("mac_binding_age_threshold", 5, group="ovn") + cfg.CONF.set_override("mac_binding_removal_limit", 100, + group="ovn_nb_global") + + # Call the maintenance task and check that the value has been + # updated in the Logical Switch + self.assertRaises(periodics.NeverAgain, + self.maint.update_mac_aging_settings) + + lr = self.nb_api.get_lrouter(router['id']) + options = self.nb_api.nb_global.options + + self.assertEqual( + '5', lr.options.get(ovn_const.LR_OPTIONS_MAC_AGE_LIMIT)) + self.assertEqual('100', options['mac_binding_removal_limit']) def test_floating_ip(self): ext_net = self._create_network('ext_networktest', external=True) diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py index 18df71821f6..123f6316eee 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py @@ -833,9 +833,13 @@ class TestDBInconsistenciesPeriodics(testlib_api.SqlTestCaseLight, periodics.NeverAgain, self.periodic.check_fdb_aging_settings) - self.fake_ovn_client._nb_idl.db_set.assert_called_once_with( - 'Logical_Switch', 'neutron-foo', - ('other_config', {constants.LS_OPTIONS_FDB_AGE_THRESHOLD: '5'})) + self.fake_ovn_client._nb_idl.db_set.assert_has_calls([ + mock.call('NB_Global', '.', + options={'fdb_removal_limit': + ovn_conf.get_fdb_removal_limit()}), + mock.call('Logical_Switch', 'neutron-foo', + ('other_config', + {constants.LS_OPTIONS_FDB_AGE_THRESHOLD: '5'}))]) def test_check_fdb_aging_settings_with_threshold_set(self): cfg.CONF.set_override('fdb_age_threshold', 5, group='ovn') @@ -850,7 +854,12 @@ class TestDBInconsistenciesPeriodics(testlib_api.SqlTestCaseLight, periodics.NeverAgain, self.periodic.check_fdb_aging_settings) - self.fake_ovn_client._nb_idl.db_set.assert_not_called() + # It doesn't really matter if db_set is called or not for the + # ls. This is called one time at startup and python-ovs will + # not send the transaction if it doesn't cause a change + self.fake_ovn_client._nb_idl.db_set.assert_called_once_with( + 'NB_Global', '.', + options={'fdb_removal_limit': ovn_conf.get_fdb_removal_limit()}) def test_remove_gw_ext_ids_from_logical_router(self): nb_idl = self.fake_ovn_client._nb_idl