From 90df090945e9800d354c34fd03236385bff82a4e Mon Sep 17 00:00:00 2001 From: Yoni Shafrir Date: Wed, 4 Feb 2015 07:42:13 +0200 Subject: [PATCH] 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 --- neutron/agent/l3/ha_router.py | 4 -- neutron/agent/linux/keepalived.py | 56 ++++++------------- .../tests/functional/agent/test_l3_agent.py | 13 ++--- .../tests/unit/agent/linux/test_keepalived.py | 45 +++------------ 4 files changed, 28 insertions(+), 90 deletions(-) diff --git a/neutron/agent/l3/ha_router.py b/neutron/agent/l3/ha_router.py index 01b013b6d3f..9a354ec38a3 100644 --- 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 215fa991630..1cfaea51fa4 100644 --- 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 03bfaf052e3..9923f627593 100755 --- 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 e111991de8f..624ccd2f851 100644 --- 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