Fix NFS/CIFS share creation failure issue

When the image count is over 25, there might not get
manila-service-image, because current manila shares
creation is using novaclient to get image info, but
novaclient can only get 25 images due to pagination
of glance server, So this change is to switch to use
glanceclient instead of novaclient to get image info,
because glanceclient can iter all image info, while
novaclient is rarely maintained with stuff of image
API.

Change-Id: Id1715d0b9cb3a4aeedeb23d9b1d9924a78d18dc6
Closes-Bug: #1741425

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