From a6a8a1cdb5d2f4fbcffcae607236908b6e4582da Mon Sep 17 00:00:00 2001 From: To Minh Hien Date: Sat, 8 Aug 2026 14:26:21 +0700 Subject: [PATCH 1/3] [OVN] Do not create a duplicated Logical_Switch A Logical_Switch created before persist_uuid was used has a random register UUID and is only identifiable by its "neutron-" name, as stated in change Icc27c2b8825d7f96c9dac87dec8bbb55d493d942: "The name of the LS continues to be neutron-$UUID to match existing usage and to keep lookups by name working". Two code paths still resolve the register by UUID: * The maintenance task passes the Neutron network ID to ``get_lswitch``, that since change If59ac6a6fc59382904a6cdf0790fcd7a773b7cfe requires the Logical_Switch name. The lookup then matches only a register UUID or a name equal to that raw ID, the switch is not found and ``_fix_create_update`` takes the create branch (``_fix_delete`` is affected too). * ``AddNetworkCommand.run_idl`` probes the Logical_Switch table only by register UUID, so a pre-existing register with the same name is invisible and ``may_exist`` does not protect against it. The Logical_Switch table has no index in the OVN_Northbound schema, thus ovsdb-server accepts the resulting second register with a duplicated name. The ports created afterwards are resolved by name and land on any of the two registers; the duplicated one has no router port and no metadata port, so the instances booted on it have neither gateway nor metadata. The port deletion is resolved by name too and leaves orphan Logical_Switch_Ports behind. The maintenance resource map now retrieves the register by name and ``AddNetworkCommand`` falls back to a name lookup before inserting, restoring the duplicated name check ``LsAddCommand`` performs. This is not only an upgrade artifact: ``ovs_persist_uuid_supported`` returns False on OVS older than 3.1.5/3.2.3/3.3.1, thus a freshly deployed cloud running an older OVS also creates random UUID registers and is exposed to the same duplication. Closes-Bug: #2162974 Related-Change: If59ac6a6fc59382904a6cdf0790fcd7a773b7cfe Related-Change: Icc27c2b8825d7f96c9dac87dec8bbb55d493d942 Change-Id: I18d78af03541f627ad75308248f0186beecd16ac Signed-off-by: To Minh Hien (cherry picked from commit 745d9da5895c5016f67ea37c71c9fb8c44af0e1b) --- .../drivers/ovn/mech_driver/ovsdb/commands.py | 24 ++++-- .../ovn/mech_driver/ovsdb/maintenance.py | 9 ++- .../ovn/mech_driver/test_mech_driver.py | 22 ++++++ .../ovn/mech_driver/ovsdb/test_commands.py | 75 +++++++++++++++++++ .../ovn/mech_driver/ovsdb/test_maintenance.py | 31 ++++++++ ...n-duplicated-lswitch-3f2c85b1a7d40e69.yaml | 24 ++++++ 6 files changed, 177 insertions(+), 8 deletions(-) create mode 100644 releasenotes/notes/bug-2162974-ovn-duplicated-lswitch-3f2c85b1a7d40e69.yaml diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py index a7faf1b212c..45bd7da7fba 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py @@ -145,18 +145,28 @@ def run_idl(self, txn): table = self.api.tables[self.table_name] try: ls = table.rows[self.network_uuid] + except KeyError: + # NOTE: a Logical_Switch created before persist_uuid was used has + # a random register UUID and is only found by its name. The + # Logical_Switch table has no index in the OVN_Northbound schema, + # thus ovsdb-server accepts a second register with the same name. + ls = idlutils.row_by_value(self.api.idl, self.table_name, 'name', + utils.ovn_name(self.network_uuid), + None) + + if ls is not None: if self.may_exist: self.result = rowview.RowView(ls) return msg = _("Switch %s already exists") % self.network_uuid raise RuntimeError(msg) - except KeyError: - # Adding a new LS - if utils.ovs_persist_uuid_supported(txn.idl): - ls = txn.insert(table, new_uuid=self.network_uuid, - persist_uuid=True) - else: - ls = txn.insert(table) + + # Adding a new LS + if utils.ovs_persist_uuid_supported(txn.idl): + ls = txn.insert(table, new_uuid=self.network_uuid, + persist_uuid=True) + else: + ls = txn.insert(table) self.set_columns(ls, **self.columns) ls.name = utils.ovn_name(self.network_uuid) self.result = ls.uuid diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py index 379f84bb3ee..ee14e81a79b 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py @@ -238,7 +238,7 @@ def __init__(self, ovn_client): self._resources_func_map = { ovn_const.TYPE_NETWORKS: { 'neutron_get': self._ovn_client._plugin.get_network, - 'ovn_get': self._nb_idl.get_lswitch, + 'ovn_get': self._get_lswitch, 'ovn_create': self._ovn_client.create_network, 'ovn_update': self._ovn_client.update_network, 'ovn_delete': self._ovn_client.delete_network, @@ -489,6 +489,13 @@ def check_for_inconsistencies(self): {'res_uuid': row.resource_uuid, 'res_type': row.resource_type}) + def _get_lswitch(self, net_id): + # NOTE: ``get_lswitch`` requires the Logical_Switch name, not the + # Neutron network ID. A Logical_Switch created before persist_uuid + # was used has a random register UUID that does not match the + # network ID; such register is only found by its name. + return self._nb_idl.get_lswitch(utils.ovn_name(net_id)) + def _create_lrouter_port(self, context, port): router_id = port['device_id'] iface_info = self._ovn_client._l3_plugin._add_neutron_router_interface( 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 284b47eb4bb..290ad85946a 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 @@ -129,6 +129,28 @@ def test_old_network_new_port(self): port_lsp = self.nb_api.lsp_get(port).execute(check_error=True) self.assertIn(port_lsp, n1_ls.ports) + def test_old_network_no_duplicated_lswitch(self): + if not utils.ovs_persist_uuid_supported(self.nb_api): + self.skipTest("OVS persist_uuid not supported") + mock_supported = mock.patch.object(utils, 'ovs_persist_uuid_supported', + return_value=False).start() + network = self._make_network(self.fmt, 'n1', True) + network_id = network['network']['id'] + ls_name = utils.ovn_name(network_id) + n1_ls = self.nb_api.ls_get(ls_name).execute(check_error=True) + self.assertNotEqual(uuid.UUID(network_id), n1_ls.uuid) + mock_supported.return_value = True + + # The Logical_Switch register UUID does not match the network ID; + # adding the same network again must not create a second register + # with the same name. + self.nb_api.ls_add(network_id=network_id, + may_exist=True).execute(check_error=True) + switches = [ls for ls in + self.nb_api.ls_list().execute(check_error=True) + if ls.name == ls_name] + self.assertEqual([n1_ls.uuid], [ls.uuid for ls in switches]) + class TestPortBinding(base.TestOVNFunctionalBase): diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_commands.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_commands.py index 7fb5ed19d66..9bb75b5946e 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_commands.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_commands.py @@ -19,6 +19,7 @@ from neutron.common.ovn import constants as ovn_const from neutron.common.ovn import exceptions as ovn_exc +from neutron.common.ovn import utils from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import commands from neutron.tests import base from neutron.tests.unit import fake_resources as fakes @@ -94,6 +95,80 @@ def test_check_liveness(self): self.assertNotEqual(cmd.result, old_ng_cfg) +class TestAddNetworkCommand(TestBaseCommand): + + def setUp(self): + super().setUp() + self.net_id = uuidutils.generate_uuid() + # The OVSDB backend exposes the tables both as "tables" and + # "_tables"; the fake NB IDL only defines the latter. + self.ovn_api.tables = self.ovn_api._tables + self.ls_table = self.ovn_api.tables['Logical_Switch'] + + def _test_network_add(self, persist_uuid): + fake_ls = fakes.FakeOvsdbRow.create_one_ovsdb_row() + self.transaction.insert.return_value = fake_ls + with mock.patch.object(idlutils, 'row_by_value', return_value=None), \ + mock.patch.object(utils, 'ovs_persist_uuid_supported', + return_value=persist_uuid): + cmd = commands.AddNetworkCommand(self.ovn_api, self.net_id) + cmd.run_idl(self.transaction) + if persist_uuid: + self.transaction.insert.assert_called_once_with( + self.ls_table, new_uuid=uuid.UUID(self.net_id), + persist_uuid=True) + else: + self.transaction.insert.assert_called_once_with(self.ls_table) + self.assertEqual(utils.ovn_name(self.net_id), fake_ls.name) + + def test_network_add(self): + self._test_network_add(True) + + def test_network_add_no_persist_uuid_support(self): + self._test_network_add(False) + + def test_network_add_exists_may_exist(self): + fake_ls = fakes.FakeOvsdbRow.create_one_ovsdb_row() + self.ls_table.rows[uuid.UUID(self.net_id)] = fake_ls + cmd = commands.AddNetworkCommand(self.ovn_api, self.net_id, + may_exist=True) + cmd.run_idl(self.transaction) + self.assertEqual(fake_ls.uuid, cmd.result.uuid) + self.transaction.insert.assert_not_called() + + def test_network_add_exists(self): + fake_ls = fakes.FakeOvsdbRow.create_one_ovsdb_row() + self.ls_table.rows[uuid.UUID(self.net_id)] = fake_ls + cmd = commands.AddNetworkCommand(self.ovn_api, self.net_id) + self.assertRaises(RuntimeError, cmd.run_idl, self.transaction) + self.transaction.insert.assert_not_called() + + def test_network_add_legacy_lswitch_may_exist(self): + # A Logical_Switch created before persist_uuid was used has a random + # register UUID and is only found by its name. + fake_ls = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs={'name': utils.ovn_name(self.net_id)}) + with mock.patch.object(idlutils, 'row_by_value', + return_value=fake_ls) as mock_row_by_value: + cmd = commands.AddNetworkCommand(self.ovn_api, self.net_id, + may_exist=True) + cmd.run_idl(self.transaction) + mock_row_by_value.assert_called_once_with( + self.ovn_api.idl, 'Logical_Switch', 'name', + utils.ovn_name(self.net_id), None) + self.assertEqual(fake_ls.uuid, cmd.result.uuid) + self.transaction.insert.assert_not_called() + + def test_network_add_legacy_lswitch(self): + fake_ls = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs={'name': utils.ovn_name(self.net_id)}) + with mock.patch.object(idlutils, 'row_by_value', + return_value=fake_ls): + cmd = commands.AddNetworkCommand(self.ovn_api, self.net_id) + self.assertRaises(RuntimeError, cmd.run_idl, self.transaction) + self.transaction.insert.assert_not_called() + + class TestAddLSwitchPortCommand(TestBaseCommand): def test_lswitch_not_found(self): diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py index f7a0b9e3a40..bf3c7f08868 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py @@ -234,6 +234,12 @@ def _test_fix_create_update_network(self, ovn_rev, neutron_rev): self.fake_ovn_client._plugin.get_network.return_value = self.net self.periodic._fix_create_update(self.ctx, row) + # The Logical_Switch must be retrieved by name; a register + # created before persist_uuid was used does not have the + # network ID as register UUID. + self.fake_ovn_client._nb_idl.get_lswitch.assert_called_once_with( + utils.ovn_name(self.net['id'])) + # Since the revision number was < 0, make sure create_network() # is invoked with the latest version of the object in the neutron # database @@ -253,6 +259,31 @@ def test_fix_network_create(self): def test_fix_network_update(self): self._test_fix_create_update_network(ovn_rev=5, neutron_rev=7) + def test_fix_network_legacy_lswitch(self): + # A Logical_Switch created before persist_uuid was used is only + # found by its name; the maintenance task must not create a second + # register for it. + _nb_idl = self.fake_ovn_client._nb_idl + with db_api.CONTEXT_WRITER.using(self.ctx): + self.net['revision_number'] = 7 + ovn_revision_numbers_db.create_initial_revision( + self.ctx, self.net['id'], constants.TYPE_NETWORKS, + revision_number=5) + row = ovn_revision_numbers_db.get_revision_row(self.ctx, + self.net['id']) + fake_ls = mock.Mock(external_ids={ + constants.OVN_REV_NUM_EXT_ID_KEY: 5}) + _nb_idl.get_lswitch.side_effect = ( + lambda name: fake_ls + if name == utils.ovn_name(self.net['id']) else None) + + self.fake_ovn_client._plugin.get_network.return_value = self.net + self.periodic._fix_create_update(self.ctx, row) + + self.fake_ovn_client.create_network.assert_not_called() + self.fake_ovn_client.update_network.assert_called_once_with( + self.ctx, self.net) + def _test_fix_create_update_port(self, ovn_rev, neutron_rev): _nb_idl = self.fake_ovn_client._nb_idl with db_api.CONTEXT_WRITER.using(self.ctx): diff --git a/releasenotes/notes/bug-2162974-ovn-duplicated-lswitch-3f2c85b1a7d40e69.yaml b/releasenotes/notes/bug-2162974-ovn-duplicated-lswitch-3f2c85b1a7d40e69.yaml new file mode 100644 index 00000000000..2d6c259e47f --- /dev/null +++ b/releasenotes/notes/bug-2162974-ovn-duplicated-lswitch-3f2c85b1a7d40e69.yaml @@ -0,0 +1,24 @@ +--- +fixes: + - | + [``ML2/OVN``] A ``Logical_Switch`` created before ``persist_uuid`` was + used has a random register UUID and is only identifiable by its + ``neutron-`` name. Two code paths still resolved it by + register UUID: the maintenance task, that passed the Neutron network ID + to ``get_lswitch``, and ``AddNetworkCommand``. Both missed such a + register and created a second one with the same name. Since the + ``Logical_Switch`` table has no index in the ``OVN_Northbound`` schema, + ``ovsdb-server`` accepts the duplicated register; the ports created + afterwards are resolved by name and land on any of them, and those + landing on the duplicated one have neither router port nor metadata + port. Both code paths now retrieve the ``Logical_Switch`` by name. For + more information, see bug + `2162974 `_. +upgrade: + - | + [``ML2/OVN``] Deployments affected by bug + `2162974 `_ can already + have duplicated ``Logical_Switch`` registers. This patch prevents the + creation of new duplicates but does not remove the existing ones. Check + for registers sharing the same ``neutron-`` name and delete + the one that has no router port and no ``ovn-metadata`` port. From 2eaa59633ba5a74378638897e7242999b602f3e0 Mon Sep 17 00:00:00 2001 From: Bartosz Bezak Date: Mon, 14 Sep 2026 11:32:49 +0200 Subject: [PATCH 2/3] [OVN] Add missing unit test scaffolding Test-only subset of openstack/neutron@f0f77d08ab, which landed on master after stable/2026.1 branched: the uuid/uuidutils imports in test_commands.py and FakeOvsdbTransaction.idl. The rest of that change is a feature and bumps ovsdbapp to 2.18.0, so it cannot be backported. Needed-By: #342 Signed-off-by: Bartosz Bezak --- neutron/tests/unit/fake_resources.py | 1 + .../plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_commands.py | 2 ++ 2 files changed, 3 insertions(+) diff --git a/neutron/tests/unit/fake_resources.py b/neutron/tests/unit/fake_resources.py index eee726c6c40..4b585d012b4 100644 --- a/neutron/tests/unit/fake_resources.py +++ b/neutron/tests/unit/fake_resources.py @@ -212,6 +212,7 @@ def __init__(self, **kwargs): class FakeOvsdbTransaction: def __init__(self, **kwargs): self.insert = mock.Mock() + self.idl = mock.Mock() class FakePlugin: diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_commands.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_commands.py index 9bb75b5946e..c6f892d71f4 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_commands.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_commands.py @@ -13,8 +13,10 @@ # from unittest import mock +import uuid from neutron_lib import constants as n_const +from oslo_utils import uuidutils from ovsdbapp.backend.ovs_idl import idlutils from neutron.common.ovn import constants as ovn_const From 4474c608005d100c986325a9fc2eb513abb3a1fa Mon Sep 17 00:00:00 2001 From: Eduardo Olivares Date: Mon, 4 May 2026 17:17:00 +0200 Subject: [PATCH 3/3] [OVN] Only set NAT gateway_port when distributed FIP is enabled The guard for setting gateway_port on FIP NAT entries checked reside-on-redirect-chassis on the LRP, but this option is always 'true' for non-DVR setups with provider networks, even on gateway routers pinned to a chassis. This caused northd to reject the NAT rule and not generate DNAT flows, breaking FIP connectivity after ovn-controller restart. Only set gateway_port when distributed floating IPs are enabled (is_ovn_distributed_floating_ip). Without it, routers get pinned to a chassis (LR.options.chassis), making northd classify them as L3 gateway routers that reject gateway_port on NAT rules. This check is race-free unlike checking LR.options.chassis at FIP creation time, since the router may not be pinned yet. Closes-Bug: #2150866 Change-Id: I772b687fb92eb9bbcafbf401b9e70c6124f78716 Signed-off-by: Eduardo Olivares Assisted-By: Claude Opus 4.6 (1M context) (cherry picked from commit 299b0c5c883a777d4c101fd73790a004d15e3d75) --- .../ovn/mech_driver/ovsdb/ovn_client.py | 23 +++--- .../ovn/mech_driver/test_mech_driver.py | 4 +- .../tests/unit/services/ovn_l3/test_plugin.py | 70 +++++++++++-------- 3 files changed, 54 insertions(+), 43 deletions(-) diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py index 5ba1539a1ce..0ad4f59011d 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py @@ -932,17 +932,18 @@ def _create_or_update_floatingip(self, context, floatingip, txn=None): 'options': options, } - # If OVN supports gateway_port column for NAT rules set gateway port - # uuid to floating IP without gw port reference - LP#2035281. - router_db = self._l3_plugin.get_router(admin_context, router_id) - gw_port_id = router_db.get('gw_port_id') - lrp = self._nb_idl.get_lrouter_port(gw_port_id) - # If LRP is not bound to a chassis, it means that router can be - # bound instead. In this case we do not want to define - # gateway_port LP#2083527. - if lrp.options.get( - ovn_const.LRP_OPTIONS_RESIDE_REDIR_CH) == 'true': - columns['gateway_port'] = lrp.uuid + # Set gateway_port on NAT rules when distributed floating IPs are + # enabled and the LRP is scheduled on a chassis. History: LP#2035281 + # added gateway_port support, LP#2083527 added a guard for gateway + # routers, and LP#2150866 fixed the guard to check ha_chassis_group. + if ovn_conf.is_ovn_distributed_floating_ip(): + router_db = self._l3_plugin.get_router(admin_context, router_id) + gw_port_id = router_db.get('gw_port_id') + lrp = self._nb_idl.get_lrouter_port(gw_port_id) + # If the gateway LRP is scheduled on a chassis (it has + # ha_chassis_group), then assign the gateway_port reference. + if lrp and lrp.ha_chassis_group: + columns['gateway_port'] = lrp.uuid if ovn_conf.is_ovn_distributed_floating_ip(): if self._nb_idl.lsp_get_up(floatingip['port_id']).execute(): 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 290ad85946a..10b7bb5611c 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 @@ -1634,7 +1634,9 @@ def test_create_floatingip(self): rules = self.nb_api.get_all_logical_routers_with_rports()[0] fip_rule = rules['dnat_and_snats'][0] - self.assertNotEqual([], fip_rule['gateway_port']) + # gateway_port is only set when distributed FIPs are enabled + # LP#2150866 + self.assertEqual([], fip_rule['gateway_port']) class TestRouterGWPort(_TestRouter): diff --git a/neutron/tests/unit/services/ovn_l3/test_plugin.py b/neutron/tests/unit/services/ovn_l3/test_plugin.py index 814dba209c7..ddd2cf7d8d4 100644 --- a/neutron/tests/unit/services/ovn_l3/test_plugin.py +++ b/neutron/tests/unit/services/ovn_l3/test_plugin.py @@ -1191,6 +1191,7 @@ def test_create_floatingip_distributed(self): external_ip='192.168.0.10', external_mac='00:01:02:03:04:05', logical_port='port_id', external_ids=expected_ext_ids, + gateway_port=mock.ANY, options={'stateless': 'false'}, ) @@ -1227,6 +1228,7 @@ def test_create_floatingip_distributed_logical_port_down(self): external_ip='192.168.0.10', logical_port='port_id', external_ids=expected_ext_ids, + gateway_port=mock.ANY, options={'stateless': 'false'}, ) @@ -1347,6 +1349,7 @@ def test_create_floatingip_lb_member_fip(self): logical_ip='10.0.0.10', type='dnat_and_snat', external_ids=expected_ext_ids, + gateway_port=mock.ANY, options={'stateless': 'false'}, ) @@ -1386,6 +1389,7 @@ def test_create_floatingip_lb_vip_fip(self): logical_port='port_id', type='dnat_and_snat', external_ids=expected_ext_ids, + gateway_port=mock.ANY, options={'stateless': 'false'}, ) self.l3_inst._nb_ovn.db_find_rows.assert_called_with( @@ -1396,18 +1400,18 @@ def test_create_floatingip_lb_vip_fip(self): mock.call('NAT', self.fake_ovn_nat_rule.uuid, 'external_mac'), mock.call('NAT', self.fake_ovn_nat_rule.uuid, 'logical_port')]) - def _test_create_floatingip_gateway_port_option(self, is_gw_port): + def _test_create_floatingip_gateway_port_option( + self, distributed_fip, has_hcg=False): _nb_ovn = self.l3_inst._nb_ovn _nb_ovn.is_col_present.return_value = True self._get_floatingip.return_value = {'floating_port_id': 'fip-port-id'} _nb_ovn.get_lrouter_nat_rules.return_value = [ {'external_ip': '192.168.0.10', 'logical_ip': '10.0.0.0/24', 'type': 'snat', 'uuid': 'uuid1'}] - lrp_options = {} - if is_gw_port: - lrp_options[ovn_const.LRP_OPTIONS_RESIDE_REDIR_CH] = 'true' + ha_chassis_group = ['fake-hcg-uuid'] if has_hcg else [] lrp = fake_resources.FakeOvsdbRow.create_one_ovsdb_row( - attrs={'options': lrp_options}) + attrs={'options': {}, + 'ha_chassis_group': ha_chassis_group}) _nb_ovn.get_lrouter_port.return_value = lrp self.l3_inst.get_router.return_value = self.fake_router_with_ext_gw @@ -1415,9 +1419,13 @@ def _test_create_floatingip_gateway_port_option(self, is_gw_port): self.context, states=(self.fake_floating_ip,), resource_id=self.fake_floating_ip['id'], request_body={'floatingip': self.fake_floating_ip}) - self.ovn_drv._process_floatingip_create(resources.FLOATING_IP, - events.AFTER_CREATE, - self, payload) + with mock.patch( + 'neutron.conf.plugins.ml2.drivers.ovn.ovn_conf.' + 'is_ovn_distributed_floating_ip', + return_value=distributed_fip): + self.ovn_drv._process_floatingip_create(resources.FLOATING_IP, + events.AFTER_CREATE, + self, payload) _nb_ovn.set_nat_rule_in_lrouter.assert_not_called() expected_ext_ids = { @@ -1431,33 +1439,32 @@ def _test_create_floatingip_gateway_port_option(self, is_gw_port): ovn_const.OVN_FIP_NET_ID: self.fake_floating_ip['floating_network_id']} - if is_gw_port: - _nb_ovn.add_nat_rule_in_lrouter.assert_called_once_with( - 'neutron-router-id', - type='dnat_and_snat', - logical_ip='10.0.0.10', - external_ip='192.168.0.10', - logical_port='port_id', - external_ids=expected_ext_ids, - gateway_port=lrp.uuid, - options={'stateless': 'false'}, - ) - else: - _nb_ovn.add_nat_rule_in_lrouter.assert_called_once_with( - 'neutron-router-id', - type='dnat_and_snat', - logical_ip='10.0.0.10', - external_ip='192.168.0.10', - logical_port='port_id', - external_ids=expected_ext_ids, - options={'stateless': 'false'}, - ) + expected_kwargs = { + 'type': 'dnat_and_snat', + 'logical_ip': '10.0.0.10', + 'external_ip': '192.168.0.10', + 'logical_port': 'port_id', + 'external_ids': expected_ext_ids, + 'options': {'stateless': 'false'}, + } + if distributed_fip and has_hcg: + expected_kwargs['gateway_port'] = lrp.uuid + if distributed_fip: + expected_kwargs['external_mac'] = 'aa:aa:aa:aa:aa:aa' + _nb_ovn.add_nat_rule_in_lrouter.assert_called_once_with( + 'neutron-router-id', **expected_kwargs) def test_create_floatingip_with_gateway_port(self): - self._test_create_floatingip_gateway_port_option(True) + self._test_create_floatingip_gateway_port_option( + distributed_fip=True, has_hcg=True) def test_create_floatingip_without_gateway_port(self): - self._test_create_floatingip_gateway_port_option(False) + self._test_create_floatingip_gateway_port_option( + distributed_fip=False) + + def test_create_floatingip_no_gateway_port_dfip_without_hcg(self): + self._test_create_floatingip_gateway_port_option( + distributed_fip=True, has_hcg=False) @mock.patch('neutron.db.l3_db.L3_NAT_dbonly_mixin.delete_floatingip') def test_delete_floatingip(self, df): @@ -1673,6 +1680,7 @@ def test_update_floatingip_associate_distributed(self, uf, gn): logical_ip='10.10.10.10', external_ip='192.168.0.10', external_mac='00:01:02:03:04:05', logical_port='new-port_id', external_ids=expected_ext_ids, + gateway_port=mock.ANY, options={'stateless': 'false'}, )