Make keepalived add_vip idempotent

L3 agent operations are designed to be idempotent to be resiliant
in the face of missed notifications, out of order notifications
processing and agent restarts. One exception is keepalived's
add_vip that was made to explicitly barf when the agent tries
to add a vip that was already inserted to keepalive'd internal
cache. This was done in patch 142630 by my request to root
out bugs. Well, we found such a bug and in retrospect may not
have been the smartest idea.

Change-Id: Iead975787e2847828286d7b644dcbe33cf57ace9
Related-Bug: #1523999

diff --git a/neutron/agent/l3/ha_router.py b/neutron/agent/l3/ha_router.py
index a9f2bfc6c761ef9d3db6ffa8430465f3fecde321..196d70c9bb982e979a2f802f407e1466bdbefa37 100644 (file)
--- a/neutron/agent/l3/ha_router.py
+++ b/neutron/agent/l3/ha_router.py
@@ -157,10 +157,7 @@ class HaRouter(router.RouterInfo):
 
     def _add_vips(self, port, interface_name):
         for ip_cidr in common_utils.fixed_ip_cidrs(port['fixed_ips']):
-            try:
-                self._add_vip(ip_cidr, interface_name)
-            except keepalived.VIPDuplicateAddressException:
-                LOG.debug("%s has already been added to keepalive", ip_cidr)
+            self._add_vip(ip_cidr, interface_name)
 
     def _add_vip(self, ip_cidr, interface, scope=None):
         instance = self._get_keepalived_instance()

diff --git a/neutron/agent/linux/keepalived.py b/neutron/agent/linux/keepalived.py
index 41da43dcde0612d8a01cb53265b61c292bff1e55..ba9c74e6bb8032b79c07214950eaacad12065381 100644 (file)
--- a/neutron/agent/linux/keepalived.py
+++ b/neutron/agent/linux/keepalived.py
@@ -79,17 +79,6 @@ class InvalidAuthenticationTypeException(exceptions.NeutronException):
         super(InvalidAuthenticationTypeException, self).__init__(**kwargs)
 
 
-class VIPDuplicateAddressException(exceptions.NeutronException):
-    message = _('Attempted to add duplicate VIP address, '
-                'existing vips are: %(existing_vips)s, '
-                'duplicate vip is: %(duplicate_vip)s')
-
-    def __init__(self, **kwargs):
-        kwargs['existing_vips'] = ', '.join(str(vip) for vip in
-                                            kwargs['existing_vips'])
-        super(VIPDuplicateAddressException, self).__init__(**kwargs)
-
-
 class KeepalivedVipAddress(object):
     """A virtual address entry of a keepalived configuration."""
 
@@ -204,10 +193,10 @@ class KeepalivedInstance(object):
 
     def add_vip(self, ip_cidr, interface_name, scope):
         vip = KeepalivedVipAddress(ip_cidr, interface_name, scope)
-        if vip in self.vips:
-            raise VIPDuplicateAddressException(existing_vips=self.vips,
-                                               duplicate_vip=vip)
-        self.vips.append(vip)
+        if vip not in self.vips:
+            self.vips.append(vip)
+        else:
+            LOG.debug('VIP %s already present in %s', vip, self.vips)
 
     def remove_vips_vroutes_by_interface(self, interface_name):
         self.vips = [vip for vip in self.vips

diff --git a/neutron/tests/unit/agent/linux/test_keepalived.py b/neutron/tests/unit/agent/linux/test_keepalived.py
index 3ffc089f624b2297ea68f46c882e3c130e53ec8c..01eb4698af8d97ac0a36cf3c665e011186293386 100644 (file)
--- a/neutron/tests/unit/agent/linux/test_keepalived.py
+++ b/neutron/tests/unit/agent/linux/test_keepalived.py
@@ -321,13 +321,12 @@ class KeepalivedVipAddressTestCase(base.BaseTestCase):
         self.assertEqual('fe80::3e97:eff:fe26:3bfa/64 dev eth1 scope link',
                          vip.build_config())
 
-    def test_add_vip_returns_exception_on_duplicate_ip(self):
+    def test_add_vip_idempotent(self):
         instance = keepalived.KeepalivedInstance('MASTER', 'eth0', 1,
                                                  ['169.254.192.0/18'])
         instance.add_vip('192.168.222.1/32', 'eth11', None)
-        self.assertRaises(keepalived.VIPDuplicateAddressException,
-                          instance.add_vip, '192.168.222.1/32', 'eth12',
-                          'link')
+        instance.add_vip('192.168.222.1/32', 'eth12', 'link')
+        self.assertEqual(1, len(instance.vips))
 
 
 class KeepalivedVirtualRouteTestCase(base.BaseTestCase):