DVR: Fix TypeError in arp update with allowed_address_pairs
There is a TyperError seen when trying to update an arp entry for the DVR service port that has associated allowed address pairs. The _get_allowed_address_pair_fixed_ips were not returing the fixed_ips dictionary but just returning the fixed_ip string. This caused the TypeError to occur. In addition to the fixed_ip it should also return the subnet_id for arp_update and it was returning it. This patch fixes the issues mentioned above. Closes-Bug: #1566046 Change-Id: I35d3145682e23e087c17129bb9556946e0aa801c
This commit is contained in:
parent
ba8c051401
commit
3d127dd767
|
@ -34,6 +34,7 @@ from neutron.db import l3_attrs_db
|
|||
from neutron.db import l3_db
|
||||
from neutron.extensions import l3
|
||||
from neutron.extensions import portbindings
|
||||
from neutron.ipam import utils as ipam_utils
|
||||
from neutron import manager
|
||||
from neutron.plugins.common import constants
|
||||
from neutron.plugins.common import utils as p_utils
|
||||
|
@ -745,14 +746,31 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin,
|
|||
return (n_utils.is_dvr_serviced(port_dict['device_owner']) and
|
||||
port_dict['fixed_ips'])
|
||||
|
||||
def _get_allowed_address_pair_fixed_ips(self, port_dict):
|
||||
def _get_subnet_id_for_given_fixed_ip(
|
||||
self, context, fixed_ip, port_dict):
|
||||
"""Returns the subnet_id that matches the fixedip on a network."""
|
||||
filters = {'network_id': [port_dict['network_id']]}
|
||||
subnets = self._core_plugin.get_subnets(context, filters)
|
||||
for subnet in subnets:
|
||||
if ipam_utils.check_subnet_ip(subnet['cidr'], fixed_ip):
|
||||
return subnet['id']
|
||||
|
||||
def _get_allowed_address_pair_fixed_ips(self, context, port_dict):
|
||||
"""Returns all fixed_ips associated with the allowed_address_pair."""
|
||||
aa_pair_fixed_ips = []
|
||||
if port_dict.get('allowed_address_pairs'):
|
||||
for address_pair in port_dict['allowed_address_pairs']:
|
||||
aap_ip_cidr = address_pair['ip_address'].split("/")
|
||||
if len(aap_ip_cidr) == 1 or int(aap_ip_cidr[1]) == 32:
|
||||
aa_pair_fixed_ips.append(aap_ip_cidr[0])
|
||||
subnet_id = self._get_subnet_id_for_given_fixed_ip(
|
||||
context, aap_ip_cidr[0], port_dict)
|
||||
if subnet_id is not None:
|
||||
fixed_ip = {'subnet_id': subnet_id,
|
||||
'ip_address': aap_ip_cidr[0]}
|
||||
aa_pair_fixed_ips.append(fixed_ip)
|
||||
else:
|
||||
LOG.debug("Subnet does not match for the given "
|
||||
"fixed_ip %s for arp update", aap_ip_cidr[0])
|
||||
return aa_pair_fixed_ips
|
||||
|
||||
def update_arp_entry_for_dvr_service_port(self, context, port_dict):
|
||||
|
@ -768,7 +786,7 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin,
|
|||
return
|
||||
fixed_ips = port_dict['fixed_ips']
|
||||
allowed_address_pair_fixed_ips = (
|
||||
self._get_allowed_address_pair_fixed_ips(port_dict))
|
||||
self._get_allowed_address_pair_fixed_ips(context, port_dict))
|
||||
changed_fixed_ips = fixed_ips + allowed_address_pair_fixed_ips
|
||||
for fixed_ip in changed_fixed_ips:
|
||||
self._generate_arp_table_and_notify_agent(
|
||||
|
@ -790,7 +808,7 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin,
|
|||
if not fixed_ips_to_delete:
|
||||
fixed_ips = port_dict['fixed_ips']
|
||||
allowed_address_pair_fixed_ips = (
|
||||
self._get_allowed_address_pair_fixed_ips(port_dict))
|
||||
self._get_allowed_address_pair_fixed_ips(context, port_dict))
|
||||
fixed_ips_to_delete = fixed_ips + allowed_address_pair_fixed_ips
|
||||
for fixed_ip in fixed_ips_to_delete:
|
||||
self._generate_arp_table_and_notify_agent(
|
||||
|
|
|
@ -358,7 +358,7 @@ def _dvr_handle_unbound_allowed_addr_pair_del(
|
|||
if updated_port:
|
||||
LOG.debug("Allowed address pair port binding removed "
|
||||
"from service port binding: %s", updated_port)
|
||||
aa_fixed_ips = plugin._get_allowed_address_pair_fixed_ips(port)
|
||||
aa_fixed_ips = plugin._get_allowed_address_pair_fixed_ips(context, port)
|
||||
if aa_fixed_ips:
|
||||
plugin.delete_arp_entry_for_dvr_service_port(
|
||||
context, port, fixed_ips_to_delete=aa_fixed_ips)
|
||||
|
@ -439,6 +439,10 @@ def _notify_l3_agent_port_update(resource, event, trigger, **kwargs):
|
|||
new_port_profile = new_port.get(portbindings.PROFILE)
|
||||
if new_port_profile:
|
||||
dest_host = new_port_profile.get('migrating_to')
|
||||
# This check is required to prevent an arp update
|
||||
# of the allowed_address_pair port.
|
||||
if new_port_profile.get('original_owner'):
|
||||
return
|
||||
# If dest_host is set, then the port profile has changed
|
||||
# and this port is in migration. The call below will
|
||||
# pre-create the router on the new host
|
||||
|
|
|
@ -453,6 +453,207 @@ class L3DvrTestCase(ml2_test_base.ML2TestFramework):
|
|||
self.assertEqual(1, len(snat_router_intfs[router1['id']]))
|
||||
self.assertEqual(1, len(fixed_ips))
|
||||
|
||||
def test_allowed_addr_pairs_arp_update_for_port_with_original_owner(self):
|
||||
HOST1 = 'host1'
|
||||
helpers.register_l3_agent(
|
||||
host=HOST1, 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'],
|
||||
device_owner=constants.DEVICE_OWNER_LOADBALANCER,
|
||||
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.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}})
|
||||
l3_notifier.routers_updated_on_host.assert_called_once_with(
|
||||
self.context, {router['id']}, HOST1)
|
||||
self.assertEqual(1, l3_notifier.add_arp_entry.call_count)
|
||||
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})
|
||||
vrrp_port_db = self.core_plugin.get_port(
|
||||
self.context, vrrp_port['port']['id'])
|
||||
self.assertNotEqual(vrrp_port_db[portbindings.HOST_ID], HOST1)
|
||||
# Now update the VM port with the allowed_address_pair
|
||||
l3_notifier.reset_mock()
|
||||
self.core_plugin.update_port(
|
||||
self.context, vm_port['id'],
|
||||
{'port': {
|
||||
'allowed_address_pairs': allowed_address_pairs}})
|
||||
updated_vm_port = self.core_plugin.get_port(
|
||||
self.context, vm_port['id'])
|
||||
expected_allowed_address_pairs = updated_vm_port.get(
|
||||
'allowed_address_pairs')
|
||||
self.assertEqual(expected_allowed_address_pairs,
|
||||
allowed_address_pairs)
|
||||
cur_vrrp_port_db = self.core_plugin.get_port(
|
||||
self.context, vrrp_port['port']['id'])
|
||||
self.assertEqual(cur_vrrp_port_db[portbindings.HOST_ID], HOST1)
|
||||
self.assertTrue(cur_vrrp_port_db.get(portbindings.PROFILE))
|
||||
port_profile = cur_vrrp_port_db.get(portbindings.PROFILE)
|
||||
self.assertTrue(port_profile)
|
||||
self.assertEqual(port_profile['original_owner'],
|
||||
constants.DEVICE_OWNER_LOADBALANCER)
|
||||
l3_notifier.reset_mock()
|
||||
port_profile['new_owner'] = 'test_owner'
|
||||
self.core_plugin.update_port(
|
||||
self.context, cur_vrrp_port_db['id'],
|
||||
{'port': {portbindings.PROFILE: port_profile}})
|
||||
# Now the vrrp port should have an 'original_owner'
|
||||
# and gets updated with a new profile. In this case
|
||||
# the update triggers a notification to the neutron
|
||||
# server, but this should not trigger another arp
|
||||
# update of this port or router_updated event to the
|
||||
# agent, otherwise this will mess up with the arp
|
||||
# table in the router namespace.
|
||||
self.assertEqual(0, l3_notifier.add_arp_entry.call_count)
|
||||
self.assertEqual(
|
||||
0, l3_notifier.routers_updated_on_host.call_count)
|
||||
|
||||
def test_allowed_address_pairs_update_arp_entry(self):
|
||||
HOST1 = 'host1'
|
||||
helpers.register_l3_agent(
|
||||
host=HOST1, 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.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}
|
||||
|
||||
l3_notifier.routers_updated_on_host.assert_called_once_with(
|
||||
self.context, {router['id']}, HOST1)
|
||||
|
||||
self.assertEqual(1, l3_notifier.add_arp_entry.call_count)
|
||||
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})
|
||||
vrrp_port_db = self.core_plugin.get_port(
|
||||
self.context, vrrp_port['port']['id'])
|
||||
self.assertNotEqual(vrrp_port_db[portbindings.HOST_ID], HOST1)
|
||||
# Now update the VM port with the allowed_address_pair
|
||||
l3_notifier.reset_mock()
|
||||
self.core_plugin.update_port(
|
||||
self.context, vm_port['id'],
|
||||
{'port': {
|
||||
'allowed_address_pairs': allowed_address_pairs}})
|
||||
self.assertEqual(
|
||||
2, l3_notifier.routers_updated_on_host.call_count)
|
||||
updated_vm_port = self.core_plugin.get_port(
|
||||
self.context, vm_port['id'])
|
||||
self.assertEqual(3, l3_notifier.add_arp_entry.call_count)
|
||||
expected_allowed_address_pairs = updated_vm_port.get(
|
||||
'allowed_address_pairs')
|
||||
self.assertEqual(expected_allowed_address_pairs,
|
||||
allowed_address_pairs)
|
||||
cur_vrrp_port_db = self.core_plugin.get_port(
|
||||
self.context, vrrp_port['port']['id'])
|
||||
vrrp_port_fixed_ips = cur_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(cur_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_update_service_port_with_allowed_address_pairs(self):
|
||||
HOST1 = 'host1'
|
||||
helpers.register_l3_agent(
|
||||
|
|
Loading…
Reference in New Issue