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
This commit is contained in:
parent
6e98d52eb8
commit
cd0cc47a6a
neutron
agent/l3
db
tests
functional/agent/l3
unit
@ -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):
|
||||
|
@ -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."""
|
||||
|
@ -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)
|
||||
|
@ -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)
|
||||
|
@ -496,12 +496,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:
|
||||
|
@ -658,6 +658,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(
|
||||
|
@ -918,6 +918,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()
|
||||
@ -1112,8 +1120,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,
|
||||
@ -1158,8 +1166,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))
|
||||
@ -1390,7 +1408,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):
|
||||
|
||||
@ -1409,7 +1427,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):
|
||||
|
||||
|
@ -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):
|
||||
@ -1987,7 +1985,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(
|
||||
@ -2063,7 +2061,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()
|
||||
@ -2091,8 +2089,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(
|
||||
@ -3693,8 +3689,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()
|
||||
|
@ -882,6 +882,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 = {
|
||||
|
Loading…
x
Reference in New Issue
Block a user