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

diff --git a/cinder/tests/test_hp3par.py b/cinder/tests/test_hp3par.py
index 3bd5d90dc402413bf7e3631151c29459ef6ccf4d..8019b767c24af4393ee10e587de6773139ab9066 100644 (file)
--- 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 0ab5f809d0d5d8b1c8f77634ff03ad64a8a1526e..161346684ce22aa5bc18c8890d1a7129e981d556 100644 (file)
--- 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})