Merge lp:~johnsca/charms/trusty/cf-go-router/services-callback-fu into lp:~cf-charmers/charms/trusty/cf-go-router/trunk

[Cory Johns]

Refactor to callback services API

https://codereview.appspot.com/104720044/

[Cory Johns (johnsca)]

Reviewers: mp+221443_code.launchpad.net,

Message:
Please take a look.

Description:
Refactor to callback services API

https://code.launchpad.net/~johnsca/charms/trusty/cf-go-router/services-callback-fu/+merge/221443

(do not edit description out of merge proposal)

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

Affected files (+480, -355 lines):
   A [revision details]
   M config.yaml
   M hooks/charmhelpers/contrib/cloudfoundry/contexts.py
   M hooks/charmhelpers/core/host.py
   M hooks/charmhelpers/core/services.py
   M hooks/charmhelpers/core/templating.py
   M hooks/config.py
   D hooks/hooks.py

[Benjamin Saller (bcsaller)]

LGTM, thanks

https://codereview.appspot.com/104720044/diff/1/hooks/config.py
File hooks/config.py (right):

https://codereview.appspot.com/104720044/diff/1/hooks/config.py#newcode23
hooks/config.py:23: result = subprocess.check_output(['dig', '+short',
'@8.8.8.8', address])
thank you google, there should be a better way, but I've done this too

https://codereview.appspot.com/104720044/diff/1/hooks/config.py#newcode38
hooks/config.py:38: 'ports': [80, 443],
we still expose 443 but its aspirational iirc

https://codereview.appspot.com/104720044/

[Cory Johns (johnsca)]

On 2014/05/29 18:51:20, benjamin.saller wrote:
> hooks/config.py:23: result = subprocess.check_output(['dig', '+short',
> mailto:, address])
> thank you google, there should be a better way, but I've done this too

> hooks/config.py:38: 'ports': [80, 443],
> we still expose 443 but its aspirational iirc

Yeah, I just moved the existing logic, but it did seem like a strange
approach to me. I expected juju to provide a way to fetch the IP
address instead of the resolved name.

https://codereview.appspot.com/104720044/

[Cory Johns (johnsca)]

*** Submitted:

Refactor to callback services API

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

https://codereview.appspot.com/104720044/

=== modified file 'config.yaml'
--- config.yaml	2014-05-12 07:19:21 +0000
+++ config.yaml	2014-05-29 18:28:48 +0000
@@ -1,8 +1,11 @@
 options:
   domain:
     type: string
-    description: "Router domain name. Currently we use xip.io service to resolve local IP's."
-    default: ''
+    description: |
+        Router domain name. If the special (default) value of 'xip.io' is used,
+        the public IP address will be prepended to it (so that it becomes, for
+        example, '127.0.0.1.xip.io').  Any other value will be used as-is.
+    default: 'xip.io'
   source:
     type: string
     default: 'ppa:cf-charm/ppa'

=== modified file 'hooks/charmhelpers/contrib/cloudfoundry/contexts.py'
--- hooks/charmhelpers/contrib/cloudfoundry/contexts.py	2014-05-20 20:06:35 +0000
+++ hooks/charmhelpers/contrib/cloudfoundry/contexts.py	2014-05-29 18:28:48 +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 'hooks/charmhelpers/core/host.py'
--- hooks/charmhelpers/core/host.py	2014-05-20 20:06:35 +0000
+++ hooks/charmhelpers/core/host.py	2014-05-29 18:28:48 +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 'hooks/charmhelpers/core/services.py'
--- hooks/charmhelpers/core/services.py	2014-05-16 22:36:50 +0000
+++ hooks/charmhelpers/core/services.py	2014-05-29 18:28:48 +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 'hooks/charmhelpers/core/templating.py'
--- hooks/charmhelpers/core/templating.py	2014-05-20 20:06:35 +0000
+++ hooks/charmhelpers/core/templating.py	2014-05-29 18:28:48 +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 symlink 'hooks/config-changed' (properties changed: -x to +x)
=== target was u'hooks.py'
--- hooks/config-changed	1970-01-01 00:00:00 +0000
+++ hooks/config-changed	2014-05-29 18:28:48 +0000
@@ -0,0 +1,11 @@
+#!/usr/bin/env python
+from charmhelpers.core import hookenv
+from charmhelpers.core import services
+import config
+
+domain = config.domain()
+for rid in hookenv.relation_ids('router'):
+    hookenv.relation_set(rid, {'domain': domain})
+
+manager = services.ServiceManager(config.SERVICES)
+manager.manage()

=== modified file 'hooks/config.py'
--- hooks/config.py	2014-04-02 01:33:00 +0000
+++ hooks/config.py	2014-05-29 18:28:48 +0000
@@ -1,4 +1,10 @@
 import os
+import re
+import subprocess
+from charmhelpers.core import hookenv
+from charmhelpers.core import services
+from charmhelpers.contrib.cloudfoundry import contexts
+
 
 __all__ = ['CF_DIR', 'ROUTER_DIR', 'ROUTER_CONFIG_DIR', 'ROUTER_CONFIG_FILE',
            'ROUTER_PACKAGES']
@@ -8,3 +14,38 @@
 ROUTER_CONFIG_DIR = os.path.join(ROUTER_DIR, 'config')
 ROUTER_CONFIG_FILE = os.path.join(ROUTER_CONFIG_DIR, 'gorouter.yml')
 ROUTER_PACKAGES = ['cfgorouter', 'python-jinja2']
+
+
+def to_ip(address):
+    if re.match('^(\d{1,3}\.){3}\d{1,3}$', address):
+        return address  # already an IP
+    else:
+        result = subprocess.check_output(['dig', '+short', '@8.8.8.8', address])
+        return result.strip()
+
+
+def domain():
+    domain = hookenv.config()['domain']
+    if domain == 'xip.io':
+        public_address = hookenv.unit_get('public-address')
+        domain = "%s.xip.io" % to_ip(public_address)
+    return domain
+
+
+SERVICES = [
+    {
+        'service': 'gorouter',
+        'ports': [80, 443],
+        'required_data': [
+            {'domain': domain()},
+            contexts.NatsRelation(),
+        ],
+        'data_ready': [
+            services.template(source='gorouter.conf',
+                              target='/etc/init/gorouter.conf'),
+            services.template(source='gorouter.yml',
+                              target=ROUTER_CONFIG_FILE,
+                              owner='vcap', perms=0644),
+        ],
+    }
+]

=== removed file 'hooks/hooks.py'
--- hooks/hooks.py	2014-05-20 21:54:24 +0000
+++ hooks/hooks.py	1970-01-01 00:00:00 +0000
@@ -1,93 +0,0 @@
-#!/usr/bin/env python
-# vim: et sta sts ai ts=4 sw=4:
-import os
-import subprocess
-import sys
-import re
-
-from charmhelpers.core import hookenv
-from charmhelpers.core.hookenv import log
-from charmhelpers.core import services
-from charmhelpers.core import templating
-from charmhelpers.contrib.cloudfoundry import contexts
-import config
-
-
-def default_domain():
-    public_address = hookenv.unit_get('public-address')
-    if not re.match('^(\d{1,3}\.){3}\d{1,3}$', public_address):
-        result = subprocess.check_output([
-            'dig', '+short', '@8.8.8.8', public_address])
-        public_address = result.strip()
-    domain = "%s.xip.io" % public_address
-    return domain
-
-
-def get_domain():
-    config = hookenv.config()
-    domain = config.get('domain')
-    if not domain:
-        domain = default_domain()
-    return domain
-
-
-hooks = hookenv.Hooks()
-fileproperties = {'owner': 'vcap', 'perms': 0644}
-
-services.register([
-    {
-        'service': 'gorouter',
-        'templates': [
-            {'source': 'gorouter.conf'},
-            {'source': 'gorouter.yml',
-             'target': config.ROUTER_CONFIG_FILE,
-             'file_properties': fileproperties,
-             'contexts': [
-                 templating.StaticContext({'domain': default_domain()}),
-                 templating.ConfigContext(),
-                 contexts.NatsContext()
-             ]},
-        ]
-    }
-])
-
-
[e-mail]()
-def start():
-    hookenv.open_port(80)
-    hookenv.open_port(443)
-
-
[e-mail]("config-changed")
-def config_changed():
-    services.reconfigure_services()
-    domain = get_domain()
-    for rid in hookenv.relation_ids('router'):
-        hookenv.relation_set(rid, {'domain': domain})
-
-
[e-mail]()
-def stop():
-    services.stop_services()
-    hookenv.close_port(80)
-    hookenv.close_port(443)
-
-
[e-mail]('nats-relation-changed')
-def nats_relation_changed():
-    services.reconfigure_services()
-
-
[e-mail]('router-relation-joined')
-def router_relation_joined():
-    domain = get_domain()
-    hookenv.relation_set(None, {'domain': domain})
-
-
-if __name__ == '__main__':
-    hook_name = os.path.basename(sys.argv[0])
-    log("Running {} hook".format(hook_name))
-    if hookenv.relation_id():
-        log("Relation {} with {}".format(
-            hookenv.relation_id(), hookenv.remote_unit()))
-    hooks.execute(sys.argv)

=== modified symlink 'hooks/nats-relation-changed' (properties changed: -x to +x)
=== target was u'hooks.py'
--- hooks/nats-relation-changed	1970-01-01 00:00:00 +0000
+++ hooks/nats-relation-changed	2014-05-29 18:28:48 +0000
@@ -0,0 +1,6 @@
+#!/usr/bin/env python
+from charmhelpers.core import services
+import config
+
+manager = services.ServiceManager(config.SERVICES)
+manager.manage()

=== modified symlink 'hooks/router-relation-joined' (properties changed: -x to +x)
=== target was u'hooks.py'
--- hooks/router-relation-joined	1970-01-01 00:00:00 +0000
+++ hooks/router-relation-joined	2014-05-29 18:28:48 +0000
@@ -0,0 +1,5 @@
+#!/usr/bin/env python
+from charmhelpers.core import hookenv
+import config
+
+hookenv.relation_set(None, {'domain': config.domain()})

=== removed symlink 'hooks/start'
=== target was u'hooks.py'
=== modified symlink 'hooks/stop' (properties changed: -x to +x)
=== target was u'hooks.py'
--- hooks/stop	1970-01-01 00:00:00 +0000
+++ hooks/stop	2014-05-29 18:28:48 +0000
@@ -0,0 +1,6 @@
+#!/usr/bin/env python
+from charmhelpers.core import services
+import config
+
+manager = services.ServiceManager(config.SERVICES)
+manager.manage()