openstack/neutron
Code Issues Proposed changes

Merge "DVR: Centralized FloatingIPs are not cleared after migration." into stable/queens

Browse Source
This commit is contained in:
Zuul 2018-11-20 03:29:35 +00:00 committed by Gerrit Code Review
commit dbd3f7e7e8
9 changed files with 85 additions and 39 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

@ -268,10 +268,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()])
@ -286,11 +289,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(n_const.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

@ -324,7 +324,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):
@ -344,7 +344,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:
@ -352,7 +352,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(
@ -367,7 +366,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

@ -571,12 +571,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))
@ -1385,7 +1403,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):
@ -1404,7 +1422,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):

20
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(

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

@ -880,6 +880,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 = {