[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
(cherry picked from commit c3602ac19b
)
This commit is contained in:
parent
b3bcab7986
commit
05ff8ce3ee
@ -42,6 +42,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
|
||||
@ -1021,6 +1022,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):
|
||||
|
||||
|
@ -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
|
||||
@ -2657,3 +2658,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)
|
||||
|
@ -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))
|
||||
|
||||
|
@ -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."""
|
||||
@ -943,3 +949,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)
|
||||
|
@ -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)
|
||||
|
@ -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 = {
|
||||
|
Loading…
Reference in New Issue
Block a user