diff --git a/cinder/brick/iscsi/iscsi.py b/cinder/brick/iscsi/iscsi.py index cb042ba832a..236e4d0a8fd 100644 --- a/cinder/brick/iscsi/iscsi.py +++ b/cinder/brick/iscsi/iscsi.py @@ -53,10 +53,6 @@ class TargetAdmin(executor.Executor): """Create a iSCSI target and logical unit.""" raise NotImplementedError() - def update_iscsi_target(self, name): - """Update an iSCSI target.""" - raise NotImplementedError() - def remove_iscsi_target(self, tid, lun, vol_id, vol_name, **kwargs): """Remove a iSCSI target and logical unit.""" raise NotImplementedError() @@ -368,9 +364,6 @@ class IetAdm(TargetAdmin): raise exception.ISCSITargetCreateFailed(volume_id=vol_id) return tid - def update_iscsi_target(self, name): - pass - def remove_iscsi_target(self, tid, lun, vol_id, vol_name, **kwargs): LOG.info(_('Removing iscsi_target for volume: %s') % vol_id) self._delete_logicalunit(tid, lun, **kwargs) @@ -525,9 +518,6 @@ class LioAdm(TargetAdmin): return tid - def update_iscsi_target(self, name): - pass - def remove_iscsi_target(self, tid, lun, vol_id, vol_name, **kwargs): LOG.info(_('Removing iscsi_target: %s') % vol_id) vol_uuid_name = vol_name diff --git a/cinder/exception.py b/cinder/exception.py index dfbb0524698..6e7c4f10afd 100644 --- a/cinder/exception.py +++ b/cinder/exception.py @@ -660,3 +660,7 @@ class GlusterfsNoSharesMounted(VolumeDriverException): class GlusterfsNoSuitableShareFound(VolumeDriverException): message = _("There is no share which can host %(volume_size)sG") + + +class RemoveExportException(VolumeDriverException): + message = _("Failed to remove export for volume %(volume)s: %(reason)s") diff --git a/cinder/tests/test_volume.py b/cinder/tests/test_volume.py index 476a9f3ff62..9f7aa760ac3 100644 --- a/cinder/tests/test_volume.py +++ b/cinder/tests/test_volume.py @@ -931,15 +931,25 @@ class VolumeTestCase(BaseVolumeTestCase): image_id='fake_id', source_volume='fake_id') - @mock.patch.object(db, 'volume_get') @mock.patch.object(db, 'volume_admin_metadata_get') + @mock.patch.object(db, 'volume_get') + @mock.patch.object(db, 'volume_update') def test_initialize_connection_fetchqos(self, - _mock_volume_admin_metadata_get, - _mock_volume_get): + _mock_volume_update, + _mock_volume_get, + _mock_volume_admin_metadata_get): """Make sure initialize_connection returns correct information.""" - _mock_volume_get.return_value = {'volume_type_id': 'fake_type_id', - 'volume_admin_metadata': {}} - _mock_volume_admin_metadata_get.return_value = {} + _fake_admin_meta = {'fake-key': 'fake-value'} + _fake_volume = {'volume_type_id': 'fake_type_id', + 'name': 'fake_name', + 'host': 'fake_host', + 'id': 'fake_volume_id', + 'volume_admin_metadata': _fake_admin_meta} + + _mock_volume_get.return_value = _fake_volume + _mock_volume_update.return_value = _fake_volume + _mock_volume_admin_metadata_get.return_value = _fake_admin_meta + connector = {'ip': 'IP', 'initiator': 'INITIATOR'} qos_values = {'consumer': 'front-end', 'specs': { diff --git a/cinder/volume/driver.py b/cinder/volume/driver.py index 50987551228..3bcc59b5829 100644 --- a/cinder/volume/driver.py +++ b/cinder/volume/driver.py @@ -373,8 +373,10 @@ class VolumeDriver(object): """Attach the volume.""" if remote: rpcapi = volume_rpcapi.VolumeAPI() + rpcapi.create_export(context, volume) conn = rpcapi.initialize_connection(context, volume, properties) else: + self.create_export(context, volume) conn = self.initialize_connection(volume, properties) # Use Brick's code to do attach/detach diff --git a/cinder/volume/drivers/lvm.py b/cinder/volume/drivers/lvm.py index c1c2b4edece..fb13a588db5 100644 --- a/cinder/volume/drivers/lvm.py +++ b/cinder/volume/drivers/lvm.py @@ -669,6 +669,7 @@ class LVMISCSIDriver(LVMVolumeDriver, driver.ISCSIDriver): LOG.info(_("Skipping remove_export. No iscsi_target " "provisioned for volume: %s"), volume['id']) return + else: iscsi_target = 0 diff --git a/cinder/volume/flows/manager/create_volume.py b/cinder/volume/flows/manager/create_volume.py index 080c16fbb37..30b7f2e1029 100644 --- a/cinder/volume/flows/manager/create_volume.py +++ b/cinder/volume/flows/manager/create_volume.py @@ -613,28 +613,6 @@ class CreateVolumeFromSpecTask(flow_utils.CinderTask): {'volume_id': volume_id, 'model': model_update}) raise exception.ExportFailure(reason=ex) - # Persist any driver exported model information. - model_update = None - try: - LOG.debug(_("Volume %s: creating export"), volume_ref['id']) - model_update = self.driver.create_export(context, volume_ref) - if model_update: - self.db.volume_update(context, volume_ref['id'], model_update) - except exception.CinderException as ex: - # If somehow the read *or* create export failed we want to ensure - # that the failure is logged (but not try rescheduling since - # the volume at this point has been created). - # - # NOTE(harlowja): Notice that since the model_update is initially - # empty, the only way it will still be empty is if there is no - # model_update (which we don't care about) or there was an - # model_update and updating failed. - if model_update: - LOG.exception(_("Failed updating model of volume %(volume_id)s" - " with driver provided model %(model)s") % - {'volume_id': volume_id, 'model': model_update}) - raise exception.ExportFailure(reason=ex) - return volume_ref diff --git a/cinder/volume/manager.py b/cinder/volume/manager.py index 100cc422c92..0b4e7a9d37b 100644 --- a/cinder/volume/manager.py +++ b/cinder/volume/manager.py @@ -182,7 +182,7 @@ def locked_snapshot_operation(f): class VolumeManager(manager.SchedulerDependentManager): """Manages attachable block storage devices.""" - RPC_API_VERSION = '1.12' + RPC_API_VERSION = '1.13' def __init__(self, volume_driver=None, service_name=None, *args, **kwargs): @@ -247,7 +247,7 @@ class VolumeManager(manager.SchedulerDependentManager): sum = 0 self.stats.update({'allocated_capacity_gb': sum}) for volume in volumes: - if volume['status'] in ['available', 'in-use']: + if volume['status'] in ['in-use']: # calculate allocated capacity for driver sum += volume['size'] self.stats['allocated_capacity_gb'] = sum @@ -393,7 +393,6 @@ class VolumeManager(manager.SchedulerDependentManager): except exception.VolumeIsBusy: LOG.error(_("Cannot delete volume %s: volume is busy"), volume_ref['id']) - self.driver.ensure_export(context, volume_ref) self.db.volume_update(context, volume_ref['id'], {'status': 'available'}) return True @@ -641,7 +640,6 @@ class VolumeManager(manager.SchedulerDependentManager): def detach_volume(self, context, volume_id): """Updates db to show volume is detached.""" # TODO(vish): refactor this into a more general "unreserve" - # TODO(sleepsonthefloor): Is this 'elevated' appropriate? volume = self.db.volume_get(context, volume_id) self._notify_about_volume_usage(context, volume, "detach.start") @@ -662,11 +660,29 @@ class VolumeManager(manager.SchedulerDependentManager): self.db.volume_admin_metadata_delete(context.elevated(), volume_id, 'attached_mode') - # Check for https://bugs.launchpad.net/cinder/+bug/1065702 + # NOTE(jdg): We used to do an ensure export here to + # catch upgrades while volumes were attached (E->F) + # this was necessary to convert in-use volumes from + # int ID's to UUID's. Don't need this any longer + + # We're going to remove the export here + # (delete the iscsi target) volume = self.db.volume_get(context, volume_id) - if (volume['provider_location'] and - volume['name'] not in volume['provider_location']): - self.driver.ensure_export(context, volume) + try: + utils.require_driver_initialized(self.driver) + LOG.debug(_("volume %s: removing export"), volume_id) + self.driver.remove_export(context, volume) + except exception.DriverNotInitialized as ex: + with excutils.save_and_reraise_exception(): + LOG.exception(_("Error detaching volume %(volume)s, " + "due to uninitialized driver."), + {"volume": volume_id}) + except Exception as ex: + LOG.exception(_("Error detaching volume %(volume)s, " + "due to remove export failure."), + {"volume": volume_id}) + raise exception.RemoveExportException(volume=volume_id, reason=ex) + self._notify_about_volume_usage(context, volume, "detach.end") def copy_volume_to_image(self, context, volume_id, image_meta): @@ -684,7 +700,6 @@ class VolumeManager(manager.SchedulerDependentManager): utils.require_driver_initialized(self.driver) volume = self.db.volume_get(context, volume_id) - self.driver.ensure_export(context.elevated(), volume) image_service, image_id = \ glance.get_remote_image_service(context, image_meta['id']) self.driver.copy_volume_to_image(context, volume, image_service, @@ -745,12 +760,34 @@ class VolumeManager(manager.SchedulerDependentManager): # before going forward. The exception will be caught # and the volume status updated. utils.require_driver_initialized(self.driver) + try: + self.driver.validate_connector(connector) + except Exception as err: + err_msg = (_('Unable to fetch connection information from ' + 'backend: %(err)s') % {'err': str(err)}) + LOG.error(err_msg) + raise exception.VolumeBackendAPIException(data=err_msg) volume = self.db.volume_get(context, volume_id) - self.driver.validate_connector(connector) + model_update = None + try: + LOG.debug(_("Volume %s: creating export"), volume_id) + model_update = self.driver.create_export(context, volume) + if model_update: + volume = self.db.volume_update(context, + volume_id, + model_update) + except exception.CinderException as ex: + if model_update: + LOG.exception(_("Failed updating model of volume %(volume_id)s" + " with driver provided model %(model)s") % + {'volume_id': volume_id, 'model': model_update}) + raise exception.ExportFailure(reason=ex) + try: conn_info = self.driver.initialize_connection(volume, connector) except Exception as err: + self.driver.remove_export(context, volume) err_msg = (_('Unable to fetch connection information from ' 'backend: %(err)s') % {'err': str(err)}) LOG.error(err_msg) diff --git a/cinder/volume/rpcapi.py b/cinder/volume/rpcapi.py index aaca9a1fbf0..01011527d69 100644 --- a/cinder/volume/rpcapi.py +++ b/cinder/volume/rpcapi.py @@ -47,6 +47,7 @@ class VolumeAPI(cinder.openstack.common.rpc.proxy.RpcProxy): 1.11 - Adds mode parameter to attach_volume() to support volume read-only attaching. 1.12 - Adds retype. + 1.13 - Adds create_export. ''' BASE_RPC_API_VERSION = '1.0' @@ -195,3 +196,11 @@ class VolumeAPI(cinder.openstack.common.rpc.proxy.RpcProxy): reservations=reservations), topic=rpc.queue_get_for(ctxt, self.topic, volume['host']), version='1.12') + + def create_export(self, ctxt, volume): + return self.call(ctxt, self.make_msg('create_export', + volume_id=volume['id']), + topic=rpc.queue_get_for(ctxt, + self.topic, + volume['host']), + version='1.13')