diff --git a/charms_openstack/charm/classes.py b/charms_openstack/charm/classes.py index e863c3b..092b684 100644 --- a/charms_openstack/charm/classes.py +++ b/charms_openstack/charm/classes.py @@ -868,7 +868,20 @@ class HAOpenStackCharm(OpenStackAPICharm): """ if not self.config.get(VIP_KEY): return - for vip in self.config[VIP_KEY].split(): + + vips = self.config[VIP_KEY].split() + + # the `vip` config option has changed and there was a value previously + # set, the principle needs to ask hacluster to remove it from + # pacemaker's configuration (LP: #1952363). + if self.config.changed(VIP_KEY) and self.config.previous(VIP_KEY): + old_vips = self.config.previous(VIP_KEY).split() + vips_to_del = set(old_vips) - set(vips) + for vip in vips_to_del: + hookenv.log("Registering %s for deletion" % vip) + hacluster.remove_vip(self.name, vip) + + for vip in vips: iface, netmask, fallback = os_ha_utils.get_vip_settings(vip) if fallback: hacluster.add_vip( diff --git a/unit_tests/charms_openstack/charm/test_classes.py b/unit_tests/charms_openstack/charm/test_classes.py index 1603440..399f131 100644 --- a/unit_tests/charms_openstack/charm/test_classes.py +++ b/unit_tests/charms_openstack/charm/test_classes.py @@ -2,7 +2,10 @@ import base64 import mock import unit_tests.utils as utils -from unit_tests.charms_openstack.charm.utils import BaseOpenStackCharmTest +from unit_tests.charms_openstack.charm.utils import ( + BaseOpenStackCharmTest, + TestConfig, +) from unit_tests.charms_openstack.charm.common import MyOpenStackCharm import charms_openstack.charm.classes as chm @@ -816,6 +819,7 @@ class TestHAOpenStackCharm(BaseOpenStackCharmTest): super(TestHAOpenStackCharm, self).setUp(make_open_stack_charm, TEST_CONFIG) + self.test_config = TestConfig() def test_all_packages(self): self.patch_target('packages', new=['pkg1']) @@ -895,9 +899,10 @@ class TestHAOpenStackCharm(BaseOpenStackCharmTest): nics = { 'vip1': ('eth1', 'netmask1', False), 'vip2': ('eth2', 'netmask2', False)} + self.test_config.set('vip', 'vip1 vip2') interface_mock = mock.Mock() self.patch_target('name', new='myservice') - self.patch_target('config', new={'vip': 'vip1 vip2'}) + self.patch_target('config', new=self.test_config) 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) @@ -910,13 +915,36 @@ class TestHAOpenStackCharm(BaseOpenStackCharmTest): mock.call('res_myservice_eth2_vip')] interface_mock.delete_resource.assert_has_calls(add_vip_calls) + def test__add_ha_vips_config_changed(self): + nics = { + 'vip1': ('eth1', 'netmask1', False), + 'vip2': ('eth2', 'netmask2', False)} + self.test_config.set('vip', 'vip1 vip2') + self.test_config.set_previous('vip', 'vip1 vip3') + interface_mock = mock.Mock() + self.patch_target('name', new='myservice') + self.patch_target('config', new=self.test_config) + 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) + add_vip_calls = [ + mock.call('myservice', 'vip1'), + mock.call('myservice', 'vip2')] + interface_mock.add_vip.assert_has_calls(add_vip_calls) + interface_mock.remove_vip.assert_called_with('myservice', 'vip3') + 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): nics = { 'vip1': ('eth1', 'netmask1', True), 'vip2': ('eth2', 'netmask2', True)} + self.test_config.set('vip', 'vip1 vip2') interface_mock = mock.Mock() self.patch_target('name', new='myservice') - self.patch_target('config', new={'vip': 'vip1 vip2'}) + self.patch_target('config', new=self.test_config) 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) diff --git a/unit_tests/charms_openstack/charm/utils.py b/unit_tests/charms_openstack/charm/utils.py index 0d9561b..2647901 100644 --- a/unit_tests/charms_openstack/charm/utils.py +++ b/unit_tests/charms_openstack/charm/utils.py @@ -42,3 +42,48 @@ class BaseOpenStackCharmTest(unit_tests.utils.BaseTestCase): **kwargs): # uses BaseTestCase.patch_object() to patch targer. self.patch_object(self.target, attr, return_value, name, new, **kwargs) + + +class TestConfig(object): + + def __init__(self): + self.config = {} + self.config_prev = {} + + def __call__(self, key=None): + if key: + return self.get(key) + else: + return self + + def previous(self, k): + return self.config_prev[k] if k in self.config_prev else self.config[k] + + def set_previous(self, k, v): + self.config_prev[k] = v + + def unset_previous(self, k): + if k in self.config_prev: + self.config_prev.pop(k) + + def changed(self, k): + if not self.config_prev: + return True + return self.get(k) != self.previous(k) + + def get(self, attr=None): + if not attr: + return self + try: + return self.config[attr] + except KeyError: + return None + + def get_all(self): + return self.config + + def set(self, attr, value): + self.config[attr] = value + + def __getitem__(self, k): + return self.get(k)