From fef04147bd68bc101c74c93d4e94f5f5b4643d6f Mon Sep 17 00:00:00 2001 From: rajeev Date: Fri, 13 Feb 2015 16:58:53 -0500 Subject: [PATCH] fix for _get_external_device_interface_name trace On removal of external gateway from DVR the code path external_gateway_removed(...) was trying to access the agent gateway port interface even when no fip namespace exists. This change checks for existence of namespace before accessing the agent gateway port interface through _get_external_device_interface_name(...) or looking for floating ips that may have been configured on the port. Change-Id: Idcf28ff93f16f1d692fe7acb678fb90aabe5af5e Closes-Bug: #1421497 --- neutron/agent/l3/agent.py | 9 +-- neutron/tests/unit/test_l3_agent.py | 85 +++++++++++++++++------------ 2 files changed, 55 insertions(+), 39 deletions(-) diff --git a/neutron/agent/l3/agent.py b/neutron/agent/l3/agent.py index c21edd693..203807d7a 100644 --- a/neutron/agent/l3/agent.py +++ b/neutron/agent/l3/agent.py @@ -899,10 +899,11 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, def external_gateway_removed(self, ri, ex_gw_port, interface_name): if ri.router['distributed']: self.process_router_floating_ip_nat_rules(ri) - to_fip_interface_name = self._get_external_device_interface_name( - ri, ex_gw_port) - self.process_router_floating_ip_addresses( - ri, to_fip_interface_name) + if ri.fip_ns: + to_fip_interface_name = ( + self._get_external_device_interface_name(ri, ex_gw_port)) + self.process_router_floating_ip_addresses( + ri, to_fip_interface_name) for p in ri.internal_ports: internal_interface = self.get_internal_device_name(p['id']) self._snat_redirect_remove(ri, p, internal_interface) diff --git a/neutron/tests/unit/test_l3_agent.py b/neutron/tests/unit/test_l3_agent.py index 5cd7f31db..1c63c39c0 100644 --- a/neutron/tests/unit/test_l3_agent.py +++ b/neutron/tests/unit/test_l3_agent.py @@ -1885,64 +1885,79 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): self.assertRaises(oslo_messaging.MessagingTimeout, l3_agent.L3NATAgent, HOSTNAME, self.conf) - def test_external_gateway_removed_ext_gw_port_and_fip(self): + def _test_external_gateway_removed_ext_gw_port_and_fip(self, fip_ns=False): self.conf.set_override('state_path', '/tmp') self.conf.set_override('router_delete_namespaces', True) agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) - agent.conf.agent_mode = 'dvr' + agent.conf.agent_mode = 'dvr_snat' router = prepare_router_data(num_internal_ports=2) router['distributed'] = True router['gw_port_host'] = HOSTNAME + self.mock_driver.unplug.reset_mock() external_net_id = router['gw_port']['network_id'] ri = dvr_router.DvrRouter(router['id'], router, **self.ri_kwargs) - ri.fip_ns = agent.get_fip_ns(external_net_id) - ri.fip_ns.agent_gateway_port = { - 'fixed_ips': [{'ip_address': '20.0.0.30', 'subnet_id': _uuid()}], - 'subnet': {'gateway_ip': '20.0.0.1'}, - 'id': _uuid(), - 'network_id': external_net_id, - 'mac_address': 'ca:fe:de:ad:be:ef', - 'ip_cidr': '20.0.0.30/24'} agent._fetch_external_net_id = mock.Mock(return_value=external_net_id) - - vm_floating_ip = '19.4.4.2' - ri.floating_ips_dict[vm_floating_ip] = FIP_PRI - ri.dist_fip_count = 1 ri.ex_gw_port = ri.router['gw_port'] del ri.router['gw_port'] - ri.rtr_fip_subnet = ri.fip_ns.local_subnets.allocate(ri.router_id) - _, fip_to_rtr = ri.rtr_fip_subnet.get_pair() + ri.fip_ns = None nat = ri.iptables_manager.ipv4['nat'] nat.clear_rules_by_tag = mock.Mock() nat.add_rule = mock.Mock() + if fip_ns: + ri.fip_ns = agent.get_fip_ns(external_net_id) + ri.fip_ns.agent_gateway_port = { + 'fixed_ips': [{ + 'ip_address': '20.0.0.30', 'subnet_id': _uuid() + }], + 'subnet': {'gateway_ip': '20.0.0.1'}, + 'id': _uuid(), + 'network_id': external_net_id, + 'mac_address': 'ca:fe:de:ad:be:ef', + 'ip_cidr': '20.0.0.30/24'} + + vm_floating_ip = '19.4.4.2' + ri.floating_ips_dict[vm_floating_ip] = FIP_PRI + ri.dist_fip_count = 1 + ri.rtr_fip_subnet = ri.fip_ns.local_subnets.allocate(ri.router_id) + _, fip_to_rtr = ri.rtr_fip_subnet.get_pair() + self.mock_ip.get_devices.return_value = [ + FakeDev(ri.fip_ns.get_ext_device_name(_uuid()))] + self.mock_ip_dev.addr.list.return_value = [ + {'cidr': vm_floating_ip + '/32'}, + {'cidr': '19.4.4.1/24'}] + self.device_exists.return_value = True + fip_ns = ri.fip_ns - self.mock_ip.get_devices.return_value = [ - FakeDev(ri.fip_ns.get_ext_device_name(_uuid()))] - self.mock_ip_dev.addr.list.return_value = [ - {'cidr': vm_floating_ip + '/32'}, - {'cidr': '19.4.4.1/24'}] - self.device_exists.return_value = True - - fip_ns = ri.fip_ns agent.external_gateway_removed( ri, ri.ex_gw_port, agent.get_external_device_name(ri.ex_gw_port['id'])) - self.mock_ip.del_veth.assert_called_once_with( - fip_ns.get_int_device_name(ri.router['id'])) - self.mock_ip_dev.route.delete_gateway.assert_called_once_with( - str(fip_to_rtr.ip), table=dvr_fip_ns.FIP_RT_TBL) - - self.assertEqual(ri.dist_fip_count, 0) - self.assertFalse(fip_ns.has_subscribers()) - self.assertEqual(self.mock_driver.unplug.call_count, 1) - self.assertIsNone(fip_ns.agent_gateway_port) - self.assertTrue(fip_ns.destroyed) - self.mock_ip.netns.delete.assert_called_once_with(fip_ns.get_name()) self.assertFalse(nat.add_rule.called) nat.clear_rules_by_tag.assert_called_once_with('floating_ip') + if fip_ns: + self.mock_ip.del_veth.assert_called_once_with( + fip_ns.get_int_device_name(ri.router['id'])) + self.mock_ip_dev.route.delete_gateway.assert_called_once_with( + str(fip_to_rtr.ip), table=dvr_fip_ns.FIP_RT_TBL) + + self.assertEqual(ri.dist_fip_count, 0) + self.assertFalse(fip_ns.has_subscribers()) + + self.assertIsNone(fip_ns.agent_gateway_port) + self.assertTrue(fip_ns.destroyed) + self.mock_ip.netns.delete.assert_called_once_with( + fip_ns.get_name()) + self.assertEqual(self.mock_driver.unplug.call_count, 1) + else: + self.assertFalse(self.mock_driver.unplug.called) + + def test_external_gateway_removed_ext_gw_port_and_fip(self): + self._test_external_gateway_removed_ext_gw_port_and_fip(fip_ns=True) + + def test_external_gateway_removed_ext_gw_port_no_fip_ns(self): + self._test_external_gateway_removed_ext_gw_port_and_fip(fip_ns=False) def test_spawn_radvd(self): router = prepare_router_data(ip_version=6) -- 2.45.2