cf-charmers team mailing list archive

[Cory Johns <cory.johns@xxxxxxxxxxxxx>]

Cory Johns has proposed merging lp:~johnsca/charms/trusty/cf-cloud-controller/services-callback-fu into lp:~cf-charmers/charms/trusty/cf-cloud-controller/trunk.

Requested reviews:
  Cloud Foundry Charmers (cf-charmers)

For more details, see:
https://code.launchpad.net/~johnsca/charms/trusty/cf-cloud-controller/services-callback-fu/+merge/220741

Fixed relation ordering issue

Fixed an relation hook ordering dependency by refactoring to use
callbacks in charmhelpers.core.services

https://codereview.appspot.com/100680044/

-- 
https://code.launchpad.net/~johnsca/charms/trusty/cf-cloud-controller/services-callback-fu/+merge/220741
Your team Cloud Foundry Charmers is requested to review the proposed merge of lp:~johnsca/charms/trusty/cf-cloud-controller/services-callback-fu into lp:~cf-charmers/charms/trusty/cf-cloud-controller/trunk.

=== modified file 'hooks/charmhelpers/core/host.py'
--- hooks/charmhelpers/core/host.py	2014-05-20 19:43:29 +0000
+++ hooks/charmhelpers/core/host.py	2014-05-23 01:34:49 +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:44:17 +0000
+++ hooks/charmhelpers/core/services.py	2014-05-23 01:34:49 +0000
@@ -7,78 +7,104 @@
 
 def register(services, templates_dir=None):
     """
-    Register a list of service configs.
+    Register a list of services, given their definitions.
 
-    Service Configs are dicts in the following formats:
+    Service definitions are dicts in the following formats:
 
         {
             "service": <service name>,
             "templates": [ {
+                'source': <source template to render>,
                 '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``.
+                'file_properties': <dict of file properties, see host.write_file>,
+                'contexts': <list of context generators>,
+            } ],
+            'complete': <list of actions to call when all templates are complete>,
+            'stop': <list of actions to call when stopping the service>,
+        }
+
+    If 'source' is omitted, it will default to ``basename(target).j2``.
+
+    If 'target' is omitted, it will default to ``/etc/init/<service name>.conf``
+    (i.e., an Upstart service of the given name).
+
+    If 'complete' is omitted, it defaults to `[host.service_restart]` (i.e.,
+    restarting the service via Upstart).
+
+    If 'stop' is ommitted, it defaults to `[host.service_stop]` (i.e., stopping
+    the service via Upstart).
+
+    The `templates_dir` param indicates where to find template source files, and
+    defaults to `$CHARM_DIR/templates`.
+
+
+    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 Monit watcher to monitor the bingod service.
+
+        >>> services.register([
+        ...     {
+        ...         'service': 'bingod',
+        ...         'templates': [
+        ...             {'source': 'bingod.conf.j2'},
+        ...             {'target': '/etc/bingod.ini',
+        ...              'file_properties': {'owner': 'bingo', 'perms': 0400},
+        ...              'contexts': [MongoRelationContext()]},
+        ...         ],
+        ...     },
+        ...     {
+        ...         'service': 'referee',
+        ...         'templates': [{'target': '/etc/monit/conf.d/referee.cfg'}],
+        ...         'complete': [monit_reload],
+        ...         'stop': [],
+        ...     },
+        ... ])
     """
     for service in services:
+        service_name = service['service']
         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.
+        for template in service['templates']:
+            template.setdefault('target', '/etc/init/{}.conf'.format(service_name))
+        SERVICES[service_name] = service
+
+
+def reconfigure_services(*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 SERVICES.keys():
+        service = _get_service(service_name)
+        complete = templating.render(service['templates'], service['templates_dir'])
+        if complete:
+            for action in service.get('complete', [host.service_restart]):
+                action(service_name)
+
+
+def stop_services(*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 SERVICES.keys():
+        service = _get_service(service_name)
+        complete = templating.render(service['templates'], service['templates_dir'])
+        if complete:
+            for action in service.get('stop', [host.service_stop]):
+                action(service_name)
+
+
+def _get_service(service_name):
+    """
+    Given the name of a registered service, return a ServiceManager
+    instance for the type of that service.
     """
     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):
-        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)
+    return service

=== modified file 'hooks/charmhelpers/core/templating.py'
--- hooks/charmhelpers/core/templating.py	2014-05-20 19:43:29 +0000
+++ hooks/charmhelpers/core/templating.py	2014-05-23 01:34:49 +0000
@@ -98,7 +98,12 @@
 
     If any of the contexts are incomplete (i.e., they return an empty dict),
     the template is considered incomplete and will not render.
+
+    If there are no context_providers given, then it is automatically
+    considered complete and an empty context (`{}`) is returned.
     """
+    if not context_providers:
+        return {}  # special case: no contexts at all is always complete
     ctx = {}
     for provider in context_providers:
         c = provider()
@@ -132,6 +137,9 @@
     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.
+
+    Note: If a template has no contexts listed, it is automatically considered
+    complete.
     """
     # lazy import jinja2 in case templating is needed in install hook
     from jinja2 import FileSystemLoader, Environment, exceptions

=== modified file 'hooks/hooks.py'
--- hooks/hooks.py	2014-05-20 19:50:35 +0000
+++ hooks/hooks.py	2014-05-23 01:34:49 +0000
@@ -13,46 +13,36 @@
 import config
 
 hooks = hookenv.Hooks()
-fileproperties = {'owner': 'vcap'}
-
-services.register([
-    {
-        'service': 'cf-cloudcontroller',
-        'templates': [
-            {'source': 'cf-cloudcontroller.conf'},
-            {'source': 'cloud_controller.yml',
-             'target': config.CC_CONFIG_FILE,
-             'file_properties': fileproperties,
-             'contexts': [contexts.NatsContext(),
-                          contexts.RouterContext(),
-                          contexts.MysqlDSNContext()]}
-        ],
-    },
-    {
-        'service': 'cf-cloudcontroller-job',
-        'templates': [{'source': 'cf-cloudcontroller-job.conf'}],
-    },
-])
-
-
-@hooks.hook('upgrade-charm')
-def upgrade_charm():
-    pass
-
-
-@hooks.hook("config-changed")
-def config_changed():
-    services.reconfigure_services()
-
-
-@hooks.hook()
-def stop():
-    services.stop_services()
-
-
-@hooks.hook('db-relation-changed')
-def db_relation_changed():
-    services.reconfigure_services()
+
+
+def register():
+    services.register([
+        {
+            'service': 'cf-cloudcontroller',
+            'templates': [
+                {'source': 'cf-cloudcontroller.conf'},
+                {'source': 'cloud_controller.yml',
+                 'target': config.CC_CONFIG_FILE,
+                 'file_properties': {'owner': 'vcap'},
+                 'contexts': [contexts.NatsContext(),
+                              contexts.RouterContext(),
+                              contexts.MysqlDSNContext()]}
+            ],
+            'complete': [db_migrate, host.service_restart],
+        },
+        {
+            'service': 'cf-cloudcontroller-job',
+            'templates': [{'source': 'cf-cloudcontroller-job.conf'}],
+        },
+    ])
+    hooks.hook('config-changed',
+               'db-relation-changed',
+               'nats-relation-changed',
+               'router-relation-changed')(services.reconfigure_services)
+    hooks.hook('stop')(services.stop_services)
+
+
+def db_migrate(service_name):
     hookenv.log("Starting db:migrate...", hookenv.DEBUG)
     with host.chdir(config.CC_DIR) as dir:
         subprocess.check_call([
@@ -62,17 +52,8 @@
         hookenv.log("Finished db:migrate in %s." % (dir))
 
 
-@hooks.hook('nats-relation-changed')
-def nats_relation_changed():
-    services.reconfigure_services()
-
-
-@hooks.hook('router-relation-changed')
-def router_relation_changed():
-    services.reconfigure_services()
-
-
 if __name__ == '__main__':
+    register()
     hook_name = os.path.basename(sys.argv[0])
     log("Running {} hook".format(hook_name))
     if hookenv.relation_id():