]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
If router is HA, get current_cidrs from keepalived object
authorSachi King <sachi.king@anchor.com.au>
Mon, 8 Dec 2014 06:42:48 +0000 (17:42 +1100)
committerSachi King <sachi.king@anchor.com.au>
Fri, 23 Jan 2015 03:10:07 +0000 (14:10 +1100)
When using L3 HA and keepalived neutron is no longer directly managing
the floating IP addresses itself. Neutron should not check against
which addresses are currently configured on the system, but the
addresses the keepalived object has configured.

Co-Authored-By: Benoit Page-Guitard <benoit.page@gmail.com>
Change-Id: I56045ede3a3dc1a7044a22913ee38ed382a81052
Closes-Bug: #1400217

neutron/agent/l3/agent.py
neutron/agent/l3/ha.py
neutron/agent/linux/keepalived.py
neutron/tests/unit/agent/linux/test_keepalived.py
neutron/tests/unit/test_l3_agent.py

index ee2dc56daa74e468c7ea17209007edb012467774..8f3604340948611f2051d28c24aaa8ff8c60de69 100644 (file)
@@ -668,6 +668,12 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback,
             if ri.router['distributed']:
                 self.floating_ip_removed_dist(ri, ip_cidr)
 
+    def _get_router_cidrs(self, ri, device):
+        if ri.is_ha:
+            return set(self._ha_get_existing_cidrs(ri, device.name))
+        else:
+            return set([addr['cidr'] for addr in device.addr.list()])
+
     def process_router_floating_ip_addresses(self, ri, ex_gw_port):
         """Configure IP addresses on router's external gateway interface.
 
@@ -684,7 +690,7 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback,
 
         device = ip_lib.IPDevice(interface_name, self.root_helper,
                                  namespace=ri.ns_name)
-        existing_cidrs = set([addr['cidr'] for addr in device.addr.list()])
+        existing_cidrs = self._get_router_cidrs(ri, device)
         new_cidrs = set()
 
         # Loop once to ensure that floating ips are configured.
index bcc9414955a5ba85afe6d764739cce7958cb6bd9..33cb3723919436344d2f867dfcfcb90e3b435de7 100644 (file)
@@ -181,6 +181,10 @@ class AgentMixin(object):
         instance = ri.keepalived_manager.config.get_instance(ri.ha_vr_id)
         instance.remove_vips_vroutes_by_interface(interface)
 
+    def _ha_get_existing_cidrs(self, ri, interface_name):
+        instance = ri.keepalived_manager.config.get_instance(ri.ha_vr_id)
+        return instance.get_existing_vip_ip_addresses(interface_name)
+
     def _add_keepalived_notifiers(self, ri):
         callback = (
             metadata_driver.MetadataDriver._get_metadata_proxy_callback(
index 596ee6e0240b73eb2050ae84fb7de054a0d7775b..34672933115eadc241ebf480d0c0524a62863434 100644 (file)
@@ -179,6 +179,10 @@ class KeepalivedInstance(object):
         self.vips = [vip for vip in self.vips
                      if vip.ip_address != ip_address]
 
+    def get_existing_vip_ip_addresses(self, interface_name):
+        return [vip.ip_address for vip in self.vips
+                if vip.interface_name == interface_name]
+
     def _build_track_interface_config(self):
         return itertools.chain(
             ['    track_interface {'],
index 5dfe0dc0fdff0bff59d7ec330db24759db212737..c29ee2dd6a2c92babdaf9b12a59a13cd6f2ce19a 100644 (file)
@@ -187,6 +187,12 @@ vrrp_instance VR_2 {
         config.reset()
         self.assertEqual('', config.get_config_str())
 
+    def test_get_existing_vip_ip_addresses_returns_list(self):
+        config = self._get_config()
+        instance = config.get_instance(1)
+        current_vips = sorted(instance.get_existing_vip_ip_addresses('eth2'))
+        self.assertEqual(['192.168.2.0/24', '192.168.3.0/24'], current_vips)
+
 
 class KeepalivedStateExceptionTestCase(base.BaseTestCase):
     def test_state_exception(self):
index 8a93601b8571ffeabc37a93d7136f90be177a95b..b5fac4207a15bfdb5bc6df57e2f5243f63b8f972 100644 (file)
@@ -981,6 +981,27 @@ class TestBasicRouterOperations(base.BaseTestCase):
         )
         self._test_process_router_floating_ip_addresses_add(ri, agent)
 
+    def test_get_router_cidrs_returns_cidrs(self):
+        agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
+        ri = mock.MagicMock()
+        ri.is_ha = False
+        addresses = ['15.1.2.2/24', '15.1.2.3/32']
+        device = mock.MagicMock()
+        device.addr.list.return_value = [{'cidr': addresses[0]},
+                                         {'cidr': addresses[1]}]
+        self.assertEqual(set(addresses), agent._get_router_cidrs(ri, device))
+
+    def test_get_router_cidrs_returns_ha_cidrs(self):
+        agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
+        ri = mock.MagicMock()
+        ri.is_ha = True
+        device = mock.MagicMock()
+        device.name.return_value = 'eth2'
+        addresses = ['15.1.2.2/24', '15.1.2.3/32']
+        agent._ha_get_existing_cidrs = mock.MagicMock()
+        agent._ha_get_existing_cidrs.return_value = addresses
+        self.assertEqual(set(addresses), agent._get_router_cidrs(ri, device))
+
     # TODO(mrsmith): refactor for DVR cases
     @mock.patch('neutron.agent.linux.ip_lib.IPDevice')
     def test_process_router_floating_ip_addresses_remove(self, IPDevice):