diff --git a/charms_openstack/charm/classes.py b/charms_openstack/charm/classes.py index 9323a65..308f5af 100644 --- a/charms_openstack/charm/classes.py +++ b/charms_openstack/charm/classes.py @@ -17,7 +17,10 @@ from charms_openstack.charm.core import ( BaseOpenStackCharmActions, BaseOpenStackCharmAssessStatus, ) -from charms_openstack.charm.utils import get_upstream_version +from charms_openstack.charm.utils import ( + get_upstream_version, + is_data_changed, +) import charms_openstack.adapters as os_adapters import charms_openstack.ip as os_ip @@ -507,22 +510,30 @@ class HAOpenStackCharm(OpenStackAPICharm): def configure_ssl(self, keystone_interface=None): """Configure SSL certificates and keys + NOTE(AJK): This function tries to minimise the work it does, + particularly with writing files and restarting apache. + @param keystone_interface KeystoneRequires class """ - keystone_interface = (reactive.RelationBase.from_state( - 'identity-service.available.ssl') or - reactive.RelationBase.from_state( - 'identity-service.available.ssl_legacy')) + keystone_interface = ( + reactive.RelationBase + .from_state('identity-service.available.ssl') or + reactive.RelationBase + .from_state('identity-service.available.ssl_legacy')) ssl_objects = self.get_certs_and_keys( keystone_interface=keystone_interface) - if ssl_objects: - for ssl in ssl_objects: - self.configure_cert(ssl['cert'], ssl['key'], cn=ssl['cn']) - self.configure_ca(ssl['ca']) - self.set_state('ssl.enabled', True) - self.configure_apache() - else: - self.set_state('ssl.enabled', False) + with is_data_changed('configure_ssl.ssl_objects', + ssl_objects) as changed: + if ssl_objects: + if changed: + for ssl in ssl_objects: + self.configure_cert( + ssl['cert'], ssl['key'], cn=ssl['cn']) + self.configure_ca(ssl['ca']) + self.configure_apache() + self.set_state('ssl.enabled', True) + else: + self.set_state('ssl.enabled', False) amqp_ssl = reactive.RelationBase.from_state('amqp.available.ssl') if amqp_ssl: self.configure_rabbit_cert(amqp_ssl) @@ -563,10 +574,23 @@ class HAOpenStackCharm(OpenStackAPICharm): subprocess.check_call(['update-ca-certificates', '--fresh']) def update_peers(self, cluster): - for addr_type in os_ip.ADDRESS_MAP.keys(): + """Update peers in the cluster about the addresses that this unit + holds. + + NOTE(AJK): This uses the helper is_data_changed() to track whether this + has already been done, and doesn't re-advertise the changes if nothing + has changed. + + @param cluster: the interface object for the cluster relation + """ + laddrs = [] + for addr_type in sorted(os_ip.ADDRESS_MAP.keys()): cidr = self.config.get(os_ip.ADDRESS_MAP[addr_type]['config']) laddr = ch_ip.get_address_in_network(cidr) - if laddr: - cluster.set_address( - os_ip.ADDRESS_MAP[addr_type]['binding'], - laddr) + laddrs.append((addr_type, laddr)) + with is_data_changed('update_peers.laddrs', laddrs) as changed: + if changed: + for (addr_type, laddr) in laddrs: + cluster.set_address( + os_ip.ADDRESS_MAP[addr_type]['binding'], + laddr) diff --git a/charms_openstack/charm/utils.py b/charms_openstack/charm/utils.py index 924e926..58a47a3 100644 --- a/charms_openstack/charm/utils.py +++ b/charms_openstack/charm/utils.py @@ -1,3 +1,7 @@ +import json +import hashlib + +import charmhelpers.core as core import charmhelpers.fetch as fetch @@ -21,3 +25,57 @@ def get_upstream_version(package): return None return apt_pkg.upstream_version(pkg.current_ver.ver_str) + + +# TODO(AJK): Once this is in charms.reactive, drop it here and just reference +# the charms.reactive version. +# NOTE(AJK): that we are breaking the camalcase rule as this is acting as a +# context manager, which doesn't look like a 'class' +class is_data_changed(object): + """ Check if the given set of data has changed since the previous call. + This works by hashing the JSON-serialization of the data. Note that, while + the data will be serialized using ``sort_keys=True``, some types of data + structures, such as sets, may lead to false positivies. + + The hash of the changed data WON'T be stored until a successful exit of the + context manager. This means if the code in the scope of the context + manager raises an exception, then the data won't be changed and the next + check will leave it unchanged. This is to allow for recovery from errors. + + Usage: + with is_data_changed() as changed: + charm_instance.some_method() + """ + + def __init__(self, data_id, data, hash_type='md5', + no_change_on_exception=True): + """Initialise the context manager: + + @param data_id: the unique name for this data + @param data: a JSON serialisable data object. + @param hash_type: A hashing function available from hashlib + default='md5' + @param no_change_on_exception: if an exception is thrown in the managed + code then the new hash is not persisited. + """ + self.data_id = data_id + self.data = data + self.hash_type = hash_type + self.no_change_on_exception = no_change_on_exception + + def __enter__(self): + """with statement as returns boolean""" + + self.key = 'charms.openstack.data_changed.{}'.format(self.data_id) + alg = getattr(hashlib, self.hash_type) + serialized = json.dumps(self.data, sort_keys=True).encode('utf8') + old_hash = core.unitdata.kv().get(self.key) + self.new_hash = alg(serialized).hexdigest() + return old_hash != self.new_hash + + def __exit__(self, e_type, *_): + # If no exception, then store the new hash. + if e_type is None or not self.no_change_on_exception: + core.unitdata.kv().set(self.key, self.new_hash) + # re-raise the exception, if there was one. + return False diff --git a/unit_tests/charms_openstack/charm/test_utils.py b/unit_tests/charms_openstack/charm/test_utils.py new file mode 100644 index 0000000..f4740da --- /dev/null +++ b/unit_tests/charms_openstack/charm/test_utils.py @@ -0,0 +1,52 @@ +from unit_tests.utils import BaseTestCase + +import charms_openstack.charm.utils as utils + + +class TestHelpers(BaseTestCase): + + def test_is_data_changed(self): + class FakeKV(object): + def __init__(self): + self.store = {} + + def get(self, key): + return self.store.get(key, None) + + def set(self, key, value): + self.store[key] = value + + store = FakeKV() + self.patch_object(utils.core.unitdata, "kv", new=lambda: store) + with utils.is_data_changed('foo', + {'foo': 'FOO', 'bar': u'\ua000BAR'}) as f: + self.assertTrue(f) + with utils.is_data_changed('foo', + {'foo': 'FOO', 'bar': u'\ua000BAR'}) as f: + self.assertFalse(f) + with utils.is_data_changed('bar', + {'foo': 'FOO', 'bar': u'\ua000BAR'}) as f: + self.assertTrue(f) + with utils.is_data_changed('bar', + {'foo': 'FOO', 'bar': u'\ua000BAR'}) as f: + self.assertFalse(f) + # check that raising an exception doesn't cause a data change + hash = store.get('charms.openstack.data_changed.bar') + try: + with utils.is_data_changed('bar', "string") as f: + self.assertTrue(f) + raise Exception() + except: + pass + self.assertEquals(hash, store.get('charms.openstack.data_changed.bar')) + # check that raising an exception AND having the flag set causes a + # change + try: + with utils.is_data_changed('bar', "string", + no_change_on_exception=False) as f: + self.assertTrue(f) + raise Exception() + except: + pass + self.assertNotEquals(hash, + store.get('charms.openstack.data_changed.bar'))