From 85ac60cc8faced17456231ee64aa0dfb4fb92688 Mon Sep 17 00:00:00 2001 From: Swaminathan Vasudevan Date: Tue, 12 Apr 2016 16:06:41 -0700 Subject: [PATCH] DVR: Fix allowed_address_pair port binding with delayed fip Today when allowed_address_pairs are configured on a dvr service port and if a floatingip is associated with the allowed_address_pair port, we inherit the dvr service port's host and the device_owner to which the port is associated. But when the floatingip is created on the allowed_address_pair port after the port is associated with a dvr service port, then we apply the right port host binding and the arp_update. This patch address the issue, by checking for the host binding when there is a new floatingip configured. If host binding is missing and if the port happens to be an allowed_address_pair port, then it checks for the associated service port and if there is a single valid and active service port, then it inherits the host binding and device owner from the dvr service port and also applies the right arp entry. If there is are more than one active ports, then it returns. Closes-Bug: #1569918 Change-Id: I80a299d3f99113f77d2e728c3d9e000d01dacebd (cherry picked from commit 3a5315ef8dbc6265ad2c47eebc1e2c42722a7cb4) --- neutron/common/utils.py | 15 ++ neutron/db/l3_dvr_db.py | 75 +++++++++- .../l3_router/test_l3_dvr_router_plugin.py | 141 ++++++++++++++++++ 3 files changed, 225 insertions(+), 6 deletions(-) diff --git a/neutron/common/utils.py b/neutron/common/utils.py index ca77300bc2d..834d2713f5a 100644 --- a/neutron/common/utils.py +++ b/neutron/common/utils.py @@ -398,6 +398,21 @@ def get_other_dvr_serviced_device_owners(): n_const.DEVICE_OWNER_DHCP] +def get_dvr_allowed_address_pair_device_owners(): + """Return device_owner names for allowed_addr_pair ports serviced by DVR + + This just returns the device owners that are used by the + allowed_address_pair ports. Right now only the device_owners shown + below are used by the allowed_address_pair ports. + Later if other device owners are used for allowed_address_pairs those + device_owners should be added to the list below. + """ + # TODO(Swami): Convert these methods to constants. + # Add the constants variable to the neutron-lib + return [n_const.DEVICE_OWNER_LOADBALANCER, + n_const.DEVICE_OWNER_LOADBALANCERV2] + + def is_dvr_serviced(device_owner): """Check if the port need to be serviced by DVR diff --git a/neutron/db/l3_dvr_db.py b/neutron/db/l3_dvr_db.py index 5cc38af5881..1487bd6c681 100644 --- a/neutron/db/l3_dvr_db.py +++ b/neutron/db/l3_dvr_db.py @@ -28,9 +28,11 @@ from neutron.callbacks import resources from neutron.common import constants as l3_const from neutron.common import exceptions as n_exc from neutron.common import utils as n_utils +from neutron.db import allowedaddresspairs_db as addr_pair_db from neutron.db import l3_agentschedulers_db as l3_sched_db from neutron.db import l3_attrs_db from neutron.db import l3_db +from neutron.db import models_v2 from neutron.extensions import l3 from neutron.extensions import portbindings from neutron.ipam import utils as ipam_utils @@ -203,6 +205,17 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin, return super(L3_NAT_with_dvr_db_mixin, self)._get_device_owner(context, router) + def _get_ports_for_allowed_address_pair_ip( + self, context, network_id, fixed_ip): + """Return all active ports associated with the allowed_addr_pair ip.""" + query = context.session.query( + models_v2.Port).filter( + models_v2.Port.id == addr_pair_db.AllowedAddressPair.port_id, + addr_pair_db.AllowedAddressPair.ip_address == fixed_ip, + models_v2.Port.network_id == network_id, + models_v2.Port.admin_state_up.is_(True)) + return query.all() + def _update_fip_assoc(self, context, fip, floatingip_db, external_port): """Override to create floating agent gw port for DVR. @@ -233,6 +246,52 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin, admin_ctx, external_port['network_id'], hostid)) LOG.debug("FIP Agent gateway port: %s", fip_agent_port) + else: + # If not hostid check if the fixed ip provided has to + # deal with allowed_address_pairs for a given service + # port. Get the port_dict, inherit the service port host + # and device owner(if it does not exist). + port = self._core_plugin.get_port( + admin_ctx, fip_port) + allowed_device_owners = ( + n_utils.get_dvr_allowed_address_pair_device_owners()) + # NOTE: We just need to deal with ports that do not + # have a device_owner and ports that are owned by the + # dvr service ports except for the compute port and + # dhcp port. + if (port['device_owner'] == "" or + port['device_owner'] in allowed_device_owners): + addr_pair_active_service_port_list = ( + self._get_ports_for_allowed_address_pair_ip( + admin_ctx, port['network_id'], + floatingip_db['fixed_ip_address'])) + if not addr_pair_active_service_port_list: + return + if len(addr_pair_active_service_port_list) > 1: + LOG.warning(_LW("Multiple active ports associated " + "with the allowed_address_pairs.")) + return + self._inherit_service_port_and_arp_update( + context, addr_pair_active_service_port_list[0], + port) + + def _inherit_service_port_and_arp_update( + self, context, service_port, allowed_address_port): + """Function inherits port host bindings for allowed_address_pair.""" + service_port_dict = self._core_plugin._make_port_dict(service_port, + None) + address_pair_list = service_port_dict.get('allowed_address_pairs') + for address_pair in address_pair_list: + updated_port = ( + self.update_unbound_allowed_address_pair_port_binding( + context, service_port_dict, + address_pair, + address_pair_port=allowed_address_port)) + if not updated_port: + LOG.warning(_LW("Allowed_address_pair port update failed: %s"), + updated_port) + self.update_arp_entry_for_dvr_service_port(context, + service_port_dict) def _get_floatingip_on_port(self, context, port_id=None): """Helper function to retrieve the fip associated with port.""" @@ -912,7 +971,8 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin, context, fip.fixed_port_id) if fip else None def update_unbound_allowed_address_pair_port_binding( - self, context, service_port_dict, port_address_pairs): + self, context, service_port_dict, + port_address_pairs, address_pair_port=None): """Update allowed address pair port with host and device_owner This function sets the host and device_owner to the port @@ -920,8 +980,9 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin, host and device_owner. """ port_addr_pair_ip = port_address_pairs['ip_address'] - address_pair_port = self._get_address_pair_active_port_with_fip( - context, service_port_dict, port_addr_pair_ip) + if not address_pair_port: + address_pair_port = self._get_address_pair_active_port_with_fip( + context, service_port_dict, port_addr_pair_ip) if address_pair_port: host = service_port_dict[portbindings.HOST_ID] dev_owner = service_port_dict['device_owner'] @@ -942,15 +1003,17 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin, return update_port def remove_unbound_allowed_address_pair_port_binding( - self, context, service_port_dict, port_address_pairs): + self, context, service_port_dict, + port_address_pairs, address_pair_port=None): """Remove allowed address pair port binding and device_owner This function clears the host and device_owner associated with the port_addr_pair_ip. """ port_addr_pair_ip = port_address_pairs['ip_address'] - address_pair_port = self._get_address_pair_active_port_with_fip( - context, service_port_dict, port_addr_pair_ip) + if not address_pair_port: + address_pair_port = self._get_address_pair_active_port_with_fip( + context, service_port_dict, port_addr_pair_ip) if address_pair_port: # Before reverting the changes, fetch the original # device owner saved in profile and update the port diff --git a/neutron/tests/functional/services/l3_router/test_l3_dvr_router_plugin.py b/neutron/tests/functional/services/l3_router/test_l3_dvr_router_plugin.py index 56c8a6ba386..a485c8edf6c 100644 --- a/neutron/tests/functional/services/l3_router/test_l3_dvr_router_plugin.py +++ b/neutron/tests/functional/services/l3_router/test_l3_dvr_router_plugin.py @@ -548,6 +548,147 @@ class L3DvrTestCase(ml2_test_base.ML2TestFramework): self.assertEqual( 0, l3_notifier.routers_updated_on_host.call_count) + def test_allowed_addr_pairs_delayed_fip_and_update_arp_entry(self): + HOST1 = 'host1' + helpers.register_l3_agent( + host=HOST1, agent_mode=constants.L3_AGENT_MODE_DVR) + HOST2 = 'host2' + helpers.register_l3_agent( + host=HOST2, agent_mode=constants.L3_AGENT_MODE_DVR) + router = self._create_router() + private_net1 = self._make_network(self.fmt, 'net1', True) + test_allocation_pools = [{'start': '10.1.0.2', + 'end': '10.1.0.20'}] + fixed_vrrp_ip = [{'ip_address': '10.1.0.201'}] + kwargs = {'arg_list': (external_net.EXTERNAL,), + external_net.EXTERNAL: True} + ext_net = self._make_network(self.fmt, '', True, **kwargs) + self._make_subnet( + self.fmt, ext_net, '10.20.0.1', '10.20.0.0/24', + ip_version=4, enable_dhcp=True) + # Set gateway to router + self.l3_plugin._update_router_gw_info( + self.context, router['id'], + {'network_id': ext_net['network']['id']}) + private_subnet1 = self._make_subnet( + self.fmt, + private_net1, + '10.1.0.1', + cidr='10.1.0.0/24', + ip_version=4, + allocation_pools=test_allocation_pools, + enable_dhcp=True) + vrrp_port = self._make_port( + self.fmt, + private_net1['network']['id'], + fixed_ips=fixed_vrrp_ip) + allowed_address_pairs = [ + {'ip_address': '10.1.0.201', + 'mac_address': vrrp_port['port']['mac_address']}] + with self.port( + subnet=private_subnet1, + device_owner=DEVICE_OWNER_COMPUTE) as int_port,\ + self.port(subnet=private_subnet1, + device_owner=DEVICE_OWNER_COMPUTE) as int_port2: + self.l3_plugin.add_router_interface( + self.context, router['id'], + {'subnet_id': private_subnet1['subnet']['id']}) + with mock.patch.object(self.l3_plugin, + '_l3_rpc_notifier') as l3_notifier: + vm_port = self.core_plugin.update_port( + self.context, int_port['port']['id'], + {'port': {portbindings.HOST_ID: HOST1}}) + vm_port_mac = vm_port['mac_address'] + vm_port_fixed_ips = vm_port['fixed_ips'] + vm_port_subnet_id = vm_port_fixed_ips[0]['subnet_id'] + vm_arp_table = { + 'ip_address': vm_port_fixed_ips[0]['ip_address'], + 'mac_address': vm_port_mac, + 'subnet_id': vm_port_subnet_id} + vm_port2 = self.core_plugin.update_port( + self.context, int_port2['port']['id'], + {'port': {portbindings.HOST_ID: HOST2}}) + l3_notifier.reset_mock() + # Now update the VM port with the allowed_address_pair + self.core_plugin.update_port( + self.context, vm_port['id'], + {'port': { + 'allowed_address_pairs': allowed_address_pairs}}) + self.core_plugin.update_port( + self.context, vm_port2['id'], + {'port': { + 'allowed_address_pairs': allowed_address_pairs}}) + self.assertEqual( + 0, l3_notifier.routers_updated_on_host.call_count) + updated_vm_port1 = self.core_plugin.get_port( + self.context, vm_port['id']) + updated_vm_port2 = self.core_plugin.get_port( + self.context, vm_port2['id']) + self.assertEqual(4, l3_notifier.add_arp_entry.call_count) + expected_allowed_address_pairs = updated_vm_port1.get( + 'allowed_address_pairs') + self.assertEqual(expected_allowed_address_pairs, + allowed_address_pairs) + expected_allowed_address_pairs_2 = updated_vm_port2.get( + 'allowed_address_pairs') + self.assertEqual(expected_allowed_address_pairs_2, + allowed_address_pairs) + # Now the VRRP port is attached to the VM port. At this + # point, the VRRP port should not have inherited the + # port host bindings from the parent VM port. + cur_vrrp_port_db = self.core_plugin.get_port( + self.context, vrrp_port['port']['id']) + self.assertNotEqual( + cur_vrrp_port_db[portbindings.HOST_ID], HOST1) + self.assertNotEqual( + cur_vrrp_port_db[portbindings.HOST_ID], HOST2) + # Before we try to associate a floatingip make sure that + # only one of the Service port associated with the + # allowed_address_pair port is active and the other one + # is DOWN + mod_vm_port2 = self.core_plugin.update_port( + self.context, updated_vm_port2['id'], + {'port': { + 'admin_state_up': False}}) + self.assertFalse(mod_vm_port2['admin_state_up']) + # Next we can try to associate the floatingip to the + # VRRP port that is already attached to the VM port + l3_notifier.reset_mock() + floating_ip = {'floating_network_id': ext_net['network']['id'], + 'router_id': router['id'], + 'port_id': vrrp_port['port']['id'], + 'tenant_id': vrrp_port['port']['tenant_id']} + floating_ip = self.l3_plugin.create_floatingip( + self.context, {'floatingip': floating_ip}) + self.assertEqual( + 2, l3_notifier.routers_updated_on_host.call_count) + self.assertEqual(3, l3_notifier.add_arp_entry.call_count) + + post_update_vrrp_port_db = self.core_plugin.get_port( + self.context, vrrp_port['port']['id']) + vrrp_port_fixed_ips = post_update_vrrp_port_db['fixed_ips'] + vrrp_port_subnet_id = vrrp_port_fixed_ips[0]['subnet_id'] + vrrp_arp_table = { + 'ip_address': vrrp_port_fixed_ips[0]['ip_address'], + 'mac_address': vm_port_mac, + 'subnet_id': vrrp_port_subnet_id} + vrrp_arp_table1 = { + 'ip_address': vrrp_port_fixed_ips[0]['ip_address'], + 'mac_address': vrrp_port['port']['mac_address'], + 'subnet_id': vrrp_port_subnet_id} + + self.assertEqual( + post_update_vrrp_port_db[portbindings.HOST_ID], HOST1) + expected_calls = [ + mock.call(self.context, + router['id'], vrrp_arp_table1), + mock.call(self.context, + router['id'], vm_arp_table), + mock.call(self.context, + router['id'], vrrp_arp_table)] + l3_notifier.add_arp_entry.assert_has_calls( + expected_calls) + def test_allowed_address_pairs_update_arp_entry(self): HOST1 = 'host1' helpers.register_l3_agent(