openstack/cinder
Code Issues Proposed changes

HP3Par: Set snapCPG when managing existing volumes

Browse Source 
This fixes a corner case where the user did not assign a "Copy CPG"
value to the volume when originally created on the back-end, and
then the user does not assign a "Volume Type" to the volume
when it is then "managed" into OpenStack.

This results in an undefined "snapCPG" value which means no
snapshots can be created for the volume.

The fix is to set the "snapCPG" value to match the "User CPG"
value associated with the volume when the volume is "managed"

Change-Id: I207d462e12e4db67bff8624bc69e7e6f1ceff75b
Closes-Bug: #1393609
This commit is contained in:
Rich Hagarty 2015-01-14 09:11:27 -08:00
parent ce98efea33
commit 59f9ddd9a8
2 changed files with 84 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

75
cinder/tests/test_hp3par.py
View File

@ -305,6 +305,12 @@ class HP3PARBaseDriver(object):
'comment': "{'display_name': 'Foo Volume'}"
}
MV_INFO_WITH_NO_SNAPCPG = {
'userCPG': 'testUserCpg0',
'provisioningType': 1,
'comment': "{'display_name': 'Foo Volume'}"
}
RETYPE_TEST_COMMENT = "{'retype_test': 'test comment'}"
RETYPE_VOLUME_INFO_0 = {
@ -2214,6 +2220,55 @@ class HP3PARBaseDriver(object):
self.standard_logout)
self.assertEqual(expected_obj, obj)
@mock.patch.object(volume_types, 'get_volume_type')
def test_manage_existing_with_no_snap_cpg(self, _mock_volume_types):
_mock_volume_types.return_value = self.volume_type
mock_client = self.setup_driver()
new_comment = {"display_name": "Foo Volume",
"name": "volume-007dbfce-7579-40bc-8f90-a20b3902283e",
"volume_id": "007dbfce-7579-40bc-8f90-a20b3902283e",
"type": "OpenStack"}
volume = {'display_name': None,
'host': 'my-stack1@3parxxx#CPGNOTUSED',
'volume_type': 'gold',
'volume_type_id': 'acfa9fa4-54a0-4340-a3d8-bfcf19aea65e',
'id': '007dbfce-7579-40bc-8f90-a20b3902283e'}
mock_client.getVolume.return_value = self.MV_INFO_WITH_NO_SNAPCPG
mock_client.modifyVolume.return_value = ("anyResponse", {'taskid': 1})
mock_client.getTask.return_value = self.STATUS_DONE
with mock.patch.object(hpcommon.HP3PARCommon,
'_create_client') as mock_create_client:
mock_create_client.return_value = mock_client
common = self.driver._login()
unm_matcher = common._get_3par_unm_name(self.volume['id'])
osv_matcher = common._get_3par_vol_name(volume['id'])
existing_ref = {'source-name': unm_matcher}
expected_obj = {'display_name': 'Foo Volume',
'host': 'my-stack1@3parxxx#fakepool'}
obj = self.driver.manage_existing(volume, existing_ref)
expected_manage = [
mock.call.getVolume(existing_ref['source-name']),
mock.call.modifyVolume(
existing_ref['source-name'],
{'newName': osv_matcher,
'comment': self.CommentMatcher(self.assertEqual,
new_comment),
# manage_existing() should be setting
# blank snapCPG to the userCPG
'snapCPG': 'testUserCpg0'})
]
mock_client.assert_has_calls(self.standard_login + expected_manage)
self.assertEqual(expected_obj, obj)
@mock.patch.object(volume_types, 'get_volume_type')
def test_manage_existing_vvs(self, _mock_volume_types):
test_volume_type = self.RETYPE_VOLUME_TYPE_2
@ -2304,7 +2359,8 @@ class HP3PARBaseDriver(object):
'volume_type_id': None,
'id': '007dbfce-7579-40bc-8f90-a20b3902283e'}
mock_client.getVolume.return_value = {'comment': comment}
mock_client.getVolume.return_value = {'comment': comment,
'userCPG': 'testUserCpg0'}
with mock.patch.object(hpcommon.HP3PARCommon,
'_create_client') as mock_create_client:
@ -2321,7 +2377,10 @@ class HP3PARBaseDriver(object):
mock.call.getVolume(existing_ref['source-name']),
mock.call.modifyVolume(existing_ref['source-name'],
{'newName': osv_matcher,
'comment': new_comment})
'comment': new_comment,
# manage_existing() should be setting
# blank snapCPG to the userCPG
'snapCPG': 'testUserCpg0'})
]
mock_client.assert_has_calls(
@ -2339,7 +2398,10 @@ class HP3PARBaseDriver(object):
mock.call.getVolume(existing_ref['source-name']),
mock.call.modifyVolume(existing_ref['source-name'],
{'newName': osv_matcher,
'comment': new_comment})
'comment': new_comment,
# manage_existing() should be setting
# blank snapCPG to the userCPG
'snapCPG': 'testUserCpg0'})
]
mock_client.assert_has_calls(
@ -2348,7 +2410,7 @@ class HP3PARBaseDriver(object):
self.standard_logout)
self.assertEqual(expected_obj, obj)
mock_client.getVolume.return_value = {}
mock_client.getVolume.return_value = {'userCPG': 'testUserCpg0'}
volume['display_name'] = None
common = self.driver._login()
@ -2359,7 +2421,10 @@ class HP3PARBaseDriver(object):
mock.call.getVolume(existing_ref['source-name']),
mock.call.modifyVolume(existing_ref['source-name'],
{'newName': osv_matcher,
'comment': new_comment})
'comment': new_comment,
# manage_existing() should be setting
# blank snapCPG to the userCPG
'snapCPG': 'testUserCpg0'})
]
mock_client.assert_has_calls(

18
cinder/volume/drivers/san/hp/hp_3par_common.py
View File

@ -163,10 +163,11 @@ class HP3PARCommon(object):
2.0.32 - Update LOG usage to fix translations. bug #1384312
2.0.33 - Fix host persona to match WSAPI mapping bug #1403997
2.0.34 - Fix log messages to match guidelines. bug #1411370
2.0.35 - Fix default snapCPG for manage_existing bug #1393609
"""
VERSION = "2.0.34"
VERSION = "2.0.35"
stats = {}
@ -367,10 +368,19 @@ class HP3PARCommon(object):
volume['volume_type_id'])
raise exception.ManageExistingVolumeTypeMismatch(reason=reason)
new_vals = {'newName': new_vol_name,
'comment': json.dumps(new_comment)}
# Ensure that snapCPG is set
if 'snapCPG' not in vol:
new_vals['snapCPG'] = vol['userCPG']
LOG.info(_LI("Virtual volume %(disp)s '%(new)s' snapCPG "
"is empty so it will be set to: %(cpg)s"),
{'disp': display_name, 'new': new_vol_name,
'cpg': new_vals['snapCPG']})
# Update the existing volume with the new name and comments.
self.client.modifyVolume(target_vol_name,
{'newName': new_vol_name,
'comment': json.dumps(new_comment)})
self.client.modifyVolume(target_vol_name, new_vals)
LOG.info(_LI("Virtual volume '%(ref)s' renamed to '%(new)s'."),
{'ref': existing_ref['source-name'], 'new': new_vol_name})