Merge "3PAR - Fix renaming volume after migration"
This commit is contained in:
commit
f2215075d4
|
@ -18,6 +18,7 @@ import ast
|
||||||
import copy
|
import copy
|
||||||
from unittest import mock
|
from unittest import mock
|
||||||
|
|
||||||
|
import ddt
|
||||||
from oslo_config import cfg
|
from oslo_config import cfg
|
||||||
from oslo_utils import units
|
from oslo_utils import units
|
||||||
from oslo_utils import uuidutils
|
from oslo_utils import uuidutils
|
||||||
|
@ -818,6 +819,7 @@ class HPE3PARBaseDriver(test.TestCase):
|
||||||
result)
|
result)
|
||||||
|
|
||||||
|
|
||||||
|
@ddt.ddt
|
||||||
class TestHPE3PARDriverBase(HPE3PARBaseDriver):
|
class TestHPE3PARDriverBase(HPE3PARBaseDriver):
|
||||||
|
|
||||||
def setup_driver(self, config=None, mock_conf=None, wsapi_version=None):
|
def setup_driver(self, config=None, mock_conf=None, wsapi_version=None):
|
||||||
|
@ -3165,7 +3167,14 @@ class TestHPE3PARDriverBase(HPE3PARBaseDriver):
|
||||||
expected = []
|
expected = []
|
||||||
mock_client.assert_has_calls(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()
|
mock_client = self.setup_driver()
|
||||||
fake_old_volume = {'id': self.VOLUME_ID}
|
fake_old_volume = {'id': self.VOLUME_ID}
|
||||||
provider_location = 'foo'
|
provider_location = 'foo'
|
||||||
|
@ -3176,26 +3185,100 @@ class TestHPE3PARDriverBase(HPE3PARBaseDriver):
|
||||||
with mock.patch.object(hpecommon.HPE3PARCommon,
|
with mock.patch.object(hpecommon.HPE3PARCommon,
|
||||||
'_create_client') as mock_create_client:
|
'_create_client') as mock_create_client:
|
||||||
mock_create_client.return_value = mock_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(
|
actual_update = self.driver.update_migrated_volume(
|
||||||
context.get_admin_context(), fake_old_volume,
|
context.get_admin_context(), fake_old_volume,
|
||||||
fake_new_volume, original_volume_status)
|
fake_new_volume, original_volume_status)
|
||||||
|
|
||||||
expected_update = {'_name_id': None,
|
if rename_side_effect is None:
|
||||||
'provider_location': 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)
|
self.assertEqual(expected_update, actual_update)
|
||||||
|
|
||||||
|
# Initial temp rename always takes place
|
||||||
expected = [
|
expected = [
|
||||||
mock.call.modifyVolume(
|
mock.call.modifyVolume(
|
||||||
'osv-0DM4qZEVSKON-DXN-NwVpw',
|
'osv-0DM4qZEVSKON-DXN-NwVpw',
|
||||||
{'newName': u'tsv-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(
|
mock_client.assert_has_calls(
|
||||||
self.standard_login +
|
self.standard_login +
|
||||||
expected +
|
expected +
|
||||||
|
|
|
@ -2867,30 +2867,70 @@ class HPE3PARCommon(object):
|
||||||
|
|
||||||
"""
|
"""
|
||||||
LOG.debug("Update volume name for %(id)s", {'id': new_volume['id']})
|
LOG.debug("Update volume name for %(id)s", {'id': new_volume['id']})
|
||||||
name_id = None
|
new_volume_renamed = False
|
||||||
provider_location = None
|
|
||||||
if original_volume_status == 'available':
|
if original_volume_status == 'available':
|
||||||
# volume isn't attached and can be updated
|
# volume isn't attached and can be updated
|
||||||
original_name = self._get_3par_vol_name(volume['id'])
|
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'])
|
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:
|
try:
|
||||||
volumeMods = {'newName': original_name}
|
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(current_name, volumeMods)
|
||||||
self.client.modifyVolume(temp_name, volumeCurrentMods)
|
new_volume_renamed = True
|
||||||
LOG.info("Volume name changed from %(tmp)s to %(orig)s",
|
LOG.info("Current volume changed from %(cur)s to %(orig)s",
|
||||||
{'tmp': current_name, 'orig': original_name})
|
{'cur': current_name, 'orig': original_name})
|
||||||
except Exception as e:
|
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",
|
"%(orig)s failed because %(reason)s",
|
||||||
{'tmp': current_name, 'orig': original_name,
|
{'cur': current_name, 'orig': original_name,
|
||||||
'reason': e})
|
'reason': e})
|
||||||
name_id = new_volume['_name_id'] or new_volume['id']
|
if original_volume_renamed:
|
||||||
provider_location = new_volume['provider_location']
|
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:
|
else:
|
||||||
# the backend can't change the name.
|
# the backend can't change the name.
|
||||||
name_id = new_volume['_name_id'] or new_volume['id']
|
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