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 25888dfbaa6..64c783db296 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py @@ -1066,7 +1066,7 @@ class OVNMechanismDriver(api.MechanismDriver): # See doc/source/design/ovn_worker.rst for more details. return [worker.MaintenanceWorker()] - def _update_dnat_entry_if_needed(self, port_id, up=True): + def _update_dnat_entry_if_needed(self, port_id): """Update DNAT entry if using distributed floating ips.""" if not self.nb_ovn: self.nb_ovn = self._ovn_client._nb_idl @@ -1087,9 +1087,9 @@ class OVNMechanismDriver(api.MechanismDriver): {ovn_const.OVN_FIP_EXT_MAC_KEY: nat['external_mac']})).execute() - 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: + 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: LOG.debug("Setting external_mac of port %s to %s", port_id, mac) self.nb_ovn.db_set( @@ -1155,7 +1155,7 @@ class OVNMechanismDriver(api.MechanismDriver): # 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, False) + self._update_dnat_entry_if_needed(port_id) admin_context = n_context.get_admin_context() try: db_port = ml2_db.get_port(admin_context, port_id) diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py index 16dca585dbb..8b61de332de 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py @@ -1117,7 +1117,7 @@ class TestOVNMechanismDriver(TestOVNMechanismDriverBase): resources.PORT, provisioning_blocks.L2_AGENT_ENTITY ) - ude.assert_called_once_with(port1['port']['id'], False) + ude.assert_called_once_with(port1['port']['id']) # If the port does NOT bellong to compute, do not notify Nova # about it's status changes @@ -1166,7 +1166,7 @@ class TestOVNMechanismDriver(TestOVNMechanismDriverBase): resources.PORT, provisioning_blocks.L2_AGENT_ENTITY ) - ude.assert_called_once_with(port1['port']['id'], False) + ude.assert_called_once_with(port1['port']['id']) def test_bind_port_unsupported_vnic_type(self): fake_port = fakes.FakePort.create_one_port( @@ -2406,9 +2406,10 @@ class TestOVNMechanismDriver(TestOVNMechanismDriverBase): self.assertTrue(agent.alive, "Agent of type %s alive=%s" % ( agent.agent_type, agent.alive)) - def _test__update_dnat_entry_if_needed(self, up=True): - ovn_conf.cfg.CONF.set_override( - 'enable_distributed_floating_ip', True, group='ovn') + 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') port_id = 'fake-port-id' fake_ext_mac_key = 'fake-ext-mac-key' fake_nat_uuid = uuidutils.generate_uuid() @@ -2421,22 +2422,24 @@ class TestOVNMechanismDriver(TestOVNMechanismDriverBase): 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, up=up) + self.mech_driver._update_dnat_entry_if_needed(port_id) - if up: + if dvr: # 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_up(self): + def test__update_dnat_entry_if_needed_dvr(self): self._test__update_dnat_entry_if_needed() - def test__update_dnat_entry_if_needed_down(self): - self._test__update_dnat_entry_if_needed(up=False) + def test__update_dnat_entry_if_needed_no_dvr(self): + self._test__update_dnat_entry_if_needed(dvr=False) @mock.patch('neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb.' 'ovn_client.OVNClient._get_router_ports') diff --git a/releasenotes/notes/dvr-external-mac-934409413e515eb2.yaml b/releasenotes/notes/dvr-external-mac-934409413e515eb2.yaml new file mode 100644 index 00000000000..fd4b8b5f7b8 --- /dev/null +++ b/releasenotes/notes/dvr-external-mac-934409413e515eb2.yaml @@ -0,0 +1,10 @@ +--- +other: + - | + The external_mac entry in the NAT table is used to distribute/centralize + the traffic to the FIPs. When there is an external_mac set the traffic + is distributed (DVR). When it is empty it is centralized through the + gateway port (no DVR). Upon port status transition to down, the + external_mac was removed regardless of DVR being enabled or not, leading + to centralize the FIP traffic for DVR -- though it was for down ports that + won't accept traffic anyway.