From fd4821f918e561fc1c973970bfe5531f58be8dca Mon Sep 17 00:00:00 2001 From: Lucas Alvares Gomes Date: Mon, 15 Jun 2020 11:45:19 +0100 Subject: [PATCH] [OVN] OVN driver to adapt to enable_distributed_floating_ip changes Prior to this patch, if the [ovn]/enable_distributed_floating_ip configuration option changed in an existing environment the OVN driver wouldn't adapt to it requiring administrators to clear up the "external_mac" column from the NAT table manually for the existing floating ips. With this patch, OVN will automatically correct existing NAT entries for floating ips whenever this option changes. To make the code simpler when handling the port up/down event this patch always set the logical_port and the neutron:fip_external_mac key in the external_ids column of the NAT entry when creating the floating ip. Note that we are not using the maintenance task for this either, we are re-using the event that set/unset the "external_mac" column for this because, whenenver the service is restarted (after the configuration is changed, we need to restart for it to take effect) the IDL will re-trigger those events. Closes-Bug: #1883559 Change-Id: I6a85fdde2558d781bf2853c5d11c5c964bbab81f Signed-off-by: Lucas Alvares Gomes (cherry picked from commit 26f6b90930509314cece4c237e4280b60bae278f) --- .../drivers/ovn/mech_driver/mech_driver.py | 24 ++++++++-------- .../ovn/mech_driver/ovsdb/ovn_client.py | 14 +++++----- .../ovn/mech_driver/test_mech_driver.py | 3 +- .../tests/unit/services/ovn_l3/test_plugin.py | 28 ++++++++++++++----- 4 files changed, 42 insertions(+), 27 deletions(-) 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 eb3c8aef1a2..a156e6b39d9 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py @@ -810,9 +810,6 @@ class OVNMechanismDriver(api.MechanismDriver): def _update_dnat_entry_if_needed(self, port_id, up=True): """Update DNAT entry if using distributed floating ips.""" - if not ovn_conf.is_ovn_distributed_floating_ip(): - return - if not self._nb_ovn: self._nb_ovn = self._ovn_client._nb_idl @@ -832,17 +829,20 @@ class OVNMechanismDriver(api.MechanismDriver): {ovn_const.OVN_FIP_EXT_MAC_KEY: nat['external_mac']})).execute() - if up: + if up and ovn_conf.is_ovn_distributed_floating_ip(): mac = nat['external_ids'][ovn_const.OVN_FIP_EXT_MAC_KEY] - LOG.debug("Setting external_mac of port %s to %s", - port_id, mac) - self._nb_ovn.db_set( - 'NAT', nat['_uuid'], - ('external_mac', mac)).execute(check_error=True) + if nat['external_mac'] != mac: + LOG.debug("Setting external_mac of port %s to %s", + port_id, mac) + self._nb_ovn.db_set( + 'NAT', nat['_uuid'], ('external_mac', mac)).execute( + check_error=True) else: - LOG.debug("Clearing up external_mac of port %s", port_id) - self._nb_ovn.db_clear( - 'NAT', nat['_uuid'], 'external_mac').execute(check_error=True) + if nat['external_mac']: + LOG.debug("Clearing up external_mac of port %s", port_id) + self._nb_ovn.db_clear( + 'NAT', nat['_uuid'], 'external_mac').execute( + check_error=True) def _should_notify_nova(self, db_port): # NOTE(twilson) It is possible for a test to override a config option 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 315caafc8da..00719a40d7b 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 @@ -739,6 +739,8 @@ class OVNClient(object): admin_context = n_context.get_admin_context() fip_db = self._l3_plugin._get_floatingip( admin_context, floatingip['id']) + port_db = self._plugin.get_port( + admin_context, fip_db['floating_port_id']) gw_lrouter_name = utils.ovn_name(router_id) # TODO(chandrav): Since the floating ip port is not @@ -756,18 +758,16 @@ class OVNClient(object): ovn_const.OVN_REV_NUM_EXT_ID_KEY: str(utils.get_revision_number( floatingip, ovn_const.TYPE_FLOATINGIPS)), ovn_const.OVN_FIP_PORT_EXT_ID_KEY: floatingip['port_id'], - ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: gw_lrouter_name} + ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: gw_lrouter_name, + ovn_const.OVN_FIP_EXT_MAC_KEY: port_db['mac_address']} columns = {'type': 'dnat_and_snat', 'logical_ip': floatingip['fixed_ip_address'], - 'external_ip': floatingip['floating_ip_address']} + 'external_ip': floatingip['floating_ip_address'], + 'logical_port': floatingip['port_id']} if ovn_conf.is_ovn_distributed_floating_ip(): - port = self._plugin.get_port( - admin_context, fip_db['floating_port_id']) - columns['logical_port'] = floatingip['port_id'] - ext_ids[ovn_const.OVN_FIP_EXT_MAC_KEY] = port['mac_address'] if self._nb_idl.lsp_get_up(floatingip['port_id']).execute(): - columns['external_mac'] = port['mac_address'] + columns['external_mac'] = port_db['mac_address'] # TODO(dalvarez): remove this check once the minimum OVS required # version contains the column (when OVS 2.8.2 is released). 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 1431c836a20..3ccad2d51da 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 @@ -1679,7 +1679,8 @@ class TestOVNMechanismDriver(test_plugin.Ml2PluginV2TestCase): 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}}) + 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] diff --git a/neutron/tests/unit/services/ovn_l3/test_plugin.py b/neutron/tests/unit/services/ovn_l3/test_plugin.py index c9aa8f3c445..9bf9ff19e85 100644 --- a/neutron/tests/unit/services/ovn_l3/test_plugin.py +++ b/neutron/tests/unit/services/ovn_l3/test_plugin.py @@ -888,12 +888,14 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): ovn_const.OVN_FIP_PORT_EXT_ID_KEY: self.fake_floating_ip['port_id'], ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: utils.ovn_name( - self.fake_floating_ip['router_id'])} + self.fake_floating_ip['router_id']), + ovn_const.OVN_FIP_EXT_MAC_KEY: 'aa:aa:aa:aa:aa:aa'} self.l3_inst._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) self.l3_inst._ovn.delete_lswitch_port.assert_called_once_with( 'fip-port-id', 'neutron-fip-net-id') @@ -959,12 +961,14 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): ovn_const.OVN_FIP_PORT_EXT_ID_KEY: self.fake_floating_ip['port_id'], ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: utils.ovn_name( - self.fake_floating_ip['router_id'])} + self.fake_floating_ip['router_id']), + ovn_const.OVN_FIP_EXT_MAC_KEY: 'aa:aa:aa:aa:aa:aa'} self.l3_inst._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) self.l3_inst._ovn.delete_lswitch_port.assert_called_once_with( 'fip-port-id', 'neutron-fip-net-id') @@ -983,12 +987,14 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): ovn_const.OVN_FIP_PORT_EXT_ID_KEY: self.fake_floating_ip['port_id'], ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: utils.ovn_name( - self.fake_floating_ip['router_id'])} + self.fake_floating_ip['router_id']), + ovn_const.OVN_FIP_EXT_MAC_KEY: 'aa:aa:aa:aa:aa:aa'} self.l3_inst._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) self.l3_inst._ovn.delete_lswitch_port.assert_called_once_with( 'fip-port-id', 'neutron-fip-net-id') @@ -1138,12 +1144,14 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): ovn_const.OVN_FIP_PORT_EXT_ID_KEY: self.fake_floating_ip_new['port_id'], ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: utils.ovn_name( - self.fake_floating_ip_new['router_id'])} + self.fake_floating_ip_new['router_id']), + ovn_const.OVN_FIP_EXT_MAC_KEY: 'aa:aa:aa:aa:aa:aa'} self.l3_inst._ovn.add_nat_rule_in_lrouter.assert_called_once_with( 'neutron-new-router-id', type='dnat_and_snat', logical_ip='10.10.10.10', external_ip='192.168.0.10', + logical_port='new-port_id', external_ids=expected_ext_ids) @mock.patch('neutron.db.extraroute_db.ExtraRoute_dbonly_mixin.' @@ -1160,12 +1168,14 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): ovn_const.OVN_FIP_PORT_EXT_ID_KEY: self.fake_floating_ip_new['port_id'], ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: utils.ovn_name( - self.fake_floating_ip_new['router_id'])} + self.fake_floating_ip_new['router_id']), + ovn_const.OVN_FIP_EXT_MAC_KEY: 'aa:aa:aa:aa:aa:aa'} self.l3_inst._ovn.add_nat_rule_in_lrouter.assert_called_once_with( 'neutron-new-router-id', type='dnat_and_snat', logical_ip='10.10.10.10', external_ip='192.168.0.10', + logical_port='new-port_id', external_ids=expected_ext_ids) @mock.patch('neutron.db.db_base_plugin_v2.NeutronDbPluginV2.get_network') @@ -1221,12 +1231,14 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): ovn_const.OVN_FIP_PORT_EXT_ID_KEY: self.fake_floating_ip_new['port_id'], ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: utils.ovn_name( - self.fake_floating_ip_new['router_id'])} + self.fake_floating_ip_new['router_id']), + ovn_const.OVN_FIP_EXT_MAC_KEY: 'aa:aa:aa:aa:aa:aa'} self.l3_inst._ovn.add_nat_rule_in_lrouter.assert_called_once_with( 'neutron-new-router-id', type='dnat_and_snat', logical_ip='10.10.10.10', external_ip='192.168.0.10', + logical_port='foo', external_ids=expected_ext_ids) @mock.patch('neutron.db.extraroute_db.ExtraRoute_dbonly_mixin.' @@ -1252,12 +1264,14 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): ovn_const.OVN_FIP_PORT_EXT_ID_KEY: self.fake_floating_ip_new['port_id'], ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: utils.ovn_name( - self.fake_floating_ip_new['router_id'])} + self.fake_floating_ip_new['router_id']), + ovn_const.OVN_FIP_EXT_MAC_KEY: 'aa:aa:aa:aa:aa:aa'} self.l3_inst._ovn.add_nat_rule_in_lrouter.assert_called_once_with( 'neutron-new-router-id', type='dnat_and_snat', logical_ip='10.10.10.10', external_ip='192.168.0.10', + logical_port='port_id', external_ids=expected_ext_ids) @mock.patch('neutron.db.l3_db.L3_NAT_dbonly_mixin.get_floatingips')