cf-charmers team mailing list archive
-
cf-charmers team
-
Mailing list archive
-
Message #00231
[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
