Merge "Fix NFS/CIFS share creation failure issue"

devstack/plugin.sh

@@ -263,7 +263,9 @@ function configure_manila {
     if is_service_enabled cinder; then
         configure_keystone_authtoken_middleware $MANILA_CONF cinder cinder
     fi
-
+    if is_service_enabled glance; then
+        configure_keystone_authtoken_middleware $MANILA_CONF glance glance
+    fi
     # Note: set up config group does not mean that this backend will be enabled.
     # To enable it, specify its name explicitly using "enabled_share_backends" opt.
     configure_default_backends

lower-constraints.txt

@@ -96,6 +96,7 @@ pyperclip==1.6.0
 python-cinderclient==3.3.0
 python-dateutil==2.7.0
 python-editor==1.0.3
+python-glanceclient==2.15.0
 python-keystoneclient==3.15.0
 python-mimeparse==1.6.0
 python-neutronclient==6.7.0

manila/image/__init__.py

@@ -0,0 +1,33 @@
+# All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+import oslo_config.cfg
+import oslo_utils.importutils
+
+
+_glance_opts = [
+    oslo_config.cfg.StrOpt('image_api_class',
+                           default='manila.image.glance.API',
+                           help='The full class name of the '
+                                'Glance API class to use.'),
+]
+
+oslo_config.cfg.CONF.register_opts(_glance_opts)
+
+
+def API():
+    importutils = oslo_utils.importutils
+    glance_api_class = oslo_config.cfg.CONF.image_api_class
+    cls = importutils.import_class(glance_api_class)
+    return cls()

manila/image/glance.py

@@ -0,0 +1,71 @@
+# All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+"""
+Handles all requests to Glance.
+"""
+
+from glanceclient import client as glance_client
+from glanceclient import exc as glance_exception
+from keystoneauth1 import loading as ks_loading
+from oslo_config import cfg
+
+from manila.common import client_auth
+from manila.common.config import core_opts
+from manila.db import base
+
+GLANCE_GROUP = 'glance'
+AUTH_OBJ = None
+
+
+glance_opts = [
+    cfg.StrOpt('api_microversion',
+               default='2',
+               help='Version of Glance API to be used.'),
+    cfg.StrOpt('region_name',
+               default='RegionOne',
+               help='Region name for connecting to glance.'),
+    ]
+
+CONF = cfg.CONF
+CONF.register_opts(core_opts)
+CONF.register_opts(glance_opts, GLANCE_GROUP)
+ks_loading.register_session_conf_options(CONF, GLANCE_GROUP)
+ks_loading.register_auth_conf_options(CONF, GLANCE_GROUP)
+
+
+def list_opts():
+    return client_auth.AuthClientLoader.list_opts(GLANCE_GROUP)
+
+
+def glanceclient(context):
+    global AUTH_OBJ
+    if not AUTH_OBJ:
+        AUTH_OBJ = client_auth.AuthClientLoader(
+            client_class=glance_client.Client,
+            exception_module=glance_exception,
+            cfg_group=GLANCE_GROUP)
+    return AUTH_OBJ.get_client(context,
+                               version=CONF[GLANCE_GROUP].api_microversion,
+                               region_name=CONF[GLANCE_GROUP].region_name)
+
+
+class API(base.Base):
+    """API for interacting with glanceclient."""
+
+    def image_list(self, context):
+        client = glanceclient(context)
+        if hasattr(client, 'images'):
+            return client.images.list()
+        return client.glance.list()

manila/opts.py

@@ -35,6 +35,8 @@ import manila.data.helper
 import manila.db.api
 import manila.db.base
 import manila.exception
+import manila.image
+import manila.image.glance
 import manila.message.api
 import manila.network
 import manila.network.linux.interface
@@ -110,6 +112,7 @@ _global_opt_lists = [
     manila.db.api.db_opts,
     [manila.db.base.db_driver_opt],
     manila.exception.exc_log_opts,
+    manila.image._glance_opts,
     manila.message.api.messages_opts,
     manila.network.linux.interface.OPTS,
     manila.network.network_opts,
@@ -198,6 +201,8 @@ _opts = [
      list(itertools.chain(manila.compute.nova.nova_opts))),
     (manila.network.neutron.api.NEUTRON_GROUP,
      list(itertools.chain(manila.network.neutron.api.neutron_opts))),
+    (manila.image.glance.GLANCE_GROUP,
+     list(itertools.chain(manila.image.glance.glance_opts))),
 ]
 
 _opts.extend(oslo_concurrency.opts.list_opts())
@@ -206,6 +211,7 @@ _opts.extend(oslo_middleware.opts.list_opts())
 _opts.extend(oslo_policy.opts.list_opts())
 _opts.extend(manila.network.neutron.api.list_opts())
 _opts.extend(manila.compute.nova.list_opts())
+_opts.extend(manila.image.glance.list_opts())
 _opts.extend(manila.volume.cinder.list_opts())
 _opts.extend(oslo_service.sslutils.list_opts())
 

manila/share/drivers/service_instance.py

@@ -33,6 +33,7 @@ from manila import compute
 from manila import context
 from manila import exception
 from manila.i18n import _
+from manila import image
 from manila.network.linux import ip_lib
 from manila.network.neutron import api as neutron
 from manila import utils
@@ -222,6 +223,7 @@ class ServiceInstanceManager(object):
         self.admin_context = context.get_admin_context()
         self._execute = utils.execute
 
+        self.image_api = image.API()
         self.compute_api = compute.API()
 
         self.path_to_private_key = self.get_config_option(
@@ -529,12 +531,18 @@ class ServiceInstanceManager(object):
     def _get_service_image(self, context):
         """Returns ID of service image for service vm creating."""
         service_image_name = self.get_config_option("service_image_name")
-        image = self.compute_api.image_get(context, service_image_name)
-        if image.status != 'active':
+        images = [image.id for image in self.image_api.image_list(context)
+                  if image.name == service_image_name
+                  and image.status == 'active']
+        if not images:
             raise exception.ServiceInstanceException(
-                _("Image with name '%s' is not in 'active' state.") %
+                _("Image with name '%s' was not found or is not in "
+                  "'active' state.") % service_image_name)
+        if len(images) != 1:
+            raise exception.ServiceInstanceException(
+                _("Multiple 'active' state images found with name '%s'!") %
                 service_image_name)
-        return image
+        return images[0]
 
     def _create_service_instance(self, context, instance_name, network_info):
         """Creates service vm and sets up networking for it."""

manila/tests/image/test_image.py

@@ -0,0 +1,125 @@
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+
+from unittest import mock
+
+from manila import context
+from manila.image import glance
+from manila import test
+from manila.tests import utils as test_utils
+
+
+class FakeGlanceClient(object):
+
+    class Image(object):
+
+        def list(self, *args, **kwargs):
+            return [{'id': 'id1'}, {'id': 'id2'}]
+
+        def __getattr__(self, item):
+            return None
+
+    def __init__(self):
+        self.image = self.Image()
+
+
+def get_fake_auth_obj():
+    return type('FakeAuthObj', (object, ), {'get_client': mock.Mock()})
+
+
+class GlanceClientTestCase(test.TestCase):
+
+    @mock.patch('manila.image.glance.AUTH_OBJ', None)
+    def test_no_auth_obj(self):
+        mock_client_loader = self.mock_object(
+            glance.client_auth, 'AuthClientLoader')
+        fake_context = 'fake_context'
+        data = {
+            'glance': {
+                'api_microversion': 'foo_api_microversion',
+                'region_name': 'foo_region_name'
+            }
+        }
+
+        with test_utils.create_temp_config_with_opts(data):
+            glance.glanceclient(fake_context)
+
+        mock_client_loader.return_value.get_client.assert_called_once_with(
+            fake_context,
+            version=data['glance']['api_microversion'],
+            region_name=data['glance']['region_name']
+        )
+
+    @mock.patch('manila.image.glance.AUTH_OBJ', get_fake_auth_obj())
+    def test_with_auth_obj(self):
+        fake_context = 'fake_context'
+        data = {
+            'glance': {
+                'api_microversion': 'foo_api_microversion',
+                'region_name': 'foo_region_name'
+            }
+        }
+
+        with test_utils.create_temp_config_with_opts(data):
+            glance.glanceclient(fake_context)
+
+        glance.AUTH_OBJ.get_client.assert_called_once_with(
+            fake_context,
+            version=data['glance']['api_microversion'],
+            region_name=data['glance']['region_name']
+        )
+
+
+class GlanceApiTestCase(test.TestCase):
+    def setUp(self):
+        super(GlanceApiTestCase, self).setUp()
+
+        self.api = glance.API()
+        self.glanceclient = FakeGlanceClient()
+        self.ctx = context.get_admin_context()
+        self.mock_object(glance, 'glanceclient',
+                         mock.Mock(return_value=self.glanceclient))
+
+    def test_image_list_glanceclient_has_no_proxy(self):
+        image_list = ['fake', 'image', 'list']
+
+        class FakeGlanceClient(object):
+            def list(self):
+                return image_list
+
+        self.glanceclient.glance = FakeGlanceClient()
+
+        result = self.api.image_list(self.ctx)
+
+        self.assertEqual(image_list, result)
+
+    def test_image_list_glanceclient_has_proxy(self):
+        image_list1 = ['fake', 'image', 'list1']
+        image_list2 = ['fake', 'image', 'list2']
+
+        class FakeImagesClient(object):
+            def list(self):
+                return image_list1
+
+        class FakeGlanceClient(object):
+            def list(self):
+                return image_list2
+
+        self.glanceclient.images = FakeImagesClient()
+        self.glanceclient.glance = FakeGlanceClient()
+
+        result = self.api.image_list(self.ctx)
+
+        self.assertEqual(image_list1, result)

manila/tests/share/drivers/test_service_instance.py

@@ -732,21 +732,47 @@ class ServiceInstanceManagerTestCase(test.TestCase):
                 self.assertEqual((None, None), result)
 
     def test_get_service_image(self):
-        fake_image = fake_compute.FakeImage(
+        fake_image1 = fake_compute.FakeImage(
             name=self._manager.get_config_option('service_image_name'),
             status='active')
-        self.mock_object(self._manager.compute_api, 'image_get',
-                         mock.Mock(return_value=fake_image))
+        fake_image2 = fake_compute.FakeImage(
+            name='service_image_name',
+            status='error')
+        fake_image3 = fake_compute.FakeImage(
+            name='another-image',
+            status='active')
+        self.mock_object(self._manager.image_api, 'image_list',
+                         mock.Mock(return_value=[fake_image1,
+                                                 fake_image2,
+                                                 fake_image3]))
 
         result = self._manager._get_service_image(self._manager.admin_context)
-        self.assertEqual(fake_image, result)
+        self.assertEqual(fake_image1.id, result)
+
+    def test_get_service_image_not_found(self):
+        self.mock_object(self._manager.image_api, 'image_list',
+                         mock.Mock(return_value=[]))
+        self.assertRaises(
+            exception.ServiceInstanceException,
+            self._manager._get_service_image, self._manager.admin_context)
 
     def test_get_service_image_not_active(self):
         fake_error_image = fake_compute.FakeImage(
             name='service_image_name',
             status='error')
-        self.mock_object(self._manager.compute_api, 'image_get',
-                         mock.Mock(return_value=fake_error_image))
+        self.mock_object(self._manager.image_api, 'image_list',
+                         mock.Mock(return_value=[fake_error_image]))
+        self.assertRaises(
+            exception.ServiceInstanceException,
+            self._manager._get_service_image, self._manager.admin_context)
+
+    def test_get_service_image_ambiguous(self):
+        fake_image = fake_compute.FakeImage(
+            name=fake_get_config_option('service_image_name'),
+            status='active')
+        fake_images = [fake_image, fake_image]
+        self.mock_object(self._manager.image_api, 'image_list',
+                         mock.Mock(return_value=fake_images))
         self.assertRaises(
             exception.ServiceInstanceException,
             self._manager._get_service_image, self._manager.admin_context)

releasenotes/notes/1741425-fix-service-image-retrieval-issue-bf7ae3e5aa9446e9.yaml

@@ -0,0 +1,21 @@
+---
+upgrade:
+  - |
+    When using a driver with the ``service-instance`` module, ``manila.conf``
+    now requires a ``[glance]`` section in addition the the previously required
+    sections for ``[neutron]``, ``[nova]``, and ``cinder`` since the glanceclient
+    is now required as well as the clients for these other services.  To generate
+    a sample manila.conf that includes sections for all of these services run
+    `` tox -egenconfig`` from the top of the manila source repository.
+fixes:
+  - |
+    Share creation sometimes failed with drivers that use the
+    ``service-instance`` module (currently, the ``generic`` and
+    ``windows smb`` because the service-instance image could not be
+    found.  The service instance module used the ``novaclient`` to
+    discover the images, it paginates lists of images, and if there
+    are more than 25 images the service-image may not be in the list.
+
+    This fix switches to use the ``glanceclient`` -- a more direct and
+    appropriate client for OpenStack images that is not subject to the
+    pagination limitation.

requirements.txt

@@ -41,4 +41,5 @@ stevedore>=1.20.0 # Apache-2.0
 tooz>=1.58.0 # Apache-2.0
 python-cinderclient!=4.0.0,>=3.3.0 # Apache-2.0
 python-novaclient>=9.1.0 # Apache-2.0
+python-glanceclient>=2.15.0 # Apache-2.0
 WebOb>=1.7.1 # MIT