From 69d6e5462a124cc8b12e5a58fe2837d18f76248c Mon Sep 17 00:00:00 2001 From: Liam Young Date: Tue, 18 Dec 2018 11:37:59 +0000 Subject: [PATCH] Do not specify iface when creating ha vip resource Pacemaker VIP resources are able to automatically detect and configure correct iface and netmask parameters based on local configuration of the unit. The hacluster interface will use this approach if nic and netmask are omitted when setting up the vip. If nic and netmask cannot be automatically detected then fallback to the old behaviour. If new behaviour is being used then ensure the old vip resource name is removed. Change-Id: Id2fbb71328572bada939d432030d9d7ab4814f1b --- charms_openstack/charm/classes.py | 20 +++++--- unit_tests/__init__.py | 2 + .../charms_openstack/charm/test_classes.py | 51 +++++++++---------- 3 files changed, 40 insertions(+), 33 deletions(-) diff --git a/charms_openstack/charm/classes.py b/charms_openstack/charm/classes.py index 6f3a117..8017b7e 100644 --- a/charms_openstack/charm/classes.py +++ b/charms_openstack/charm/classes.py @@ -10,6 +10,7 @@ import subprocess import charmhelpers.contrib.network.ip as ch_ip import charmhelpers.contrib.openstack.utils as os_utils import charmhelpers.contrib.openstack.ha as os_ha +import charmhelpers.contrib.openstack.ha.utils as os_ha_utils import charmhelpers.contrib.openstack.cert_utils as cert_utils import charmhelpers.core.hookenv as hookenv import charmhelpers.core.host as ch_host @@ -682,12 +683,19 @@ class HAOpenStackCharm(OpenStackAPICharm): if not self.config.get(VIP_KEY): return for vip in self.config[VIP_KEY].split(): - iface = (ch_ip.get_iface_for_address(vip) or - self.config.get(IFACE_KEY)) - netmask = (ch_ip.get_netmask_for_address(vip) or - self.config.get(CIDR_KEY)) - if iface is not None: - hacluster.add_vip(self.name, vip, iface, netmask) + iface, netmask, fallback = os_ha_utils.get_vip_settings(vip) + if fallback: + hacluster.add_vip( + self.name, + vip, + iface, + netmask) + else: + hacluster.add_vip(self.name, vip) + if iface: + # Remove vip resource using old raw nic name. + old_vip_key = 'res_{}_{}_vip'.format(self.name, iface) + hacluster.delete_resource(old_vip_key) def _add_ha_haproxy_config(self, hacluster): """Add a InitService object for haproxy to self.resources diff --git a/unit_tests/__init__.py b/unit_tests/__init__.py index 06a0477..89ebd80 100644 --- a/unit_tests/__init__.py +++ b/unit_tests/__init__.py @@ -31,6 +31,8 @@ sys.modules['charmhelpers.contrib'] = charmhelpers.contrib sys.modules['charmhelpers.contrib.openstack'] = charmhelpers.contrib.openstack sys.modules['charmhelpers.contrib.openstack.ha'] = ( charmhelpers.contrib.openstack.ha) +sys.modules['charmhelpers.contrib.openstack.ha.utils'] = ( + charmhelpers.contrib.openstack.ha.utils) sys.modules['charmhelpers.contrib.openstack.cert_utils'] = ( charmhelpers.contrib.openstack.cert_utils) sys.modules['charmhelpers.contrib.openstack.utils'] = ( diff --git a/unit_tests/charms_openstack/charm/test_classes.py b/unit_tests/charms_openstack/charm/test_classes.py index e83f3a8..7c028ac 100644 --- a/unit_tests/charms_openstack/charm/test_classes.py +++ b/unit_tests/charms_openstack/charm/test_classes.py @@ -481,42 +481,39 @@ class TestHAOpenStackCharm(BaseOpenStackCharmTest): interface_mock.bind_resources.assert_called_once_with(iface='ens12') def test__add_ha_vips_config(self): - ifaces = { - 'vip1': 'eth1', - 'vip2': 'eth2'} - masks = { - 'vip1': 'netmask1', - 'vip2': 'netmask2'} + nics = { + 'vip1': ('eth1', 'netmask1', False), + 'vip2': ('eth2', 'netmask2', False)} interface_mock = mock.Mock() self.patch_target('name', new='myservice') self.patch_target('config', new={'vip': 'vip1 vip2'}) - self.patch_object(chm.ch_ip, 'get_iface_for_address') - self.get_iface_for_address.side_effect = lambda x: ifaces[x] - self.patch_object(chm.ch_ip, 'get_netmask_for_address') - self.get_netmask_for_address.side_effect = lambda x: masks[x] + self.patch_object(chm.os_ha_utils, 'get_vip_settings') + self.get_vip_settings.side_effect = lambda x: nics[x] self.target._add_ha_vips_config(interface_mock) - calls = [ - mock.call('myservice', 'vip1', 'eth1', 'netmask1'), - mock.call('myservice', 'vip2', 'eth2', 'netmask2')] - interface_mock.add_vip.assert_has_calls(calls) + add_vip_calls = [ + mock.call('myservice', 'vip1'), + mock.call('myservice', 'vip2')] + interface_mock.add_vip.assert_has_calls(add_vip_calls) + add_vip_calls = [ + mock.call('res_myservice_eth1_vip'), + mock.call('res_myservice_eth2_vip')] + interface_mock.delete_resource.assert_has_calls(add_vip_calls) def test__add_ha_vips_config_fallback(self): - config = { - 'vip_cidr': 'user_cidr', - 'vip_iface': 'user_iface', - 'vip': 'vip1 vip2'} + nics = { + 'vip1': ('eth1', 'netmask1', True), + 'vip2': ('eth2', 'netmask2', True)} interface_mock = mock.Mock() self.patch_target('name', new='myservice') - self.patch_target('config', new=config) - self.patch_object(chm.ch_ip, 'get_iface_for_address') - self.patch_object(chm.ch_ip, 'get_netmask_for_address') - self.get_iface_for_address.return_value = None - self.get_netmask_for_address.return_value = None + self.patch_target('config', new={'vip': 'vip1 vip2'}) + self.patch_object(chm.os_ha_utils, 'get_vip_settings') + self.get_vip_settings.side_effect = lambda x: nics[x] self.target._add_ha_vips_config(interface_mock) - calls = [ - mock.call('myservice', 'vip1', 'user_iface', 'user_cidr'), - mock.call('myservice', 'vip2', 'user_iface', 'user_cidr')] - interface_mock.add_vip.assert_has_calls(calls) + add_vip_calls = [ + mock.call('myservice', 'vip1', 'eth1', 'netmask1'), + mock.call('myservice', 'vip2', 'eth2', 'netmask2')] + interface_mock.add_vip.assert_has_calls(add_vip_calls) + self.assertFalse(interface_mock.delete_resource.called) def test__add_ha_vips_config_novip(self): config = {'vip': None}