openstack
/
neutron
Code Issues Proposed changes

DVR: Centralized FloatingIPs are not cleared after migration. 

With DVR routers, if a port is associated with a FloatingIP,
before it is used by a VM, the FloatingIP will be initially
started at the Network Node SNAT Namespace, since the port
is not bound to any host.

Then when the port is attached to a VM, the port gets its
host binding, and then the FloatingIP setup should be migrated
to the Compute host and the original FloatingIP in the Network
Node SNAT Namespace should be cleared.

But the original FloatingIP setup in SNAT Namespace was not
cleared by the agent.

This patch addresses the issue.

Change-Id: I55a16bcc0020087aa1abe76f5bc85cd64ccdaecd
Closes-Bug: #1796491
(cherry picked from commit cd0cc47a6a)
This commit is contained in:
Swaminathan Vasudevan 2018-10-11 23:25:44 -07:00 committed by Manuel Rodriguez
parent c0a834ed9e
commit 7ce626b407
9 changed files with 85 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
neutron/agent/l3/dvr_edge_ha_router.py
View File

@ -76,9 +76,12 @@ class DvrEdgeHaRouter(dvr_edge_router.DvrEdgeRouter,
super(DvrEdgeHaRouter, self).remove_centralized_floatingip(
fip_cidr)
def _get_centralized_fip_cidr_set(self):
def get_centralized_fip_cidr_set(self):
ex_gw_port = self.get_ex_gw_port()
if not ex_gw_port:
return set()
interface_name = self.get_snat_external_device_interface_name(
self.get_ex_gw_port())
ex_gw_port)
return set(self._get_cidrs_from_keepalived(interface_name))
def external_gateway_added(self, ex_gw_port, interface_name):

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

@ -267,10 +267,13 @@ class DvrEdgeRouter(dvr_local_router.DvrLocalRouter):
long_name = router.EXTERNAL_DEV_PREFIX + ex_gw_port['id']
return long_name[:self.driver.DEV_NAME_LEN]
def _get_centralized_fip_cidr_set(self):
def get_centralized_fip_cidr_set(self):
"""Returns the fip_cidr set for centralized floatingips."""
ex_gw_port = self.get_ex_gw_port()
if not ex_gw_port:
return set()
interface_name = self.get_snat_external_device_interface_name(
self.get_ex_gw_port())
ex_gw_port)
device = ip_lib.IPDevice(
interface_name, namespace=self.snat_namespace.name)
return set([addr['cidr'] for addr in device.addr.list()])
@ -285,11 +288,10 @@ class DvrEdgeRouter(dvr_local_router.DvrLocalRouter):
"""
fip_cidrs = super(DvrEdgeRouter, self).get_router_cidrs(device)
centralized_cidrs = set()
# Call _get_centralized_fip_cidr only when snat_namespace exists
# Call get_centralized_fip_cidr_set only when snat_namespace exists
if self.get_ex_gw_port() and self.snat_namespace.exists():
centralized_cidrs = self._get_centralized_fip_cidr_set()
existing_centralized_cidrs = self.centralized_floatingips_set
return fip_cidrs | centralized_cidrs | existing_centralized_cidrs
centralized_cidrs = self.get_centralized_fip_cidr_set()
return fip_cidrs | centralized_cidrs
def remove_centralized_floatingip(self, fip_cidr):
"""Function to handle the centralized Floatingip remove."""

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

@ -42,16 +42,12 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase):
super(DvrLocalRouter, self).__init__(host, *args, **kwargs)
self.floating_ips_dict = {}
self.centralized_floatingips_set = set()
# Linklocal subnet for router and floating IP namespace link
self.rtr_fip_subnet = None
self.rtr_fip_connect = False
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'])
@ -113,8 +109,6 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase):
"""Add floating IP to respective namespace based on agent mode."""
if fip.get(lib_constants.DVR_SNAT_BOUND):
floating_ip_status = self.add_centralized_floatingip(fip, fip_cidr)
if floating_ip_status == lib_constants.FLOATINGIP_STATUS_ACTIVE:
self.centralized_floatingips_set.add(fip_cidr)
return floating_ip_status
if not self._check_if_floatingip_bound_to_host(fip):
# TODO(Swami): Need to figure out what status
@ -172,9 +166,9 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase):
def floating_ip_removed_dist(self, fip_cidr):
"""Remove floating IP from FIP namespace."""
if fip_cidr in self.centralized_floatingips_set:
centralized_fip_cidrs = self.get_centralized_fip_cidr_set()
if fip_cidr in centralized_fip_cidrs:
self.remove_centralized_floatingip(fip_cidr)
self.centralized_floatingips_set.remove(fip_cidr)
return
floating_ip = fip_cidr.split('/')[0]
fip_2_rtr_name = self.fip_ns.get_int_device_name(self.router_id)

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

@ -326,7 +326,7 @@ 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):
def get_centralized_fip_cidr_set(self):
return set()
def process_floating_ip_addresses(self, interface_name):
@ -346,7 +346,7 @@ class RouterInfo(object):
existing_cidrs = self.get_router_cidrs(device)
new_cidrs = set()
gw_cidrs = self._get_gw_ips_cidr()
centralized_fip_cidrs = self.get_centralized_fip_cidr_set()
floating_ips = self.get_floating_ips()
# Loop once to ensure that floating ips are configured.
for fip in floating_ips:
@ -354,7 +354,6 @@ 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(
@ -369,7 +368,7 @@ 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
elif (ip_cidr in centralized_fip_cidrs and
fip.get('host') == self.host):
LOG.debug("Floating IP is migrating from centralized "
"to distributed: %s", fip)

5
neutron/db/l3_dvrscheduler_db.py
View File

@ -586,12 +586,11 @@ 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
# 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:
if (not original_port[portbindings.HOST_ID] and
not original_port['device_owner']):
l3plugin.dvr_handle_new_service_port(context, new_port,
unbound_migrate=True)
else:

5
neutron/tests/functional/agent/l3/framework.py
View File

@ -653,6 +653,11 @@ class L3AgentTestFramework(base.BaseSudoTestCase):
self._assert_ip_address_on_interface(namespace, interface,
ip_address)
def _assert_ip_address_not_on_interface(
self, namespace, interface, ip_address):
self.assertNotIn(
ip_address, self._get_addresses_on_device(namespace, interface))
def _assert_ip_address_on_interface(self,
namespace, interface, ip_address):
self.assertIn(

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

@ -914,6 +914,14 @@ class TestDvrRouter(framework.L3AgentTestFramework):
str(iptables_manager.IptablesRule(rule[0], rule[1])), rules)
return True
def _assert_iptables_rules_not_exist(
self, router_iptables_manager, table_name, expected_rules):
rules = router_iptables_manager.get_rules_for_table(table_name)
for rule in expected_rules:
self.assertNotIn(
str(iptables_manager.IptablesRule(rule[0], rule[1])), rules)
return True
def test_prevent_snat_rule_exist_on_restarted_agent(self):
self.agent.conf.agent_mode = 'dvr_snat'
router_info = self.generate_dvr_router_info()
@ -1108,8 +1116,8 @@ 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."""
def test_floating_ip_migrate_when_unbound_port_is_bound_to_a_host(self):
"""Test to check floating ips migrate from unbound 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,
@ -1154,8 +1162,18 @@ class TestDvrRouter(framework.L3AgentTestFramework):
qrouter_ns = router_updated.ns_name
fixed_ip_dist = distributed_fip['fixed_ip_address']
self._assert_snat_namespace_exists(router_updated)
snat_ns = router_updated.snat_namespace.name
fixed_ip_cent = centralized_floatingip['fixed_ip_address']
router_updated.get_centralized_fip_cidr_set = mock.Mock(
return_value=set(["19.4.4.3/32"]))
self.assertTrue(self._assert_iptables_rules_not_exist(
router_updated.snat_iptables_manager, 'nat', expected_rules))
port = router_updated.get_ex_gw_port()
interface_name = router_updated.get_external_device_name(port['id'])
self._assert_ip_address_not_on_interface(
snat_ns, interface_name,
centralized_floatingip['floating_ip_address'])
self.assertTrue(self._fixed_ip_rule_exists(qrouter_ns, fixed_ip_dist))
self.assertFalse(self._fixed_ip_rule_exists(snat_ns, fixed_ip_dist))
self.assertTrue(self._fixed_ip_rule_exists(qrouter_ns, fixed_ip_cent))
@ -1386,7 +1404,7 @@ class TestDvrRouter(framework.L3AgentTestFramework):
@mock.patch.object(dvr_local_router.DvrLocalRouter, 'connect_rtr_2_fip')
@mock.patch.object(
dvr_ha_router.DvrEdgeHaRouter, '_get_centralized_fip_cidr_set')
dvr_ha_router.DvrEdgeHaRouter, 'get_centralized_fip_cidr_set')
def test_dvr_ha_router_with_centralized_fip_calls_keepalived_cidr(
self, connect_rtr_2_fip_mock, fip_cidr_centralized_mock):
@ -1405,7 +1423,7 @@ class TestDvrRouter(framework.L3AgentTestFramework):
@mock.patch.object(dvr_local_router.DvrLocalRouter, 'connect_rtr_2_fip')
@mock.patch.object(
dvr_edge_router.DvrEdgeRouter, '_get_centralized_fip_cidr_set')
dvr_edge_router.DvrEdgeRouter, 'get_centralized_fip_cidr_set')
def test_dvr_router_with_centralized_fip_calls_keepalived_cidr(
self, connect_rtr_2_fip_mock, fip_cidr_centralized_mock):

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

@ -1145,8 +1145,6 @@ 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'):
@ -1331,6 +1329,8 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
with mock.patch.object(ri, 'get_floating_ips') as fips, \
mock.patch.object(ri,
'add_centralized_floatingip') as add_fip, \
mock.patch.object(ri, 'get_centralized_fip_cidr_set'
) as get_fip_cidrs, \
mock.patch.object(ri, 'get_floating_agent_gw_interface'
) as fip_gw_port, \
mock.patch.object(ri.fip_ns,
@ -1351,22 +1351,20 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
status = ri.floating_ip_added_dist(fips, "192.168.0.1/32")
add_fip.assert_called_once_with(fips, "192.168.0.1/32")
self.assertEqual(lib_constants.FLOATINGIP_STATUS_ACTIVE, status)
self.assertEqual(set(["192.168.0.1/32"]),
ri.centralized_floatingips_set)
# Now let us add the second fip
status = ri.floating_ip_added_dist(fips, "192.168.0.2/32")
self.assertEqual(lib_constants.FLOATINGIP_STATUS_ACTIVE, status)
self.assertEqual(set(["192.168.0.2/32", "192.168.0.1/32"]),
ri.centralized_floatingips_set)
device = mock.Mock()
get_fip_cidrs.return_value = set(
["192.168.0.2/32", "192.168.0.1/32"])
self.assertEqual(set(["192.168.0.2/32", "192.168.0.1/32"]),
ri.get_router_cidrs(device))
ri.floating_ip_removed_dist("192.168.0.1/32")
rem_fip.assert_called_once_with("192.168.0.1/32")
self.assertTrue(get_fip_cidrs.called)
get_fip_cidrs.return_value = set(["192.168.0.2/32"])
self.assertEqual(set(["192.168.0.2/32"]),
ri.get_router_cidrs(device))
self.assertEqual(set(["192.168.0.2/32"]),
ri.centralized_floatingips_set)
@mock.patch.object(lla.LinkLocalAllocator, '_write')
def test_create_dvr_fip_interfaces_for_late_binding(self, lla_write):
@ -1984,7 +1982,7 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
agent.plugin_rpc, 'update_floatingip_statuses'
) as mock_update_fip_status,\
mock.patch.object(
ri, 'get_centralized_router_cidrs') as cent_cidrs,\
ri, 'get_centralized_fip_cidr_set') 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(
@ -2060,7 +2058,7 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
agent.plugin_rpc, 'update_floatingip_statuses'
) as mock_update_fip_status,\
mock.patch.object(
ri, 'get_centralized_router_cidrs') as cent_cidrs,\
ri, 'get_centralized_fip_cidr_set') 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()
@ -2088,8 +2086,6 @@ 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(
@ -3769,8 +3765,6 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
pf_used_fip + need_to_remove_fip + [gw_cidr])
ri.iptables_manager.ipv4['nat'] = mock.MagicMock()
mock_get_gw_cidr.return_value = set([gw_cidr['cidr']])
ri.get_centralized_router_cidrs = mock.Mock(
return_value=set())
ri.add_floating_ip = mock.Mock(
return_value=lib_constants.FLOATINGIP_STATUS_ACTIVE)
ri.remove_floating_ip = mock.Mock()

30
neutron/tests/unit/scheduler/test_l3_agent_scheduler.py
View File

@ -881,6 +881,36 @@ class L3DvrSchedulerTestCase(L3SchedulerBaseMixin,
self.assertTrue(
l3plugin.update_arp_entry_for_dvr_service_port.called)
def test__notify_l3_agent_when_unbound_port_migrates_to_bound_host(self):
port_id = 'fake-port'
kwargs = {
'context': self.adminContext,
'original_port': {
'id': port_id,
portbindings.HOST_ID: '',
'device_owner': '',
'admin_state_up': True,
},
'port': {
'id': port_id,
portbindings.HOST_ID: 'vm-host',
'device_owner': DEVICE_OWNER_COMPUTE,
'mac_address': '02:04:05:17:18:19'
},
}
port = kwargs.get('port')
plugin = directory.get_plugin()
l3plugin = mock.Mock()
l3plugin.supported_extension_aliases = [
'router', constants.L3_AGENT_SCHEDULER_EXT_ALIAS,
constants.L3_DISTRIBUTED_EXT_ALIAS
]
directory.add_plugin(plugin_constants.L3, l3plugin)
l3_dvrscheduler_db._notify_l3_agent_port_update(
'port', 'after_update', plugin, **kwargs)
l3plugin.dvr_handle_new_service_port.assert_called_once_with(
self.adminContext, port, unbound_migrate=True)
def test__notify_l3_agent_update_port_no_removing_routers(self):
port_id = 'fake-port'
kwargs = {