diff --git a/neutron/agent/ovn/metadata/agent.py b/neutron/agent/ovn/metadata/agent.py index 1e708c1fed2..5b1284f3572 100644 --- a/neutron/agent/ovn/metadata/agent.py +++ b/neutron/agent/ovn/metadata/agent.py @@ -269,7 +269,7 @@ class PortBindingDeletedEvent(PortBindingEvent): return True -class ChassisCreateEventBase(row_event.RowEvent): +class ChassisPrivateCreateEvent(row_event.RowEvent): """Row create event - Chassis name == our_chassis. On connection, we get a dump of all chassis so if we catch a creation @@ -277,14 +277,12 @@ class ChassisCreateEventBase(row_event.RowEvent): to do a full sync to make sure that we capture all changes while the connection to OVSDB was down. """ - table = None - def __init__(self, metadata_agent): self.agent = metadata_agent self.first_time = True events = (self.ROW_CREATE,) - super(ChassisCreateEventBase, self).__init__( - events, self.table, (('name', '=', self.agent.chassis),)) + super(ChassisPrivateCreateEvent, self).__init__( + events, 'Chassis_Private', (('name', '=', self.agent.chassis),)) self.event_name = self.__class__.__name__ def run(self, event, row, old): @@ -299,14 +297,6 @@ class ChassisCreateEventBase(row_event.RowEvent): self.agent.sync() -class ChassisCreateEvent(ChassisCreateEventBase): - table = 'Chassis' - - -class ChassisPrivateCreateEvent(ChassisCreateEventBase): - table = 'Chassis_Private' - - class SbGlobalUpdateEvent(row_event.RowEvent): """Row update event on SB_Global table.""" diff --git a/neutron/common/ovn/utils.py b/neutron/common/ovn/utils.py index 14ab1870875..2b2c79255cf 100644 --- a/neutron/common/ovn/utils.py +++ b/neutron/common/ovn/utils.py @@ -902,11 +902,7 @@ def create_neutron_pg_drop(): def get_ovn_chassis_other_config(chassis): - # NOTE(ralonsoh): LP#1990229 to be removed when min OVN version is 22.09 - try: - return chassis.other_config - except AttributeError: - return chassis.external_ids + return chassis.other_config def get_subnets_address_scopes(context, subnets_by_id, fixed_ips, ml2_plugin): diff --git a/neutron/plugins/ml2/drivers/ovn/agent/neutron_agent.py b/neutron/plugins/ml2/drivers/ovn/agent/neutron_agent.py index 8f5e26a1f94..81e3e6a2151 100644 --- a/neutron/plugins/ml2/drivers/ovn/agent/neutron_agent.py +++ b/neutron/plugins/ml2/drivers/ovn/agent/neutron_agent.py @@ -45,33 +45,21 @@ class NeutronAgent(abc.ABC): def update(self, chassis_private, clear_down=False): self.chassis_private = chassis_private - # When use the Chassis_Private table for agents health check, - # chassis_private has attribute nb_cfg_timestamp. - # nb_cfg_timestamp: the timestamp when ovn-controller finishes - # processing the change corresponding to nb_cfg( - # https://www.ovn.org/support/dist-docs/ovn-sb.5.html). - # it can better reflect the status of chassis. - # nb_cfg_timestamp is milliseconds, need to convert to datetime. - if hasattr(chassis_private, 'nb_cfg_timestamp'): - updated_at = datetime.datetime.fromtimestamp( - chassis_private.nb_cfg_timestamp / 1000, - datetime.timezone.utc) - else: - updated_at = timeutils.utcnow(with_timezone=True) + updated_at = datetime.datetime.fromtimestamp( + chassis_private.nb_cfg_timestamp / 1000, + datetime.timezone.utc) self.updated_at = updated_at if clear_down: self.set_down = False @staticmethod - def _get_chassis(chassis_or_chassis_private): + def _get_chassis(chassis_private): """Return the chassis register The input could be the chassis register itself or the chassis private. """ try: - return chassis_or_chassis_private.chassis[0] - except AttributeError: - return chassis_or_chassis_private + return chassis_private.chassis[0] except IndexError: # Chassis register has been deleted but not Chassis_Private. return DeletedChassis @@ -93,8 +81,7 @@ class NeutronAgent(abc.ABC): 'configurations': { 'chassis_name': self.chassis.name, 'bridge-mappings': - ovn_utils.get_ovn_chassis_other_config(self.chassis).get( - 'ovn-bridge-mappings', '')}, + self.chassis.other_config.get('ovn-bridge-mappings', '')}, 'start_flag': True, 'agent_type': self.agent_type, 'id': self.agent_id, @@ -147,8 +134,8 @@ class ControllerAgent(NeutronAgent): @staticmethod # it is by default, but this makes pep8 happy def __new__(cls, chassis_private, driver): _chassis = cls._get_chassis(chassis_private) - other_config = ovn_utils.get_ovn_chassis_other_config(_chassis) - if 'enable-chassis-as-gw' in other_config.get('ovn-cms-options', []): + if 'enable-chassis-as-gw' in _chassis.other_config.get( + 'ovn-cms-options', []): cls = ControllerGatewayAgent return super().__new__(cls) @@ -171,9 +158,8 @@ class ControllerAgent(NeutronAgent): def update(self, chassis_private, clear_down=False): super().update(chassis_private, clear_down) - _chassis = self._get_chassis(chassis_private) - other_config = ovn_utils.get_ovn_chassis_other_config(_chassis) - if 'enable-chassis-as-gw' in other_config.get('ovn-cms-options', []): + if 'enable-chassis-as-gw' in self.chassis.other_config.get( + 'ovn-cms-options', []): self.__class__ = ControllerGatewayAgent @@ -182,10 +168,8 @@ class ControllerGatewayAgent(ControllerAgent): def update(self, chassis_private, clear_down=False): super().update(chassis_private, clear_down) - _chassis = self._get_chassis(chassis_private) - other_config = ovn_utils.get_ovn_chassis_other_config(_chassis) if ('enable-chassis-as-gw' not in - other_config.get('ovn-cms-options', [])): + self.chassis.other_config.get('ovn-cms-options', [])): self.__class__ = ControllerAgent 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 437918bad55..9d4a472ebd9 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py @@ -138,7 +138,6 @@ class OVNMechanismDriver(api.MechanismDriver): OVN_MIN_GENEVE_MAX_HEADER_SIZE) raise SystemExit(1) self._setup_vif_port_bindings() - self.agent_chassis_table = 'Chassis_Private' self.subscribe() self.qos_driver = qos_driver.OVNQosDriver.create(self) self.trunk_driver = trunk_driver.OVNTrunkDriver.create(self) 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 e1fab9fd461..383bbe10dd3 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 @@ -294,6 +294,7 @@ class PortBindingChassisUpdateEvent(row_event.RowEvent): class ChassisAgentEvent(BaseEvent): GLOBAL = True + table = 'Chassis_Private' # NOTE (twilson) Do not run new transactions out of a GLOBAL Event since # it will be running on every single process, and you almost certainly @@ -302,16 +303,6 @@ class ChassisAgentEvent(BaseEvent): self.driver = driver super().__init__() - @property - def table(self): - # It probably doesn't matter, but since agent_chassis_table changes - # in post_fork_initialize(), resolve this at runtime - return self.driver.agent_chassis_table - - @table.setter - def table(self, value): - pass - class ChassisAgentDownEvent(ChassisAgentEvent): events = (BaseEvent.ROW_DELETE,) @@ -346,9 +337,7 @@ class ChassisAgentWriteEvent(ChassisAgentEvent): # On updates to Chassis_Private because the Chassis has been deleted, # don't update the AgentCache. We use chassis_private.chassis to return # data about the agent. - return event == self.ROW_CREATE or ( - hasattr(old, 'nb_cfg') and not - (self.table == 'Chassis_Private' and not row.chassis)) + return event == self.ROW_CREATE or hasattr(old, 'nb_cfg') def run(self, event, row, old): n_agent.AgentCache().update(ovn_const.OVN_CONTROLLER_AGENT, row, @@ -361,21 +350,29 @@ class ChassisAgentTypeChangeEvent(ChassisEvent): events = (BaseEvent.ROW_UPDATE,) def match_fn(self, event, row, old=None): - # NOTE(ralonsoh): LP#1990229 to be removed when min OVN version is - # 22.09 - other_config = ('other_config' if hasattr(row, 'other_config') else - 'external_ids') - if not getattr(old, other_config, False): + try: + return row.other_config.get('ovn-cms-options', []) != ( + old.other_config.get('ovn-cms-options', [])) + except AttributeError: + # No change to other_config return False - new_other_config = utils.get_ovn_chassis_other_config(row) - old_other_config = utils.get_ovn_chassis_other_config(old) - agent_type_change = new_other_config.get('ovn-cms-options', []) != ( - old_other_config.get('ovn-cms-options', [])) - return agent_type_change def run(self, event, row, old): - n_agent.AgentCache().update(ovn_const.OVN_CONTROLLER_AGENT, row, - clear_down=event == self.ROW_CREATE) + # the row is in the Chassis table but the agent cache uses + # Chassis_Private rows + try: + ch_private = self.driver.sb_ovn.db_find_rows( + 'Chassis_Private', ('chassis', '=', row.uuid)).execute( + check_error=True)[0] + except IndexError: + # The chassis private row was not found, this should never happen + LOG.error("The Chassis_Private row for Chassis %s was not found.", + row.uuid) + return + # The passed agent type is significant to the method obtaining the + # agent chassis id. This method is the same for both Gateway and + # Controller agent types so the type below can be either. + n_agent.AgentCache().update(ovn_const.OVN_CONTROLLER_AGENT, ch_private) class ChassisMetadataAgentWriteEvent(ChassisAgentEvent): @@ -400,7 +397,7 @@ class ChassisMetadataAgentWriteEvent(ChassisAgentEvent): # On updates to Chassis_Private because the Chassis has been # deleted, don't update the AgentCache. We use # chassis_private.chassis to return data about the agent. - if self.table == 'Chassis_Private' and not row.chassis: + if not row.chassis: return False return self._metadata_nb_cfg(row) != self._metadata_nb_cfg(old) except (AttributeError, KeyError): @@ -710,24 +707,9 @@ class OvnIdlDistributedLock(BaseOvnIdl): self._hash_ring = hash_ring_manager.HashRingManager( self.driver.hash_ring_group) self._last_touch = None - # This is a map of tables that may be new after OVN database is updated - self._tables_to_register = { - 'OVN_Southbound': ['Chassis_Private'], - } - - def handle_db_schema_changes(self, event, row): - if (event == row_event.RowEvent.ROW_CREATE and - row._table.name == 'Database'): - try: - tables = self._tables_to_register[row.name] - except KeyError: - return - - self.update_tables(tables, row.schema[0]) def notify(self, event, row, updates=None): try: - self.handle_db_schema_changes(event, row) self.notify_handler.notify(event, row, updates, global_=True) try: target_node = self._hash_ring.get_node(str(row.uuid)) diff --git a/neutron/tests/functional/agent/ovn/metadata/test_metadata_agent.py b/neutron/tests/functional/agent/ovn/metadata/test_metadata_agent.py index bd6ace7392c..34ca13a4a83 100644 --- a/neutron/tests/functional/agent/ovn/metadata/test_metadata_agent.py +++ b/neutron/tests/functional/agent/ovn/metadata/test_metadata_agent.py @@ -36,6 +36,8 @@ from neutron.tests.common import net_helpers from neutron.tests.functional import base from neutron.tests.functional.common import ovn as ovn_common +AGENT_CHASSIS_TABLE = 'Chassis_Private' + class NoDatapathProvision(Exception): pass @@ -44,11 +46,13 @@ class NoDatapathProvision(Exception): class MetadataAgentHealthEvent(event.WaitEvent): event_name = 'MetadataAgentHealthEvent' - def __init__(self, chassis, sb_cfg, table, timeout=5): + def __init__(self, chassis, sb_cfg, timeout=5): self.chassis = chassis self.sb_cfg = sb_cfg super(MetadataAgentHealthEvent, self).__init__( - (self.ROW_UPDATE,), table, (('name', '=', self.chassis),), + (self.ROW_UPDATE,), + AGENT_CHASSIS_TABLE, + (('name', '=', self.chassis),), timeout=timeout) def matches(self, event, row, old=None): @@ -90,10 +94,6 @@ class TestMetadataAgent(base.TestOVNFunctionalBase): return_value=self.OVN_BRIDGE).start() self.agent = self._start_metadata_agent() - @property - def agent_chassis_table(self): - return 'Chassis_Private' - def _start_metadata_agent(self): conf = self.useFixture(fixture_config.Config()).conf conf.register_opts(meta_config.SHARED_OPTS) @@ -124,7 +124,7 @@ class TestMetadataAgent(base.TestOVNFunctionalBase): def test_metadata_agent_healthcheck(self): chassis_row = self.sb_api.db_find( - self.agent_chassis_table, + AGENT_CHASSIS_TABLE, ('name', '=', self.chassis_name)).execute( check_error=True)[0] @@ -139,8 +139,7 @@ class TestMetadataAgent(base.TestOVNFunctionalBase): # this event, Metadata agent will update the external_ids on its # Chassis row to signal that it's healthy. - row_event = MetadataAgentHealthEvent(self.chassis_name, 1, - self.agent_chassis_table) + row_event = MetadataAgentHealthEvent(self.chassis_name, 1) self.handler.watch_event(row_event) self.new_list_request('agents').get_response(self.api) @@ -292,7 +291,7 @@ class TestMetadataAgent(base.TestOVNFunctionalBase): def test_agent_registration_at_chassis_create_event(self): def check_for_metadata(): chassis = self.sb_api.lookup( - self.agent_chassis_table, self.chassis_name) + AGENT_CHASSIS_TABLE, self.chassis_name) return ovn_const.OVN_AGENT_METADATA_ID_KEY in chassis.external_ids exc = Exception('Chassis not created, %s is not in chassis ' @@ -365,25 +364,23 @@ class TestMetadataAgent(base.TestOVNFunctionalBase): self.assertEqual(other_chassis, other_name) event = MetadataAgentHealthEvent(chassis=other_name, sb_cfg=-1, - table=self.agent_chassis_table, timeout=0) # Use the agent's sb_idl to watch for the event since it has condition self.agent.sb_idl.idl.notify_handler.watch_event(event) # Use the test sb_api to set other_chassis values since shouldn't exist # on agent's sb_idl self.sb_api.db_set( - self.agent_chassis_table, other_chassis, + AGENT_CHASSIS_TABLE, other_chassis, ('external_ids', {'test': 'value'})).execute(check_error=True) - event2 = MetadataAgentHealthEvent(chassis=self.chassis_name, sb_cfg=-1, - table=self.agent_chassis_table) + event2 = MetadataAgentHealthEvent(chassis=self.chassis_name, sb_cfg=-1) self.agent.sb_idl.idl.notify_handler.watch_event(event2) # Use the test's sb_api again to send a command so we can see if it # completes and short-circuit the need to wait for a timeout to pass # the test. If we get the result to this, we would have gotten the # previous result as well. self.sb_api.db_set( - self.agent_chassis_table, self.chassis_name, + AGENT_CHASSIS_TABLE, self.chassis_name, ('external_ids', {'test': 'value'})).execute(check_error=True) self.assertTrue(event2.wait()) self.assertFalse(event.wait()) diff --git a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/test_placement.py b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/test_placement.py index 91bf1d3d169..caa269e83e8 100644 --- a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/test_placement.py +++ b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/test_placement.py @@ -194,8 +194,7 @@ class TestOVNClientPlacementExtension(base.TestOVNFunctionalBase): def test_chassis_bandwidth_config_event(self, mock_send_placement): ch_host = 'fake-chassis-host' ch_name = uuidutils.generate_uuid() - ch_event = test_ovsdb_monitor.WaitForChassisPrivateCreateEvent( - ch_name, self.mech_driver.agent_chassis_table) + ch_event = test_ovsdb_monitor.WaitForChassisPrivateCreateEvent(ch_name) self.mech_driver.sb_ovn.idl.notify_handler.watch_event(ch_event) self.chassis_name = self.add_fake_chassis(ch_host, name=ch_name) self.assertTrue(ch_event.wait()) 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 96e08999aeb..05da5f269fa 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 @@ -56,10 +56,10 @@ class WaitForDataPathBindingCreateEvent(event.WaitEvent): class WaitForChassisPrivateCreateEvent(event.WaitEvent): event_name = 'WaitForChassisPrivateCreateEvent' - def __init__(self, chassis_name, chassis_table): + def __init__(self, chassis_name): events = (self.ROW_CREATE,) conditions = (('name', '=', chassis_name),) - super().__init__(events, chassis_table, conditions, timeout=15) + super().__init__(events, 'Chassis_Private', conditions, timeout=15) class DistributedLockTestEvent(event.WaitEvent): @@ -493,8 +493,7 @@ class TestAgentMonitor(base.TestOVNFunctionalBase): self.handler = self.sb_api.idl.notify_handler self.mock_ovsdb_idl = mock.Mock() chassis_name = uuidutils.generate_uuid() - row_event = WaitForChassisPrivateCreateEvent( - chassis_name, self.mech_driver.agent_chassis_table) + row_event = WaitForChassisPrivateCreateEvent(chassis_name) self.mech_driver.sb_ovn.idl.notify_handler.watch_event(row_event) self.chassis_name = self.add_fake_chassis( self.FAKE_CHASSIS_HOST, name=chassis_name, @@ -509,10 +508,16 @@ class TestAgentMonitor(base.TestOVNFunctionalBase): self.sb_api.db_set('Chassis', self.chassis_name, ('other_config', {'ovn-cms-options': ''})).execute(check_error=True) n_utils.wait_until_true(lambda: - neutron_agent.AgentCache().get(self.chassis_name). - chassis.other_config['ovn-cms-options'] == '') - self.assertEqual(neutron_agent.ControllerAgent, - type(neutron_agent.AgentCache().get(self.chassis_name))) + type(neutron_agent.AgentCache().get(self.chassis_name)) is + neutron_agent.ControllerAgent) + + # Change back to gw chassis + self.sb_api.db_set('Chassis', self.chassis_name, ('other_config', + {'ovn-cms-options': 'enable-chassis-as-gw'})).execute( + check_error=True) + n_utils.wait_until_true(lambda: + type(neutron_agent.AgentCache().get(self.chassis_name)) is + neutron_agent.ControllerGatewayAgent) def test_agent_updated_at_use_nb_cfg_timestamp(self): def check_agent_ts(): diff --git a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py index 115a86f1373..cf932b27163 100644 --- a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py +++ b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py @@ -1123,10 +1123,9 @@ class AgentWaitEvent(event.WaitEvent): ONETIME = False def __init__(self, driver, chassis_names, events=None): - table = driver.agent_chassis_table events = events or (self.ROW_CREATE,) self.chassis_names = chassis_names - super().__init__(events, table, None) + super().__init__(events, 'Chassis_Private', None) self.event_name = "AgentWaitEvent" def match_fn(self, event, row, old): diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py index c438d168872..0bf92fc6ef9 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py @@ -283,44 +283,6 @@ class TestOvnIdlDistributedLock(base.BaseTestCase): self.idl.notify_handler.notify.assert_called_once_with( self.fake_event, self.fake_row, None, global_=True) - @staticmethod - def _create_fake_row(table_name): - # name is a parameter in Mock() so it can't be passed to constructor - table = mock.Mock() - table.name = table_name - return fakes.FakeOvsdbRow.create_one_ovsdb_row( - attrs={'_table': table, 'schema': ['foo']}) - - def test_handle_db_schema_changes_no_match_events(self): - other_table_row = self._create_fake_row('other') - database_table_row = self._create_fake_row('Database') - - self.idl.handle_db_schema_changes( - ovsdb_monitor.BaseEvent.ROW_UPDATE, other_table_row) - self.idl.handle_db_schema_changes( - ovsdb_monitor.BaseEvent.ROW_CREATE, other_table_row) - self.idl.handle_db_schema_changes( - ovsdb_monitor.BaseEvent.ROW_UPDATE, database_table_row) - - self.assertFalse(self.mock_update_tables.called) - - def _test_handle_db_schema(self, agent_table): - database_table_row = self._create_fake_row('Database') - self.idl._tables_to_register[database_table_row.name] = 'foo' - - self.fake_driver.agent_chassis_table = agent_table - self.idl.tables['Chassis_Private'] = 'foo' - self.idl.handle_db_schema_changes( - ovsdb_monitor.BaseEvent.ROW_CREATE, database_table_row) - - def test_handle_db_schema_changes_new_schema_to_new_schema(self): - """Agents use Chassis_Private and should keep using Chassis_Private - table. - """ - self._test_handle_db_schema('Chassis_Private') - self.assertEqual('Chassis_Private', - self.fake_driver.agent_chassis_table) - class TestPortBindingChassisUpdateEvent(base.BaseTestCase): def setUp(self): @@ -377,7 +339,6 @@ class TestOvnNbIdlNotifyHandler(test_mech_driver.OVNMechanismDriverTestCase): self.lp_table = self.idl.tables.get('Logical_Switch_Port') self.mech_driver.set_port_status_up = mock.Mock() self.mech_driver.set_port_status_down = mock.Mock() - mock.patch.object(self.idl, 'handle_db_schema_changes').start() self._mock_hash_ring = mock.patch.object( self.idl._hash_ring, 'get_node', return_value=self.idl._node_uuid) self._mock_hash_ring.start() @@ -522,7 +483,6 @@ class TestOvnSbIdlNotifyHandler(test_mech_driver.OVNMechanismDriverTestCase): super(TestOvnSbIdlNotifyHandler, self).setUp() sb_helper = ovs_idl.SchemaHelper(schema_json=OVN_SB_SCHEMA) sb_helper.register_table('Chassis') - self.driver.agent_chassis_table = 'Chassis' self.sb_idl = ovsdb_monitor.OvnSbIdl(self.mech_driver, "remote", sb_helper) self.sb_idl.post_connect()