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
(cherry picked from commit d8062063a3)
This commit is contained in:
Alan Bishop 2020-05-18 18:56:43 -07:00
parent d077c4f66f
commit 780986481b
3 changed files with 154 additions and 23 deletions

View File

@ -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 +

View File

@ -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']

View File

@ -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)