Merge "[OVN] Fix OVN agent register process"

neutron/agent/ovn/agent/ovn_neutron_agent.py

@@ -150,6 +150,15 @@ class OVNNeutronAgent(service.Service):
         return ovsdb.MonitorAgentOvnSbIdl(tables, events,
                                           chassis=self.chassis).start()
 
+    def register_ovn_agent(self):
+        # NOTE(lucasagomes): db_add() will not overwrite the UUID if
+        # it's already set.
+        # Generate unique, but consistent ovn agent id for chassis name
+        agent_id = uuid.uuid5(self.chassis_id, 'ovn_agent')
+        ext_ids = {ovn_const.OVN_AGENT_NEUTRON_ID_KEY: str(agent_id)}
+        self.sb_idl.db_add('Chassis_Private', self.chassis, 'external_ids',
+                           ext_ids).execute(check_error=True)
+
     def update_neutron_sb_cfg_key(self):
         nb_cfg = self.sb_idl.db_get('Chassis_Private',
                                     self.chassis, 'nb_cfg').execute()
@@ -167,6 +176,7 @@ class OVNNeutronAgent(service.Service):
         self.ext_manager_api.nb_idl = self._load_nb_idl()
         self.ext_manager.start()
 
+        self.register_ovn_agent()
         self.update_neutron_sb_cfg_key()
         LOG.info('OVN Neutron Agent started')
 

neutron/agent/ovn/extensions/metadata.py

@@ -178,9 +178,7 @@ class MetadataExtension(extension_manager.OVNAgentExtension,
         self.sync()
 
         # Register the agent with its corresponding Chassis
-        self.register_metadata_agent()
         self._update_chassis_private_config()
-        self.agent_api.update_neutron_sb_cfg_key()
 
         # Start the metadata server.
         proxy_thread = threading.Thread(target=self._proxy.wait)

neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py

@@ -377,17 +377,27 @@ class ChassisAgentTypeChangeEvent(ChassisEvent):
         n_agent.AgentCache().update(ovn_const.OVN_CONTROLLER_AGENT, ch_private)
 
 
-class ChassisMetadataAgentWriteEvent(ChassisAgentEvent):
+class ChassisOVNAgentWriteEvent(ChassisAgentEvent):
     events = (BaseEvent.ROW_CREATE, BaseEvent.ROW_UPDATE)
 
     @staticmethod
-    def _metadata_nb_cfg(row):
+    def _agent_sb_cfg(row):
+        external_ids = row.external_ids
+        # Try the OVN agent SB cfg first, then fallback to the OVN Metadata
+        # agent
+        ovn_sb_cfg = external_ids.get(ovn_const.OVN_AGENT_NEUTRON_SB_CFG_KEY)
+        if ovn_sb_cfg:
+            return int(ovn_sb_cfg)
+        # NOTE(ralonsoh): to remove when the OVN Metadata agent is removed.
         return int(
-            row.external_ids.get(ovn_const.OVN_AGENT_METADATA_SB_CFG_KEY, -1))
+            external_ids.get(ovn_const.OVN_AGENT_METADATA_SB_CFG_KEY, -1))
 
     @staticmethod
     def agent_id(row):
-        return row.external_ids.get(ovn_const.OVN_AGENT_METADATA_ID_KEY)
+        external_ids = row.external_ids
+        # NOTE(ralonsoh): to update when the OVN Metadata agent is removed.
+        return (external_ids.get(ovn_const.OVN_AGENT_NEUTRON_ID_KEY) or
+                external_ids.get(ovn_const.OVN_AGENT_METADATA_ID_KEY))
 
     def match_fn(self, event, row, old=None):
         if not self.agent_id(row):
@@ -395,19 +405,35 @@ class ChassisMetadataAgentWriteEvent(ChassisAgentEvent):
             return False
         if event == self.ROW_CREATE:
             return True
+
+        # 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 not getattr(row, 'chassis', None):
+            return False
+
+        # Check if both rows have external_ids before comparing nb_cfg
+        if not (hasattr(old, 'external_ids') and row.external_ids):
+            return False
+
         try:
-            # On updates to Chassis_Private because the Chassis has been
+            # Cache the nb_cfg values to avoid duplicate calculations
-            # deleted, don't update the AgentCache. We use
+            row_sb_cfg = self._agent_sb_cfg(row)
-            # chassis_private.chassis to return data about the agent.
+            old_sb_cfg = self._agent_sb_cfg(old)
-            if not row.chassis:
+            return row_sb_cfg != old_sb_cfg
-                return False
+        except (AttributeError, KeyError, TypeError):
-            return self._metadata_nb_cfg(row) != self._metadata_nb_cfg(old)
-        except (AttributeError, KeyError):
             return False
 
     def run(self, event, row, old):
-        n_agent.AgentCache().update(ovn_const.OVN_METADATA_AGENT, row,
+        external_ids = row.external_ids
-                                    clear_down=True)
+        if external_ids.get(ovn_const.OVN_AGENT_NEUTRON_ID_KEY):
+            n_agent.AgentCache().update(ovn_const.OVN_NEUTRON_AGENT, row,
+                                        clear_down=True)
+        else:
+            # NOTE(ralonsoh): to remove when the OVN Metadata agent is
+            # removed.
+            n_agent.AgentCache().update(ovn_const.OVN_METADATA_AGENT, row,
+                                        clear_down=True)
 
 
 class PortBindingChassisEvent(row_event.RowEvent):
@@ -900,7 +926,7 @@ class OvnSbIdl(OvnIdlDistributedLock):
             ChassisAgentDownEvent(self.driver),
             ChassisAgentWriteEvent(self.driver),
             ChassisAgentTypeChangeEvent(self.driver),
-            ChassisMetadataAgentWriteEvent(self.driver),
+            ChassisOVNAgentWriteEvent(self.driver),
             PortBindingUpdateVirtualPortsEvent(driver),
             placement.ChassisBandwidthConfigEvent(driver),
         ])

neutron/tests/functional/agent/ovn/agent/test_ovn_neutron_agent.py

@@ -138,9 +138,9 @@ class TestOVNNeutronAgentMetadataExtension(TestOVNNeutronAgentBase):
 
         # Check the metadata extension is registered.
         chassis_id = uuid.UUID(self.chassis_name)
-        agent_id = uuid.uuid5(chassis_id, 'metadata_agent')
+        ovn_agent_id = uuid.uuid5(chassis_id, 'ovn_agent')
-        ext_ids = {ovn_const.OVN_AGENT_METADATA_ID_KEY: str(agent_id),
+        ext_ids = {ovn_const.OVN_AGENT_OVN_BRIDGE: 'br-int',
-                   ovn_const.OVN_AGENT_OVN_BRIDGE: 'br-int',
+                   ovn_const.OVN_AGENT_NEUTRON_ID_KEY: str(ovn_agent_id),
                    ovn_const.OVN_AGENT_NEUTRON_SB_CFG_KEY: '0',
                    }
         n_utils.wait_until_true(

neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py

@@ -756,3 +756,97 @@ class TestChassisEvent(base.BaseTestCase):
         # after it became a Gateway chassis
         self._test_handle_ha_chassis_group_changes_create(
             self.event.ROW_UPDATE)
+
+
+class TestChassisOVNAgentWriteEvent(base.BaseTestCase):
+
+    def setUp(self):
+        super().setUp()
+        self.driver = mock.MagicMock()
+        self.event = ovsdb_monitor.ChassisOVNAgentWriteEvent(self.driver)
+
+        self.chassis_private_table = fakes.FakeOvsdbTable.create_one_ovsdb_table(
+            attrs={'name': 'Chassis_Private'})
+        self.ovsdb_row = fakes.FakeOvsdbRow.create_one_ovsdb_row
+
+    def test_match_fn_no_agent_id(self):
+        # Should not match if no agent ID
+        row = self.ovsdb_row(attrs={'external_ids': {}})
+        self.assertFalse(self.event.match_fn(self.event.ROW_CREATE, row))
+
+    def test_match_fn_create_event(self):
+        # Should match CREATE events with valid agent ID
+        row = self.ovsdb_row(
+            attrs={'external_ids': {
+                ovn_const.OVN_AGENT_NEUTRON_ID_KEY: 'neutron-123'}})
+        self.assertTrue(self.event.match_fn(self.event.ROW_CREATE, row))
+
+    def test_match_fn_update_no_chassis(self):
+        # Should not match UPDATE events if no chassis
+        row = self.ovsdb_row(
+            attrs={'external_ids': {
+                ovn_const.OVN_AGENT_NEUTRON_ID_KEY: 'neutron-123'},
+                   'chassis': None})
+        old = self.ovsdb_row(attrs={'external_ids': {}})
+        self.assertFalse(self.event.match_fn(self.event.ROW_UPDATE, row, old))
+
+    def test_match_fn_update_no_old_external_ids(self):
+        # Should not match UPDATE events if old row has no external_ids
+        row = self.ovsdb_row(
+            attrs={'external_ids': {
+                ovn_const.OVN_AGENT_NEUTRON_ID_KEY: 'neutron-123'},
+                   'chassis': 'chassis-1'})
+        old = self.ovsdb_row(attrs={})
+        self.assertFalse(self.event.match_fn(self.event.ROW_UPDATE, row, old))
+
+    def test_match_fn_update_sb_cfg_changed(self):
+        # Should match UPDATE events when sb_cfg changes
+        row = self.ovsdb_row(
+            attrs={'external_ids': {
+                ovn_const.OVN_AGENT_NEUTRON_ID_KEY: 'neutron-123',
+                ovn_const.OVN_AGENT_NEUTRON_SB_CFG_KEY: '456'},
+                   'chassis': 'chassis-1'})
+        old = self.ovsdb_row(
+            attrs={'external_ids': {
+                ovn_const.OVN_AGENT_NEUTRON_ID_KEY: 'neutron-123',
+                ovn_const.OVN_AGENT_NEUTRON_SB_CFG_KEY: '123'}})
+        self.assertTrue(self.event.match_fn(self.event.ROW_UPDATE, row, old))
+
+    def test_match_fn_update_sb_cfg_unchanged(self):
+        # Should not match UPDATE events when sb_cfg is unchanged
+        row = self.ovsdb_row(
+            attrs={'external_ids': {
+                ovn_const.OVN_AGENT_NEUTRON_ID_KEY: 'neutron-123',
+                ovn_const.OVN_AGENT_NEUTRON_SB_CFG_KEY: '123'},
+                   'chassis': 'chassis-1'})
+        old = self.ovsdb_row(
+            attrs={'external_ids': {
+                ovn_const.OVN_AGENT_NEUTRON_ID_KEY: 'neutron-123',
+                ovn_const.OVN_AGENT_NEUTRON_SB_CFG_KEY: '123'}})
+        self.assertFalse(self.event.match_fn(self.event.ROW_UPDATE, row, old))
+
+    def test_run_ovn_neutron_agent(self):
+        # Test run method with neutron agent
+        row = self.ovsdb_row(
+            attrs={'external_ids': {
+                ovn_const.OVN_AGENT_NEUTRON_ID_KEY: 'neutron-123'}})
+
+        with mock.patch('neutron.plugins.ml2.drivers.ovn.agent.neutron_agent.'
+                        'AgentCache') as agent_cache:
+            self.event.run(self.event.ROW_CREATE, row, None)
+            agent_cache.assert_has_calls([
+                mock.call().update(
+                    ovn_const.OVN_NEUTRON_AGENT, row, clear_down=True)])
+
+    def test_run_metadata_agent(self):
+        # Test run method with metadata agent
+        row = self.ovsdb_row(
+            attrs={'external_ids': {
+                ovn_const.OVN_AGENT_METADATA_ID_KEY: 'metadata-456'}})
+
+        with mock.patch('neutron.plugins.ml2.drivers.ovn.agent.neutron_agent.'
+                        'AgentCache') as agent_cache:
+            self.event.run(self.event.ROW_CREATE, row, None)
+            agent_cache.assert_has_calls([
+                mock.call().update(
+                    ovn_const.OVN_METADATA_AGENT, row, clear_down=True)])