]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
Don't delete HA router primary VIP on agent restarts
authorAssaf Muller <amuller@redhat.com>
Mon, 16 Mar 2015 19:43:24 +0000 (15:43 -0400)
committerAssaf Muller <amuller@redhat.com>
Tue, 17 Mar 2015 22:45:51 +0000 (18:45 -0400)
An HA router's primary VIP was being deleted from the router
namespace when the L3 agent is restarted. Make sure that
doesn't happen and change the functional test to make sure
the bug stays squashed.

Change-Id: I0e5b416152bacf496de54bedee0fca8d3950be2b
Closes-Bug: #1432806
Closes-Bug: #1432785

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

index 0b4e87127b681f28a58d6d13a3f5f41adf3d8b60..3518e8eef86440e0d0c0a672cbe247f8d9d3e23b 100644 (file)
@@ -62,13 +62,13 @@ class AgentMixin(object):
             return
 
         ri._set_subnet_info(ha_port)
+        ri.ha_port = ha_port
+        ri._init_keepalived_manager(self.process_monitor)
         ri.ha_network_added(ha_port['network_id'],
                             ha_port['id'],
                             ha_port['ip_cidr'],
                             ha_port['mac_address'])
-        ri.ha_port = ha_port
 
-        ri._init_keepalived_manager(self.process_monitor)
         ri._add_keepalived_notifiers()
 
     def process_ha_router_removed(self, ri):
index 36bf0431e3a65bf8d244168ef334cf7127388f4d..f3a0571b6db05fae74a64d82eefb396a72fab82e 100644 (file)
@@ -131,6 +131,9 @@ class HaRouter(router.RouterInfo):
     def _get_keepalived_instance(self):
         return self.keepalived_manager.config.get_instance(self.ha_vr_id)
 
+    def _get_primary_vip(self):
+        return self._get_keepalived_instance().get_primary_vip()
+
     def get_ha_device_name(self, port_id):
         return (HA_DEV_PREFIX + port_id)[:self.driver.DEV_NAME_LEN]
 
@@ -141,7 +144,8 @@ class HaRouter(router.RouterInfo):
                          namespace=self.ns_name,
                          prefix=HA_DEV_PREFIX)
         self.driver.init_l3(interface_name, [internal_cidr],
-                            namespace=self.ns_name)
+                            namespace=self.ns_name,
+                            preserve_ips=[self._get_primary_vip()])
 
     def ha_network_removed(self):
         interface_name = self.get_ha_device_name(self.ha_port['id'])
index 04fba57b61d2a38ff68cb171240ad988fc6d1650..c51c2ca98a22158ed014bc72c589249757923bd6 100644 (file)
@@ -185,7 +185,7 @@ class KeepalivedInstance(object):
             ('        %s' % i for i in self.track_interfaces),
             ['    }'])
 
-    def _generate_primary_vip(self):
+    def get_primary_vip(self):
         """Return an address in the primary_vip_range CIDR, with the router's
         VRID in the host section.
 
@@ -197,7 +197,7 @@ class KeepalivedInstance(object):
 
         ip = (netaddr.IPNetwork(self.primary_vip_range).network +
               self.vrouter_id)
-        return netaddr.IPNetwork('%s/%s' % (ip, PRIMARY_VIP_RANGE_SIZE))
+        return str(netaddr.IPNetwork('%s/%s' % (ip, PRIMARY_VIP_RANGE_SIZE)))
 
     def _build_vips_config(self):
         # NOTE(amuller): The primary VIP must be consistent in order to avoid
@@ -212,8 +212,7 @@ class KeepalivedInstance(object):
         # interface IP and floating IPs) are placed in the
         # virtual_ipaddress_excluded section.
 
-        primary = KeepalivedVipAddress(str(self._generate_primary_vip()),
-                                       self.interface)
+        primary = KeepalivedVipAddress(self.get_primary_vip(), self.interface)
         vips_result = ['    virtual_ipaddress {',
                        '        %s' % primary.build_config(),
                        '    }']
index 331406175373b310b476f0f71ebfb05c19381043..7777f4fd35b7f90f105e12dd806ba41127033704 100755 (executable)
@@ -402,7 +402,8 @@ class L3AgentTestCase(L3AgentTestFramework):
         if enable_ha:
             port = router.get_ex_gw_port()
             interface_name = self.agent.get_external_device_name(port['id'])
-            self._assert_no_ip_addresses_on_interface(router, interface_name)
+            self._assert_no_ip_addresses_on_interface(router.ns_name,
+                                                      interface_name)
             utils.wait_until_true(lambda: router.ha_state == 'master')
 
             # Keepalived notifies of a state transition when it starts,
@@ -466,18 +467,28 @@ class L3AgentTestCase(L3AgentTestFramework):
             router.router[l3_constants.HA_INTERFACE_KEY],
             router.get_ha_device_name, router.ns_name))
 
-    def _assert_no_ip_addresses_on_interface(self, router, interface):
-        device = ip_lib.IPDevice(interface, namespace=router.ns_name)
-        self.assertEqual([], device.addr.list())
+    @classmethod
+    def _get_addresses_on_device(cls, namespace, interface):
+        return [address['cidr'] for address in
+                ip_lib.IPDevice(interface, namespace=namespace).addr.list()]
+
+    def _assert_no_ip_addresses_on_interface(self, namespace, interface):
+        self.assertEqual(
+            [], self._get_addresses_on_device(namespace, interface))
 
     def test_ha_router_conf_on_restarted_agent(self):
         router_info = self.generate_router_info(enable_ha=True)
-        router1 = self._create_router(self.agent, router_info)
+        router1 = self.manage_router(self.agent, router_info)
         self._add_fip(router1, '192.168.111.12')
         restarted_agent = neutron_l3_agent.L3NATAgentWithStateReport(
             self.agent.host, self.agent.conf)
         self._create_router(restarted_agent, router1.router)
         utils.wait_until_true(lambda: self.floating_ips_configured(router1))
+        self.assertIn(
+            router1._get_primary_vip(),
+            self._get_addresses_on_device(
+                router1.ns_name,
+                router1.get_ha_device_name(router1.ha_port['id'])))
 
 
 class L3HATestFramework(L3AgentTestFramework):
index f0ed6c52219272b8a7b5011c3079fe6d60aed438..b76c29ae1af5f826c9e1298a438a465caeb390db 100644 (file)
@@ -200,11 +200,10 @@ class KeepalivedStateExceptionTestCase(base.BaseTestCase):
 
 class KeepalivedInstanceTestCase(base.BaseTestCase,
                                  KeepalivedConfBaseMixin):
-    def test_generate_primary_vip(self):
+    def test_get_primary_vip(self):
         instance = keepalived.KeepalivedInstance('MASTER', 'ha0', 42,
                                                  '169.254.192.0/18')
-        self.assertEqual('169.254.0.42/24',
-                         str(instance._generate_primary_vip()))
+        self.assertEqual('169.254.0.42/24', instance.get_primary_vip())
 
     def test_remove_adresses_by_interface(self):
         config = self._get_config()