Merge lp:~johnsca/charm-helpers/services-callback-fu into lp:~cf-charmers/charm-helpers/cloud-foundry

[Cory Johns]

Callbacks instead of ServiceManagers

Refactored charmhelpers.core.services to use callbacks instead of
service types / managers, to make it easier for charms to specify
custom behavior.

https://codereview.appspot.com/98490043/

[Cory Johns (johnsca)]

Reviewers: mp+220733_code.launchpad.net,

Message:
Please take a look.

Description:
Callbacks instead of ServiceManagers

Refactored charmhelpers.core.services to use callbacks instead of
service types / managers, to make it easier for charms to specify
custom behavior.

https://code.launchpad.net/~johnsca/charm-helpers/services-callback-fu/+merge/220733

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/98490043/

Affected files (+146, -92 lines):
   A [revision details]
   M charmhelpers/core/host.py
   M charmhelpers/core/services.py
   M charmhelpers/core/templating.py
   M tests/core/test_host.py
   M tests/core/test_services.py
   M tox.ini

[Benjamin Saller (bcsaller)]

LGTM with some trivials

Thanks for this.

https://codereview.appspot.com/98490043/diff/1/charmhelpers/core/host.py
File charmhelpers/core/host.py (right):

https://codereview.appspot.com/98490043/diff/1/charmhelpers/core/host.py#newcode67
charmhelpers/core/host.py:67: """Determine whether a system service is
available"""
'an OS service' or 'an init service' or 'an upstart service'

or something similar, service is very overloaded in this context

https://codereview.appspot.com/98490043/diff/1/charmhelpers/core/services.py
File charmhelpers/core/services.py (right):

https://codereview.appspot.com/98490043/diff/1/charmhelpers/core/services.py#newcode32
charmhelpers/core/services.py:32: restarting the service via Upstart).
Indicate that callback is called with service name,

We should also consider an 'incomplete' event, if a relation is broken
we might stop the service for example.

https://codereview.appspot.com/98490043/diff/1/charmhelpers/core/services.py#newcode35
charmhelpers/core/services.py:35: the service via Upstart).
stop is fine, for a stop hook, but incomplete would be for
relation-broken hooks. We can defer that till we actually intend to
support it though.

https://codereview.appspot.com/98490043/diff/1/charmhelpers/core/services.py#newcode94
charmhelpers/core/services.py:94: for service_name in service_names or
SERVICES.keys():
Handle single argument string calls,
if service_names and isinstance(service_names, str):
    service_names = [service_names]

Makes sense with the 'incomplete' mentioned above

https://codereview.appspot.com/98490043/diff/1/charmhelpers/core/services.py#newcode104
charmhelpers/core/services.py:104: Given the name of a registered
service, return a ServiceManager
This isn't still a ServiceManager as per the comment if I'm reading this
properly.

https://codereview.appspot.com/98490043/

[lp:~johnsca/charm-helpers/services-callback-fu]

183. By Cory Johns on 2014-05-23

Updated services.register docstring to clarify when and how the callbacks are called

[Cory Johns (johnsca)]

On 2014/05/22 23:47:43, benjamin.saller wrote:
> We should also consider an 'incomplete' event, if a relation is broken
we might
> stop the service for example.

> stop is fine, for a stop hook, but incomplete would be for
relation-broken
> hooks. We can defer that till we actually intend to support it though.

I wonder if we should change "complete" to "start," since what we're
really saying is "this code should run when the service is ready to
start." Then, "stop" can be called if stop_services() is called or if a
relationship gets broken such that the service no longer has all of the
valid information ("this code should run when the service has to stop,
for whatever reason").

> Handle single argument string calls,

https://codereview.appspot.com/98490043/diff/1/tests/core/test_services.py#newcode94

`foo(*args)` is a "varargs" definition. It will accept `foo()`,
`foo('bar')`, `foo('bar', 'baz')`, etc. (but not `params = ['bar',
'baz']; foo(params)`; that would have to be written `params = ['bar',
'baz']; foo(*params)`), and `args` will automatically be a list.

https://codereview.appspot.com/98490043/

[lp:~johnsca/charm-helpers/services-callback-fu]

184. By Cory Johns on 2014-05-26

Refined services API to deemphasize template management and emphasize service management

[Cory Johns (johnsca)]

Please take a look.

https://codereview.appspot.com/98490043/

[lp:~johnsca/charm-helpers/services-callback-fu]

185. By Cory Johns on 2014-05-26

Fixed missed key change in services API

186. By Cory Johns on 2014-05-26

Fixed key error checking services.is_ready() before any relations are present

[Cory Johns (johnsca)]

Please take a look.

https://codereview.appspot.com/98490043/

[lp:~johnsca/charm-helpers/services-callback-fu]

187. By Cory Johns on 2014-05-27

Switch services API from module global to instance, per review

[Cory Johns (johnsca)]

Please take a look.

https://codereview.appspot.com/98490043/

[Benjamin Saller (bcsaller)]

Comments for this went into the charm, but this LGTM as well, thanks.

https://codereview.appspot.com/98490043/

[lp:~johnsca/charm-helpers/services-callback-fu]

188. By Cory Johns on 2014-05-27

Added managed list of ports to services API

[Cory Johns (johnsca)]

Please take a look.

https://codereview.appspot.com/98490043/

[lp:~johnsca/charm-helpers/services-callback-fu]

189. By Cory Johns on 2014-05-29

Removed default target for template helper, as it is too prone to errors

[Cory Johns (johnsca)]

Please take a look.

https://codereview.appspot.com/98490043/

[Benjamin Saller (bcsaller)]

Thanks for tracking down the err0r, and yes, explicit win (esp. since we
hit this in practice).

+1

https://codereview.appspot.com/98490043/

[Cory Johns (johnsca)]

*** Submitted:

Callbacks instead of ServiceManagers

Refactored charmhelpers.core.services to use callbacks instead of
service types / managers, to make it easier for charms to specify
custom behavior.

R=benjamin.saller
CC=
https://codereview.appspot.com/98490043

https://codereview.appspot.com/98490043/

=== modified file 'charmhelpers/contrib/cloudfoundry/contexts.py'
--- charmhelpers/contrib/cloudfoundry/contexts.py	2014-05-16 23:28:28 +0000
+++ charmhelpers/contrib/cloudfoundry/contexts.py	2014-05-29 17:24:14 +0000
@@ -1,55 +1,64 @@
 import os
-
-from charmhelpers.core.templating import (
-    ContextGenerator,
-    RelationContext,
-    StorableContext,
-)
-
-
-# Stores `config_data` hash into yaml file with `file_name` as a name
-# if `file_name` already exists, then it loads data from `file_name`.
-class StoredContext(ContextGenerator, StorableContext):
+import yaml
+
+from charmhelpers.core.services import RelationContext
+
+
+class StoredContext(dict):
+    """
+    A data context that always returns the data that it was first created with.
+    """
     def __init__(self, file_name, config_data):
+        """
+        If the file exists, populate `self` with the data from the file.
+        Otherwise, populate with the given data and persist it to the file.
+        """
         if os.path.exists(file_name):
-            self.data = self.read_context(file_name)
+            self.update(self.read_context(file_name))
         else:
             self.store_context(file_name, config_data)
-            self.data = config_data
-
-    def __call__(self):
-        return self.data
-
-
-class NatsContext(RelationContext):
+            self.update(config_data)
+
+    def store_context(self, file_name, config_data):
+        with open(file_name, 'w') as file_stream:
+            yaml.dump(config_data, file_stream)
+
+    def read_context(self, file_name):
+        with open(file_name, 'r') as file_stream:
+            data = yaml.load(file_stream)
+            if not data:
+                raise OSError("%s is empty" % file_name)
+            return data
+
+
+class NatsRelation(RelationContext):
     interface = 'nats'
     required_keys = ['nats_port', 'nats_address', 'nats_user', 'nats_password']
 
 
-class MysqlDSNContext(RelationContext):
+class MysqlRelation(RelationContext):
     interface = 'db'
     required_keys = ['user', 'password', 'host', 'database']
     dsn_template = "mysql2://{user}:{password}@{host}:{port}/{database}"
 
-    def __call__(self):
-        ctx = RelationContext.__call__(self)
-        if ctx:
-            if 'port' not in ctx:
-                ctx['db']['port'] = '3306'
-            ctx['db']['dsn'] = self.dsn_template.format(**ctx['db'])
-        return ctx
-
-
-class RouterContext(RelationContext):
+    def get_data(self):
+        RelationContext.get_data(self)
+        if self.is_ready():
+            if 'port' not in self['db']:
+                self['db']['port'] = '3306'
+            self['db']['dsn'] = self.dsn_template.format(**self['db'])
+
+
+class RouterRelation(RelationContext):
     interface = 'router'
     required_keys = ['domain']
 
 
-class LogRouterContext(RelationContext):
+class LogRouterRelation(RelationContext):
     interface = 'logrouter'
     required_keys = ['shared-secret', 'logrouter-address']
 
 
-class LoggregatorContext(RelationContext):
+class LoggregatorRelation(RelationContext):
     interface = 'loggregator'
     required_keys = ['shared_secret', 'loggregator_address']

=== modified file 'charmhelpers/core/host.py'
--- charmhelpers/core/host.py	2014-05-20 15:40:48 +0000
+++ charmhelpers/core/host.py	2014-05-29 17:24:14 +0000
@@ -63,6 +63,11 @@
             return False
 
 
+def service_available(service_name):
+    """Determine whether a system service is available"""
+    return service('status', service_name)
+
+
 def adduser(username, password=None, shell='/bin/bash', system_user=False):
     """Add a user to the system"""
     try:

=== modified file 'charmhelpers/core/services.py'
--- charmhelpers/core/services.py	2014-05-16 21:48:06 +0000
+++ charmhelpers/core/services.py	2014-05-29 17:24:14 +0000
@@ -1,84 +1,321 @@
+import os
+import sys
+from collections import Iterable
 from charmhelpers.core import templating
 from charmhelpers.core import host
-
-
-SERVICES = {}
-
-
-def register(services, templates_dir=None):
-    """
-    Register a list of service configs.
-
-    Service Configs are dicts in the following formats:
-
-        {
-            "service": <service name>,
-            "templates": [ {
-                'target': <render target of template>,
-                'source': <optional name of template in passed in templates_dir>
-                'file_properties': <optional dict taking owner and octal mode>
-                'contexts': [ context generators, see contexts.py ]
-                }
-            ] }
-
-    Either `source` or `target` must be provided.
-
-    If 'source' is not provided for a template the templates_dir will
-    be consulted for ``basename(target).j2``.
-
-    If `target` is not provided, it will be assumed to be
-    ``/etc/init/<service name>.conf``.
-    """
-    for service in services:
-        service.setdefault('templates_dir', templates_dir)
-        SERVICES[service['service']] = service
-
-
-def reconfigure_services(restart=True):
-    """
-    Update all files for all services and optionally restart them, if ready.
-    """
-    for service_name in SERVICES.keys():
-        reconfigure_service(service_name, restart=restart)
-
-
-def reconfigure_service(service_name, restart=True):
-    """
-    Update all files for a single service and optionally restart it, if ready.
-    """
-    service = SERVICES.get(service_name)
-    if not service or service['service'] != service_name:
-        raise KeyError('Service not registered: %s' % service_name)
-
-    manager_type = service.get('type', UpstartService)
-    manager_type(service).reconfigure(restart)
-
-
-def stop_services():
-    for service_name in SERVICES.keys():
-        if host.service_running(service_name):
-            host.service_stop(service_name)
-
-
-class ServiceTypeManager(object):
-    def __init__(self, service_definition):
-        self.service_name = service_definition['service']
-        self.templates = service_definition['templates']
-        self.templates_dir = service_definition['templates_dir']
-
-    def reconfigure(self, restart=True):
+from charmhelpers.core import hookenv
+
+
+class ServiceManager(object):
+    def __init__(self, services=None):
+        """
+        Register a list of services, given their definitions.
+
+        Service definitions are dicts in the following formats (all keys except
+        'service' are optional):
+
+            {
+                "service": <service name>,
+                "required_data": <list of required data contexts>,
+                "data_ready": <one or more callbacks>,
+                "data_lost": <one or more callbacks>,
+                "start": <one or more callbacks>,
+                "stop": <one or more callbacks>,
+                "ports": <list of ports to manage>,
+            }
+
+        The 'required_data' list should contain dicts of required data (or
+        dependency managers that act like dicts and know how to collect the data).
+        Only when all items in the 'required_data' list are populated are the list
+        of 'data_ready' and 'start' callbacks executed.  See `is_ready()` for more
+        information.
+
+        The 'data_ready' value should be either a single callback, or a list of
+        callbacks, to be called when all items in 'required_data' pass `is_ready()`.
+        Each callback will be called with the service name as the only parameter.
+        After these all of the 'data_ready' callbacks are called, the 'start'
+        callbacks are fired.
+
+        The 'data_lost' value should be either a single callback, or a list of
+        callbacks, to be called when a 'required_data' item no longer passes
+        `is_ready()`.  Each callback will be called with the service name as the
+        only parameter.  After these all of the 'data_ready' callbacks are called,
+        the 'stop' callbacks are fired.
+
+        The 'start' value should be either a single callback, or a list of
+        callbacks, to be called when starting the service, after the 'data_ready'
+        callbacks are complete.  Each callback will be called with the service
+        name as the only parameter.  This defaults to
+        `[host.service_start, services.open_ports]`.
+
+        The 'stop' value should be either a single callback, or a list of
+        callbacks, to be called when stopping the service.  If the service is
+        being stopped because it no longer has all of its 'required_data', this
+        will be called after all of the 'data_lost' callbacks are complete.
+        Each callback will be called with the service name as the only parameter.
+        This defaults to `[services.close_ports, host.service_stop]`.
+
+        The 'ports' value should be a list of ports to manage.  The default
+        'start' handler will open the ports after the service is started,
+        and the default 'stop' handler will close the ports prior to stopping
+        the service.
+
+
+        Examples:
+
+        The following registers an Upstart service called bingod that depends on
+        a mongodb relation and which runs a custom `db_migrate` function prior to
+        restarting the service, and a Runit serivce called spadesd.
+
+            >>> manager = services.ServiceManager([
+            ...     {
+            ...         'service': 'bingod',
+            ...         'ports': [80, 443],
+            ...         'required_data': [MongoRelation(), config()],
+            ...         'data_ready': [
+            ...             services.template(source='bingod.conf'),
+            ...             services.template(source='bingod.ini',
+            ...                               target='/etc/bingod.ini',
+            ...                               owner='bingo', perms=0400),
+            ...         ],
+            ...     },
+            ...     {
+            ...         'service': 'spadesd',
+            ...         'data_ready': services.template(source='spadesd_run.j2',
+            ...                                         target='/etc/sv/spadesd/run',
+            ...                                         perms=0555),
+            ...         'start': runit_start,
+            ...         'stop': runit_stop,
+            ...     },
+            ... ])
+            ... manager.manage()
+        """
+        self.services = {}
+        for service in services or []:
+            service_name = service['service']
+            self.services[service_name] = service
+
+    def manage(self):
+        """
+        Handle the current hook by doing The Right Thing with the registered services.
+        """
+        hook_name = os.path.basename(sys.argv[0])
+        if hook_name == 'stop':
+            self.stop_services()
+        else:
+            self.reconfigure_services()
+
+    def reconfigure_services(self, *service_names):
+        """
+        Update all files for one or more registered services, and,
+        if ready, optionally restart them.
+
+        If no service names are given, reconfigures all registered services.
+        """
+        for service_name in service_names or self.services.keys():
+            if self.is_ready(service_name):
+                self.fire_event('data_ready', service_name)
+                self.fire_event('start', service_name, default=[
+                    host.service_restart,
+                    open_ports])
+                self.save_ready(service_name)
+            else:
+                if self.was_ready(service_name):
+                    self.fire_event('data_lost', service_name)
+                self.fire_event('stop', service_name, default=[
+                    close_ports,
+                    host.service_stop])
+                self.save_lost(service_name)
+
+    def stop_services(self, *service_names):
+        """
+        Stop one or more registered services, by name.
+
+        If no service names are given, stops all registered services.
+        """
+        for service_name in service_names or self.services.keys():
+            self.fire_event('stop', service_name, default=[
+                close_ports,
+                host.service_stop])
+
+    def get_service(self, service_name):
+        """
+        Given the name of a registered service, return its service definition.
+        """
+        service = self.services.get(service_name)
+        if not service:
+            raise KeyError('Service not registered: %s' % service_name)
+        return service
+
+    def fire_event(self, event_name, service_name, default=None):
+        """
+        Fire a data_ready, data_lost, start, or stop event on a given service.
+        """
+        service = self.get_service(service_name)
+        callbacks = service.get(event_name, default)
+        if not callbacks:
+            return
+        if not isinstance(callbacks, Iterable):
+            callbacks = [callbacks]
+        for callback in callbacks:
+            if isinstance(callback, ManagerCallback):
+                callback(self, service_name, event_name)
+            else:
+                callback(service_name)
+
+    def is_ready(self, service_name):
+        """
+        Determine if a registered service is ready, by checking its 'required_data'.
+
+        A 'required_data' item can be any mapping type, and is considered ready
+        if `bool(item)` evaluates as True.
+        """
+        service = self.get_service(service_name)
+        reqs = service.get('required_data', [])
+        return all(bool(req) for req in reqs)
+
+    def save_ready(self, service_name):
+        """
+        Save an indicator that the given service is now data_ready.
+        """
+        ready_file = '{}/.ready.{}'.format(hookenv.charm_dir(), service_name)
+        with open(ready_file, 'a'):
+            pass
+
+    def save_lost(self, service_name):
+        """
+        Save an indicator that the given service is no longer data_ready.
+        """
+        ready_file = '{}/.ready.{}'.format(hookenv.charm_dir(), service_name)
+        if os.path.exists(ready_file):
+            os.remove(ready_file)
+
+    def was_ready(self, service_name):
+        """
+        Determine if the given service was previously data_ready.
+        """
+        ready_file = '{}/.ready.{}'.format(hookenv.charm_dir(), service_name)
+        return os.path.exists(ready_file)
+
+
+class RelationContext(dict):
+    """
+    Base class for a context generator that gets relation data from juju.
+
+    Subclasses must provide `interface`, which is the interface type of interest,
+    and `required_keys`, which is the set of keys required for the relation to
+    be considered complete.  The first relation for the interface that is complete
+    will be used to populate the data for template.
+
+    The generated context will be namespaced under the interface type, to prevent
+    potential naming conflicts.
+    """
+    interface = None
+    required_keys = []
+
+    def __bool__(self):
+        """
+        Updates the data and returns True if all of the required_keys are available.
+        """
+        self.get_data()
+        return self.is_ready()
+
+    __nonzero__ = __bool__
+
+    def is_ready(self):
+        """
+        Returns True if all of the required_keys are available.
+        """
+        return set(self.get(self.interface, {}).keys()).issuperset(set(self.required_keys))
+
+    def get_data(self):
+        """
+        Retrieve the relation data and store it under `self[self.interface]`.
+
+        If there are more than one units related on the desired interface,
+        then each unit will have its data stored under `self[self.interface][unit_id]`
+        and one of the units with complete information will chosen at random
+        to fill the values at `self[self.interface]`.
+
+
+        For example:
+
+            {
+                'foo': 'bar',
+                'unit/0': {
+                    'foo': 'bar',
+                },
+                'unit/1': {
+                    'foo': 'baz',
+                },
+            }
+        """
+        if not hookenv.relation_ids(self.interface):
+            return
+
+        ns = self.setdefault(self.interface, {})
+        required = set(self.required_keys)
+        for rid in hookenv.relation_ids(self.interface):
+            for unit in hookenv.related_units(rid):
+                reldata = hookenv.relation_get(rid=rid, unit=unit)
+                unit_ns = ns.setdefault(unit, {})
+                unit_ns.update(reldata)
+                if set(reldata.keys()).issuperset(required):
+                    ns.update(reldata)
+
+
+class ManagerCallback(object):
+    """
+    Special case of a callback that takes the `ServiceManager` instance
+    in addition to the service name.
+
+    Subclasses should implement `__call__` which should accept two parameters:
+
+        * `manager`       The `ServiceManager` instance
+        * `service_name`  The name of the service it's being triggered for
+        * `event_name`    The name of the event that this callback is handling
+    """
+    def __call__(self, manager, service_name, event_name):
         raise NotImplementedError()
 
 
-class UpstartService(ServiceTypeManager):
-    def __init__(self, service_definition):
-        super(UpstartService, self).__init__(service_definition)
-        for tmpl in self.templates:
-            if 'target' not in tmpl:
-                tmpl['target'] = '/etc/init/%s.conf' % self.service_name
-
-    def reconfigure(self, restart):
-        complete = templating.render(self.templates, self.templates_dir)
-
-        if restart and complete:
-            host.service_restart(self.service_name)
+class TemplateCallback(ManagerCallback):
+    """
+    Callback class that will render a template, for use as a ready action.
+
+    The `target` param, if omitted, will default to `/etc/init/<service name>`.
+    """
+    def __init__(self, source, target, owner='root', group='root', perms=0444):
+        self.source = source
+        self.target = target
+        self.owner = owner
+        self.group = group
+        self.perms = perms
+
+    def __call__(self, manager, service_name, event_name):
+        service = manager.get_service(service_name)
+        context = {}
+        for ctx in service.get('required_data', []):
+            context.update(ctx)
+        templating.render(self.source, self.target, context,
+                          self.owner, self.group, self.perms)
+
+
+class PortManagerCallback(ManagerCallback):
+    """
+    Callback class that will open or close ports, for use as either
+    a start or stop action.
+    """
+    def __call__(self, manager, service_name, event_name):
+        service = manager.get_service(service_name)
+        for port in service.get('ports', []):
+            if event_name == 'start':
+                hookenv.open_port(port)
+            elif event_name == 'stop':
+                hookenv.close_port(port)
+
+
+# Convenience aliases
+template = TemplateCallback
+open_ports = PortManagerCallback()
+close_ports = PortManagerCallback()

=== modified file 'charmhelpers/core/templating.py'
--- charmhelpers/core/templating.py	2014-05-20 15:40:48 +0000
+++ charmhelpers/core/templating.py	2014-05-29 17:24:14 +0000
@@ -1,158 +1,51 @@
 import os
-import yaml
 
 from charmhelpers.core import host
 from charmhelpers.core import hookenv
 
 
-class ContextGenerator(object):
-    """
-    Base interface for template context container generators.
-
-    A template context is a dictionary that contains data needed to populate
-    the template.  The generator instance should produce the context when
-    called (without arguments) by collecting information from juju (config-get,
-    relation-get, etc), the system, or whatever other sources are appropriate.
-
-    A context generator should only return any values if it has enough information
-    to provide all of its values.  Any context that is missing data is considered
-    incomplete and will cause that template to not render until it has all of its
-    necessary data.
-
-    The template may receive several contexts, which will be merged together,
-    so care should be taken in the key names.
-    """
-    def __call__(self):
-        raise NotImplementedError
-
-
-class StorableContext(object):
-    """
-    A mixin for persisting a context to disk.
-    """
-    def store_context(self, file_name, config_data):
-        with open(file_name, 'w') as file_stream:
-            yaml.dump(config_data, file_stream)
-
-    def read_context(self, file_name):
-        with open(file_name, 'r') as file_stream:
-            data = yaml.load(file_stream)
-            if not data:
-                raise OSError("%s is empty" % file_name)
-            return data
-
-
-class ConfigContext(ContextGenerator):
-    """
-    A context generator that generates a context containing all of the
-    juju config values.
-    """
-    def __call__(self):
-        return hookenv.config()
-
-
-class RelationContext(ContextGenerator):
-    """
-    Base class for a context generator that gets relation data from juju.
-
-    Subclasses must provide `interface`, which is the interface type of interest,
-    and `required_keys`, which is the set of keys required for the relation to
-    be considered complete.  The first relation for the interface that is complete
-    will be used to populate the data for template.
-
-    The generated context will be namespaced under the interface type, to prevent
-    potential naming conflicts.
-    """
-    interface = None
-    required_keys = []
-
-    def __call__(self):
-        if not hookenv.relation_ids(self.interface):
-            return {}
-
-        ctx = {}
-        for rid in hookenv.relation_ids(self.interface):
-            for unit in hookenv.related_units(rid):
-                reldata = hookenv.relation_get(rid=rid, unit=unit)
-                required = set(self.required_keys)
-                if set(reldata.keys()).issuperset(required):
-                    ns = ctx.setdefault(self.interface, {})
-                    for k, v in reldata.items():
-                        ns[k] = v
-                    return ctx
-
-        return {}
-
-
-class StaticContext(ContextGenerator):
-    def __init__(self, data):
-        self.data = data
-
-    def __call__(self):
-        return self.data
-
-
-def _collect_contexts(context_providers):
-    """
-    Helper function to collect and merge contexts from a list of providers.
-
-    If any of the contexts are incomplete (i.e., they return an empty dict),
-    the template is considered incomplete and will not render.
-    """
-    ctx = {}
-    for provider in context_providers:
-        c = provider()
-        if not c:
-            return False
-        ctx.update(c)
-    return ctx
-
-
-def render(template_definitions, templates_dir=None):
-    """
-    Render one or more templates, given a list of template definitions.
-
-    The template definitions should be dicts with the keys: `source`, `target`,
-    `file_properties`, and `contexts`.
-
-    The `source` path, if not absolute, is relative to the `templates_dir`
-    given when the rendered was created.  If `source` is not provided
-    for a template the `template_dir` will be consulted for
-    ``basename(target).j2``.
+def render(source, target, context, owner='root', group='root', perms=0444, templates_dir=None):
+    """
+    Render a template.
+
+    The `source` path, if not absolute, is relative to the `templates_dir`.
 
     The `target` path should be absolute.
 
-    The `file_properties` should be a dict optionally containing
-    `owner`, `group`, or `perms` options, to be passed to `write_file`.
-
-    The `contexts` should be a list containing zero or more ContextGenerators.
-
-    The `template_dir` defaults to `$CHARM_DIR/templates`
-
-    Returns True if all of the templates were "complete" (i.e., the context
-    generators were able to collect the information needed to render the
-    template) and were rendered.
+    The context should be a dict containing the values to be replaced in the
+    template.
+
+    The `owner`, `group`, and `perms` options will be passed to `write_file`.
+
+    If omitted, `templates_dir` defaults to the `templates` folder in the charm.
+
+    Note: Using this requires python-jinja2; if it is not installed, calling
+    this will attempt to use charmhelpers.fetch.apt_install to install it.
     """
-    # lazy import jinja2 in case templating is needed in install hook
-    from jinja2 import FileSystemLoader, Environment, exceptions
-    all_complete = True
+    try:
+        from jinja2 import FileSystemLoader, Environment, exceptions
+    except ImportError:
+        try:
+            from charmhelpers.fetch import apt_install
+        except ImportError:
+            hookenv.log('Could not import jinja2, and could not import '
+                        'charmhelpers.fetch to install it',
+                        level=hookenv.ERROR)
+            raise
+        apt_install('python-jinja2', fatal=True)
+        from jinja2 import FileSystemLoader, Environment, exceptions
+
     if templates_dir is None:
         templates_dir = os.path.join(hookenv.charm_dir(), 'templates')
     loader = Environment(loader=FileSystemLoader(templates_dir))
-    for tmpl in template_definitions:
-        ctx = _collect_contexts(tmpl.get('contexts', []))
-        if ctx is False:
-            all_complete = False
-            continue
-        try:
-            source = tmpl.get('source', os.path.basename(tmpl['target'])+'.j2')
-            template = loader.get_template(source)
-        except exceptions.TemplateNotFound as e:
-            hookenv.log('Could not load template %s from %s.' %
-                        (tmpl['source'], templates_dir),
-                        level=hookenv.ERROR)
-            raise e
-        content = template.render(ctx)
-        host.mkdir(os.path.dirname(tmpl['target']))
-        host.write_file(tmpl['target'], content, **tmpl.get('file_properties', {}))
-    return all_complete
+    try:
+        source = source
+        template = loader.get_template(source)
+    except exceptions.TemplateNotFound as e:
+        hookenv.log('Could not load template %s from %s.' %
+                    (source, templates_dir),
+                    level=hookenv.ERROR)
+        raise e
+    content = template.render(context)
+    host.mkdir(os.path.dirname(target))
+    host.write_file(target, content, owner, group, perms)

=== modified file 'test-requirements-tox.txt'
--- test-requirements-tox.txt	2014-05-09 16:10:54 +0000
+++ test-requirements-tox.txt	2014-05-29 17:24:14 +0000
@@ -15,4 +15,4 @@
 wsgiref==0.1.2
 launchpadlib==1.10.2
 bzr==2.6.0
-
+ipdb

=== modified file 'tests/contrib/cloudfoundry/test_render_context.py'
--- tests/contrib/cloudfoundry/test_render_context.py	2014-05-13 20:14:01 +0000
+++ tests/contrib/cloudfoundry/test_render_context.py	2014-05-29 17:24:14 +0000
@@ -5,56 +5,61 @@
 from charmhelpers.contrib.cloudfoundry import contexts
 
 
-class TestNatsContext(unittest.TestCase):
+class TestNatsRelation(unittest.TestCase):
 
     @mock.patch('charmhelpers.core.hookenv.relation_ids')
-    def test_nats_context_empty(self, mid):
+    def test_nats_relation_empty(self, mid):
         mid.return_value = None
-        n = contexts.NatsContext()
-        self.assertEqual(n(), {})
+        n = contexts.NatsRelation()
+        self.assertEqual(n, {})
 
     @mock.patch('charmhelpers.core.hookenv.related_units')
     @mock.patch('charmhelpers.core.hookenv.relation_ids')
     @mock.patch('charmhelpers.core.hookenv.relation_get')
-    def test_nats_context_populated(self, mrel, mid, mrelated):
+    def test_nats_relation_populated(self, mrel, mid, mrelated):
         mid.return_value = ['nats']
         mrel.return_value = {'nats_port': 1234, 'nats_address': 'host',
                              'nats_user': 'user', 'nats_password': 'password'}
         mrelated.return_value = ['router/0']
-        n = contexts.NatsContext()
+        n = contexts.NatsRelation()
         expected = {'nats': {'nats_port': 1234, 'nats_address': 'host',
-                             'nats_user': 'user', 'nats_password': 'password'}}
-        self.assertEqual(n(), expected)
+                             'nats_user': 'user', 'nats_password': 'password',
+                             'router/0': {'nats_port': 1234, 'nats_address': 'host',
+                                          'nats_user': 'user', 'nats_password': 'password'}}}
+        self.assertTrue(bool(n))
+        self.assertEqual(n, expected)
 
     @mock.patch('charmhelpers.core.hookenv.related_units')
     @mock.patch('charmhelpers.core.hookenv.relation_ids')
     @mock.patch('charmhelpers.core.hookenv.relation_get')
-    def test_nats_context_partial(self, mrel, mid, mrelated):
+    def test_nats_relation_partial(self, mrel, mid, mrelated):
         mid.return_value = ['nats']
         mrel.return_value = {'nats_address': 'host'}
         mrelated.return_value = ['router/0']
-        n = contexts.NatsContext()
-        self.assertEqual(n(), {})
-
-
-class TestRouterContext(unittest.TestCase):
+        n = contexts.NatsRelation()
+        self.assertEqual(n, {})
+
+
+class TestRouterRelation(unittest.TestCase):
 
     @mock.patch('charmhelpers.core.hookenv.relation_ids')
-    def test_router_context_empty(self, mid):
+    def test_router_relation_empty(self, mid):
         mid.return_value = None
-        n = contexts.RouterContext()
-        self.assertEqual(n(), {})
+        n = contexts.RouterRelation()
+        self.assertEqual(n, {})
 
     @mock.patch('charmhelpers.core.hookenv.related_units')
     @mock.patch('charmhelpers.core.hookenv.relation_ids')
     @mock.patch('charmhelpers.core.hookenv.relation_get')
-    def test_router_context_populated(self, mrel, mid, mrelated):
+    def test_router_relation_populated(self, mrel, mid, mrelated):
         mid.return_value = ['router']
         mrel.return_value = {'domain': 'example.com'}
         mrelated.return_value = ['router/0']
-        n = contexts.RouterContext()
-        expected = {'router': {'domain': 'example.com'}}
-        self.assertEqual(n(), expected)
+        n = contexts.RouterRelation()
+        expected = {'router': {'domain': 'example.com',
+                               'router/0': {'domain': 'example.com'}}}
+        self.assertTrue(bool(n))
+        self.assertEqual(n, expected)
 
 
 class TestStoredContext(unittest.TestCase):
@@ -71,13 +76,13 @@
         os.unlink(file_name)
         contexts.StoredContext(file_name, {'key': 'initial_value'})
         self.assertTrue(os.path.isfile(file_name))
-        context = contexts.StoredContext(file_name, {'key': 'random_value'})()
+        context = contexts.StoredContext(file_name, {'key': 'random_value'})
         self.assertIn('key', context)
         self.assertEqual(context['key'], 'initial_value')
 
     def test_stored_context_raise(self):
         _, file_name = tempfile.mkstemp()
-        with self.assertRaises(OSError) as cm:
+        with self.assertRaises(OSError):
             contexts.StoredContext(file_name, {'key': 'initial_value'})
         os.unlink(file_name)
 

=== modified file 'tests/core/test_host.py'
--- tests/core/test_host.py	2014-05-20 15:40:48 +0000
+++ tests/core/test_host.py	2014-05-29 17:24:14 +0000
@@ -96,6 +96,16 @@
         service.assert_called_with('reload', service_name)
 
     @patch.object(host, 'service')
+    def test_service_available(self, service):
+        service_name = 'foo-service'
+        service.side_effect = [True]
+        self.assertTrue(host.service_available(service_name))
+        service.side_effect = [False]
+        self.assertFalse(host.service_available(service_name))
+
+        service.assert_called_with('status', service_name)
+
+    @patch.object(host, 'service')
     def test_failed_reload_restarts_a_service(self, service):
         service_name = 'foo-service'
         service.side_effect = [False, True]

=== modified file 'tests/core/test_services.py'
--- tests/core/test_services.py	2014-05-16 21:48:06 +0000
+++ tests/core/test_services.py	2014-05-29 17:24:14 +0000
@@ -1,126 +1,406 @@
-import pkg_resources
 import mock
 import unittest
 from charmhelpers.core import services
 
-TEMPLATES_DIR = pkg_resources.resource_filename(__name__, 'templates')
-
-
-def static_content(data):
-    def context():
-        return data
-    return context
-
-
-# Sample render context used to test rendering paths
-default_context = {
-    'nats': {
-        'nats_port': '1234',
-        'nats_host': 'example.com',
-    },
-    'router': {
-        'domain': 'api.foo.com'
-    }
-}
-
-
-# Method returning a mutable copy of a default config
-# for services.register()
-def default_service_config():
-    return [{
-        'service': 'cf-cloudcontroller',
-        'templates': [
-            {'target': 'cc.yml',
-             'source': 'fake_cc.yml',
-             'file_properties': {},
-             'contexts': [static_content(default_context)]
-             }]
-    }]
-
-
-class TestService(unittest.TestCase):
+
+class TestServiceManager(unittest.TestCase):
+    def test_register(self):
+        manager = services.ServiceManager([
+            {'service': 'service1',
+             'foo': 'bar'},
+            {'service': 'service2',
+             'qux': 'baz'},
+        ])
+        self.assertEqual(manager.services, {
+            'service1': {'service': 'service1',
+                         'foo': 'bar'},
+            'service2': {'service': 'service2',
+                         'qux': 'baz'},
+        })
+
+    @mock.patch.object(services.ServiceManager, 'reconfigure_services')
+    @mock.patch.object(services.ServiceManager, 'stop_services')
+    @mock.patch.object(services, 'sys')
+    def test_manage_stop(self, msys, stop_services, reconfigure_services):
+        manager = services.ServiceManager()
+        msys.argv = ['charm_dir/hooks/stop']
+        manager.manage()
+        stop_services.assert_called_once_with()
+        assert not reconfigure_services.called
+
+    @mock.patch.object(services.ServiceManager, 'reconfigure_services')
+    @mock.patch.object(services.ServiceManager, 'stop_services')
+    @mock.patch.object(services, 'sys')
+    def test_manage_other(self, msys, stop_services, reconfigure_services):
+        manager = services.ServiceManager()
+        msys.argv = ['charm_dir/hooks/config-changed']
+        manager.manage()
+        assert not stop_services.called
+        reconfigure_services.assert_called_once_with()
+
+    @mock.patch.object(services.ServiceManager, 'save_ready')
+    @mock.patch.object(services.ServiceManager, 'fire_event')
+    @mock.patch.object(services.ServiceManager, 'is_ready')
+    def test_reconfigure_ready(self, is_ready, fire_event, save_ready):
+        manager = services.ServiceManager([
+            {'service': 'service1'}, {'service': 'service2'}])
+        is_ready.return_value = True
+        manager.reconfigure_services()
+        is_ready.assert_has_calls([
+            mock.call('service1'),
+            mock.call('service2'),
+        ], any_order=True)
+        fire_event.assert_has_calls([
+            mock.call('data_ready', 'service1'),
+            mock.call('start', 'service1', default=[
+                services.host.service_restart,
+                services.open_ports]),
+        ], any_order=False)
+        fire_event.assert_has_calls([
+            mock.call('data_ready', 'service2'),
+            mock.call('start', 'service2', default=[
+                services.host.service_restart,
+                services.open_ports]),
+        ], any_order=False)
+        save_ready.assert_has_calls([
+            mock.call('service1'),
+            mock.call('service2'),
+        ], any_order=True)
+
+    @mock.patch.object(services.ServiceManager, 'save_ready')
+    @mock.patch.object(services.ServiceManager, 'fire_event')
+    @mock.patch.object(services.ServiceManager, 'is_ready')
+    def test_reconfigure_ready_list(self, is_ready, fire_event, save_ready):
+        manager = services.ServiceManager([
+            {'service': 'service1'}, {'service': 'service2'}])
+        is_ready.return_value = True
+        manager.reconfigure_services('service3', 'service4')
+        self.assertEqual(is_ready.call_args_list, [
+            mock.call('service3'),
+            mock.call('service4'),
+        ])
+        self.assertEqual(fire_event.call_args_list, [
+            mock.call('data_ready', 'service3'),
+            mock.call('start', 'service3', default=[
+                services.host.service_restart,
+                services.open_ports]),
+            mock.call('data_ready', 'service4'),
+            mock.call('start', 'service4', default=[
+                services.host.service_restart,
+                services.open_ports]),
+        ])
+        self.assertEqual(save_ready.call_args_list, [
+            mock.call('service3'),
+            mock.call('service4'),
+        ])
+
+    @mock.patch.object(services.ServiceManager, 'save_lost')
+    @mock.patch.object(services.ServiceManager, 'fire_event')
+    @mock.patch.object(services.ServiceManager, 'was_ready')
+    @mock.patch.object(services.ServiceManager, 'is_ready')
+    def test_reconfigure_not_ready(self, is_ready, was_ready, fire_event, save_lost):
+        manager = services.ServiceManager([
+            {'service': 'service1'}, {'service': 'service2'}])
+        is_ready.return_value = False
+        was_ready.return_value = False
+        manager.reconfigure_services()
+        is_ready.assert_has_calls([
+            mock.call('service1'),
+            mock.call('service2'),
+        ], any_order=True)
+        fire_event.assert_has_calls([
+            mock.call('stop', 'service1', default=[
+                services.close_ports,
+                services.host.service_stop]),
+            mock.call('stop', 'service2', default=[
+                services.close_ports,
+                services.host.service_stop]),
+        ], any_order=True)
+        save_lost.assert_has_calls([
+            mock.call('service1'),
+            mock.call('service2'),
+        ], any_order=True)
+
+    @mock.patch.object(services.ServiceManager, 'save_lost')
+    @mock.patch.object(services.ServiceManager, 'fire_event')
+    @mock.patch.object(services.ServiceManager, 'was_ready')
+    @mock.patch.object(services.ServiceManager, 'is_ready')
+    def test_reconfigure_no_longer_ready(self, is_ready, was_ready, fire_event, save_lost):
+        manager = services.ServiceManager([
+            {'service': 'service1'}, {'service': 'service2'}])
+        is_ready.return_value = False
+        was_ready.return_value = True
+        manager.reconfigure_services()
+        is_ready.assert_has_calls([
+            mock.call('service1'),
+            mock.call('service2'),
+        ], any_order=True)
+        fire_event.assert_has_calls([
+            mock.call('data_lost', 'service1'),
+            mock.call('stop', 'service1', default=[
+                services.close_ports,
+                services.host.service_stop]),
+        ], any_order=False)
+        fire_event.assert_has_calls([
+            mock.call('data_lost', 'service2'),
+            mock.call('stop', 'service2', default=[
+                services.close_ports,
+                services.host.service_stop]),
+        ], any_order=False)
+        save_lost.assert_has_calls([
+            mock.call('service1'),
+            mock.call('service2'),
+        ], any_order=True)
+
+    @mock.patch.object(services.ServiceManager, 'fire_event')
+    def test_stop_services(self, fire_event):
+        manager = services.ServiceManager([
+            {'service': 'service1'}, {'service': 'service2'}])
+        manager.stop_services()
+        fire_event.assert_has_calls([
+            mock.call('stop', 'service1', default=[
+                services.close_ports,
+                services.host.service_stop]),
+            mock.call('stop', 'service2', default=[
+                services.close_ports,
+                services.host.service_stop]),
+        ], any_order=True)
+
+    @mock.patch.object(services.ServiceManager, 'fire_event')
+    def test_stop_services_list(self, fire_event):
+        manager = services.ServiceManager([
+            {'service': 'service1'}, {'service': 'service2'}])
+        manager.stop_services('service3', 'service4')
+        self.assertEqual(fire_event.call_args_list, [
+            mock.call('stop', 'service3', default=[
+                services.close_ports,
+                services.host.service_stop]),
+            mock.call('stop', 'service4', default=[
+                services.close_ports,
+                services.host.service_stop]),
+        ])
+
+    def test_get_service(self):
+        service = {'service': 'test', 'test': 'test_service'}
+        manager = services.ServiceManager([service])
+        self.assertEqual(manager.get_service('test'), service)
+
+    def test_get_service_not_registered(self):
+        service = {'service': 'test', 'test': 'test_service'}
+        manager = services.ServiceManager([service])
+        self.assertRaises(KeyError, manager.get_service, 'foo')
+
+    @mock.patch.object(services.ServiceManager, 'get_service')
+    def test_fire_event_default(self, get_service):
+        get_service.return_value = {}
+        cb = mock.Mock()
+        manager = services.ServiceManager()
+        manager.fire_event('event', 'service', cb)
+        cb.assert_called_once_with('service')
+
+    @mock.patch.object(services.ServiceManager, 'get_service')
+    def test_fire_event_default_list(self, get_service):
+        get_service.return_value = {}
+        cb = mock.Mock()
+        manager = services.ServiceManager()
+        manager.fire_event('event', 'service', [cb])
+        cb.assert_called_once_with('service')
+
+    @mock.patch.object(services.ServiceManager, 'get_service')
+    def test_fire_event_simple_callback(self, get_service):
+        cb = mock.Mock()
+        dcb = mock.Mock()
+        get_service.return_value = {'event': cb}
+        manager = services.ServiceManager()
+        manager.fire_event('event', 'service', dcb)
+        assert not dcb.called
+        cb.assert_called_once_with('service')
+
+    @mock.patch.object(services.ServiceManager, 'get_service')
+    def test_fire_event_simple_callback_list(self, get_service):
+        cb = mock.Mock()
+        dcb = mock.Mock()
+        get_service.return_value = {'event': [cb]}
+        manager = services.ServiceManager()
+        manager.fire_event('event', 'service', dcb)
+        assert not dcb.called
+        cb.assert_called_once_with('service')
+
+    @mock.patch.object(services.ManagerCallback, '__call__')
+    @mock.patch.object(services.ServiceManager, 'get_service')
+    def test_fire_event_manager_callback(self, get_service, mcall):
+        cb = services.ManagerCallback()
+        dcb = mock.Mock()
+        get_service.return_value = {'event': cb}
+        manager = services.ServiceManager()
+        manager.fire_event('event', 'service', dcb)
+        assert not dcb.called
+        mcall.assert_called_once_with(manager, 'service', 'event')
+
+    @mock.patch.object(services.ManagerCallback, '__call__')
+    @mock.patch.object(services.ServiceManager, 'get_service')
+    def test_fire_event_manager_callback_list(self, get_service, mcall):
+        cb = services.ManagerCallback()
+        dcb = mock.Mock()
+        get_service.return_value = {'event': [cb]}
+        manager = services.ServiceManager()
+        manager.fire_event('event', 'service', dcb)
+        assert not dcb.called
+        mcall.assert_called_once_with(manager, 'service', 'event')
+
+    @mock.patch.object(services.ServiceManager, 'get_service')
+    def test_is_ready(self, get_service):
+        get_service.side_effect = [
+            {},
+            {'required_data': [True]},
+            {'required_data': [False]},
+            {'required_data': [True, False]},
+        ]
+        manager = services.ServiceManager()
+        assert manager.is_ready('foo')
+        assert manager.is_ready('bar')
+        assert not manager.is_ready('foo')
+        assert not manager.is_ready('foo')
+        get_service.assert_has_calls([mock.call('foo'), mock.call('bar')])
+
+    @mock.patch.object(services.hookenv, 'charm_dir')
+    @mock.patch.object(services, 'open', create=True)
+    def test_save_ready(self, mopen, charm_dir):
+        charm_dir.return_value = 'charm_dir'
+        manager = services.ServiceManager()
+        manager.save_ready('foo')
+        mopen.assert_called_once_with('charm_dir/.ready.foo', 'a')
+
+    @mock.patch.object(services.hookenv, 'charm_dir')
+    @mock.patch('os.remove')
+    @mock.patch('os.path.exists')
+    def test_save_lost(self, exists, remove, charm_dir):
+        charm_dir.return_value = 'charm_dir'
+        manager = services.ServiceManager()
+        manager.save_lost('foo')
+        exists.assert_called_once_with('charm_dir/.ready.foo')
+        remove.assert_called_once_with('charm_dir/.ready.foo')
+
+    @mock.patch.object(services.hookenv, 'charm_dir')
+    @mock.patch('os.path.exists')
+    def test_was_ready(self, exists, charm_dir):
+        charm_dir.return_value = 'charm_dir'
+        manager = services.ServiceManager()
+        manager.was_ready('foo')
+        exists.assert_called_once_with('charm_dir/.ready.foo')
+
+
+class TestRelationContext(unittest.TestCase):
     def setUp(self):
-        services.SERVICES = {}
-
-    @mock.patch('charmhelpers.core.host.service_restart')
-    @mock.patch('charmhelpers.core.services.templating')
-    def test_register_no_target(self, mtemplating, mservice_restart):
-        config = default_service_config()
-        del config[0]['templates'][0]['target']
-        services.register(config, TEMPLATES_DIR)
-        services.reconfigure_services()
-        self.assertEqual(mtemplating.render.call_args[0][0][0]['target'],
-                         '/etc/init/cf-cloudcontroller.conf')
-        self.assertEqual(mtemplating.render.call_args[0][1], TEMPLATES_DIR)
-
-    @mock.patch('charmhelpers.core.host.service_restart')
-    @mock.patch('charmhelpers.core.services.templating')
-    def test_register_default_tmpl_dir(self, mtemplating, mservice_restart):
-        config = default_service_config()
-        services.register(config)
-        services.reconfigure_services()
-        self.assertEqual(mtemplating.render.call_args[0][1], None)
-
-    @mock.patch('charmhelpers.core.services.templating')
-    @mock.patch('charmhelpers.core.services.reconfigure_service')
-    def test_reconfigure_services(self, mreconfig, mtemplating):
-        services.register(default_service_config(), TEMPLATES_DIR)
-        services.reconfigure_services()
-        mreconfig.assert_called_once_with('cf-cloudcontroller', restart=True)
-
-    @mock.patch('charmhelpers.core.services.templating')
-    @mock.patch('charmhelpers.core.host.service_restart')
-    def test_reconfigure_service_restart(self, mrestart, mtemplating):
-        services.register(default_service_config(), TEMPLATES_DIR)
-        services.reconfigure_service('cf-cloudcontroller', restart=True)
-        mrestart.assert_called_once_with('cf-cloudcontroller')
-
-    @mock.patch('charmhelpers.core.services.templating')
-    @mock.patch('charmhelpers.core.host.service_restart')
-    def test_reconfigure_service_no_restart(self, mrestart, mtemplating):
-        services.register(default_service_config(), TEMPLATES_DIR)
-        services.reconfigure_service('cf-cloudcontroller', restart=False)
-        self.assertFalse(mrestart.called)
-
-    @mock.patch('charmhelpers.core.services.templating')
-    @mock.patch('charmhelpers.core.host.service_restart')
-    def test_reconfigure_service_incomplete(self, mrestart, mtemplating):
-        config = default_service_config()
-        mtemplating.render.return_value = False  # render fails when incomplete
-        services.register(config, TEMPLATES_DIR)
-        services.reconfigure_service('cf-cloudcontroller', restart=True)
-        # verify that we did not restart the service
-        self.assertFalse(mrestart.called)
-
-    @mock.patch('charmhelpers.core.services.templating')
-    @mock.patch('charmhelpers.core.host.service_restart')
-    def test_reconfigure_service_no_context(self, mrestart, mtemplating):
-        config = default_service_config()
-        services.register(config, 'foo')
-        services.reconfigure_service('cf-cloudcontroller', restart=False)
-        # verify that we called render template with the expected name
-        mtemplating.render.assert_called_once_with(config[0]['templates'], 'foo')
-        self.assertFalse(mrestart.called)
-        self.assertRaises(KeyError, services.reconfigure_service, 'unknownservice', restart=False)
-
-    @mock.patch('charmhelpers.core.services.templating')
-    @mock.patch('charmhelpers.core.host.service_restart')
-    def test_custom_service_type(self, mrestart, mtemplating):
-        config = default_service_config()
-        config[0]['type'] = mock.Mock(name='CustomService')
-        services.register(config, 'foo')
-        services.reconfigure_service('cf-cloudcontroller', restart=False)
-        config[0]['type'].assert_called_once_with(config[0])
-        config[0]['type'].return_value.reconfigure.assert_called_once_with(False)
-
-    @mock.patch('charmhelpers.core.services.templating')
-    @mock.patch('charmhelpers.core.host.service_stop')
-    @mock.patch('charmhelpers.core.host.service_running')
-    def test_stop_services(self, mrunning, mstop, mtemplating):
-        services.register(default_service_config(), TEMPLATES_DIR)
-        services.stop_services()
-        mrunning.assert_called_once_with('cf-cloudcontroller')
-        mstop.assert_called_once_with('cf-cloudcontroller')
+        self.context = services.RelationContext()
+        self.context.interface = 'http'
+        self.context.required_keys = ['foo', 'bar']
+
+    @mock.patch.object(services, 'hookenv')
+    def test_no_relations(self, mhookenv):
+        mhookenv.relation_ids.return_value = []
+        self.context.get_data()
+        self.assertFalse(self.context.is_ready())
+        self.assertEqual(self.context, {})
+        mhookenv.relation_ids.assert_called_once_with('http')
+
+    @mock.patch.object(services, 'hookenv')
+    def test_no_units(self, mhookenv):
+        mhookenv.relation_ids.return_value = ['nginx']
+        mhookenv.related_units.return_value = []
+        self.context.get_data()
+        self.assertFalse(self.context.is_ready())
+        self.assertEqual(self.context, {'http': {}})
+
+    @mock.patch.object(services, 'hookenv')
+    def test_incomplete(self, mhookenv):
+        mhookenv.relation_ids.return_value = ['nginx', 'apache']
+        mhookenv.related_units.side_effect = lambda i: [i+'/0']
+        mhookenv.relation_get.side_effect = [{}, {'foo': '1'}]
+        self.assertFalse(bool(self.context))
+        self.assertEqual(mhookenv.relation_get.call_args_list, [
+            mock.call(rid='nginx', unit='nginx/0'),
+            mock.call(rid='apache', unit='apache/0'),
+        ])
+
+    @mock.patch.object(services, 'hookenv')
+    def test_complete(self, mhookenv):
+        mhookenv.relation_ids.return_value = ['nginx', 'apache', 'tomcat']
+        mhookenv.related_units.side_effect = lambda i: [i+'/0']
+        mhookenv.relation_get.side_effect = [{'foo': '1'}, {'foo': '2', 'bar': '3'}, {}]
+        self.context.get_data()
+        self.assertEqual(self.context, {'http': {
+            'foo': '2',
+            'bar': '3',
+            'nginx/0': {
+                'foo': '1',
+            },
+            'apache/0': {
+                'foo': '2',
+                'bar': '3',
+            },
+            'tomcat/0': {
+            },
+        }})
+        mhookenv.relation_ids.assert_called_with('http')
+        self.assertEqual(mhookenv.relation_get.call_args_list, [
+            mock.call(rid='nginx', unit='nginx/0'),
+            mock.call(rid='apache', unit='apache/0'),
+            mock.call(rid='tomcat', unit='tomcat/0'),
+        ])
+
+
+class TestTemplateCallback(unittest.TestCase):
+    @mock.patch.object(services, 'templating')
+    def test_template_defaults(self, mtemplating):
+        manager = mock.Mock(**{'get_service.return_value': {
+            'required_data': [{'foo': 'bar'}]}})
+        self.assertRaises(TypeError, services.template, source='foo.yml')
+        callback = services.template(source='foo.yml', target='bar.yml')
+        assert isinstance(callback, services.ManagerCallback)
+        assert not mtemplating.render.called
+        callback(manager, 'test', 'event')
+        mtemplating.render.assert_called_once_with(
+            'foo.yml', 'bar.yml', {'foo': 'bar'},
+            'root', 'root', 0444)
+
+    @mock.patch.object(services, 'templating')
+    def test_template_explicit(self, mtemplating):
+        manager = mock.Mock(**{'get_service.return_value': {
+            'required_data': [{'foo': 'bar'}]}})
+        callback = services.template(
+            source='foo.yml', target='bar.yml',
+            owner='user', group='group', perms=0555
+        )
+        assert isinstance(callback, services.ManagerCallback)
+        assert not mtemplating.render.called
+        callback(manager, 'test', 'event')
+        mtemplating.render.assert_called_once_with(
+            'foo.yml', 'bar.yml', {'foo': 'bar'},
+            'user', 'group', 0555)
+
+
+class TestPortsCallback(unittest.TestCase):
+    @mock.patch.object(services, 'hookenv')
+    def test_no_ports(self, hookenv):
+        manager = mock.Mock(**{'get_service.return_value': {}})
+        services.PortManagerCallback()(manager, 'service', 'event')
+        assert not hookenv.open_port.called
+        assert not hookenv.close_port.called
+
+    @mock.patch.object(services, 'hookenv')
+    def test_open_ports(self, hookenv):
+        manager = mock.Mock(**{'get_service.return_value': {'ports': [1, 2]}})
+        services.open_ports(manager, 'service', 'start')
+        hookenv.open_port.has_calls([mock.call(1), mock.call(2)])
+        assert not hookenv.close_port.called
+
+    @mock.patch.object(services, 'hookenv')
+    def test_close_ports(self, hookenv):
+        manager = mock.Mock(**{'get_service.return_value': {'ports': [1, 2]}})
+        services.close_ports(manager, 'service', 'stop')
+        assert not hookenv.open_port.called
+        hookenv.close_port.has_calls([mock.call(1), mock.call(2)])
 
 if __name__ == '__main__':
     unittest.main()

=== modified file 'tests/core/test_templating.py'
--- tests/core/test_templating.py	2014-05-16 21:48:06 +0000
+++ tests/core/test_templating.py	2014-05-29 17:24:14 +0000
@@ -1,7 +1,7 @@
-import os
 import pkg_resources
 import tempfile
 import unittest
+import jinja2
 
 import mock
 from charmhelpers.core import templating
@@ -10,30 +10,6 @@
 TEMPLATES_DIR = pkg_resources.resource_filename(__name__, 'templates')
 
 
-def noop():
-    return {}
-
-
-def found():
-    return {'foo': 'bar'}
-
-
-DEFAULT_CONTEXT = {
-    'nats': {
-        'nats_port': '1234',
-        'nats_host': 'example.com',
-    },
-    'router': {
-        'domain': 'api.foo.com'
-    },
-    'nginx_port': 80,
-}
-
-
-def default_context_provider():
-    return DEFAULT_CONTEXT
-
-
 class TestTemplating(unittest.TestCase):
     def setUp(self):
         self.charm_dir = pkg_resources.resource_filename(__name__, '')
@@ -50,164 +26,33 @@
     def test_render(self, log, mkdir, fchown):
         _, fn1 = tempfile.mkstemp()
         _, fn2 = tempfile.mkstemp()
-        template_definitions = [
-            {'source': 'fake_cc.yml',
-             'target': fn1,
-             'contexts': [default_context_provider]},
-            {'source': 'test.conf',
-             'target': fn2,
-             'contexts': [default_context_provider]},
-        ]
-        self.assertTrue(templating.render(template_definitions))
+        context = {
+            'nats': {
+                'nats_port': '1234',
+                'nats_host': 'example.com',
+            },
+            'router': {
+                'domain': 'api.foo.com'
+            },
+            'nginx_port': 80,
+        }
+        templating.render('fake_cc.yml', fn1, context, templates_dir=TEMPLATES_DIR)
         contents = open(fn1).read()
         self.assertRegexpMatches(contents, 'port: 1234')
         self.assertRegexpMatches(contents, 'host: example.com')
         self.assertRegexpMatches(contents, 'domain: api.foo.com')
+
+        templating.render('test.conf', fn2, context, templates_dir=TEMPLATES_DIR)
         contents = open(fn2).read()
         self.assertRegexpMatches(contents, 'listen 80')
         self.assertEqual(fchown.call_count, 2)
         self.assertEqual(mkdir.call_count, 2)
 
-    @mock.patch.object(templating.host.os, 'fchown')
-    @mock.patch.object(templating.host, 'mkdir')
-    @mock.patch.object(templating.host, 'log')
-    def test_render_incomplete(self, log, mkdir, fchown):
-        _, fn1 = tempfile.mkstemp()
-        _, fn2 = tempfile.mkstemp()
-        os.remove(fn1)
-        os.remove(fn2)
-        template_definitions = [
-            {'source': 'fake_cc.yml',
-             'target': fn1,
-             'contexts': [lambda: {}]},
-            {'source': 'test.conf',
-             'target': fn2,
-             'contexts': [default_context_provider]},
-        ]
-        self.assertFalse(templating.render(template_definitions))
-        self.assertFalse(os.path.exists(fn1))
-        contents = open(fn2).read()
-        self.assertRegexpMatches(contents, 'listen 80')
-        self.assertEqual(fchown.call_count, 1)
-        self.assertEqual(mkdir.call_count, 1)
-
-    @mock.patch('jinja2.Environment')
-    @mock.patch.object(templating.host, 'mkdir')
-    @mock.patch.object(templating.host, 'write_file')
-    def test_render_no_source(self, write_file, mkdir, Env):
-        template_definitions = [{
-            'target': 'fake_cc.yml',
-            'contexts': [default_context_provider],
-        }]
-        self.assertTrue(templating.render(template_definitions))
-        Env().get_template.assert_called_once_with('fake_cc.yml.j2')
-        Env().get_template.return_value.render.assert_called_once()
-        write_file.assert_called_once()
-
-    @mock.patch('jinja2.Environment')
-    @mock.patch.object(templating.host, 'mkdir')
-    @mock.patch.object(templating.host, 'write_file')
-    def test_render_no_contexts(self, write_file, mkdir, Env):
-        template_definitions = [{
-            'target': 'fake_cc.yml',
-            'contexts': [],
-        }]
-        self.assertTrue(templating.render(template_definitions))
-        Env().get_template.assert_called_once_with('fake_cc.yml.j2')
-        Env().get_template.return_value.render.assert_called_once_with({})
-        write_file.assert_called_once()
-
-    @mock.patch('jinja2.FileSystemLoader')
-    @mock.patch('jinja2.Environment')
-    @mock.patch.object(templating.host, 'mkdir')
-    @mock.patch.object(templating.host, 'write_file')
-    def test_render_implicit_dir(self, write_file, mkdir, Env, FSL):
-        self.charm_dir = 'foo'
-        template_definitions = [{
-            'target': 'fake_cc.yml',
-            'contexts': [default_context_provider],
-        }]
-        self.assertTrue(templating.render(template_definitions))
-        FSL.assert_called_once_with('foo/templates')
-
-    @mock.patch('jinja2.FileSystemLoader')
-    @mock.patch('jinja2.Environment')
-    @mock.patch.object(templating.host, 'mkdir')
-    @mock.patch.object(templating.host, 'write_file')
-    def test_render_explicit_dir(self, write_file, mkdir, Env, FSL):
-        self.charm_dir = 'foo'
-        template_definitions = [{
-            'target': 'fake_cc.yml',
-            'contexts': [default_context_provider],
-        }]
-        self.assertTrue(templating.render(template_definitions, 'bar'))
-        FSL.assert_called_once_with('bar')
-
-    def test_collect_contexts_fail(self):
-        cc = templating._collect_contexts
-        self.assertEqual(cc([]), {})
-        self.assertEqual(cc([noop]), False)
-        self.assertEqual(cc([noop, noop]), False)
-        self.assertEqual(cc([noop, found]), False)
-
-    def test_collect_contexts_found(self):
-        cc = templating._collect_contexts
-        expected = {'foo': 'bar'}
-        self.assertEqual(cc([found]), expected)
-        self.assertEqual(cc([found, found]), expected)
-
-
-class TestConfigContext(unittest.TestCase):
-    @mock.patch('charmhelpers.core.hookenv.config')
-    def test_config_context(self, mconfig):
-        templating.ConfigContext()()
-        self.assertTrue(mconfig.called)
-
-
-class TestStaticContext(unittest.TestCase):
-    def test_static_context(self):
-        a = templating.StaticContext('a')
-        self.assertEqual(a.data, 'a')
-        self.assertEqual(a(), 'a')
-
-
-class TestRelationContext(unittest.TestCase):
-    def setUp(self):
-        self.context_provider = templating.RelationContext()
-        self.context_provider.interface = 'http'
-        self.context_provider.required_keys = ['foo', 'bar']
-
-    @mock.patch.object(templating, 'hookenv')
-    def test_no_relations(self, mhookenv):
-        mhookenv.relation_ids.return_value = []
-        self.assertEqual(self.context_provider(), {})
-        mhookenv.relation_ids.assert_called_once_with('http')
-
-    @mock.patch.object(templating, 'hookenv')
-    def test_no_units(self, mhookenv):
-        mhookenv.relation_ids.return_value = ['nginx']
-        mhookenv.related_units.return_value = []
-        self.assertEqual(self.context_provider(), {})
-
-    @mock.patch.object(templating, 'hookenv')
-    def test_incomplete(self, mhookenv):
-        mhookenv.relation_ids.return_value = ['nginx', 'apache']
-        mhookenv.related_units.side_effect = lambda i: [i+'/0']
-        mhookenv.relation_get.side_effect = [{}, {'foo': '1'}]
-        self.assertEqual(self.context_provider(), {})
-        self.assertEqual(mhookenv.relation_get.call_args_list, [
-            mock.call(rid='nginx', unit='nginx/0'),
-            mock.call(rid='apache', unit='apache/0'),
-        ])
-
-    @mock.patch.object(templating, 'hookenv')
-    def test_complete(self, mhookenv):
-        mhookenv.relation_ids.return_value = ['nginx', 'apache', 'tomcat']
-        mhookenv.related_units.side_effect = lambda i: [i+'/0']
-        mhookenv.relation_get.side_effect = [{'foo': '1'}, {'foo': '2', 'bar': '3'}, {}]
-        self.assertEqual(self.context_provider(), {'http': {'foo': '2', 'bar': '3'}})
-        mhookenv.relation_ids.assert_called_with('http')
-        self.assertEqual(mhookenv.relation_get.call_args_list, [
-            mock.call(rid='nginx', unit='nginx/0'),
-            mock.call(rid='apache', unit='apache/0'),
-        ])
+    @mock.patch.object(templating, 'hookenv')
+    @mock.patch('jinja2.Environment')
+    def test_load_error(self, Env, hookenv):
+        Env().get_template.side_effect = jinja2.exceptions.TemplateNotFound('fake_cc.yml')
+        self.assertRaises(
+            jinja2.exceptions.TemplateNotFound, templating.render,
+            'fake.src', 'fake.tgt', {}, templates_dir='tmpl')
+        hookenv.log.assert_called_once_with('Could not load template fake.src from tmpl.', level=hookenv.ERROR)

=== modified file 'tox.ini'
--- tox.ini	2014-05-09 16:10:54 +0000
+++ tox.ini	2014-05-29 17:24:14 +0000
@@ -2,13 +2,13 @@
 envlist = py27
 
 [testenv]
-install_command=pip install --pre {opts}  
-   --allow-all-external 
-   --allow-unverified launchpadlib 
-   --allow-unverified python-apt 
-   --allow-unverified bzr 
+install_command=pip install --pre {opts}
+   --allow-all-external
+   --allow-unverified launchpadlib
+   --allow-unverified python-apt
+   --allow-unverified bzr
    --allow-unverified lazr.authentication
-   {packages} 
+   {packages}
 deps=-r{toxinidir}/test-requirements-tox.txt
 commands =
-         nosetests --nologcapture 
+         nosetests --nologcapture {posargs}