Merge "Use only Glance V2 by default (with a compatibility option)"

etc/ironic/ironic.conf.sample

@@ -1382,6 +1382,11 @@
 # [hostname|IP]:port. (list value)
 #glance_api_servers = <None>
 
+# Glance API version (1 or 2) to use. (integer value)
+# Minimum value: 1
+# Maximum value: 2
+#glance_api_version = 2
+
 # Optional path to a CA certificate bundle to be used to
 # validate the SSL certificate served by glance. It is used
 # when glance_api_insecure is set to False. (string value)

ironic/common/image_service.py

@@ -45,8 +45,11 @@ def _get_glance_session():
     return _GLANCE_SESSION
 
 
-def GlanceImageService(client=None, version=1, context=None):
+def GlanceImageService(client=None, version=None, context=None):
     module_str = 'ironic.common.glance_service'
+    if version is None:
+        version = CONF.glance.glance_api_version
+
     module = importutils.import_versioned_module(module_str, version,
                                                  'image_service')
     service_class = getattr(module, 'GlanceImageService')
@@ -261,7 +264,7 @@ protocol_mapping = {
 }
 
 
-def get_image_service(image_href, client=None, version=1, context=None):
+def get_image_service(image_href, client=None, version=None, context=None):
     """Get image service instance to download the image.
 
     :param image_href: String containing href to get image service for.

ironic/conf/glance.py

@@ -141,6 +141,9 @@ opts = [
                help=_('Optional path to a CA certificate bundle to be used to '
                       'validate the SSL certificate served by glance. It is '
                       'used when glance_api_insecure is set to False.')),
+    cfg.IntOpt('glance_api_version',
+               help=_('Glance API version (1 or 2) to use.'),
+               min=1, max=2, default=2),
 ]
 
 

ironic/drivers/modules/pxe.py

@@ -96,7 +96,8 @@ def _get_instance_image_info(node, ctx):
     labels = ('kernel', 'ramdisk')
     d_info = deploy_utils.get_image_instance_info(node)
     if not (i_info.get('kernel') and i_info.get('ramdisk')):
-        glance_service = service.GlanceImageService(version=1, context=ctx)
+        glance_service = service.GlanceImageService(
+            version=CONF.glance.glance_api_version, context=ctx)
         iproperties = glance_service.show(d_info['image_source'])['properties']
         for label in labels:
             i_info[label] = str(iproperties[label + '_id'])

ironic/tests/unit/common/test_image_service.py

@@ -24,6 +24,7 @@ from six.moves import http_client
 
 from ironic.common import exception
 from ironic.common.glance_service.v1 import image_service as glance_v1_service
+from ironic.common.glance_service.v2 import image_service as glance_v2_service
 from ironic.common import image_service
 from ironic.tests import base
 
@@ -265,31 +266,44 @@ class FileImageServiceTestCase(base.TestCase):
 class ServiceGetterTestCase(base.TestCase):
 
     @mock.patch.object(image_service, '_get_glance_session')
-    @mock.patch.object(glance_v1_service.GlanceImageService, '__init__',
+    @mock.patch.object(glance_v2_service.GlanceImageService, '__init__',
                        return_value=None, autospec=True)
     def test_get_glance_image_service(self, glance_service_mock,
                                       session_mock):
         image_href = 'image-uuid'
         self.context.auth_token = 'fake'
         image_service.get_image_service(image_href, context=self.context)
+        glance_service_mock.assert_called_once_with(mock.ANY, None, 2,
+                                                    self.context)
+        self.assertFalse(session_mock.called)
+
+    @mock.patch.object(image_service, '_get_glance_session')
+    @mock.patch.object(glance_v1_service.GlanceImageService, '__init__',
+                       return_value=None, autospec=True)
+    def test_get_glance_image_service_default_v1(self, glance_service_mock,
+                                                 session_mock):
+        self.config(glance_api_version=1, group='glance')
+        image_href = 'image-uuid'
+        self.context.auth_token = 'fake'
+        image_service.get_image_service(image_href, context=self.context)
         glance_service_mock.assert_called_once_with(mock.ANY, None, 1,
                                                     self.context)
         self.assertFalse(session_mock.called)
 
     @mock.patch.object(image_service, '_get_glance_session')
-    @mock.patch.object(glance_v1_service.GlanceImageService, '__init__',
+    @mock.patch.object(glance_v2_service.GlanceImageService, '__init__',
                        return_value=None, autospec=True)
     def test_get_glance_image_service_url(self, glance_service_mock,
                                           session_mock):
         image_href = 'glance://image-uuid'
         self.context.auth_token = 'fake'
         image_service.get_image_service(image_href, context=self.context)
-        glance_service_mock.assert_called_once_with(mock.ANY, None, 1,
+        glance_service_mock.assert_called_once_with(mock.ANY, None, 2,
                                                     self.context)
         self.assertFalse(session_mock.called)
 
     @mock.patch.object(image_service, '_get_glance_session')
-    @mock.patch.object(glance_v1_service.GlanceImageService, '__init__',
+    @mock.patch.object(glance_v2_service.GlanceImageService, '__init__',
                        return_value=None, autospec=True)
     def test_get_glance_image_service_no_token(self, glance_service_mock,
                                                session_mock):
@@ -299,13 +313,13 @@ class ServiceGetterTestCase(base.TestCase):
         sess.get_token.return_value = 'admin-token'
         session_mock.return_value = sess
         image_service.get_image_service(image_href, context=self.context)
-        glance_service_mock.assert_called_once_with(mock.ANY, None, 1,
+        glance_service_mock.assert_called_once_with(mock.ANY, None, 2,
                                                     self.context)
         sess.get_token.assert_called_once_with()
         self.assertEqual('admin-token', self.context.auth_token)
 
     @mock.patch.object(image_service, '_get_glance_session')
-    @mock.patch.object(glance_v1_service.GlanceImageService, '__init__',
+    @mock.patch.object(glance_v2_service.GlanceImageService, '__init__',
                        return_value=None, autospec=True)
     def test_get_glance_image_service_token_not_needed(self,
                                                        glance_service_mock,
@@ -314,7 +328,7 @@ class ServiceGetterTestCase(base.TestCase):
         self.context.auth_token = None
         self.config(auth_strategy='noauth', group='glance')
         image_service.get_image_service(image_href, context=self.context)
-        glance_service_mock.assert_called_once_with(mock.ANY, None, 1,
+        glance_service_mock.assert_called_once_with(mock.ANY, None, 2,
                                                     self.context)
         self.assertFalse(session_mock.called)
         self.assertIsNone(self.context.auth_token)

releasenotes/notes/glance-v2-83b04fec247cd22f.yaml

@@ -0,0 +1,8 @@
+---
+upgrade:
+  - |
+    Ironic now uses only Image (glance) V2 API by default. Usage of deprecated
+    V1 API for certain basic tasks can still be enabled by setting
+    "[glance]glance_api_version" to "1". This option, however, does not affect
+    temporary URL generation, as it always required V2 API and cannot work
+    with V1.