Merge "3PAR - Fix renaming volume after migration" into stable/stein
This commit is contained in:
commit
0bde875803
|
@ -18,6 +18,7 @@ import mock
|
|||
|
||||
import ast
|
||||
|
||||
import ddt
|
||||
from oslo_utils import units
|
||||
|
||||
from cinder import context
|
||||
|
@ -796,6 +797,7 @@ class HPE3PARBaseDriver(test.TestCase):
|
|||
result)
|
||||
|
||||
|
||||
@ddt.ddt
|
||||
class TestHPE3PARDriverBase(HPE3PARBaseDriver):
|
||||
|
||||
def setup_driver(self, config=None, mock_conf=None, wsapi_version=None):
|
||||
|
@ -3055,7 +3057,14 @@ class TestHPE3PARDriverBase(HPE3PARBaseDriver):
|
|||
expected = []
|
||||
mock_client.assert_has_calls(expected)
|
||||
|
||||
def test_update_migrated_volume(self):
|
||||
@ddt.data({'temp_rename_side_effect': None,
|
||||
'rename_side_effect': None},
|
||||
{'temp_rename_side_effect': hpeexceptions.HTTPNotFound,
|
||||
'rename_side_effect': None})
|
||||
@ddt.unpack
|
||||
def test_update_migrated_volume(self,
|
||||
temp_rename_side_effect,
|
||||
rename_side_effect):
|
||||
mock_client = self.setup_driver()
|
||||
fake_old_volume = {'id': self.VOLUME_ID}
|
||||
provider_location = 'foo'
|
||||
|
@ -3066,26 +3075,100 @@ class TestHPE3PARDriverBase(HPE3PARBaseDriver):
|
|||
with mock.patch.object(hpecommon.HPE3PARCommon,
|
||||
'_create_client') as mock_create_client:
|
||||
mock_create_client.return_value = mock_client
|
||||
mock_client.modifyVolume.side_effect = [
|
||||
temp_rename_side_effect,
|
||||
rename_side_effect,
|
||||
None
|
||||
]
|
||||
actual_update = self.driver.update_migrated_volume(
|
||||
context.get_admin_context(), fake_old_volume,
|
||||
fake_new_volume, original_volume_status)
|
||||
|
||||
expected_update = {'_name_id': None,
|
||||
'provider_location': None}
|
||||
if rename_side_effect is None:
|
||||
expected_update = {'_name_id': None,
|
||||
'provider_location': None}
|
||||
else:
|
||||
expected_update = {'_name_id': fake_new_volume['_name_id'],
|
||||
'provider_location': provider_location}
|
||||
self.assertEqual(expected_update, actual_update)
|
||||
|
||||
# Initial temp rename always takes place
|
||||
expected = [
|
||||
mock.call.modifyVolume(
|
||||
'osv-0DM4qZEVSKON-DXN-NwVpw',
|
||||
{'newName': u'tsv-0DM4qZEVSKON-DXN-NwVpw'}),
|
||||
mock.call.modifyVolume(
|
||||
'osv-0DM4qZEVSKON-AAAAAAAAA',
|
||||
{'newName': u'osv-0DM4qZEVSKON-DXN-NwVpw'}),
|
||||
mock.call.modifyVolume(
|
||||
'tsv-0DM4qZEVSKON-DXN-NwVpw',
|
||||
{'newName': u'osv-0DM4qZEVSKON-AAAAAAAAA'})
|
||||
]
|
||||
|
||||
# Primary rename will occur unless the temp rename fails
|
||||
if temp_rename_side_effect != hpeexceptions.HTTPConflict:
|
||||
expected += [
|
||||
mock.call.modifyVolume(
|
||||
'osv-0DM4qZEVSKON-AAAAAAAAA',
|
||||
{'newName': u'osv-0DM4qZEVSKON-DXN-NwVpw'}),
|
||||
]
|
||||
|
||||
# Final temp rename will occur if both of the previous renames
|
||||
# succeed.
|
||||
if (temp_rename_side_effect is None and
|
||||
rename_side_effect is None):
|
||||
expected += [
|
||||
mock.call.modifyVolume(
|
||||
'tsv-0DM4qZEVSKON-DXN-NwVpw',
|
||||
{'newName': u'osv-0DM4qZEVSKON-AAAAAAAAA'})
|
||||
]
|
||||
|
||||
mock_client.assert_has_calls(
|
||||
self.standard_login +
|
||||
expected +
|
||||
self.standard_logout)
|
||||
|
||||
@ddt.data({'temp_rename_side_effect': hpeexceptions.HTTPConflict,
|
||||
'rename_side_effect': None},
|
||||
{'temp_rename_side_effect': None,
|
||||
'rename_side_effect': hpeexceptions.HTTPConflict},
|
||||
{'temp_rename_side_effect': hpeexceptions.HTTPNotFound,
|
||||
'rename_side_effect': hpeexceptions.HTTPConflict})
|
||||
@ddt.unpack
|
||||
def test_update_migrated_volume_failed(self,
|
||||
temp_rename_side_effect,
|
||||
rename_side_effect):
|
||||
mock_client = self.setup_driver()
|
||||
fake_old_volume = {'id': self.VOLUME_ID}
|
||||
provider_location = 'foo'
|
||||
fake_new_volume = {'id': self.CLONE_ID,
|
||||
'_name_id': self.CLONE_ID,
|
||||
'provider_location': provider_location}
|
||||
original_volume_status = 'available'
|
||||
with mock.patch.object(hpecommon.HPE3PARCommon,
|
||||
'_create_client') as mock_create_client:
|
||||
mock_create_client.return_value = mock_client
|
||||
mock_client.modifyVolume.side_effect = [
|
||||
temp_rename_side_effect,
|
||||
rename_side_effect,
|
||||
None
|
||||
]
|
||||
self.assertRaises(hpeexceptions.HTTPConflict,
|
||||
self.driver.update_migrated_volume,
|
||||
context.get_admin_context(),
|
||||
fake_old_volume,
|
||||
fake_new_volume,
|
||||
original_volume_status)
|
||||
|
||||
# Initial temp rename always takes place
|
||||
expected = [
|
||||
mock.call.modifyVolume(
|
||||
'osv-0DM4qZEVSKON-DXN-NwVpw',
|
||||
{'newName': u'tsv-0DM4qZEVSKON-DXN-NwVpw'}),
|
||||
]
|
||||
|
||||
# Primary rename will occur unless the temp rename fails
|
||||
if temp_rename_side_effect != hpeexceptions.HTTPConflict:
|
||||
expected += [
|
||||
mock.call.modifyVolume(
|
||||
'osv-0DM4qZEVSKON-AAAAAAAAA',
|
||||
{'newName': u'osv-0DM4qZEVSKON-DXN-NwVpw'}),
|
||||
]
|
||||
|
||||
mock_client.assert_has_calls(
|
||||
self.standard_login +
|
||||
expected +
|
||||
|
|
|
@ -2804,30 +2804,70 @@ class HPE3PARCommon(object):
|
|||
|
||||
"""
|
||||
LOG.debug("Update volume name for %(id)s", {'id': new_volume['id']})
|
||||
name_id = None
|
||||
provider_location = None
|
||||
new_volume_renamed = False
|
||||
if original_volume_status == 'available':
|
||||
# volume isn't attached and can be updated
|
||||
original_name = self._get_3par_vol_name(volume['id'])
|
||||
temp_name = self._get_3par_vol_name(volume['id'], temp_vol=True)
|
||||
current_name = self._get_3par_vol_name(new_volume['id'])
|
||||
temp_name = self._get_3par_vol_name(volume['id'], temp_vol=True)
|
||||
|
||||
# In case the original volume is on the same backend, try
|
||||
# renaming it to a temporary name.
|
||||
original_volume_renamed = False
|
||||
try:
|
||||
volumeTempMods = {'newName': temp_name}
|
||||
self.client.modifyVolume(original_name, volumeTempMods)
|
||||
original_volume_renamed = True
|
||||
except hpeexceptions.HTTPNotFound:
|
||||
pass
|
||||
except Exception as e:
|
||||
LOG.error("Changing the original volume name from %(orig)s to "
|
||||
"%(temp)s failed because %(reason)s",
|
||||
{'orig': original_name, 'temp': temp_name,
|
||||
'reason': e})
|
||||
raise
|
||||
|
||||
# change the current volume name to the original
|
||||
try:
|
||||
volumeMods = {'newName': original_name}
|
||||
volumeTempMods = {'newName': temp_name}
|
||||
volumeCurrentMods = {'newName': current_name}
|
||||
# swap volume name in backend
|
||||
self.client.modifyVolume(original_name, volumeTempMods)
|
||||
self.client.modifyVolume(current_name, volumeMods)
|
||||
self.client.modifyVolume(temp_name, volumeCurrentMods)
|
||||
LOG.info("Volume name changed from %(tmp)s to %(orig)s",
|
||||
{'tmp': current_name, 'orig': original_name})
|
||||
new_volume_renamed = True
|
||||
LOG.info("Current volume changed from %(cur)s to %(orig)s",
|
||||
{'cur': current_name, 'orig': original_name})
|
||||
except Exception as e:
|
||||
LOG.error("Changing the volume name from %(tmp)s to "
|
||||
LOG.error("Changing the migrating volume name from %(cur)s to "
|
||||
"%(orig)s failed because %(reason)s",
|
||||
{'tmp': current_name, 'orig': original_name,
|
||||
{'cur': current_name, 'orig': original_name,
|
||||
'reason': e})
|
||||
name_id = new_volume['_name_id'] or new_volume['id']
|
||||
provider_location = new_volume['provider_location']
|
||||
if original_volume_renamed:
|
||||
LOG.error("To restore the original volume, it must be "
|
||||
"manually renamed from %(temp)s to %(orig)s",
|
||||
{'temp': temp_name, 'orig': original_name})
|
||||
raise
|
||||
|
||||
# If it was renamed, rename the original volume again to the
|
||||
# migrated volume's name (effectively swapping the names). If
|
||||
# this operation fails, the newly migrated volume is OK but the
|
||||
# original volume (with the temp name) may need to be manually
|
||||
# cleaned up on the backend.
|
||||
if original_volume_renamed:
|
||||
try:
|
||||
volumeCurrentMods = {'newName': current_name}
|
||||
self.client.modifyVolume(temp_name, volumeCurrentMods)
|
||||
except Exception as e:
|
||||
LOG.error("Changing the original volume name from "
|
||||
"%(tmp)s to %(cur)s failed because %(reason)s",
|
||||
{'tmp': temp_name, 'cur': current_name,
|
||||
'reason': e})
|
||||
# Don't fail the migration, but help the user fix the
|
||||
# source volume stuck in error_deleting.
|
||||
LOG.error("To delete the original volume, it must be "
|
||||
"manually renamed from %(temp)s to %(orig)s",
|
||||
{'temp': temp_name, 'orig': current_name})
|
||||
|
||||
if new_volume_renamed:
|
||||
name_id = None
|
||||
provider_location = None
|
||||
else:
|
||||
# the backend can't change the name.
|
||||
name_id = new_volume['_name_id'] or new_volume['id']
|
||||
|
|
|
@ -0,0 +1,8 @@
|
|||
---
|
||||
fixes:
|
||||
- |
|
||||
Fix the HPE 3PAR driver's attempt to rename the backend volume after
|
||||
it was migrated. If the original volume resides on the same 3PAR backend
|
||||
then the pre and post migration volume names are swapped. Otherwise, the
|
||||
newly migrated volume is renamed to match the original name.
|
||||
(bug 1858119)
|
Loading…
Reference in New Issue