Add validation for volume_type of volume object

During copy_volume_to_image(), this tries to return
volume['volume_type'] attribute in a volume object, but
if the volume does not have a volume type, this would
trigger a db call which raise an 404 not found error.

Volume object should check volume_type_id at first and
then decide whether returning volume_type or None to
avoid VolumeTypeNotFound exception.

Also this change includes LOG.info messages fix to show
right behavior of copy_volume_to_image().

Change-Id: Id2372621962406282eece2bcbc56c242f9dff8a0
Closes-Bug: #1523362

diff --git a/cinder/objects/volume.py b/cinder/objects/volume.py
index a423f9191093a11bef6a6d22804129324cec43fe..70533c54d43465ccdb1310251d36c3a60047b43f 100644 (file)
--- a/cinder/objects/volume.py
+++ b/cinder/objects/volume.py
@@ -290,8 +290,11 @@ class Volume(base.CinderPersistentObject, base.CinderObject,
             self.glance_metadata = db.volume_glance_metadata_get(
                 self._context, self.id)
         elif attrname == 'volume_type':
-            self.volume_type = objects.VolumeType.get_by_id(
-                self._context, self.volume_type_id)
+            # If the volume doesn't have volume_type, VolumeType.get_by_id
+            # would trigger a db call which raise VolumeTypeNotFound exception.
+            self.volume_type = (objects.VolumeType.get_by_id(
+                self._context, self.volume_type_id) if self.volume_type_id
+                else None)
         elif attrname == 'volume_attachment':
             attachments = objects.VolumeAttachmentList.get_all_by_volume_id(
                 self._context, self.id)

diff --git a/cinder/tests/unit/api/contrib/test_volume_actions.py b/cinder/tests/unit/api/contrib/test_volume_actions.py
index 0a72048c6b9353f3877945217480e37baaab4dd2..c34da3b9bd01507f177ad9e48644e740183c4fcd 100644 (file)
--- a/cinder/tests/unit/api/contrib/test_volume_actions.py
+++ b/cinder/tests/unit/api/contrib/test_volume_actions.py
@@ -27,6 +27,7 @@ from cinder.api.contrib import volume_actions
 from cinder import context
 from cinder import exception
 from cinder.image import glance
+from cinder import objects
 from cinder import test
 from cinder.tests.unit.api import fakes
 from cinder.tests.unit.api.v2 import stubs
@@ -562,6 +563,7 @@ class VolumeImageActionsTest(test.TestCase):
         self.controller = volume_actions.VolumeActionsController()
 
         self.stubs.Set(volume_api.API, 'get', stub_volume_get)
+        self.context = context.RequestContext('fake', 'fake', is_admin=False)
 
     def _get_os_volume_upload_image(self):
         vol = {
@@ -967,3 +969,47 @@ class VolumeImageActionsTest(test.TestCase):
                     }
 
                     self.assertDictMatch(expected_res, res_dict)
+
+    @mock.patch.object(volume_api.API, "get_volume_image_metadata")
+    @mock.patch.object(glance.GlanceImageService, "create")
+    @mock.patch.object(volume_api.API, "get")
+    @mock.patch.object(volume_api.API, "update")
+    @mock.patch.object(volume_rpcapi.VolumeAPI, "copy_volume_to_image")
+    def test_copy_volume_to_image_volume_type_none(
+            self,
+            mock_copy_volume_to_image,
+            mock_update,
+            mock_get,
+            mock_create,
+            mock_get_volume_image_metadata):
+        """Test create image from volume with none type volume."""
+
+        db_volume = fake_volume.fake_db_volume()
+        volume_obj = objects.Volume._from_db_object(self.context,
+                                                    objects.Volume(),
+                                                    db_volume)
+
+        mock_create.side_effect = self.fake_image_service_create
+        mock_get.return_value = volume_obj
+        mock_copy_volume_to_image.side_effect = (
+            self.fake_rpc_copy_volume_to_image)
+
+        req = fakes.HTTPRequest.blank('/v2/tenant1/volumes/%s/action' % id)
+        body = self._get_os_volume_upload_image()
+        res_dict = self.controller._volume_upload_image(req, id, body)
+        expected_res = {
+            'os-volume_upload_image': {
+                'id': '1',
+                'updated_at': None,
+                'status': 'uploading',
+                'display_description': None,
+                'size': 1,
+                'volume_type': None,
+                'image_id': 1,
+                'container_format': u'bare',
+                'disk_format': u'raw',
+                'image_name': u'image_name'
+            }
+        }
+
+        self.assertDictMatch(expected_res, res_dict)

diff --git a/cinder/tests/unit/objects/test_volume.py b/cinder/tests/unit/objects/test_volume.py
index 2875af8b9e7b68b66a399ba42161b7ce6389f971..aa199e17a649819db15f03e49cf8ca59d5a7f21f 100644 (file)
--- a/cinder/tests/unit/objects/test_volume.py
+++ b/cinder/tests/unit/objects/test_volume.py
@@ -187,11 +187,19 @@ class TestVolume(test_objects.BaseObjectsTestCase):
             self.context, volume.id)
 
         # Test volume_type lazy-loaded field
-        volume_type = objects.VolumeType(context=self.context, id=5)
+        # Case1. volume.volume_type_id = None
+        self.assertIsNone(volume.volume_type)
+
+        # Case2. volume2.volume_type_id = 1
+        fake2 = fake_volume.fake_db_volume()
+        fake2.update({'volume_type_id': 1})
+        volume2 = objects.Volume._from_db_object(
+            self.context, objects.Volume(), fake2)
+        volume_type = objects.VolumeType(context=self.context, id=1)
         mock_vt_get_by_id.return_value = volume_type
-        self.assertEqual(volume_type, volume.volume_type)
+        self.assertEqual(volume_type, volume2.volume_type)
         mock_vt_get_by_id.assert_called_once_with(self.context,
-                                                  volume.volume_type_id)
+                                                  volume2.volume_type_id)
 
         # Test consistencygroup lazy-loaded field
         consistencygroup = objects.ConsistencyGroup(context=self.context, id=2)

diff --git a/cinder/volume/api.py b/cinder/volume/api.py
index ac01271a3ea7fcac94217b7be276fbf6f2dd3240..fa03788dfe86da9afadfc662da1ac0219b86402d 100644 (file)
--- a/cinder/volume/api.py
+++ b/cinder/volume/api.py
@@ -1217,7 +1217,7 @@ class API(base.Base):
                     "container_format": recv_metadata['container_format'],
                     "disk_format": recv_metadata['disk_format'],
                     "image_name": recv_metadata.get('name', None)}
-        LOG.info(_LI("Copy image to volume completed successfully."),
+        LOG.info(_LI("Copy volume to image completed successfully."),
                  resource=volume)
         return response