From b285cac66335b676c6c0f89e0badd5617d2e49a4 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Micha=C5=82=20Dulko?= Date: Wed, 16 Dec 2015 13:08:01 +0100 Subject: [PATCH] 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 --- .../volume/flows/test_manage_volume_flow.py | 31 ++++--------------- cinder/volume/flows/api/manage_existing.py | 1 + 2 files changed, 7 insertions(+), 25 deletions(-) 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 be32af933..c14220994 100644 --- 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 6b9017bd8..54036f036 100644 --- 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 -- 2.45.2