3PAR - Fix renaming volume after migration
After migrating a volume, if the original and new volumes are on the same 3PAR backend, then the volume names are swapped. However, if the original volume is not in the same 3PAR backend, the new volume is renamed to the original volume's name. Closes-Bug: 1858119 Change-Id: I1f9d4143d1ad2637a2173090b290f6f96b64c492
This commit is contained in:
Alan Bishop
parent
06c4ab01b8
commit
d8062063a3
@ -18,6 +18,7 @@ import ast
|
||||
import copy
|
||||
from unittest import mock
|
||||
|
||||
import ddt
|
||||
from oslo_config import cfg
|
||||
from oslo_utils import units
|
||||
from oslo_utils import uuidutils
|
||||
@ -818,6 +819,7 @@ class HPE3PARBaseDriver(test.TestCase):
|
||||
result)
|
||||
|
||||
|
||||
@ddt.ddt
|
||||
class TestHPE3PARDriverBase(HPE3PARBaseDriver):
|
||||
|
||||
def setup_driver(self, config=None, mock_conf=None, wsapi_version=None):
|
||||
@ -3165,7 +3167,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'
|
||||
@ -3176,26 +3185,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 +
|
||||
|
||||
@ -2867,30 +2867,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
Block a user