Remove use of keepalived 'vrrp_sync_group' as it is unused

Now keepalived configuration wraps the VRRP instances with a
'vrrp_sync_group'. The VRRP sync group functionality is only
relevant when more then one VR instance is contained in it.
In that case the VRs in the group will have the same state.
Our use of keepalived uses a single instance per router.

This patch simply removes the 'vrrp_sync_group'.
In this patch VR instances are used on their own and they now
hold the 'notify_scripts'.

Note that the same VRRP functionality is preserved with this
patch.

Another motiviation for this patch, aside from removing
useless configuration, is to lay the foundation for a future
patch that will the related bug by adding 'track_script'
that are not supported with 'vrrp_sync_group'.

Change-Id: I33b81049cd9cf140244bbf121d1a71492161c77c
Related-Bug: #1365461

diff --git a/neutron/agent/l3/ha_router.py b/neutron/agent/l3/ha_router.py
index 01b013b6d3f9a5500824b1339e5023474b1485d2..9a354ec38a314683deaf0d6f0dd1da8f2bb905ba 100644 (file)
--- a/neutron/agent/l3/ha_router.py
+++ b/neutron/agent/l3/ha_router.py
@@ -98,10 +98,6 @@ class HaRouter(router.RouterInfo):
             instance.set_authentication(self.agent_conf.ha_vrrp_auth_type,
                                         self.agent_conf.ha_vrrp_auth_password)
 
-        group = keepalived.KeepalivedGroup(self.ha_vr_id)
-        group.add_instance(instance)
-
-        config.add_group(group)
         config.add_instance(instance)
 
     def spawn_keepalived(self):

diff --git a/neutron/agent/linux/keepalived.py b/neutron/agent/linux/keepalived.py
index 215fa991630c1cc03926bfa9b544eb6652259a59..1cfaea51fa47216d8dfb51cbecdfcafbea99ffaf 100644 (file)
--- a/neutron/agent/linux/keepalived.py
+++ b/neutron/agent/linux/keepalived.py
@@ -107,33 +107,6 @@ class KeepalivedVirtualRoute(object):
         return output
 
 
-class KeepalivedGroup(object):
-    """Group section of a keepalived configuration."""
-
-    def __init__(self, ha_vr_id):
-        self.ha_vr_id = ha_vr_id
-        self.name = 'VG_%s' % ha_vr_id
-        self.instance_names = set()
-        self.notifiers = []
-
-    def add_instance(self, instance):
-        self.instance_names.add(instance.name)
-
-    def set_notify(self, state, path):
-        if state not in VALID_NOTIFY_STATES:
-            raise InvalidNotifyStateException(state=state)
-        self.notifiers.append((state, path))
-
-    def build_config(self):
-        return itertools.chain(['vrrp_sync_group %s {' % self.name,
-                                '    group {'],
-                               ('        %s' % i for i in self.instance_names),
-                               ['    }'],
-                               ('    notify_%s "%s"' % (state, path)
-                                for state, path in self.notifiers),
-                               ['}'])
-
-
 class KeepalivedInstance(object):
     """Instance section of a keepalived configuration."""
 
@@ -156,6 +129,7 @@ class KeepalivedInstance(object):
         self.vips = []
         self.virtual_routes = []
         self.authentication = None
+        self.notifiers = []
         metadata_cidr = '169.254.169.254/32'
         self.primary_vip_range = get_free_range(
             parent_range='169.254.0.0/16',
@@ -188,6 +162,11 @@ class KeepalivedInstance(object):
         return [vip.ip_address for vip in self.vips
                 if vip.interface_name == interface_name]
 
+    def set_notify(self, state, path):
+        if state not in VALID_NOTIFY_STATES:
+            raise InvalidNotifyStateException(state=state)
+        self.notifiers.append((state, path))
+
     def _build_track_interface_config(self):
         return itertools.chain(
             ['    track_interface {'],
@@ -244,6 +223,10 @@ class KeepalivedInstance(object):
                                 for route in self.virtual_routes),
                                ['    }'])
 
+    def _build_notify_scripts(self):
+        return itertools.chain(('    notify_%s "%s"' % (state, path)
+                                for state, path in self.notifiers))
+
     def build_config(self):
         config = ['vrrp_instance %s {' % self.name,
                   '    state %s' % self.state,
@@ -276,6 +259,9 @@ class KeepalivedInstance(object):
         if self.virtual_routes:
             config.extend(self._build_virtual_routes_config())
 
+        if self.notifiers:
+            config.extend(self._build_notify_scripts())
+
         config.append('}')
 
         return config
@@ -288,15 +274,8 @@ class KeepalivedConf(object):
         self.reset()
 
     def reset(self):
-        self.groups = {}
         self.instances = {}
 
-    def add_group(self, group):
-        self.groups[group.ha_vr_id] = group
-
-    def get_group(self, ha_vr_id):
-        return self.groups.get(ha_vr_id)
-
     def add_instance(self, instance):
         self.instances[instance.vrouter_id] = instance
 
@@ -306,9 +285,6 @@ class KeepalivedConf(object):
     def build_config(self):
         config = []
 
-        for group in self.groups.values():
-            config.extend(group.build_config())
-
         for instance in self.instances.values():
             config.extend(instance.build_config())
 
@@ -341,7 +317,7 @@ class KeepalivedNotifierMixin(object):
         state_path = self._get_full_config_file_path('state')
         return '%s\necho -n %s > %s' % (script, state, state_path)
 
-    def add_notifier(self, script, state, ha_vr_id):
+    def add_notifier(self, script, state, vrouter_id):
         """Add a master, backup or fault notifier.
 
         These notifiers are executed when keepalived invokes a state
@@ -353,8 +329,8 @@ class KeepalivedNotifierMixin(object):
         full_script = self._append_state(script_with_prefix, state)
         self._write_notify_script(state, full_script)
 
-        group = self.config.get_group(ha_vr_id)
-        group.set_notify(state, self._get_notifier_path(state))
+        vr_instance = self.config.get_instance(vrouter_id)
+        vr_instance.set_notify(state, self._get_notifier_path(state))
 
     def get_conf_dir(self):
         confs_dir = os.path.abspath(os.path.normpath(self.conf_path))

diff --git a/neutron/tests/functional/agent/test_l3_agent.py b/neutron/tests/functional/agent/test_l3_agent.py
index 03bfaf052e325241c6b78eb6d19baa3e2a5dd6af..9923f627593ff04787520a01c4ee79468b902839 100755 (executable)
--- a/neutron/tests/functional/agent/test_l3_agent.py
+++ b/neutron/tests/functional/agent/test_l3_agent.py
@@ -163,15 +163,7 @@ class L3AgentTestFramework(base.BaseOVSLinuxTestCase):
             self.agent.get_floating_ips(router)[0]['floating_ip_address'])
         default_gateway_ip = external_port['subnet'].get('gateway_ip')
 
-        return """vrrp_sync_group VG_1 {
-    group {
-        VR_1
-    }
-    notify_master "%(ha_confs_path)s/%(router_id)s/notify_master.sh"
-    notify_backup "%(ha_confs_path)s/%(router_id)s/notify_backup.sh"
-    notify_fault "%(ha_confs_path)s/%(router_id)s/notify_fault.sh"
-}
-vrrp_instance VR_1 {
+        return """vrrp_instance VR_1 {
     state BACKUP
     interface %(ha_device_name)s
     virtual_router_id 1
@@ -195,6 +187,9 @@ vrrp_instance VR_1 {
         0.0.0.0/0 via %(default_gateway_ip)s dev %(external_device_name)s
         8.8.8.0/24 via 19.4.4.4
     }
+    notify_master "%(ha_confs_path)s/%(router_id)s/notify_master.sh"
+    notify_backup "%(ha_confs_path)s/%(router_id)s/notify_backup.sh"
+    notify_fault "%(ha_confs_path)s/%(router_id)s/notify_fault.sh"
 }""" % {
             'ha_confs_path': ha_confs_path,
             'router_id': router_id,

diff --git a/neutron/tests/unit/agent/linux/test_keepalived.py b/neutron/tests/unit/agent/linux/test_keepalived.py
index e111991de8f981a29aacf6302e12a25bfec74060..624ccd2f851ea68a81ee024ae256fc4d061a4ff0 100644 (file)
--- a/neutron/tests/unit/agent/linux/test_keepalived.py
+++ b/neutron/tests/unit/agent/linux/test_keepalived.py
@@ -60,16 +60,12 @@ class KeepalivedConfBaseMixin(object):
     def _get_config(self):
         config = keepalived.KeepalivedConf()
 
-        group1 = keepalived.KeepalivedGroup(1)
-        group2 = keepalived.KeepalivedGroup(2)
-
-        group1.set_notify('master', '/tmp/script.sh')
-
         instance1 = keepalived.KeepalivedInstance('MASTER', 'eth0', 1,
                                                   '169.254.192.0/18',
                                                   advert_int=5)
         instance1.set_authentication('AH', 'pass123')
         instance1.track_interfaces.append("eth0")
+        instance1.set_notify('master', '/tmp/script.sh')
 
         vip_address1 = keepalived.KeepalivedVipAddress('192.168.1.0/24',
                                                        'eth1')
@@ -93,8 +89,6 @@ class KeepalivedConfBaseMixin(object):
                                                           "eth1")
         instance1.virtual_routes.append(virtual_route)
 
-        group1.add_instance(instance1)
-
         instance2 = keepalived.KeepalivedInstance('MASTER', 'eth4', 2,
                                                   '169.254.192.0/18',
                                                   mcast_src_ip='224.0.0.1')
@@ -107,11 +101,7 @@ class KeepalivedConfBaseMixin(object):
         instance2.vips.append(vip_address2)
         instance2.vips.append(vip_address_ex)
 
-        group2.add_instance(instance2)
-
-        config.add_group(group1)
         config.add_instance(instance1)
-        config.add_group(group2)
         config.add_instance(instance2)
 
         return config
@@ -120,18 +110,7 @@ class KeepalivedConfBaseMixin(object):
 class KeepalivedConfTestCase(base.BaseTestCase,
                              KeepalivedConfBaseMixin):
 
-    expected = """vrrp_sync_group VG_1 {
-    group {
-        VR_1
-    }
-    notify_master "/tmp/script.sh"
-}
-vrrp_sync_group VG_2 {
-    group {
-        VR_2
-    }
-}
-vrrp_instance VR_1 {
+    expected = """vrrp_instance VR_1 {
     state MASTER
     interface eth0
     virtual_router_id 1
@@ -156,6 +135,7 @@ vrrp_instance VR_1 {
     virtual_routes {
         0.0.0.0/0 via 192.168.1.1 dev eth1
     }
+    notify_master "/tmp/script.sh"
 }
 vrrp_instance VR_2 {
     state MASTER
@@ -196,11 +176,12 @@ vrrp_instance VR_2 {
 
 class KeepalivedStateExceptionTestCase(base.BaseTestCase):
     def test_state_exception(self):
-        group = keepalived.KeepalivedGroup('group2')
+        instance = keepalived.KeepalivedInstance('MASTER', 'eth0', 1,
+                                                 '169.254.192.0/18')
 
         invalid_notify_state = 'a seal walks'
         self.assertRaises(keepalived.InvalidNotifyStateException,
-                          group.set_notify,
+                          instance.set_notify,
                           invalid_notify_state, '/tmp/script.sh')
 
         invalid_vrrp_state = 'into a club'
@@ -230,18 +211,7 @@ class KeepalivedInstanceTestCase(base.BaseTestCase,
         instance.remove_vips_vroutes_by_interface('eth2')
         instance.remove_vips_vroutes_by_interface('eth10')
 
-        expected = """vrrp_sync_group VG_1 {
-    group {
-        VR_1
-    }
-    notify_master "/tmp/script.sh"
-}
-vrrp_sync_group VG_2 {
-    group {
-        VR_2
-    }
-}
-vrrp_instance VR_1 {
+        expected = """vrrp_instance VR_1 {
     state MASTER
     interface eth0
     virtual_router_id 1
@@ -263,6 +233,7 @@ vrrp_instance VR_1 {
     virtual_routes {
         0.0.0.0/0 via 192.168.1.1 dev eth1
     }
+    notify_master "/tmp/script.sh"
 }
 vrrp_instance VR_2 {
     state MASTER