From 5be55ac161adfec9085121fe59ea3bf59daa92cf Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Wed, 24 Sep 2014 10:20:46 -0700 Subject: [PATCH] fix dvr snat bindings for external-gw-clear When router_gateway_clear happens, the schedule_router calls the unbind_snat_servicenode in the plugin. This will clear the agent binding from the binding table. But the l3-agent was expecting the ex_gw_port binding to be present. The agent needs to check its cache of the router['gw_host_port'] value now. Change-Id: I051fb97d802b0508b30683a33673b85f5ab24000 Closes-bug: #1373524 --- neutron/agent/l3_agent.py | 2 +- neutron/tests/unit/test_l3_agent.py | 101 ++++++++++++++++++---------- 2 files changed, 65 insertions(+), 38 deletions(-) diff --git a/neutron/agent/l3_agent.py b/neutron/agent/l3_agent.py index 1dd673f3a..a83185800 100644 --- a/neutron/agent/l3_agent.py +++ b/neutron/agent/l3_agent.py @@ -1387,7 +1387,7 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, self._snat_redirect_remove(ri, p, internal_interface) if self.conf.agent_mode == 'dvr_snat' and ( - ex_gw_port['binding:host_id'] == self.host): + ri.router['gw_port_host'] == self.host): ns_name = self.get_snat_ns_name(ri.router['id']) else: # not hosting agent - no work to do diff --git a/neutron/tests/unit/test_l3_agent.py b/neutron/tests/unit/test_l3_agent.py index 8c0d0256f..55ee9876f 100644 --- a/neutron/tests/unit/test_l3_agent.py +++ b/neutron/tests/unit/test_l3_agent.py @@ -376,6 +376,27 @@ class TestBasicRouterOperations(base.BaseTestCase): 'neutron.openstack.common.loopingcall.FixedIntervalLoopingCall') self.looping_call_p.start() + self.snat_ports = [{'subnet': {'cidr': '152.2.0.0/16', + 'gateway_ip': '152.2.0.1', + 'id': _uuid()}, + 'network_id': _uuid(), + 'device_owner': 'network:router_centralized_snat', + 'ip_cidr': '152.2.0.13/16', + 'mac_address': 'fa:16:3e:80:8d:80', + 'fixed_ips': [{'subnet_id': _uuid(), + 'ip_address': '152.2.0.13'}], + 'id': _uuid(), 'device_id': _uuid()}, + {'subnet': {'cidr': '152.10.0.0/16', + 'gateway_ip': '152.10.0.1', + 'id': _uuid()}, + 'network_id': _uuid(), + 'device_owner': 'network:router_centralized_snat', + 'ip_cidr': '152.10.0.13/16', + 'mac_address': 'fa:16:3e:80:8d:80', + 'fixed_ips': [{'subnet_id': _uuid(), + 'ip_address': '152.10.0.13'}], + 'id': _uuid(), 'device_id': _uuid()}] + def _prepare_internal_network_data(self): port_id = _uuid() router_id = _uuid() @@ -497,11 +518,17 @@ class TestBasicRouterOperations(base.BaseTestCase): def test_agent_remove_internal_network(self): self._test_internal_network_action('remove') - def _test_external_gateway_action(self, action): - router = prepare_router_data(num_internal_ports=2) + def _test_external_gateway_action(self, action, router): ri = l3_agent.RouterInfo(router['id'], self.conf.root_helper, self.conf.use_namespaces, router=router) agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) + # Special setup for dvr routers + if router.get('distributed'): + agent.conf.agent_mode = 'dvr_snat' + agent.host = HOSTNAME + agent._create_dvr_gateway = mock.Mock() + agent.get_snat_interfaces = mock.Mock(return_value=self.snat_ports) + ex_gw_port = {'fixed_ips': [{'ip_address': '20.0.0.30', 'subnet_id': _uuid()}], 'subnet': {'gateway_ip': '20.0.0.1'}, @@ -520,17 +547,23 @@ class TestBasicRouterOperations(base.BaseTestCase): 'port_id': _uuid()}]} router[l3_constants.FLOATINGIP_KEY] = fake_fip['floatingips'] agent.external_gateway_added(ri, ex_gw_port, interface_name) - self.assertEqual(self.mock_driver.plug.call_count, 1) - self.assertEqual(self.mock_driver.init_l3.call_count, 1) - self.send_arp.assert_called_once_with(ri.ns_name, interface_name, - '20.0.0.30') - kwargs = {'preserve_ips': ['192.168.1.34/32'], - 'namespace': 'qrouter-' + router['id'], - 'gateway': '20.0.0.1', - 'extra_subnets': [{'cidr': '172.16.0.0/24'}]} - self.mock_driver.init_l3.assert_called_with(interface_name, - ['20.0.0.30/24'], - **kwargs) + if not router.get('distributed'): + self.assertEqual(self.mock_driver.plug.call_count, 1) + self.assertEqual(self.mock_driver.init_l3.call_count, 1) + self.send_arp.assert_called_once_with(ri.ns_name, + interface_name, + '20.0.0.30') + kwargs = {'preserve_ips': ['192.168.1.34/32'], + 'namespace': 'qrouter-' + router['id'], + 'gateway': '20.0.0.1', + 'extra_subnets': [{'cidr': '172.16.0.0/24'}]} + self.mock_driver.init_l3.assert_called_with(interface_name, + ['20.0.0.30/24'], + **kwargs) + else: + agent._create_dvr_gateway.assert_called_once_with( + ri, ex_gw_port, interface_name, + self.snat_ports) elif action == 'remove': self.device_exists.return_value = True @@ -614,7 +647,14 @@ class TestBasicRouterOperations(base.BaseTestCase): 'dvr_snat', 1) def test_agent_add_external_gateway(self): - self._test_external_gateway_action('add') + router = prepare_router_data(num_internal_ports=2) + self._test_external_gateway_action('add', router) + + def test_agent_add_external_gateway_dist(self): + router = prepare_router_data(num_internal_ports=2) + router['distributed'] = True + router['gw_port_host'] = HOSTNAME + self._test_external_gateway_action('add', router) def _test_arping(self, namespace): if not namespace: @@ -642,7 +682,14 @@ class TestBasicRouterOperations(base.BaseTestCase): self._test_arping(namespace=False) def test_agent_remove_external_gateway(self): - self._test_external_gateway_action('remove') + router = prepare_router_data(num_internal_ports=2) + self._test_external_gateway_action('remove', router) + + def test_agent_remove_external_gateway_dist(self): + router = prepare_router_data(num_internal_ports=2) + router['distributed'] = True + router['gw_port_host'] = HOSTNAME + self._test_external_gateway_action('remove', router) def _check_agent_method_called(self, agent, calls, namespace): self.mock_ip.netns.execute.assert_has_calls( @@ -2077,31 +2124,11 @@ vrrp_instance VR_1 { 'mac_address': 'ca:fe:de:ad:be:ef', 'ip_cidr': '20.0.0.30/24'} - snat_ports = [{'subnet': {'cidr': '152.2.0.0/16', - 'gateway_ip': '152.2.0.1', - 'id': _uuid()}, - 'network_id': _uuid(), - 'device_owner': 'network:router_centralized_snat', - 'ip_cidr': '152.2.0.13/16', - 'mac_address': 'fa:16:3e:80:8d:80', - 'fixed_ips': [{'subnet_id': _uuid(), - 'ip_address': '152.2.0.13'}], - 'id': _uuid(), 'device_id': _uuid()}, - {'subnet': {'cidr': '152.10.0.0/16', - 'gateway_ip': '152.10.0.1', - 'id': _uuid()}, - 'network_id': _uuid(), - 'device_owner': 'network:router_centralized_snat', - 'ip_cidr': '152.10.0.13/16', - 'mac_address': 'fa:16:3e:80:8d:80', - 'fixed_ips': [{'subnet_id': _uuid(), - 'ip_address': '152.10.0.13'}], - 'id': _uuid(), 'device_id': _uuid()}] - interface_name = agent.get_snat_int_device_name(port_id) self.device_exists.return_value = False - agent._create_dvr_gateway(ri, dvr_gw_port, interface_name, snat_ports) + agent._create_dvr_gateway(ri, dvr_gw_port, interface_name, + self.snat_ports) # check 2 internal ports are plugged # check 1 ext-gw-port is plugged