diff --git a/neutron/agent/l3/dvr_edge_ha_router.py b/neutron/agent/l3/dvr_edge_ha_router.py index 9c9035f4ab7..f49093919e2 100644 --- a/neutron/agent/l3/dvr_edge_ha_router.py +++ b/neutron/agent/l3/dvr_edge_ha_router.py @@ -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): diff --git a/neutron/agent/l3/dvr_edge_router.py b/neutron/agent/l3/dvr_edge_router.py index 36ba61065d1..f624da415aa 100644 --- a/neutron/agent/l3/dvr_edge_router.py +++ b/neutron/agent/l3/dvr_edge_router.py @@ -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.""" diff --git a/neutron/agent/l3/dvr_local_router.py b/neutron/agent/l3/dvr_local_router.py index 5b0f5edb4d4..961187f3313 100644 --- a/neutron/agent/l3/dvr_local_router.py +++ b/neutron/agent/l3/dvr_local_router.py @@ -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) diff --git a/neutron/agent/l3/router_info.py b/neutron/agent/l3/router_info.py index beac1c8c903..9b5f1ff36eb 100644 --- a/neutron/agent/l3/router_info.py +++ b/neutron/agent/l3/router_info.py @@ -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) diff --git a/neutron/db/l3_dvrscheduler_db.py b/neutron/db/l3_dvrscheduler_db.py index a82bfe2e957..a2a86b264cd 100644 --- a/neutron/db/l3_dvrscheduler_db.py +++ b/neutron/db/l3_dvrscheduler_db.py @@ -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: diff --git a/neutron/tests/functional/agent/l3/framework.py b/neutron/tests/functional/agent/l3/framework.py index f70786a5c6e..f21afb551ff 100644 --- a/neutron/tests/functional/agent/l3/framework.py +++ b/neutron/tests/functional/agent/l3/framework.py @@ -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( diff --git a/neutron/tests/functional/agent/l3/test_dvr_router.py b/neutron/tests/functional/agent/l3/test_dvr_router.py index ae82180d577..571896e5a7b 100644 --- a/neutron/tests/functional/agent/l3/test_dvr_router.py +++ b/neutron/tests/functional/agent/l3/test_dvr_router.py @@ -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): diff --git a/neutron/tests/unit/agent/l3/test_agent.py b/neutron/tests/unit/agent/l3/test_agent.py index 555685aae38..f02dfd3baa3 100644 --- a/neutron/tests/unit/agent/l3/test_agent.py +++ b/neutron/tests/unit/agent/l3/test_agent.py @@ -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() diff --git a/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py b/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py index ea7336a8c26..8289bfb1365 100644 --- a/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py +++ b/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py @@ -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 = {