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

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

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()
 

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)

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