Change some helpers to do less work
These changes cause some helper functions to do less work if the underlying data hasn't changed. This is part of some work to 'quieten' down openstack.charms based reactive charms that do 'too much' work during update-status. Change-Id: I3717387d1d8d2ab875d51f262efd1df7f2529689 Related-Bug: #1708396
This commit is contained in:
Alex Kavanagh
parent
80d09b770d
commit
f36f4d0707
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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: <String> 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
|
||||
|
|
|
|||
|
|
@ -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'))
|
||||
Loading…
Reference in New Issue