diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 5b61e4f75446..d63a7a8e692a 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -4766,7 +4766,7 @@ class ComputeManager(manager.Manager): self._notify_about_instance_usage( context, instance, "volume.attach", extra_usage_info=info) - def _driver_detach_volume(self, context, instance, bdm): + def _driver_detach_volume(self, context, instance, bdm, connection_info): """Do the actual driver detach using block device mapping.""" mp = bdm.device_name volume_id = bdm.volume_id @@ -4775,11 +4775,6 @@ class ComputeManager(manager.Manager): {'volume_id': volume_id, 'mp': mp}, context=context, instance=instance) - connection_info = jsonutils.loads(bdm.connection_info) - # NOTE(vish): We currently don't use the serial when disconnecting, - # but added for completeness in case we ever do. - if connection_info and 'serial' not in connection_info: - connection_info['serial'] = volume_id try: if not self.driver.instance_exists(instance): LOG.warning(_LW('Detaching volume from unknown instance'), @@ -4805,8 +4800,6 @@ class ComputeManager(manager.Manager): context=context, instance=instance) self.volume_api.roll_detaching(context, volume_id) - return connection_info - def _detach_volume(self, context, volume_id, instance, destroy_bdm=True, attachment_id=None): """Detach a volume from an instance. @@ -4851,8 +4844,21 @@ class ComputeManager(manager.Manager): self.notifier.info(context, 'volume.usage', compute_utils.usage_volume_info(vol_usage)) - connection_info = self._driver_detach_volume(context, instance, bdm) + connection_info = jsonutils.loads(bdm.connection_info) connector = self.driver.get_volume_connector(instance) + if CONF.host == instance.host: + # Only attempt to detach and disconnect from the volume if the + # instance is currently associated with the local compute host. + self._driver_detach_volume(context, instance, bdm, connection_info) + elif not destroy_bdm: + LOG.debug("Skipping _driver_detach_volume during remote rebuild.", + instance=instance) + elif destroy_bdm: + LOG.error(_LE("Unable to call for a driver detach of volume " + "%(vol_id)s due to the instance being registered to " + "the remote host %(inst_host)s."), + {'vol_id': volume_id, 'inst_host': instance.host}, + instance=instance) if connection_info and not destroy_bdm and ( connector.get('host') != instance.host): @@ -5041,7 +5047,8 @@ class ComputeManager(manager.Manager): try: bdm = objects.BlockDeviceMapping.get_by_volume_and_instance( context, volume_id, instance.uuid) - self._driver_detach_volume(context, instance, bdm) + connection_info = jsonutils.loads(bdm.connection_info) + self._driver_detach_volume(context, instance, bdm, connection_info) connector = self.driver.get_volume_connector(instance) self.volume_api.terminate_connection(context, volume_id, connector) except exception.NotFound: diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index 035859515344..d2023a944d68 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -363,7 +363,8 @@ class ComputeVolumeTestCase(BaseTestCase): } self.fake_volume = fake_block_device.FakeDbBlockDeviceDict( {'source_type': 'volume', 'destination_type': 'volume', - 'volume_id': uuids.volume_id, 'device_name': '/dev/vdb'}) + 'volume_id': uuids.volume_id, 'device_name': '/dev/vdb', + 'connection_info': jsonutils.dumps({})}) self.instance_object = objects.Instance._from_db_object( self.context, objects.Instance(), fake_instance.fake_db_instance()) @@ -440,7 +441,7 @@ class ComputeVolumeTestCase(BaseTestCase): self.context, 'fake', instance, 'fake_id') mock_internal_detach.assert_called_once_with(self.context, instance, - fake_bdm) + fake_bdm, {}) self.assertTrue(mock_destroy.called) def test_await_block_device_created_too_slow(self): @@ -11552,8 +11553,10 @@ class EvacuateHostTestCase(BaseTestCase): instance = db.instance_get(self.context, self.inst.id) self.assertEqual(instance['host'], 'fake_host_2') - def test_rebuild_on_host_with_volumes(self): - """Confirm evacuate scenario reconnects volumes.""" + def test_rebuild_on_remote_host_with_volumes(self): + """Confirm that the evacuate scenario does not attempt a driver detach + when rebuilding an instance with volumes on a remote host + """ values = {'instance_uuid': self.inst.uuid, 'source_type': 'volume', 'device_name': '/dev/vdc', @@ -11574,11 +11577,6 @@ class EvacuateHostTestCase(BaseTestCase): result["detached"] = volume["id"] == 'fake_volume_id' self.stubs.Set(cinder.API, "detach", fake_detach) - self.mox.StubOutWithMock(self.compute, '_driver_detach_volume') - self.compute._driver_detach_volume(mox.IsA(self.context), - mox.IsA(instance_obj.Instance), - mox.IsA(objects.BlockDeviceMapping)) - def fake_terminate_connection(self, context, volume, connector): return {} self.stubs.Set(cinder.API, "terminate_connection", @@ -11598,6 +11596,7 @@ class EvacuateHostTestCase(BaseTestCase): self.mox.ReplayAll() self._rebuild() + self.mox.VerifyAll() # cleanup bdms = db.block_device_mapping_get_all_by_instance(self.context, diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index 473a993d3e68..d7ff03ceb3ba 100644 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -2170,6 +2170,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase): @mock.patch('nova.objects.Instance._from_db_object') def test_remove_volume_connection(self, inst_from_db, detach, bdm_get): bdm = mock.sentinel.bdm + bdm.connection_info = jsonutils.dumps({}) inst_obj = mock.Mock() inst_obj.uuid = 'uuid' bdm_get.return_value = bdm @@ -2177,7 +2178,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase): with mock.patch.object(self.compute, 'volume_api'): self.compute.remove_volume_connection(self.context, 'vol', inst_obj) - detach.assert_called_once_with(self.context, inst_obj, bdm) + detach.assert_called_once_with(self.context, inst_obj, bdm, {}) bdm_get.assert_called_once_with(self.context, 'vol', 'uuid') def test_detach_volume(self): @@ -2195,10 +2196,12 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase): volume_id = uuids.volume inst_obj = mock.Mock() inst_obj.uuid = uuids.instance + inst_obj.host = CONF.host attachment_id = uuids.attachment bdm = mock.MagicMock(spec=objects.BlockDeviceMapping) bdm.device_name = 'vdb' + bdm.connection_info = jsonutils.dumps({}) bdm_get.return_value = bdm detach.return_value = {} @@ -2213,7 +2216,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase): destroy_bdm=destroy_bdm, attachment_id=attachment_id) - detach.assert_called_once_with(self.context, inst_obj, bdm) + detach.assert_called_once_with(self.context, inst_obj, bdm, {}) driver.get_volume_connector.assert_called_once_with(inst_obj) volume_api.terminate_connection.assert_called_once_with( self.context, volume_id, connector_sentinel) @@ -2289,6 +2292,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase): instance, destroy_bdm=False) + driver._driver_detach_volume.assert_not_called() driver.get_volume_connector.assert_called_once_with(instance) volume_api.terminate_connection.assert_called_once_with( self.context, volume_id, expected_connector) @@ -2301,22 +2305,6 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase): extra_usage_info={'volume_id': volume_id} ) - def test__driver_detach_volume_return(self): - """_driver_detach_volume returns the connection_info from loads().""" - with mock.patch.object(jsonutils, 'loads') as loads: - conn_info_str = 'test-expected-loads-param' - bdm = mock.Mock() - bdm.connection_info = conn_info_str - loads.return_value = {'test-loads-key': 'test loads return value'} - instance = fake_instance.fake_instance_obj(self.context) - - ret = self.compute._driver_detach_volume(self.context, - instance, - bdm) - - self.assertEqual(loads.return_value, ret) - loads.assert_called_once_with(conn_info_str) - def _test_rescue(self, clean_shutdown=True): instance = fake_instance.fake_instance_obj( self.context, vm_state=vm_states.ACTIVE)