From a991b097c1d0809fffaf13fbd8e32081dc1c6dc5 Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Tue, 19 Dec 2023 08:55:10 +0000 Subject: [PATCH 1/2] [OVN] Retrieve the OVN agent extensions correctly Now the OVN agent implements a method ``__getitem__`` that retrieves, from ``self.ext_manager``, a loaded extension by its name. The method returns the instantiated extension object. Closes-Bug: #2046892 Change-Id: Ibb6dc7c9150bf99639d5b6180356963998dc4e49 (cherry picked from commit fa46584af99c677489ee8e92eaabb25b801c1ce7) --- neutron/agent/ovn/agent/ovn_neutron_agent.py | 4 ++++ .../agent/ovn/extensions/extension_manager.py | 5 +++++ neutron/agent/ovn/extensions/noop.py | 4 ++++ neutron/agent/ovn/extensions/qos_hwol.py | 21 ++++++++++++------- .../ovn/agent/fake_ovn_agent_extension.py | 4 ++++ .../agent/ovn/agent/test_ovn_neutron_agent.py | 10 ++++++++- .../agent/ovn/extensions/test_qos_hwol.py | 16 +++++++------- 7 files changed, 47 insertions(+), 17 deletions(-) diff --git a/neutron/agent/ovn/agent/ovn_neutron_agent.py b/neutron/agent/ovn/agent/ovn_neutron_agent.py index dbb59965238..ac12b33c9c8 100644 --- a/neutron/agent/ovn/agent/ovn_neutron_agent.py +++ b/neutron/agent/ovn/agent/ovn_neutron_agent.py @@ -61,6 +61,10 @@ def __init__(self, conf): self.ext_manager = ext_mgr.OVNAgentExtensionManager(self._conf) self.ext_manager.initialize(None, 'ovn', self.ext_manager_api) + def __getitem__(self, name): + """Return the named extension objet from ``self.ext_manager``""" + return self.ext_manager[name].obj + @property def ovs_idl(self): return self.ext_manager_api.ovs_idl diff --git a/neutron/agent/ovn/extensions/extension_manager.py b/neutron/agent/ovn/extensions/extension_manager.py index 447e1f1e5d9..6c1711df676 100644 --- a/neutron/agent/ovn/extensions/extension_manager.py +++ b/neutron/agent/ovn/extensions/extension_manager.py @@ -48,6 +48,11 @@ def __init__(self): super().__init__() self.agent_api = None + @property + @abc.abstractmethod + def name(self): + pass + def initialize(self, *args): """Initialize agent extension.""" self.agent_api = None diff --git a/neutron/agent/ovn/extensions/noop.py b/neutron/agent/ovn/extensions/noop.py index c972e6fd041..76857169667 100644 --- a/neutron/agent/ovn/extensions/noop.py +++ b/neutron/agent/ovn/extensions/noop.py @@ -18,6 +18,10 @@ class NoopOVNAgentExtension(extension_manager.OVNAgentExtension): + @property + def name(self): + return 'Noop OVN agent extension' + @property def ovs_idl_events(self): return [] diff --git a/neutron/agent/ovn/extensions/qos_hwol.py b/neutron/agent/ovn/extensions/qos_hwol.py index 5652f0f6d42..ac1c6c73bb7 100644 --- a/neutron/agent/ovn/extensions/qos_hwol.py +++ b/neutron/agent/ovn/extensions/qos_hwol.py @@ -27,6 +27,7 @@ LOG = logging.getLogger(__name__) +EXT_NAME = 'qos_hwol' # NOTE(ralonsoh): move these constants from ``eswitch_manager`` to ``pci_lib``. MAX_TX_RATE = eswitch_manager.IP_LINK_CAPABILITY_RATE MIN_TX_RATE = eswitch_manager.IP_LINK_CAPABILITY_MIN_TX_RATE @@ -59,10 +60,10 @@ def match_fn(self, event, row, old): def run(self, event, row, old): if event in (self.ROW_CREATE, self.ROW_UPDATE): - self.ovn_agent.qos_hwol_ext.add_port( + self.ovn_agent[EXT_NAME].add_port( row.external_ids['iface-id'], row.name) elif event == self.ROW_DELETE: - self.ovn_agent.qos_hwol_ext.remove_egress( + self.ovn_agent[EXT_NAME].remove_egress( row.external_ids['iface-id']) LOG.debug(self.LOG_MSG, row.external_ids['iface-id'], row.name, event) @@ -82,7 +83,7 @@ def match_fn(self, event, row, old): # Check if the port has a Port ID and if this ID is bound to this host. port_id = row.external_ids.get(ovn_const.OVN_PORT_EXT_ID_KEY) - if not port_id or not self.ovn_agent.qos_hwol_ext.get_port(port_id): + if not port_id or not self.ovn_agent[EXT_NAME].get_port(port_id): return False if event in (self.ROW_CREATE, self.ROW_DELETE): @@ -104,7 +105,7 @@ def run(self, event, row, old): max_kbps, min_kbps = agent_ovsdb.get_port_qos(self.ovn_agent.nb_idl, port_id) LOG.debug(self.LOG_MSG, str(row.uuid), port_id, max_kbps, event) - self.ovn_agent.qos_hwol_ext.update_egress(port_id, max_kbps, min_kbps) + self.ovn_agent[EXT_NAME].update_egress(port_id, max_kbps, min_kbps) class QoSMinimumBandwidthEvent(row_event.RowEvent): @@ -129,7 +130,7 @@ def match_fn(self, event, row, old): except (KeyError, AttributeError): return False - if not self.ovn_agent.qos_hwol_ext.get_port(row.name): + if not self.ovn_agent[EXT_NAME].get_port(row.name): return False return True @@ -138,7 +139,7 @@ def run(self, event, row, old): max_kbps, min_kbps = agent_ovsdb.get_port_qos(self.ovn_agent.nb_idl, row.name) LOG.debug(self.LOG_MSG, row.name, min_kbps, event) - self.ovn_agent.qos_hwol_ext.update_egress(row.name, max_kbps, min_kbps) + self.ovn_agent[EXT_NAME].update_egress(row.name, max_kbps, min_kbps) class _PortBindingChassisEvent(row_event.RowEvent): @@ -176,8 +177,8 @@ def run(self, event, row, old): event) max_kbps, min_kbps = agent_ovsdb.get_port_qos(self.ovn_agent.nb_idl, row.logical_port) - self.ovn_agent.qos_hwol_ext.update_egress(row.logical_port, max_kbps, - min_kbps) + self.ovn_agent[EXT_NAME].update_egress(row.logical_port, max_kbps, + min_kbps) class QoSHardwareOffloadExtension(extension_manager.OVNAgentExtension): @@ -187,6 +188,10 @@ def __init__(self): # _ovs_ports = {Neutron port ID: OVS port name} self._ovs_ports = {} + @property + def name(self): + return 'QoS hardware offloaded extension' + @property def ovs_idl_events(self): return [OVSInterfaceEvent, diff --git a/neutron/tests/functional/agent/ovn/agent/fake_ovn_agent_extension.py b/neutron/tests/functional/agent/ovn/agent/fake_ovn_agent_extension.py index b93c31b9e27..4ca94da63b4 100644 --- a/neutron/tests/functional/agent/ovn/agent/fake_ovn_agent_extension.py +++ b/neutron/tests/functional/agent/ovn/agent/fake_ovn_agent_extension.py @@ -54,6 +54,10 @@ def run(self, event, row, old): class FakeOVNAgentExtension(ext_mgr.OVNAgentExtension): + @property + def name(self): + return 'Fake OVN agent extension' + @property def ovs_idl_events(self): return [OVSInterfaceEvent] diff --git a/neutron/tests/functional/agent/ovn/agent/test_ovn_neutron_agent.py b/neutron/tests/functional/agent/ovn/agent/test_ovn_neutron_agent.py index cd19d5848e6..90bbf3f2242 100644 --- a/neutron/tests/functional/agent/ovn/agent/test_ovn_neutron_agent.py +++ b/neutron/tests/functional/agent/ovn/agent/test_ovn_neutron_agent.py @@ -25,6 +25,9 @@ from neutron.tests.functional import base +TEST_EXTENSION = 'testing' + + class TestOVNNeutronAgent(base.TestOVNFunctionalBase): OVN_BRIDGE = 'br-int' @@ -36,9 +39,13 @@ def setUp(self, **kwargs): agent_ovsdb, 'get_own_chassis_name').start() self.ovn_agent = self._start_ovn_neutron_agent() + def _check_loaded_extensions(self, ovn_agent): + loaded_ext = ovn_agent[TEST_EXTENSION] + self.assertEqual('Fake OVN agent extension', loaded_ext.name) + def _start_ovn_neutron_agent(self): conf = self.useFixture(fixture_config.Config()).conf - conf.set_override('extensions', 'testing', group='agent') + conf.set_override('extensions', TEST_EXTENSION, group='agent') ovn_nb_db = self.ovsdb_server_mgr.get_ovsdb_connection_path('nb') conf.set_override('ovn_nb_connection', ovn_nb_db, group='ovn') ovn_sb_db = self.ovsdb_server_mgr.get_ovsdb_connection_path('sb') @@ -52,6 +59,7 @@ def _start_ovn_neutron_agent(self): agt.test_ovn_sb_idl = [] agt.test_ovn_nb_idl = [] agt.start() + self._check_loaded_extensions(agt) self.add_fake_chassis(self.FAKE_CHASSIS_HOST, name=self.chassis_name) diff --git a/neutron/tests/functional/agent/ovn/extensions/test_qos_hwol.py b/neutron/tests/functional/agent/ovn/extensions/test_qos_hwol.py index a6457dcd0ee..02d5efc8a22 100644 --- a/neutron/tests/functional/agent/ovn/extensions/test_qos_hwol.py +++ b/neutron/tests/functional/agent/ovn/extensions/test_qos_hwol.py @@ -37,7 +37,7 @@ def _cleanup(self): def test_port_creation_and_deletion(self): def check_add_port_called(): try: - mock_agent.qos_hwol_ext.add_port.assert_has_calls( + mock_agent[qos_hwol.EXT_NAME].add_port.assert_has_calls( [mock.call(port_iface_id, self.port_name)]) return True except AssertionError: @@ -52,7 +52,7 @@ def check_remove_egress_called(): return False port_iface_id = 'port_iface-id' - mock_agent = mock.Mock() + mock_agent = mock.MagicMock() events = [qos_hwol.OVSInterfaceEvent(mock_agent)] self.ovs_idl = agent_ovsdb.MonitorAgentOvsIdl(events=events).start() self.br_name = ('brtest-' + uuidutils.generate_uuid())[:13] @@ -91,13 +91,13 @@ def setUp(self, **kwargs): def test_qos_bw_limit_created_and_updated(self): def check_update_egress_called(rate): try: - mock_agent.qos_hwol_ext.update_egress.assert_has_calls( + mock_agent[qos_hwol.EXT_NAME].update_egress.assert_has_calls( [mock.call(port_id, rate, 0)]) return True except AssertionError: return False - mock_agent = mock.Mock(nb_idl=self.nb_api) + mock_agent = mock.MagicMock(nb_idl=self.nb_api) events = [qos_hwol.QoSBandwidthLimitEvent(mock_agent)] agent_ovsdb.MonitorAgentOvnNbIdl(qos_hwol.NB_IDL_TABLES, events).start() @@ -133,13 +133,13 @@ def setUp(self, **kwargs): def test_qos_min_bw_created_and_updated(self): def check_update_egress_called(max_kbps, min_kbps): try: - mock_agent.qos_hwol_ext.update_egress.assert_has_calls( + mock_agent[qos_hwol.EXT_NAME].update_egress.assert_has_calls( [mock.call(port_id, max_kbps, min_kbps)]) return True except AssertionError: return False - mock_agent = mock.Mock(nb_idl=self.nb_api) + mock_agent = mock.MagicMock(nb_idl=self.nb_api) events = [qos_hwol.QoSMinimumBandwidthEvent(mock_agent)] agent_ovsdb.MonitorAgentOvnNbIdl(qos_hwol.NB_IDL_TABLES, events).start() @@ -169,7 +169,7 @@ def test_port_binding_chassis_create_event(self, mock_get_port_qos, *args): def check_update_egress_called(max_kbps, min_kbps): try: - mock_agent.qos_hwol_ext.update_egress.assert_has_calls( + mock_agent[qos_hwol.EXT_NAME].update_egress.assert_has_calls( [mock.call(self.port['id'], max_kbps, min_kbps)]) return True except AssertionError: @@ -177,7 +177,7 @@ def check_update_egress_called(max_kbps, min_kbps): max_kbps, min_kbps = 1000, 800 mock_get_port_qos.return_value = max_kbps, min_kbps - mock_agent = mock.Mock(nb_idl=self.nb_api) + mock_agent = mock.MagicMock(nb_idl=self.nb_api) events = [qos_hwol.PortBindingChassisCreatedEvent(mock_agent)] chassis_name = self.add_fake_chassis('ovn-host-fake') mock_agent.chassis = chassis_name From 643dbbbf6bbf7601b3deb07096339e0ec72e57a5 Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Tue, 19 Dec 2023 10:57:56 +0000 Subject: [PATCH 2/2] [OVN] OVN agent extensions correctly consume agent API Now the ``OVNAgentExtension`` class do not clear the agent API during the extension initialization. This patch also passes the agent object to the OVN agent extensions as agent API. Any method required will be implemented directly on the OVN agent class. Closes-Bug: #2046939 Change-Id: Ia635ca1ff97c3db43a34d3dec6a7f9df154dfe28 (cherry picked from commit 86efc8be9934713ad79b3415b8b5b72bd475e01c) --- neutron/agent/ovn/agent/ovn_neutron_agent.py | 2 +- neutron/agent/ovn/extensions/extension_manager.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/neutron/agent/ovn/agent/ovn_neutron_agent.py b/neutron/agent/ovn/agent/ovn_neutron_agent.py index dbb59965238..a54c65a736b 100644 --- a/neutron/agent/ovn/agent/ovn_neutron_agent.py +++ b/neutron/agent/ovn/agent/ovn_neutron_agent.py @@ -59,7 +59,7 @@ def __init__(self, conf): self.ovn_bridge = None self.ext_manager_api = ext_mgr.OVNAgentExtensionAPI() self.ext_manager = ext_mgr.OVNAgentExtensionManager(self._conf) - self.ext_manager.initialize(None, 'ovn', self.ext_manager_api) + self.ext_manager.initialize(None, 'ovn', self) @property def ovs_idl(self): diff --git a/neutron/agent/ovn/extensions/extension_manager.py b/neutron/agent/ovn/extensions/extension_manager.py index 447e1f1e5d9..2e8dac7d5f2 100644 --- a/neutron/agent/ovn/extensions/extension_manager.py +++ b/neutron/agent/ovn/extensions/extension_manager.py @@ -50,7 +50,7 @@ def __init__(self): def initialize(self, *args): """Initialize agent extension.""" - self.agent_api = None + pass def consume_api(self, agent_api): """Configure the Agent API.