From 659403d17c4ed148267ce2ff1911f07f7bcaf1b4 Mon Sep 17 00:00:00 2001 From: Alex Kavanagh Date: Thu, 9 Jun 2016 10:38:47 +0000 Subject: [PATCH 1/9] Added assess_status() functionality - no tests yet This adds the assess_status functionality, but without any tests yet. It also adds a register function to enable the release to be specified before an OpenStackCharm class is instantiated, thus providing the mechanism to choose the class to use for a release (or series of releases). This was missing from earlier commits. --- charms_openstack/charm.py | 209 +++++++++++++++++++++++++++++++++++++- 1 file changed, 205 insertions(+), 4 deletions(-) diff --git a/charms_openstack/charm.py b/charms_openstack/charm.py index 28082e1..c5316ef 100644 --- a/charms_openstack/charm.py +++ b/charms_openstack/charm.py @@ -8,6 +8,7 @@ import os import subprocess import contextlib import collections +import itertools import six @@ -34,6 +35,11 @@ _releases = {} # hook invocation. _singleton = None +# `_release_selector_function` holds a function that takes optionally takes a +# release and commutes it to another release or just returns a release. +# This is to enable the defining code to define which release is used. +_release_selector_function = None + # List of releases that OpenStackCharm based charms know about KNOWN_RELEASES = [ 'diablo', @@ -75,6 +81,10 @@ def get_charm_instance(release=None, *args, **kwargs): "Release {} is not supported by this charm. Earliest support is " "{} release".format(release, known_releases[0])) else: + # check that the release is a valid release + if release not in KNOWN_RELEASES: + raise RuntimeError( + "Release {} is not a known OpenStack release?".format(release)) # try to find the release that is supported. for known_release in reversed(known_releases): if release >= known_release: @@ -85,6 +95,30 @@ def get_charm_instance(release=None, *args, **kwargs): return cls(release=release, *args, **kwargs) +def register_os_release_selector(f): + """Register a function that determines what the release is for the + invocation run. This allows the charm to define HOW the release is + determined. + + Usage: + + @register_os_release_selector + def my_release_selector(): + return os_release_chooser() + + The function should return a string which is an OS release. + """ + global _release_selector_function + if _release_selector_function is None: + # we can only do this once in a system invocation. + _release_selector_function = f + else: + raise RuntimeError( + "Only a single release_selector_function is supported." + " Called with {}".format(f.__name__)) + return f + + class OpenStackCharmMeta(type): """Metaclass to provide a classproperty of 'singleton' so that class methods in the derived OpenStackCharm() class can simply use cls.singleton @@ -141,8 +175,17 @@ class OpenStackCharmMeta(type): @property def singleton(cls): + """Either returns the already created charm, or create a new one. + + This uses the _release_selector_function to choose the release is one + has been registered, otherwise None is passed to get_charm_instance() + """ global _singleton if _singleton is None: + release = None + # see if a _release_selector_function has been registered. + if _release_selector_function is not None: + release = _release_selector_function() _singleton = get_charm_instance() return _singleton @@ -187,6 +230,10 @@ class OpenStackCharm(object): # } restart_map = {} + # The list of required services that are checked for assess_status + # e.g. required_relations = ['identity-service', 'shared-db'] + required_relations = [] + # The command used to sync the database sync_cmd = [] @@ -226,11 +273,9 @@ class OpenStackCharm(object): if packages: hookenv.status_set('maintenance', 'Installing packages') charmhelpers.fetch.apt_install(packages, fatal=True) - # TODO need a call to assess_status(...) or equivalent so that we - # can determine the workload status at the end of the handler. At - # the end of install the 'status' is stuck in maintenance until the - # next hook is run. self.set_state('{}-installed'.format(self.name)) + hookenv.status_set('maintenance', + 'Installation complete - awaiting next status') def set_state(self, state, value=None): """proxy for charms.reactive.bus.set_state()""" @@ -379,3 +424,159 @@ class OpenStackCharm(object): # Restart services immediatly after db sync as # render_domain_config needs a working system self.restart_all() + + def assess_status(self): + """Assess the status of the unit and set the status and a useful + message as appropriate. + + The 3 checks are: + + 1. Check if the unit has been paused (using + os_utils.is_unit_paused_set(). + 2. Check if the interfaces are all present (using the states that are + set by each interface as it comes 'live'. + 3. Do a custom_assess_status_check() check. + 4. Check that services that should be running are running. + + Each sub-function determins what checks are taking place. + + If custom assess_status() functionality is required then the derived + class should override any of the 4 check functions to alter the + behaviour as required. + + Note that if ports are NOT to be checked, then the derived class should + override :meth:`ports_to_check()` and return an empty list. + + SIDE EFFECT: this function calls status_set(state, message) to set the + workload status in juju. + """ + for f in [self.check_if_paused, + self.check_interfaces, + self.custom_assess_status_check, + self.check_services_running]: + state, message = f() + if state is not None: + hookenv.status_set(state, message) + return + # No state was particularly set, so assume the unit is active + hookenv.state_set('active', 'Unit is ready') + + def custom_assess_status_check(self): + """Override this function in a derived class if there are any other + status checks that need to be done that aren't about relations, etc. + + Return (None, None) if the status is okay (i.e. the unit is active). + Return ('active', message) do shortcut and force the unit to the active + status. + Return (other_status, message) to set the status to desired state. + + :returns: None, None - no action in this function. + """ + return None, None + + def check_if_paused(self): + """Check if the unit is paused and return either the paused status, + message or None, None if the unit is not paused. If the unit is paused + but a service is incorrectly running, then the function returns a + broken status. + + :returns: (status, message) or (None, None) + """ + return os_utils._ows_check_if_paused( + services=self.services, + ports=self.ports_to_check(self.api_ports)) + + def check_interfaces(self): + """Check that the required interfaces have both connected and availble + states set. + + This requires a convention from the OS interfaces that they set the + '{relation_name}.connected' state on connection, and the + '{relation_name}.available' state when the connection information is + available and the interface is ready to go. + + The interfaces (relations) that are checked are named in + self.required_relations which is a list of strings representing the + generic relation name. e.g. 'identity-service' rather than 'keystone'. + + Returns (None, None) if the interfaces are okay, or a status, message + if any of the interfaces are not ready. + + Derived classes can augment/alter the checks done by overriding the + companion method :property:`states_to_check` which converts a relation + into the states to confirm existence, along with the error message. + + :returns (status, message) or (None, None) + """ + states_to_check = self.states_to_check + # bail if there is nothing to do. + if not states_to_check: + return None, None + available_states = charms.reactive.bus.get_states().keys() + status = None + messages = [] + for relation, states in states_to_check.items(): + for state, err_status, err_msg in states: + if state not in available_states: + messages.append(err_msg) + status = os_utils.workload_state_compare(status, + err_status) + # as soon as we error on a relation, skip to the next one. + break + if status is not None: + return status, ", ".join(messages) + # Everything is fine. + return None, None + + @property + def states_to_check(self): + """Construct a default set of connected and available states for each + of the relations passed, along with error messages and new status + conditions if they are missing. + + The method returns a {relation: [(state, err_status, err_msg), (...),]} + This corresponds to the relation, the state to check for, the error + status to set if that state is missing, and the message to show if the + state is missing. + + The list of tuples is evaulated in order for each relation, and stops + after the first failure. This means that it doesn't check (say) + available if connected is not available. + """ + states_to_check = { + relation: [("{}.connected".format(relation), + "blocked", + "'{}' missing".format(relation)), + ("{}.available".format(relation), + "waiting", + "'{}' incomplete".format(relation))] + for relation in self.required_relations} + return states_to_check + + def check_services_running(self): + """Check that the services that should be running are actually running. + + This uses the self.services and self.api_ports to determine what should + be checked. + + :returns: (status, message) or (None, None). + """ + # This returns either a None, None or a status, message if the service + # is not running or the ports are not open. + return os_utils._ows_check_services_running( + services=self.services, + ports=self.ports_to_check(self.api_ports)) + + def ports_to_check(self, ports): + """Return a flattened, sorted, unique list of ports from self.api_ports + + NOTE. To disable port checking, simply override this method in the + derived class and return an empty []. + + :param ports: {key: {subkey: value}} + :returns: [value1, value2, ...] + """ + # NB self.api_ports = {key: {space: value}} + # The chain .. map flattens all the values into a single list + return sorted(set(itertools.chain(*map(lambda x: x.values(), + self.api_ports.values())))) From b9eb56712d6b881f42bf1eea50f1d0ccb38cd836 Mon Sep 17 00:00:00 2001 From: Alex Kavanagh Date: Fri, 10 Jun 2016 14:15:17 +0000 Subject: [PATCH 2/9] Add README.md documentation for how to use the module This provides a summary/description of how to use the charms_openstack module and the classes therein. In particular it highlights the use of the key features and how to incorporate them into the derived charm class. --- README.md | 318 ++++++++++++++++++++++++++++++++++++++ charms_openstack/charm.py | 3 +- 2 files changed, 319 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index bf16284..31d3e05 100644 --- a/README.md +++ b/README.md @@ -14,3 +14,321 @@ If you prefer live discussions, some of us also hang out in Bug reports can be filed at https://bugs.launchpad.net/charms.openstack/+filebug +# Using `charms.openstack` + +charms.openstack provides a module `charms_openstack` which is included in +layer-openstack's `wheelhouse.txt`. It is provides the fundamental +functionality required of _most_ OpenStack charms. + +The main classes that the module provides are: + + * :class:`OpenStackRelationAdapter` + * :class:`RabbitMQRelationAdapter` + * :class:`DatabaseRelationAdapter` + * :class:`ConfigurationAdapter` + * :class:`OpenStackRelationsAdapter` + * :class:`OpenStackCharm` + +# Key features of `charms.openstack` + +The main features that `charms.openstack` provides are: + + * a base `OpenStackCharm` that provides: + * The ability to specify the OpenStack release that the charm works with. + * The list of packages to install on the charm. + * The ports that the charm exposes. + * The keystone service type (if applicable) + * A mapping of config files to services to restart if the configuration + changes. + * The required relations for the charm (workload status) + * The sync command that the database (if associated) will need for its + schema. + * a default install that gets the packages, installs them, and sets the + appropriate workload status. + * A configuration file renderer (using the relation adapters) to write + the configuration files for the service being managed. + * A workload status helper (`assess_status()`) that checks the state of + interfaces, the services, and ports, and sets the workload status. This + is automatically provided for the `update-status` hook in the `layer-openstack` + layer. + +# How to leverage `charms.openstack` classes + +## Using `OpenStackCharm` + +`OpenStackCharm()` and the related classes provide a powerful framework to +build an OpenStack charm on. There are two approaches to writing charms that +support multiple OpenStack releases. Note that determining the release _is up +to the charm author_, and can be signalled to `OpenStackCharm` in two ways. + + 1. Write a single `OpenStackCharm` derived class that uses `self.release` to + determine what functionality to exhibit depending on the release. In this + case, there is no need to register multiple charms and provide a _chooser_ + to determine which class to use. + + 2. Write muliple `OpenStackCharm` derived classes which map to each difference + in charm functionality depending on the release, and register a _chooser_ + function using the `@register_os_release_selector` decorator. + +e.g. + +```python +class LibertyCharm(OpenStackCharm): + release = 'liberty' + +class MitakaCharm(OpenStackCharm): + release = 'mitaka' + +@register_os_release_selector +def choose_release(): + """Determine the release based on the python-keystonemiddleware that is + installed. + """ + return ch_utils.os_release('python-keystonemiddleware') +``` + +This will automatically select `LibertyCharm` for a liberty release and +`MitakaCharm` for the mitaka release. Note, that it will also _set_ `release` +on the `OpenStackCharm` instance via the `__init__()` method, so that the +instance knows what the charm is. + +If only a single charm class is needed, the the `__init__()` method of the +class can be used to determine the release instead: + +```python +class TheCharm(OpenStackCharm): + release = 'liberty' + + def __init__(release=None, *args, **kwargs): + if release is None: + release = ch_utils.os_release('python-keystonemiddleware') + super(TheCharm, self).__init__(release=release, *args, **kwargs) +``` + +If the release selector function is registered, then the overridden +`__init__()` method is not needed as the release will be passed into the +default `__init__()` method. However, there may be other functionality that +the charm author needs to include in the initialiser. + +Note that using `os_release()` can typically be used to determine the release +of OpenStack. + +## Using the relation adapter classes - OpenStackRelationAdapter + +The relation adapter classes adapt a reactive interface for use in the +rendering functions. Their pricipal use is to provide an iterator of the +attributes declared in the `assessors` attribute of the instance. + +A reactive `BaseRelation` derived instance has an `auto_accessors` attribute +which declares the variables that the relation has. These are copied into the +`accessors` attribute of the `OpenStackRelationAdapter` class, and additional +attributes can be added as part of class instantiation. + +Note that the `accessor` properties are _dynamic_, in that they call the +underlying relation property when they are accessed. + +The _purpose_ of the `OpenStackRelation` class is for the instance to be used +as part of configuration file rendering, as an instance of an +`OpenStackRelation` class can be passed to the render function, and the +iterator will provide the _key value_ pairs to the template processor. + +A derived `OpenStackRelation` class can provide additional _computed_ +properties as required. e.g. the `RabbitMQRelationAdapter` implementation: + +```python +class RabbitMQRelationAdapter(OpenStackRelationAdapter): + """ + Adapter for the RabbitMQRequires relation interface. + """ + + interface_type = "messaging" + + def __init__(self, relation): + add_accessors = ['vhost', 'username'] + super(RabbitMQRelationAdapter, self).__init__(relation, add_accessors) + + @property + def host(self): + """ + Hostname that should be used to access RabbitMQ. + """ + if self.vip: + return self.vip + else: + return self.private_address + + @property + def hosts(self): + """ + Comma separated list of hosts that should be used + to access RabbitMQ. + """ + hosts = self.relation.rabbitmq_hosts() + if len(hosts) > 1: + return ','.join(hosts) + else: + return None +``` + +Note that the additional accessors `vhost` and `username` are provided in the +overridden `__init__()` method. + +## The `ConfigurationAdapter` + +The `ConfigurationAdapter` class simply provides _snapshot_ of the +configuration opentions for the current charm, such that they can be accessed +as attributes of an instance of the class. e.g. rather than `config('vip')` +then user can use `c_adapter.vip`. + +The benefit, is that a _derived_ version of `ConfigurationAdapter` can be +provided that has _computed_ properties that can be used like static properties +on the instance. The `ConfigurationAdapter`, or derived class, is used with +the `OpenStackRelationAdapters` class (not the plural _...Adapters_) class that +brings together all of the relations into one place. + +## The `OpenStackRelationAdapters` class + +The `OpenStackRelationAdapters` class joins together the relation adapter +classes, with the `ConfigurationAdapter` (or derived) class, and works _like_ +a charmhelpers `OSRenderConfig` instance to the rendering functions in +charmhelpers. + +Thus an instance of the `OpenStackRelationAdapters` (or derived) class is used +in the `charmhelpers.core.templating.render()` function to provide the +variables needed to render templates. + +The `OpenStackRelationAdapters` class can be subclassed (derived) with +additional custom `OpenStackRelationAdapter` classes (to map to particular +relations) using the `relation_adapters` class property: + +```python +class MyRelationAdapters(OpenStackRelationAdapters): + + relation_adapters = { + 'my-relation': MyRelationAdapter, + } +``` + +This enables custome relation adapters to be mapped to particular relations +such that custom functionality can be implemented for a particular reactive +relationship. + +## HighAvailability Support + +To be completed. + +## Workload status + +OpenStack charms support the concept of _workload status_ which helps to inform +a user of the charm of the current state of the charm. The following workload +statuses are supported: + + * unknown - The charm _doesn't_ support workload status. This should **not** + be used for charms that DO support workload status. + * active - The unit under the charms control is fully configuration + and available for use. + * maintenance - the unit is installing, or doing something of that nature. + * waiting - The unit is waiting for a relation to become available. i.e. the + relation is not yet _complete_ in that some data is missing still. + * blocked - a relation is not yet connected, or some other blocking + condition. + * paused - (Not yet availble) - the unit has been put into the paused state. + +The default is for charms to support workload status, and the default installation method sets the status to maintenance with an install message. + +If the charm is not going to support workload status, _and this is not +recommended_, then the charm author will need to override the `install()` +method of `OpenStackCharm` derived class to disable setting the `maintenance` +state, and override the `assess_status()` method to a NOP. + +The `assess_status()` method on `OpenStackCharm` provides a helper to enable +the charm author to provide workload status. By default: + + * The install method provides the maintenance status. + * The `layer-openstack` layer provides a hook for `update-status` which + calls the `assess_status()` function on the charm class. + * The `assess_status()` method uses various attributes of the class to provide + a default mechanism for assessing the workload status of the charm/unit. + +The latter is extremely useful for determining the workload status. The +`assess_status()` method does the following checks: + + 1. The unit checks if it is paused. (Not yet available as a feature). + 2. The unit checks the relations to see if they are connected and available. + 3. The unit checks `custom_assess_status_check()` + 4. The unit checks that the services are running and ports are open. + +### Checking of relations + +The assess_status function checks that the relations named in the class +attribute `required_relations` are connected and available. It does this using +the convention of: + + * A connected relation has the `{relation}.connected` state set. + * An available relation has the `{relation}.available` state set. + +This is a convention that the interfaces (e.g. interface-keystone, etc.) use. +interface-keystone sets `identity-service.connected` when it has a connection +with keystone, and `identity-service.available` when the connection is +completed and all information transferred. + +That if `required_relations` is `['identity-service']`, then the +`assess_status()` function will check for `identity-service.connected` and +`identity-service.available` states. + +If the charm author requires additional states to be checked for an interface, +then the method `states_to_check` should be overriden in the derived class and +additional states, the status and error message provided. See the code for +further details. + +e.g. + +```python +def states_to_check(): + states = super(MyCharm, self).states_to_check() + states['some-relation'].append( + ("some-relation.available.ssl", "waiting", "'some-relation' incomplete")) + return states +``` + +### The `custom_assess_status_check()` method + +If the charm author needs to do additional status checking, then the +`custom_assess_status_check()` method should be overridden in the derived +class. The return value from the method is: + + * (None, None) - the unit is fine. + * status, message - the unit's workload status is not active. + +### Not checking services are running + +By default, the `assess_status()` method checks that the services declared in +the class attribute `services` (list of strings) are checked to ensure that +they are running. Additionally, the ports declared in the class attribute +`api_ports` are also checked for being _listened on_. + +However, if the services check is not required, then the derived class should +overload the `check_running_services()` method and return `None, None`. + +Additionally, if the services running check _is_ required, but the ports should +not be checked, then the `ports_to_check` method can be overridden and return +an empty list `[]`. + +### Using `assess_status()` + +The `assess_status()` method should be used on any hook or state method where +the unit's status may have changed. e.g. interfaces connecting or becoming +available, configuration changes, etc. + +e.g. + +```python +@reactive.when('amqp.connected')¬ +def setup_amqp_req(amqp):¬ + """Use the amqp interface to request access to the amqp broker using our + local configuration. + """ + amqp.request_access(username=hookenv.config('rabbit-user'), + vhost=hookenv.config('rabbit-vhost')) + MyCharm.singleton.assess_status() +``` diff --git a/charms_openstack/charm.py b/charms_openstack/charm.py index c5316ef..a299d3f 100644 --- a/charms_openstack/charm.py +++ b/charms_openstack/charm.py @@ -508,7 +508,7 @@ class OpenStackCharm(object): :returns (status, message) or (None, None) """ - states_to_check = self.states_to_check + states_to_check = self.states_to_check() # bail if there is nothing to do. if not states_to_check: return None, None @@ -528,7 +528,6 @@ class OpenStackCharm(object): # Everything is fine. return None, None - @property def states_to_check(self): """Construct a default set of connected and available states for each of the relations passed, along with error messages and new status From 8e865b785da1782ac4b5c387bdc82e7b7fd1e985 Mon Sep 17 00:00:00 2001 From: Alex Kavanagh Date: Fri, 10 Jun 2016 14:27:21 +0000 Subject: [PATCH 3/9] Fix error where release is not passed to get_charm_instance() --- charms_openstack/charm.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/charms_openstack/charm.py b/charms_openstack/charm.py index 7bcaac9..16e2ece 100644 --- a/charms_openstack/charm.py +++ b/charms_openstack/charm.py @@ -194,7 +194,7 @@ class OpenStackCharmMeta(type): # see if a _release_selector_function has been registered. if _release_selector_function is not None: release = _release_selector_function() - _singleton = get_charm_instance() + _singleton = get_charm_instance(release=release) return _singleton From 2da69bf76cbb4090b6b05489c782f70e83e401d4 Mon Sep 17 00:00:00 2001 From: Alex Kavanagh Date: Fri, 10 Jun 2016 14:35:21 +0000 Subject: [PATCH 4/9] Fix test due to change in install method The method was changed to set the status twice, once at the beginning of the install and also at the end to indicate that the unit is waiting for the next status set. --- unit_tests/test_charms_openstack_charm.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/unit_tests/test_charms_openstack_charm.py b/unit_tests/test_charms_openstack_charm.py index f0e7b32..d31f337 100644 --- a/unit_tests/test_charms_openstack_charm.py +++ b/unit_tests/test_charms_openstack_charm.py @@ -444,8 +444,10 @@ class TestMyOpenStackCharm(BaseOpenStackCharmTest): self.target.install() self.target.set_state.assert_called_once_with('my-charm-installed') self.fip.assert_called_once_with(self.target.packages) - self.status_set.assert_called_once_with('maintenance', - 'Installing packages') + self.status_set.assert_has_calls([ + mock.call('maintenance', 'Installing packages'), + mock.call('maintenance', + 'Installation complete - awaiting next status')]) def test_api_port(self): self.assertEqual(self.target.api_port('service1'), 1) From ea6d6007f2ff7f3b252bcd386b84d7e3beb6ca0f Mon Sep 17 00:00:00 2001 From: Alex Kavanagh Date: Fri, 10 Jun 2016 15:36:28 +0000 Subject: [PATCH 5/9] Fix typo in README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 31d3e05..089d8a2 100644 --- a/README.md +++ b/README.md @@ -323,7 +323,7 @@ available, configuration changes, etc. e.g. ```python -@reactive.when('amqp.connected')¬ +@reactive.when('amqp.connected') def setup_amqp_req(amqp):¬ """Use the amqp interface to request access to the amqp broker using our local configuration. From 61b03f8536d4595582b44b824d35d16049af2c38 Mon Sep 17 00:00:00 2001 From: Alex Kavanagh Date: Mon, 13 Jun 2016 11:28:34 +0000 Subject: [PATCH 6/9] Fix bug in assess_status() which calls state_set() Should actually call status_set() --- charms_openstack/charm.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/charms_openstack/charm.py b/charms_openstack/charm.py index 16e2ece..0c31840 100644 --- a/charms_openstack/charm.py +++ b/charms_openstack/charm.py @@ -499,7 +499,7 @@ class OpenStackCharm(object): hookenv.status_set(state, message) return # No state was particularly set, so assume the unit is active - hookenv.state_set('active', 'Unit is ready') + hookenv.status_set('active', 'Unit is ready') def custom_assess_status_check(self): """Override this function in a derived class if there are any other From 9dbb7402efdbe5c500a39499c8020d16d6167d8c Mon Sep 17 00:00:00 2001 From: Alex Kavanagh Date: Mon, 13 Jun 2016 11:29:39 +0000 Subject: [PATCH 7/9] Add tests for assess_status() This adds unit tests for the assess_status() functionality added to OpenStackCharm() --- unit_tests/test_charms_openstack_charm.py | 146 ++++++++++++++++++++++ 1 file changed, 146 insertions(+) diff --git a/unit_tests/test_charms_openstack_charm.py b/unit_tests/test_charms_openstack_charm.py index d31f337..4629f4e 100644 --- a/unit_tests/test_charms_openstack_charm.py +++ b/unit_tests/test_charms_openstack_charm.py @@ -7,6 +7,7 @@ from __future__ import absolute_import import collections +import unittest import mock @@ -122,6 +123,34 @@ class TestFunctions(BaseOpenStackCharmTest): self.assertTrue(isinstance(chm.get_charm_instance(), self.C3)) +class TestRegisterOSReleaseSelector(unittest.TestCase): + + def test_register(self): + save_rsf = chm._release_selector_function + chm._release_selector_function = None + @chm.register_os_release_selector + def test_func(): + pass + + self.assertEqual(chm._release_selector_function, test_func) + chm._release_selector_function = save_rsf + + def test_cant_register_more_than_once(self): + save_rsf = chm._release_selector_function + chm._release_selector_function = None + @chm.register_os_release_selector + def test_func1(): + pass + + with self.assertRaises(RuntimeError): + @chm.register_os_release_selector + def test_func2(): + pass + + self.assertEqual(chm._release_selector_function, test_func1) + chm._release_selector_function = save_rsf + + class TestOpenStackCharm(BaseOpenStackCharmTest): # Note that this only tests the OpenStackCharm() class, which has not very # useful defaults for testing. In order to test all the code without too @@ -411,6 +440,11 @@ class MyOpenStackCharm(chm.OpenStackCharm): adapters_class = MyAdapter +class MyNextOpenStackCharm(MyOpenStackCharm): + + release = 'mitaka' + + class TestMyOpenStackCharm(BaseOpenStackCharmTest): def setUp(self): @@ -421,8 +455,13 @@ class TestMyOpenStackCharm(BaseOpenStackCharmTest): TEST_CONFIG) def test_singleton(self): + # because we have two releases, we expect this to be the latter. + # e.g. MyNextOpenStackCharm s = self.target.singleton + self.assertEqual(s.__class__.release, 'mitaka') self.assertTrue(isinstance(s, MyOpenStackCharm)) + # should also be the second one, as it's the latest + self.assertTrue(isinstance(s, MyNextOpenStackCharm)) self.assertTrue(isinstance(MyOpenStackCharm.singleton, MyOpenStackCharm)) self.assertTrue(isinstance(chm.OpenStackCharm.singleton, @@ -430,6 +469,21 @@ class TestMyOpenStackCharm(BaseOpenStackCharmTest): self.assertEqual(s, chm.OpenStackCharm.singleton) # Note that get_charm_instance() returns NEW instance each time. self.assertNotEqual(s, chm.get_charm_instance()) + # now clear out the singleton and make sure we get the first one using + # a release function + rsf_save = chm._release_selector_function + chm._release_selector_function = None + @chm.register_os_release_selector + def selector(): + return 'icehouse' + + # This should choose the icehouse version instead of the mitaka version + chm._singleton = None + s = self.target.singleton + self.assertEqual(s.release, 'icehouse') + self.assertEqual(s.__class__.release, 'icehouse') + self.assertFalse(isinstance(s, MyNextOpenStackCharm)) + chm._release_selector_function = rsf_save def test_install(self): # tests that the packages are filtered before installation @@ -549,3 +603,95 @@ class TestMyOpenStackCharm(BaseOpenStackCharmTest): # Assert that None was not passed to render via the context kwarg for call in self.render.call_args_list: self.assertTrue(call[1]['context']) + + def test_assess_status_active(self): + self.patch_object(chm.hookenv, 'status_set') + # disable all of the check functions + self.patch_target('check_if_paused', return_value=(None, None)) + self.patch_target('check_interfaces', return_value=(None, None)) + self.patch_target('custom_assess_status_check', + return_value=(None, None)) + self.patch_target('check_services_running', return_value=(None, None)) + self.target.assess_status() + self.status_set.assert_called_once_with('active', 'Unit is ready') + # check all the check functions got called + self.check_if_paused.assert_called_once_with() + self.check_interfaces.assert_called_once_with() + self.custom_assess_status_check.assert_called_once_with() + self.check_services_running.assert_called_once_with() + + def test_assess_status_paused(self): + self.patch_object(chm.hookenv, 'status_set') + # patch out _ows_check_if_paused + self.patch_object(chm.os_utils, '_ows_check_if_paused', + return_value=('paused', '123')) + self.target.assess_status() + self.status_set.assert_called_once_with('paused', '123') + self._ows_check_if_paused.assert_called_once_with( + services=self.target.services, + ports=[1, 2, 3, 1234, 2468, 3579]) + + def test_states_to_check(self): + self.patch_target('required_relations', new=['rel1', 'rel2']) + states = self.target.states_to_check() + self.assertEqual( + states, + { + 'rel1': [ + ('rel1.connected', 'blocked', "'rel1' missing"), + ('rel1.available', 'waiting', "'rel1' incomplete") + ], + 'rel2': [ + ('rel2.connected', 'blocked', "'rel2' missing"), + ('rel2.available', 'waiting', "'rel2' incomplete") + ] + }) + + def test_assess_status_check_interfaces(self): + self.patch_object(chm.hookenv, 'status_set') + self.patch_target('check_if_paused', return_value=(None, None)) + # first check it returns None, None if there are no states + with mock.patch.object(self.target, + 'states_to_check', + return_value={}): + self.assertEqual(self.target.check_interfaces(), (None, None)) + # next check that we get back the states we think we should + self.patch_object(chm.charms.reactive.bus, + 'get_states', + return_value={'rel1.connected': 1,}) + self.patch_target('required_relations', new=['rel1', 'rel2']) + def my_compare(x, y): + if x is None: + x = 'unknown' + if x <= y: + return x + return y + + self.patch_object(chm.os_utils, 'workload_state_compare', + new=my_compare) + self.assertEqual(self.target.check_interfaces(), + ('blocked', "'rel1' incomplete, 'rel2' missing")) + # check that the assess_status give the same result + self.target.assess_status() + self.status_set.assert_called_once_with( + 'blocked', "'rel1' incomplete, 'rel2' missing") + + # Now check it returns None, None if all states are available + self.get_states.return_value = { + 'rel1.connected': 1, + 'rel1.available': 2, + 'rel2.connected': 3, + 'rel2.available': 4, + } + self.assertEqual(self.target.check_interfaces(), (None, None)) + + def test_check_assess_status_check_services_running(self): + # verify that the function calls _ows_check_services_running() with the + # valid information + self.patch_object(chm.os_utils, '_ows_check_services_running', + return_value = ('active', 'that')) + status, message = self.target.check_services_running() + self.assertEqual((status, message), ('active', 'that')) + self._ows_check_services_running.assert_called_once_with( + services=['my-default-service', 'my-second-service'], + ports=[1, 2, 3, 1234, 2468, 3579]) From 3805807fca928efbb3b3bc2dd106d3ad5a326582 Mon Sep 17 00:00:00 2001 From: Alex Kavanagh Date: Mon, 13 Jun 2016 11:34:09 +0000 Subject: [PATCH 8/9] Fix linting whitespace issues on tests --- unit_tests/test_charms_openstack_charm.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/unit_tests/test_charms_openstack_charm.py b/unit_tests/test_charms_openstack_charm.py index 4629f4e..406da37 100644 --- a/unit_tests/test_charms_openstack_charm.py +++ b/unit_tests/test_charms_openstack_charm.py @@ -128,6 +128,7 @@ class TestRegisterOSReleaseSelector(unittest.TestCase): def test_register(self): save_rsf = chm._release_selector_function chm._release_selector_function = None + @chm.register_os_release_selector def test_func(): pass @@ -138,6 +139,7 @@ class TestRegisterOSReleaseSelector(unittest.TestCase): def test_cant_register_more_than_once(self): save_rsf = chm._release_selector_function chm._release_selector_function = None + @chm.register_os_release_selector def test_func1(): pass @@ -473,6 +475,7 @@ class TestMyOpenStackCharm(BaseOpenStackCharmTest): # a release function rsf_save = chm._release_selector_function chm._release_selector_function = None + @chm.register_os_release_selector def selector(): return 'icehouse' @@ -658,8 +661,9 @@ class TestMyOpenStackCharm(BaseOpenStackCharmTest): # next check that we get back the states we think we should self.patch_object(chm.charms.reactive.bus, 'get_states', - return_value={'rel1.connected': 1,}) + return_value={'rel1.connected': 1, }) self.patch_target('required_relations', new=['rel1', 'rel2']) + def my_compare(x, y): if x is None: x = 'unknown' @@ -689,7 +693,7 @@ class TestMyOpenStackCharm(BaseOpenStackCharmTest): # verify that the function calls _ows_check_services_running() with the # valid information self.patch_object(chm.os_utils, '_ows_check_services_running', - return_value = ('active', 'that')) + return_value=('active', 'that')) status, message = self.target.check_services_running() self.assertEqual((status, message), ('active', 'that')) self._ows_check_services_running.assert_called_once_with( From 63c70381cf5808ac12ac209a37f3b4d1bd13b6ae Mon Sep 17 00:00:00 2001 From: Alex Kavanagh Date: Wed, 15 Jun 2016 14:31:13 +0000 Subject: [PATCH 9/9] Change states_to_check() to return OrderedDict Previously it returned a simple dictionary. However, this means that the function (potentially) gives different ordering of states to check between Py27 and Py35, but more importantly, different to the order of the required_relations. This provides a fix for that. --- charms_openstack/charm.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/charms_openstack/charm.py b/charms_openstack/charm.py index 0c31840..d8a4612 100644 --- a/charms_openstack/charm.py +++ b/charms_openstack/charm.py @@ -582,14 +582,15 @@ class OpenStackCharm(object): after the first failure. This means that it doesn't check (say) available if connected is not available. """ - states_to_check = { - relation: [("{}.connected".format(relation), - "blocked", - "'{}' missing".format(relation)), - ("{}.available".format(relation), - "waiting", - "'{}' incomplete".format(relation))] - for relation in self.required_relations} + states_to_check = collections.OrderedDict() + for relation in self.required_relations: + states_to_check[relation] = [ + ("{}.connected".format(relation), + "blocked", + "'{}' missing".format(relation)), + ("{}.available".format(relation), + "waiting", + "'{}' incomplete".format(relation))] return states_to_check def check_services_running(self):