Merge "Refactor methods in cinder store"2.0.0

3 files changed, 123 insertions, 134 deletions

diff --git a/glance_store/_drivers/cinder.py b/glance_store/_drivers/cinder.py
index 3350e39..2a83e95 100644
--- a/glance_store/_drivers/cinder.py
+++ b/glance_store/_drivers/cinder.py
@@ -37,7 +37,7 @@ import glance_store.location
 
 try:
     from cinderclient import exceptions as cinder_exception
-    from cinderclient.v2 import client as cinderclient
+    from cinderclient.v3 import client as cinderclient
     from os_brick.initiator import connector
     from oslo_privsep import priv_context
 except ImportError:
@@ -345,80 +345,6 @@ Possible values:
 ]
 
 
-def get_root_helper(backend=None):
-    if backend:
-        rootwrap = getattr(CONF, backend).rootwrap_config
-    else:
-        rootwrap = CONF.glance_store.rootwrap_config
-
-    return 'sudo glance-rootwrap %s' % rootwrap
-
-
-def is_user_overriden(conf, backend=None):
-    if backend:
-        store_conf = getattr(conf, backend)
-    else:
-        store_conf = conf.glance_store
-
-    return all([store_conf.get('cinder_store_' + key)
-                for key in ['user_name', 'password',
-                            'project_name', 'auth_address']])
-
-
-def get_cinderclient(conf, context=None, backend=None):
-    if backend:
-        glance_store = getattr(conf, backend)
-    else:
-        glance_store = conf.glance_store
-
-    user_overriden = is_user_overriden(conf, backend=backend)
-    if user_overriden:
-        username = glance_store.cinder_store_user_name
-        password = glance_store.cinder_store_password
-        project = glance_store.cinder_store_project_name
-        url = glance_store.cinder_store_auth_address
-    else:
-        username = context.user
-        password = context.auth_token
-        project = context.tenant
-
-        if glance_store.cinder_endpoint_template:
-            url = glance_store.cinder_endpoint_template % context.to_dict()
-        else:
-            info = glance_store.cinder_catalog_info
-            service_type, service_name, interface = info.split(':')
-            try:
-                catalog = keystone_sc.ServiceCatalogV2(context.service_catalog)
-                url = catalog.url_for(
-                    region_name=glance_store.cinder_os_region_name,
-                    service_type=service_type,
-                    service_name=service_name,
-                    interface=interface)
-            except keystone_exc.EndpointNotFound:
-                reason = _("Failed to find Cinder from a service catalog.")
-                raise exceptions.BadStoreConfiguration(store_name="cinder",
-                                                       reason=reason)
-
-    c = cinderclient.Client(username,
-                            password,
-                            project,
-                            auth_url=url,
-                            region_name=glance_store.cinder_os_region_name,
-                            insecure=glance_store.cinder_api_insecure,
-                            retries=glance_store.cinder_http_retries,
-                            cacert=glance_store.cinder_ca_certificates_file)
-
-    LOG.debug('Cinderclient connection created for user %(user)s using URL: '
-              '%(url)s.', {'user': username, 'url': url})
-
-    # noauth extracts user_id:project_id from auth_token
-    if not user_overriden:
-        c.client.auth_token = context.auth_token or '%s:%s' % (username,
-                                                               project)
-    c.client.management_url = url
-    return c
-
-
 class StoreLocation(glance_store.location.StoreLocation):
 
     """Class describing a Cinder URI."""
@@ -442,24 +368,6 @@ class StoreLocation(glance_store.location.StoreLocation):
             raise exceptions.BadStoreUri(message=reason)
 
 
-@contextlib.contextmanager
-def temporary_chown(path, backend=None):
-    owner_uid = os.getuid()
-    orig_uid = os.stat(path).st_uid
-
-    if orig_uid != owner_uid:
-        processutils.execute('chown', owner_uid, path,
-                             run_as_root=True,
-                             root_helper=get_root_helper(backend=backend))
-    try:
-        yield
-    finally:
-        if orig_uid != owner_uid:
-            processutils.execute('chown', orig_uid, path,
-                                 run_as_root=True,
-                                 root_helper=get_root_helper(backend=backend))
-
-
 class Store(glance_store.driver.Store):
 
     """Cinder backend store adapter."""
@@ -475,6 +383,96 @@ class Store(glance_store.driver.Store):
         if self.backend_group:
             self._set_url_prefix()
 
+    def get_root_helper(self):
+        if self.backend_group:
+            rootwrap = getattr(CONF, self.backend_group).rootwrap_config
+        else:
+            rootwrap = CONF.glance_store.rootwrap_config
+
+        return 'sudo glance-rootwrap %s' % rootwrap
+
+    def is_user_overriden(self):
+        if self.backend_group:
+            store_conf = getattr(self.conf, self.backend_group)
+        else:
+            store_conf = self.conf.glance_store
+
+        return all([store_conf.get('cinder_store_' + key)
+                    for key in ['user_name', 'password',
+                                'project_name', 'auth_address']])
+
+    def get_cinderclient(self, context=None):
+        if self.backend_group:
+            glance_store = getattr(self.conf, self.backend_group)
+        else:
+            glance_store = self.conf.glance_store
+
+        user_overriden = self.is_user_overriden()
+        if user_overriden:
+            username = glance_store.cinder_store_user_name
+            password = glance_store.cinder_store_password
+            project = glance_store.cinder_store_project_name
+            url = glance_store.cinder_store_auth_address
+        else:
+            username = context.user
+            password = context.auth_token
+            project = context.tenant
+
+            if glance_store.cinder_endpoint_template:
+                url = glance_store.cinder_endpoint_template % context.to_dict()
+            else:
+                info = glance_store.cinder_catalog_info
+                service_type, service_name, interface = info.split(':')
+                try:
+                    catalog = keystone_sc.ServiceCatalogV2(
+                        context.service_catalog)
+                    url = catalog.url_for(
+                        region_name=glance_store.cinder_os_region_name,
+                        service_type=service_type,
+                        service_name=service_name,
+                        interface=interface)
+                except keystone_exc.EndpointNotFound:
+                    reason = _("Failed to find Cinder from a service catalog.")
+                    raise exceptions.BadStoreConfiguration(store_name="cinder",
+                                                           reason=reason)
+
+        c = cinderclient.Client(
+            username, password, project, auth_url=url,
+            region_name=glance_store.cinder_os_region_name,
+            insecure=glance_store.cinder_api_insecure,
+            retries=glance_store.cinder_http_retries,
+            cacert=glance_store.cinder_ca_certificates_file)
+
+        LOG.debug(
+            'Cinderclient connection created for user %(user)s using URL: '
+            '%(url)s.', {'user': username, 'url': url})
+
+        # noauth extracts user_id:project_id from auth_token
+        if not user_overriden:
+            c.client.auth_token = context.auth_token or '%s:%s' % (username,
+                                                                   project)
+        c.client.management_url = url
+        return c
+
+    @contextlib.contextmanager
+    def temporary_chown(self, path):
+        owner_uid = os.getuid()
+        orig_uid = os.stat(path).st_uid
+
+        if orig_uid != owner_uid:
+            processutils.execute(
+                'chown', owner_uid, path,
+                run_as_root=True,
+                root_helper=self.get_root_helper())
+        try:
+            yield
+        finally:
+            if orig_uid != owner_uid:
+                processutils.execute(
+                    'chown', orig_uid, path,
+                    run_as_root=True,
+                    root_helper=self.get_root_helper())
+
     def get_schemes(self):
         return ('cinder',)
 
@@ -482,8 +480,7 @@ class Store(glance_store.driver.Store):
         self._url_prefix = "cinder://"
 
     def _check_context(self, context, require_tenant=False):
-        user_overriden = is_user_overriden(self.conf,
-                                           backend=self.backend_group)
+        user_overriden = self.is_user_overriden()
         if user_overriden and not require_tenant:
             return
         if context is None:
@@ -532,7 +529,7 @@ class Store(glance_store.driver.Store):
     def _open_cinder_volume(self, client, volume, mode):
         attach_mode = 'rw' if mode == 'wb' else 'ro'
         device = None
-        root_helper = get_root_helper(backend=self.backend_group)
+        root_helper = self.get_root_helper()
         priv_context.init(root_helper=shlex.split(root_helper))
         host = socket.gethostname()
         if self.backend_group:
@@ -573,9 +570,9 @@ class Store(glance_store.driver.Store):
                not conn.do_local_attach):
                 yield device['path']
             else:
-                with temporary_chown(device['path'],
-                                     backend=self.backend_group), \
-                        open(device['path'], mode) as f:
+                with self.temporary_chown(
+                        device['path'], backend=self.backend_group
+                ), open(device['path'], mode) as f:
                     yield f
         except Exception:
             LOG.exception(_LE('Exception while accessing to cinder volume '
@@ -652,8 +649,7 @@ class Store(glance_store.driver.Store):
         loc = location.store_location
         self._check_context(context)
         try:
-            client = get_cinderclient(self.conf, context,
-                                      backend=self.backend_group)
+            client = self.get_cinderclient(context)
             volume = client.volumes.get(loc.volume_id)
             size = int(volume.metadata.get('image_size',
                                            volume.size * units.Gi))
@@ -687,9 +683,7 @@ class Store(glance_store.driver.Store):
 
         try:
             self._check_context(context)
-            volume = get_cinderclient(
-                self.conf, context,
-                backend=self.backend_group).volumes.get(loc.volume_id)
+            volume = self.get_cinderclient(context).volumes.get(loc.volume_id)
             return int(volume.metadata.get('image_size',
                                            volume.size * units.Gi))
         except cinder_exception.NotFound:
@@ -723,8 +717,7 @@ class Store(glance_store.driver.Store):
         """
 
         self._check_context(context, require_tenant=True)
-        client = get_cinderclient(self.conf, context,
-                                  backend=self.backend_group)
+        client = self.get_cinderclient(context)
         os_hash_value = hashlib.new(str(hashing_algo))
         checksum = hashlib.md5()
         bytes_written = 0
@@ -849,9 +842,7 @@ class Store(glance_store.driver.Store):
         loc = location.store_location
         self._check_context(context)
         try:
-            volume = get_cinderclient(
-                self.conf, context,
-                backend=self.backend_group).volumes.get(loc.volume_id)
+            volume = self.get_cinderclient(context).volumes.get(loc.volume_id)
             volume.delete()
         except cinder_exception.NotFound:
             raise exceptions.NotFound(image=loc.volume_id)
diff --git a/glance_store/tests/unit/test_cinder_store.py b/glance_store/tests/unit/test_cinder_store.py
index b7dea33..de999d5 100644
--- a/glance_store/tests/unit/test_cinder_store.py
+++ b/glance_store/tests/unit/test_cinder_store.py
@@ -24,7 +24,7 @@ import tempfile
 import time
 import uuid
 
-from cinderclient.v2 import client as cinderclient
+from cinderclient.v3 import client as cinderclient
 from os_brick.initiator import connector
 from oslo_concurrency import processutils
 from oslo_utils import units
@@ -64,7 +64,7 @@ class TestCinderStore(base.StoreBaseTest,
         self.hash_algo = 'sha256'
 
     def test_get_cinderclient(self):
-        cc = cinder.get_cinderclient(self.conf, self.context)
+        cc = self.store.get_cinderclient(self.context)
         self.assertEqual('fake_token', cc.client.auth_token)
         self.assertEqual('http://foo/public_url', cc.client.management_url)
 
@@ -73,7 +73,7 @@ class TestCinderStore(base.StoreBaseTest,
         self.config(cinder_store_password='test_password')
         self.config(cinder_store_project_name='test_project')
         self.config(cinder_store_auth_address='test_address')
-        cc = cinder.get_cinderclient(self.conf, self.context)
+        cc = self.store.get_cinderclient(self.context)
         self.assertIsNone(cc.client.auth_token)
         self.assertEqual('test_address', cc.client.management_url)
 
@@ -95,9 +95,9 @@ class TestCinderStore(base.StoreBaseTest,
         with mock.patch.object(os, 'stat', return_value=fake_stat()), \
                 mock.patch.object(os, 'getuid', return_value=2), \
                 mock.patch.object(processutils, 'execute') as mock_execute, \
-                mock.patch.object(cinder, 'get_root_helper',
+                mock.patch.object(cinder.Store, 'get_root_helper',
                                   return_value='sudo'):
-            with cinder.temporary_chown('test'):
+            with self.store.temporary_chown('test'):
                 pass
             expected_calls = [mock.call('chown', 2, 'test', run_as_root=True,
                                         root_helper='sudo'),
@@ -178,9 +178,9 @@ class TestCinderStore(base.StoreBaseTest,
         with mock.patch.object(cinder.Store,
                                '_wait_volume_status',
                                return_value=fake_volume), \
-                mock.patch.object(cinder, 'temporary_chown',
+                mock.patch.object(cinder.Store, 'temporary_chown',
                                   side_effect=fake_chown), \
-                mock.patch.object(cinder, 'get_root_helper',
+                mock.patch.object(cinder.Store, 'get_root_helper',
                                   return_value=root_helper), \
                 mock.patch.object(connector.InitiatorConnector, 'factory',
                                   side_effect=fake_factory):
@@ -251,7 +251,7 @@ class TestCinderStore(base.StoreBaseTest,
             self.assertEqual('rb', mode)
             yield volume_file
 
-        with mock.patch.object(cinder, 'get_cinderclient') as mock_cc, \
+        with mock.patch.object(cinder.Store, 'get_cinderclient') as mock_cc, \
                 mock.patch.object(self.store, '_open_cinder_volume',
                                   side_effect=fake_open):
             mock_cc.return_value = FakeObject(client=fake_client,
@@ -277,7 +277,7 @@ class TestCinderStore(base.StoreBaseTest,
         fake_volume = FakeObject(size=5, metadata={})
         fake_volumes = {fake_volume_uuid: fake_volume}
 
-        with mock.patch.object(cinder, 'get_cinderclient') as mocked_cc:
+        with mock.patch.object(cinder.Store, 'get_cinderclient') as mocked_cc:
             mocked_cc.return_value = FakeObject(client=fake_client,
                                                 volumes=fake_volumes)
 
@@ -294,7 +294,7 @@ class TestCinderStore(base.StoreBaseTest,
                                  metadata={'image_size': expected_image_size})
         fake_volumes = {fake_volume_uuid: fake_volume}
 
-        with mock.patch.object(cinder, 'get_cinderclient') as mocked_cc:
+        with mock.patch.object(cinder.Store, 'get_cinderclient') as mocked_cc:
             mocked_cc.return_value = FakeObject(client=fake_client,
                                                 volumes=fake_volumes)
 
@@ -322,7 +322,7 @@ class TestCinderStore(base.StoreBaseTest,
             self.assertEqual('wb', mode)
             yield volume_file
 
-        with mock.patch.object(cinder, 'get_cinderclient') as mock_cc, \
+        with mock.patch.object(cinder.Store, 'get_cinderclient') as mock_cc, \
                 mock.patch.object(self.store, '_open_cinder_volume',
                                   side_effect=fake_open):
             mock_cc.return_value = FakeObject(client=fake_client,
@@ -376,7 +376,7 @@ class TestCinderStore(base.StoreBaseTest,
         fake_volume = FakeObject(delete=mock.Mock())
         fake_volumes = {fake_volume_uuid: fake_volume}
 
-        with mock.patch.object(cinder, 'get_cinderclient') as mocked_cc:
+        with mock.patch.object(cinder.Store, 'get_cinderclient') as mocked_cc:
             mocked_cc.return_value = FakeObject(client=fake_client,
                                                 volumes=fake_volumes)
 
diff --git a/glance_store/tests/unit/test_multistore_cinder.py b/glance_store/tests/unit/test_multistore_cinder.py
index 4b546da..597c4bc 100644
--- a/glance_store/tests/unit/test_multistore_cinder.py
+++ b/glance_store/tests/unit/test_multistore_cinder.py
@@ -93,8 +93,7 @@ class TestMultiCinderStore(base.MultiStoreBaseTest,
         self.assertEqual("cinder://", self.store.url_prefix)
 
     def test_get_cinderclient(self):
-        cc = cinder.get_cinderclient(self.conf, self.context,
-                                     backend="cinder1")
+        cc = self.store.get_cinderclient(self.context)
         self.assertEqual('fake_token', cc.client.auth_token)
         self.assertEqual('http://foo/public_url', cc.client.management_url)
 
@@ -103,8 +102,7 @@ class TestMultiCinderStore(base.MultiStoreBaseTest,
         self.config(cinder_store_password='test_password', group="cinder1")
         self.config(cinder_store_project_name='test_project', group="cinder1")
         self.config(cinder_store_auth_address='test_address', group="cinder1")
-        cc = cinder.get_cinderclient(self.conf, self.context,
-                                     backend="cinder1")
+        cc = self.store.get_cinderclient(self.context)
         self.assertIsNone(cc.client.auth_token)
         self.assertEqual('test_address', cc.client.management_url)
 
@@ -115,9 +113,9 @@ class TestMultiCinderStore(base.MultiStoreBaseTest,
         with mock.patch.object(os, 'stat', return_value=fake_stat()), \
                 mock.patch.object(os, 'getuid', return_value=2), \
                 mock.patch.object(processutils, 'execute') as mock_execute, \
-                mock.patch.object(cinder, 'get_root_helper',
+                mock.patch.object(cinder.Store, 'get_root_helper',
                                   return_value='sudo'):
-            with cinder.temporary_chown('test'):
+            with self.store.temporary_chown('test'):
                 pass
             expected_calls = [mock.call('chown', 2, 'test', run_as_root=True,
                                         root_helper='sudo'),
@@ -198,9 +196,9 @@ class TestMultiCinderStore(base.MultiStoreBaseTest,
         with mock.patch.object(cinder.Store,
                                '_wait_volume_status',
                                return_value=fake_volume), \
-                mock.patch.object(cinder, 'temporary_chown',
+                mock.patch.object(cinder.Store, 'temporary_chown',
                                   side_effect=fake_chown), \
-                mock.patch.object(cinder, 'get_root_helper',
+                mock.patch.object(cinder.Store, 'get_root_helper',
                                   return_value=root_helper), \
                 mock.patch.object(connector.InitiatorConnector, 'factory',
                                   side_effect=fake_factory):
@@ -272,7 +270,7 @@ class TestMultiCinderStore(base.MultiStoreBaseTest,
             self.assertEqual('rb', mode)
             yield volume_file
 
-        with mock.patch.object(cinder, 'get_cinderclient') as mock_cc, \
+        with mock.patch.object(cinder.Store, 'get_cinderclient') as mock_cc, \
                 mock.patch.object(self.store, '_open_cinder_volume',
                                   side_effect=fake_open):
             mock_cc.return_value = FakeObject(client=fake_client,
@@ -300,7 +298,7 @@ class TestMultiCinderStore(base.MultiStoreBaseTest,
         fake_volume = FakeObject(size=5, metadata={})
         fake_volumes = {fake_volume_uuid: fake_volume}
 
-        with mock.patch.object(cinder, 'get_cinderclient') as mocked_cc:
+        with mock.patch.object(cinder.Store, 'get_cinderclient') as mocked_cc:
             mocked_cc.return_value = FakeObject(client=fake_client,
                                                 volumes=fake_volumes)
 
@@ -319,7 +317,7 @@ class TestMultiCinderStore(base.MultiStoreBaseTest,
                                  metadata={'image_size': expected_image_size})
         fake_volumes = {fake_volume_uuid: fake_volume}
 
-        with mock.patch.object(cinder, 'get_cinderclient') as mocked_cc:
+        with mock.patch.object(cinder.Store, 'get_cinderclient') as mocked_cc:
             mocked_cc.return_value = FakeObject(client=fake_client,
                                                 volumes=fake_volumes)
 
@@ -348,7 +346,7 @@ class TestMultiCinderStore(base.MultiStoreBaseTest,
             self.assertEqual('wb', mode)
             yield volume_file
 
-        with mock.patch.object(cinder, 'get_cinderclient') as mock_cc, \
+        with mock.patch.object(cinder.Store, 'get_cinderclient') as mock_cc, \
                 mock.patch.object(self.store, '_open_cinder_volume',
                                   side_effect=fake_open):
             mock_cc.return_value = FakeObject(client=fake_client,
@@ -404,7 +402,7 @@ class TestMultiCinderStore(base.MultiStoreBaseTest,
         fake_volume = FakeObject(delete=mock.Mock())
         fake_volumes = {fake_volume_uuid: fake_volume}
 
-        with mock.patch.object(cinder, 'get_cinderclient') as mocked_cc:
+        with mock.patch.object(cinder.Store, 'get_cinderclient') as mocked_cc:
             mocked_cc.return_value = FakeObject(client=fake_client,
                                                 volumes=fake_volumes)