diff options
6 files changed, 212 insertions, 58 deletions
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 ce26906b8c..17b0525d9a 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py @@ -42,6 +42,7 @@ from neutron.db import ovn_revision_numbers_db as revision_numbers_db from neutron.db import segments_db 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 @@ -957,6 +958,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): diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py index 4c821bcf43..b8a13a7406 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py @@ -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 @@ -2587,3 +2588,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) diff --git a/neutron/services/logapi/drivers/ovn/driver.py b/neutron/services/logapi/drivers/ovn/driver.py index 31d32de038..b6e94d62da 100644 --- a/neutron/services/logapi/drivers/ovn/driver.py +++ b/neutron/services/logapi/drivers/ovn/driver.py @@ -90,6 +90,10 @@ class OVNDriver(base.DriverBase): 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 @@ -303,7 +307,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)) 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 3a85d4ef22..8286a3f9d7 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 @@ -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.""" @@ -1042,3 +1048,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) diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_client.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_client.py new file mode 100644 index 0000000000..11a17657c3 --- /dev/null +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_client.py @@ -0,0 +1,92 @@ +# Copyright 2022 Canonical +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from unittest import mock + +from neutron.common.ovn import constants +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.services.logapi.drivers.ovn \ + import test_driver as test_log_driver +from neutron_lib.services.logapi import constants as log_const + + +class TestOVNClientBase(base.BaseTestCase): + + def setUp(self): + ovn_conf.register_opts() + super(TestOVNClientBase, self).setUp() + self.nb_idl = mock.MagicMock() + self.sb_idl = mock.MagicMock() + self.ovn_client = ovn_client.OVNClient(self.nb_idl, self.sb_idl) + + +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) diff --git a/neutron/tests/unit/services/logapi/drivers/ovn/test_driver.py b/neutron/tests/unit/services/logapi/drivers/ovn/test_driver.py index 35c2ec5765..4f4cd7a7ba 100644 --- a/neutron/tests/unit/services/logapi/drivers/ovn/test_driver.py +++ b/neutron/tests/unit/services/logapi/drivers/ovn/test_driver.py @@ -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 = { |