From 1ca4bd0ab941f6c7aebf86bdf5ff47fdc1d2d3e2 Mon Sep 17 00:00:00 2001 From: Dmitrii Shcherbakov Date: Sat, 27 Jan 2018 22:42:16 +0300 Subject: [PATCH] add string template rendering capability In some cases software deliberately allows drop-in config file usage capabilities, for example, when it comes to enforcing policy, the desired behavior varies from an operator to operator. For that reason it is sometimes desirable to supply custom templates via config options. Another use-case is templates that are passed from subordinates for a primary charm to render. Given that properties and desired adapters can be arbitrary the change uses a dict of meta tuples of the following format to render templates from strings based on adapter properties: {config_file_path: (relation_name, adapter property)} relation names must be normalized (lowercase, underscores instead of dashes. "options" relation name is used for a config adapter as usual. In summary a string config file path should be used: 1. in the restart_map for a given derived class; 2. in string_templates dict as a key for a meta tuple Change-Id: Ic85b22d0e5d497c49c75243e3c280140f940df66 Closes-Bug: #1741723 --- charms_openstack/charm/core.py | 55 ++++++++- .../charms_openstack/charm/test_core.py | 111 ++++++++++++++++-- 2 files changed, 156 insertions(+), 10 deletions(-) diff --git a/charms_openstack/charm/core.py b/charms_openstack/charm/core.py index 8244520..41721a6 100644 --- a/charms_openstack/charm/core.py +++ b/charms_openstack/charm/core.py @@ -690,6 +690,34 @@ class BaseOpenStackCharmActions(object): self.render_configs(self.full_restart_map.keys(), adapters_instance=adapters_instance) + def _get_string_template(self, conf, adapters_instance): + """ + Find out if a charm class provides meta information about whether + this is a template to be fetched from a string dynamically or not. + """ + config_template = None + tmpl_meta = self.string_templates.get(conf) + if tmpl_meta: + # meta information exists but not clear if an attribute has + # been set yet either via config option or via relation data + config_template = False + rel_name, _property = tmpl_meta + try: + config_template_adapter = getattr(adapters_instance, + rel_name) + try: + config_template = getattr(config_template_adapter, + _property) + except AttributeError: + raise RuntimeError('{} does not contain {} property' + .format(config_template_adapter, + _property)) + except AttributeError: + hookenv.log('Skipping a string template for {} as a ' + 'relation adapter is not present' + .format(rel_name), level=hookenv.DEBUG) + return config_template + def render_configs(self, configs, adapters_instance=None): """Render the configuration files identified in the list passed as configs. @@ -703,19 +731,37 @@ class BaseOpenStackCharmActions(object): render_with_interfaces() which constructs a new adapters_instance anyway. + Configs may not only be loaded via OpenStack loaders but also via + string templates passed via config options or from relation data. + This must be explicitly declared via string_templates dict of a given + derived charm class by using a relation name that identifies a relation + adapter or config option adapter and a property to be used from that + adapter instance. + :param configs: list of strings, the names of the configuration files. :param adapters_instance: [optional] the adapters_instance to use. """ if adapters_instance is None: adapters_instance = self.adapters_instance + with self.restart_on_change(): for conf in configs: + # check if we need to load a template from a string + config_template = self._get_string_template(conf, + adapters_instance) + if config_template is False: + # got a string template but it was not provided which + # means we need to skip this config to avoid rendering + return + charmhelpers.core.templating.render( source=os.path.basename(conf), template_loader=os_templating.get_loader( 'templates/', self.release), target=conf, - context=adapters_instance) + context=adapters_instance, + config_template=config_template + ) def render_with_interfaces(self, interfaces, configs=None): """Render the configs using the interfaces passed; overrides any @@ -931,6 +977,13 @@ class BaseOpenStackCharmAssessStatus(object): services = [] """ + # a dict of meta tuples of the following format to render templates + # from strings based on adapter properties (resolved at runtime): + # {config_file_path: (relation_name, adapter property)} + # relation names should be normalized (lowercase, underscores instead of + # dashes; use "options" relation name for a config adapter + string_templates = {} + def __init__(self, *args, **kwargs): """Set up specific mixin requirements""" self.__run_assess_status = False diff --git a/unit_tests/charms_openstack/charm/test_core.py b/unit_tests/charms_openstack/charm/test_core.py index c53f08a..fcd8bfb 100644 --- a/unit_tests/charms_openstack/charm/test_core.py +++ b/unit_tests/charms_openstack/charm/test_core.py @@ -3,6 +3,7 @@ import mock import unittest import charms_openstack.charm.core as chm_core +import charms_openstack.adapters as os_adapters from unit_tests.charms_openstack.charm.utils import BaseOpenStackCharmTest from unit_tests.charms_openstack.charm.common import ( @@ -471,12 +472,88 @@ class TestMyOpenStackCharm(BaseOpenStackCharmTest): source='path1', template_loader='my-loader', target='path1', - context=mock.ANY) + context=mock.ANY, + config_template=None + ) # assert the context was an MyAdapter instance. context = self.render.call_args_list[0][1]['context'] assert isinstance(context, MyAdapter) self.assertEqual(context.interfaces, ['interface1', 'interface2']) + def test_render_config_from_string(self): + # give us a way to check that the context manager was called. + from contextlib import contextmanager + d = [0] + + @contextmanager + def fake_restart_on_change(): + d[0] += 1 + yield + + self.target.string_templates = {'path1': ('options', 't_prop')} + + self.patch_target('restart_on_change', new=fake_restart_on_change) + self.patch_object(chm_core.charmhelpers.core.templating, 'render') + self.patch_object(chm_core.os_templating, + 'get_loader', + return_value='my-loader') + + config_template = 'justatest' + adapters_instance = self.target.adapters_instance + adapters_instance.options = mock.MagicMock() + adapters_instance.options.t_prop = config_template + + self.target.render_configs(['path1']) + self.assertEqual(d[0], 1) + self.render.assert_called_once_with( + source='path1', + template_loader='my-loader', + target='path1', + context=mock.ANY, + config_template=config_template, + ) + # assert the context was an MyAdapter instance. + context = self.render.call_args_list[0][1]['context'] + assert isinstance(context, MyAdapter) + self.assertEqual(context.interfaces, ['interface1', 'interface2']) + + def test_render_config_from_string_no_property(self): + self.target.string_templates = {'path1': ('options', 't_prop')} + + self.patch_object(chm_core.charmhelpers.core.templating, 'render') + self.patch_object(chm_core.os_templating, + 'get_loader', + return_value='my-loader') + + adapters_instance = self.target.adapters_instance + adapters_instance.options = mock.create_autospec( + os_adapters.ConfigurationAdapter + ) + + self.assertRaises(RuntimeError, self.target.render_configs, ['path1']) + + def test_render_config_from_string_no_relation(self): + """ + Make sure that if there is no relation adapter yet for a provided + string template metadata there are no error conditions triggered. + In other words, 'render' function should not be called while an attempt + to get a template from an adapter property should be made. + """ + self.target.string_templates = {'path1': ('options', 't_prop')} + self.patch_object(chm_core.charmhelpers.core.templating, 'render') + self.patch_object(chm_core.os_templating, + 'get_loader', + return_value='my-loader') + + with mock.patch.object(MyOpenStackCharm, '_get_string_template', + wraps=self.target._get_string_template) as m: + + adapters_instance = self.target.adapters_instance + + self.target.render_configs(['path1']) + m.assert_called_once_with('path1', adapters_instance) + self.render.assert_not_called() + def test_render_configs_singleton_render_with_interfaces(self): self.patch_object(chm_core.charmhelpers.core.templating, 'render') self.patch_object(chm_core.os_templating, @@ -498,22 +575,30 @@ class TestMyOpenStackCharm(BaseOpenStackCharmTest): source='path1', template_loader='my-loader', target='path1', - context=mock.ANY), + context=mock.ANY, + config_template=None + ), mock.call( source='path2', template_loader='my-loader', target='path2', - context=mock.ANY), + context=mock.ANY, + config_template=None + ), mock.call( source='path3', template_loader='my-loader', target='path3', - context=mock.ANY), + context=mock.ANY, + config_template=None + ), mock.call( source='path4', template_loader='my-loader', target='path4', - context=mock.ANY), + context=mock.ANY, + config_template=None + ), ] self.render.assert_has_calls(calls, any_order=True) # Assert that None was not passed to render via the context kwarg @@ -549,22 +634,30 @@ class TestMyOpenStackCharm(BaseOpenStackCharmTest): source='path1', template_loader='my-loader', target='path1', - context=mock.ANY), + context=mock.ANY, + config_template=None + ), mock.call( source='path2', template_loader='my-loader', target='path2', - context=mock.ANY), + context=mock.ANY, + config_template=None + ), mock.call( source='path3', template_loader='my-loader', target='path3', - context=mock.ANY), + context=mock.ANY, + config_template=None + ), mock.call( source='path4', template_loader='my-loader', target='path4', - context=mock.ANY), + context=mock.ANY, + config_template=None + ), ] self.render.assert_has_calls(calls, any_order=True) # Assert that None was not passed to render via the context kwarg