openstack
/
cinder
Code Issues Proposed changes

Merge "Don't call driver.terminate_connection if there is no connector"

This commit is contained in:
Zuul 2017-12-15 18:50:31 +00:00 committed by Gerrit Code Review
parent 1a7141c462 1bcab6a48d
commit 4464a50031
2 changed files with 68 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -197,3 +197,54 @@ class AttachmentManagerTestCase(test.TestCase):
mock_db_meta_delete.called_once_with(self.context, vref.id,
'attached_mode')
_test()
def test_connection_terminate_no_connector_force_false(self):
# Tests that calling _connection_terminate with an attachment that
# does not have a connector will not call the driver and return None
# if the force flag is False.
attachment = mock.MagicMock(connector={})
with mock.patch.object(self.manager.driver, '_initialized',
create=True, new=True):
with mock.patch.object(self.manager.driver,
'terminate_connection') as term_conn:
has_shared_connection = self.manager._connection_terminate(
self.context, mock.sentinel.volume, attachment)
self.assertIsNone(has_shared_connection)
term_conn.assert_not_called()
def test_connection_terminate_no_connector_force_true(self):
# Tests that calling _connection_terminate with an attachment that
# does not have a connector will call the driver when force is True.
volume = mock.MagicMock()
attachment = mock.MagicMock(connector={})
with mock.patch.object(self.manager.driver, '_initialized',
create=True, new=True):
with mock.patch.object(self.manager.driver,
'terminate_connection') as term_conn:
has_shared_connection = self.manager._connection_terminate(
self.context, volume, attachment, force=True)
self.assertFalse(has_shared_connection)
term_conn.assert_called_once_with(volume, {}, force=True)
@mock.patch('cinder.volume.manager.VolumeManager.'
'_notify_about_volume_usage')
@mock.patch('cinder.volume.manager.VolumeManager._connection_terminate',
return_value=None)
@mock.patch('cinder.db.volume_detached')
@mock.patch('cinder.db.volume_admin_metadata_delete')
def test_do_attachment_delete_none_shared_connection(self, mock_meta_del,
mock_vol_detached,
mock_conn_term,
mock_notify):
# Tests that _do_attachment_delete does not call remove_export
# if _connection_terminate returns None indicating there is nothing
# to consider for the export.
volume = mock.MagicMock()
attachment = mock.MagicMock()
with mock.patch.object(self.manager.driver, '_initialized',
create=True, new=True):
with mock.patch.object(self.manager.driver,
'remove_export') as remove_export:
self.manager._do_attachment_delete(
self.context, volume, attachment)
remove_export.assert_not_called()

19
cinder/volume/manager.py
View File

@ -4421,9 +4421,24 @@ class VolumeManager(manager.CleanableManager,
def _connection_terminate(self, context, volume,
attachment, force=False):
"""Remove a volume connection, but leave attachment."""
"""Remove a volume connection, but leave attachment.
Exits early if the attachment does not have a connector and returns
None to indicate shared connections are irrelevant.
"""
utils.require_driver_initialized(self.driver)
connector = attachment.connector
if not connector and not force:
# It's possible to attach a volume to a shelved offloaded server
# in nova, and a shelved offloaded server is not on a compute host,
# which means the attachment was made without a host connector,
# so if we don't have a connector we can't terminate a connection
# that was never actually made to the storage backend, so just
# log a message and exit.
LOG.debug('No connector for attachment %s; skipping storage '
'backend terminate_connection call.', attachment.id)
# None indicates we don't know and don't care.
return None
try:
shared_connections = self.driver.terminate_connection(volume,
connector,
@ -4476,7 +4491,7 @@ class VolumeManager(manager.CleanableManager,
{'attachment_id': attachment.id},
resource=vref)
self.driver.detach_volume(context, vref, attachment)
if not has_shared_connection:
if has_shared_connection is not None and not has_shared_connection:
self.driver.remove_export(context.elevated(), vref)
except Exception:
# FIXME(jdg): Obviously our volume object is going to need some