openstack
/
cinder
Code Issues Proposed changes

Update attachment_delete for multiple attachments 

- change attachment_delete to not remove the export if multiple
attachments exist

Also remove the unused return of has_shared_connections in
attachment_delete method.  It's not used or needed in this
method.

Closes-Bug: 1697008

Change-Id: I62b90c4cd4ad2bfb8a68718b43dc606e02fe776a
This commit is contained in:
Steve Noyes 2017-06-09 11:14:31 -04:00 committed by John Griffith
parent 54affb4437
commit 3451891531
3 changed files with 72 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

54
cinder/tests/unit/attachments/test_attachments_manager.py
View File

@ -18,6 +18,7 @@ from cinder import context
from cinder import db
from cinder import exception
from cinder.objects import fields
from cinder.objects import volume_attachment
from cinder import test
from cinder.tests.unit import fake_constants as fake
from cinder.tests.unit import utils as tests_utils
@ -143,3 +144,56 @@ class AttachmentManagerTestCase(test.TestCase):
db.volume_attachment_get,
self.context,
attachment_ref.id)
def test_attachment_delete_multiple_attachments(self):
volume_params = {'status': 'available'}
vref = tests_utils.create_volume(self.context, **volume_params)
attachment1 = volume_attachment.VolumeAttachment()
attachment2 = volume_attachment.VolumeAttachment()
attachment1.id = fake.UUID1
attachment2.id = fake.UUID2
@mock.patch.object(self.manager.db, 'volume_admin_metadata_delete')
@mock.patch.object(self.manager.db, 'volume_detached')
@mock.patch.object(self.context, 'elevated')
@mock.patch.object(self.manager, '_connection_terminate')
@mock.patch.object(self.manager.driver, 'remove_export')
@mock.patch.object(self.manager.driver, 'detach_volume')
def _test(mock_detach, mock_rm_export, mock_con_term,
mock_elevated, mock_db_detached, mock_db_meta_delete):
mock_elevated.return_value = self.context
mock_con_term.return_value = False
# test single attachment. This should call
# detach and remove_export
vref.volume_attachment.objects.append(attachment1)
self.manager._do_attachment_delete(self.context, vref, attachment1)
mock_detach.assert_called_once_with(self.context, vref,
attachment1)
mock_db_detached.called_once_with(self.context, vref,
attachment1.id)
mock_db_meta_delete.called_once_with(self.context, vref.id,
'attached_mode')
mock_rm_export.assert_called_once_with(self.context, vref)
# test more than 1 attachment. This should skip
# detach and remove_export
mock_con_term.return_value = True
vref.volume_attachment.objects.append(attachment2)
mock_detach.reset_mock()
mock_rm_export.reset_mock()
mock_db_detached.reset_mock()
mock_db_meta_delete.reset_mock()
self.manager._do_attachment_delete(self.context, vref, attachment2)
mock_rm_export.assert_not_called()
mock_db_detached.called_once_with(self.context, vref,
attachment2.id)
mock_db_meta_delete.called_once_with(self.context, vref.id,
'attached_mode')
_test()

16
cinder/volume/drivers/lvm.py
View File

@ -801,5 +801,17 @@ class LVMVolumeDriver(driver.VolumeDriver):
return self.target_driver.validate_connector(connector)
def terminate_connection(self, volume, connector, **kwargs):
return self.target_driver.terminate_connection(volume, connector,
**kwargs)
# NOTE(jdg): LVM has a single export for each volume, so what
# we need to do here is check if there is more than one attachment for
# the volume, if there is; let the caller know that they should NOT
# remove the export.
has_shared_connections = False
if len(volume.volume_attachment) > 1:
has_shared_connections = True
# NOTE(jdg): For the TGT driver this is a noop, for LIO this removes
# the initiator IQN from the targets access list, so we're good
self.target_driver.terminate_connection(volume, connector,
**kwargs)
return has_shared_connections

12
cinder/volume/manager.py
View File

@ -4184,17 +4184,13 @@ class VolumeManager(manager.CleanableManager,
NOTE if the attachment reference is None, we remove all existing
attachments for the specified volume object.
"""
has_shared_connection = False
attachment_ref = objects.VolumeAttachment.get_by_id(context,
attachment_id)
if not attachment_ref:
for attachment in VA_LIST.get_all_by_volume_id(context, vref.id):
if self._do_attachment_delete(context, vref, attachment):
has_shared_connection = True
self._do_attachment_delete(context, vref, attachment)
else:
has_shared_connection = (
self._do_attachment_delete(context, vref, attachment_ref))
return has_shared_connection
self._do_attachment_delete(context, vref, attachment_ref)
def _do_attachment_delete(self, context, vref, attachment):
utils.require_driver_initialized(self.driver)
@ -4207,7 +4203,8 @@ class VolumeManager(manager.CleanableManager,
{'attachment_id': attachment.id},
resource=vref)
self.driver.detach_volume(context, vref, attachment)
self.driver.remove_export(context.elevated(), vref)
if not has_shared_connection:
self.driver.remove_export(context.elevated(), vref)
except Exception:
# FIXME(jdg): Obviously our volume object is going to need some
# changes to deal with multi-attach and figuring out how to
@ -4224,7 +4221,6 @@ class VolumeManager(manager.CleanableManager,
vref.id,
'attached_mode')
self._notify_about_volume_usage(context, vref, "detach.end")
return has_shared_connection
# Replication group API (Tiramisu)
def enable_replication(self, ctxt, group):