Provide correct connector for evacuate terminate
During evacuation a local connector is built. This is the wrong connector to use for cinder terminate_connection. In order to fix this, store the initial connector with the BDM connection_info. Use the stored connector when we detect that we have this wrong host situation. This fix does not work for existing attachments (made prior to this patch) because existing attachments don't have the connector stashed in the bdm.connection_info. In cases where the original connector was not saved, leave the behavior as-is. Change-Id: I793f2996fc0af1c321a240ad9348dc9bce816030 Partial-Bug: #1522496
This commit is contained in:
mark.sturdevant
John Garbutt
parent
38c3f883db
commit
a686185fc0
|
|
@ -4753,6 +4753,8 @@ 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.
|
||||
|
|
@ -4797,8 +4799,46 @@ class ComputeManager(manager.Manager):
|
|||
self.notifier.info(context, 'volume.usage',
|
||||
compute_utils.usage_volume_info(vol_usage))
|
||||
|
||||
self._driver_detach_volume(context, instance, bdm)
|
||||
connection_info = self._driver_detach_volume(context, instance, bdm)
|
||||
connector = self.driver.get_volume_connector(instance)
|
||||
|
||||
if connection_info and not destroy_bdm and (
|
||||
connector.get('host') != instance.host):
|
||||
# If the volume is attached to another host (evacuate) then
|
||||
# this connector is for the wrong host. Use the connector that
|
||||
# was stored in connection_info instead (if we have one, and it
|
||||
# is for the expected host).
|
||||
stashed_connector = connection_info.get('connector')
|
||||
if not stashed_connector:
|
||||
# Volume was attached before we began stashing connectors
|
||||
LOG.warning(_LW("Host mismatch detected, but stashed "
|
||||
"volume connector not found. Instance host is "
|
||||
"%(ihost)s, but volume connector host is "
|
||||
"%(chost)s."),
|
||||
{'ihost': instance.host,
|
||||
'chost': connector.get('host')})
|
||||
elif stashed_connector.get('host') != instance.host:
|
||||
# Unexpected error. The stashed connector is also not matching
|
||||
# the needed instance host.
|
||||
LOG.error(_LE("Host mismatch detected in stashed volume "
|
||||
"connector. Will use local volume connector. "
|
||||
"Instance host is %(ihost)s. Local volume "
|
||||
"connector host is %(chost)s. Stashed volume "
|
||||
"connector host is %(schost)s."),
|
||||
{'ihost': instance.host,
|
||||
'chost': connector.get('host'),
|
||||
'schost': stashed_connector.get('host')})
|
||||
else:
|
||||
# Fix found. Use stashed connector.
|
||||
LOG.debug("Host mismatch detected. Found usable stashed "
|
||||
"volume connector. Instance host is %(ihost)s. "
|
||||
"Local volume connector host was %(chost)s. "
|
||||
"Stashed volume connector host is %(schost)s.",
|
||||
{'ihost': instance.host,
|
||||
'chost': connector.get('host'),
|
||||
'schost': stashed_connector.get('host')})
|
||||
connector = stashed_connector
|
||||
|
||||
self.volume_api.terminate_connection(context, volume_id, connector)
|
||||
|
||||
if destroy_bdm:
|
||||
|
|
|
|||
|
|
@ -23,6 +23,7 @@ from mox3 import mox
|
|||
import netaddr
|
||||
from oslo_config import cfg
|
||||
import oslo_messaging as messaging
|
||||
from oslo_serialization import jsonutils
|
||||
from oslo_utils import importutils
|
||||
from oslo_utils import timeutils
|
||||
from oslo_utils import uuidutils
|
||||
|
|
@ -2199,6 +2200,8 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase):
|
|||
bdm.device_name = 'vdb'
|
||||
bdm_get.return_value = bdm
|
||||
|
||||
detach.return_value = {}
|
||||
|
||||
with mock.patch.object(self.compute, 'volume_api') as volume_api:
|
||||
with mock.patch.object(self.compute, 'driver') as driver:
|
||||
connector_sentinel = mock.sentinel.connector
|
||||
|
|
@ -2226,6 +2229,93 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase):
|
|||
else:
|
||||
self.assertFalse(bdm.destroy.called)
|
||||
|
||||
def test_detach_volume_evacuate(self):
|
||||
"""For evacuate, terminate_connection is called with original host."""
|
||||
expected_connector = {'host': 'evacuated-host'}
|
||||
conn_info_str = '{"connector": {"host": "evacuated-host"}}'
|
||||
self._test_detach_volume_evacuate(conn_info_str,
|
||||
expected=expected_connector)
|
||||
|
||||
def test_detach_volume_evacuate_legacy(self):
|
||||
"""Test coverage for evacuate with legacy attachments.
|
||||
|
||||
In this case, legacy means the volume was attached to the instance
|
||||
before nova stashed the connector in connection_info. The connector
|
||||
sent to terminate_connection will still be for the local host in this
|
||||
case because nova does not have the info to get the connector for the
|
||||
original (evacuated) host.
|
||||
"""
|
||||
conn_info_str = '{"foo": "bar"}' # Has no 'connector'.
|
||||
self._test_detach_volume_evacuate(conn_info_str)
|
||||
|
||||
def test_detach_volume_evacuate_mismatch(self):
|
||||
"""Test coverage for evacuate with connector mismatch.
|
||||
|
||||
For evacuate, if the stashed connector also has the wrong host,
|
||||
then log it and stay with the local connector.
|
||||
"""
|
||||
conn_info_str = '{"connector": {"host": "other-host"}}'
|
||||
self._test_detach_volume_evacuate(conn_info_str)
|
||||
|
||||
@mock.patch('nova.objects.BlockDeviceMapping.get_by_volume_and_instance')
|
||||
@mock.patch('nova.compute.manager.ComputeManager.'
|
||||
'_notify_about_instance_usage')
|
||||
def _test_detach_volume_evacuate(self, conn_info_str, notify_inst_usage,
|
||||
bdm_get, expected=None):
|
||||
"""Re-usable code for detach volume evacuate test cases.
|
||||
|
||||
:param conn_info_str: String form of the stashed connector.
|
||||
:param expected: Dict of the connector that is expected in the
|
||||
terminate call (optional). Default is to expect the
|
||||
local connector to be used.
|
||||
"""
|
||||
volume_id = 'vol_id'
|
||||
instance = fake_instance.fake_instance_obj(self.context,
|
||||
host='evacuated-host')
|
||||
bdm = mock.Mock()
|
||||
bdm.connection_info = conn_info_str
|
||||
bdm_get.return_value = bdm
|
||||
|
||||
local_connector = {'host': 'local-connector-host'}
|
||||
expected_connector = local_connector if not expected else expected
|
||||
|
||||
with mock.patch.object(self.compute, 'volume_api') as volume_api:
|
||||
with mock.patch.object(self.compute, 'driver') as driver:
|
||||
driver.get_volume_connector.return_value = local_connector
|
||||
|
||||
self.compute._detach_volume(self.context,
|
||||
volume_id,
|
||||
instance,
|
||||
destroy_bdm=False)
|
||||
|
||||
driver.get_volume_connector.assert_called_once_with(instance)
|
||||
volume_api.terminate_connection.assert_called_once_with(
|
||||
self.context, volume_id, expected_connector)
|
||||
volume_api.detach.assert_called_once_with(mock.ANY,
|
||||
volume_id,
|
||||
instance.uuid,
|
||||
None)
|
||||
notify_inst_usage.assert_called_once_with(
|
||||
self.context, instance, "volume.detach",
|
||||
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)
|
||||
|
|
|
|||
|
|
@ -289,15 +289,22 @@ class CinderApiTestCase(test.NoDBTestCase):
|
|||
|
||||
self.api.detach(self.ctx, 'id1', instance_uuid='fake_uuid')
|
||||
|
||||
def test_initialize_connection(self):
|
||||
cinder.cinderclient(self.ctx).AndReturn(self.cinderclient)
|
||||
self.mox.StubOutWithMock(self.cinderclient.volumes,
|
||||
'initialize_connection',
|
||||
use_mock_anything=True)
|
||||
self.cinderclient.volumes.initialize_connection('id1', 'connector')
|
||||
self.mox.ReplayAll()
|
||||
@mock.patch('nova.volume.cinder.cinderclient')
|
||||
def test_initialize_connection(self, mock_cinderclient):
|
||||
connection_info = {'foo': 'bar'}
|
||||
mock_cinderclient.return_value.volumes. \
|
||||
initialize_connection.return_value = connection_info
|
||||
|
||||
self.api.initialize_connection(self.ctx, 'id1', 'connector')
|
||||
volume_id = 'fake_vid'
|
||||
connector = {'host': 'fakehost1'}
|
||||
actual = self.api.initialize_connection(self.ctx, volume_id, connector)
|
||||
|
||||
expected = connection_info
|
||||
expected['connector'] = connector
|
||||
self.assertEqual(expected, actual)
|
||||
|
||||
mock_cinderclient.return_value.volumes. \
|
||||
initialize_connection.assert_called_once_with(volume_id, connector)
|
||||
|
||||
@mock.patch('nova.volume.cinder.cinderclient')
|
||||
def test_initialize_connection_rollback(self, mock_cinderclient):
|
||||
|
|
|
|||
|
|
@ -395,8 +395,10 @@ class API(object):
|
|||
@translate_volume_exception
|
||||
def initialize_connection(self, context, volume_id, connector):
|
||||
try:
|
||||
return cinderclient(context).volumes.initialize_connection(
|
||||
volume_id, connector)
|
||||
connection_info = cinderclient(
|
||||
context).volumes.initialize_connection(volume_id, connector)
|
||||
connection_info['connector'] = connector
|
||||
return connection_info
|
||||
except cinder_exception.ClientException as ex:
|
||||
with excutils.save_and_reraise_exception():
|
||||
LOG.error(_LE('Initialize connection failed for volume '
|
||||
|
|
|
|||
Loading…
Reference in New Issue