Merge lp:~hopem/charms/trusty/cinder/lp1493931 into lp:~openstack-charmers-archive/charms/trusty/cinder/next

[Edward Hope-Morley]

[James Page (james-page)]

Hi Ed

I think we need to add features to backend subordinates so that the sub can determine and flag that its stateful/stateless; that was we don't need to update the cinder charm every time we add a new backend that is stateless.

I would suggest a default assumption of 'stateful', meaning that we would just need to update the cinder-ceph charm for now to set this new flag.

I would suggest something like:

   relation_get('stateful') or True

for the cinder charm, with the cinder-ceph charm setting this on its relation to cinder:

   relation_set('stateful', True)

review: Needs Fixing

[Edward Hope-Morley (hopem)]

Fair dos i'll get that done.

[uosci-testing-bot (uosci-testing-bot)]

charm_lint_check #9701 cinder-next for hopem mp270658
    LINT FAIL: lint-test failed

LINT Results (max last 2 lines):
make: *** [lint] Error 1
ERROR:root:Make target returned non-zero.

Full lint test output: http://paste.ubuntu.com/12328230/
Build: http://10.245.162.77:8080/job/charm_lint_check/9701/

[uosci-testing-bot (uosci-testing-bot)]

charm_unit_test #8944 cinder-next for hopem mp270658
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/8944/

[uosci-testing-bot (uosci-testing-bot)]

charm_amulet_test #6332 cinder-next for hopem mp270658
    AMULET FAIL: amulet-test failed

AMULET Results (max last 2 lines):
make: *** [functional_test] Error 1
ERROR:root:Make target returned non-zero.

Full amulet test output: http://paste.ubuntu.com/12328604/
Build: http://10.245.162.77:8080/job/charm_amulet_test/6332/

[James Page (james-page)]

review: Resubmit

[lp:~hopem/charms/trusty/cinder/lp1493931]

128. By Edward Hope-Morley on 2015-09-11

fixed unit test lint errors

[Ryan Beisner (1chb1n)]

Amulet fail #6332 appears to have been a (new?) juju bug. I've not filed one yet, but after walking through the logs of the various stages, here are observances:

http://paste.ubuntu.com/12366632/

2015-09-10 13:33:59 DEBUG juju.service discovery.go:111 failed to find init system "upstart": exec "/sbin/initctl" failed: exit status 1

Unmerged revisions

=== modified file 'hooks/cinder_contexts.py'
--- hooks/cinder_contexts.py	2014-11-25 10:19:07 +0000
+++ hooks/cinder_contexts.py	2015-09-10 11:18:26 +0000
@@ -3,12 +3,15 @@
     relation_ids,
     service_name,
     related_units,
-    relation_get
+    relation_get,
+    log,
+    WARNING,
 )
 
 from charmhelpers.contrib.openstack.context import (
     OSContextGenerator,
     ApacheSSLContext as SSLContext,
+    SubordinateConfigContext,
 )
 
 from charmhelpers.contrib.openstack.utils import (
@@ -110,3 +113,44 @@
 
     def __call__(self):
         return {'debug': config('debug'), 'verbose': config('verbose')}
+
+
+class CinderSubordinateConfigContext(SubordinateConfigContext):
+
+    def __call__(self):
+        ctxt = super(CinderSubordinateConfigContext, self).__call__()
+
+        # If all backends are stateless we can allow host setting to be set
+        # across hosts/units to allow for HA volume failover but otherwise we
+        # have to leave it as unique (LP: #1493931).
+        rids = []
+        for interface in self.interfaces:
+            rids.extend(relation_ids(interface))
+
+        stateless = None
+        any_stateless = False
+        for rid in rids:
+            for unit in related_units(rid):
+                val = relation_get('stateless', rid=rid, unit=unit) or ""
+                if val.lower() == 'true':
+                    if stateless is None:
+                        stateless = True
+                    else:
+                        stateless = stateless and True
+                else:
+                    stateless = False
+
+                any_stateless = any_stateless or stateless
+
+        if stateless:
+            if 'DEFAULT' in ctxt['sections']:
+                ctxt['sections']['DEFAULT'].append(('host', service_name()))
+            else:
+                ctxt['sections']['DEFAULT'] = [('host', service_name())]
+
+        elif any_stateless:
+            log("One or more stateless backends configured but unable to "
+                "set host param since there appear to also be stateful "
+                "backends configured.", level=WARNING)
+
+        return ctxt

=== modified file 'hooks/cinder_utils.py'
--- hooks/cinder_utils.py	2015-08-03 20:18:39 +0000
+++ hooks/cinder_utils.py	2015-09-10 11:18:26 +0000
@@ -172,7 +172,7 @@
                           cinder_contexts.CephContext(),
                           cinder_contexts.HAProxyContext(),
                           cinder_contexts.ImageServiceContext(),
-                          context.SubordinateConfigContext(
+                          cinder_contexts.CinderSubordinateConfigContext(
                               interface='storage-backend',
                               service='cinder',
                               config_file=CINDER_CONF),

=== modified file 'unit_tests/test_cinder_contexts.py'
--- unit_tests/test_cinder_contexts.py	2014-10-01 22:07:44 +0000
+++ unit_tests/test_cinder_contexts.py	2015-09-10 11:18:26 +0000
@@ -135,3 +135,209 @@
                                    'namespace': 'cinder'})
         self.assertTrue(mock_https.called)
         mock_unit_get.assert_called_with('private-address')
+
+    @patch('%s.relation_get' % (mod_ch_context))
+    @patch('%s.related_units' % (mod_ch_context))
+    @patch('%s.relation_ids' % (mod_ch_context))
+    @patch('%s.log' % (mod_ch_context), lambda *args, **kwargs: None)
+    def test_subordinate_config_context_stateless(self, mock_rel_ids,
+                                                  mock_rel_units,
+                                                  mock_rel_get):
+        mock_rel_ids.return_value = ['storage-backend:0']
+        self.relation_ids.return_value = ['storage-backend:0']
+
+        mock_rel_units.return_value = ['cinder-ceph/0']
+        self.related_units.return_value = ['cinder-ceph/0']
+
+        self.service_name.return_value = 'cinder'
+
+        settings = \
+            {'backend_name': 'cinder-ceph',
+             'private-address': '10.5.8.191',
+             'stateless': 'True',
+             'subordinate_configuration':
+                 '{"cinder": '
+                 '{"/etc/cinder/cinder.conf": '
+                 '{"sections": '
+                 '{"cinder-ceph": '
+                 '[["volume_backend_name", '
+                 '"cinder-ceph"], '
+                 '["volume_driver", '
+                 '"cinder.volume.drivers.rbd.RBDDriver"], '
+                 '["rbd_pool", '
+                 '"cinder-ceph"], '
+                 '["rbd_user", '
+                 '"cinder-ceph"]]}}}}'}
+
+        def fake_rel_get(attribute=None, unit=None, rid=None):
+            return settings.get(attribute)
+
+        mock_rel_get.side_effect = fake_rel_get
+        self.relation_get.side_effect = fake_rel_get
+
+        ctxt = contexts.CinderSubordinateConfigContext(
+            interface='storage-backend',
+            service='cinder',
+            config_file='/etc/cinder/cinder.conf')()
+
+        exp = {'sections': {'DEFAULT': [('host', 'cinder')],
+               u'cinder-ceph': [[u'volume_backend_name', u'cinder-ceph'],
+                                [u'volume_driver',
+                                 u'cinder.volume.drivers.rbd.RBDDriver'],
+                                [u'rbd_pool', u'cinder-ceph'],
+                                [u'rbd_user', u'cinder-ceph']]}}
+
+        self.assertEquals(ctxt, exp)
+
+    @patch('%s.relation_get' % (mod_ch_context))
+    @patch('%s.related_units' % (mod_ch_context))
+    @patch('%s.relation_ids' % (mod_ch_context))
+    @patch('%s.log' % (mod_ch_context), lambda *args, **kwargs: None)
+    def test_subordinate_config_context_statefull(self, mock_rel_ids,
+                                                  mock_rel_units,
+                                                  mock_rel_get):
+        mock_rel_ids.return_value = ['storage-backend:0']
+        self.relation_ids.return_value = ['storage-backend:0']
+
+        mock_rel_units.return_value = ['cinder-ceph/0']
+        self.related_units.return_value = ['cinder-ceph/0']
+
+        self.service_name.return_value = 'cinder'
+
+        settings = \
+            {'backend_name': 'cinder-ceph',
+             'private-address': '10.5.8.191',
+             'stateless': 'False',
+             'subordinate_configuration':
+                 '{"cinder": '
+                 '{"/etc/cinder/cinder.conf": '
+                 '{"sections": '
+                 '{"cinder-ceph": '
+                 '[["volume_backend_name", '
+                 '"cinder-ceph"], '
+                 '["volume_driver", '
+                 '"cinder.volume.drivers.rbd.RBDDriver"], '
+                 '["rbd_pool", '
+                 '"cinder-ceph"], '
+                 '["rbd_user", '
+                 '"cinder-ceph"]]}}}}'}
+
+        def fake_rel_get(attribute=None, unit=None, rid=None):
+            return settings.get(attribute)
+
+        mock_rel_get.side_effect = fake_rel_get
+        self.relation_get.side_effect = fake_rel_get
+
+        ctxt = contexts.CinderSubordinateConfigContext(
+            interface='storage-backend',
+            service='cinder',
+            config_file='/etc/cinder/cinder.conf')()
+
+        exp = {'sections':
+               {u'cinder-ceph': [[u'volume_backend_name',
+                                  u'cinder-ceph'],
+                                 [u'volume_driver',
+                                  u'cinder.volume.drivers.rbd.RBDDriver'],
+                                 [u'rbd_pool', u'cinder-ceph'],
+                                 [u'rbd_user', u'cinder-ceph']]}}
+
+        self.assertEquals(ctxt, exp)
+
+        del settings['stateless']
+
+        ctxt = contexts.CinderSubordinateConfigContext(
+            interface='storage-backend',
+            service='cinder',
+            config_file='/etc/cinder/cinder.conf')()
+
+        exp = {'sections':
+               {u'cinder-ceph': [[u'volume_backend_name',
+                                  u'cinder-ceph'],
+                                 [u'volume_driver',
+                                  u'cinder.volume.drivers.rbd.RBDDriver'],
+                                 [u'rbd_pool', u'cinder-ceph'],
+                                 [u'rbd_user', u'cinder-ceph']]}}
+
+        self.assertEquals(ctxt, exp)
+
+    @patch('%s.relation_get' % (mod_ch_context))
+    @patch('%s.related_units' % (mod_ch_context))
+    @patch('%s.relation_ids' % (mod_ch_context))
+    @patch.object(contexts, 'log', lambda *args, **kwargs: None)
+    @patch('%s.log' % (mod_ch_context), lambda *args, **kwargs: None)
+    def test_subordinate_config_context_mixed(self, mock_rel_ids,
+                                              mock_rel_units,
+                                              mock_rel_get):
+        mock_rel_ids.return_value = ['storage-backend:0', 'storage-backend:1']
+        self.relation_ids.return_value = ['storage-backend:0',
+                                          'storage-backend:1']
+
+        def fake_rel_units(rid):
+            if rid == 'storage-backend:0':
+                return ['cinder-ceph/0']
+            else:
+                return ['cinder-other/0']
+
+        mock_rel_units.side_effect = fake_rel_units
+        self.related_units.side_effect = fake_rel_units
+
+        self.service_name.return_value = 'cinder'
+
+        cinder_ceph_settings = \
+            {'backend_name': 'cinder-ceph',
+             'private-address': '10.5.8.191',
+             'stateless': 'True',
+             'subordinate_configuration':
+                 '{"cinder": '
+                 '{"/etc/cinder/cinder.conf": '
+                 '{"sections": '
+                 '{"cinder-ceph": '
+                 '[["volume_backend_name", '
+                 '"cinder-ceph"], '
+                 '["volume_driver", '
+                 '"cinder.volume.drivers.rbd.RBDDriver"], '
+                 '["rbd_pool", '
+                 '"cinder-ceph"], '
+                 '["rbd_user", '
+                 '"cinder-ceph"]]}}}}'}
+
+        cinder_other_settings = \
+            {'backend_name': 'cinder-other',
+             'private-address': '10.5.8.192',
+             'subordinate_configuration':
+                 '{"cinder": '
+                 '{"/etc/cinder/cinder.conf": '
+                 '{"sections": '
+                 '{"cinder-other": '
+                 '[["volume_backend_name", '
+                 '"cinder-other"], '
+                 '["volume_driver", '
+                 '"cinder.volume.drivers.OtherDriver"]]}}}}'}
+
+        def fake_rel_get(attribute=None, unit=None, rid=None):
+            if unit == 'cinder-ceph/0':
+                return cinder_ceph_settings.get(attribute)
+            elif unit == 'cinder-other/0':
+                return cinder_other_settings.get(attribute)
+
+        mock_rel_get.side_effect = fake_rel_get
+        self.relation_get.side_effect = fake_rel_get
+
+        ctxt = contexts.CinderSubordinateConfigContext(
+            interface='storage-backend',
+            service='cinder',
+            config_file='/etc/cinder/cinder.conf')()
+
+        exp = {'sections':
+               {u'cinder-ceph': [[u'volume_backend_name',
+                                  u'cinder-ceph'],
+                                 [u'volume_driver',
+                                  u'cinder.volume.drivers.rbd.RBDDriver'],
+                                 [u'rbd_pool', u'cinder-ceph'],
+                                 [u'rbd_user', u'cinder-ceph']],
+                u'cinder-other': [[u'volume_backend_name',
+                                   u'cinder-other'],
+                                  [u'volume_driver',
+                                   u'cinder.volume.drivers.OtherDriver']]}}
+
+        self.assertEquals(ctxt, exp)