From: rajeev Date: Fri, 25 Jul 2014 22:50:34 +0000 (-0400) Subject: Improve external gateway update handling X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=596908f8a8aecff06b079147b957c7da3dc7a51b;p=openstack-build%2Fneutron-build.git Improve external gateway update handling Once gateway is set, external_gateway_added() was getting called every time a router update was received. The check for change in external gateway compared previously cached copy of gateway port (ri.ex_gw_port) with the one passed in through update router (ri.router['gw_port']). The cached copy was already being modified by code so the two values would always appear to be different. Making the change to compare correctly and remove actions not required for gateway update. Change-Id: I1a703b327e6c569dfaa8263a222e4bc797e5dbfd Closes-Bug: 1348737 --- diff --git a/neutron/agent/l3_agent.py b/neutron/agent/l3_agent.py index 15390d873..705585335 100644 --- a/neutron/agent/l3_agent.py +++ b/neutron/agent/l3_agent.py @@ -768,9 +768,12 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager): interface_name = None if ex_gw_port_id: interface_name = self.get_external_device_name(ex_gw_port_id) - if ex_gw_port and ex_gw_port != ri.ex_gw_port: + if ex_gw_port: self._set_subnet_info(ex_gw_port) - self.external_gateway_added(ri, ex_gw_port, interface_name) + if not ri.ex_gw_port: + self.external_gateway_added(ri, ex_gw_port, interface_name) + elif ex_gw_port != ri.ex_gw_port: + self.external_gateway_updated(ri, ex_gw_port, interface_name) elif not ex_gw_port and ri.ex_gw_port: self.external_gateway_removed(ri, ri.ex_gw_port, interface_name) @@ -1117,6 +1120,19 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager): self._external_gateway_added(ri, ex_gw_port, interface_name, ri.ns_name, preserve_ips) + def external_gateway_updated(self, ri, ex_gw_port, interface_name): + preserve_ips = [] + if ri.router['distributed']: + ns_name = self.get_snat_ns_name(ri.router['id']) + else: + ns_name = ri.ns_name + floating_ips = self.get_floating_ips(ri) + preserve_ips = [ip['floating_ip_address'] + FLOATING_IP_CIDR_SUFFIX + for ip in floating_ips] + + self._external_gateway_added(ri, ex_gw_port, interface_name, + ns_name, preserve_ips) + def _external_gateway_added(self, ri, ex_gw_port, interface_name, ns_name, preserve_ips): if not ip_lib.device_exists(interface_name, diff --git a/neutron/tests/unit/test_l3_agent.py b/neutron/tests/unit/test_l3_agent.py index 12515b097..6c1033112 100644 --- a/neutron/tests/unit/test_l3_agent.py +++ b/neutron/tests/unit/test_l3_agent.py @@ -374,6 +374,41 @@ class TestBasicRouterOperations(base.BaseTestCase): else: raise Exception("Invalid action %s" % action) + def test_external_gateway_updated(self): + router = prepare_router_data(num_internal_ports=2) + ri = l3_agent.RouterInfo(router['id'], self.conf.root_helper, + self.conf.use_namespaces, router=router) + agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) + ex_gw_port = {'fixed_ips': [{'ip_address': '20.0.0.30', + 'subnet_id': _uuid()}], + 'subnet': {'gateway_ip': '20.0.0.1'}, + 'extra_subnets': [{'cidr': '172.16.0.0/24'}], + 'id': _uuid(), + 'network_id': _uuid(), + 'mac_address': 'ca:fe:de:ad:be:ef', + 'ip_cidr': '20.0.0.30/24'} + interface_name = agent.get_external_device_name(ex_gw_port['id']) + + self.device_exists.return_value = True + fake_fip = {'floatingips': [{'id': _uuid(), + 'floating_ip_address': '192.168.1.34', + 'fixed_ip_address': '192.168.0.1', + 'port_id': _uuid()}]} + router[l3_constants.FLOATINGIP_KEY] = fake_fip['floatingips'] + agent.external_gateway_updated(ri, ex_gw_port, + interface_name) + self.assertEqual(self.mock_driver.plug.call_count, 0) + 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) + def test_agent_add_external_gateway(self): self._test_external_gateway_action('add') @@ -679,6 +714,7 @@ class TestBasicRouterOperations(base.BaseTestCase): agent.process_router_floating_ip_addresses.return_value = { fake_fip_id: 'ACTIVE'} agent.external_gateway_added = mock.Mock() + agent.external_gateway_updated = mock.Mock() fake_floatingips1 = {'floatingips': [ {'id': fake_fip_id, 'floating_ip_address': '8.8.8.8', @@ -691,6 +727,7 @@ class TestBasicRouterOperations(base.BaseTestCase): agent.process_router_floating_ip_addresses.reset_mock() agent.process_router_floating_ip_nat_rules.assert_called_with(ri) agent.process_router_floating_ip_nat_rules.reset_mock() + agent.external_gateway_added.reset_mock() # remap floating IP to a new fixed ip fake_floatingips2 = copy.deepcopy(fake_floatingips1) @@ -704,6 +741,24 @@ class TestBasicRouterOperations(base.BaseTestCase): agent.process_router_floating_ip_addresses.reset_mock() agent.process_router_floating_ip_nat_rules.assert_called_with(ri) agent.process_router_floating_ip_nat_rules.reset_mock() + self.assertEqual(agent.external_gateway_added.call_count, 0) + self.assertEqual(agent.external_gateway_updated.call_count, 0) + agent.external_gateway_added.reset_mock() + agent.external_gateway_updated.reset_mock() + + # change the ex_gw_port a bit to test gateway update + new_gw_port = copy.deepcopy(ri.router['gw_port']) + ri.router['gw_port'] = new_gw_port + old_ip = (netaddr.IPAddress(ri.router['gw_port'] + ['fixed_ips'][0]['ip_address'])) + ri.router['gw_port']['fixed_ips'][0]['ip_address'] = str(old_ip + 1) + + agent.process_router(ri) + ex_gw_port = agent._get_ex_gw_port(ri) + agent.process_router_floating_ip_addresses.reset_mock() + agent.process_router_floating_ip_nat_rules.reset_mock() + self.assertEqual(agent.external_gateway_added.call_count, 0) + self.assertEqual(agent.external_gateway_updated.call_count, 1) # remove just the floating ips del router[l3_constants.FLOATINGIP_KEY]