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
This commit is contained in:
Swaminathan Vasudevan 2016-04-12 16:06:41 -07:00 committed by Brian Haley
parent d8ae9cf475
commit 3a5315ef8d
3 changed files with 225 additions and 6 deletions

View File

@ -254,6 +254,21 @@ def get_other_dvr_serviced_device_owners():
n_const.DEVICE_OWNER_DHCP] 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): def is_dvr_serviced(device_owner):
"""Check if the port need to be serviced by DVR """Check if the port need to be serviced by DVR

View File

@ -29,9 +29,11 @@ from neutron.callbacks import registry
from neutron.callbacks import resources from neutron.callbacks import resources
from neutron.common import constants as l3_const from neutron.common import constants as l3_const
from neutron.common import utils as n_utils from neutron.common import utils as n_utils
from neutron.db.allowed_address_pairs import models as addr_pair_db
from neutron.db import l3_agentschedulers_db as l3_sched_db from neutron.db import l3_agentschedulers_db as l3_sched_db
from neutron.db import l3_attrs_db from neutron.db import l3_attrs_db
from neutron.db import l3_db from neutron.db import l3_db
from neutron.db import models_v2
from neutron.extensions import l3 from neutron.extensions import l3
from neutron.extensions import portbindings from neutron.extensions import portbindings
from neutron.ipam import utils as ipam_utils from neutron.ipam import utils as ipam_utils
@ -204,6 +206,17 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin,
return super(L3_NAT_with_dvr_db_mixin, return super(L3_NAT_with_dvr_db_mixin,
self)._get_device_owner(context, router) 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 == True)
return query.all()
def _update_fip_assoc(self, context, fip, floatingip_db, external_port): def _update_fip_assoc(self, context, fip, floatingip_db, external_port):
"""Override to create floating agent gw port for DVR. """Override to create floating agent gw port for DVR.
@ -234,6 +247,52 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin,
admin_ctx, external_port['network_id'], admin_ctx, external_port['network_id'],
hostid)) hostid))
LOG.debug("FIP Agent gateway port: %s", fip_agent_port) 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): def _get_floatingip_on_port(self, context, port_id=None):
"""Helper function to retrieve the fip associated with port.""" """Helper function to retrieve the fip associated with port."""
@ -914,7 +973,8 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin,
context, fip.fixed_port_id) if fip else None context, fip.fixed_port_id) if fip else None
def update_unbound_allowed_address_pair_port_binding( 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 """Update allowed address pair port with host and device_owner
This function sets the host and device_owner to the port This function sets the host and device_owner to the port
@ -922,8 +982,9 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin,
host and device_owner. host and device_owner.
""" """
port_addr_pair_ip = port_address_pairs['ip_address'] port_addr_pair_ip = port_address_pairs['ip_address']
address_pair_port = self._get_address_pair_active_port_with_fip( if not address_pair_port:
context, service_port_dict, port_addr_pair_ip) address_pair_port = self._get_address_pair_active_port_with_fip(
context, service_port_dict, port_addr_pair_ip)
if address_pair_port: if address_pair_port:
host = service_port_dict[portbindings.HOST_ID] host = service_port_dict[portbindings.HOST_ID]
dev_owner = service_port_dict['device_owner'] dev_owner = service_port_dict['device_owner']
@ -944,15 +1005,17 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin,
return update_port return update_port
def remove_unbound_allowed_address_pair_port_binding( 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 """Remove allowed address pair port binding and device_owner
This function clears the host and device_owner associated with This function clears the host and device_owner associated with
the port_addr_pair_ip. the port_addr_pair_ip.
""" """
port_addr_pair_ip = port_address_pairs['ip_address'] port_addr_pair_ip = port_address_pairs['ip_address']
address_pair_port = self._get_address_pair_active_port_with_fip( if not address_pair_port:
context, service_port_dict, port_addr_pair_ip) address_pair_port = self._get_address_pair_active_port_with_fip(
context, service_port_dict, port_addr_pair_ip)
if address_pair_port: if address_pair_port:
# Before reverting the changes, fetch the original # Before reverting the changes, fetch the original
# device owner saved in profile and update the port # device owner saved in profile and update the port

View File

@ -548,6 +548,147 @@ class L3DvrTestCase(ml2_test_base.ML2TestFramework):
self.assertEqual( self.assertEqual(
0, l3_notifier.routers_updated_on_host.call_count) 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=n_const.L3_AGENT_MODE_DVR)
HOST2 = 'host2'
helpers.register_l3_agent(
host=HOST2, agent_mode=n_const.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): def test_allowed_address_pairs_update_arp_entry(self):
HOST1 = 'host1' HOST1 = 'host1'
helpers.register_l3_agent( helpers.register_l3_agent(