Remove unnecessary metadata from the 3PAR drivers

Currently, both the HP 3PAR iSCSI and FC drivers are populating
the volume and snapshot metadata field with unnecessary data that
should actually be stored on the backend. This data should not be
accessible by the user which it is today and could be updated as well.

This patch will remove all custom 3PAR data from the metadata fields.

Change-Id: I3086be3856192a27fa1db1ada89d95c290c68df7
Fixes: bug 1203144

diff --git a/cinder/tests/test_hp3par.py b/cinder/tests/test_hp3par.py
index 9e6935382e9072ecc62f7e18a6fb38b58ba98963..7b562a7c6e0c4f6481b20a02d8d96966e09e54ba 100644 (file)
--- a/cinder/tests/test_hp3par.py
+++ b/cinder/tests/test_hp3par.py
@@ -18,6 +18,7 @@
 """
 Unit tests for OpenStack Cinder volume drivers
 """
+import ast
 import mox
 import shutil
 import tempfile
@@ -305,7 +306,7 @@ class HP3PARBaseDriver():
     VOLUME_ID_SNAP = '761fc5e5-5191-4ec7-aeba-33e36de44156'
     FAKE_DESC = 'test description name'
     FAKE_FC_PORTS = ['0987654321234', '123456789000987']
-    QOS = "{'qos:maxIOPS': 1000, 'qos:maxBWS': 50}"
+    QOS = {'qos:maxIOPS': '1000', 'qos:maxBWS': '50'}
     FAKE_ISCSI_PORTS = {'1.1.1.2': {'nsp': '8:1:1',
                                     'iqn': ('iqn.2000-05.com.3pardata:'
                                             '21810002ac00383d'),
@@ -383,6 +384,8 @@ class HP3PARBaseDriver():
                        self.fake_create_3par_vlun)
         self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "get_ports",
                        self.fake_get_ports)
+        self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "get_cpg",
+                       self.fake_get_cpg)
         self.stubs.Set(hpfcdriver.hpcommon.HP3PARCommon, "get_domain",
                        self.fake_get_domain)
 
@@ -393,6 +396,9 @@ class HP3PARBaseDriver():
     def fake_create_client(self):
         return FakeHP3ParClient(self.driver.configuration.hp3par_api_url)
 
+    def fake_get_cpg(self, volume):
+        return HP3PAR_CPG
+
     def fake_get_domain(self, cpg):
         return HP3PAR_DOMAIN
 
@@ -440,16 +446,14 @@ class HP3PARBaseDriver():
     def fake_copy_volume(self, src_name, dest_name):
         pass
 
-    def fake_get_volume_state(self, vol_name):
+    def fake_get_volume_stats(self, vol_name):
         return "normal"
 
     def test_create_volume(self):
         self.flags(lock_path=self.tempdir)
-        model_update = self.driver.create_volume(self.volume)
-        metadata = model_update['metadata']
-        self.assertFalse(metadata['3ParName'] is None)
-        self.assertEqual(metadata['CPG'], HP3PAR_CPG)
-        self.assertEqual(metadata['snapCPG'], HP3PAR_CPG_SNAP)
+        self.driver.create_volume(self.volume)
+        volume = self.driver.common.client.getVolume(self.VOLUME_3PAR_NAME)
+        self.assertEqual(volume['name'], self.VOLUME_3PAR_NAME)
 
     def test_create_volume_qos(self):
         self.flags(lock_path=self.tempdir)
@@ -461,12 +465,11 @@ class HP3PARBaseDriver():
         self.stubs.Set(hpdriver.hpcommon.HP3PARCommon,
                        "_add_volume_to_volume_set",
                        self.fake_add_volume_to_volume_set)
-        model_update = self.driver.create_volume(self.volume_qos)
-        metadata = model_update['metadata']
-        self.assertFalse(metadata['3ParName'] is None)
-        self.assertEqual(metadata['CPG'], HP3PAR_CPG)
-        self.assertEqual(metadata['snapCPG'], HP3PAR_CPG_SNAP)
-        self.assertEqual(metadata['qos'], True)
+        self.driver.create_volume(self.volume_qos)
+        volume = self.driver.common.client.getVolume(self.VOLUME_3PAR_NAME)
+
+        self.assertEqual(volume['name'], self.VOLUME_3PAR_NAME)
+        self.assertNotIn(self.QOS, dict(ast.literal_eval(volume['comment'])))
 
     def test_delete_volume(self):
         self.flags(lock_path=self.tempdir)
@@ -475,6 +478,23 @@ class HP3PARBaseDriver():
                           self.driver.common.client.getVolume,
                           self.VOLUME_ID)
 
+    def test_create_cloned_volume(self):
+        self.flags(lock_path=self.tempdir)
+        self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "get_volume_stats",
+                       self.fake_get_volume_stats)
+        self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "_copy_volume",
+                       self.fake_copy_volume)
+        self.state_tries = 0
+        volume = {'name': HP3PARBaseDriver.VOLUME_NAME,
+                  'id': HP3PARBaseDriver.CLONE_ID,
+                  'display_name': 'Foo Volume',
+                  'size': 2,
+                  'host': HP3PARBaseDriver.FAKE_HOST,
+                  'source_volid': HP3PARBaseDriver.VOLUME_ID}
+        src_vref = {}
+        model_update = self.driver.create_cloned_volume(volume, src_vref)
+        self.assertTrue(model_update is not None)
+
     def test_create_snapshot(self):
         self.flags(lock_path=self.tempdir)
         self.driver.create_snapshot(self.snapshot)
@@ -520,12 +540,10 @@ class HP3PARBaseDriver():
         self.stubs.Set(hpdriver.hpcommon.HP3PARCommon,
                        "_add_volume_to_volume_set",
                        self.fake_add_volume_to_volume_set)
-        model_update = self.driver.create_volume_from_snapshot(self.volume_qos,
-                                                               self.snapshot)
-        metadata = model_update['metadata']
-        self.assertEqual(metadata['qos'], True)
+        self.driver.create_volume_from_snapshot(self.volume_qos, self.snapshot)
         snap_vol = self.driver.common.client.getVolume(self.VOLUME_3PAR_NAME)
         self.assertEqual(snap_vol['name'], self.VOLUME_3PAR_NAME)
+        self.assertNotIn(self.QOS, dict(ast.literal_eval(snap_vol['comment'])))
 
         volume = self.volume.copy()
         volume['size'] = 1
@@ -635,25 +653,6 @@ class TestHP3PARFCDriver(HP3PARBaseDriver, test.TestCase):
         self.assertEquals(self.VOLUME_3PAR_NAME, vlun['volumeName'])
         self.assertEquals(self.FAKE_HOST, vlun['hostname'])
 
-    def test_create_cloned_volume(self):
-        self.flags(lock_path=self.tempdir)
-        self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "_copy_volume",
-                       self.fake_copy_volume)
-        self.state_tries = 0
-        volume = {'name': HP3PARBaseDriver.VOLUME_NAME,
-                  'id': HP3PARBaseDriver.CLONE_ID,
-                  'display_name': 'Foo Volume',
-                  'size': 2,
-                  'host': HP3PARBaseDriver.FAKE_HOST,
-                  'source_volid': HP3PARBaseDriver.VOLUME_ID}
-        src_vref = {}
-        model_update = self.driver.create_cloned_volume(volume, src_vref)
-        self.assertTrue(model_update is not None)
-        metadata = model_update['metadata']
-        self.assertFalse(metadata['3ParName'] is None)
-        self.assertEqual(metadata['CPG'], HP3PAR_CPG)
-        self.assertEqual(metadata['snapCPG'], HP3PAR_CPG_SNAP)
-
     def test_get_volume_stats(self):
         self.flags(lock_path=self.tempdir)
 
@@ -687,6 +686,8 @@ class TestHP3PARFCDriver(HP3PARBaseDriver, test.TestCase):
 
         #record
         self.clear_mox()
+        self.stubs.Set(hpfcdriver.hpcommon.HP3PARCommon, "get_cpg",
+                       self.fake_get_cpg)
         self.stubs.Set(hpfcdriver.hpcommon.HP3PARCommon, "get_domain",
                        self.fake_get_domain)
         _run_ssh = self.mox.CreateMock(hpdriver.hpcommon.HP3PARCommon._run_ssh)
@@ -710,6 +711,8 @@ class TestHP3PARFCDriver(HP3PARBaseDriver, test.TestCase):
 
         #record
         self.clear_mox()
+        self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "get_cpg",
+                       self.fake_get_cpg)
         self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "get_domain",
                        self.fake_get_domain)
         _run_ssh = self.mox.CreateMock(hpdriver.hpcommon.HP3PARCommon._run_ssh)
@@ -737,6 +740,8 @@ class TestHP3PARFCDriver(HP3PARBaseDriver, test.TestCase):
 
         #record
         self.clear_mox()
+        self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "get_cpg",
+                       self.fake_get_cpg)
         self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "get_domain",
                        self.fake_get_domain)
         _run_ssh = self.mox.CreateMock(hpdriver.hpcommon.HP3PARCommon._run_ssh)
@@ -839,25 +844,6 @@ class TestHP3PARISCSIDriver(HP3PARBaseDriver, test.TestCase):
         self.assertEquals(self.VOLUME_3PAR_NAME, vlun['volumeName'])
         self.assertEquals(self.FAKE_HOST, vlun['hostname'])
 
-    def test_create_cloned_volume(self):
-        self.flags(lock_path=self.tempdir)
-        self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "_copy_volume",
-                       self.fake_copy_volume)
-        self.state_tries = 0
-        volume = {'name': HP3PARBaseDriver.VOLUME_NAME,
-                  'id': HP3PARBaseDriver.CLONE_ID,
-                  'display_name': 'Foo Volume',
-                  'size': 2,
-                  'host': HP3PARBaseDriver.FAKE_HOST,
-                  'source_volid': HP3PARBaseDriver.VOLUME_ID}
-        src_vref = {}
-        model_update = self.driver.create_cloned_volume(volume, src_vref)
-        self.assertTrue(model_update is not None)
-        metadata = model_update['metadata']
-        self.assertFalse(metadata['3ParName'] is None)
-        self.assertEqual(metadata['CPG'], HP3PAR_CPG)
-        self.assertEqual(metadata['snapCPG'], HP3PAR_CPG_SNAP)
-
     def test_get_volume_stats(self):
         self.flags(lock_path=self.tempdir)
 
@@ -891,6 +877,8 @@ class TestHP3PARISCSIDriver(HP3PARBaseDriver, test.TestCase):
 
         #record
         self.clear_mox()
+        self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "get_cpg",
+                       self.fake_get_cpg)
         self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "get_domain",
                        self.fake_get_domain)
         _run_ssh = self.mox.CreateMock(hpdriver.hpcommon.HP3PARCommon._run_ssh)
@@ -915,6 +903,8 @@ class TestHP3PARISCSIDriver(HP3PARBaseDriver, test.TestCase):
 
         #record
         self.clear_mox()
+        self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "get_cpg",
+                       self.fake_get_cpg)
         self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "get_domain",
                        self.fake_get_domain)
         _run_ssh = self.mox.CreateMock(hpdriver.hpcommon.HP3PARCommon._run_ssh)
@@ -942,6 +932,8 @@ class TestHP3PARISCSIDriver(HP3PARBaseDriver, test.TestCase):
 
         #record
         self.clear_mox()
+        self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "get_cpg",
+                       self.fake_get_cpg)
         self.stubs.Set(hpdriver.hpcommon.HP3PARCommon, "get_domain",
                        self.fake_get_domain)
         _run_ssh = self.mox.CreateMock(hpdriver.hpcommon.HP3PARCommon._run_ssh)

diff --git a/cinder/volume/drivers/san/hp/hp_3par_common.py b/cinder/volume/drivers/san/hp/hp_3par_common.py
index 60bc48d1a16c1730c4ebe3d6744b27235f2c97c3..6f20a65895109f870644ec4d5d88bcc50568ab73 100644 (file)
--- a/cinder/volume/drivers/san/hp/hp_3par_common.py
+++ b/cinder/volume/drivers/san/hp/hp_3par_common.py
@@ -38,7 +38,7 @@ for credentials to talk to the REST service on the 3PAR
 array.
 """
 
-
+import ast
 import base64
 import json
 import paramiko
@@ -653,6 +653,15 @@ exit
     def _remove_volume_from_volume_set(self, volume_name, vvs_name):
         self._cli_run('removevvset -f %s %s' % (vvs_name, volume_name), None)
 
+    def get_cpg(self, volume):
+        volume_name = self._get_3par_vol_name(volume['id'])
+        vol = self.client.getVolume(volume_name)
+        return vol['userCPG']
+
+    def _get_3par_vol_comment(self, volume_name):
+        vol = self.client.getVolume(volume_name)
+        return vol['comment']
+
     def get_persona_type(self, volume, hp3par_keys=None):
         default_persona = self.valid_persona_values[0]
         type_id = volume.get('volume_type_id', None)
@@ -692,7 +701,6 @@ exit
             vvs_name = None
             hp3par_keys = {}
             qos = {}
-            qos_on_volume = False
             type_id = volume.get('volume_type_id', None)
             if type_id is not None:
                 volume_type = self._get_volume_type(type_id)
@@ -700,8 +708,6 @@ exit
                 vvs_name = self._get_key_value(hp3par_keys, 'vvs')
                 if vvs_name is None:
                     qos = self._get_qos_by_volume_type(volume_type)
-                    if qos:
-                        qos_on_volume = True
 
             cpg = self._get_key_value(hp3par_keys, 'cpg',
                                       self.config.hp3par_cpg)
@@ -780,11 +786,6 @@ exit
             LOG.error(str(ex))
             raise exception.CinderException(ex.get_description())
 
-        metadata = {'3ParName': volume_name, 'CPG': cpg,
-                    'snapCPG': extras['snapCPG'], 'qos': qos_on_volume,
-                    'vvs': vvs_name}
-        return metadata
-
     def _copy_volume(self, src_name, dest_name):
         self._cli_run('createvvcopy -p %s %s' % (src_name, dest_name), None)
 
@@ -798,12 +799,10 @@ exit
         word = re.search(search_string.strip(' ') + ' ([^ ]*)', s)
         return word.groups()[0].strip(' ')
 
-    def get_volume_metadata_value(self, volume, key):
-        metadata = volume.get('volume_metadata')
-        if metadata:
-            for i in volume['volume_metadata']:
-                if i['key'] == key:
-                    return i['value']
+    def _get_3par_vol_comment_value(self, vol_comment, key):
+        comment_dict = dict(ast.literal_eval(vol_comment))
+        if key in comment_dict:
+            return comment_dict[key]
         return None
 
     def create_cloned_volume(self, volume, src_vref):
@@ -833,11 +832,12 @@ exit
     def delete_volume(self, volume):
         try:
             volume_name = self._get_3par_vol_name(volume['id'])
-            qos = self.get_volume_metadata_value(volume, 'qos')
-            vvs_name = self.get_volume_metadata_value(volume, 'vvs')
+            vol_comment = self._get_3par_vol_comment(volume_name)
+            qos = self._get_3par_vol_comment_value(vol_comment, 'qos')
+            vvs_name = self._get_3par_vol_comment_value(vol_comment, 'vvs')
             if vvs_name is not None:
                 self._remove_volume_from_volume_set(volume_name, vvs_name)
-            elif qos:
+            elif qos is not None:
                 self._remove_volume_set(self._get_3par_vvs_name(volume['id']))
             self.client.deleteVolume(volume_name)
         except hpexceptions.HTTPNotFound as ex:
@@ -878,7 +878,6 @@ exit
             type_id = volume.get('volume_type_id', None)
             vvs_name = None
             qos = {}
-            qos_on_volume = False
             hp3par_keys = {}
             if type_id is not None:
                 volume_type = self._get_volume_type(type_id)
@@ -886,8 +885,6 @@ exit
                 vvs_name = self._get_key_value(hp3par_keys, 'vvs')
                 if vvs_name is None:
                     qos = self._get_qos_by_volume_type(volume_type)
-                    if qos:
-                        qos_on_volume = True
 
             name = snapshot.get('display_name', None)
             if name:
@@ -920,10 +917,6 @@ exit
             LOG.error(str(ex))
             raise exception.CinderException(ex.get_description())
 
-        metadata = {'3ParName': volume_name, 'qos': qos_on_volume,
-                    'vvs': vvs_name}
-        return metadata
-
     def create_snapshot(self, snapshot):
         LOG.debug("Create Snapshot\n%s" % pprint.pformat(snapshot))
 

diff --git a/cinder/volume/drivers/san/hp/hp_3par_fc.py b/cinder/volume/drivers/san/hp/hp_3par_fc.py
index 0553ae6f019088cfe84cce88961324432a4bf291..e627886d281b058ad7b67df15eed7add63b8a206 100644 (file)
--- a/cinder/volume/drivers/san/hp/hp_3par_fc.py
+++ b/cinder/volume/drivers/san/hp/hp_3par_fc.py
@@ -220,7 +220,7 @@ class HP3PARFCDriver(cinder.volume.driver.FibreChannelDriver):
         """Creates or modifies existing 3PAR host."""
         host = None
         hostname = self.common._safe_hostname(connector['host'])
-        cpg = self.common.get_volume_metadata_value(volume, 'CPG')
+        cpg = self.common.get_cpg(volume)
         domain = self.common.get_domain(cpg)
         try:
             host = self.common._get_3par_host(hostname)

diff --git a/cinder/volume/drivers/san/hp/hp_3par_iscsi.py b/cinder/volume/drivers/san/hp/hp_3par_iscsi.py
index 48db1bee7f5fe389a4ec9c29b0a0537d9db8679e..656ef423a9ebba7801f205b8284486389be17dc7 100644 (file)
--- a/cinder/volume/drivers/san/hp/hp_3par_iscsi.py
+++ b/cinder/volume/drivers/san/hp/hp_3par_iscsi.py
@@ -278,7 +278,7 @@ class HP3PARISCSIDriver(cinder.volume.driver.ISCSIDriver):
         # make sure we don't have the host already
         host = None
         hostname = self.common._safe_hostname(connector['host'])
-        cpg = self.common.get_volume_metadata_value(volume, 'CPG')
+        cpg = self.common.get_cpg(volume)
         domain = self.common.get_domain(cpg)
         try:
             host = self.common._get_3par_host(hostname)