← Back to team overview

cf-charmers team mailing list archive

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

 

Cory Johns has proposed merging lp:~johnsca/charm-helpers/services-callback-fu into lp:~cf-charmers/charm-helpers/cloud-foundry.

Requested reviews:
  Cloud Foundry Charmers (cf-charmers)

For more details, see:
https://code.launchpad.net/~johnsca/charm-helpers/services-callback-fu/+merge/220733

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/

-- 
https://code.launchpad.net/~johnsca/charm-helpers/services-callback-fu/+merge/220733
Your team Cloud Foundry Charmers is requested to review the proposed merge of lp:~johnsca/charm-helpers/services-callback-fu into lp:~cf-charmers/charm-helpers/cloud-foundry.
=== modified file 'charmhelpers/core/host.py'
--- charmhelpers/core/host.py	2014-05-20 15:40:48 +0000
+++ charmhelpers/core/host.py	2014-05-22 22:53:53 +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-22 22:53:53 +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 'charmhelpers/core/templating.py'
--- charmhelpers/core/templating.py	2014-05-20 15:40:48 +0000
+++ charmhelpers/core/templating.py	2014-05-22 22:53:53 +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 'tests/core/test_host.py'
--- tests/core/test_host.py	2014-05-20 15:40:48 +0000
+++ tests/core/test_host.py	2014-05-22 22:53:53 +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-22 22:53:53 +0000
@@ -61,34 +61,28 @@
         services.reconfigure_services()
         self.assertEqual(mtemplating.render.call_args[0][1], None)
 
+    @mock.patch('charmhelpers.core.host.service_restart')
     @mock.patch('charmhelpers.core.services.templating')
-    @mock.patch('charmhelpers.core.services.reconfigure_service')
-    def test_reconfigure_services(self, mreconfig, mtemplating):
+    def test_reconfigure_services(self, mtemplating, mrestart):
         services.register(default_service_config(), TEMPLATES_DIR)
         services.reconfigure_services()
-        mreconfig.assert_called_once_with('cf-cloudcontroller', restart=True)
+        mtemplating.render.assert_called_once()
+        mrestart.assert_called_once_with('cf-cloudcontroller')
 
     @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)
+        services.reconfigure_services('cf-cloudcontroller')
         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)
+        services.reconfigure_services('cf-cloudcontroller')
         # verify that we did not restart the service
         self.assertFalse(mrestart.called)
 
@@ -97,30 +91,39 @@
     def test_reconfigure_service_no_context(self, mrestart, mtemplating):
         config = default_service_config()
         services.register(config, 'foo')
-        services.reconfigure_service('cf-cloudcontroller', restart=False)
+        services.reconfigure_services('cf-cloudcontroller')
         # verify that we called render template with the expected name
         mtemplating.render.assert_called_once_with(config[0]['templates'], 'foo')
+        mrestart.assert_called_once()
+        self.assertRaises(KeyError, services.reconfigure_services, 'unknownservice')
+
+    @mock.patch('charmhelpers.core.services.templating')
+    @mock.patch('charmhelpers.core.host.service_restart')
+    def test_custom_complete_action(self, mrestart, mtemplating):
+        config = default_service_config()
+        actions = config[0]['complete'] = [mock.Mock(name='custom_action')]
+        services.register(config, 'foo')
+        services.reconfigure_services('cf-cloudcontroller')
+        actions[0].assert_called_once_with(config[0]['service'])
         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):
+    @mock.patch('charmhelpers.core.host.service_stop')
+    def test_custom_stop_action(self, mstop, mtemplating):
         config = default_service_config()
-        config[0]['type'] = mock.Mock(name='CustomService')
+        actions = config[0]['stop'] = [mock.Mock(name='custom_action')]
         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)
+        services.stop_services()
+        actions[0].assert_called_once_with(config[0]['service'])
+        self.assertFalse(mstop.called)
 
     @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):
+    def test_stop_services(self, 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')
 
+
 if __name__ == '__main__':
     unittest.main()

=== modified file 'tox.ini'
--- tox.ini	2014-05-09 16:10:54 +0000
+++ tox.ini	2014-05-22 22:53:53 +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}


References