openstack
/
neutron
Code Issues Proposed changes

DVR: Fix unbound fip port migration to bound port 

With the current change in allowing the unbound fip
to be associated with the snat node, we are seeing
that all floating IPs that are associated with an
unbound port are created at the snat node.
This is also applicable for floating IPs that are
created just before associating the port to a VM.
We have seen such scenarios in the test cases.

This is the right behavior as per design. But when
the port is bound to a host, the floating IP should
be migrated to the respective host.

This patch fixes the issue by sending notification to
the respective node, when the port is bound and also
clear the fip from the snat node.

Closes-Bug: #1718788
Change-Id: I6b1f3ffc3c3336035632f6a82d3a87b3be57b403
This commit is contained in:
Swaminathan Vasudevan 2017-09-19 06:54:14 -07:00
parent b9ecb3804c
commit 27fcf86bcb
6 changed files with 189 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
neutron/agent/l3/dvr_local_router.py
View File

@ -49,6 +49,16 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase):
self.fip_ns = None
self._pending_arp_set = set()
def get_centralized_router_cidrs(self):
return self.centralized_floatingips_set
def migrate_centralized_floating_ip(self, fip, interface_name, device):
# Remove the centralized fip first and then add fip to the host
ip_cidr = common_utils.ip_to_cidr(fip['floating_ip_address'])
self.floating_ip_removed_dist(ip_cidr)
# Now add the floating_ip to the current host
self.floating_ip_added_dist(fip, ip_cidr)
def floating_forward_rules(self, fip):
"""Override this function defined in router_info for dvr routers."""
if not self.fip_ns:

14
neutron/agent/l3/router_info.py
View File

@ -304,6 +304,9 @@ class RouterInfo(object):
def add_floating_ip(self, fip, interface_name, device):
raise NotImplementedError()
def migrate_centralized_floating_ip(self, fip, interface_name, device):
pass
def gateway_redirect_cleanup(self, rtr_interface):
pass
@ -319,6 +322,9 @@ class RouterInfo(object):
def get_router_cidrs(self, device):
return set([addr['cidr'] for addr in device.addr.list()])
def get_centralized_router_cidrs(self):
return set()
def process_floating_ip_addresses(self, interface_name):
"""Configure IP addresses on router's external gateway interface.
@ -344,6 +350,8 @@ class RouterInfo(object):
ip_cidr = common_utils.ip_to_cidr(fip_ip)
new_cidrs.add(ip_cidr)
fip_statuses[fip['id']] = lib_constants.FLOATINGIP_STATUS_ACTIVE
cent_router_cidrs = self.get_centralized_router_cidrs()
if ip_cidr not in existing_cidrs:
fip_statuses[fip['id']] = self.add_floating_ip(
fip, interface_name, device)
@ -357,6 +365,12 @@ class RouterInfo(object):
{'old': self.fip_map[fip_ip],
'new': fip['fixed_ip_address']})
fip_statuses[fip['id']] = self.move_floating_ip(fip)
elif (ip_cidr in cent_router_cidrs and
fip.get('host') == self.host):
LOG.debug("Floating IP is migrating from centralized "
"to distributed: %s", fip)
fip_statuses[fip['id']] = self.migrate_centralized_floating_ip(
fip, interface_name, device)
elif fip_statuses[fip['id']] == fip['status']:
# mark the status as not changed. we can't remove it because
# that's how the caller determines that it was removed

58
neutron/db/l3_dvrscheduler_db.py
View File

@ -79,7 +79,8 @@ class L3_DVRsch_db_mixin(l3agent_sch_db.L3AgentSchedulerDbMixin):
the state of the router and the Compute Nodes.
"""
def dvr_handle_new_service_port(self, context, port, dest_host=None):
def dvr_handle_new_service_port(self, context, port,
dest_host=None, unbound_migrate=False):
"""Handle new dvr service port creation.
When a new dvr service port is created, this function will
@ -87,6 +88,8 @@ class L3_DVRsch_db_mixin(l3agent_sch_db.L3AgentSchedulerDbMixin):
l3 agent on that node.
The 'dest_host' will provide the destination host of the port in
case of service port migration.
If an unbound port migrates and becomes a bound port, send
notification to the snat_agents and to the bound host.
"""
port_host = dest_host or port[portbindings.HOST_ID]
l3_agent_on_host = (self.get_l3_agents(
@ -106,13 +109,37 @@ class L3_DVRsch_db_mixin(l3agent_sch_db.L3AgentSchedulerDbMixin):
subnet_ids = [ip['subnet_id'] for ip in port['fixed_ips']]
router_ids = self.get_dvr_routers_by_subnet_ids(context, subnet_ids)
if router_ids:
if not router_ids:
return
agent_port_host_match = False
if unbound_migrate:
# This might be a case were it is migrating from unbound
# to a bound port.
# In that case please forward the notification to the
# snat_nodes hosting the routers.
# Make a call here to notify the snat nodes.
snat_agent_list = self.get_dvr_snat_agent_list(context)
for agent in snat_agent_list:
LOG.debug('DVR: Handle new unbound migration port, '
'host %(host)s, router_ids %(router_ids)s',
{'host': agent.host, 'router_ids': router_ids})
self.l3_rpc_notifier.routers_updated_on_host(
context, router_ids, agent.host)
if agent.host == port_host:
agent_port_host_match = True
if not agent_port_host_match:
LOG.debug('DVR: Handle new service port, host %(host)s, '
'router ids %(router_ids)s',
{'host': port_host, 'router_ids': router_ids})
self.l3_rpc_notifier.routers_updated_on_host(
context, router_ids, port_host)
def get_dvr_snat_agent_list(self, context):
agent_filters = {'agent_modes': [n_const.L3_AGENT_MODE_DVR_SNAT]}
state = agentschedulers_db.get_admin_state_up_filter()
return self.get_l3_agents(context, active=state,
filters=agent_filters)
def get_dvr_routers_by_subnet_ids(self, context, subnet_ids):
"""Gets the dvr routers on vmport subnets."""
if not subnet_ids:
@ -397,19 +424,13 @@ def _notify_l3_agent_port_update(resource, event, trigger, **kwargs):
original_port = kwargs.get('original_port')
if new_port and original_port:
original_device_owner = original_port.get('device_owner', '')
new_device_owner = new_port.get('device_owner', '')
is_new_device_dvr_serviced = n_utils.is_dvr_serviced(new_device_owner)
l3plugin = directory.get_plugin(plugin_constants.L3)
context = kwargs['context']
is_port_no_longer_serviced = (
n_utils.is_dvr_serviced(original_device_owner) and
not n_utils.is_dvr_serviced(new_device_owner))
is_port_moved = (
is_bound_port_moved = (
original_port[portbindings.HOST_ID] and
original_port[portbindings.HOST_ID] !=
new_port[portbindings.HOST_ID])
if is_port_no_longer_serviced or is_port_moved:
if is_bound_port_moved:
removed_routers = l3plugin.get_dvr_routers_to_remove(
context,
original_port)
@ -429,8 +450,6 @@ def _notify_l3_agent_port_update(resource, event, trigger, **kwargs):
l3plugin.l3_rpc_notifier.routers_updated_on_host(
context, [fip['router_id']],
original_port[portbindings.HOST_ID])
if not is_new_device_dvr_serviced:
return
is_new_port_binding_changed = (
new_port[portbindings.HOST_ID] and
(original_port[portbindings.HOST_ID] !=
@ -446,10 +465,17 @@ def _notify_l3_agent_port_update(resource, event, trigger, **kwargs):
# 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
if ((is_new_port_binding_changed or dest_host) and
is_new_device_dvr_serviced):
l3plugin.dvr_handle_new_service_port(context, new_port,
dest_host=dest_host)
# No need to check for device_owner since we are scheduling
# the routers without checking for device_owner.
# If the original_port is None, then it is a migration
# from unbound to bound.
if (is_new_port_binding_changed or dest_host):
if original_port[portbindings.HOST_ID] is None:
l3plugin.dvr_handle_new_service_port(context, new_port,
unbound_migrate=True)
else:
l3plugin.dvr_handle_new_service_port(context, new_port,
dest_host=dest_host)
l3plugin.update_arp_entry_for_dvr_service_port(
context, new_port)
return

41
neutron/tests/functional/agent/l3/test_dvr_router.py
View File

@ -490,9 +490,8 @@ class TestDvrRouter(framework.L3AgentTestFramework):
floating_ip['port_id'] = internal_ports[0]['id']
floating_ip['status'] = 'ACTIVE'
if not snat_bound_fip:
self._add_fip_agent_gw_port_info_to_router(router,
external_gw_port)
self._add_fip_agent_gw_port_info_to_router(router,
external_gw_port)
return router
def _get_fip_agent_gw_port_for_router(
@ -1019,6 +1018,42 @@ class TestDvrRouter(framework.L3AgentTestFramework):
self._assert_iptables_rules_exist(
router1.snat_iptables_manager, 'nat', expected_rules)
def test_floating_ip_migration_from_unbound_to_bound(self):
"""Test to check floating ips migrate from unboun to bound host."""
self.agent.conf.agent_mode = lib_constants.L3_AGENT_MODE_DVR_SNAT
router_info = self.generate_dvr_router_info(
enable_floating_ip=True, enable_centralized_fip=True,
enable_snat=True, snat_bound_fip=True)
router1 = self.manage_router(self.agent, router_info)
centralized_floatingips = router_info[lib_constants.FLOATINGIP_KEY]
# For private ports hosted in dvr_no_fip agent, the floatingip
# dict will contain the fip['host'] key, but the value will always
# be None to emulate the legacy router.
self.assertIsNone(centralized_floatingips[0]['host'])
self.assertTrue(self._namespace_exists(router1.ns_name))
fip_ns = router1.fip_ns.get_name()
self.assertTrue(self._namespace_exists(fip_ns))
self._assert_snat_namespace_exists(router1)
# If fips are centralized then, the DNAT rules are only
# configured in the SNAT Namespace and not in the router-ns.
router_ns = router1.ns_name
fixed_ip = centralized_floatingips[0]['fixed_ip_address']
for fip in centralized_floatingips:
expected_rules = router1.floating_forward_rules(fip)
self.assertFalse(self._assert_iptables_rules_exist(
router1.snat_iptables_manager, 'nat', expected_rules))
self.assertFalse(self._fixed_ip_rule_exists(router_ns, fixed_ip))
# Now let us edit the floatingIP info with 'host' and remove
# the 'dvr_snat_bound'
router1.router[lib_constants.FLOATINGIP_KEY][0]['host'] = (
self.agent.conf.host)
del router1.router[lib_constants.FLOATINGIP_KEY][0]['dvr_snat_bound']
self.agent._process_updated_router(router1.router)
router_updated = self.agent.router_info[router_info['id']]
router_ns = router_updated.ns_name
self.assertTrue(self._fixed_ip_rule_exists(router_ns, fixed_ip))
self.assertTrue(self._namespace_exists(fip_ns))
def test_floating_ip_not_deployed_on_dvr_no_external_agent(self):
"""Test to check floating ips not configured for dvr_no_external."""
self.agent.conf.agent_mode = n_const.L3_AGENT_MODE_DVR_NO_EXTERNAL

75
neutron/tests/functional/services/l3_router/test_l3_dvr_router_plugin.py
View File

@ -829,6 +829,81 @@ class L3DvrTestCase(L3DvrTestCaseBase):
self.assertEqual(HOST1, floatingips[0]['host'])
self.assertIsNone(floatingips[0]['dest_host'])
def test_dvr_router_unbound_floating_ip_migrate_to_bound_host(self):
HOST1 = 'host1'
helpers.register_l3_agent(
host=HOST1, agent_mode=constants.L3_AGENT_MODE_DVR)
router = self._create_router(ha=False)
private_net1 = self._make_network(self.fmt, 'net1', True)
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)
self.l3_plugin.schedule_router(self.context,
router['id'],
candidates=[self.l3_agent])
# 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,
enable_dhcp=True)
with self.port(
subnet=private_subnet1,
device_owner=DEVICE_OWNER_COMPUTE) as int_port1:
self.l3_plugin.add_router_interface(
self.context, router['id'],
{'subnet_id': private_subnet1['subnet']['id']})
router_handle = (
self.l3_plugin.list_active_sync_routers_on_active_l3_agent(
self.context, self.l3_agent['host'], [router['id']]))
self.assertEqual(self.l3_agent['host'],
router_handle[0]['gw_port_host'])
with mock.patch.object(self.l3_plugin,
'_l3_rpc_notifier') as l3_notifier:
# Next we can try to associate the floatingip to the
# VM port
floating_ip = {'floating_network_id': ext_net['network']['id'],
'router_id': router['id'],
'port_id': int_port1['port']['id'],
'tenant_id': int_port1['port']['tenant_id']}
floating_ip = self.l3_plugin.create_floatingip(
self.context, {'floatingip': floating_ip})
expected_routers_updated_calls = [
mock.call(self.context, mock.ANY, 'host0')]
l3_notifier.routers_updated_on_host.assert_has_calls(
expected_routers_updated_calls)
self.assertFalse(l3_notifier.routers_updated.called)
router_info = (
self.l3_plugin.list_active_sync_routers_on_active_l3_agent(
self.context, self.l3_agent['host'], [router['id']]))
floatingips = router_info[0][constants.FLOATINGIP_KEY]
self.assertTrue(floatingips[0][n_const.DVR_SNAT_BOUND])
# Now do the host binding to the fip port
self.core_plugin.update_port(
self.context, int_port1['port']['id'],
{'port': {portbindings.HOST_ID: HOST1}})
expected_routers_updated_calls = [
mock.call(self.context, mock.ANY, 'host0'),
mock.call(self.context, mock.ANY, HOST1)]
l3_notifier.routers_updated_on_host.assert_has_calls(
expected_routers_updated_calls)
updated_router_info = (
self.l3_plugin.list_active_sync_routers_on_active_l3_agent(
self.context, HOST1, [router['id']]))
floatingips = updated_router_info[0][constants.FLOATINGIP_KEY]
self.assertFalse(floatingips[0].get(n_const.DVR_SNAT_BOUND))
self.assertEqual(HOST1, floatingips[0]['host'])
def test_dvr_router_centralized_floating_ip(self):
HOST1 = 'host1'
helpers.register_l3_agent(

10
neutron/tests/unit/agent/l3/test_agent.py
View File

@ -1131,6 +1131,8 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
ri.iptables_manager.ipv4['nat'] = mock.MagicMock()
ex_gw_port = {'id': _uuid(), 'network_id': mock.sentinel.ext_net_id}
ri.get_centralized_router_cidrs = mock.Mock(
return_value=set())
ri.add_floating_ip = mock.Mock(
return_value=lib_constants.FLOATINGIP_STATUS_ACTIVE)
with mock.patch.object(lla.LinkLocalAllocator, '_write'):
@ -1967,7 +1969,10 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
with mock.patch.object(
agent.plugin_rpc, 'update_floatingip_statuses'
) as mock_update_fip_status,\
mock.patch.object(
ri, 'get_centralized_router_cidrs') as cent_cidrs,\
mock.patch.object(ri, 'get_router_cidrs') as mock_get_cidrs:
cent_cidrs.return_value = set()
mock_get_cidrs.return_value = set(
[fip1['floating_ip_address'] + '/32'])
ri.process()
@ -2039,8 +2044,11 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
with mock.patch.object(
agent.plugin_rpc, 'update_floatingip_statuses'
) as mock_update_fip_status,\
mock.patch.object(
ri, 'get_centralized_router_cidrs') as cent_cidrs,\
mock.patch.object(ri, 'get_router_cidrs') as mock_get_cidrs:
mock_get_cidrs.return_value = set()
cent_cidrs.return_value = set()
ri.process()
# make sure both was sent since not existed in existing cidrs
mock_update_fip_status.assert_called_once_with(
@ -2065,6 +2073,8 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
router,
**self.ri_kwargs)
ri.external_gateway_added = mock.Mock()
ri.get_centralized_router_cidrs = mock.Mock(
return_value=set())
ri.process()
# Assess the call for putting the floating IP up was performed
mock_update_fip_status.assert_called_once_with(