Pass volume_id in request_spec for manage_existing

In order to _set_volume_state_and_notify in scheduler.manager to work
correctly and error out volume on failure we need to pass volume_id in
the request_spec from c-api to c-sch. This commit implements that and
adds check for it in the unit tests for manage_existing flow.

Apart from that it removes unnecessary test that was setting volume to
MagicMock(return_value=None), which isn't actually testing anything
(volume() will return None, but we're never doing calls on volume
object).

Closes-Bug: 1526771
Change-Id: I93e16e281e952433870077309547e0ce09356a60

diff --git a/cinder/tests/unit/volume/flows/test_manage_volume_flow.py b/cinder/tests/unit/volume/flows/test_manage_volume_flow.py
index be32af93336e0ae621b2746254d5b348bc25151d..c14220994ea1e57759eb62b382e64cac59b80764 100644 (file)
--- a/cinder/tests/unit/volume/flows/test_manage_volume_flow.py
+++ b/cinder/tests/unit/volume/flows/test_manage_volume_flow.py
@@ -11,10 +11,9 @@
 #    under the License.
 """ Tests for manage_existing TaskFlow """
 
-import mock
-
 from cinder import context
 from cinder import test
+from cinder.tests.unit import fake_volume
 from cinder.tests.unit.volume.flows import fake_volume_api
 from cinder.volume.flows.api import manage_existing
 
@@ -27,28 +26,7 @@ class ManageVolumeFlowTestCase(test.TestCase):
         self.counter = float(0)
 
     def test_cast_manage_existing(self):
-
-        volume = mock.MagicMock(return_value=None)
-        spec = {
-            'name': 'name',
-            'description': 'description',
-            'host': 'host',
-            'ref': 'ref',
-            'volume_type': 'volume_type',
-            'metadata': 'metadata',
-            'availability_zone': 'availability_zone',
-            'bootable': 'bootable'}
-
-        # Fake objects assert specs
-        task = manage_existing.ManageCastTask(
-            fake_volume_api.FakeSchedulerRpcAPI(spec, self),
-            fake_volume_api.FakeDb())
-
-        create_what = spec.copy()
-        create_what.update({'volume': volume})
-        task.execute(self.ctxt, **create_what)
-
-        volume = mock.MagicMock(return_value={'id': 1})
+        volume = fake_volume.fake_volume_type_obj(self.ctxt)
 
         spec = {
             'name': 'name',
@@ -58,7 +36,9 @@ class ManageVolumeFlowTestCase(test.TestCase):
             'volume_type': 'volume_type',
             'metadata': 'metadata',
             'availability_zone': 'availability_zone',
-            'bootable': 'bootable'}
+            'bootable': 'bootable',
+            'volume_id': volume.id,
+        }
 
         # Fake objects assert specs
         task = manage_existing.ManageCastTask(
@@ -67,4 +47,5 @@ class ManageVolumeFlowTestCase(test.TestCase):
 
         create_what = spec.copy()
         create_what.update({'volume': volume})
+        create_what.pop('volume_id')
         task.execute(self.ctxt, **create_what)

diff --git a/cinder/volume/flows/api/manage_existing.py b/cinder/volume/flows/api/manage_existing.py
index 6b9017bd806dcca692e7bf4efdd1174142d8ea13..54036f03699bebe2375ff55919a36c9397a734da 100644 (file)
--- a/cinder/volume/flows/api/manage_existing.py
+++ b/cinder/volume/flows/api/manage_existing.py
@@ -112,6 +112,7 @@ class ManageCastTask(flow_utils.CinderTask):
     def execute(self, context, **kwargs):
         volume = kwargs.pop('volume')
         request_spec = kwargs.copy()
+        request_spec['volume_id'] = volume.id
 
         # Call the scheduler to ensure that the host exists and that it can
         # accept the volume