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

[Edward Hope-Morley]

[Jorge Niedbalski (niedbalski)]

LGTM, just a quick fix required; charmhelpers/contrib/openstack/context.py:724 is missed from tests coverage, and that line appears to be a relevant condition.

review: Needs Fixing

[Edward Hope-Morley (hopem)]

> LGTM, just a quick fix required; charmhelpers/contrib/openstack/context.py:724
> is missed from tests coverage, and that line appears to be a relevant
> condition.

Fixed.

review: Needs Resubmitting

[Jorge Niedbalski (niedbalski)]

LGTM, thanks for the fix.

review: Approve

[Edward Hope-Morley (hopem)]

Apologies, i messed up the MP so had to resubmit.

[Jorge Niedbalski (niedbalski)]

review: Approve

[Benjamin Saller (bcsaller)]

LGTM, Thanks.

As a counterpoint you might be interested in lp:~cf-charmers/charm-helpers/cloud-foundry which moves something like (inspired by) the context framework into charmhelpers.core. I believe the pattern has been made more general in the CF usage and there is an MP for helpers upsteam. Interestingly it doesn't have any special support for subordinates which might be something we didn't capture so I found looking this branch over informative.

review: Approve

[Matt Bruzek (mbruzek)]

Unit tests pass. Thanks for the other reviews!

review: Approve

=== modified file 'charmhelpers/contrib/openstack/context.py'
--- charmhelpers/contrib/openstack/context.py	2014-06-26 13:42:36 +0000
+++ charmhelpers/contrib/openstack/context.py	2014-07-03 14:54:27 +0000
@@ -24,6 +24,7 @@
     unit_get,
     unit_private_ip,
     ERROR,
+    INFO
 )
 
 from charmhelpers.contrib.hahelpers.cluster import (
@@ -689,7 +690,7 @@
         self.interface = interface
 
     def __call__(self):
-        ctxt = {}
+        ctxt = {'sections': {}}
         for rid in relation_ids(self.interface):
             for unit in related_units(rid):
                 sub_config = relation_get('subordinate_configuration',
@@ -715,10 +716,14 @@
 
                     sub_config = sub_config[self.config_file]
                     for k, v in sub_config.iteritems():
-                        ctxt[k] = v
+                        if k == 'sections':
+                            for section, config_dict in v.iteritems():
+                                log("adding section '%s'" % (section))
+                                ctxt[k][section] = config_dict
+                        else:
+                            ctxt[k] = v
 
-        if not ctxt:
-            ctxt['sections'] = {}
+        log("%d section(s) found" % (len(ctxt['sections'])), level=INFO)
 
         return ctxt
 

=== modified file 'tests/contrib/openstack/test_os_contexts.py'
--- tests/contrib/openstack/test_os_contexts.py	2014-06-24 12:16:53 +0000
+++ tests/contrib/openstack/test_os_contexts.py	2014-07-03 14:54:27 +0000
@@ -213,6 +213,24 @@
                 - [glance-key2, value2]
 """
 
+CINDER_SUB_CONFIG1 = """
+cinder:
+    /etc/cinder/cinder.conf:
+        sections:
+            cinder-1-section:
+                - [key1, value1]
+"""
+
+CINDER_SUB_CONFIG2 = """
+cinder:
+    /etc/cinder/cinder.conf:
+        sections:
+            cinder-2-section:
+                - [key2, value2]
+        not-a-section:
+            1234
+"""
+
 SUB_CONFIG_RELATION = {
     'nova-subordinate:0': {
         'nova-subordinate/0': {
@@ -231,7 +249,19 @@
             'private-address': 'foo_node1',
             'subordinate_configuration': 'ea8e09324jkadsfh',
         },
-    }
+    },
+    'cinder-subordinate:0': {
+        'cinder-subordinate/0': {
+            'private-address': 'cinder_node1',
+            'subordinate_configuration': json.dumps(yaml.load(CINDER_SUB_CONFIG1)),
+        },
+    },
+    'cinder-subordinate:1': {
+        'cinder-subordinate/1': {
+            'private-address': 'cinder_node1',
+            'subordinate_configuration': json.dumps(yaml.load(CINDER_SUB_CONFIG2)),
+        },
+    },
 }
 
 # Imported in contexts.py and needs patching in setUp()
@@ -1121,6 +1151,11 @@
             config_file='/etc/glance/glance.conf',
             interface='glance-subordinate',
         )
+        cinder_sub_ctxt = context.SubordinateConfigContext(
+            service='cinder',
+            config_file='/etc/cinder/cinder.conf',
+            interface='cinder-subordinate',
+        )
         foo_sub_ctxt = context.SubordinateConfigContext(
             service='foo',
             config_file='/etc/foo/foo.conf',
@@ -1142,6 +1177,16 @@
                     ['glance-key2', 'value2']]
             }}
         )
+        self.assertEquals(
+            cinder_sub_ctxt(),
+            {'sections': {
+                'cinder-1-section': [
+                    ['key1', 'value1']],
+                'cinder-2-section': [
+                    ['key2', 'value2']]
+             
+            }, 'not-a-section': 1234}
+        )
 
         # subrodinate supplies nothing for given config
         glance_sub_ctxt.config_file = '/etc/glance/glance-api-paste.ini'

=== modified file 'tests/contrib/storage/test_linux_storage_lvm.py'
--- tests/contrib/storage/test_linux_storage_lvm.py	2014-06-23 10:00:22 +0000
+++ tests/contrib/storage/test_linux_storage_lvm.py	2014-07-03 14:54:27 +0000
@@ -22,7 +22,7 @@
 EMPTY_VG_IN_PVDISPLAY = """
   --- Physical volume ---
   PV Name               /dev/loop0
-  VG Name   
+  VG Name
   PV Size               10.00 MiB / not usable 2.00 MiB
   Allocatable           yes
   PE Size               4.00 MiB