[OVN] Update ovn meter when neutron server reloads

Up until now, we needed to remove all logging objects to see the
meter-band properties being changed after a server restart. Now we check
for inconsistencies between the neutron configuration and the OVN
meter-band object after a restart. The function create_ovn_fair_meter is
now located in the ovn_driver instead of the log_driver so as to be able
to call it from the maintenance task.

Closes-bug: #2017145
Signed-off-by: Elvira García <egarciar@redhat.com>
Change-Id: I24cef85ed68c893a740445707f88296d763c8de8
This commit is contained in:
Elvira García 2023-04-20 16:39:12 +02:00
parent 232a67f444
commit c3602ac19b
6 changed files with 181 additions and 102 deletions

View File

@ -41,6 +41,7 @@ from neutron.db import ovn_revision_numbers_db as revision_numbers_db
from neutron.objects import ports as ports_obj
from neutron.objects import router as router_obj
from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import ovn_db_sync
from neutron.services.logapi.drivers.ovn import driver as log_driver
CONF = cfg.CONF
@ -1016,6 +1017,24 @@ class DBInconsistenciesPeriodics(SchemaAwarePeriodicsBase):
raise periodics.NeverAgain()
@periodics.periodic(spacing=600, run_immediately=True)
def check_fair_meter_consistency(self):
"""Update the logging meter after neutron-server reload
When we change the rate and burst limit we need to update the fair
meter band to apply the new values. This is called from the ML2/OVN
driver after the OVN NB idl is loaded
"""
if not self.has_lock:
return
if log_driver.OVNDriver.network_logging_supported(self._nb_idl):
meter_name = (
cfg.CONF.network_log.local_output_log_base or "acl_log_meter")
self._ovn_client.create_ovn_fair_meter(meter_name,
from_reload=True)
raise periodics.NeverAgain()
class HashRingHealthCheckPeriodics(object):

View File

@ -31,6 +31,7 @@ from neutron_lib.exceptions import l3 as l3_exc
from neutron_lib.plugins import constants as plugin_constants
from neutron_lib.plugins import directory
from neutron_lib.plugins import utils as p_utils
from neutron_lib.services.logapi import constants as log_const
from neutron_lib.utils import helpers
from neutron_lib.utils import net as n_net
from oslo_config import cfg
@ -2647,3 +2648,54 @@ class OVNClient(object):
if ls_dns_record.records.get(ptr_record):
txn.add(self._nb_idl.dns_remove_record(
ls_dns_record.uuid, ptr_record, if_exists=True))
def create_ovn_fair_meter(self, meter_name, from_reload=False, txn=None):
"""Create row in Meter table with fair attribute set to True.
Create a row in OVN's NB Meter table based on well-known name. This
method uses the network_log configuration to specify the attributes
of the meter. Current implementation needs only one 'fair' meter row
which is then referred by multiple ACL rows.
:param meter_name: ovn northbound meter name.
:param from_reload: whether we update the meter values or create them.
:txn: ovn northbound idl transaction.
"""
meter = self._nb_idl.db_find_rows(
"Meter", ("name", "=", meter_name)).execute(check_error=True)
# The meter is created when a log object is created, not by default.
# This condition avoids creating the meter if it wasn't there already
commands = []
if from_reload and not meter:
return
if meter:
meter = meter[0]
meter_band = self._nb_idl.lookup("Meter_Band",
meter.bands[0].uuid, default=None)
if meter_band:
if all((meter.unit == "pktps",
meter.fair[0],
meter_band.rate == cfg.CONF.network_log.rate_limit,
meter_band.burst_size ==
cfg.CONF.network_log.burst_limit)):
# Meter (and its meter-band) unchanged: noop.
return
# Re-create meter (and its meter-band) with the new attributes.
# This is supposed to happen only if configuration changed, so
# doing updates is an overkill: better to leverage the ovsdbapp
# library to avoid the complexity.
LOG.info("Deleting outdated log fair meter %s", meter_name)
commands.append(self._nb_idl.meter_del(meter.uuid))
# Create meter
LOG.info("Creating network log fair meter %s", meter_name)
commands.append(self._nb_idl.meter_add(
name=meter_name,
unit="pktps",
rate=cfg.CONF.network_log.rate_limit,
fair=True,
burst_size=cfg.CONF.network_log.burst_limit,
may_exist=False,
external_ids={ovn_const.OVN_DEVICE_OWNER_EXT_ID_KEY:
log_const.LOGGING_PLUGIN}))
self._transaction(commands, txn=txn)

View File

@ -89,54 +89,14 @@ class OVNDriver(base.DriverBase):
log_objs = self._log_plugin.get_logs(context)
return [self._log_dict_to_obj(lo) for lo in log_objs]
@property
def _ovn_client(self):
return self.plugin_driver._ovn_client
@property
def ovn_nb(self):
return self.plugin_driver.nb_ovn
def _create_ovn_fair_meter(self, ovn_txn):
"""Create row in Meter table with fair attribute set to True.
Create a row in OVN's NB Meter table based on well-known name. This
method uses the network_log configuration to specify the attributes
of the meter. Current implementation needs only one 'fair' meter row
which is then referred by multiple ACL rows.
:param ovn_txn: ovn northbound idl transaction.
"""
meter = self.ovn_nb.db_find_rows(
"Meter", ("name", "=", self.meter_name)).execute(check_error=True)
if meter:
meter = meter[0]
try:
meter_band = self.ovn_nb.lookup("Meter_Band",
meter.bands[0].uuid)
if all((meter.unit == "pktps",
meter.fair[0],
meter_band.rate == cfg.CONF.network_log.rate_limit,
meter_band.burst_size ==
cfg.CONF.network_log.burst_limit)):
# Meter (and its meter-band) unchanged: noop.
return
except idlutils.RowNotFound:
pass
# Re-create meter (and its meter-band) with the new attributes.
# This is supposed to happen only if configuration changed, so
# doing updates is an overkill: better to leverage the ovsdbapp
# library to avoid the complexity.
ovn_txn.add(self.ovn_nb.meter_del(meter.uuid))
# Create meter
LOG.info("Creating network log fair meter %s", self.meter_name)
ovn_txn.add(self.ovn_nb.meter_add(
name=self.meter_name,
unit="pktps",
rate=cfg.CONF.network_log.rate_limit,
fair=True,
burst_size=cfg.CONF.network_log.burst_limit,
may_exist=False,
external_ids={ovn_const.OVN_DEVICE_OWNER_EXT_ID_KEY:
log_const.LOGGING_PLUGIN}))
@staticmethod
def _acl_actions_enabled(log_obj):
if not log_obj.enabled:
@ -303,7 +263,8 @@ class OVNDriver(base.DriverBase):
pgs = self._pgs_from_log_obj(context, log_obj)
actions_enabled = self._acl_actions_enabled(log_obj)
with self.ovn_nb.transaction(check_error=True) as ovn_txn:
self._create_ovn_fair_meter(ovn_txn)
self._ovn_client.create_ovn_fair_meter(self.meter_name,
txn=ovn_txn)
self._set_acls_log(pgs, ovn_txn, actions_enabled,
utils.ovn_name(log_obj.id))

View File

@ -30,9 +30,15 @@ from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf as ovn_config
from neutron.db import ovn_revision_numbers_db as db_rev
from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import maintenance
from neutron.tests.functional import base
from neutron.tests.functional.services.logapi.drivers.ovn \
import test_driver as test_log_driver
from neutron.tests.unit.api import test_extensions
from neutron.tests.unit.extensions import test_extraroute
CFG_NEW_BURST = 50
CFG_NEW_RATE = 150
class _TestMaintenanceHelper(base.TestOVNFunctionalBase):
"""A helper class to keep the code more organized."""
@ -942,3 +948,37 @@ class TestMaintenance(_TestMaintenanceHelper):
# Assert load balancer for port forwarding is gone
self.assertFalse(self._find_pf_lb(router_id, fip_id))
class TestLogMaintenance(_TestMaintenanceHelper,
test_log_driver.LogApiTestCaseBase):
def test_check_for_logging_conf_change(self):
# Check logging is supported
if not self.log_driver.network_logging_supported(self.nb_api):
self.skipTest("The current OVN version does not offer support "
"for neutron network log functionality.")
self.assertIsNotNone(self.log_plugin)
# Check no meter exists
self.assertFalse(self.nb_api._tables['Meter'].rows.values())
# Add a log object
self.log_plugin.create_log(self.context, self._log_data())
# Check a meter and fair meter exist
self.assertTrue(self.nb_api._tables['Meter'].rows)
self.assertTrue(self.nb_api._tables['Meter_Band'].rows)
self.assertEqual(cfg.CONF.network_log.burst_limit,
[*self.nb_api._tables['Meter_Band'].rows.values()][0].burst_size)
self.assertEqual(cfg.CONF.network_log.rate_limit,
[*self.nb_api._tables['Meter_Band'].rows.values()][0].rate)
# Update burst and rate limit values on the configuration
ovn_config.cfg.CONF.set_override('burst_limit', CFG_NEW_BURST,
group='network_log')
ovn_config.cfg.CONF.set_override('rate_limit', CFG_NEW_RATE,
group='network_log')
# Call the maintenance task
self.assertRaises(periodics.NeverAgain,
self.maint.check_fair_meter_consistency)
# Check meter band was effectively changed after the maintenance call
self.assertEqual(CFG_NEW_BURST,
[*self.nb_api._tables['Meter_Band'].rows.values()][0].burst_size)
self.assertEqual(CFG_NEW_RATE,
[*self.nb_api._tables['Meter_Band'].rows.values()][0].rate)

View File

@ -21,9 +21,12 @@ from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf
from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import ovn_client
from neutron.tests import base
from neutron.tests.unit import fake_resources as fakes
from neutron.tests.unit.services.logapi.drivers.ovn \
import test_driver as test_log_driver
from neutron_lib.api.definitions import l3
from neutron_lib.api.definitions import portbindings
from neutron_lib import constants as const
from neutron_lib.services.logapi import constants as log_const
class TestOVNClientBase(base.BaseTestCase):
@ -191,3 +194,61 @@ class TestOVNClientDetermineBindHost(TestOVNClientBase):
self.assertEqual(
self.fake_smartnic_hostname,
self.ovn_client.determine_bind_host({}, port_context=context))
class TestOVNClientFairMeter(TestOVNClientBase,
test_log_driver.TestOVNDriverBase):
def test_create_ovn_fair_meter(self):
mock_find_rows = mock.Mock()
mock_find_rows.execute.return_value = None
self.nb_idl.db_find_rows.return_value = mock_find_rows
self.ovn_client.create_ovn_fair_meter(self._log_driver.meter_name)
self.assertFalse(self.nb_idl.meter_del.called)
self.assertTrue(self.nb_idl.meter_add.called)
self.nb_idl.meter_add.assert_called_once_with(
name=self._log_driver.meter_name,
unit="pktps",
rate=self.fake_cfg_network_log.rate_limit,
fair=True,
burst_size=self.fake_cfg_network_log.burst_limit,
may_exist=False,
external_ids={constants.OVN_DEVICE_OWNER_EXT_ID_KEY:
log_const.LOGGING_PLUGIN})
def test_create_ovn_fair_meter_unchanged(self):
mock_find_rows = mock.Mock()
mock_find_rows.execute.return_value = [self._fake_meter()]
self.nb_idl.db_find_rows.return_value = mock_find_rows
self.nb_idl.lookup.side_effect = lambda table, key, default: (
self._fake_meter_band() if key == "test_band" else default)
self.ovn_client.create_ovn_fair_meter(self._log_driver.meter_name)
self.assertFalse(self.nb_idl.meter_del.called)
self.assertFalse(self.nb_idl.meter_add.called)
def test_create_ovn_fair_meter_changed(self):
mock_find_rows = mock.Mock()
mock_find_rows.execute.return_value = [self._fake_meter(fair=[False])]
self.nb_idl.db_find_rows.return_value = mock_find_rows
self.nb_idl.lookup.return_value = self._fake_meter_band()
self.ovn_client.create_ovn_fair_meter(self._log_driver.meter_name)
self.assertTrue(self.nb_idl.meter_del.called)
self.assertTrue(self.nb_idl.meter_add.called)
def test_create_ovn_fair_meter_band_changed(self):
mock_find_rows = mock.Mock()
mock_find_rows.execute.return_value = [self._fake_meter()]
self.nb_idl.db_find_rows.return_value = mock_find_rows
self.nb_idl.lookup.return_value = self._fake_meter_band(rate=666)
self.ovn_client.create_ovn_fair_meter(self._log_driver.meter_name)
self.assertTrue(self.nb_idl.meter_del.called)
self.assertTrue(self.nb_idl.meter_add.called)
def test_create_ovn_fair_meter_band_missing(self):
mock_find_rows = mock.Mock()
mock_find_rows.execute.return_value = [self._fake_meter()]
self.nb_idl.db_find_rows.return_value = mock_find_rows
self.nb_idl.lookup.side_effect = None
self.ovn_client.create_ovn_fair_meter(self._log_driver.meter_name)
self.assertTrue(self.nb_idl.meter_del.called)
self.assertTrue(self.nb_idl.meter_add.called)

View File

@ -29,7 +29,7 @@ FAKE_CFG_RATE = 123
FAKE_CFG_BURST = 321
class TestOVNDriver(base.BaseTestCase):
class TestOVNDriverBase(base.BaseTestCase):
def setUp(self):
super().setUp()
@ -91,6 +91,8 @@ class TestOVNDriver(base.BaseTestCase):
meter_band_obj_dict = {**meter_band_defaults_dict, **kwargs}
return mock.Mock(**meter_band_obj_dict)
class TestOVNDriver(TestOVNDriverBase):
def test_create(self):
driver = self._log_driver
self.assertEqual(self.log_plugin, driver._log_plugin)
@ -106,62 +108,6 @@ class TestOVNDriver(base.BaseTestCase):
driver2 = self._log_driver_reinit()
self.assertEqual(test_log_base, driver2.meter_name)
def test__create_ovn_fair_meter(self):
mock_find_rows = mock.Mock()
mock_find_rows.execute.return_value = None
self._nb_ovn.db_find_rows.return_value = mock_find_rows
self._log_driver._create_ovn_fair_meter(self._nb_ovn.transaction)
self.assertFalse(self._nb_ovn.meter_del.called)
self.assertTrue(self._nb_ovn.meter_add.called)
self.assertFalse(
self._nb_ovn.transaction.return_value.__enter__.called)
self._nb_ovn.meter_add.assert_called_once_with(
name="acl_log_meter",
unit="pktps",
rate=FAKE_CFG_RATE,
fair=True,
burst_size=FAKE_CFG_BURST,
may_exist=False,
external_ids={ovn_const.OVN_DEVICE_OWNER_EXT_ID_KEY:
log_const.LOGGING_PLUGIN})
def test__create_ovn_fair_meter_unchanged(self):
mock_find_rows = mock.Mock()
mock_find_rows.execute.return_value = [self._fake_meter()]
self._nb_ovn.db_find_rows.return_value = mock_find_rows
self._nb_ovn.lookup.side_effect = lambda table, key: (
self._fake_meter_band() if key == "test_band" else None)
self._log_driver._create_ovn_fair_meter(self._nb_ovn.transaction)
self.assertFalse(self._nb_ovn.meter_del.called)
self.assertFalse(self._nb_ovn.meter_add.called)
def test__create_ovn_fair_meter_changed(self):
mock_find_rows = mock.Mock()
mock_find_rows.execute.return_value = [self._fake_meter(fair=[False])]
self._nb_ovn.db_find_rows.return_value = mock_find_rows
self._nb_ovn.lookup.return_value = self._fake_meter_band()
self._log_driver._create_ovn_fair_meter(self._nb_ovn.transaction)
self.assertTrue(self._nb_ovn.meter_del.called)
self.assertTrue(self._nb_ovn.meter_add.called)
def test__create_ovn_fair_meter_band_changed(self):
mock_find_rows = mock.Mock()
mock_find_rows.execute.return_value = [self._fake_meter()]
self._nb_ovn.db_find_rows.return_value = mock_find_rows
self._nb_ovn.lookup.return_value = self._fake_meter_band(rate=666)
self._log_driver._create_ovn_fair_meter(self._nb_ovn.transaction)
self.assertTrue(self._nb_ovn.meter_del.called)
self.assertTrue(self._nb_ovn.meter_add.called)
def test__create_ovn_fair_meter_band_missing(self):
mock_find_rows = mock.Mock()
mock_find_rows.execute.return_value = [self._fake_meter()]
self._nb_ovn.db_find_rows.return_value = mock_find_rows
self._nb_ovn.lookup.side_effect = idlutils.RowNotFound
self._log_driver._create_ovn_fair_meter(self._nb_ovn.transaction)
self.assertTrue(self._nb_ovn.meter_del.called)
self.assertTrue(self._nb_ovn.meter_add.called)
class _fake_acl():
def __init__(self, name=None, **acl_dict):
acl_defaults_dict = {