Merge lp:~saviq/charm-helpers/confg-patterns into lp:charm-helpers

[Michał Sawicz]

[saviq] Add support for config file patterns

[lp:~saviq/charm-helpers/confg-patterns]

384. By Michał Sawicz on 2015-06-01

Fix lint

385. By Michał Sawicz on 2015-06-01

Fix for python3

[Corey Bryant (corey.bryant)]

Hi Michal,

I know the initial target is for the horizon subordinate plugin charms to use this, but would you mind giving an example of how horizon would use it?

I have a couple of comments inline below.

Corey

[Michał Sawicz (saviq)]

> I know the initial target is for the horizon subordinate plugin charms
> to use this, but would you mind giving an example of how horizon would use it?

Sure. The charm would register a pattern config:

> register_config('/path/to/dashboard/enabled/_{}_juju_{}.py')

Then,

> class RouterSettingContext(OSPatternContextGenerator):
> def __call__(self):
> ctxt = {
> (40, 'router'): {'DISABLED': False if config('profile') in ['cisco'] else True}
> }
> return ctxt

and, for example
> class PluginContext(OSPatternContextGenerator):
> def __call__(self):
> # ...
> ctxt = {}
> for rid in relation_ids('plugin'):
> # ...
> rdata = relation_get(rid=rid, unit=unit)
> ctxt[(rdata['priority'], unit): {'DASHBOARD': rdata['DASHBOARD'],
> 'DISABLED': rdata['DISABLED'],
> # ...
> return ctxt

This would result in generating files, say:

> /path/to/config/enabled/_40_juju_router.py
> /path/to/config/enabled/_99_juju_horizon-contrail.py

In PluginContext I can then warn about conflicts etc. Multiple subordinates could even override one another.

HTH, let me know if I should explain more of this in the docstring.

[lp:~saviq/charm-helpers/confg-patterns]

386. By Michał Sawicz on 2015-06-01

Fix a bug in pattern context generation and add some more comments

[Corey Bryant (corey.bryant)]

> > I know the initial target is for the horizon subordinate plugin charms
> > to use this, but would you mind giving an example of how horizon would use
> it?
>
> Sure. The charm would register a pattern config:
>
> > register_config('/path/to/dashboard/enabled/_{}_juju_{}.py')
>
> Then,
>
> > class RouterSettingContext(OSPatternContextGenerator):
> > def __call__(self):
> > ctxt = {
> > (40, 'router'): {'DISABLED': False if config('profile') in
> ['cisco'] else True}
> > }
> > return ctxt
>
> and, for example
> > class PluginContext(OSPatternContextGenerator):
> > def __call__(self):
> > # ...
> > ctxt = {}
> > for rid in relation_ids('plugin'):
> > # ...
> > rdata = relation_get(rid=rid, unit=unit)
> > ctxt[(rdata['priority'], unit): {'DASHBOARD':
> rdata['DASHBOARD'],
> > 'DISABLED': rdata['DISABLED'],
> > # ...
> > return ctxt
>
> This would result in generating files, say:
>
> > /path/to/config/enabled/_40_juju_router.py
> > /path/to/config/enabled/_99_juju_horizon-contrail.py
>
> In PluginContext I can then warn about conflicts etc. Multiple subordinates
> could even override one another.
>
> HTH, let me know if I should explain more of this in the docstring.

Thanks for the explanation. I'm wondering if this can be solved without adding the pattern context code. I was just looking at neutron-openvswitch, which is a subordinate charm. For example it has:

BASE_RESOURCE_MAP = OrderedDict([
     (NEUTRON_CONF, {
         'services': ['neutron-plugin-openvswitch-agent'],
         'contexts': [neutron_ovs_context.OVSPluginContext(),
                      context.AMQPContext(ssl_dir=NEUTRON_CONF_DIR),
                      context.ZeroMQContext(),
                      context.NotificationDriverContext()],
     }),
     ...

Can you do something similar for subordinate horizon charms? In other words, the subordinate charms would write the plugin file themselves.

For example. horizon subordinate 1 could have:

BASE_RESOURCE_MAP = OrderedDict([
     (/path/to/config/enabled/_40_juju_router.py, {
         'contexts': [RouterContext()],
     }),
     ...

and horizon subordinate 2 could have:

BASE_RESOURCE_MAP = OrderedDict([
     (/path/to/config/enabled//_99_juju_horizon-contrail.py, {
         'contexts': [ContrailContext()],
     }),
     ...

You could make the precedence (ie. 40 and 99) more flexibly in the subordinate charm and allow them to be changed via a config option.

[Michał Sawicz (saviq)]

@Corey,

Sure, the subordinate could write that file itself, if that's preferable. Unfortunately the remaining problem for the contrail use case is that before Juno there's no way to update HORIZON_CONFIG from the split files, so we need to amend the main settings.py file. But this isn't really related to this MP...

[Michał Sawicz (saviq)]

I will update the horizon-contrail charm to do this, so this MP can be dropped, really.

[Michał Sawicz (saviq)]

Hah, I can't change the status of this MP to Rejected, can you?

[Stuart Bishop (stub)]

review: Disapprove

Unmerged revisions

386. By Michał Sawicz on 2015-06-01

Fix a bug in pattern context generation and add some more comments

385. By Michał Sawicz on 2015-06-01

Fix for python3

384. By Michał Sawicz on 2015-06-01

Fix lint

383. By Michał Sawicz on 2015-06-01

Refactor test to disallow additional calls

382. By Michał Sawicz on 2015-06-01

Implement support for generating multiple config files based on a pattern

381. By Michał Sawicz on 2015-06-01

Add context and templating classes to support generating multiple files based on a pattern

=== modified file 'charmhelpers/contrib/openstack/context.py'
--- charmhelpers/contrib/openstack/context.py	2015-04-16 19:19:18 +0000
+++ charmhelpers/contrib/openstack/context.py	2015-06-01 22:06:07 +0000
@@ -194,6 +194,15 @@
         raise NotImplementedError
 
 
+class OSPatternContextGenerator(OSContextGenerator):
+    """Base class for pattern context generators.
+
+    __call__ should return a dictionary of { tuple: dict }, where the tuple
+    will be used as input for format() for the filename pattern as registered
+    by OSConfigRenderer.register_pattern().
+    """
+
+
 class SharedDBContext(OSContextGenerator):
     interfaces = ['shared-db']
 

=== modified file 'charmhelpers/contrib/openstack/templating.py'
--- charmhelpers/contrib/openstack/templating.py	2015-01-22 06:06:03 +0000
+++ charmhelpers/contrib/openstack/templating.py	2015-06-01 22:06:07 +0000
@@ -14,7 +14,9 @@
 # You should have received a copy of the GNU Lesser General Public License
 # along with charm-helpers.  If not, see <http://www.gnu.org/licenses/>.
 
+import glob
 import os
+import string
 
 import six
 
@@ -25,6 +27,7 @@
     INFO
 )
 from charmhelpers.contrib.openstack.utils import OPENSTACK_CODENAMES
+from charmhelpers.contrib.openstack.context import OSPatternContextGenerator
 
 try:
     from jinja2 import FileSystemLoader, ChoiceLoader, Environment, exceptions
@@ -96,7 +99,7 @@
         else:
             self.contexts = contexts
 
-        self._complete_contexts = []
+        self._complete_contexts = set()
 
     def context(self):
         ctxt = {}
@@ -105,9 +108,7 @@
             if _ctxt:
                 ctxt.update(_ctxt)
                 # track interfaces for every complete context.
-                [self._complete_contexts.append(interface)
-                 for interface in context.interfaces
-                 if interface not in self._complete_contexts]
+                self._complete_contexts.update(context.interfaces)
         return ctxt
 
     def complete_contexts(self):
@@ -120,6 +121,41 @@
         return self._complete_contexts
 
 
+class OSPatternConfigTemplate(OSConfigTemplate):
+    """
+    Associates a config pattern template with a list of context generators.
+    Responsible for constructing a template context based on those generators.
+    """
+    def __init__(self, pattern, contexts):
+        self.pattern = pattern
+        super(OSPatternConfigTemplate, self).__init__(config_file=None,
+                                                      contexts=contexts)
+
+    def context(self):
+        base_ctxt = {}
+        ctxt = {}
+        for context in self.contexts:
+            _ctxt = context()
+            if not _ctxt:
+                continue
+            elif isinstance(context, OSPatternContextGenerator):
+                # for each returned key initialize its context with base_ctxt
+                # if not defined before and update with new data
+                for k, v in _ctxt.items():
+                    if k not in ctxt:
+                        ctxt[k] = base_ctxt.copy()
+                    ctxt[k].update(v)
+            else:
+                # update the base context and all pre-existing file-specific
+                # contexts
+                base_ctxt.update(_ctxt)
+                for key in ctxt:
+                    ctxt[key].update(_ctxt)
+            # track interfaces for every complete context.
+            self._complete_contexts.update(context.interfaces)
+        return ctxt
+
+
 class OSConfigRenderer(object):
     """
     This class provides a common templating system to be used by OpenStack
@@ -132,6 +168,13 @@
         # import some common context generates from charmhelpers
         from charmhelpers.contrib.openstack import context
 
+        # or create your own
+        class SimpleContextGenerator(OSContextGenerator):
+            def __call__():
+                return {
+                    'key': 'value'
+                }
+
         # Create a renderer object for a specific OS release.
         configs = OSConfigRenderer(templates_dir='/tmp/templates',
                                    openstack_release='folsom')
@@ -142,12 +185,31 @@
         configs.register(config_file='/etc/nova/api-paste.ini',
                          contexts=[context.IdentityServiceContext()])
         configs.register(config_file='/etc/haproxy/haproxy.conf',
-                         contexts=[context.HAProxyContext()])
+                         contexts=[context.HAProxyContext(),
+                                   SimpleContextGenerator()])
         # write out a single config
         configs.write('/etc/nova/nova.conf')
         # write out all registered configs
         configs.write_all()
 
+    Using patterns::
+        class DashboardContextGenerator(OSPatternContextGenerator):
+            def __call__():
+                return {
+                    (40, 'router'): { 'DISABLED': True }
+                }
+
+        configs.register_pattern(
+            pattern='/usr/share/openstack-dashboard/openstack_dashboard'
+                    '/enabled/_{}_juju_{}.py',
+            contexts=[
+                DashboardContextGenerator()
+            ])
+        # delete all files matching the pattern and
+        # write _40_juju_router.py anew
+        configs.write('/usr/share/openstack-dashboard/openstack_dashboard'
+                      '/enabled/_{}_juju_{}.py')
+
     **OpenStack Releases and template loading**
 
     When the object is instantiated, it is associated with a specific OS
@@ -219,6 +281,15 @@
                                                        contexts=contexts)
         log('Registered config file: %s' % config_file, level=INFO)
 
+    def register_pattern(self, pattern, contexts):
+        """
+        Register a config file name pattern with a list of context generators
+        to be called during rendering. Use standard format() specification.
+        """
+        self.templates[pattern] = OSPatternConfigTemplate(pattern=pattern,
+                                                          contexts=contexts)
+        log('Registered config pattern: %s' % pattern, level=INFO)
+
     def _get_tmpl_env(self):
         if not self._tmpl_env:
             loader = get_loader(self.templates_dir, self.openstack_release)
@@ -234,7 +305,8 @@
         if config_file not in self.templates:
             log('Config not registered: %s' % config_file, level=ERROR)
             raise OSConfigException
-        ctxt = self.templates[config_file].context()
+        ostemplate = self.templates[config_file]
+        ctxt = ostemplate.context()
 
         _tmpl = os.path.basename(config_file)
         try:
@@ -253,7 +325,14 @@
                 raise e
 
         log('Rendering from template: %s' % _tmpl, level=INFO)
-        return template.render(ctxt)
+
+        if not isinstance(ostemplate, OSPatternConfigTemplate):
+            ctxt = {(): ctxt}
+
+        renders = {}
+        for args, file_ctxt in ctxt.items():
+            renders[args] = template.render(file_ctxt)
+        return renders
 
     def write(self, config_file):
         """
@@ -263,10 +342,16 @@
             log('Config not registered: %s' % config_file, level=ERROR)
             raise OSConfigException
 
-        _out = self.render(config_file)
-
-        with open(config_file, 'wb') as out:
-            out.write(_out)
+        renders = self.render(config_file)
+
+        files = glob.glob(''.join([t[0] + ('' if t[1] is None else '*')
+                          for t in string.Formatter().parse(config_file)]))
+        for name in files:
+            os.unlink(name)
+
+        for args, render in renders.items():
+            with open(config_file.format(*args), 'wb') as out:
+                out.write(render)
 
         log('Wrote template %s.' % config_file, level=INFO)
 

=== modified file 'tests/contrib/openstack/test_os_templating.py'
--- tests/contrib/openstack/test_os_templating.py	2015-02-11 21:41:57 +0000
+++ tests/contrib/openstack/test_os_templating.py	2015-06-01 22:06:07 +0000
@@ -2,9 +2,11 @@
 import os
 import unittest
 
-from mock import patch, call, MagicMock
+from mock import patch, call, Mock, MagicMock
 
 import charmhelpers.contrib.openstack.templating as templating
+from charmhelpers.contrib.openstack.context import OSPatternContextGenerator,\
+    OSContextGenerator
 
 from jinja2.exceptions import TemplateNotFound
 
@@ -15,7 +17,7 @@
     builtin_open = 'builtins.open'
 
 
-class FakeContextGenerator(object):
+class FakeContextGenerator(OSContextGenerator):
     interfaces = None
 
     def set(self, interfaces, context):
@@ -26,6 +28,10 @@
         return self.context
 
 
+class FakePatternContextGenerator(FakeContextGenerator, OSPatternContextGenerator):
+    pass
+
+
 class FakeLoader(object):
     def set(self, template):
         self.template = template
@@ -55,6 +61,7 @@
         path = os.path.dirname(__file__)
         self.loader = FakeLoader()
         self.context = FakeContextGenerator()
+        self.pattern_context = FakePatternContextGenerator()
 
         self.addCleanup(patch.object(templating, 'apt_install').start().stop())
         self.addCleanup(patch.object(templating, 'log').start().stop())
@@ -167,9 +174,11 @@
         '''It renders template if it finds it by config file basename'''
 
     @patch(builtin_open)
+    @patch('glob.glob')
     @patch.object(templating, 'get_loader')
-    def test_write_out_config(self, loader, _open):
+    def test_write_out_config(self, loader, _glob, _open):
         '''It writes a templated config when provided a complete context'''
+        _glob.return_value = []
         self.context.set(interfaces=['fooservice'], context={'foo': 'bar'})
         self.renderer.register('/tmp/foo', [self.context])
         with patch.object(self.renderer, '_get_template') as _get_t:
@@ -178,8 +187,10 @@
             self.renderer.write('/tmp/foo')
             _open.assert_called_with('/tmp/foo', 'wb')
 
-    def test_write_all(self):
+    @patch('glob.glob')
+    def test_write_all(self, _glob):
         '''It writes out all configuration files at once'''
+        _glob.return_value = []
         self.context.set(interfaces=['fooservice'], context={'foo': 'bar'})
         self.renderer.register('/tmp/foo', [self.context])
         self.renderer.register('/tmp/bar', [self.context])
@@ -192,6 +203,53 @@
             self.assertEquals(sorted(ex_calls), sorted(_write.call_args_list))
             pass
 
+    @patch(builtin_open)
+    @patch('glob.glob')
+    @patch.object(templating, 'get_loader')
+    def test_write_out_config_pattern(self, loader, _glob, _open):
+        '''It writes a templated pattern config when provided a complete context'''
+        self.pattern_context.set(interfaces=[], context={(1,): {'foo': 'bar'},
+                                                         (2,): {'foo': 'baz'}})
+        self.renderer.register_pattern('/tmp/foo_{}', [self.pattern_context])
+        _glob.return_value = []
+
+        ex_calls = [
+            call('/tmp/foo_1', 'wb'),
+            call('/tmp/foo_2', 'wb')
+        ]
+
+        with patch.object(self.renderer, '_get_template') as _get_t:
+            fake_tmpl = MockTemplate()
+            _get_t.return_value = fake_tmpl
+            self.renderer.write('/tmp/foo_{}')
+            self.assertEquals(sorted(ex_calls), sorted(_open.call_args_list))
+
+    @patch(builtin_open)
+    @patch('os.unlink')
+    @patch('glob.glob')
+    @patch.object(templating, 'get_loader')
+    def test_write_out_config_pattern_clean(self, loader, _glob, _unlink, _open):
+        '''It deletes all matching files prior to writing new ones'''
+        self.pattern_context.set(interfaces=[], context={(1,): {'foo': 'bar'}})
+        self.renderer.register_pattern('/tmp/foo_{}', [self.pattern_context])
+        _glob.return_value = ['/tmp/foo_1', '/tmp/foo_2']
+
+        m = Mock()
+        m.attach_mock(_open, 'open')
+        m.attach_mock(_unlink, 'unlink')
+        m.attach_mock(_glob, 'glob')
+
+        with patch.object(self.renderer, '_get_template') as _get_t:
+            fake_tmpl = MockTemplate()
+            _get_t.return_value = fake_tmpl
+            self.renderer.write('/tmp/foo_{}')
+            m.assert_has_calls([
+                call.glob('/tmp/foo_*'),
+                call.unlink('/tmp/foo_1'),
+                call.unlink('/tmp/foo_2'),
+                call.open('/tmp/foo_1', 'wb')
+            ])
+
     @patch.object(templating, 'get_loader')
     def test_reset_template_loader_for_new_os_release(self, loader):
         self.loader.set('')
@@ -282,3 +340,53 @@
         tmpl = templating.OSConfigTemplate(config_file='/tmp/foo',
                                            contexts=_c1)
         self.assertEquals(tmpl.contexts, [_c1])
+
+    def test_generate_context(self):
+        '''Ensure context is generated correctly'''
+        def _c1():
+            return {'a': 1, 'b': 2}
+        _c1.interfaces = []
+
+        def _c2():
+            return {'a': 3}
+        _c2.interfaces = []
+        tmpl = templating.OSConfigTemplate(config_file='/tmp/foo',
+                                           contexts=[_c1, _c2])
+        self.assertEqual(tmpl.context(), {'a': 3, 'b': 2})
+
+    def test_generate_pattern_context(self):
+        class _c1(OSContextGenerator):
+            def __call__(self):
+                return {'a': 1, 'b': 2, 'c': 3, 'd': 4}
+
+        class _c2(OSPatternContextGenerator):
+            def __call__(self):
+                return {
+                    ('foo',): {'a': 4},
+                    ('bar',): {'b': 5}
+                }
+
+        class _c3(OSContextGenerator):
+            def __call__(self):
+                return {'d': 6}
+
+        class _c4(OSPatternContextGenerator):
+            def __call__(self):
+                return {
+                    ('bar',): {'a': 7},
+                    ('baz',): {}
+                }
+
+        class _c5(OSContextGenerator):
+            def __call__(self):
+                return {'e': 0}
+
+        tmpl = templating.OSPatternConfigTemplate(pattern='/tmp/foo_{}',
+                                                  contexts=[_c1(), _c2(),
+                                                            _c3(), _c4(),
+                                                            _c5()])
+        self.assertEqual(tmpl.context(), {
+            ('foo',): {'a': 4, 'b': 2, 'c': 3, 'd': 6, 'e': 0},
+            ('bar',): {'a': 7, 'b': 5, 'c': 3, 'd': 6, 'e': 0},
+            ('baz',): {'a': 1, 'b': 2, 'c': 3, 'd': 6, 'e': 0},
+        })