Add some more tests

hooks/cinder_contexts.py

@@ -1,14 +1,13 @@
 from charmhelpers.core.hookenv import (
     service_name,
     is_relation_made,
-    config
 )
 
 from charmhelpers.contrib.openstack.context import (
     OSContextGenerator,
 )
 
-from charmhelpers.contrib.openstack.utils import get_os_codename_install_source
+from charmhelpers.contrib.openstack.utils import get_os_codename_package
 
 
 class CephSubordinateContext(OSContextGenerator):
@@ -22,8 +21,7 @@ class CephSubordinateContext(OSContextGenerator):
         if not is_relation_made('ceph', 'key'):
             return {}
         service = service_name()
-        if get_os_codename_install_source(config('openstack-origin')) \
-                >= "icehouse":
+        if get_os_codename_package('cinder-common') >= "icehouse":
             volume_driver = 'cinder.volume.drivers.rbd.RBDDriver'
         else:
             volume_driver = 'cinder.volume.driver.RBDDriver'

hooks/cinder_hooks.py

@@ -18,13 +18,14 @@ from charmhelpers.core.hookenv import (
     config,
     service_name,
     relation_set,
-    relation_ids
+    relation_ids,
+    log
 )
 
 from cinder_contexts import CephSubordinateContext
 
 from charmhelpers.fetch import apt_install, apt_update
-from charmhelpers.core.host import restart_on_change, log
+from charmhelpers.core.host import restart_on_change
 
 from charmhelpers.contrib.storage.linux.ceph import ensure_ceph_keyring
 from charmhelpers.contrib.hahelpers.cluster import eligible_leader
@@ -39,15 +40,10 @@ CONFIGS = register_configs()
 @hooks.hook('install')
 def install():
     execd_preinstall()
-    apt_update()
+    apt_update(fatal=True)
     apt_install(PACKAGES, fatal=True)
 
 
-@hooks.hook('config-changed')
-def config_changed():
-    CONFIGS.write_all()
-
-
 @hooks.hook('ceph-relation-joined')
 def ceph_joined():
     if not os.path.isdir('/etc/ceph'):
@@ -74,8 +70,8 @@ def ceph_changed():
                 storage_backend(rid)
 
 
-@hooks.hook('ceph-relation-broken')
-@hooks.hook('upgrade-charm')
+@hooks.hook('ceph-relation-broken',
+            'config-changed')
 @restart_on_change(restart_map())
 def write_and_restart():
     CONFIGS.write_all()
@@ -93,6 +89,16 @@ def storage_backend(rel_id=None):
         )
 
 
+@hooks.hook('upgrade-charm')
+@restart_on_change(restart_map())
+def upgrade_charm():
+    if 'ceph' in CONFIGS.complete_contexts():
+        CONFIGS.write_all()
+        set_ceph_env_variables(service=service_name())
+        for rid in relation_ids('storage-backend'):
+            storage_backend(rid)
+
+
 if __name__ == '__main__':
     try:
         hooks.execute(sys.argv)

hooks/upgrade-charm

@@ -0,0 +1 @@
+cinder_hooks.py

unit_tests/test_cinder_contexts.py

@@ -7,8 +7,7 @@ from test_utils import (
 TO_PATCH = [
     'is_relation_made',
     'service_name',
-    'config',
-    'get_os_codename_install_source'
+    'get_os_codename_package'
 ]
 
 
@@ -23,7 +22,7 @@ class TestCinderContext(CharmTestCase):
 
     def test_ceph_related(self):
         self.is_relation_made.return_value = True
-        self.get_os_codename_install_source.return_value = "havana"
+        self.get_os_codename_package.return_value = "havana"
         service = 'mycinder'
         self.service_name.return_value = service
         self.assertEquals(
@@ -44,7 +43,7 @@ class TestCinderContext(CharmTestCase):
 
     def test_ceph_related_icehouse(self):
         self.is_relation_made.return_value = True
-        self.get_os_codename_install_source.return_value = "icehouse"
+        self.get_os_codename_package.return_value = "icehouse"
         service = 'mycinder'
         self.service_name.return_value = service
         self.assertEquals(

unit_tests/test_cinder_hooks.py

@@ -37,30 +37,17 @@ TO_PATCH = [
 ]
 
 
-class TestInstallHook(CharmTestCase):
-
+class TestCinderHooks(CharmTestCase):
     def setUp(self):
-        super(TestInstallHook, self).setUp(hooks, TO_PATCH)
+        super(TestCinderHooks, self).setUp(hooks, TO_PATCH)
         self.config.side_effect = self.test_config.get
 
-    def test_correct_install_packages(self):
+    def test_install(self):
         hooks.hooks.execute(['hooks/install'])
+        self.assertTrue(self.execd_preinstall.called)
+        self.assertTrue(self.apt_update.called)
         self.apt_install.assert_called_with(['ceph-common'], fatal=True)
 
-
-class TestChangedHooks(CharmTestCase):
-
-    def setUp(self):
-        super(TestChangedHooks, self).setUp(hooks, TO_PATCH)
-        self.config.side_effect = self.test_config.get
-
-
-class TestJoinedHooks(CharmTestCase):
-
-    def setUp(self):
-        super(TestJoinedHooks, self).setUp(hooks, TO_PATCH)
-        self.config.side_effect = self.test_config.get
-
     @patch('os.mkdir')
     def test_ceph_joined(self, mkdir):
         '''It correctly prepares for a ceph changed hook'''
@@ -107,3 +94,12 @@ class TestJoinedHooks(CharmTestCase):
         self.ensure_ceph_keyring.return_value = True
         hooks.hooks.execute(['hooks/ceph-relation-changed'])
         self.assertFalse(self.ensure_ceph_pool.called)
+
+    @patch.object(hooks, 'storage_backend')
+    def test_upgrade_charm_related(self, _storage_backend):
+        self.CONFIGS.complete_contexts.return_value = ['ceph']
+        self.relation_ids.return_value = ['ceph:1']
+        hooks.hooks.execute(['hooks/upgrade-charm'])
+        _storage_backend.assert_called_with('ceph:1')
+        assert self.CONFIGS.write_all.called
+        assert self.set_ceph_env_variables.called