Merge pull request #75 from stackhpc/2023.1-lb-fip-on-ovn-fix

Revert: Ensure traffic is not centralized if DVR is enabled

Author: MoteHue

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

@@ -1061,7 +1061,7 @@ def get_workers(self):
         # See doc/source/design/ovn_worker.rst for more details.
         return [worker.MaintenanceWorker()]
 
-    def _update_dnat_entry_if_needed(self, port_id):
+    def _update_dnat_entry_if_needed(self, port_id, up=True):
         """Update DNAT entry if using distributed floating ips."""
         if not self.nb_ovn:
             self.nb_ovn = self._ovn_client._nb_idl
@@ -1082,9 +1082,9 @@ def _update_dnat_entry_if_needed(self, port_id):
                                 {ovn_const.OVN_FIP_EXT_MAC_KEY:
                                  nat['external_mac']})).execute()
 
-        if ovn_conf.is_ovn_distributed_floating_ip():
-            mac = nat['external_ids'].get(ovn_const.OVN_FIP_EXT_MAC_KEY)
-            if mac and nat['external_mac'] != mac:
+        if up and ovn_conf.is_ovn_distributed_floating_ip():
+            mac = nat['external_ids'][ovn_const.OVN_FIP_EXT_MAC_KEY]
+            if nat['external_mac'] != mac:
                 LOG.debug("Setting external_mac of port %s to %s",
                           port_id, mac)
                 self.nb_ovn.db_set(
@@ -1152,7 +1152,7 @@ def set_port_status_down(self, port_id):
         # to prevent another entity from bypassing the block with its own
         # port status update.
         LOG.info("OVN reports status down for port: %s", port_id)
-        self._update_dnat_entry_if_needed(port_id)
+        self._update_dnat_entry_if_needed(port_id, False)
         admin_context = n_context.get_admin_context()
         try:
             db_port = ml2_db.get_port(admin_context, port_id)

neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py

@@ -1130,7 +1130,7 @@ def _test_set_port_status_down(self, is_compute_port=False):
                 resources.PORT,
                 provisioning_blocks.L2_AGENT_ENTITY
             )
-            ude.assert_called_once_with(port1['port']['id'])
+            ude.assert_called_once_with(port1['port']['id'], False)
 
             # If the port does NOT bellong to compute, do not notify Nova
             # about it's status changes
@@ -1175,7 +1175,7 @@ def test_set_port_status_concurrent_delete(self):
                                   side_effect=exc) as apc, \
                 mock.patch.object(self.mech_driver,
                                   '_update_dnat_entry_if_needed') as ude:
-            self.mech_driver.set_port_status_down(port1['port']['id'])
+            self.mech_driver.set_port_status_down(port1['port']['id'], False)
             apc.assert_called_once_with(
                 mock.ANY,
                 port1['port']['id'],
@@ -2382,40 +2382,32 @@ def test_agent_with_nb_cfg_timestamp_not_timeout(self):
         self.assertTrue(agent.alive, "Agent of type %s alive=%s" % (
             agent.agent_type, agent.alive))
 
-    def _test__update_dnat_entry_if_needed(self, dvr=True):
-        if dvr:
-            ovn_conf.cfg.CONF.set_override(
-                'enable_distributed_floating_ip', True, group='ovn')
+        def _test__update_dnat_entry_if_needed(self, up=True):
+        ovn_conf.cfg.CONF.set_override(
+            'enable_distributed_floating_ip', True, group='ovn')
         port_id = 'fake-port-id'
         fake_ext_mac_key = 'fake-ext-mac-key'
         fake_nat_uuid = uuidutils.generate_uuid()
         nat_row = fakes.FakeOvsdbRow.create_one_ovsdb_row(
             attrs={'_uuid': fake_nat_uuid, 'external_ids': {
                 ovn_const.OVN_FIP_EXT_MAC_KEY: fake_ext_mac_key},
                 'external_mac': 'aa:aa:aa:aa:aa:aa'})
-
         fake_db_find = mock.Mock()
         fake_db_find.execute.return_value = [nat_row]
         self.nb_ovn.db_find.return_value = fake_db_find
-
-        self.mech_driver._update_dnat_entry_if_needed(port_id)
-
-        if dvr:
+        self.mech_driver._update_dnat_entry_if_needed(port_id, up=up)
+        if up:
             # Assert that we are setting the external_mac in the NAT table
             self.nb_ovn.db_set.assert_called_once_with(
                 'NAT', fake_nat_uuid, ('external_mac', fake_ext_mac_key))
-            self.nb_ovn.db_clear.assert_not_called()
         else:
-            self.nb_ovn.db_set.assert_not_called()
             # Assert that we are cleaning the external_mac from the NAT table
             self.nb_ovn.db_clear.assert_called_once_with(
                 'NAT', fake_nat_uuid, 'external_mac')
-
-    def test__update_dnat_entry_if_needed_dvr(self):
+    def test__update_dnat_entry_if_needed_up(self):
         self._test__update_dnat_entry_if_needed()
-
-    def test__update_dnat_entry_if_needed_no_dvr(self):
-        self._test__update_dnat_entry_if_needed(dvr=False)
+    def test__update_dnat_entry_if_needed_down(self):
+        self._test__update_dnat_entry_if_needed(up=False)
 
     @mock.patch('neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb.'
                 'ovn_client.OVNClient._get_router_ports')