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
(cherry picked from commit 3d127dd767
)
This commit is contained in:
parent
41e0fcd633
commit
ca690cc4fc
|
@ -33,6 +33,7 @@ from neutron.db import l3_attrs_db
|
||||||
from neutron.db import l3_db
|
from neutron.db import l3_db
|
||||||
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 import manager
|
from neutron import manager
|
||||||
from neutron.plugins.common import constants
|
from neutron.plugins.common import constants
|
||||||
from neutron.plugins.common import utils as p_utils
|
from neutron.plugins.common import utils as p_utils
|
||||||
|
@ -744,14 +745,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
|
return (n_utils.is_dvr_serviced(port_dict['device_owner']) and
|
||||||
port_dict['fixed_ips'])
|
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."""
|
"""Returns all fixed_ips associated with the allowed_address_pair."""
|
||||||
aa_pair_fixed_ips = []
|
aa_pair_fixed_ips = []
|
||||||
if port_dict.get('allowed_address_pairs'):
|
if port_dict.get('allowed_address_pairs'):
|
||||||
for address_pair in port_dict['allowed_address_pairs']:
|
for address_pair in port_dict['allowed_address_pairs']:
|
||||||
aap_ip_cidr = address_pair['ip_address'].split("/")
|
aap_ip_cidr = address_pair['ip_address'].split("/")
|
||||||
if len(aap_ip_cidr) == 1 or int(aap_ip_cidr[1]) == 32:
|
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
|
return aa_pair_fixed_ips
|
||||||
|
|
||||||
def update_arp_entry_for_dvr_service_port(self, context, port_dict):
|
def update_arp_entry_for_dvr_service_port(self, context, port_dict):
|
||||||
|
@ -767,7 +785,7 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin,
|
||||||
return
|
return
|
||||||
fixed_ips = port_dict['fixed_ips']
|
fixed_ips = port_dict['fixed_ips']
|
||||||
allowed_address_pair_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
|
changed_fixed_ips = fixed_ips + allowed_address_pair_fixed_ips
|
||||||
for fixed_ip in changed_fixed_ips:
|
for fixed_ip in changed_fixed_ips:
|
||||||
self._generate_arp_table_and_notify_agent(
|
self._generate_arp_table_and_notify_agent(
|
||||||
|
@ -789,7 +807,7 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin,
|
||||||
if not fixed_ips_to_delete:
|
if not fixed_ips_to_delete:
|
||||||
fixed_ips = port_dict['fixed_ips']
|
fixed_ips = port_dict['fixed_ips']
|
||||||
allowed_address_pair_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
|
fixed_ips_to_delete = fixed_ips + allowed_address_pair_fixed_ips
|
||||||
for fixed_ip in fixed_ips_to_delete:
|
for fixed_ip in fixed_ips_to_delete:
|
||||||
self._generate_arp_table_and_notify_agent(
|
self._generate_arp_table_and_notify_agent(
|
||||||
|
|
|
@ -357,7 +357,7 @@ def _dvr_handle_unbound_allowed_addr_pair_del(
|
||||||
if updated_port:
|
if updated_port:
|
||||||
LOG.debug("Allowed address pair port binding removed "
|
LOG.debug("Allowed address pair port binding removed "
|
||||||
"from service port binding: %s", updated_port)
|
"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:
|
if aa_fixed_ips:
|
||||||
plugin.delete_arp_entry_for_dvr_service_port(
|
plugin.delete_arp_entry_for_dvr_service_port(
|
||||||
context, port, fixed_ips_to_delete=aa_fixed_ips)
|
context, port, fixed_ips_to_delete=aa_fixed_ips)
|
||||||
|
@ -438,6 +438,10 @@ def _notify_l3_agent_port_update(resource, event, trigger, **kwargs):
|
||||||
new_port_profile = new_port.get(portbindings.PROFILE)
|
new_port_profile = new_port.get(portbindings.PROFILE)
|
||||||
if new_port_profile:
|
if new_port_profile:
|
||||||
dest_host = new_port_profile.get('migrating_to')
|
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
|
# If dest_host is set, then the port profile has changed
|
||||||
# and this port is in migration. The call below will
|
# and this port is in migration. The call below will
|
||||||
# pre-create the router on the new host
|
# 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(snat_router_intfs[router1['id']]))
|
||||||
self.assertEqual(1, len(fixed_ips))
|
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=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'],
|
||||||
|
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=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.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):
|
def test_update_service_port_with_allowed_address_pairs(self):
|
||||||
HOST1 = 'host1'
|
HOST1 = 'host1'
|
||||||
helpers.register_l3_agent(
|
helpers.register_l3_agent(
|
||||||
|
|
Loading…
Reference in New Issue