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 15390d873cc0ba4277b18ebacbc7e3376e40c332..7055853355fc01b00fa85867086df5dbb65a8896 100644 (file)
--- 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 12515b0979546890b18731d7f54bf350952382ca..6c10331123009a1fec1de9f5d72ac5cf2281f772 100644 (file)
--- 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]