From 59f9ddd9a86d1ead381772ae45fa1a10eae5969b Mon Sep 17 00:00:00 2001 From: Rich Hagarty Date: Wed, 14 Jan 2015 09:11:27 -0800 Subject: [PATCH] HP3Par: Set snapCPG when managing existing volumes 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 --- cinder/tests/test_hp3par.py | 75 +++++++++++++++++-- .../volume/drivers/san/hp/hp_3par_common.py | 18 ++++- 2 files changed, 84 insertions(+), 9 deletions(-) diff --git a/cinder/tests/test_hp3par.py b/cinder/tests/test_hp3par.py index 3bd5d90dc..8019b767c 100644 --- a/cinder/tests/test_hp3par.py +++ b/cinder/tests/test_hp3par.py @@ -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( diff --git a/cinder/volume/drivers/san/hp/hp_3par_common.py b/cinder/volume/drivers/san/hp/hp_3par_common.py index 0ab5f809d..161346684 100644 --- a/cinder/volume/drivers/san/hp/hp_3par_common.py +++ b/cinder/volume/drivers/san/hp/hp_3par_common.py @@ -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}) -- 2.45.2