From: Assaf Muller Date: Tue, 8 Dec 2015 22:44:07 +0000 (-0500) Subject: Make keepalived add_vip idempotent X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=eb84048bbc27d6743016da9beeda071ccb92cd9c;p=openstack-build%2Fneutron-build.git 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 a9f2bfc6c..196d70c9b 100644 --- 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 41da43dcd..ba9c74e6b 100644 --- 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 3ffc089f6..01eb4698a 100644 --- 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):