From b767825d1329dbb7fcff264d6047d2513d02db4e Mon Sep 17 00:00:00 2001 From: Daniel Alvarez Sanchez Date: Thu, 7 Oct 2021 09:23:21 +0200 Subject: [PATCH] [ovn] Stop monitoring the SB MAC_Binding table to reduce mem footprint The MAC_Binding table in the SB database may grow indefinitely (due to a lack of an aging mechanism of its entries) and eventually lead to OOM killers for neutron-server which maintains an in-memory copy of the database. In order to stop monitoring this table, this patch is invoking the ovsdb-client tool to remove the entries associated to Floating IPs that have just been detached. The execution of this tool is really fast as it will just invoke a JSON-RPC transact command which doesn't require downloading the database contents. In a scale test, the memory consumption of neutron-server dropped from 75GB to 7GB with this patch. Closes-Bug: #1946318 Conflicts: neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py Note: The functional test has been changed for this stable branch because the update_tables() method from ovsdbapp is not availabie for this version of Neutron. So I changed the functional test to add the MAC_Binding entry via ovsdb-client instead of using ovsdbapp to do so. Signed-off-by: Daniel Alvarez Sanchez Change-Id: Id84bf17953527c415d611bfc198038fb6f811de3 (cherry picked from commit f6c35527698119ee6f73a6a3613c9beebb563840) --- .../drivers/ovn/mech_driver/mech_driver.py | 17 +++-- .../ovn/mech_driver/ovsdb/ovsdb_monitor.py | 1 - .../mech_driver/ovsdb/test_ovsdb_monitor.py | 68 +++++++++++++------ .../ovn/mech_driver/test_mech_driver.py | 25 +++++++ 4 files changed, 84 insertions(+), 27 deletions(-) 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 94458ebcaa7..2ebecfbcb82 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py @@ -32,6 +32,7 @@ from neutron_lib import context as n_context from neutron_lib import exceptions as n_exc from neutron_lib.plugins import directory from neutron_lib.plugins.ml2 import api +from oslo_concurrency import processutils from oslo_config import cfg from oslo_db import exception as os_db_exc from oslo_log import log @@ -1054,10 +1055,18 @@ class OVNMechanismDriver(api.MechanismDriver): def delete_mac_binding_entries(self, external_ip): """Delete all MAC_Binding entries associated to this IP address""" - mac_binds = self._sb_ovn.db_find_rows( - 'MAC_Binding', ('ip', '=', external_ip)).execute() or [] - for entry in mac_binds: - self._sb_ovn.db_destroy('MAC_Binding', entry.uuid).execute() + cmd = ['ovsdb-client', 'transact', ovn_conf.get_ovn_sb_connection()] + + if ovn_conf.get_ovn_sb_private_key(): + cmd += ['-p', ovn_conf.get_ovn_sb_private_key(), '-c', + ovn_conf.get_ovn_sb_certificate(), '-C', + ovn_conf.get_ovn_sb_ca_cert()] + + cmd += ['["OVN_Southbound", {"op": "delete", "table": "MAC_Binding", ' + '"where": [["ip", "==", "%s"]]}]' % external_ip] + + return processutils.execute(*cmd, + log_errors=processutils.LOG_FINAL_ERROR) def update_segment_host_mapping(self, host, phy_nets): """Update SegmentHostMapping in DB""" diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py index 2d50d74d008..ac5a6299211 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py @@ -543,7 +543,6 @@ class OvnSbIdl(OvnIdlDistributedLock): helper.register_table('Encap') helper.register_table('Port_Binding') helper.register_table('Datapath_Binding') - helper.register_table('MAC_Binding') helper.register_table('Connection') return cls(driver, connection_string, helper) diff --git a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py index 314f7e8736e..7377db76af0 100644 --- a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py +++ b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py @@ -15,7 +15,9 @@ import mock import fixtures as og_fixtures +from oslo_concurrency import processutils from oslo_config import cfg +from oslo_serialization import jsonutils from oslo_utils import uuidutils from neutron.common.ovn import constants as ovn_const @@ -32,17 +34,6 @@ from neutron_lib.plugins import directory from ovsdbapp.backend.ovs_idl import event -class WaitForMACBindingDeleteEvent(event.WaitEvent): - event_name = 'WaitForMACBindingDeleteEvent' - - def __init__(self, entry): - table = 'MAC_Binding' - events = (self.ROW_DELETE,) - conditions = (('_uuid', '=', entry),) - super(WaitForMACBindingDeleteEvent, self).__init__( - events, table, conditions, timeout=15) - - class WaitForDataPathBindingCreateEvent(event.WaitEvent): event_name = 'WaitForDataPathBindingCreateEvent' @@ -127,6 +118,40 @@ class TestNBDbMonitor(base.TestOVNFunctionalBase): 'port_id': port['id']}}) return r1_f2 + def _check_mac_binding_exists(self, macb_id): + cmd = ['ovsdb-client', 'transact', + self.mech_driver.sb_ovn.connection_string] + + if self._ovsdb_protocol == 'ssl': + cmd += ['-p', self.ovsdb_server_mgr.private_key, '-c', + self.ovsdb_server_mgr.certificate, '-C', + self.ovsdb_server_mgr.ca_cert] + + cmd += ['["OVN_Southbound", {"op": "select", "table": "MAC_Binding", ' + '"where": [["_uuid", "==", ["uuid", "%s"]]]}]' % macb_id] + + out, _ = processutils.execute(*cmd, + log_errors=False) + return str(macb_id) in out + + def _add_mac_binding_row(self, ip, datapath): + cmd = ['ovsdb-client', 'transact', + self.mech_driver.sb_ovn.connection_string] + + if self._ovsdb_protocol == 'ssl': + cmd += ['-p', self.ovsdb_server_mgr.private_key, '-c', + self.ovsdb_server_mgr.certificate, '-C', + self.ovsdb_server_mgr.ca_cert] + + cmd += ['["OVN_Southbound", {"op": "insert", "table": "MAC_Binding", ' + '"row": {"ip": "%s", "datapath": ' + '["uuid", "%s"]}}]' % (ip, datapath)] + + out, _ = processutils.execute(*cmd, + log_errors=False) + out_parsed = jsonutils.loads(out) + return out_parsed[0]['uuid'][1] + def test_floatingip_mac_bindings(self): """Check that MAC_Binding entries are cleared on FIP add/removal @@ -148,27 +173,26 @@ class TestNBDbMonitor(base.TestOVNFunctionalBase): dp = self.sb_api.db_find( 'Datapath_Binding', ('external_ids', '=', {'name2': net_name})).execute() - macb_id = self.sb_api.db_create('MAC_Binding', datapath=dp[0]['_uuid'], - ip='100.0.0.21').execute() + macb_id = self._add_mac_binding_row( + ip='100.0.0.21', datapath=dp[0]['_uuid']) port = self.create_port() # Ensure that the MAC_Binding entry gets deleted after creating a FIP - row_event = WaitForMACBindingDeleteEvent(macb_id) - self.mech_driver._sb_ovn.idl.notify_handler.watch_event(row_event) fip = self._create_fip(port, '100.0.0.21') - self.assertTrue(row_event.wait()) + n_utils.wait_until_true( + lambda: not self._check_mac_binding_exists(macb_id), + timeout=15, sleep=1) # Now that the FIP is created, add a new MAC_Binding entry with the # same IP address - - macb_id = self.sb_api.db_create('MAC_Binding', datapath=dp[0]['_uuid'], - ip='100.0.0.21').execute() + macb_id = self._add_mac_binding_row( + ip='100.0.0.21', datapath=dp[0]['_uuid']) # Ensure that the MAC_Binding entry gets deleted after deleting the FIP - row_event = WaitForMACBindingDeleteEvent(macb_id) - self.mech_driver._sb_ovn.idl.notify_handler.watch_event(row_event) self.l3_plugin.delete_floatingip(self.context, fip['id']) - self.assertTrue(row_event.wait()) + n_utils.wait_until_true( + lambda: not self._check_mac_binding_exists(macb_id), + timeout=15, sleep=1) def _test_port_binding_and_status(self, port_id, action, status): # This function binds or unbinds port to chassis and 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 8e4ece29f53..83528059a70 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 @@ -14,6 +14,7 @@ import copy import datetime +import shlex import uuid import mock @@ -30,6 +31,7 @@ from neutron_lib import exceptions as n_exc from neutron_lib.plugins import directory from neutron_lib.tests import tools from neutron_lib.utils import net as n_net +from oslo_concurrency import processutils from oslo_config import cfg from oslo_db import exception as os_db_exc from oslo_serialization import jsonutils @@ -112,6 +114,29 @@ class TestOVNMechanismDriver(test_plugin.Ml2PluginV2TestCase): p.start() self.addCleanup(p.stop) + def test_delete_mac_binding_entries(self): + self.config(group='ovn', ovn_sb_private_key=None) + expected = ('ovsdb-client transact tcp:127.0.0.1:6642 ' + '\'["OVN_Southbound", {"op": "delete", "table": ' + '"MAC_Binding", "where": [["ip", "==", "1.1.1.1"]]}]\'') + with mock.patch.object(processutils, 'execute') as mock_execute: + self.mech_driver.delete_mac_binding_entries('1.1.1.1') + mock_execute.assert_called_once_with(*shlex.split(expected), + log_errors=processutils.LOG_FINAL_ERROR) + + def test_delete_mac_binding_entries_ssl(self): + self.config(group='ovn', ovn_sb_private_key='pk') + self.config(group='ovn', ovn_sb_certificate='cert') + self.config(group='ovn', ovn_sb_ca_cert='ca') + expected = ('ovsdb-client transact tcp:127.0.0.1:6642 ' + '-p pk -c cert -C ca ' + '\'["OVN_Southbound", {"op": "delete", "table": ' + '"MAC_Binding", "where": [["ip", "==", "1.1.1.1"]]}]\'') + with mock.patch.object(processutils, 'execute') as mock_execute: + self.mech_driver.delete_mac_binding_entries('1.1.1.1') + mock_execute.assert_called_once_with(*shlex.split(expected), + log_errors=processutils.LOG_FINAL_ERROR) + @mock.patch.object(ovsdb_monitor.OvnInitPGNbIdl, 'from_server') @mock.patch.object(ovsdb_monitor, 'short_living_ovsdb_api') def test__create_neutron_pg_drop_non_existing(