From e2ab1636b07cafe0552dcadc71244c17ac59c355 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20J=C3=B3zefczyk?= Date: Wed, 15 Jan 2020 10:20:49 +0000 Subject: [PATCH] Centralize traffic when LB and member has FIP When Load Balancer and its member has FIP assigned and environment is configured to use DVR the member FIP needs to be centralized. It is current core OVN limitation, that should be solved in [1]. This patch adds this mechanism to OVN Client and OVN Octavia provider driver. It covers cases: 1) FIP association on port that is a member of some LB - make it centralized. 2) FIP association on LB VIP - find a members FIPs and centralized them. 3) Add a member to LB that has FIP already configured - checks if a member has FIP and centralize it. 4) The reverse of each of the above cases. In addition I needed to extend OVN LB member external_id entry to add an information about member subnet_id in order to easly track member port from mechanism OVN driver. That means I needed also to support both old and new conventions. This patch adds also this code. Old convention: member_`member_id`_`ip_address`:`port` New convention: member_`member_id`_`ip_address`:`port`_`subnet_id` [1] https://bugzilla.redhat.com/show_bug.cgi?id=1793897 (Cherry-picked from networking-ovn 57ac38921efa6bbf0bc4a22950355256cc3ebe6d) Related-Bug: #1860662 Change-Id: I254f0ac28f7585b699a8238e01ffb37dd70282ef --- neutron/common/ovn/constants.py | 12 ++ .../ovn/mech_driver/ovsdb/ovn_client.py | 169 +++++++++++++++++- neutron/tests/unit/fake_resources.py | 1 + .../tests/unit/services/ovn_l3/test_plugin.py | 162 +++++++++++++++-- 4 files changed, 332 insertions(+), 12 deletions(-) diff --git a/neutron/common/ovn/constants.py b/neutron/common/ovn/constants.py index c007db0f60f..4d8136ad609 100644 --- a/neutron/common/ovn/constants.py +++ b/neutron/common/ovn/constants.py @@ -157,9 +157,21 @@ MAINTENANCE_DELETE_TYPE_ORDER = { # peer router port (connecting to the logical router). DEFAULT_ADDR_FOR_LSP_WITH_PEER = 'router' +# FIP ACTIONS +FIP_ACTION_ASSOCIATE = 'fip_associate' +FIP_ACTION_DISASSOCIATE = 'fip_disassociate' + # Loadbalancer constants LRP_PREFIX = "lrp-" LB_VIP_PORT_PREFIX = "ovn-lb-vip-" +LB_EXT_IDS_LS_REFS_KEY = 'ls_refs' +LB_EXT_IDS_LR_REF_KEY = 'lr_ref' +LB_EXT_IDS_POOL_PREFIX = 'pool_' +LB_EXT_IDS_LISTENER_PREFIX = 'listener_' +LB_EXT_IDS_MEMBER_PREFIX = 'member_' +LB_EXT_IDS_VIP_KEY = 'neutron:vip' +LB_EXT_IDS_VIP_FIP_KEY = 'neutron:vip_fip' +LB_EXT_IDS_VIP_PORT_ID_KEY = 'neutron:vip_port_id' # Hash Ring constants HASH_RING_NODES_TIMEOUT = 60 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 4edea85696e..9ad1b2ea883 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 @@ -841,6 +841,21 @@ class OVNClient(object): if self._nb_idl.is_col_present('NAT', 'external_ids'): columns['external_ids'] = ext_ids + # TODO(mjozefcz): Remove this workaround when OVN LB + # will support both decentralized FIPs on LB and member. + lb_member_fip = self._is_lb_member_fip(admin_context, floatingip) + if (ovn_conf.is_ovn_distributed_floating_ip() and + lb_member_fip): + LOG.warning("Port %s is configured as a member " + "of one of OVN Load_Balancers and " + "Load_Balancer has FIP assigned. " + "In order to make traffic work member " + "FIP needs to be centralized, even if " + "this environment is configured as DVR. " + "Removing logical_port and external_mac from " + "NAT entry.", floatingip['port_id']) + columns.pop('logical_port', None) + columns.pop('external_mac', None) commands.append(self._nb_idl.add_nat_rule_in_lrouter(gw_lrouter_name, **columns)) @@ -857,12 +872,158 @@ class OVNClient(object): self._nb_idl.db_set('Logical_Switch_Port', private_lsp.uuid, ('external_ids', port_fip)) ) + if not lb_member_fip: + commands.extend( + self._handle_lb_fip_cmds( + admin_context, private_lsp, + action=ovn_const.FIP_ACTION_ASSOCIATE)) else: LOG.warning("LSP for floatingip %s, has not been found! " "Cannot set FIP on VIP.", floatingip['id']) self._transaction(commands, txn=txn) + def _is_lb_member_fip(self, context, fip): + port = self._plugin.get_port( + context, fip['port_id']) + member_subnet = [ip['subnet_id'] for ip in port['fixed_ips'] + if ip['ip_address'] == fip['fixed_ip_address']] + if not member_subnet: + return False + member_subnet = member_subnet[0] + + ls = self._nb_idl.lookup( + 'Logical_Switch', utils.ovn_name(port['network_id'])) + for lb in ls.load_balancer: + for ext_id in lb.external_ids.keys(): + if ext_id.startswith(ovn_const.LB_EXT_IDS_POOL_PREFIX): + members = lb.external_ids[ext_id] + if not members: + continue + for member in members.split(','): + if ('%s:' % fip['fixed_ip_address'] in member and + '_%s' % member_subnet in member): + return True + return False + + def _handle_lb_fip_cmds(self, context, lb_lsp, + action=ovn_const.FIP_ACTION_ASSOCIATE): + commands = [] + if not ovn_conf.is_ovn_distributed_floating_ip(): + return commands + + lb_lsp_fip_port = lb_lsp.external_ids.get( + ovn_const.OVN_PORT_NAME_EXT_ID_KEY, '') + + if not lb_lsp_fip_port.startswith(ovn_const.LB_VIP_PORT_PREFIX): + return commands + + # This is a FIP on LB VIP. + # Loop over members and delete FIP external_mac/logical_port enteries. + # Find all LBs with this LSP as VIP. + lbs = self._nb_idl.db_find_rows( + 'Load_Balancer', + ('external_ids', '=', { + ovn_const.LB_EXT_IDS_VIP_PORT_ID_KEY: lb_lsp.name}) + ).execute(check_error=True) + for lb in lbs: + # GET all LS where given LB is linked. + ls_linked = [ + item + for item in self._nb_idl.db_find_rows( + 'Logical_Switch').execute(check_error=True) + if lb in item.load_balancer] + + if not ls_linked: + return + + # Find out IP addresses and subnets of configured members. + members_to_verify = [] + for ext_id in lb.external_ids.keys(): + if ext_id.startswith(ovn_const.LB_EXT_IDS_POOL_PREFIX): + members = lb.external_ids[ext_id] + if not members: + continue + for member in members.split(','): + # NOTE(mjozefcz): Remove this workaround in W release. + # Last argument of member info is a subnet_id from + # from which member comes from. + # member_`id`_`ip`:`port`_`subnet_ip` + member_info = member.split('_') + if len(member_info) >= 4: + m = {} + m['id'] = member_info[1] + m['ip'] = member_info[2].split(':')[0] + m['subnet_id'] = member_info[3] + try: + subnet = self._plugin.get_subnet( + context, m['subnet_id']) + m['network_id'] = subnet['network_id'] + members_to_verify.append(m) + except n_exc.SubnetNotFound: + LOG.debug("Cannot find subnet details " + "for OVN LB member " + "%s.", m['id']) + + # Find a member LSPs from all linked LS to this LB. + for member in members_to_verify: + ls = self._nb_idl.lookup( + 'Logical_Switch', utils.ovn_name(member['network_id'])) + for lsp in ls.ports: + if not lsp.addresses: + continue + if member['ip'] in utils.remove_macs_from_lsp_addresses( + lsp.addresses): + member['lsp'] = lsp + nats = self._nb_idl.db_find_rows( + 'NAT', + ('external_ids', '=', { + ovn_const.OVN_FIP_PORT_EXT_ID_KEY: lsp.name}) + ).execute(check_error=True) + + for nat in nats: + if action == ovn_const.FIP_ACTION_ASSOCIATE: + # NOTE(mjozefcz): We should delete logical_port + # and external_mac entries from member NAT in + # order to make traffic work. + LOG.warning( + "Port %s is configured as a member " + "of one of OVN Load_Balancers and " + "Load_Balancer has FIP assigned. " + "In order to make traffic work member " + "FIP needs to be centralized, even if " + "this environment is configured as " + "DVR. Removing logical_port and " + "external_mac from NAT entry.", + lsp.name) + commands.extend([ + self._nb_idl.db_clear( + 'NAT', nat.uuid, 'external_mac'), + self._nb_idl.db_clear( + 'NAT', nat.uuid, 'logical_port')]) + else: + # NOTE(mjozefcz): The FIP from LB VIP is + # dissassociated now. We can decentralize + # member FIPs now. + LOG.warning( + "Port %s is configured as a member " + "of one of OVN Load_Balancers and " + "Load_Balancer has FIP disassociated. " + "DVR for this port can be enabled back.", + lsp.name) + commands.append(self._nb_idl.db_set( + 'NAT', nat.uuid, + ('logical_port', lsp.name))) + port = self._plugin.get_port(context, lsp.name) + if port['status'] == const.PORT_STATUS_ACTIVE: + commands.append( + self._nb_idl.db_set( + 'NAT', nat.uuid, + ('external_mac', + port['mac_address']))) + + return commands + def _delete_floatingip(self, fip, lrouter, txn=None): commands = [self._nb_idl.delete_nat_rule_in_lrouter( lrouter, type='dnat_and_snat', @@ -878,8 +1039,12 @@ class OVNClient(object): self._nb_idl.db_remove( 'Logical_Switch_Port', private_lsp.uuid, 'external_ids', - (ovn_const.OVN_PORT_FIP_EXT_ID_KEY)) - ) + (ovn_const.OVN_PORT_FIP_EXT_ID_KEY))) + commands.extend( + self._handle_lb_fip_cmds( + n_context.get_admin_context(), + private_lsp, + action=ovn_const.FIP_ACTION_DISASSOCIATE)) except KeyError: LOG.debug("FIP %s doesn't have external_ids.", fip) self._transaction(commands, txn=txn) diff --git a/neutron/tests/unit/fake_resources.py b/neutron/tests/unit/fake_resources.py index e6b89f11d0e..44a981f437b 100644 --- a/neutron/tests/unit/fake_resources.py +++ b/neutron/tests/unit/fake_resources.py @@ -139,6 +139,7 @@ class FakeOvsdbNbOvnIdl(object): self.db_list_rows = mock.Mock() self.lsp_list = mock.MagicMock() self.db_find = mock.Mock() + self.db_find_rows = mock.Mock() self.db_set = mock.Mock() self.db_clear = mock.Mock() self.db_remove = mock.Mock() diff --git a/neutron/tests/unit/services/ovn_l3/test_plugin.py b/neutron/tests/unit/services/ovn_l3/test_plugin.py index eae1519a5ac..f89210e4025 100644 --- a/neutron/tests/unit/services/ovn_l3/test_plugin.py +++ b/neutron/tests/unit/services/ovn_l3/test_plugin.py @@ -25,6 +25,7 @@ from neutron_lib import exceptions as n_exc from neutron_lib.plugins import constants as plugin_constants from neutron_lib.plugins import directory from oslo_config import cfg +from oslo_utils import uuidutils from neutron.common.ovn import constants as ovn_const from neutron.common.ovn import utils @@ -62,6 +63,7 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): 'network_id': self.fake_network['id'], 'device_owner': 'network:router_interface', 'mac_address': 'aa:aa:aa:aa:aa:aa', + 'status': constants.PORT_STATUS_ACTIVE, 'fixed_ips': [{'ip_address': '10.0.0.100', 'subnet_id': 'subnet-id'}], 'id': 'router-port-id'} @@ -147,17 +149,65 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): self.fake_floating_ip_new = ( fake_resources.FakeFloatingIp.create_one_fip( attrs=self.fake_floating_ip_new_attrs)) - self.fake_ovn_nat_rule = { - 'logical_ip': self.fake_floating_ip['fixed_ip_address'], - 'external_ip': self.fake_floating_ip['floating_ip_address'], - 'type': 'dnat_and_snat', - 'external_ids': { - ovn_const.OVN_FIP_EXT_ID_KEY: self.fake_floating_ip['id'], - 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_ovn_nat_rule = ( + fake_resources.FakeOvsdbRow.create_one_ovsdb_row({ + 'logical_ip': self.fake_floating_ip['fixed_ip_address'], + 'external_ip': self.fake_floating_ip['floating_ip_address'], + 'type': 'dnat_and_snat', + 'external_ids': { + ovn_const.OVN_FIP_EXT_ID_KEY: self.fake_floating_ip['id'], + 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.l3_inst = directory.get_plugin(plugin_constants.L3) + self.lb_id = uuidutils.generate_uuid() + self.member_subnet = {'id': 'subnet-id', + 'ip_version': 4, + 'cidr': '10.0.0.0/24', + 'network_id': self.fake_network['id']} + self.member_id = uuidutils.generate_uuid() + self.member_port_id = uuidutils.generate_uuid() + self.member_address = '10.0.0.10' + self.member_l4_port = '80' + self.member_port = { + 'network_id': self.fake_network['id'], + 'mac_address': 'aa:aa:aa:aa:aa:aa', + 'fixed_ips': [{'ip_address': self.member_address, + 'subnet_id': self.member_subnet['id']}], + 'id': 'fake-port-id'} + self.member_lsp = fake_resources.FakeOvsdbRow.create_one_ovsdb_row( + attrs={ + 'addresses': ['10.0.0.10 ff:ff:ff:ff:ff:ff'], + 'uuid': self.member_port['id']}) + self.listener_id = uuidutils.generate_uuid() + self.pool_id = uuidutils.generate_uuid() + self.ovn_lb = mock.MagicMock() + self.ovn_lb.protocol = ['tcp'] + self.ovn_lb.uuid = uuidutils.generate_uuid() + self.member_line = ( + 'member_%s_%s:%s_%s' % + (self.member_id, self.member_address, + self.member_l4_port, self.member_subnet['id'])) + self.ovn_lb.external_ids = { + ovn_const.LB_EXT_IDS_VIP_KEY: '10.22.33.4', + ovn_const.LB_EXT_IDS_VIP_FIP_KEY: '123.123.123.123', + ovn_const.LB_EXT_IDS_VIP_PORT_ID_KEY: 'foo_port', + 'enabled': True, + 'pool_%s' % self.pool_id: self.member_line, + 'listener_%s' % self.listener_id: '80:pool_%s' % self.pool_id} + self.lb_vip_lsp = fake_resources.FakeOvsdbRow.create_one_ovsdb_row( + attrs={'external_ids': {ovn_const.OVN_PORT_NAME_EXT_ID_KEY: + '%s%s' % (ovn_const.LB_VIP_PORT_PREFIX, + self.ovn_lb.uuid)}, + 'name': uuidutils.generate_uuid(), + 'addresses': ['10.0.0.100 ff:ff:ff:ff:ff:ee'], + 'uuid': uuidutils.generate_uuid()}) + self.lb_network = fake_resources.FakeOvsdbRow.create_one_ovsdb_row( + attrs={'load_balancer': [self.ovn_lb], + 'name': 'neutron-%s' % self.fake_network['id'], + 'ports': [self.lb_vip_lsp, self.member_lsp], + 'uuid': self.fake_network['id']}) self.nb_idl = self._start_mock( 'neutron.services.ovn_l3.plugin.OVNL3RouterPlugin._ovn', new_callable=mock.PropertyMock, @@ -230,6 +280,11 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): return_value=self.admin_context) self.addCleanup(self.get_a_ctx_mock_p.stop) self.get_a_ctx_mock_p.start() + self.mock_is_lb_member_fip = mock.patch( + 'neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb.ovn_client' + '.OVNClient._is_lb_member_fip', + return_value=False) + self.mock_is_lb_member_fip.start() @mock.patch('neutron.db.l3_db.L3_NAT_dbonly_mixin.add_router_interface') def test_add_router_interface(self, func): @@ -988,6 +1043,57 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): '192.168.0.10'}))] self.l3_inst._ovn.db_set.assert_has_calls(calls) + @mock.patch('neutron.db.l3_db.L3_NAT_dbonly_mixin._get_floatingip') + @mock.patch('neutron.db.db_base_plugin_v2.NeutronDbPluginV2.get_port') + def test_create_floatingip_lb_member_fip(self, gp, gf): + self.get_a_ctx_mock_p.stop() + config.cfg.CONF.set_override( + 'enable_distributed_floating_ip', True, group='ovn') + # Stop this mock. + self.mock_is_lb_member_fip.stop() + gp.return_value = self.member_port + gf.return_value = self.fake_floating_ip + self.l3_inst._ovn.lookup.return_value = self.lb_network + self.l3_inst._ovn.get_lswitch_port.return_value = self.member_lsp + self.l3_inst.create_floatingip(self.context, 'floatingip') + # Validate that there is no external_mac and logical_port while + # setting the NAT entry. + self.l3_inst._ovn.add_nat_rule_in_lrouter.assert_called_once_with( + 'neutron-router-id', + external_ip='192.168.0.10', + logical_ip='10.0.0.10', + type='dnat_and_snat') + + @mock.patch('neutron.db.db_base_plugin_v2.NeutronDbPluginV2.get_subnet') + def test_create_floatingip_lb_vip_fip(self, gs): + self.get_a_ctx_mock_p.stop() + config.cfg.CONF.set_override( + 'enable_distributed_floating_ip', True, group='ovn') + gs.return_value = self.member_subnet + self.l3_inst._ovn.get_lswitch_port.return_value = self.lb_vip_lsp + self.l3_inst._ovn.db_find_rows.return_value.execute.side_effect = [ + [self.ovn_lb], + [self.lb_network], + [self.fake_ovn_nat_rule], + ] + self.l3_inst._ovn.lookup.return_value = self.lb_network + + self.l3_inst.create_floatingip(self.context, 'floatingip') + self.l3_inst._ovn.add_nat_rule_in_lrouter.assert_called_once_with( + 'neutron-router-id', + external_ip='192.168.0.10', + external_mac='aa:aa:aa:aa:aa:aa', + logical_ip='10.0.0.10', + logical_port='port_id', + type='dnat_and_snat') + self.l3_inst._ovn.db_find_rows.assert_called_with( + 'NAT', ('external_ids', '=', {ovn_const.OVN_FIP_PORT_EXT_ID_KEY: + self.member_lsp.name})) + # Validate that it clears external_mac/logical_port for member NAT. + self.l3_inst._ovn.db_clear.assert_has_calls([ + mock.call('NAT', self.fake_ovn_nat_rule.uuid, 'external_mac'), + mock.call('NAT', self.fake_ovn_nat_rule.uuid, 'logical_port')]) + @mock.patch('neutron.db.l3_db.L3_NAT_dbonly_mixin.delete_floatingip') def test_delete_floatingip(self, df): self.l3_inst._ovn.get_floatingip.return_value = ( @@ -999,6 +1105,42 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): logical_ip='10.0.0.10', external_ip='192.168.0.10') + @mock.patch('neutron.db.db_base_plugin_v2.NeutronDbPluginV2.get_subnet') + @mock.patch('neutron.db.l3_db.L3_NAT_dbonly_mixin._get_floatingip') + @mock.patch('neutron.db.l3_db.L3_NAT_dbonly_mixin.delete_floatingip') + def test_delete_floatingip_lb_vip_fip(self, df, gf, gs): + config.cfg.CONF.set_override( + 'enable_distributed_floating_ip', True, group='ovn') + gs.return_value = self.member_subnet + gf.return_value = self.fake_floating_ip + self.l3_inst._ovn.get_floatingip.return_value = ( + self.fake_ovn_nat_rule) + self.l3_inst._ovn.get_lswitch_port.return_value = self.lb_vip_lsp + self.l3_inst._ovn.db_find_rows.return_value.execute.side_effect = [ + [self.ovn_lb], + [self.lb_network], + [self.fake_ovn_nat_rule], + ] + self.l3_inst._ovn.lookup.return_value = self.lb_network + + self.l3_inst.delete_floatingip(self.context, 'floatingip-id') + self.l3_inst._ovn.delete_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') + self.l3_inst._ovn.db_find_rows.assert_called_with( + 'NAT', ('external_ids', '=', + {ovn_const.OVN_FIP_PORT_EXT_ID_KEY: self.member_lsp.name})) + self.l3_inst._plugin.get_port.assert_called_once_with( + mock.ANY, self.member_lsp.name) + # Validate that it adds external_mac/logical_port back. + self.l3_inst._ovn.db_set.assert_has_calls([ + mock.call('NAT', self.fake_ovn_nat_rule.uuid, + ('logical_port', self.member_lsp.name)), + mock.call('NAT', self.fake_ovn_nat_rule.uuid, + ('external_mac', 'aa:aa:aa:aa:aa:aa'))]) + @mock.patch('neutron.db.l3_db.L3_NAT_dbonly_mixin._get_floatingip') @mock.patch('neutron.db.l3_db.L3_NAT_dbonly_mixin.delete_floatingip') def test_delete_floatingip_lsp_external_id(self, df, gf):