From 7f85be418669286392c833479dfdc7c2a33f86d1 Mon Sep 17 00:00:00 2001
From: whoami-rajat <rajatdhasmana@gmail.com>
Date: Thu, 27 Sep 2018 16:45:34 +0530
Subject: [PATCH] Fix multiattach set to false after retype

The return value of get_by_name_or_id in the volume_types module
returns a dictionary.
During the function call to _is_multiattach, the getattr() returns
{}(default value) because a dictionary is passed instead of an object.
After importing the cinder.objects.volume_type module,
the current function call to get_by_name_or_id() returns an object
hence getattr() will return the appropriate extra_specs instead of
the default value({}).

Closes-Bug: 1790840
Change-Id: I869bc0c9b18887da1ea83f855d255557f0f3cba0
---
 .../tests/unit/volume/test_volume_retype.py   | 36 ++++++++++++++++---
 cinder/volume/api.py                          |  4 ++-
 2 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/cinder/tests/unit/volume/test_volume_retype.py b/cinder/tests/unit/volume/test_volume_retype.py
index 3a0e6ee1f3a..195f2a860f8 100644
--- a/cinder/tests/unit/volume/test_volume_retype.py
+++ b/cinder/tests/unit/volume/test_volume_retype.py
@@ -21,6 +21,7 @@ from cinder.policies import volume_actions as vol_action_policies
 from cinder.policies import volumes as volume_policies
 from cinder import quota
 from cinder.tests.unit import fake_constants as fake
+from cinder.tests.unit import utils as tests_utils
 from cinder.tests.unit import volume as base
 from cinder.volume import volume_types
 
@@ -40,10 +41,6 @@ class VolumeRetypeTestCase(base.BaseVolumeTestCase):
         self.user_context = context.RequestContext(user_id=fake.USER_ID,
                                                    project_id=fake.PROJECT_ID)
 
-        volume_types.create(self.context,
-                            "old-type",
-                            {},
-                            description="test-multiattach")
         volume_types.create(self.context,
                             "fake_vol_type",
                             {},
@@ -52,16 +49,25 @@ class VolumeRetypeTestCase(base.BaseVolumeTestCase):
                             "multiattach-type",
                             {'multiattach': "<is> True"},
                             description="test-multiattach")
+        volume_types.create(self.context,
+                            "multiattach-type2",
+                            {'multiattach': "<is> True"},
+                            description="test-multiattach")
         self.default_vol_type = objects.VolumeType.get_by_name_or_id(
             self.context,
             'fake_vol_type')
         self.multiattach_type = objects.VolumeType.get_by_name_or_id(
             self.context,
             'multiattach-type')
+        self.multiattach_type2 = objects.VolumeType.get_by_name_or_id(
+            self.context,
+            'multiattach-type2')
 
     def fake_get_vtype(self, context, identifier):
         if identifier == "multiattach-type":
             return self.multiattach_type
+        elif identifier == 'multiattach-type2':
+            return self.multiattach_type2
         else:
             return self.default_vol_type
 
@@ -97,7 +103,7 @@ class VolumeRetypeTestCase(base.BaseVolumeTestCase):
         vol.save()
         self.volume_api.retype(self.user_context,
                                vol,
-                               'fake_vol-type')
+                               'fake_vol_type')
         vol = objects.Volume.get_by_id(self.context, vol.id)
         self.assertFalse(vol.multiattach)
 
@@ -120,3 +126,23 @@ class VolumeRetypeTestCase(base.BaseVolumeTestCase):
              mock.call(vol_action_policies.RETYPE_POLICY, target_obj=mock.ANY),
              mock.call(vol_action_policies.RETYPE_POLICY, target_obj=mock.ANY),
              ])
+
+    @mock.patch('cinder.context.RequestContext.authorize')
+    def test_multiattach_to_multiattach_retype(self, mock_authorize):
+        # Test going from multiattach to multiattach
+
+        vol = tests_utils.create_volume(self.context,
+                                        multiattach=True,
+                                        volume_type_id=
+                                        self.multiattach_type.id)
+
+        self.assertTrue(vol.multiattach)
+        self.volume_api.retype(self.user_context,
+                               vol,
+                               'multiattach-type2')
+        vol.refresh()
+        self.assertTrue(vol.multiattach)
+
+        mock_authorize.assert_has_calls(
+            [mock.call(vol_action_policies.RETYPE_POLICY, target_obj=mock.ANY)
+             ])
diff --git a/cinder/volume/api.py b/cinder/volume/api.py
index cc01c5b6084..f6c6bf93ae1 100644
--- a/cinder/volume/api.py
+++ b/cinder/volume/api.py
@@ -44,6 +44,7 @@ from cinder.message import message_field
 from cinder import objects
 from cinder.objects import base as objects_base
 from cinder.objects import fields
+from cinder.objects import volume_type
 from cinder.policies import attachments as attachment_policy
 from cinder.policies import services as svr_policy
 from cinder.policies import snapshot_metadata as s_meta_policy
@@ -1603,7 +1604,8 @@ class API(base.Base):
         # Support specifying volume type by ID or name
         try:
             new_type = (
-                volume_types.get_by_name_or_id(context.elevated(), new_type))
+                volume_type.VolumeType.get_by_name_or_id(context.elevated(),
+                                                         new_type))
         except exception.InvalidVolumeType:
             msg = _('Invalid volume_type passed: %s.') % new_type
             LOG.error(msg)