Merge lp:~hopem/charm-helpers/lp1246796 into lp:charm-helpers

[Edward Hope-Morley]

[James Page (james-page)]

review: Approve

=== modified file 'charmhelpers/contrib/openstack/context.py'
--- charmhelpers/contrib/openstack/context.py	2013-10-16 11:57:08 +0000
+++ charmhelpers/contrib/openstack/context.py	2013-12-04 14:24:54 +0000
@@ -433,25 +433,55 @@
 
 
 class OSConfigFlagContext(OSContextGenerator):
-        '''
-        Responsible adding user-defined config-flags in charm config to a
-        to a template context.
-        '''
+        """
+        Responsible for adding user-defined config-flags in charm config to a
+        template context.
+
+        NOTE: the value of config-flags may be a comma-separated list of
+              key=value pairs and some Openstack config files support
+              comma-separated lists as values.
+        """
         def __call__(self):
             config_flags = config('config-flags')
-            if not config_flags or config_flags in ['None', '']:
+            if not config_flags:
                 return {}
-            config_flags = config_flags.split(',')
+
+            if config_flags.find('==') >= 0:
+                log("config_flags is not in expected format (key=value)",
+                    level=ERROR)
+                raise OSContextError
+
+            # strip the following from each value.
+            post_strippers = ' ,'
+            # we strip any leading/trailing '=' or ' ' from the string then
+            # split on '='.
+            split = config_flags.strip(' =').split('=')
+            limit = len(split)
             flags = {}
-            for flag in config_flags:
-                if '=' not in flag:
-                    log('Improperly formatted config-flag, expected k=v '
-                        'got %s' % flag, level=WARNING)
-                    continue
-                k, v = flag.split('=')
-                flags[k.strip()] = v
-            ctxt = {'user_config_flags': flags}
-            return ctxt
+            for i in xrange(0, limit - 1):
+                current = split[i]
+                next = split[i + 1]
+                vindex = next.rfind(',')
+                if (i == limit - 2) or (vindex < 0):
+                    value = next
+                else:
+                    value = next[:vindex]
+
+                if i == 0:
+                    key = current
+                else:
+                    # if this not the first entry, expect an embedded key.
+                    index = current.rfind(',')
+                    if index < 0:
+                        log("invalid config value(s) at index %s" % (i),
+                            level=ERROR)
+                        raise OSContextError
+                    key = current[index + 1:]
+
+                # Add to collection.
+                flags[key.strip(post_strippers)] = value.rstrip(post_strippers)
+
+            return {'user_config_flags': flags}
 
 
 class SubordinateConfigContext(OSContextGenerator):

=== modified file 'tests/contrib/openstack/test_os_contexts.py'
--- tests/contrib/openstack/test_os_contexts.py	2013-10-22 23:00:14 +0000
+++ tests/contrib/openstack/test_os_contexts.py	2013-12-04 14:24:54 +0000
@@ -636,6 +636,15 @@
     def test_os_configflag_context(self, config):
         flags = context.OSConfigFlagContext()
 
+        # single
+        config.return_value = 'deadbeef=True'
+        self.assertEquals({
+            'user_config_flags': {
+                'deadbeef': 'True',
+            }
+        }, flags())
+
+        # multi
         config.return_value = 'floating_ip=True,use_virtio=False,max=5'
         self.assertEquals({
             'user_config_flags': {
@@ -649,14 +658,23 @@
             config.return_value = empty
             self.assertEquals({}, flags())
 
+        # multi with commas
         config.return_value = 'good_flag=woot,badflag,great_flag=w00t'
         self.assertEquals({
             'user_config_flags': {
-                'good_flag': 'woot',
+                'good_flag': 'woot,badflag',
                 'great_flag': 'w00t',
             }
         }, flags())
 
+        # missing key
+        config.return_value = 'good_flag=woot=toow'
+        self.assertRaises(context.OSContextError, flags)
+
+        # bad value
+        config.return_value = 'good_flag=woot=='
+        self.assertRaises(context.OSContextError, flags)
+
     def test_os_subordinate_config_context(self):
         relation = FakeRelation(relation_data=SUB_CONFIG_RELATION)
         self.relation_get.side_effect = relation.get